From aae173bb3144e87d15b6a3a12c21db7a45e322c3 Mon Sep 17 00:00:00 2001 From: ysyneu Date: Fri, 29 May 2026 00:03:20 +0800 Subject: [PATCH] feat(cli): render API timestamps as RFC3339 in structured output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flashduty's API returns time fields as bare Unix integers, which are opaque to an LLM reading the CLI's JSON/TOON output. Humanize them to RFC3339 (in the local/environment timezone) at the structured-output chokepoints — JSONPrinter, TOONPrinter, and marshalStructured. Detection is by field name (*_time, *_at, timestamp), excluding ID-like fields (*_by/*_id/*_ids); values below 1e9 stay numeric so durations/counts aren't mistaken for dates. Table mode is unchanged (already formats via FormatTime). --- internal/cli/root.go | 1 + internal/output/humanize_wiring_test.go | 44 ++++++++ internal/output/json.go | 2 +- internal/output/timestamps.go | 95 ++++++++++++++++ internal/output/timestamps_test.go | 142 ++++++++++++++++++++++++ internal/output/toon.go | 2 +- 6 files changed, 284 insertions(+), 2 deletions(-) create mode 100644 internal/output/humanize_wiring_test.go create mode 100644 internal/output/timestamps.go create mode 100644 internal/output/timestamps_test.go diff --git a/internal/cli/root.go b/internal/cli/root.go index 33e7280..2657c37 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -290,6 +290,7 @@ func currentOutputFormat() output.Format { // FormatJSON (byte-compatible with the legacy --json path) and TOON via the SDK // for FormatTOON. func marshalStructured(v any) ([]byte, error) { + v = output.HumanizeTimestamps(v) if currentOutputFormat() == output.FormatTOON { return flashduty.Marshal(v, flashduty.OutputFormatTOON) } diff --git a/internal/output/humanize_wiring_test.go b/internal/output/humanize_wiring_test.go new file mode 100644 index 0000000..983ac81 --- /dev/null +++ b/internal/output/humanize_wiring_test.go @@ -0,0 +1,44 @@ +package output + +import ( + "bytes" + "encoding/json" + "strings" + "testing" +) + +// These tests lock the wiring: each structured printer must route data through +// HumanizeTimestamps so a raw Unix integer never reaches the agent. They guard +// against a future printer being added without the conversion. + +func TestJSONPrinter_HumanizesTimestamps(t *testing.T) { + const ts = 1748419200 + var buf bytes.Buffer + if err := (&JSONPrinter{w: &buf}).Print(map[string]any{"start_time": ts}, nil); err != nil { + t.Fatalf("Print: %v", err) + } + if strings.Contains(buf.String(), "1748419200") { + t.Fatalf("raw unix timestamp leaked into JSON output: %s", buf.String()) + } + var got map[string]any + if err := json.Unmarshal(buf.Bytes(), &got); err != nil { + t.Fatalf("output not valid JSON: %v", err) + } + if inst := asInstant(t, got["start_time"]); inst != ts { + t.Fatalf("start_time instant = %d, want %d", inst, ts) + } +} + +func TestTOONPrinter_HumanizesTimestamps(t *testing.T) { + var buf bytes.Buffer + if err := (&TOONPrinter{w: &buf}).Print(map[string]any{"start_time": 1748419200}, nil); err != nil { + t.Fatalf("Print: %v", err) + } + out := buf.String() + if strings.Contains(out, "1748419200") { + t.Fatalf("raw unix timestamp leaked into TOON output: %s", out) + } + if !strings.Contains(out, "start_time") { + t.Fatalf("expected start_time key in TOON output: %s", out) + } +} diff --git a/internal/output/json.go b/internal/output/json.go index bd941fe..2104fb7 100644 --- a/internal/output/json.go +++ b/internal/output/json.go @@ -12,7 +12,7 @@ type JSONPrinter struct { } func (p *JSONPrinter) Print(data any, columns []Column) error { - out, err := json.MarshalIndent(data, "", " ") + out, err := json.MarshalIndent(HumanizeTimestamps(data), "", " ") if err != nil { return fmt.Errorf("failed to marshal JSON: %w", err) } diff --git a/internal/output/timestamps.go b/internal/output/timestamps.go new file mode 100644 index 0000000..fd0d052 --- /dev/null +++ b/internal/output/timestamps.go @@ -0,0 +1,95 @@ +package output + +import ( + "bytes" + "encoding/json" + "strings" + "time" +) + +// HumanizeTimestamps returns a copy of v with Unix-timestamp fields rendered as +// RFC3339 strings in the local timezone, leaving everything else untouched. +// +// Flashduty's API returns time fields as bare Unix integers, which is opaque to +// an LLM reading structured (JSON/TOON) output. RFC3339 is unambiguous, +// sortable, and the format models are most fluent in. The local timezone is the +// sandbox/environment timezone, so CLI output, `date`, and the agent's clock +// all agree. +// +// Detection is by JSON field name: a field ending in "_time" or "_at", or named +// exactly "timestamp", whose value is an integer large enough to be a real +// timestamp (>= 1e9 seconds, i.e. year 2001+). Millisecond values (>= 1e12) are +// detected by magnitude. ID-like fields (*_by, *_id, *_ids) are never touched. +// +// Implementation note: v is round-tripped through JSON into a generic structure +// so the same walk handles both typed SDK structs and the map[string]any +// payloads tools build by hand. On any marshal/decode error it returns v +// unchanged — humanization is best-effort and never blocks output. +func HumanizeTimestamps(v any) any { + b, err := json.Marshal(v) + if err != nil { + return v + } + dec := json.NewDecoder(bytes.NewReader(b)) + dec.UseNumber() + var generic any + if err := dec.Decode(&generic); err != nil { + return v + } + return humanizeWalk(generic, "") +} + +func humanizeWalk(v any, key string) any { + switch val := v.(type) { + case map[string]any: + for k, child := range val { + val[k] = humanizeWalk(child, k) + } + return val + case []any: + for i, child := range val { + val[i] = humanizeWalk(child, key) + } + return val + case json.Number: + if isTimestampField(key) { + if s, ok := renderTimestamp(val); ok { + return s + } + } + return val + default: + return val + } +} + +// isTimestampField reports whether a JSON field name denotes an absolute time. +// ID-like suffixes are excluded first so e.g. "timeline_id" / "updated_by" +// never match. +func isTimestampField(key string) bool { + k := strings.ToLower(key) + if strings.HasSuffix(k, "_id") || strings.HasSuffix(k, "_ids") || strings.HasSuffix(k, "_by") { + return false + } + return k == "timestamp" || strings.HasSuffix(k, "_time") || strings.HasSuffix(k, "_at") +} + +// renderTimestamp converts a numeric Unix timestamp to RFC3339 in local time. +// Values below 1e9 are treated as durations/counts, not absolute timestamps, +// and left unconverted; values at/above 1e12 are interpreted as milliseconds. +func renderTimestamp(n json.Number) (string, bool) { + i, err := n.Int64() + if err != nil { + return "", false + } + var t time.Time + switch { + case i >= 1e12: + t = time.UnixMilli(i) + case i >= 1e9: + t = time.Unix(i, 0) + default: + return "", false + } + return t.In(time.Local).Format(time.RFC3339), true +} diff --git a/internal/output/timestamps_test.go b/internal/output/timestamps_test.go new file mode 100644 index 0000000..693e5e1 --- /dev/null +++ b/internal/output/timestamps_test.go @@ -0,0 +1,142 @@ +package output + +import ( + "testing" + "time" +) + +// asInstant parses an RFC3339 string and returns its Unix seconds, failing the +// test if the value isn't a valid RFC3339 timestamp. Keeps assertions +// timezone-independent: we check the rendered instant, not its wall-clock text. +func asInstant(t *testing.T, v any) int64 { + t.Helper() + s, ok := v.(string) + if !ok { + t.Fatalf("expected RFC3339 string, got %T (%v)", v, v) + } + parsed, err := time.Parse(time.RFC3339, s) + if err != nil { + t.Fatalf("value %q is not RFC3339: %v", s, err) + } + return parsed.Unix() +} + +func TestHumanizeTimestamps_ConvertsSeconds(t *testing.T) { + const ts = 1748419200 // 2025-05-28T08:00:00Z + got := HumanizeTimestamps(map[string]any{"start_time": ts}) + m := got.(map[string]any) + if inst := asInstant(t, m["start_time"]); inst != ts { + t.Fatalf("start_time instant = %d, want %d", inst, ts) + } +} + +func TestHumanizeTimestamps_ConvertsMillis(t *testing.T) { + const sec = 1748419200 + got := HumanizeTimestamps(map[string]any{"created_at": int64(sec) * 1000}) + m := got.(map[string]any) + if inst := asInstant(t, m["created_at"]); inst != sec { + t.Fatalf("created_at instant = %d, want %d", inst, sec) + } +} + +func TestHumanizeTimestamps_DetectsByFieldName(t *testing.T) { + const ts = 1748419200 + in := map[string]any{ + "start_time": ts, + "ack_time": ts, + "close_time": ts, + "assigned_at": ts, + "acknowledged_at": ts, + "timestamp": ts, + "trigger_time": ts, + "end_time": ts, + } + m := HumanizeTimestamps(in).(map[string]any) + for k := range in { + if inst := asInstant(t, m[k]); inst != ts { + t.Fatalf("%s instant = %d, want %d", k, inst, ts) + } + } +} + +func TestHumanizeTimestamps_LeavesIDFieldsAlone(t *testing.T) { + // All large enough to convert by magnitude — proves the field-name + // exclusion (not the magnitude guard) is what keeps these numeric. + in := map[string]any{ + "updated_by": int64(1748419200), + "created_by": int64(1748419200), + "timeline_id": int64(1748419200), + "channel_id": int64(1748419200), + "channel_ids": []any{int64(1748419200)}, + } + m := HumanizeTimestamps(in).(map[string]any) + for k := range in { + if _, isStr := m[k].(string); isStr { + t.Fatalf("%s was converted to a string but is an ID field", k) + } + } +} + +func TestHumanizeTimestamps_NilPassesThrough(t *testing.T) { + if got := HumanizeTimestamps(nil); got != nil { + t.Fatalf("HumanizeTimestamps(nil) = %v, want nil", got) + } +} + +func TestHumanizeTimestamps_LeavesSmallDurationsAlone(t *testing.T) { + // A *_time-named field holding a small value is a duration, not an absolute + // unix timestamp — must not be rendered as a 1970 date. + in := map[string]any{"snooze_time": int64(300)} + m := HumanizeTimestamps(in).(map[string]any) + if _, isStr := m["snooze_time"].(string); isStr { + t.Fatalf("snooze_time=300 was converted; small values must stay numeric") + } +} + +func TestHumanizeTimestamps_SkipsZero(t *testing.T) { + in := map[string]any{"ack_time": 0} + m := HumanizeTimestamps(in).(map[string]any) + if _, isStr := m["ack_time"].(string); isStr { + t.Fatalf("ack_time=0 (omitted) must not be rendered as a date") + } +} + +func TestHumanizeTimestamps_RecursesNestedAndSlices(t *testing.T) { + const ts = 1748419200 + in := map[string]any{ + "incidents": []any{ + map[string]any{ + "start_time": ts, + "labels": map[string]any{"close_time": ts}, + }, + }, + } + m := HumanizeTimestamps(in).(map[string]any) + inc := m["incidents"].([]any)[0].(map[string]any) + if inst := asInstant(t, inc["start_time"]); inst != ts { + t.Fatalf("nested start_time instant = %d, want %d", inst, ts) + } + if inst := asInstant(t, inc["labels"].(map[string]any)["close_time"]); inst != ts { + t.Fatalf("deeply nested close_time instant = %d, want %d", inst, ts) + } +} + +func TestHumanizeTimestamps_ConvertsTypedStruct(t *testing.T) { + // Real SDK results are structs, not maps — the helper must humanize them too. + type incident struct { + Title string `json:"title"` + StartTime int64 `json:"start_time"` + UpdatedBy int64 `json:"updated_by"` + } + const ts = 1748419200 + m := HumanizeTimestamps(incident{Title: "db down", StartTime: ts, UpdatedBy: 7}).(map[string]any) + if inst := asInstant(t, m["start_time"]); inst != ts { + t.Fatalf("struct start_time instant = %d, want %d", inst, ts) + } + if _, isStr := m["updated_by"].(string); isStr { + t.Fatalf("struct updated_by must remain numeric") + } + if m["title"] != "db down" { + t.Fatalf("title = %v, want \"db down\"", m["title"]) + } +} diff --git a/internal/output/toon.go b/internal/output/toon.go index dd129f7..5270f72 100644 --- a/internal/output/toon.go +++ b/internal/output/toon.go @@ -16,7 +16,7 @@ type TOONPrinter struct { } func (p *TOONPrinter) Print(data any, _ []Column) error { - out, err := sdk.Marshal(data, sdk.OutputFormatTOON) + out, err := sdk.Marshal(HumanizeTimestamps(data), sdk.OutputFormatTOON) if err != nil { return fmt.Errorf("failed to marshal TOON: %w", err) }