feat(cli): add P6 backend-unblocked CLI surfaces (issue #16)#19
Merged
Conversation
Mirrors issue #16 scope (7 command surfaces) as 4 grouped implementation phases with TDD per the user's request. Backend evidence: digitopvn/goclaw PR #37 + PR #44. Verified beta tag containing PR #44 is v3.12.0-beta.20. Phases: 1. Scope Lock — contract re-verification, drift sweep 2. Traces Follow + Providers Reconnect (PR #37) 3. Sessions Branch + Follow (PR #44 chat) 4. Channels Writers Test (PR #44 channels) 5. Activity + Logs Runtime Aggregate (PR #44 aggregation) 6. Tests + Docs sweep with out-of-scope red-team 7. Ship Readiness — single PR to dev Out-of-scope list retained verbatim from issue #16: traces replay, generic logs aggregate, WS/SSE chat history, watch loops.
Seven one-shot HTTP commands wired to backend PRs #37 and #44
(gateway tag v3.12.0-beta.20+). TDD: failing tests landed before each
implementation slice; all suites green under -count=1.
PR #37 surfaces:
- traces follow --session-key|--agent [--since --limit --include-spans
--status --channel]: GET /v1/traces/follow
- providers reconnect <id>: POST /v1/providers/{id}/reconnect
PR #44 surfaces:
- sessions branch <key> --up-to-index N [--new-session-key --label
--metadata k=v]: POST /v1/chat/sessions/{key}/branch.
--up-to-index=0 preserved literally on the wire (bypasses buildBody
int-zero skip).
- sessions follow <key> [--cursor N --limit N]: one-shot cursor-based
history poll, NOT a watch loop. --cursor=0 preserved in raw query.
- channels writers test <id> --group-id G --user-id U: POST writers/test
with body containing exactly two keys.
- activity aggregate --group-by {action|actor_type|entity_type|actor_id}
[--from --to --limit + scope filters]: subcommand of existing activityCmd.
- logs aggregate [--group-by {level|source} --level --source --from]:
ring-buffer summary, distinct from logs tail.
Shared formatLastSeen helper type-switches RFC3339-string vs epoch-millis
last_seen so neither aggregate command renders scientific notation in
table view.
All path params url.PathEscape'd; all flag validation runs before HTTP
call; central error handler honored (no direct error printing).
Docs: README + docs/codebase-summary + CHANGELOG updated.
Closes #16
mrgoonie
added a commit
that referenced
this pull request
May 28, 2026
…#21) * feat(cli): add P6 backend-unblocked CLI surfaces (issue #16) (#19) * plan(p6): add domain coverage P6 backend-unblocked CLI plan Mirrors issue #16 scope (7 command surfaces) as 4 grouped implementation phases with TDD per the user's request. Backend evidence: digitopvn/goclaw PR #37 + PR #44. Verified beta tag containing PR #44 is v3.12.0-beta.20. Phases: 1. Scope Lock — contract re-verification, drift sweep 2. Traces Follow + Providers Reconnect (PR #37) 3. Sessions Branch + Follow (PR #44 chat) 4. Channels Writers Test (PR #44 channels) 5. Activity + Logs Runtime Aggregate (PR #44 aggregation) 6. Tests + Docs sweep with out-of-scope red-team 7. Ship Readiness — single PR to dev Out-of-scope list retained verbatim from issue #16: traces replay, generic logs aggregate, WS/SSE chat history, watch loops. * feat(cli): add P6 backend-unblocked CLI surfaces (issue #16) Seven one-shot HTTP commands wired to backend PRs #37 and #44 (gateway tag v3.12.0-beta.20+). TDD: failing tests landed before each implementation slice; all suites green under -count=1. PR #37 surfaces: - traces follow --session-key|--agent [--since --limit --include-spans --status --channel]: GET /v1/traces/follow - providers reconnect <id>: POST /v1/providers/{id}/reconnect PR #44 surfaces: - sessions branch <key> --up-to-index N [--new-session-key --label --metadata k=v]: POST /v1/chat/sessions/{key}/branch. --up-to-index=0 preserved literally on the wire (bypasses buildBody int-zero skip). - sessions follow <key> [--cursor N --limit N]: one-shot cursor-based history poll, NOT a watch loop. --cursor=0 preserved in raw query. - channels writers test <id> --group-id G --user-id U: POST writers/test with body containing exactly two keys. - activity aggregate --group-by {action|actor_type|entity_type|actor_id} [--from --to --limit + scope filters]: subcommand of existing activityCmd. - logs aggregate [--group-by {level|source} --level --source --from]: ring-buffer summary, distinct from logs tail. Shared formatLastSeen helper type-switches RFC3339-string vs epoch-millis last_seen so neither aggregate command renders scientific notation in table view. All path params url.PathEscape'd; all flag validation runs before HTTP call; central error handler honored (no direct error printing). Docs: README + docs/codebase-summary + CHANGELOG updated. Closes #16 * 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. * docs: add planning artifacts for P6 backend-unblocked CLI and skill scaffold
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the 7 CLI command surfaces unblocked by
digitopvn/goclawbackend PRs:v3.12.0-beta.16+):traces follow,providers reconnectv3.12.0-beta.20+):sessions branch,sessions follow,channels writers test,activity aggregate,logs aggregateAll seven are one-shot HTTP commands — no new watch loops, no new WS streams.
Surface inventory
goclaw traces follow --session-key|--agentGET /v1/traces/followgoclaw providers reconnect <id>POST /v1/providers/{id}/reconnectgoclaw sessions branch <key> --up-to-index NPOST /v1/chat/sessions/{key}/branchgoclaw sessions follow <key>GET /v1/chat/sessions/{key}/history/followgoclaw channels writers test <id> --group-id G --user-id UPOST /v1/channels/instances/{id}/writers/testgoclaw activity aggregate --group-by ...GET /v1/activity/aggregategoclaw logs aggregate [--group-by ...]GET /v1/logs/runtime/aggregateNotable technical decisions
up_to_index=0andcursor=0are valid semantic values. Request body / query is constructed directly (bypassingbuildBody's int-zero skip atcmd/helpers.go:86-89) so zeros appear literally on the wire. Regression tests assert this.formatLastSeenhelper: type-switches RFC3339 string (activity endpoint) vs epoch-millis number (logs runtime endpoint) so neither aggregate command renders1.76e+12in the table view.internal/client/http.goauto-retries 429/5xx three times. The original "502 fail-fast" assertion in the plan was dropped because it conflicts with built-in behavior; atomic-counter + structural "not a watch loop" checks preserve intent.activity aggregateattaches to the pre-existingactivityCmdatcmd/admin.go:133.cmd_test.gotop-level inventory unchanged.{id}/{key}substitution usesurl.PathEscape; each command has a path-escape regression test.Verification
go vet ./...— cleango build ./...— cleango test -count=1 ./...— all packages PASSv3.12.0-beta.20post-mergeBackend evidence (locked)
56e227c443049d3bv3.12.0-beta.20Out of scope (explicit)
traces follow/sessions follow— deferred; one-shot polling only.Plan + reports
plans/260527-1412-domain-coverage-p6-backend-unblocked/plan.mdreports/scope-lock-260527-p6.mdreports/code-review-260527-p6.mdreports/sweep-red-team-diff-260527-p6.mdCloses #16