ROB-589 Steer call_aws JSON-argument commands to the reliable no-JSON path (cut ~87s of retries) - #2270
Conversation
The AWS API MCP server tokenizes call_aws commands with shlex.split, so a JSON
argument written without shell quotes (e.g. --filter {"Dimensions":...}) has its
double quotes stripped and AWS receives invalid JSON. In a real investigation the
model burned ~87s on this: it wrote unquoted JSON, then tried to write the JSON to
a file / mkdir (no writable filesystem in the MCP sandbox), and even concluded "I
can't use single quotes in this tool" — which is false.
shlex.split preserves double quotes inside a single-quoted token, so the fix is
simply to single-quote JSON arguments. Add a prominent section to the AWS MCP
default llm_instructions covering: single-quote JSON args (with right/wrong
example), don't attempt file/dir workarounds, prefer --group-by shorthand +
client-side aggregation, and use suggest_aws_commands when unsure.
No MCP server code change needed — this is model guidance. The underlying shlex
behavior is standard; an upstream tool-description hint would remove the need for
this downstream, tracked separately.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FonHqnDVjnkM9qoa7gdM9v
Signed-off-by: Claude <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughThis change adds an AWS MCP instruction block in the Helm template that explains how to pass JSON arguments to ChangesAWS MCP call_aws Instructions
Estimated code review effort: 1 (Trivial) | ~3 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Docker images ready for
Use these tags to pull the images for testing. 📋 Copy commandsgcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:8e888cda0
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes:8e888cda0 me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:8e888cda0
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-dev:8e888cda0
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:8e888cda0
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/holmes-operator:8e888cda0 me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:8e888cda0
docker push me-west1-docker.pkg.dev/robusta-development/development/holmes-operator-dev:8e888cda0Patch Helm values in one line (choose the chart you use): HolmesGPT chart: helm upgrade --install holmesgpt ./helm/holmes \
--set registry=me-west1-docker.pkg.dev/robusta-development/development \
--set image=holmes-dev:8e888cda0 \
--set operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set operator.image=holmes-operator-dev:8e888cda0Robusta wrapper chart: helm upgrade --install robusta robusta/robusta \
--reuse-values \
--set holmes.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.image=holmes-dev:8e888cda0 \
--set holmes.operator.registry=me-west1-docker.pkg.dev/robusta-development/development \
--set holmes.operator.image=holmes-operator-dev:8e888cda0 |
✅ Deploy Preview for holmes-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
helm/holmes/templates/mcp-servers/aws/_helpers.tpl (1)
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse placeholders instead of fixed dates in the example.
The hard-coded
2026-01-01/2026-07-08dates will age quickly and make the guidance stale. A placeholder or relative-date example would keep the instruction accurate.♻️ Proposed fix
- `aws ce get-cost-and-usage --time-period Start=2026-01-01,End=2026-07-08 --granularity MONTHLY --metrics UnblendedCost --filter '{"Dimensions":{"Key":"SERVICE","Values":["Amazon Bedrock"]}}'` + `aws ce get-cost-and-usage --time-period Start=<START_DATE>,End=<END_DATE> --granularity MONTHLY --metrics UnblendedCost --filter '{"Dimensions":{"Key":"SERVICE","Values":["Amazon Bedrock"]}}'`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@helm/holmes/templates/mcp-servers/aws/_helpers.tpl` at line 17, Replace the hard-coded dates in the AWS Cost Explorer example inside the helper template with placeholders or a relative-date example so the guidance stays current. Update the example command in the AWS helper content to use variable or generic date values instead of fixed values, keeping the rest of the `aws ce get-cost-and-usage` example unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@helm/holmes/templates/mcp-servers/aws/_helpers.tpl`:
- Line 17: Replace the hard-coded dates in the AWS Cost Explorer example inside
the helper template with placeholders or a relative-date example so the guidance
stays current. Update the example command in the AWS helper content to use
variable or generic date values instead of fixed values, keeping the rest of the
`aws ce get-cost-and-usage` example unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c3dfe149-66b9-49f4-9aab-7bd3b7df847c
📒 Files selected for processing (1)
helm/holmes/templates/mcp-servers/aws/_helpers.tpl
Production repro (Opus 4.6, baseline without these instructions) showed the model flailing ~87s on call_aws: repeated inline-JSON --filter attempts with stripped quotes, then echo/mkdir file workarounds (no writable fs), then a single-quote attempt that still failed, before finally falling back to a dimension query. So "just single-quote the JSON" is necessary-but-not- sufficient guidance. Rewrite the AWS MCP llm_instructions to lead with the robust path: 1. Prefer shorthand that needs no JSON (--group-by Type=DIMENSION,Key=SERVICE) and filter/aggregate client-side — sidesteps the quoting problem entirely. 2. Do NOT attempt filesystem workarounds (mkdir/echo/redirect/file://) — the MCP sandbox is read-only; that was pure wasted steps in the repro. 3. Only if a JSON arg is truly needed, single-quote the whole value (no double quotes, no backslash-escaping); if it still fails, fall back to #1 instead of retrying variations. 4. Use suggest_aws_commands when unsure. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FonHqnDVjnkM9qoa7gdM9v Signed-off-by: Claude <noreply@anthropic.com>
The filesystem workaround guidance (do not mkdir/redirect/--filter file://) is being handled globally in the core system prompt and bash toolset (ROB-591), so it no longer belongs in the AWS-specific instructions. Keep the AWS section focused on the call_aws JSON-quoting problem and its --group-by workaround. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FonHqnDVjnkM9qoa7gdM9v Signed-off-by: Claude <noreply@anthropic.com>
… path (cut ~87s of retries) (#2270) ## Problem In a real Ask-Holmes investigation ("show me graphs of AWS Bedrock spend mom…"), the model burned **~87 s (54% of the turn's LLM time)** fighting the `call_aws` tool before it could retrieve any AWS Cost Explorer data. **Reproduced live on production (Opus 4.6) — see the trace below.** The flailing sequence (baseline, without this change): 1. `aws ce get-cost-and-usage … --filter <inline JSON>` → error (⚠️ ) 2. retry → error 3. `echo '{...}' > /tmp/aws-api-mcp/workdir/bedrock.json` → error (no writable filesystem) 4. `mkdir -p /tmp/aws-api-mcp/workdir && echo …` → error 5. another `--filter` attempt → error; reasoning: *"the tool is stripping double quotes from the JSON"* 6. a single-quote attempt → still error 7. eventually falls back to a dimension/`--query` approach and gets data Root cause: the AWS API MCP server (`aws-api-mcp-server`, upstream `awslabs.aws-api-mcp-server`) tokenizes the command with `shlex.split()`. Unquoted JSON loses its double quotes. But the repro shows single-quoting alone isn't a dependable fix in practice — the model also wastes steps on filesystem workarounds and quote-variation retries. Not an awslabs code bug (standard shell tokenization) — a usage/guidance problem. ## Fix Rewrite the AWS MCP default `llm_instructions` (`helm/holmes/templates/mcp-servers/aws/_helpers.tpl`) to steer toward the **reliable no-JSON path**, in priority order: 1. **Prefer shorthand that needs no JSON** — `--group-by Type=DIMENSION,Key=SERVICE` and filter/aggregate client-side. Sidesteps the quoting problem entirely; this is what the model eventually fell back to anyway. 2. **Do NOT attempt filesystem workarounds** (`mkdir`/`echo`/redirect/`file://`) — the MCP sandbox is read-only. (Directly kills steps 3–4 above.) 3. **Only if a JSON arg is truly needed**, single-quote the whole value (no double quotes, no backslash-escaping); if it still fails, fall back to #1 instead of retrying variations. 4. Use `suggest_aws_commands` when unsure. No MCP server code change — model guidance only. ## Validation status (honest) The behavior is **confirmed reproduced on production** (baseline). A controlled sandbox harness under-reproduced it (current Opus single-quotes fine in a minimal setup), so the reliable test for this change is **deploying it and re-running the same prompt** — the shorthand-first guidance should eliminate the flailing. Not yet re-run with the change applied. ## Context Linear: ROB-589. One of two independent slowdowns found in one slow chat: ~87 s AWS-tool friction (this PR) and a ~30–60 s visualization retry loop (fixed in robusta-frontend#3369 / FRO-175, which has a deterministic client-side repair). ## Follow-up (upstream DX, out of scope) `awslabs/mcp` `call_aws` could add a single-quoting hint to its tool description or return an actionable error on bare-JSON tokens. Draft note prepared. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added a new AWS MCP instruction block with guidance for using JSON arguments reliably, including required single-quote wrapping rules and avoidance of double-quotes/backslash escaping. * Provided an example using a JSON-based `--filter`. * Clarified a safer decision flow: prefer shorthand options and client-side filtering/aggregation, use command suggestions first when unsure, and only then execute via `call_aws`. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Arik Alon <alon.arik@gmail.com>
…2280) Follow-up to #2270. The single-quote guidance for `call_aws` JSON arguments (Cost Explorer `--filter`, `--cli-input-json`, CloudWatch `--metric-data-queries`, etc.) didn't hold up in practice — the model second-guessed it and flailed, still producing invalid JSON. This switches the AWS MCP `llm_instructions` (option 2 of the "READ THIS FIRST" section) to recommend **escaped double-quoted** JSON — the shlex-valid form models emit most reliably: ``` --filter "{\"Dimensions\":{\"Key\":\"SERVICE\",\"Values\":[\"Amazon Bedrock\"]}}" ``` It also explicitly warns against the unescaped `"{"Dimensions":...}"` form (quotes get stripped). `--group-by` + client-side aggregation stays as the primary/most-reliable option 1. Linear: https://linear.app/robusta/issue/ROB-597/aws-mcp-steer-call-aws-json-args-to-escaped-double-quotes 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Updated AWS command guidance for passing JSON arguments: wrap the full JSON string in outer double quotes and escape inner double quotes to preserve correct shell parsing. * Removed the older single-quote example that could lead to command errors. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: alonelish <alon.elish@gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Problem
In a real Ask-Holmes investigation ("show me graphs of AWS Bedrock spend mom…"), the model burned ~87 s (54% of the turn's LLM time) fighting the
call_awstool before it could retrieve any AWS Cost Explorer data. Reproduced live on production (Opus 4.6) — see the trace below.The flailing sequence (baseline, without this change):
aws ce get-cost-and-usage … --filter <inline JSON>→ error (echo '{...}' > /tmp/aws-api-mcp/workdir/bedrock.json→ error (no writable filesystem)mkdir -p /tmp/aws-api-mcp/workdir && echo …→ error--filterattempt → error; reasoning: "the tool is stripping double quotes from the JSON"--queryapproach and gets dataRoot cause: the AWS API MCP server (
aws-api-mcp-server, upstreamawslabs.aws-api-mcp-server) tokenizes the command withshlex.split(). Unquoted JSON loses its double quotes. But the repro shows single-quoting alone isn't a dependable fix in practice — the model also wastes steps on filesystem workarounds and quote-variation retries. Not an awslabs code bug (standard shell tokenization) — a usage/guidance problem.Fix
Rewrite the AWS MCP default
llm_instructions(helm/holmes/templates/mcp-servers/aws/_helpers.tpl) to steer toward the reliable no-JSON path, in priority order:--group-by Type=DIMENSION,Key=SERVICEand filter/aggregate client-side. Sidesteps the quoting problem entirely; this is what the model eventually fell back to anyway.mkdir/echo/redirect/file://) — the MCP sandbox is read-only. (Directly kills steps 3–4 above.)suggest_aws_commandswhen unsure.No MCP server code change — model guidance only.
Validation status (honest)
The behavior is confirmed reproduced on production (baseline). A controlled sandbox harness under-reproduced it (current Opus single-quotes fine in a minimal setup), so the reliable test for this change is deploying it and re-running the same prompt — the shorthand-first guidance should eliminate the flailing. Not yet re-run with the change applied.
Context
Linear: ROB-589. One of two independent slowdowns found in one slow chat: ~87 s AWS-tool friction (this PR) and a ~30–60 s visualization retry loop (fixed in robusta-frontend#3369 / FRO-175, which has a deterministic client-side repair).
Follow-up (upstream DX, out of scope)
awslabs/mcpcall_awscould add a single-quoting hint to its tool description or return an actionable error on bare-JSON tokens. Draft note prepared.🤖 Generated with Claude Code
Summary by CodeRabbit
--filter.call_aws.