From 8ead9d1835a1b5a49b56a0c75c2af631257e0fd3 Mon Sep 17 00:00:00 2001 From: ysyneu Date: Fri, 29 May 2026 00:03:25 +0800 Subject: [PATCH] feat: render API timestamps as RFC3339 in tool output Flashduty's API returns time fields as bare Unix integers, which are opaque to an LLM reading tool results. Humanize them to RFC3339 (in the local timezone) in MarshalResultWithFormat so every tool response is readable. Detection is by field name (*_time, *_at, timestamp), excluding ID-like fields; values below 1e9 stay numeric. Drop the unused, humanization-bypassing MarshalledTextResult. --- pkg/flashduty/format.go | 9 +-- pkg/flashduty/timestamps.go | 94 ++++++++++++++++++++++++ pkg/flashduty/timestamps_test.go | 122 +++++++++++++++++++++++++++++++ 3 files changed, 217 insertions(+), 8 deletions(-) create mode 100644 pkg/flashduty/timestamps.go create mode 100644 pkg/flashduty/timestamps_test.go diff --git a/pkg/flashduty/format.go b/pkg/flashduty/format.go index 4d5cb81..43213cd 100644 --- a/pkg/flashduty/format.go +++ b/pkg/flashduty/format.go @@ -41,16 +41,9 @@ func MarshalResult(v any) *mcp.CallToolResult { // MarshalResultWithFormat serializes the given value using the specified format func MarshalResultWithFormat(v any, format OutputFormat) *mcp.CallToolResult { - data, err := sdk.Marshal(v, format) + data, err := sdk.Marshal(humanizeTimestamps(v), format) if err != nil { return mcp.NewToolResultError(fmt.Sprintf("failed to marshal result: %v", err)) } return mcp.NewToolResultText(string(data)) } - -// MarshalledTextResult is the original function that always uses JSON. -// Kept for backward compatibility. New code should use MarshalResult. -func MarshalledTextResult(v any) *mcp.CallToolResult { - data, _ := sdk.Marshal(v, OutputFormatJSON) - return mcp.NewToolResultText(string(data)) -} diff --git a/pkg/flashduty/timestamps.go b/pkg/flashduty/timestamps.go new file mode 100644 index 0000000..41bd3aa --- /dev/null +++ b/pkg/flashduty/timestamps.go @@ -0,0 +1,94 @@ +package flashduty + +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 tool output. RFC3339 is unambiguous, sortable, and the format +// models are most fluent in. The local timezone is the process timezone (the +// sandbox/environment timezone when the server runs inside an agent sandbox). +// +// 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. +// +// 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/pkg/flashduty/timestamps_test.go b/pkg/flashduty/timestamps_test.go new file mode 100644 index 0000000..1ed7b38 --- /dev/null +++ b/pkg/flashduty/timestamps_test.go @@ -0,0 +1,122 @@ +package flashduty + +import ( + "strings" + "testing" + "time" + + "github.com/mark3labs/mcp-go/mcp" +) + +// TestMarshalResult_HumanizesTimestamps locks the wiring: every tool result +// routed through MarshalResultWithFormat must have its timestamps humanized, so +// a raw Unix integer never reaches the model. +func TestMarshalResult_HumanizesTimestamps(t *testing.T) { + const ts = 1748419200 + res := MarshalResultWithFormat(map[string]any{"start_time": ts}, OutputFormatJSON) + tc, ok := mcp.AsTextContent(res.Content[0]) + if !ok { + t.Fatalf("expected text content, got %#v", res.Content[0]) + } + if strings.Contains(tc.Text, "1748419200") { + t.Fatalf("raw unix timestamp leaked into tool result: %s", tc.Text) + } + if !strings.Contains(tc.Text, "start_time") { + t.Fatalf("expected start_time key in result: %s", tc.Text) + } +} + +func tsInstant(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_ConvertsSecondsAndMillis(t *testing.T) { + const sec = 1748419200 + m := humanizeTimestamps(map[string]any{ + "start_time": sec, + "created_at": int64(sec) * 1000, + }).(map[string]any) + if inst := tsInstant(t, m["start_time"]); inst != sec { + t.Fatalf("start_time instant = %d, want %d", inst, sec) + } + if inst := tsInstant(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{ + "ack_time": ts, "close_time": ts, "assigned_at": ts, + "acknowledged_at": ts, "timestamp": ts, "end_time": ts, "trigger_time": ts, + } + m := humanizeTimestamps(in).(map[string]any) + for k := range in { + if inst := tsInstant(t, m[k]); inst != ts { + t.Fatalf("%s instant = %d, want %d", k, inst, ts) + } + } +} + +func TestHumanizeTimestamps_LeavesIDAndDurationFields(t *testing.T) { + in := map[string]any{ + // Large values that WOULD convert by magnitude — proves the field-name + // exclusion (not just the magnitude guard) is what keeps IDs numeric. + "updated_by": int64(1748419200), + "timeline_id": int64(1748419200), + "channel_ids": []any{int64(1748419200)}, + "snooze_time": int64(300), // small => duration, not a 1970 date + "ack_time": 0, // zero => not a timestamp + } + m := humanizeTimestamps(in).(map[string]any) + for k := range in { + if _, isStr := m[k].(string); isStr { + t.Fatalf("%s must not be converted to a date string", k) + } + } +} + +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 := tsInstant(t, inc["start_time"]); inst != ts { + t.Fatalf("nested start_time instant = %d, want %d", inst, ts) + } + if inst := tsInstant(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) { + 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 := tsInstant(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"]) + } +}