fix(building-dashboards): validate chart IDs to prevent corrupted (#31)

* fix(building-dashboards): validate chart IDs to prevent corrupted dashboards

Charts missing `id` fields or with mismatched layout `i` references
cause dashboards to enter a permanently corrupted UI state (blank,
unable to save/revert, stuck edit panels). This was reported by a
customer using Claude Code with the build-dashboards skill.

Changes:
- dashboard-validate: add check for charts missing `id` field
- dashboard-validate: promote chart/layout ID mismatch from warning to error
- dashboard-create: run validation before deploying
- dashboard-update: run validation before deploying
- SKILL.md: add explicit "Required Chart Structure" section documenting
  that every chart must have a unique `id` and every layout `i` must match

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: address review feedback — validate before jq, detect duplicate IDs

- Move dashboard-validate call before BODY=$(jq ...) in both
  dashboard-create and dashboard-update so malformed JSON gets the
  validator's friendly error instead of a raw jq crash
- Add duplicate chart ID detection (check #6) — two charts sharing
  the same id with matching layout entries previously passed silently

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(building-dashboards): keep deploy script stdout machine-readable

Amp-Thread-ID: https://ampcode.com/threads/T-019c9eee-3c59-721b-82d3-fcb245ed29ef
Co-authored-by: Amp <amp@ampcode.com>

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Tomás Senart <tsenart@gmail.com>
Co-authored-by: Amp <amp@ampcode.com>

Author: 4 people

skills/building-dashboards/SKILL.md

@@ -100,6 +100,30 @@ The console uses `react-grid-layout` which requires `minH`, `minW`, `moved`, and
 
 ---
 
+## Required Chart Structure
+
+**Every chart MUST have a unique `id` field.** Every layout entry's `i` field MUST reference a chart `id`. Missing or mismatched IDs will corrupt the dashboard in the UI (blank state, unable to save/revert).
+
+```json
+{
+  "charts": [
+    {
+      "id": "error-rate",
+      "name": "Error Rate",
+      "type": "Statistic",
+      "query": { "apl": "..." }
+    }
+  ],
+  "layout": [
+    {"i": "error-rate", "x": 0, "y": 0, "w": 3, "h": 2}
+  ]
+}
+```
+
+Use descriptive kebab-case IDs (e.g. `error-rate`, `p95-latency`, `traffic-rps`). The `dashboard-validate` and deploy scripts enforce this automatically.
+
+---
+
 ## Chart Types
 
 **Note:** Dashboard queries inherit time from the UI picker—no explicit `_time` filter needed.

skills/building-dashboards/scripts/dashboard-create

@@ -27,21 +27,19 @@ if [[ ! -f "$JSON_FILE" ]]; then
     exit 1
 fi
 
+# Validate dashboard structure before deploying
+if ! "$SCRIPT_DIR/dashboard-validate" "$JSON_FILE" --strict >&2; then
+    echo "Error: Dashboard validation failed. Fix the errors above before deploying." >&2
+    exit 1
+fi
+
 # Read, strip server-managed fields, and normalize layout for react-grid-layout
 BODY=$(jq -L "$SCRIPT_DIR" '
   include "dashboard-normalize";
   del(.id, .version, .createdAt, .updatedAt, .createdBy, .updatedBy) |
   normalize_dashboard_layout
 ' "$JSON_FILE")
 
-# Validate required fields
-for field in name owner; do
-    if [[ $(echo "$BODY" | jq -r ".$field // empty") == "" ]]; then
-        echo "Error: Missing required field: $field" >&2
-        exit 1
-    fi
-done
-
 RESPONSE=$("$SCRIPT_DIR/axiom-api" "$DEPLOYMENT" POST "/internal/dashboards" "$BODY")
 
 # Extract and print the new dashboard ID

skills/building-dashboards/scripts/dashboard-update

@@ -29,6 +29,12 @@ if [[ ! -f "$JSON_FILE" ]]; then
     exit 1
 fi
 
+# Validate dashboard structure before deploying
+if ! "$SCRIPT_DIR/dashboard-validate" "$JSON_FILE" --strict >&2; then
+    echo "Error: Dashboard validation failed. Fix the errors above before deploying." >&2
+    exit 1
+fi
+
 # Normalize layout for react-grid-layout
 BODY=$(jq -L "$SCRIPT_DIR" '
   include "dashboard-normalize";

skills/building-dashboards/scripts/dashboard-validate

@@ -6,9 +6,11 @@
 # Checks:
 #   1. Valid JSON structure
 #   2. Required fields present (name, owner, charts, layout)
-#   3. Chart IDs match layout IDs
-#   4. LogStream queries have take limits
-#   5. Grid layout doesn't exceed 12 columns
+#   3. All charts have an id field
+#   4. No duplicate chart IDs
+#   5. Chart IDs match layout IDs
+#   6. LogStream queries have take limits
+#   7. Grid layout doesn't exceed 12 columns
 #
 # Options:
 #   --strict  Treat warnings as errors
@@ -84,37 +86,54 @@ if [[ $(jq -r '.layout // "null"' "$FILE") == "null" ]]; then
     error "Missing layout array"
 fi
 
-# 5. Check chart IDs match layout IDs
-chart_ids=$(jq -r '.charts[].id' "$FILE" 2>/dev/null | sort)
+# 5. Check all charts have an id field
+missing_ids=$(jq -r '.charts | to_entries[] | select(.value.id == null or .value.id == "") | .key' "$FILE" 2>/dev/null || true)
+if [[ -n "$missing_ids" ]]; then
+    for idx in $missing_ids; do
+        chart_name=$(jq -r ".charts[$idx].name // \"(unnamed)\"" "$FILE")
+        error "Chart at index $idx ($chart_name) is missing required 'id' field"
+    done
+fi
+
+# 6. Check for duplicate chart IDs
+duplicate_ids=$(jq -r '[.charts[].id // empty] | group_by(.) | map(select(length > 1) | .[0]) | .[]' "$FILE" 2>/dev/null | sort -u || true)
+if [[ -n "$duplicate_ids" ]]; then
+    for dup_id in $duplicate_ids; do
+        error "Duplicate chart id: '$dup_id'"
+    done
+fi
+
+# 7. Check chart IDs match layout IDs
+chart_ids=$(jq -r '.charts[].id // empty' "$FILE" 2>/dev/null | sort)
 layout_ids=$(jq -r '.layout[].i' "$FILE" 2>/dev/null | sort)
 
 if [[ "$chart_ids" != "$layout_ids" ]]; then
-    warn "Chart IDs and layout IDs don't match"
+    error "Chart IDs and layout IDs don't match"
     echo "  Charts: $(echo $chart_ids | tr '\n' ' ')"
     echo "  Layout: $(echo $layout_ids | tr '\n' ' ')"
 fi
 
-# 6. Check LogStream queries for take limits
+# 8. Check LogStream queries for take limits
 logstream_without_take=$(jq -r '.charts[] | select(.type == "LogStream") | select(.query.apl != null) | select(.query.apl | test("take ") | not) | .id' "$FILE" 2>/dev/null || true)
 if [[ -n "$logstream_without_take" ]]; then
     for id in $logstream_without_take; do
         warn "LogStream '$id' may be missing 'take N' limit"
     done
 fi
 
-# 7. Check grid width (should be 12 columns max)
+# 9. Check grid width (should be 12 columns max)
 max_right=$(jq '[.layout[] | (.x + .w)] | max // 0' "$FILE" 2>/dev/null || echo 0)
 if [[ "$max_right" -gt 12 ]]; then
     warn "Layout exceeds 12-column grid (max right edge = $max_right)"
 fi
 
-# 8. Check schemaVersion
+# 10. Check schemaVersion
 schema_version=$(jq -r '.schemaVersion // 0' "$FILE")
 if [[ "$schema_version" != "2" ]]; then
     warn "schemaVersion is $schema_version, expected 2"
 fi
 
-# 9. Check layout entries have minH/minW
+# 11. Check layout entries have minH/minW
 missing_min=$(jq -r '.layout[] | select(.minH == null or .minW == null) | .i' "$FILE" 2>/dev/null || true)
 if [[ -n "$missing_min" ]]; then
     info "$(echo "$missing_min" | wc -l | tr -d ' ') layout entries missing minH/minW (will be auto-fixed on deploy)"

skills/building-dashboards/tests/test-script-output.sh

@@ -0,0 +1,103 @@
+#!/usr/bin/env bash
+# test-script-output.sh: Ensure deployment scripts keep machine-readable stdout
+#
+# Usage: ./test-script-output.sh
+
+set -euo pipefail
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+SCRIPTS_DIR="$SCRIPT_DIR/../scripts"
+
+passed=0
+failed=0
+
+ok() {
+    ((passed++)) || true
+    echo "  ✓ $1"
+}
+
+fail() {
+    ((failed++)) || true
+    echo "  ✗ $1: $2"
+}
+
+TMPDIR="$(mktemp -d)"
+trap 'rm -rf "$TMPDIR"' EXIT
+
+cp "$SCRIPTS_DIR/dashboard-create" "$TMPDIR/"
+cp "$SCRIPTS_DIR/dashboard-update" "$TMPDIR/"
+cp "$SCRIPTS_DIR/dashboard-validate" "$TMPDIR/"
+cp "$SCRIPTS_DIR/dashboard-normalize.jq" "$TMPDIR/"
+
+chmod +x "$TMPDIR/dashboard-create" "$TMPDIR/dashboard-update" "$TMPDIR/dashboard-validate"
+
+cat > "$TMPDIR/input.json" <<'JSON'
+{
+  "id": "dashboard-root-id",
+  "version": "v1",
+  "createdAt": "2026-02-01T10:00:00Z",
+  "updatedAt": "2026-02-02T11:00:00Z",
+  "createdBy": "alice@example.com",
+  "updatedBy": "bob@example.com",
+  "schemaVersion": 2,
+  "name": "Test Dashboard",
+  "description": "Test",
+  "owner": "user-123",
+  "charts": [
+    {
+      "id": "error-rate",
+      "name": "Error Rate",
+      "type": "Statistic",
+      "query": { "apl": "['logs'] | summarize c=count()" }
+    }
+  ],
+  "layout": [
+    { "i": "error-rate", "x": 0, "y": 0, "w": 3, "h": 2 }
+  ]
+}
+JSON
+
+cat > "$TMPDIR/axiom-api" <<'BASH'
+#!/usr/bin/env bash
+set -euo pipefail
+METHOD="${2:-}"
+PATH_="${3:-}"
+
+case "$METHOD:$PATH_" in
+  "POST:/internal/dashboards")
+    echo '{"id":"created-id"}'
+    ;;
+  "PUT:/internal/dashboards/dashboard-root-id")
+    echo '{"id":"dashboard-root-id","updated":true}'
+    ;;
+  *)
+    echo "Unexpected call: $METHOD $PATH_" >&2
+    exit 1
+    ;;
+esac
+BASH
+
+chmod +x "$TMPDIR/axiom-api"
+
+echo "Script Stdout Contract"
+echo "======================"
+
+create_out=$("$TMPDIR/dashboard-create" prod "$TMPDIR/input.json")
+if [[ "$create_out" == "created-id" ]]; then
+    ok "dashboard-create outputs only dashboard ID"
+else
+    fail "dashboard-create outputs only dashboard ID" "got: $create_out"
+fi
+
+update_out=$("$TMPDIR/dashboard-update" prod dashboard-root-id "$TMPDIR/input.json")
+if echo "$update_out" | jq -e '.id == "dashboard-root-id" and .updated == true' > /dev/null 2>&1; then
+    ok "dashboard-update outputs valid JSON only"
+else
+    fail "dashboard-update outputs valid JSON only" "got: $update_out"
+fi
+
+echo ""
+echo "======================"
+echo "Passed: $passed | Failed: $failed"
+
+[[ $failed -eq 0 ]]