From 51caa5ea433ac6807d9a218e7056c8825fd76f8d Mon Sep 17 00:00:00 2001 From: ysyneu Date: Mon, 1 Jun 2026 12:38:47 +0800 Subject: [PATCH] feat(incident): resolve 6-char short ids in detail/get + add list --nums `incident detail ` and `incident get ` assumed a full 24-char ObjectID and sent the positional arg straight into ObjectID-typed fields, so a 6-char short id (the "num" shown in the UI, e.g. 311510) failed with HTTP 400. The only backend path that accepts a num is /incident/list, which the agent previously reached only by luck and a wide-enough --since. Detect a 6-hex arg and resolve it to a full id via /incident/list with `nums` over the last 30 days (the list API caps its query span at ~30 days): - unique hit -> proceed with the resolved full id - multiple -> surface the candidates (num is non-unique by design) and stop; never silently analyze the wrong incident - none -> clear error pointing at the full-id fallback A 24-char id (or any non-short value) passes through unchanged. Also add `incident list --nums` for explicit short-id filtering. --- internal/cli/incident.go | 88 ++++++++++- internal/cli/incident_short_id_test.go | 200 +++++++++++++++++++++++++ 2 files changed, 285 insertions(+), 3 deletions(-) create mode 100644 internal/cli/incident_short_id_test.go diff --git a/internal/cli/incident.go b/internal/cli/incident.go index 9393abe..54d4012 100644 --- a/internal/cli/incident.go +++ b/internal/cli/incident.go @@ -2,9 +2,11 @@ package cli import ( "bufio" + "errors" "fmt" "io" "os" + "regexp" "strconv" "strings" "time" @@ -73,7 +75,7 @@ func pastIncidentColumns() []output.Column { } func newIncidentListCmd() *cobra.Command { - var progress, severity, query, since, until string + var progress, severity, query, since, until, nums string var channelID int64 var limit, page int @@ -103,6 +105,9 @@ func newIncidentListCmd() *cobra.Command { if channelID != 0 { req.ChannelIDs = []int64{channelID} } + if nums != "" { + req.Nums = parseStringSlice(nums) + } result, _, err := ctx.Client.Incidents.List(cmdContext(ctx.Cmd), req) if err != nil { @@ -118,6 +123,7 @@ func newIncidentListCmd() *cobra.Command { cmd.Flags().StringVar(&severity, "severity", "", "Filter: Critical,Warning,Info") cmd.Flags().Int64Var(&channelID, "channel", 0, "Filter by channel ID") cmd.Flags().StringVar(&query, "query", "", "Free-text search across title/labels/content (also resolves a 24-char incident ID or 6-char incident num to a direct lookup)") + cmd.Flags().StringVar(&nums, "nums", "", "Comma-separated short incident ids (num, the 6-char id shown in the UI) to filter by") cmd.Flags().StringVar(&since, "since", "24h", "Start time (duration, date, datetime, or unix timestamp)") cmd.Flags().StringVar(&until, "until", "now", "End time") cmd.Flags().IntVar(&limit, "limit", 20, "Max results (max 100)") @@ -126,6 +132,62 @@ func newIncidentListCmd() *cobra.Command { return cmd } +// shortIDResolveDays bounds how far back a 6-char short id is resolved. +// /incident/list is the only endpoint that accepts a num, and the backend caps +// its query span at ~30 days, so older incidents can only be looked up by their +// full 24-char id. +const shortIDResolveDays = 30 + +var reShortIncidentID = regexp.MustCompile(`^[0-9a-fA-F]{6}$`) + +// resolveIncidentArg maps a user-supplied incident argument to a full 24-char +// incident id. A full id — or any value that isn't a 6-char short id — passes +// through unchanged, preserving existing behavior. A 6-char short id ("num", as +// shown in the UI) is resolved against the last shortIDResolveDays days via +// /incident/list. A unique hit returns the full id; multiple hits return the +// candidates (the short id is non-unique by design) so the caller can surface +// them; no hit returns a descriptive error. +func resolveIncidentArg(ctx *RunContext, arg string) (fullID string, candidates []flashduty.IncidentInfo, err error) { + if !reShortIncidentID.MatchString(arg) { + return arg, nil, nil + } + + end := time.Now().Unix() + start := end - int64(shortIDResolveDays)*24*60*60 + res, _, err := ctx.Client.Incidents.List(cmdContext(ctx.Cmd), &flashduty.ListIncidentsRequest{ + Nums: []string{strings.ToUpper(arg)}, + StartTime: start, + EndTime: end, + }) + if err != nil { + return "", nil, err + } + + switch len(res.Items) { + case 0: + return "", nil, fmt.Errorf( + "no incident with short id %q in the last %d days; older incidents must be queried by their full 24-char id", + arg, shortIDResolveDays) + case 1: + return res.Items[0].IncidentID, nil, nil + default: + return "", res.Items, nil + } +} + +// ambiguousShortIDError reports a short id that resolved to more than one +// incident, listing each candidate's full id so the caller can disambiguate. +func ambiguousShortIDError(shortID string, candidates []flashduty.IncidentInfo) error { + var b strings.Builder + _, _ = fmt.Fprintf(&b, "short id %q matches %d incidents in the last %d days — re-run with one of these full ids:", + shortID, len(candidates), shortIDResolveDays) + for _, c := range candidates { + _, _ = fmt.Fprintf(&b, "\n %s %-8s %-10s %s %s", + c.IncidentID, orDash(c.IncidentSeverity), orDash(c.Progress), output.FormatTime(c.StartTime), c.Title) + } + return errors.New(b.String()) +} + func newIncidentGetCmd() *cobra.Command { return &cobra.Command{ Use: "get [ ...]", @@ -133,8 +195,20 @@ func newIncidentGetCmd() *cobra.Command { Args: requireArgs("incident_id"), RunE: func(cmd *cobra.Command, args []string) error { return runCommand(cmd, args, func(ctx *RunContext) error { + ids := make([]string, 0, len(ctx.Args)) + for _, a := range ctx.Args { + fullID, candidates, err := resolveIncidentArg(ctx, a) + if err != nil { + return err + } + if len(candidates) > 0 { + return ambiguousShortIDError(a, candidates) + } + ids = append(ids, fullID) + } + result, _, err := ctx.Client.Incidents.List(cmdContext(ctx.Cmd), &flashduty.ListIncidentsRequest{ - IncidentIDs: ctx.Args, + IncidentIDs: ids, }) if err != nil { return err @@ -1333,8 +1407,16 @@ func newIncidentDetailCmd() *cobra.Command { Args: requireArgs("incident_id"), RunE: func(cmd *cobra.Command, args []string) error { return runCommand(cmd, args, func(ctx *RunContext) error { + fullID, candidates, err := resolveIncidentArg(ctx, ctx.Args[0]) + if err != nil { + return err + } + if len(candidates) > 0 { + return ambiguousShortIDError(ctx.Args[0], candidates) + } + result, _, err := ctx.Client.Incidents.Info(cmdContext(ctx.Cmd), &flashduty.IncidentInfoRequest{ - IncidentID: ctx.Args[0], + IncidentID: fullID, }) if err != nil { return err diff --git a/internal/cli/incident_short_id_test.go b/internal/cli/incident_short_id_test.go new file mode 100644 index 0000000..0323978 --- /dev/null +++ b/internal/cli/incident_short_id_test.go @@ -0,0 +1,200 @@ +package cli + +import ( + "strings" + "testing" +) + +const ( + testShortID = "311510" + testFullID = "6a12a4502f0a2396b3311510" +) + +func incidentListData(items ...map[string]any) map[string]any { + return map[string]any{"items": items, "total": len(items)} +} + +func incidentItem(id, num, title string) map[string]any { + return map[string]any{"incident_id": id, "num": num, "title": title} +} + +// TestIncidentDetailShortIDResolves: `detail <6-hex>` first resolves the short +// id via /incident/list (nums + a 30-day window), then fetches /incident/info +// with the RESOLVED full id — never the short id. +func TestIncidentDetailShortIDResolves(t *testing.T) { + saveAndResetGlobals(t) + stub := newGFStub(t) + + var paths []string + stub.dataForPath = func(path string, body map[string]any) any { + paths = append(paths, path) + switch path { + case "/incident/list": + return incidentListData(incidentItem(testFullID, testShortID, "kafka backlog")) + case "/incident/info": + return incidentItem(testFullID, testShortID, "kafka backlog") + default: + return nil + } + } + + if _, err := execCommand("incident", "detail", testShortID); err != nil { + t.Fatalf("execCommand: %v", err) + } + + if want := []string{"/incident/list", "/incident/info"}; !equalStrings(paths, want) { + t.Fatalf("paths = %v, want %v", paths, want) + } + + // Resolve call: nums set, 30-day span. + listBody := stub.bodies[0] + nums, _ := listBody["nums"].([]any) + if len(nums) != 1 || nums[0] != testShortID { + t.Errorf("resolve nums = %#v, want [%q]", listBody["nums"], testShortID) + } + st, _ := listBody["start_time"].(float64) + et, _ := listBody["end_time"].(float64) + if et <= st { + t.Errorf("end_time %v must be > start_time %v", et, st) + } + if span := et - st; span != float64(shortIDResolveDays*24*60*60) { + t.Errorf("resolve span = %v s, want %d-day span", span, shortIDResolveDays) + } + + // Detail call: the resolved full id, not the short id. + if got := stub.bodies[1]["incident_id"]; got != testFullID { + t.Errorf("info incident_id = %#v, want resolved full id %q", got, testFullID) + } +} + +// TestIncidentDetailShortIDAmbiguous: a short id that matches >1 incident fails +// with a candidate list and never calls /incident/info (no silent wrong-pick). +func TestIncidentDetailShortIDAmbiguous(t *testing.T) { + saveAndResetGlobals(t) + stub := newGFStub(t) + + other := "6a0ea49000000000b3311510" + var paths []string + stub.dataForPath = func(path string, body map[string]any) any { + paths = append(paths, path) + if path == "/incident/list" { + return incidentListData( + incidentItem(testFullID, testShortID, "kafka backlog"), + incidentItem(other, testShortID, "another incident"), + ) + } + return nil + } + + _, err := execCommand("incident", "detail", testShortID) + if err == nil { + t.Fatal("want ambiguous error, got nil") + } + if !strings.Contains(err.Error(), testFullID) || !strings.Contains(err.Error(), other) { + t.Errorf("error should list both candidate ids, got: %v", err) + } + if want := []string{"/incident/list"}; !equalStrings(paths, want) { + t.Errorf("paths = %v, want %v (info must be skipped on ambiguity)", paths, want) + } +} + +// TestIncidentDetailShortIDNotFound: a short id with no match in the window +// fails with a descriptive error pointing at the full-id fallback. +func TestIncidentDetailShortIDNotFound(t *testing.T) { + saveAndResetGlobals(t) + stub := newGFStub(t) + stub.dataForPath = func(path string, body map[string]any) any { + if path == "/incident/list" { + return incidentListData() + } + return nil + } + + _, err := execCommand("incident", "detail", testShortID) + if err == nil || !strings.Contains(err.Error(), "no incident with short id") { + t.Errorf("want not-found error, got %v", err) + } +} + +// TestIncidentDetailFullIDSkipsResolve: a 24-hex id goes straight to +// /incident/info with no resolve round-trip (existing behavior preserved). +func TestIncidentDetailFullIDSkipsResolve(t *testing.T) { + saveAndResetGlobals(t) + stub := newGFStub(t) + var paths []string + stub.dataForPath = func(path string, body map[string]any) any { + paths = append(paths, path) + if path == "/incident/info" { + return incidentItem(testFullID, testShortID, "kafka backlog") + } + return nil + } + + if _, err := execCommand("incident", "detail", testFullID); err != nil { + t.Fatalf("execCommand: %v", err) + } + if want := []string{"/incident/info"}; !equalStrings(paths, want) { + t.Fatalf("paths = %v, want %v (full id must not trigger a resolve)", paths, want) + } + if got := stub.bodies[0]["incident_id"]; got != testFullID { + t.Errorf("info incident_id = %#v, want %q", got, testFullID) + } +} + +// TestIncidentGetShortIDResolves: `get <6-hex>` resolves the short id, then +// fetches by the resolved full id via incident_ids. +func TestIncidentGetShortIDResolves(t *testing.T) { + saveAndResetGlobals(t) + stub := newGFStub(t) + stub.dataForPath = func(path string, body map[string]any) any { + // Both the resolve and the final fetch hit /incident/list; the canned + // row is fine for either. + return incidentListData(incidentItem(testFullID, testShortID, "kafka backlog")) + } + + if _, err := execCommand("incident", "get", testShortID); err != nil { + t.Fatalf("execCommand: %v", err) + } + if stub.requests != 2 { + t.Fatalf("requests = %d, want 2 (resolve + fetch)", stub.requests) + } + + // Resolve sent nums; final fetch sent the resolved full id via incident_ids. + if _, ok := stub.bodies[0]["nums"]; !ok { + t.Errorf("first request should carry nums, got %#v", stub.bodies[0]) + } + ids, _ := stub.bodies[1]["incident_ids"].([]any) + if len(ids) != 1 || ids[0] != testFullID { + t.Errorf("fetch incident_ids = %#v, want [%q]", stub.bodies[1]["incident_ids"], testFullID) + } +} + +// TestIncidentListNumsReachesWire: --nums is split and sent as the nums array. +func TestIncidentListNumsReachesWire(t *testing.T) { + saveAndResetGlobals(t) + stub := newGFStub(t) + stub.data = incidentListData() + + if _, err := execCommand("incident", "list", "--nums", "311510,ABC123"); err != nil { + t.Fatalf("execCommand: %v", err) + } + if stub.lastPath != "/incident/list" { + t.Fatalf("path = %q, want /incident/list", stub.lastPath) + } + nums, _ := stub.lastBody["nums"].([]any) + if len(nums) != 2 || nums[0] != "311510" || nums[1] != "ABC123" { + t.Errorf("nums = %#v, want [311510 ABC123]", stub.lastBody["nums"]) + } +} + +func equalStrings(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +}