fix(cli): render trace details by id with validation + exit-code mapping (issue #17) (#20)

Closes #17.

Bug surface: `goclaw traces get <id>` was unreadable in TTY mode (dumped raw JSON),
silently swallowed unmarshal errors (empty `{}`), accepted any raw id including
path-traversal sequences (`..`, `/`, control chars), and collapsed every server
failure to exit code 1 regardless of HTTP status.

Fixes:
- `cmd/traces.go`
  - `tracesGetCmd.RunE`: validate id allowlist before HTTP, decode payload with
    error surfacing, render header card + span tree + events list for TTY,
    pass-through JSON for `-o json` / piped stdout.
  - `validateTraceID`: regex allowlist `^[A-Za-z0-9._-]+$` + reserved-token reject
    (`.`, `..`, empty/whitespace). Returns typed `*client.APIError{Code: INVALID_REQUEST}`
    so the central error handler maps it to exit code 4.
  - `renderTraceTable` + `buildSpanTree`: insertion-ordered span tree, orphan spans
    attach to virtual root. No relink walk, cycle-safe (cycles silently drop).
- `internal/client/http.go`
  - Retry loop bug: previously closed `resp.Body` on every iteration including the
    final attempt, then `io.ReadAll` after the loop failed with
    "read on closed response body". This collapsed typed `APIError` into an opaque
    wrapped error and lost the exit-code mapping for 5xx/429. Fixed by skipping
    the per-iteration close on the final attempt; outer `defer` handles cleanup.

Exit-code contract for `traces get`:
- 0 success / 2 permission denied / 3 not-found /
  4 malformed id (pre-HTTP) / 5 server failure / 6 rate-limit.

Tests: 10 new tests in `cmd/traces_get_test.go` lock the wire contract,
table+JSON rendering, exit-code mapping per HTTP status, and the
"malformed id makes zero HTTP calls" invariant (parameterized table).
Fixture `cmd/testdata/trace_detail_get.json` is a scrubbed stub with
`_TODO_refresh` marker; refresh against live gateway before merging to default.

Docs: CHANGELOG `[Unreleased]` Fixed entry, README `Reading a Trace by ID`
section with exit-code table, `docs/codebase-summary.md` traces bullet updated.

Author: mrgoonie

CHANGELOG.md

@@ -55,6 +55,10 @@ Format: [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - `goclaw activity aggregate --group-by {action|actor_type|entity_type|actor_id} [--from --to --limit --actor-type --actor-id --action --entity-type --entity-id]` — group audit-log activity by dimension with bucket counts (`GET /v1/activity/aggregate`). Attached as subcommand of existing `activity` parent.
 - `goclaw logs aggregate [--group-by {level|source}] [--level --source --from]` — summarize the runtime log ring buffer (`GET /v1/logs/runtime/aggregate`, admin-only). Distinct from `logs tail`. Epoch-millis `last_seen` rendered as RFC3339, never scientific notation.
 
+### Fixed
+
+- `goclaw traces get <id>` — TTY mode now renders a human-readable summary (header card + span tree + events list) instead of dumping raw JSON. JSON-mode payload unchanged. Decode failures surface as wrapped errors instead of an empty `{}`. Trace ids are validated against `^[A-Za-z0-9._-]+$` and reserved tokens (`.`, `..`) are rejected before any HTTP call. Distinct exit codes per failure: not-found → 3, permission-denied → 2, malformed-id → 4, server-failure → 5. Latent retry-body bug in `internal/client/http.go` fixed: the final 5xx/429 response body is now preserved so the typed `APIError` reaches the caller (previously collapsed to exit 1). Closes #17.
+
 ### Notes
 - All new commands honor the AI-first ergonomics contract: `--output=json` envelope, central error handler, `--yes` for destructive ops, `--quiet` for CI.
 - P4/P5 backlog was re-swept against the current CLI surface; already-covered items were removed from residual scope before implementation.

README.md

@@ -124,6 +124,18 @@ goclaw logs aggregate [--group-by <level|source>] [--level <l>] [--source <s>] [
 
 All are one-shot HTTP — no watch loops or WS streams. `logs aggregate` is admin-only on the server; `activity aggregate --group-by actor_id` is also admin-only (server-enforced).
 
+### Reading a Trace by ID
+
+```bash
+# Human-readable: header + span tree + events
+goclaw traces get <trace-id>
+
+# Machine-readable JSON (also auto-selected when stdout is piped)
+goclaw traces get <trace-id> -o json
+```
+
+Exit codes for `traces get`: `0` on success, `2` on permission denied, `3` on not-found, `4` on malformed id (rejected before any HTTP call — allowlist `^[A-Za-z0-9._-]+$`), `5` on upstream server failure, `6` on rate-limit / network-resource exhaustion.
+
 ## Backup & Restore
 
 ### System Backup

cmd/testdata/trace_detail_get.json

@@ -0,0 +1,54 @@
+{
+  "_TODO_refresh": "stub fixture derived from traces follow payload shape; refresh against goclaw.zuey.me before merge per phase-03 reviewer gate",
+  "trace_id": "trace_FIXTURE_001",
+  "agent_id": "agent_FIXTURE_001",
+  "session_key": "session_FIXTURE_001",
+  "user_id": "user_REDACTED",
+  "tenant_id": "tenant_REDACTED",
+  "status": "success",
+  "started_at": "2026-05-28T10:00:00Z",
+  "ended_at": "2026-05-28T10:00:02Z",
+  "duration_ms": 2000,
+  "input_tokens": 120,
+  "output_tokens": 80,
+  "cost": "0.0042",
+  "spans": [
+    {
+      "span_id": "span_001",
+      "parent_span_id": null,
+      "name": "agent.run",
+      "kind": "agent",
+      "started_at": "2026-05-28T10:00:00Z",
+      "ended_at": "2026-05-28T10:00:02Z",
+      "duration_ms": 2000,
+      "status": "success"
+    },
+    {
+      "span_id": "span_002",
+      "parent_span_id": "span_001",
+      "name": "llm.call",
+      "kind": "llm",
+      "started_at": "2026-05-28T10:00:00Z",
+      "ended_at": "2026-05-28T10:00:01Z",
+      "duration_ms": 1500,
+      "status": "success",
+      "input_tokens": 120,
+      "output_tokens": 80
+    },
+    {
+      "span_id": "span_003",
+      "parent_span_id": "span_001",
+      "name": "tool.call",
+      "kind": "tool",
+      "started_at": "2026-05-28T10:00:01Z",
+      "ended_at": "2026-05-28T10:00:02Z",
+      "duration_ms": 400,
+      "status": "success"
+    }
+  ],
+  "events": [
+    {"event_id": "ev_001", "span_id": "span_002", "type": "llm.prompt", "timestamp": "2026-05-28T10:00:00Z"},
+    {"event_id": "ev_002", "span_id": "span_002", "type": "llm.completion", "timestamp": "2026-05-28T10:00:01Z"},
+    {"event_id": "ev_003", "span_id": "span_003", "type": "tool.invoke", "timestamp": "2026-05-28T10:00:01Z"}
+  ]
+}

cmd/traces.go

@@ -1,12 +1,16 @@
 package cmd
 
 import (
+	"encoding/json"
 	"fmt"
 	"io"
 	"net/url"
 	"os"
+	"regexp"
+	"strings"
 	"time"
 
+	"github.com/nextlevelbuilder/goclaw-cli/internal/client"
 	"github.com/nextlevelbuilder/goclaw-cli/internal/output"
 	"github.com/spf13/cobra"
 )
@@ -61,19 +65,130 @@ var tracesListCmd = &cobra.Command{
 var tracesGetCmd = &cobra.Command{
 	Use: "get <traceID>", Short: "Get trace with span tree", Args: cobra.ExactArgs(1),
 	RunE: func(cmd *cobra.Command, args []string) error {
+		id := strings.TrimSpace(args[0])
+		if err := validateTraceID(id); err != nil {
+			return err
+		}
 		c, err := newHTTP()
 		if err != nil {
 			return err
 		}
-		data, err := c.Get("/v1/traces/" + args[0])
+		data, err := c.Get("/v1/traces/" + url.PathEscape(id))
 		if err != nil {
 			return err
 		}
-		printer.Print(unmarshalMap(data))
+		var trace map[string]any
+		if err := json.Unmarshal(data, &trace); err != nil {
+			return fmt.Errorf("decode trace payload: %w", err)
+		}
+		if cfg.OutputFormat != "table" {
+			printer.Print(trace)
+			return nil
+		}
+		renderTraceTable(trace, os.Stdout)
 		return nil
 	},
 }
 
+// traceIDPattern restricts trace ids to a safe, URL-safe allowlist.
+// Blocks path-traversal (`..`, `/`, `\`), control characters, and whitespace
+// before any HTTP call is issued. PathEscape is still applied on top.
+var traceIDPattern = regexp.MustCompile(`^[A-Za-z0-9._-]+$`)
+
+func validateTraceID(id string) error {
+	if id == "" || id == "." || id == ".." {
+		return &client.APIError{Code: "INVALID_REQUEST", Message: "trace id is empty or reserved"}
+	}
+	if !traceIDPattern.MatchString(id) {
+		return &client.APIError{Code: "INVALID_REQUEST", Message: "trace id contains invalid characters (allowed: A-Z a-z 0-9 . _ -)"}
+	}
+	return nil
+}
+
+// renderTraceTable prints a human-readable summary: header card, span tree, events.
+func renderTraceTable(t map[string]any, w io.Writer) {
+	for _, row := range [][2]string{
+		{"TRACE_ID", str(t, "trace_id")}, {"AGENT_ID", str(t, "agent_id")},
+		{"SESSION_KEY", str(t, "session_key")}, {"STATUS", str(t, "status")},
+		{"DURATION_MS", str(t, "duration_ms")},
+	} {
+		if row[1] != "" {
+			fmt.Fprintf(w, "%-12s %s\n", row[0]+":", row[1])
+		}
+	}
+	if in, out, cost := str(t, "input_tokens"), str(t, "output_tokens"), str(t, "cost"); in+out+cost != "" {
+		fmt.Fprintf(w, "%-12s in=%s out=%s cost=%s\n", "TOKENS:", in, out, cost)
+	}
+	spans, _ := t["spans"].([]any)
+	if len(spans) == 0 {
+		fmt.Fprintln(w, "\nSPANS: (none)")
+	} else {
+		fmt.Fprintln(w, "\nSPANS:")
+		output.PrintTreeRoot(buildSpanTree(spans), w)
+	}
+	events, _ := t["events"].([]any)
+	fmt.Fprintf(w, "\nEVENTS (n=%d):\n", len(events))
+	for _, e := range events {
+		if m, ok := e.(map[string]any); ok {
+			fmt.Fprintf(w, "  - %s\n", str(m, "type"))
+		}
+	}
+}
+
+// buildSpanTree links spans via parent_span_id; spans whose parent isn't in this
+// trace attach to a virtual root. Children are kept in insertion order.
+func buildSpanTree(spans []any) output.TreeNode {
+	order := make([]string, 0, len(spans))
+	labels := make(map[string]string, len(spans))
+	children := make(map[string][]string, len(spans))
+	parentOf := make(map[string]string, len(spans))
+	for _, s := range spans {
+		m, ok := s.(map[string]any)
+		if !ok {
+			continue
+		}
+		id := str(m, "span_id")
+		if id == "" {
+			continue
+		}
+		label := id
+		if name := str(m, "name"); name != "" {
+			label = name + " [" + id + "]"
+		}
+		if kind := str(m, "kind"); kind != "" {
+			label += " kind=" + kind
+		}
+		if dur := str(m, "duration_ms"); dur != "" {
+			label += " " + dur + "ms"
+		}
+		labels[id] = label
+		order = append(order, id)
+		parentOf[id], _ = m["parent_span_id"].(string)
+	}
+	for _, id := range order {
+		if p := parentOf[id]; p != "" {
+			if _, ok := labels[p]; ok {
+				children[p] = append(children[p], id)
+				continue
+			}
+		}
+		children[""] = append(children[""], id)
+	}
+	var build func(id string) output.TreeNode
+	build = func(id string) output.TreeNode {
+		n := output.TreeNode{Name: labels[id]}
+		for _, c := range children[id] {
+			n.Children = append(n.Children, build(c))
+		}
+		return n
+	}
+	root := output.TreeNode{Name: "trace"}
+	for _, id := range children[""] {
+		root.Children = append(root.Children, build(id))
+	}
+	return root
+}
+
 var tracesExportCmd = &cobra.Command{
 	Use: "export <traceID>", Short: "Export trace to file", Args: cobra.ExactArgs(1),
 	RunE: func(cmd *cobra.Command, args []string) error {