Skip to content

feat(dsql): Add system diagnostics workflow (Workflow 12) - #207

Merged
krokoko merged 16 commits into
awslabs:mainfrom
Morlej:feat/dsql-system-diagnostics
Jul 17, 2026
Merged

feat(dsql): Add system diagnostics workflow (Workflow 12)#207
krokoko merged 16 commits into
awslabs:mainfrom
Morlej:feat/dsql-system-diagnostics

Conversation

@Morlej

@MorlejMorlej commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Adds CloudWatch AAS-based system diagnostics to the DSQL skill as Workflow 12.

What it does

Uses PromQL queries against db.active_sessions.avg OTel metrics to run a mandatory full diagnostic sweep across 5 phases:

  1. Discovery & Baseline Comparison — wait event distribution shifts (current vs yesterday vs last week)
  2. Top-SQL Regression Detection — queries that are new or growing in the top-N
  3. Workload Attribution — application/role changes
  4. Commit & OCC Analysis — distinguish volume growth from conflict growth via CW metrics
  5. Inflection Point Detection — pinpoint when the change occurred

Routes identified queries to Workflow 9 (Query Plan Explainability) for per-query investigation.

OTel attribute naming

Uses the naming convention:

  • db.wait.event, db.wait.class, db.session.state
  • db.query.id, db.query.normalized_text
  • aws.auroradsql.session.role.arn, application.name

Key design decisions

  • No absolute AAS thresholds — only relative distribution shifts (>30% change flagged)
  • A single slow query = max 1 AAS; high AAS = high concurrency/frequency
  • All per-query recommendations deferred to Workflow 9 (EXPLAIN analysis)
  • Agent MUST execute all phases — no stopping at first finding
  • Performance Routing table prevents bypass via other workflows
  • PromQL discovery requires match parameter (documented as critical rule)
  • Requires CloudWatch MCP server in the same region as the DSQL cluster

Files

  • references/system-diagnostics/workflow.md — 5-phase diagnostic procedure
  • references/system-diagnostics/wait-events.md — canonical DSQL wait event reference
  • references/system-diagnostics/promql-patterns.md — reusable PromQL templates
  • .mcp.json — adds cloudwatch MCP server (disabled by default)
  • SKILL.md — updated description, tags, reference table, Workflow 12, Performance Routing table

Eval coverage

  • trigger_evals.json — 3 Workflow 12 triggering cases (cluster performing differently, wait-event/AAS breakdown, "where is compute time spent"); now 37 cases (22 trigger / 15 no-trigger)
  • evals.json — functional eval chore(doc): add troubleshooting guide #15: AAS wait-event interpretation (identifies the shifted event vs baseline, rules out proportional load growth, makes no absolute-AAS claim, defers per-query root cause to Workflow 9); now 15 prompts / 54 assertions
  • README counts corrected and evals 13–15 documented

Versioning

Bumped databases-on-aws to 1.6.0 (plugin.json, codex plugin.json, marketplace.json). This resolves a version collision: EF Core PR #208 landed on main and took 1.5.0, which this branch had also claimed for Workflow 12. 1.6.0 places the diagnostics feature on top of current main.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.


Update: observe-only boundary hardening

Follow-up from dogfooding Workflow 12 against a live cluster. The diagnostic ran correctly, but the observe-only rule (defer per-query root cause to Workflow 9) was phrased narrowly as "don't recommend index/schema changes" — which let a different mistake through: restating a db.wait.event label as an EXPLAIN diagnosis, e.g. calling SequentialScanRead a "full table scan" and inferring a missing/still-building index. A live Workflow 9 pass disproved it — the top SequentialScanRead query was a 1.78 ms Index Only Scan on the primary key, no missing index.

Changes

  • workflow.md — added an explicit principle: a wait-event label is not an EXPLAIN node type; MUST NOT infer scan type, index state, or per-query root cause from AAS. Constrained the Workflow 9 handoff to describe only the observation (share vs baseline), never a hypothesized cause.
  • wait-events.md — added an observe-only guardrail banner; reframed the read/IO event sections (SequentialScanRead, ScatteredBatchRead, SingleRead) from "Root causes / Remediation" to "Possible causes (confirm in Workflow 9)" + "Observe-only steps".
  • evals.json — added regression eval chore(doc): update agents.md and security scanners workflow trigger #16 (asserts the agent does not claim a full scan or a missing index, and routes to Workflow 9); realigned eval chore(doc): add troubleshooting guide #15 so it rewards observing the concentrated wait-event shift rather than diagnosing a "plan regression" from CloudWatch data alone. The original chore(doc): add troubleshooting guide #15 wording had itself encoded the mistake this change fixes.

Eval results

Functional evals 15 and 16: 5/5 each (LLM-judge grader).

Morlej added 4 commits June 29, 2026 14:18
Add CloudWatch AAS-based system diagnostics to the DSQL skill. Uses
PromQL queries against db.active_sessions.avg to detect temporal anomalies
in wait event distribution and identify regressed queries, then routes to
Workflow 9 (Query Plan Explainability) for per-query investigation.
OTel attribute names use the proposed naming convention:
- db.wait.event, db.wait.class, db.session.state
- db.query.id, db.query.normalized_text
- aws.auroradsql.session.role.arn, application.name
New files:
- references/system-diagnostics/workflow.md — 6 diagnostic sub-workflows
- references/system-diagnostics/wait-events.md — canonical wait event reference
- references/system-diagnostics/promql-patterns.md — reusable PromQL templates
Also:
- Adds cloudwatch MCP server to .mcp.json (disabled by default)
- Bumps plugin version to 1.5.0
Add a decision table before Common Workflows that routes performance
complaints to Workflow 12 (System Diagnostics) instead of allowing
them to fall through to Workflow 9 (Query Plan Explainability) directly.
Rule: when in doubt, start with Workflow 12 — it identifies specific
queries and routes to Workflow 9 with context.
- Use correct get_promql_label_values syntax with match parameter
- Add note that calls without match filter return empty
- Add PromQL syntax rules: quote labels with dots/@, use __name__ selector
- Add explicit discovery step to Workflow 1
- Fix promql-patterns.md to show actual tool parameter names (label_name, match)
Replace separate numbered workflows (1-6) with a single diagnostic
procedure of 5 mandatory phases. The agent MUST execute ALL phases
before presenting results — no stopping at the first finding.
Phases:
1. Discovery and Baseline Comparison (distribution shifts)
2. Top-SQL Regression Detection (new/growing queries)
3. Workload Attribution (application/role changes)
4. Commit and OCC Analysis (volume vs conflicts)
5. Inflection Point Detection (when did it change)
Adds 'Presenting Results' section mandating a unified report across
all dimensions before handoff to Workflow 9.
@Morlej
Morlej marked this pull request as ready for review June 29, 2026 20:46
@Morlej
Morlej requested review from a team as code ownersJune 29, 2026 20:46
@krokoko

Copy link
Copy Markdown
Contributor

Thanks @Morlej !
Quick notes: it seems some checks are failing (build and security scanners), could you please have a look ?
Also, another PR got merged on this plugin, would need to fix the conflicts.
Thank you !

Morlejand others added 4 commits July 8, 2026 09:28
Signed-off-by: James Morle <7480305+Morlej@users.noreply.github.com>
Workflow 12 (System Diagnostics) required the CloudWatch MCP server but
gave no way to enable it, and agents that hit unavailable PromQL tools
had no recovery guidance. Address the observed failure modes:
- Add a CloudWatch MCP Server section to mcp/mcp-setup.md: enabling the
shipped-disabled server, AWS_REGION/AWS_PROFILE config, the
PromQL-enabled region limitation, and the session restart needed for
its tools to register (a mid-session enable shows "Connected" but
exposes no callable tools until restart).
- Add AWS_REGION/AWS_PROFILE placeholders to the disabled cloudwatch
entry in .mcp.json so users see what to fill in.
- workflow.md: add an explicit "If the PromQL tools are unavailable"
callout in Prerequisites (enable + restart, no fabrication/CLI
fallback) and a matching Error Handling row, so a blocked agent
surfaces the fix rather than only reporting missing data.
- Document the discovery-window gotcha: get_promql_label_values
defaults to a window ending "now" and returns empty for older data;
pass explicit start/end. An empty result means "no data in window,"
not "no such cluster."
- Fix "6 diagnostic workflows/sub-workflows" -> "5 diagnostic phases"
in SKILL.md (both the reference table and Workflow 12 section).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolves the 1.5.0 version collision with EF Core PR awslabs#208 (both this
branch's system-diagnostics work and awslabs#208 independently claimed 1.5.0).
Bumps to 1.6.0 across plugin.json, codex plugin.json, and marketplace.json.
Adds eval coverage for Workflow 12 (system diagnostics), which previously
had none: 3 triggering cases and 1 functional AAS-interpretation eval.
Corrects stale eval README counts (37 trigger cases, 15 functional
prompts / 54 assertions) and documents evals 13-15.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@krokoko

Copy link
Copy Markdown
Contributor

@awslabs/agent-plugins-dsql could anyone review ? Thanks !

Morlejand others added 2 commits July 8, 2026 18:15
Workflow 12 (system diagnostics) must observe via CloudWatch and defer
all per-query root cause to Workflow 9. In practice it was easy to
restate a db.wait.event label as an EXPLAIN diagnosis — e.g. calling
SequentialScanRead a "full table scan" or inferring a missing/building
index — which a live investigation disproved (the top SequentialScanRead
query was a 1.78ms Index Only Scan on the primary key).
- workflow.md: add an explicit principle that a wait-event label is not
an EXPLAIN node type; MUST NOT infer scan type, index state, or root
cause from it. Constrain the Workflow 9 handoff to describe only the
observation, never a hypothesized cause.
- wait-events.md: add an observe-only guardrail banner; reframe the
read/IO event sections (SequentialScanRead, ScatteredBatchRead,
SingleRead) from "Root causes/Remediation" to "Possible causes
(confirm in Workflow 9)" + "Observe-only steps".
- evals.json: add regression eval 16 (asserts no full-scan/missing-index
claim, routes to Workflow 9); realign eval 15 so it rewards observing
the concentrated wait-event shift rather than diagnosing a "plan
regression" from CloudWatch data alone.
Functional evals 15 and 16: 5/5 each.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@praba2210

praba2210 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Code review — Workflow 12 (System Diagnostics)

Solid feature, and the observe-only hardening is the right instinct. The two Blocking items land Phase 4 data and template parsing; the rest are small.

AreaFixSev
workflow.md#L191-L212 (+promql-patterns:204-205, wait-events:189)Use namespace="AWS/AuroraDSQL" in the Phase-4 get_metric_data calls. Verified live: the AWS/-prefixed namespace returns metrics; bare "AuroraDSQL" returns [], so Phase 4 currently receives no data. Metric names and ClusterId dimension are already correct.Blocking
promql-patterns.md#L54-L96Quote the dotted metric name inside the braces{__name__="db.active_sessions.avg", …} — in the ~11 bare-form templates (54,60,66,72,78,84,90,96,109,118,127). CloudWatch PromQL (Prometheus 3.0 spec) requires it; also matches the file's own rule at workflow.md:25.Blocking
README.md#L84Set the counts to "16 prompts / 59 assertions" (lines 17 & 84) and add a "What it checks" row for eval #16. evals.json at HEAD has 16 / 59; current "15 / 54" is evals 1–15 only.Fix
mcp-setup.md#L139List the required permissions as cloudwatch:GetMetricData + cloudwatch:ListMetrics. The CloudWatch PromQL docs name those two actions; aps:* belongs to Amazon Managed Prometheus (a separate service), and ListMetrics is currently absent.Fix
wait-events.md#L79Route the ClientWrite result-size question to Workflow 9, keeping the client-side steps (throughput / TCP buffers). This matches the read/IO events already hardened to "confirm in Workflow 9"; "Add LIMIT / pagination" is a query rewrite that workflow.md:18 reserves for Workflow 9.Fix
wait-events.md#L88-L99State the observe-only boundary once per file, affirmatively, and link to it (e.g. "A wait event reports where time was spent; Workflow 9's EXPLAIN ANALYZE establishes scan type / index state / root cause"). It currently repeats ~9× and re-prints the labels the agent should avoid emitting; one statement per file suffices.Style
workflow.md#L233Mark the 7-day limit with MUST to match its MUST/SHOULD peers in the Critical-rules list.Nit
workflow.md#L101Keep the >30% trigger in one place — Phase-1 Step 7 (line 92) already states it imperatively.Nit
workflow.md#L231Use 900s (6–24h) in both files — workflow.md:231 says > 6h, promql-patterns.md:103 says 6–24h.Nit

Applies review feedback from praba2210 on PR awslabs#207.
Blocking:
- Phase 4 Commit/OCC analysis used the wrong CloudWatch namespace
("AuroraDSQL"), which returns no datapoints. Corrected to
"AWS/AuroraDSQL" in workflow.md, promql-patterns.md, and wait-events.md.
- Quoted the dotted metric name as {__name__="db.active_sessions.avg", ...}
in all 11 bare-form PromQL templates in promql-patterns.md, per the
CloudWatch/Prometheus 3.0 spec and the file set's own rule.
Fixes:
- README eval counts: 15/54 -> 16/59, plus a "What it checks" row for
eval 16.
- mcp-setup.md: list cloudwatch:GetMetricData + cloudwatch:ListMetrics as
the required permissions (dropped the incorrect aps reference — that is
Amazon Managed Prometheus, a different service).
- wait-events.md ClientWrite: route the result-size question (LIMIT /
pagination is a query rewrite) to Workflow 9; keep client-side steps.
Style/nits:
- Consolidated the per-section observe-only caveat to defer to the
file-level guardrail banner.
- MUST on the 7-day range limit; de-duplicated the >30% trigger; aligned
the 900s step guidance (6-24h) across both files.
Functional evals 15 and 16: 5/5 each.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Morlej

Copy link
Copy Markdown
ContributorAuthor

Thanks @praba2210 — thorough review, and all nine items verified and applied in c55b04a.

Blocking

  • Phase-4 namespaceAWS/AuroraDSQL in workflow.md (×3), promql-patterns.md, wait-events.md. Confirmed your finding independently: with the bare AuroraDSQL namespace get_metric_data returns empty datapoints, so Phase 4 was silently receiving no data.
  • Dotted metric-name quoting → all 11 bare templates in promql-patterns.md now use {__name__="db.active_sessions.avg", …} (also quoted the dotted @resource… group-by labels), matching the rule at workflow.md:25.

Fix

  • README counts16 prompts / 59 assertions (lines 17 & 84) + a "What it checks" row for eval 16.
  • mcp-setup.md perms → now cloudwatch:GetMetricData + cloudwatch:ListMetrics; dropped the aps reference (Amazon Managed Prometheus, different service).
  • ClientWrite → result-size reduction (LIMIT/pagination) now routed to Workflow 9 as a query rewrite; client-side steps (throughput/TCP buffers, GC/IO) retained.

Style/Nit

  • Consolidated the per-section observe-only caveat to defer to the file-level guardrail banner.
  • MUST on the 7-day range limit; de-duplicated the >30% trigger (kept in Phase-1 Step 7); aligned the step guidance to 900s (6–24h) in both files.

Functional evals 15 and 16 both pass 5/5 after the changes. Branch is also up to date with main.

Follow-up to the Workflow 12 review: apply the same __name__ / dotted-label
quoting to the 9 illustrative "Identify query" snippets in wait-events.md
that the review scoped to the promql-patterns.md templates. Uses the
{__name__="db.active_sessions.avg", "db.wait.event"=...} form and quotes the
bare db.query.normalized_text group-by labels, so every PromQL example in the
system-diagnostics reference set is copy-paste-correct and internally
consistent.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@praba2210

praba2210 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Thanks @praba2210 — thorough review, and all nine items verified and applied in c55b04a.

Blocking

  • Phase-4 namespaceAWS/AuroraDSQL in workflow.md (×3), promql-patterns.md, wait-events.md. Confirmed your finding independently: with the bare AuroraDSQL namespace get_metric_data returns empty datapoints, so Phase 4 was silently receiving no data.
  • Dotted metric-name quoting → all 11 bare templates in promql-patterns.md now use {__name__="db.active_sessions.avg", …} (also quoted the dotted @resource… group-by labels), matching the rule at workflow.md:25.

Fix

  • README counts16 prompts / 59 assertions (lines 17 & 84) + a "What it checks" row for eval 16.
  • mcp-setup.md perms → now cloudwatch:GetMetricData + cloudwatch:ListMetrics; dropped the aps reference (Amazon Managed Prometheus, different service).
  • ClientWrite → result-size reduction (LIMIT/pagination) now routed to Workflow 9 as a query rewrite; client-side steps (throughput/TCP buffers, GC/IO) retained.

Style/Nit

  • Consolidated the per-section observe-only caveat to defer to the file-level guardrail banner.
  • MUST on the 7-day range limit; de-duplicated the >30% trigger (kept in Phase-1 Step 7); aligned the step guidance to 900s (6–24h) in both files.

Functional evals 15 and 16 both pass 5/5 after the changes. Branch is also up to date with main.

Re-checked at 95cff54 — 8 of 9 confirmed applied, thanks for the quick turnaround.

One spot to finish #1: add the AWS/ prefix in the remaining three get_metric_data references so they match the workflow.md fix

  • promql-patterns.md:204-205 (the Commit/OCC snippets)
  • wait-events.md:189 ("standard CloudWatch metrics: AWS/AuroraDSQL namespace")

Using namespace="AWS/AuroraDSQL" there returns the metrics (verified live); the bare form comes back empty, same as the workflow.md calls did before the fix. Everything else looks great!

@Morlej

Copy link
Copy Markdown
ContributorAuthor

Iterative self-review (multi-agent) — findings

Ran the /pr-review-toolkit:review-pr + /code-review:code-review workflow: 4 finder sub-agents across correctness / cross-file-consistency / manifest / docs angles, consolidated, then validated each finding against the live clusterybty7yn4hivsmgt6gvkxzcxsdy and authoritative AWS docs. Manifest/version/JSON angle came back clean. 15 findings confirmed (fixing all), 1 refuted.

Blocking — functional bugs (verified against the live cluster)

  1. Wrong CloudWatch namespace (wait-events.md:189, promql-patterns.md:204-205): bare AuroraDSQL returns 0 metrics; AWS/AuroraDSQL returns all 19. My earlier fix corrected workflow.md but missed these two files. Phase 4 silently gets no data.
  2. Missing statistic="Sum" (promql-patterns.md:204-205): TotalTransactions/OccConflicts are counters — live check shows Average=1.0 vs Sum=9.1M/hr. Default AVG makes the commit/OCC analysis meaningless.
  3. Non-RFC3339 relative times (workflow.md:108,114,120,281; promql-patterns.md:226): start="NOW-1h" etc. The API rejects these with 400 Bad Request (hit first-hand); contradicts the file's own MUST … RFC 3339 rule at line 231. Every Phase-1 example breaks verbatim.
  4. Phase-4 get_metric_data omits start_time/end_time (workflow.md:201, promql-patterns.md:204): defaults to last 3h, can't align with the multi-day AAS baselines it's compared against.

Important — observe-only doctrine leaks (the hardening pass only reached 4 of 10 wait-event sections)

  1. Prescriptive remediations remain: PgSleep "remove the pg_sleep() call", ClientRead "set idle_in_transaction_session_timeout/audit pool", plus Remediation headings on OnCpu/ClientRead/FkExistenceCheck/UniqueConstraintCheck/Commit/PgSleep — direct change recommendations from an observe-only workflow (violates workflow.md:18).
  2. Root causes headings on 7 sections contradict the line-5 guardrail that calls them "Possible causes … candidates to confirm in Workflow 9".
  3. "Defer to dsql skill" (4 sections) vs "Hand off to Workflow 9" (elsewhere) — inconsistent handoff wording.

Minor

  1. get_promql_label_values discovery templates (promql-patterns.md:13-45) omit start/end — contradicts workflow.md:87,99.
  2. Literal ... ellipsis inside wait-events PromQL snippets — invalid if pasted verbatim.
  3. 3 wait-events queries group only by db.query.normalized_text, omitting db.query.id (violates workflow.md:139 MUST).
  4. wait-events.md:5 links to "Important principles" which is bold text, not a heading — no resolvable anchor.
  5. README.md:110: "LLM judge (evals 6-9)" — actually evals 6-16.
  6. evals.json eval 15: expectation says "12%→35% of total AAS", but from the numbers the prompt gives the shares are 18.6%→47.4% (the 3.7x clause is correct).
  7. SKILL.md:3: description dropped "EF Core" from the ORM list while the reference table and Workflow 11 title still include it, and evals 13/14 test it.
  8. Step-size guidance framed as SHOULD in workflow.md but plain text in promql-patterns.md.

Refuted

  • IAM permissions in mcp-setup.md: cloudwatch:GetMetricData + cloudwatch:ListMetrics is complete per AWS PromQL docs (label/series endpoints need only ListMetrics). No change.

Fixing all 15 confirmed items now.

Multi-agent self-review (validated against a live DSQL cluster + AWS docs)
surfaced 15 confirmed findings; this fixes all of them.
Functional bugs (verified live):
- Wrong CloudWatch namespace in wait-events.md and promql-patterns.md
("AuroraDSQL" returns 0 metrics; "AWS/AuroraDSQL" returns all 19). The
earlier fix only reached workflow.md.
- promql-patterns.md Phase-4 get_metric_data omitted statistic="Sum"
(TotalTransactions/OccConflicts are counters — default AVG reports ~1.0)
and start_time/end_time (defaults to last 3h, misaligning with baselines).
- Non-RFC3339 relative times (NOW-1h etc.) in workflow.md and
promql-patterns.md examples — the API rejects these; contradicted the
file's own "MUST ... RFC 3339" rule. Replaced with placeholder timestamps
and a compute-the-window note.
- Discovery get_promql_label_values templates now document the required
start/end for non-recent data.
Observe-only doctrine (the hardening pass had reached only 4 of 10 sections):
- Reframed OnCpu, ClientRead, ClientWrite, FkExistenceCheck,
UniqueConstraintCheck, Commit, PgSleep from "Root causes"/"Remediation"
to "Possible causes (confirm in Workflow 9)"/"Observe-only steps".
- Removed prescriptive fixes (remove pg_sleep(), set GUCs, audit pool) —
now surfaced as candidate causes for the owner to confirm.
- Unified handoff wording to "hand off to Workflow 9".
- Added db.query.id to the Fk/Unique/Single/OnCpu grouping queries
(workflow.md:139 MUST).
- Added a snippet convention note explaining the "..." cluster-filter
placeholder.
Docs:
- README grader-modes: LLM judge is evals 6-16 (was "6-9").
- SKILL.md description: restored "EF Core" to the ORM list.
- eval 15: corrected the derivable shares to ~19%->~47% (89/478 vs
336/709) and made the percentage non-mandatory; kept the correct 3.7x.
- Aligned step-size guidance as SHOULD across both files.
Refuted (no change): mcp-setup.md IAM permissions — GetMetricData +
ListMetrics is complete per AWS PromQL docs.
Build clean; functional evals 15 and 16 pass 5/5.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Morlej

Copy link
Copy Markdown
ContributorAuthor

Self-review findings resolved — commit 6a8d3c3

All 15 confirmed findings fixed; build clean; functional evals 15 & 16 pass 5/5.

Blocking (functional, verified live against the cluster):

  1. ✅ Namespace → AWS/AuroraDSQL in wait-events.md:189 + promql-patterns.md:204-205.
  2. ✅ Added statistic="Sum" to the Phase-4 counter metrics (promql-patterns.md).
  3. ✅ Replaced NOW-1h/NOW-24h literals with placeholder RFC3339 windows + a "compute the window first" note (workflow.md ×2 blocks, promql-patterns.md idle-detection).
  4. ✅ Added start_time/end_time to Phase-4 get_metric_data in both files, with a note that it defaults to 3h.

Important (observe-only doctrine — extended hardening to the remaining 6 of 10 sections):
5. ✅ OnCpu, ClientRead, ClientWrite, FkExistenceCheck, UniqueConstraintCheck, Commit, PgSleep reframed to "Possible causes (confirm in Workflow 9)" + "Observe-only steps"; dropped prescriptive fixes (remove pg_sleep(), set GUCs, audit pool).
6. ✅ All "Root causes" headings gone.
7. ✅ Unified "Defer to dsql skill" → "hand off to Workflow 9".

Minor:
8. ✅ Discovery label-value templates now document the required start/end.
9. ✅ Added a snippet-convention note explaining the ... cluster-filter placeholder.
10. ✅ Added db.query.id to the Fk/Unique/Single/OnCpu grouping queries (workflow.md:139 MUST).
11. ✅ Fixed the "Important principles" link (no longer points at a non-existent anchor).
12. ✅ README grader-modes: "evals 6-16" (was 6-9).
13. ✅ eval 15: shares corrected to the derivable 19%→47% (89/478 vs 336/709), percentage made non-mandatory, 3.7x kept.
14. ✅ SKILL.md description: restored "EF Core" to the ORM list.
15. ✅ Step-size guidance aligned as SHOULD across both files.

Refuted (no change): mcp-setup.md IAM permissions — cloudwatch:GetMetricData + cloudwatch:ListMetrics is complete per the AWS PromQL docs.

…cluster
The aurora-dsql MCP server binds a single cluster at startup (--cluster_endpoint
is a launch flag; the db tools take no per-call endpoint), so using it against a
different cluster means editing .mcp.json and restarting the session. Guide the
skill to only use the MCP tools when the server already targets the intended
cluster, and otherwise prefer the CLI + psql path (scripts/psql-connect.sh)
rather than silently reconfiguring.
- SKILL.md: new concise "Choosing How to Connect: MCP vs CLI/psql" decision
rule, plus a Quick Start step 0 pointing to it. Kept SKILL.md under the
300-line target by keeping the section brief and linking to the reference.
- references/auth/connectivity-tools.md: full "Ad-hoc Queries: MCP vs CLI/psql"
subsection with the decision rule and psql-connect.sh examples.
- mcp-setup.md: note that --cluster_endpoint is a per-instance startup flag and
to prefer CLI/psql over reconfiguring; contrasts with the per-call CloudWatch
MCP.
Doc-only change; build + cross-ref lint clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Morlej

Copy link
Copy Markdown
ContributorAuthor

Added: MCP-vs-CLI connection guidance (050dccb)

The aurora-dsql MCP server binds a single cluster at startup (--cluster_endpoint is a launch flag; the db tools take no per-call endpoint), so pointing it at another cluster requires editing .mcp.json and restarting the session. The skill now guides toward the lower-friction path:

  • SKILL.md — new concise "Choosing How to Connect: MCP vs CLI/psql" section with the decision rule (use MCP only when it already targets your cluster; otherwise use scripts/psql-connect.sh; if unsure, confirm or default to CLI), plus a Quick Start step 0 pointing to it. Kept SKILL.md under the 300-line target by linking detail out.
  • references/auth/connectivity-tools.md — full "Ad-hoc Queries: MCP vs CLI/psql" subsection with the rule and psql-connect.sh examples.
  • mcp-setup.md — note that --cluster_endpoint is a per-instance startup flag; contrasts with the per-call CloudWatch MCP.

Doc-only; build + cross-ref lint clean. Branch remains current with base.

@Morlej

Copy link
Copy Markdown
ContributorAuthor

Iterative self-review — round 2 (multi-agent)

Re-ran the /pr-review-toolkit:review-pr + /code-review:code-review workflow on the current branch (now including the round-1 fixes + the new connectivity-guidance commit). 4 finder sub-agents, consolidated, then validated against the live cluster and the actual psql-connect.sh source. The docs/eval angle came back clean (round-1 fixes verified correct: eval counts 16/59, grader range 6-16, eval-15 shares 19%→47%, EF Core restored). 9 findings confirmed (fixing), 2 refuted, 2 left as intentional.

Blocking — the new connectivity commit introduced two accuracy bugs

  1. connectivity-tools.md: "read-only credentials by default" is false.psql-connect.sh defaults to USER=admin (full DDL/DML). --admin only switches the IAM token action, not SQL privileges. An agent trusting "read-only by default" could run --command "DELETE ..." believing it safe. Verified against the script + scripts/README.md ("Uses admin user by default").
  2. CloudWatch "one config serves any cluster" needs reconciling. Verified live: the per-call region arg is honored (querying us-east-1's cluster with region=us-west-2 returns empty). But my wording sits in tension with mcp-setup.md's "server must run in the same region as the DSQL cluster." Harmonizing to: you must query in the region where the cluster's metrics live; the per-call region lets one server do that for any PromQL-enabled region.

Important

  1. development-guide.md:12 unconditional "PREFER MCP tools for ad-hoc queries" contradicts the new conditional rule; an agent loading it but not the new section uses MCP against whatever cluster it targets. Adding a cross-reference.
  2. workflow.md:3 opening sentence still says the skill uses "the dsql skill's database tools for root cause analysis" — the pre-hardening framing the observe-only pass was meant to remove.
  3. promql-patterns.md: 3 of 4 discovery get_promql_label_values templates omit start/end, contradicting the caveat added right above them (only "List clusters" got the fix).
  4. workflow.md Phase 5 example uses stale 2024-01-15..2024-01-22 dates (inconsistent era; a 7-day span that can trip the tool's "max 7-day incl. lookback" limit).

Minor

  1. wait-events.md FkExistenceCheck step references TotalTransactions without the AWS/AuroraDSQL + statistic="Sum" reminder.
  2. workflow.md:35 prereq wording blurs the W9 boundary; 9. residual "defer to Workflow 9" not unified to "hand off"; 10. placeholder tokens lack a local "concrete RFC3339" note.

Refuted / intentional (no change)

  • Unquoted dimensions=[{name:...}] keys — illustrative pseudocode, consistent throughout.
  • "~3.7x" vs 3.78x in eval 15 — grader explicitly tolerates ("do NOT require a specific" figure); text says "roughly".
  • ClientRead/PgSleep guardrail parenthetical omitting "in Workflow 9" — intentional; these are client-side, not query-plan handoffs.

Fixing the 9 confirmed items now.

…y, doctrine)
Second multi-agent self-review pass (validated against the live cluster and
the actual psql-connect.sh source). 9 confirmed findings fixed.
Connectivity guidance (bugs introduced by the prior connectivity commit):
- connectivity-tools.md: removed the false "read-only credentials by default"
claim. psql-connect.sh connects as the `admin` DB user by default (full
read/write/DDL); --admin only changes the IAM token action, not SQL
privileges. Now states this explicitly and points to --user for least
privilege. Verified against the script + scripts/README.md.
- Reconciled the CloudWatch "one config serves any cluster" wording across
SKILL.md, connectivity-tools.md, and mcp-setup.md with the existing
"Region matters" note: the per-call `region` arg IS honored (verified live
— querying an us-east-1 cluster with region=us-west-2 returns empty), so one
server can query any PromQL-enabled region by passing the region per call;
AWS_REGION only sets the default.
- development-guide.md: the unconditional "PREFER MCP tools for ad-hoc queries"
and "MUST ALWAYS Execute DIRECTLY using MCP" now cross-reference the
conditional "only when the MCP targets the intended cluster" rule.
Observe-only doctrine:
- workflow.md opening sentence no longer says the skill does "root cause
analysis" via db tools — it observes and hands off to Workflow 9.
- Clarified the aurora-dsql MCP prerequisite is for the Workflow 9 handoff,
not this workflow; unified "defer to" -> "hand off to Workflow 9".
PromQL/CloudWatch:
- Added start/end to the 3 remaining discovery get_promql_label_values
templates (only the first had it).
- Phase 5 example: replaced stale 2024 dates + exactly-7-day span with
placeholder window + a note to keep the span under the 7-day (incl.
lookback) limit.
- FkExistenceCheck: added the AWS/AuroraDSQL + statistic="Sum" reminder to the
TotalTransactions reference.
- Added a "must be concrete RFC3339" note to the Range Queries section.
Refuted / left intentional: unquoted dimensions pseudocode; "~3.7x" (grader
tolerates); ClientRead/PgSleep client-side guardrail phrasing.
Build + cross-ref lint clean; functional evals 15 and 16 pass 5/5.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@Morlej

Copy link
Copy Markdown
ContributorAuthor

Round-2 findings resolved — commit e8f8be4

All 9 confirmed findings fixed; build + cross-ref lint clean; functional evals 15 & 16 pass 5/5 (an initial 4/5 on eval 16 was LLM-judge run variance — confirmed 5/5 on an isolated re-run; no eval files were touched this round).

Blocking:

  1. ✅ Removed the false "read-only credentials by default" note — psql-connect.sh connects as admin (full DDL/DML) by default; now says so and points to --user for least privilege. Verified against the script + scripts/README.md.
  2. ✅ Reconciled the CloudWatch region wording across SKILL.md / connectivity-tools.md / mcp-setup.md. Verified live that the per-call region arg is honored (us-east-1 cluster queried with region=us-west-2 → empty), so "one server, any PromQL-enabled region via per-call region" — and the "Region matters" note no longer contradicts it.

Important:
3. ✅ development-guide.md "PREFER MCP" / "MUST ALWAYS Execute DIRECTLY using MCP" now cross-reference the conditional "only when the MCP targets the intended cluster" rule.
4. ✅ workflow.md opening sentence no longer claims the skill does "root cause analysis" — it observes and hands off to Workflow 9.
5. ✅ Added start/end to the 3 remaining discovery get_promql_label_values templates.
6. ✅ Phase 5 example: stale 2024 dates + exactly-7-day span → placeholder window + a keep-under-7-days note.

Minor:
7. ✅ FkExistenceCheck TotalTransactions ref now carries the AWS/AuroraDSQL + statistic="Sum" reminder.
8. ✅ Clarified the aurora-dsql MCP prerequisite is for the W9 handoff; 9. ✅ unified "defer to" → "hand off to Workflow 9"; added an RFC3339 note to the Range Queries section.

Refuted / intentional: unquoted dimensions pseudocode (consistent illustrative notation); "~3.7x" vs 3.78x (grader tolerates, text says "roughly"); ClientRead/PgSleep client-side guardrail phrasing (intentional — not query-plan handoffs).

Comment threadplugins/databases-on-aws/skills/dsql/mcp/mcp-setup.md
Comment threadplugins/databases-on-aws/skills/dsql/mcp/mcp-setup.md
Comment threadplugins/databases-on-aws/skills/dsql/mcp/mcp-setup.md

@anwesham-labanwesham-lab 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.

Left comments, but I don't consider them blocking to intention, the skill still matches behavior as expected in evals and isn't absurd on token consumption.

@krokoko
krokoko enabled auto-merge July 17, 2026 23:12
@krokoko
krokoko added this pull request to the merge queueJul 17, 2026
Merged via the queue into awslabs:main with commit d2822e9Jul 17, 2026
22 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Morlej@krokoko@praba2210@anwesham-lab