From 5491bdfef6374f09b90c4f80d6d808b1e8453196 Mon Sep 17 00:00:00 2001 From: ysyneu Date: Wed, 26 Aug 2026 08:35:39 -0700 Subject: [PATCH 1/2] fix(cli): announce truncated structured list pages on stderr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In json/toon mode PrintList suppresses the "Showing N results" footer to keep stdout byte-pure for pipelines, so a page short of the server total (e.g. the default --limit 20 of a much larger total) was indistinguishable from the whole set. PrintList now prints a "note: showing N of T total results (page P)" line on stderr when the page does not cover the total; table mode is unchanged. Also pin, at both the command and the built-binary level, that a compact list projection which cannot fit its byte budget fails hard: non-zero exit, the refusal on stderr, and nothing on stdout — so a pipeline reading stdout sees a failed call rather than an empty page. --- cmd/flashduty/main_test.go | 49 ++++++++++++++ internal/cli/command.go | 12 +++- internal/cli/command_test.go | 125 +++++++++++++++++++++++++++++++++++ 3 files changed, 184 insertions(+), 2 deletions(-) diff --git a/cmd/flashduty/main_test.go b/cmd/flashduty/main_test.go index 5f8225e..1143742 100644 --- a/cmd/flashduty/main_test.go +++ b/cmd/flashduty/main_test.go @@ -3,6 +3,9 @@ package main import ( "bytes" "fmt" + "net/http" + "net/http/httptest" + "os" "os/exec" "path/filepath" "runtime" @@ -108,3 +111,49 @@ func TestSetVersionInfoBeforeExecute(t *testing.T) { } } } + +// Test 79: When a compact list projection overflows its byte budget, the +// binary exits non-zero, writes nothing to stdout, and reports the error on +// stderr — a pipeline reading stdout must see a failed call, never an empty +// page masquerading as "no data". +func TestProjectionOverflowFailsHard(t *testing.T) { + binPath := buildTestBinary(t, "") + + // Stub the alert-event list endpoint with a page whose projection stays + // over the 16 KiB budget even after value shortening. + var body strings.Builder + body.WriteString(`{"request_id":"r","error":{"code":"OK","message":""},"data":{"total":100,"items":[`) + for i := 0; i < 100; i++ { + if i > 0 { + body.WriteByte(',') + } + fmt.Fprintf(&body, `{"event_id":"%024x","alert_id":"%024x","event_severity":"Warning","event_status":"Triggered","event_time":1712000000,"title":%q}`, + i, i+1_000_000, strings.Repeat("x", 200)) + } + body.WriteString(`]}}`) + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(body.String())) + })) + defer srv.Close() + + run := exec.Command(binPath, "alert-event", "list", "--limit", "100", + "--output-format", "json", "--app-key", "test-key", "--base-url", srv.URL) + // Isolate HOME so the test never reads the developer's real CLI config. + run.Env = append(os.Environ(), "HOME="+t.TempDir()) + var stdout, stderr bytes.Buffer + run.Stdout = &stdout + run.Stderr = &stderr + + err := run.Run() + if err == nil { + t.Fatalf("[#79] expected non-zero exit code for an over-budget projection, got success; stderr:\n%s", stderr.String()) + } + if stdout.Len() != 0 { + t.Errorf("[#79] a failed projection must write nothing to stdout, got %d bytes:\n%s", stdout.Len(), stdout.String()) + } + if !strings.Contains(stderr.String(), "Error: projected list is") || !strings.Contains(stderr.String(), "exceeds the 16384-byte limit") { + t.Errorf("[#79] stderr should report the byte-limit refusal, got:\n%s", stderr.String()) + } +} diff --git a/internal/cli/command.go b/internal/cli/command.go index c7826a8..d12fda7 100644 --- a/internal/cli/command.go +++ b/internal/cli/command.go @@ -48,13 +48,21 @@ func runCommand(cmd *cobra.Command, args []string, fn func(ctx *RunContext) erro } // PrintList prints items as a table and appends a "Showing N results (page P, total T)." footer. +// In structured mode the footer is suppressed to keep stdout byte-pure for +// jq/toon pipelines, so a page that doesn't cover the total is announced on +// stderr instead — without it a consumer sees a partial page +// (e.g. the default --limit 20 of a far larger total) as the whole set. func (ctx *RunContext) PrintList(items any, cols []output.Column, count, page, total int) error { if err := ctx.Printer.Print(items, cols); err != nil { return err } - if !ctx.Structured() { - _, _ = fmt.Fprintf(ctx.Writer, "Showing %d results (page %d, total %d).\n", count, page, total) + if ctx.Structured() { + if total > count { + _, _ = fmt.Fprintf(ctx.Cmd.ErrOrStderr(), "note: showing %d of %d total results (page %d); raise --limit or use --page for the rest\n", count, total, page) + } + return nil } + _, _ = fmt.Fprintf(ctx.Writer, "Showing %d results (page %d, total %d).\n", count, page, total) return nil } diff --git a/internal/cli/command_test.go b/internal/cli/command_test.go index 9754dce..f1efa2a 100644 --- a/internal/cli/command_test.go +++ b/internal/cli/command_test.go @@ -1594,6 +1594,131 @@ type readerFunc func([]byte) (int, error) func (f readerFunc) Read(p []byte) (int, error) { return f(p) } +// --------------------------------------------------------------------------- +// Structured list truncation indicator +// --------------------------------------------------------------------------- + +// TestCommandAlertListStructuredAnnouncesTruncation pins that a structured +// list page which doesn't cover the server-reported total says so on stderr: +// stdout is reserved for the jq/toon pipeline, so without the note a consumer +// sees the default --limit page as the whole set. Table mode keeps its +// "Showing N results" footer on stdout and emits no stderr note. +func TestCommandAlertListStructuredAnnouncesTruncation(t *testing.T) { + twoOfFive := map[string]any{"items": []any{alertRow(), alertRow()}, "total": 5} + + t.Run("json page short of total", func(t *testing.T) { + saveAndResetGlobals(t) + stub := newGFStub(t) + stub.data = twoOfFive + + out, stderrText, err := execCommandSplit("alert", "list", "--output-format", "json") + if err != nil { + t.Fatalf("execCommandSplit: %v", err) + } + var rows []map[string]any + if err := json.Unmarshal([]byte(strings.TrimSpace(out)), &rows); err != nil { + t.Fatalf("stdout must stay parseable JSON: %v\n%s", err, out) + } + if len(rows) != 2 { + t.Fatalf("got %d rows, want the 2 the page carries", len(rows)) + } + if !strings.Contains(stderrText, "note: showing 2 of 5 total results (page 1)") { + t.Errorf("truncated structured page should announce itself on stderr, got:\n%s", stderrText) + } + }) + + t.Run("json page covers total", func(t *testing.T) { + saveAndResetGlobals(t) + stub := newGFStub(t) + stub.data = map[string]any{"items": []any{alertRow(), alertRow()}, "total": 2} + + _, stderrText, err := execCommandSplit("alert", "list", "--output-format", "json") + if err != nil { + t.Fatalf("execCommandSplit: %v", err) + } + if strings.Contains(stderrText, "note: showing") { + t.Errorf("a page covering the total must not cry truncation, got:\n%s", stderrText) + } + }) + + t.Run("table keeps the footer on stdout", func(t *testing.T) { + saveAndResetGlobals(t) + stub := newGFStub(t) + stub.data = twoOfFive + + out, stderrText, err := execCommandSplit("alert", "list") + if err != nil { + t.Fatalf("execCommandSplit: %v", err) + } + if !strings.Contains(out, "Showing 2 results (page 1, total 5).") { + t.Errorf("table mode should keep the stdout footer, got:\n%s", out) + } + if strings.Contains(stderrText, "note: showing") { + t.Errorf("table mode already footers the count; no stderr note wanted, got:\n%s", stderrText) + } + }) +} + +// --------------------------------------------------------------------------- +// Projection overflow is a hard failure +// --------------------------------------------------------------------------- + +// TestCommandListProjectionOverflowFails pins that a compact list projection +// which cannot fit the byte budget fails the command instead of emitting +// anything: Execute returns the error and stdout stays empty, so a pipeline +// reading stdout sees a failed call, never an empty page masquerading as +// "no data". +func TestCommandListProjectionOverflowFails(t *testing.T) { + t.Run("incident list", func(t *testing.T) { + saveAndResetGlobals(t) + stub := newGFStub(t) + items := make([]any, 100) + for i := range items { + row := incidentRow() + row["incident_id"] = fmt.Sprintf("inc-%024d", i) + row["title"] = strings.Repeat("x", 200) + items[i] = row + } + stub.data = map[string]any{"items": items, "total": len(items)} + + out, stderrText, err := execCommandSplit("incident", "list", "--limit", "100", "--output-format", "json") + if err == nil || !strings.Contains(err.Error(), "exceeds the 16384-byte limit") { + t.Fatalf("irreducible projection error = %v, want the byte-limit refusal", err) + } + if out != "" { + t.Errorf("a failed projection must write nothing to stdout, got %d bytes", len(out)) + } + if strings.Contains(stderrText, "exceeds the") { + t.Errorf("the error is returned for the entrypoint to report, not printed mid-run, got:\n%s", stderrText) + } + }) + + t.Run("alert-event list", func(t *testing.T) { + saveAndResetGlobals(t) + stub := newGFStub(t) + items := make([]any, 100) + for i := range items { + items[i] = map[string]any{ + "event_id": fmt.Sprintf("%024x", i), + "alert_id": fmt.Sprintf("%024x", i+1_000_000), + "event_severity": "Warning", + "event_status": "Triggered", + "event_time": 1712000000 + i, + "title": strings.Repeat("x", 200), + } + } + stub.data = map[string]any{"items": items, "total": len(items)} + + out, _, err := execCommandSplit("alert-event", "list", "--limit", "100", "--output-format", "json") + if err == nil || !strings.Contains(err.Error(), "exceeds the 16384-byte limit") { + t.Fatalf("irreducible projection error = %v, want the byte-limit refusal", err) + } + if out != "" { + t.Errorf("a failed projection must write nothing to stdout, got %d bytes", len(out)) + } + }) +} + // --------------------------------------------------------------------------- // Helpers // --------------------------------------------------------------------------- From 8a3a83c612d171d9cbb548f954e307d131b18239 Mon Sep 17 00:00:00 2001 From: ysyneu Date: Wed, 26 Aug 2026 09:39:17 -0700 Subject: [PATCH 2/2] fix(cli): account for the page offset in the structured truncation note The note fired whenever total > count, which misjudges the last page: --limit 20 --page 2 against a total of 40 printed "use --page for the rest" even though page 2 is the end. PrintList now takes the page size and computes hasMore as (page-1)*limit+count < total, so the note only appears when rows actually remain beyond the current page. --- internal/cli/alert.go | 4 ++-- internal/cli/alert_event.go | 4 ++-- internal/cli/audit.go | 2 +- internal/cli/change.go | 2 +- internal/cli/command.go | 10 ++++++---- internal/cli/command_test.go | 36 ++++++++++++++++++++++++++++++++++++ internal/cli/incident.go | 4 ++-- internal/cli/insight.go | 2 +- 8 files changed, 51 insertions(+), 13 deletions(-) diff --git a/internal/cli/alert.go b/internal/cli/alert.go index d2a3098..7ccf0eb 100644 --- a/internal/cli/alert.go +++ b/internal/cli/alert.go @@ -93,7 +93,7 @@ func newAlertListCmd() *cobra.Command { if err != nil { return err } - return ctx.PrintList(proj, nil, len(result.Items), page, int(result.Total)) + return ctx.PrintList(proj, nil, len(result.Items), page, limit, int(result.Total)) } cols := []output.Column{ @@ -106,7 +106,7 @@ func newAlertListCmd() *cobra.Command { {Header: "STARTED", Field: func(v any) string { return output.FormatTime(v.(flashduty.AlertItem).StartTime) }}, } - return ctx.PrintList(result.Items, cols, len(result.Items), page, int(result.Total)) + return ctx.PrintList(result.Items, cols, len(result.Items), page, limit, int(result.Total)) }) }, } diff --git a/internal/cli/alert_event.go b/internal/cli/alert_event.go index 893b31a..73b04fb 100644 --- a/internal/cli/alert_event.go +++ b/internal/cli/alert_event.go @@ -102,10 +102,10 @@ func newAlertEventListCmd() *cobra.Command { return err } noteProjectionShortening(cmd.ErrOrStderr(), note) - return ctx.PrintList(proj, nil, len(result.Items), page, int(result.Total)) + return ctx.PrintList(proj, nil, len(result.Items), page, limit, int(result.Total)) } - return ctx.PrintList(result.Items, cols, len(result.Items), page, int(result.Total)) + return ctx.PrintList(result.Items, cols, len(result.Items), page, limit, int(result.Total)) }) }, } diff --git a/internal/cli/audit.go b/internal/cli/audit.go index d325a66..5c586b4 100644 --- a/internal/cli/audit.go +++ b/internal/cli/audit.go @@ -96,7 +96,7 @@ func newAuditSearchCmd() *cobra.Command { }}, } - return ctx.PrintList(result.Docs, cols, len(result.Docs), page, int(result.Total)) + return ctx.PrintList(result.Docs, cols, len(result.Docs), page, limit, int(result.Total)) }) }, } diff --git a/internal/cli/change.go b/internal/cli/change.go index a8dbb1e..425b7e0 100644 --- a/internal/cli/change.go +++ b/internal/cli/change.go @@ -86,7 +86,7 @@ func newChangeListCmd() *cobra.Command { {Header: "TIME", Field: func(v any) string { return output.FormatTime(v.(flashduty.ChangeItem).StartTime) }}, } - return ctx.PrintList(result.Items, cols, len(result.Items), page, int(result.Total)) + return ctx.PrintList(result.Items, cols, len(result.Items), page, limit, int(result.Total)) }) }, } diff --git a/internal/cli/command.go b/internal/cli/command.go index d12fda7..5ef5166 100644 --- a/internal/cli/command.go +++ b/internal/cli/command.go @@ -49,15 +49,17 @@ func runCommand(cmd *cobra.Command, args []string, fn func(ctx *RunContext) erro // PrintList prints items as a table and appends a "Showing N results (page P, total T)." footer. // In structured mode the footer is suppressed to keep stdout byte-pure for -// jq/toon pipelines, so a page that doesn't cover the total is announced on +// jq/toon pipelines, so a page with more rows beyond it is announced on // stderr instead — without it a consumer sees a partial page -// (e.g. the default --limit 20 of a far larger total) as the whole set. -func (ctx *RunContext) PrintList(items any, cols []output.Column, count, page, total int) error { +// (e.g. the default --limit 20 of a far larger total) as the whole set. The +// judgment accounts for the page offset: on the last page +// ((page-1)*limit+count reaches total) there is no rest, so no note. +func (ctx *RunContext) PrintList(items any, cols []output.Column, count, page, limit, total int) error { if err := ctx.Printer.Print(items, cols); err != nil { return err } if ctx.Structured() { - if total > count { + if (page-1)*limit+count < total { _, _ = fmt.Fprintf(ctx.Cmd.ErrOrStderr(), "note: showing %d of %d total results (page %d); raise --limit or use --page for the rest\n", count, total, page) } return nil diff --git a/internal/cli/command_test.go b/internal/cli/command_test.go index f1efa2a..2ff00d1 100644 --- a/internal/cli/command_test.go +++ b/internal/cli/command_test.go @@ -1657,6 +1657,42 @@ func TestCommandAlertListStructuredAnnouncesTruncation(t *testing.T) { t.Errorf("table mode already footers the count; no stderr note wanted, got:\n%s", stderrText) } }) + + // The truncation judgment must account for the page offset: page 2 of a + // total 40 at --limit 20 IS the last page — there is no rest, so no note. + fullPage := make([]any, 20) + for i := range fullPage { + fullPage[i] = alertRow() + } + lastPage := map[string]any{"items": fullPage, "total": 40} + + t.Run("json last page prints no note", func(t *testing.T) { + saveAndResetGlobals(t) + stub := newGFStub(t) + stub.data = lastPage + + _, stderrText, err := execCommandSplit("alert", "list", "--limit", "20", "--page", "2", "--output-format", "json") + if err != nil { + t.Fatalf("execCommandSplit: %v", err) + } + if strings.Contains(stderrText, "note: showing") { + t.Errorf("the last page has no rest to page for; no note wanted, got:\n%s", stderrText) + } + }) + + t.Run("json first page of same total still notes", func(t *testing.T) { + saveAndResetGlobals(t) + stub := newGFStub(t) + stub.data = lastPage + + _, stderrText, err := execCommandSplit("alert", "list", "--limit", "20", "--page", "1", "--output-format", "json") + if err != nil { + t.Fatalf("execCommandSplit: %v", err) + } + if !strings.Contains(stderrText, "note: showing 20 of 40 total results (page 1)") { + t.Errorf("page 1 with a page 2 beyond it should announce itself on stderr, got:\n%s", stderrText) + } + }) } // --------------------------------------------------------------------------- diff --git a/internal/cli/incident.go b/internal/cli/incident.go index 2cb7fbc..d15e75d 100644 --- a/internal/cli/incident.go +++ b/internal/cli/incident.go @@ -142,10 +142,10 @@ func newIncidentListCmd() *cobra.Command { return err } noteProjectionShortening(cmd.ErrOrStderr(), note) - return ctx.PrintList(proj, nil, len(result.Items), page, int(result.Total)) + return ctx.PrintList(proj, nil, len(result.Items), page, limit, int(result.Total)) } - return ctx.PrintList(result.Items, incidentColumns(), len(result.Items), page, int(result.Total)) + return ctx.PrintList(result.Items, incidentColumns(), len(result.Items), page, limit, int(result.Total)) }) }, } diff --git a/internal/cli/insight.go b/internal/cli/insight.go index a2e02bd..75a9d32 100644 --- a/internal/cli/insight.go +++ b/internal/cli/insight.go @@ -131,7 +131,7 @@ func newInsightIncidentsCmd() *cobra.Command { }}, } - return ctx.PrintList(result.Items, cols, len(result.Items), page, int(result.Total)) + return ctx.PrintList(result.Items, cols, len(result.Items), page, limit, int(result.Total)) }) }, }