diff --git a/internal/cli/fieldproject.go b/internal/cli/fieldproject.go index a38b025..835f7ed 100644 --- a/internal/cli/fieldproject.go +++ b/internal/cli/fieldproject.go @@ -180,13 +180,25 @@ func boundProjectedDetail(row map[string]any, maxBytes int) error { len(encoded), maxBytes, largest) } +// isIdentifierField reports whether a projected field is an identifier: +// keys ending in _id or _key (incident_id, alert_key, …). Identifier values +// are matched, filtered, and passed back verbatim by the consumer — a jq +// exact-match over --json output, or a follow-up detail call — so clipping +// one silently defeats that consumer: an identifier either survives a +// projection intact or the projection errors out. +func isIdentifierField(key string) bool { + return strings.HasSuffix(key, "_id") || strings.HasSuffix(key, "_key") +} + // boundProjectedList shortens a list projection's string values fairly when // the compact rows themselves overflow the budget: it finds the largest // per-field byte cap that still makes everything fit, then applies that one -// cap to every string value across every row. A field already shorter than -// the cap is left completely untouched — only the field(s) actually -// responsible for the overflow (typically a long title) get shortened, each -// marked with "...". The cap never drops low enough to make the "..." +// cap to every shortenable string value across every row. A field already +// shorter than the cap is left completely untouched — only the field(s) +// actually responsible for the overflow (typically a long title) get +// shortened, each marked with "...". Identifier fields (keys ending in _id +// or _key) are exempt at every step — sizing, fitting, and applying — so +// they survive byte-intact. The cap never drops low enough to make the "..." // marker itself disappear, so a shortened value is always distinguishable // from a genuinely short one; if no cap at or above that floor fits, the // command fails with a small error instead of emitting values that look @@ -213,7 +225,10 @@ func boundProjectedList(rows []map[string]any, maxBytes int) (string, error) { maxLen := 0 for _, row := range rows { - for _, value := range row { + for key, value := range row { + if isIdentifierField(key) { + continue + } if text, ok := value.(string); ok && len(text) > maxLen { maxLen = len(text) } @@ -228,7 +243,7 @@ func boundProjectedList(rows []map[string]any, maxBytes int) (string, error) { for i, row := range rows { trialRow := make(map[string]any, len(row)) for key, value := range row { - if text, ok := value.(string); ok { + if text, ok := value.(string); ok && !isIdentifierField(key) { trialRow[key] = truncateUTF8Bytes(text, limit) } else { trialRow[key] = value @@ -280,6 +295,9 @@ func boundProjectedList(rows []map[string]any, maxBytes int) (string, error) { fields := map[string]bool{} for _, row := range rows { for key, value := range row { + if isIdentifierField(key) { + continue + } text, ok := value.(string) if !ok { continue diff --git a/internal/cli/fieldproject_test.go b/internal/cli/fieldproject_test.go index 0429c89..f3a3e38 100644 --- a/internal/cli/fieldproject_test.go +++ b/internal/cli/fieldproject_test.go @@ -781,12 +781,16 @@ func TestAlertEventListFieldsProjectionUnchanged(t *testing.T) { // dropped to 3 bytes or below, truncateUTF8Bytes's no-room-for-a-marker // fallback returned raw, unmarked bytes indistinguishable from a genuinely // short value. Even under extreme row/field pressure that forces every -// string field to shrink, every shortened value must carry the "..." marker. +// shortenable string field to shrink, every shortened value must carry the +// "..." marker. (Row count is sized so the exempt _id fields and the JSON +// envelope still fit at the truncation floor — more rows would tip the page +// into the identifier-overflow error pinned by +// TestBoundProjectedListIdentifierOnlyOverflowErrors.) func TestBoundProjectedListNeverEmitsUnmarkedTruncation(t *testing.T) { saveAndResetGlobals(t) flagOutputFormat = "json" - rows := make([]map[string]any, 100) + rows := make([]map[string]any, 90) for i := range rows { rows[i] = map[string]any{ "event_id": fmt.Sprintf("%024x", i), @@ -919,3 +923,99 @@ func TestBoundProjectedListErrorNamesLargestFields(t *testing.T) { t.Fatalf("list overflow error = %q, want it to name the largest fields", err) } } + +// TestBoundProjectedListNeverShortensIdentifierFields pins the identifier +// exemption: keys ending in _id/_key carry values a consumer matches, +// filters, or passes back verbatim (a jq exact-match over --json output, a +// follow-up detail call), so shortening one silently defeats that consumer. +// Even when a page overflows badly enough that the fair cap lands below an +// identifier's own length, identifiers must come back byte-identical and +// only free-text fields shorten; the note must name only the clipped fields. +func TestBoundProjectedListNeverShortensIdentifierFields(t *testing.T) { + for _, format := range []string{"json", "toon"} { + t.Run(format, func(t *testing.T) { + saveAndResetGlobals(t) + flagOutputFormat = format + + rows := make([]map[string]any, 10) + wantIDs := make([]map[string]string, len(rows)) + for i := range rows { + eventID := fmt.Sprintf("%024x", i) + alertKey := fmt.Sprintf("%032x", i) + rows[i] = map[string]any{ + "event_id": eventID, + "alert_key": alertKey, + "title": strings.Repeat("a", 500), + } + wantIDs[i] = map[string]string{"event_id": eventID, "alert_key": alertKey} + } + + const budget = 1400 + note, err := boundProjectedOutput(rows, budget) + if err != nil { + t.Fatalf("bound projected output: %v", err) + } + + for i, row := range rows { + for _, field := range []string{"event_id", "alert_key"} { + if got := row[field].(string); got != wantIDs[i][field] { + t.Errorf("row %d %s was shortened: got %q, want byte-identical %q", i, field, got, wantIDs[i][field]) + } + } + if title := row["title"].(string); !strings.HasSuffix(title, "...") { + t.Errorf("row %d title should be shortened with the \"...\" marker, got %q", i, title) + } + } + + if note == "" { + t.Fatal("shortened projection returned no note; caller cannot tell values were clipped") + } + if !strings.Contains(note, "title") { + t.Errorf("note = %q, want it to name the shortened field (title)", note) + } + if strings.Contains(note, "event_id") || strings.Contains(note, "alert_key") { + t.Errorf("note = %q, want it to name only shortened fields, never exempt identifiers", note) + } + + encoded, err := marshalStructured(rows) + if err != nil { + t.Fatalf("marshal bounded output: %v", err) + } + if len(encoded)+1 >= budget { + t.Errorf("bounded %s output is %d bytes, want <%d", format, len(encoded)+1, budget) + } + }) + } +} + +// TestBoundProjectedListIdentifierOnlyOverflowErrors pins the other half of +// the identifier exemption: when a page carries nothing shortenable and its +// identifier content alone overflows the budget, the command must fail with +// the narrowing error instead of clipping identifiers to fit — and the rows +// must come back untouched. +func TestBoundProjectedListIdentifierOnlyOverflowErrors(t *testing.T) { + saveAndResetGlobals(t) + flagOutputFormat = "json" + + // Sized so the full rows overflow 512 bytes while rows with ids clipped + // to the truncation floor would still fit: the old fair cap "succeeded" + // by shipping mangled ids, the exemption must instead refuse. + rows := make([]map[string]any, 12) + for i := range rows { + rows[i] = map[string]any{"incident_id": fmt.Sprintf("%024x", i)} + } + originals := make([]string, len(rows)) + for i, row := range rows { + originals[i] = row["incident_id"].(string) + } + + _, err := boundProjectedOutput(rows, 512) + if err == nil || !strings.Contains(err.Error(), "request fewer rows") { + t.Fatalf("identifier-only overflow error = %v, want bounded guidance", err) + } + for i, row := range rows { + if got := row["incident_id"].(string); got != originals[i] { + t.Errorf("row %d incident_id was mutated despite the error: got %q, want %q", i, got, originals[i]) + } + } +}