Skip to content

Commit 4ac23d1

Browse files
authored
feat(incident): resolve 6-char short ids in detail/get + add list --nums (#26)
`incident detail <id>` and `incident get <id>` 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.
1 parent c9000d5 commit 4ac23d1

2 files changed

Lines changed: 285 additions & 3 deletions

File tree

internal/cli/incident.go

Lines changed: 85 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@ package cli
22

33
import (
44
"bufio"
5+
"errors"
56
"fmt"
67
"io"
78
"os"
9+
"regexp"
810
"strconv"
911
"strings"
1012
"time"
@@ -73,7 +75,7 @@ func pastIncidentColumns() []output.Column {
7375
}
7476

7577
func newIncidentListCmd() *cobra.Command {
76-
var progress, severity, query, since, until string
78+
var progress, severity, query, since, until, nums string
7779
var channelID int64
7880
var limit, page int
7981

@@ -103,6 +105,9 @@ func newIncidentListCmd() *cobra.Command {
103105
if channelID != 0 {
104106
req.ChannelIDs = []int64{channelID}
105107
}
108+
if nums != "" {
109+
req.Nums = parseStringSlice(nums)
110+
}
106111

107112
result, _, err := ctx.Client.Incidents.List(cmdContext(ctx.Cmd), req)
108113
if err != nil {
@@ -118,6 +123,7 @@ func newIncidentListCmd() *cobra.Command {
118123
cmd.Flags().StringVar(&severity, "severity", "", "Filter: Critical,Warning,Info")
119124
cmd.Flags().Int64Var(&channelID, "channel", 0, "Filter by channel ID")
120125
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)")
126+
cmd.Flags().StringVar(&nums, "nums", "", "Comma-separated short incident ids (num, the 6-char id shown in the UI) to filter by")
121127
cmd.Flags().StringVar(&since, "since", "24h", "Start time (duration, date, datetime, or unix timestamp)")
122128
cmd.Flags().StringVar(&until, "until", "now", "End time")
123129
cmd.Flags().IntVar(&limit, "limit", 20, "Max results (max 100)")
@@ -126,15 +132,83 @@ func newIncidentListCmd() *cobra.Command {
126132
return cmd
127133
}
128134

135+
// shortIDResolveDays bounds how far back a 6-char short id is resolved.
136+
// /incident/list is the only endpoint that accepts a num, and the backend caps
137+
// its query span at ~30 days, so older incidents can only be looked up by their
138+
// full 24-char id.
139+
const shortIDResolveDays = 30
140+
141+
var reShortIncidentID = regexp.MustCompile(`^[0-9a-fA-F]{6}$`)
142+
143+
// resolveIncidentArg maps a user-supplied incident argument to a full 24-char
144+
// incident id. A full id — or any value that isn't a 6-char short id — passes
145+
// through unchanged, preserving existing behavior. A 6-char short id ("num", as
146+
// shown in the UI) is resolved against the last shortIDResolveDays days via
147+
// /incident/list. A unique hit returns the full id; multiple hits return the
148+
// candidates (the short id is non-unique by design) so the caller can surface
149+
// them; no hit returns a descriptive error.
150+
func resolveIncidentArg(ctx *RunContext, arg string) (fullID string, candidates []flashduty.IncidentInfo, err error) {
151+
if !reShortIncidentID.MatchString(arg) {
152+
return arg, nil, nil
153+
}
154+
155+
end := time.Now().Unix()
156+
start := end - int64(shortIDResolveDays)*24*60*60
157+
res, _, err := ctx.Client.Incidents.List(cmdContext(ctx.Cmd), &flashduty.ListIncidentsRequest{
158+
Nums: []string{strings.ToUpper(arg)},
159+
StartTime: start,
160+
EndTime: end,
161+
})
162+
if err != nil {
163+
return "", nil, err
164+
}
165+
166+
switch len(res.Items) {
167+
case 0:
168+
return "", nil, fmt.Errorf(
169+
"no incident with short id %q in the last %d days; older incidents must be queried by their full 24-char id",
170+
arg, shortIDResolveDays)
171+
case 1:
172+
return res.Items[0].IncidentID, nil, nil
173+
default:
174+
return "", res.Items, nil
175+
}
176+
}
177+
178+
// ambiguousShortIDError reports a short id that resolved to more than one
179+
// incident, listing each candidate's full id so the caller can disambiguate.
180+
func ambiguousShortIDError(shortID string, candidates []flashduty.IncidentInfo) error {
181+
var b strings.Builder
182+
_, _ = fmt.Fprintf(&b, "short id %q matches %d incidents in the last %d days — re-run with one of these full ids:",
183+
shortID, len(candidates), shortIDResolveDays)
184+
for _, c := range candidates {
185+
_, _ = fmt.Fprintf(&b, "\n %s %-8s %-10s %s %s",
186+
c.IncidentID, orDash(c.IncidentSeverity), orDash(c.Progress), output.FormatTime(c.StartTime), c.Title)
187+
}
188+
return errors.New(b.String())
189+
}
190+
129191
func newIncidentGetCmd() *cobra.Command {
130192
return &cobra.Command{
131193
Use: "get <id> [<id2> ...]",
132194
Short: "Get incident details",
133195
Args: requireArgs("incident_id"),
134196
RunE: func(cmd *cobra.Command, args []string) error {
135197
return runCommand(cmd, args, func(ctx *RunContext) error {
198+
ids := make([]string, 0, len(ctx.Args))
199+
for _, a := range ctx.Args {
200+
fullID, candidates, err := resolveIncidentArg(ctx, a)
201+
if err != nil {
202+
return err
203+
}
204+
if len(candidates) > 0 {
205+
return ambiguousShortIDError(a, candidates)
206+
}
207+
ids = append(ids, fullID)
208+
}
209+
136210
result, _, err := ctx.Client.Incidents.List(cmdContext(ctx.Cmd), &flashduty.ListIncidentsRequest{
137-
IncidentIDs: ctx.Args,
211+
IncidentIDs: ids,
138212
})
139213
if err != nil {
140214
return err
@@ -1333,8 +1407,16 @@ func newIncidentDetailCmd() *cobra.Command {
13331407
Args: requireArgs("incident_id"),
13341408
RunE: func(cmd *cobra.Command, args []string) error {
13351409
return runCommand(cmd, args, func(ctx *RunContext) error {
1410+
fullID, candidates, err := resolveIncidentArg(ctx, ctx.Args[0])
1411+
if err != nil {
1412+
return err
1413+
}
1414+
if len(candidates) > 0 {
1415+
return ambiguousShortIDError(ctx.Args[0], candidates)
1416+
}
1417+
13361418
result, _, err := ctx.Client.Incidents.Info(cmdContext(ctx.Cmd), &flashduty.IncidentInfoRequest{
1337-
IncidentID: ctx.Args[0],
1419+
IncidentID: fullID,
13381420
})
13391421
if err != nil {
13401422
return err
Lines changed: 200 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,200 @@
1+
package cli
2+
3+
import (
4+
"strings"
5+
"testing"
6+
)
7+
8+
const (
9+
testShortID = "311510"
10+
testFullID = "6a12a4502f0a2396b3311510"
11+
)
12+
13+
func incidentListData(items ...map[string]any) map[string]any {
14+
return map[string]any{"items": items, "total": len(items)}
15+
}
16+
17+
func incidentItem(id, num, title string) map[string]any {
18+
return map[string]any{"incident_id": id, "num": num, "title": title}
19+
}
20+
21+
// TestIncidentDetailShortIDResolves: `detail <6-hex>` first resolves the short
22+
// id via /incident/list (nums + a 30-day window), then fetches /incident/info
23+
// with the RESOLVED full id — never the short id.
24+
func TestIncidentDetailShortIDResolves(t *testing.T) {
25+
saveAndResetGlobals(t)
26+
stub := newGFStub(t)
27+
28+
var paths []string
29+
stub.dataForPath = func(path string, body map[string]any) any {
30+
paths = append(paths, path)
31+
switch path {
32+
case "/incident/list":
33+
return incidentListData(incidentItem(testFullID, testShortID, "kafka backlog"))
34+
case "/incident/info":
35+
return incidentItem(testFullID, testShortID, "kafka backlog")
36+
default:
37+
return nil
38+
}
39+
}
40+
41+
if _, err := execCommand("incident", "detail", testShortID); err != nil {
42+
t.Fatalf("execCommand: %v", err)
43+
}
44+
45+
if want := []string{"/incident/list", "/incident/info"}; !equalStrings(paths, want) {
46+
t.Fatalf("paths = %v, want %v", paths, want)
47+
}
48+
49+
// Resolve call: nums set, 30-day span.
50+
listBody := stub.bodies[0]
51+
nums, _ := listBody["nums"].([]any)
52+
if len(nums) != 1 || nums[0] != testShortID {
53+
t.Errorf("resolve nums = %#v, want [%q]", listBody["nums"], testShortID)
54+
}
55+
st, _ := listBody["start_time"].(float64)
56+
et, _ := listBody["end_time"].(float64)
57+
if et <= st {
58+
t.Errorf("end_time %v must be > start_time %v", et, st)
59+
}
60+
if span := et - st; span != float64(shortIDResolveDays*24*60*60) {
61+
t.Errorf("resolve span = %v s, want %d-day span", span, shortIDResolveDays)
62+
}
63+
64+
// Detail call: the resolved full id, not the short id.
65+
if got := stub.bodies[1]["incident_id"]; got != testFullID {
66+
t.Errorf("info incident_id = %#v, want resolved full id %q", got, testFullID)
67+
}
68+
}
69+
70+
// TestIncidentDetailShortIDAmbiguous: a short id that matches >1 incident fails
71+
// with a candidate list and never calls /incident/info (no silent wrong-pick).
72+
func TestIncidentDetailShortIDAmbiguous(t *testing.T) {
73+
saveAndResetGlobals(t)
74+
stub := newGFStub(t)
75+
76+
other := "6a0ea49000000000b3311510"
77+
var paths []string
78+
stub.dataForPath = func(path string, body map[string]any) any {
79+
paths = append(paths, path)
80+
if path == "/incident/list" {
81+
return incidentListData(
82+
incidentItem(testFullID, testShortID, "kafka backlog"),
83+
incidentItem(other, testShortID, "another incident"),
84+
)
85+
}
86+
return nil
87+
}
88+
89+
_, err := execCommand("incident", "detail", testShortID)
90+
if err == nil {
91+
t.Fatal("want ambiguous error, got nil")
92+
}
93+
if !strings.Contains(err.Error(), testFullID) || !strings.Contains(err.Error(), other) {
94+
t.Errorf("error should list both candidate ids, got: %v", err)
95+
}
96+
if want := []string{"/incident/list"}; !equalStrings(paths, want) {
97+
t.Errorf("paths = %v, want %v (info must be skipped on ambiguity)", paths, want)
98+
}
99+
}
100+
101+
// TestIncidentDetailShortIDNotFound: a short id with no match in the window
102+
// fails with a descriptive error pointing at the full-id fallback.
103+
func TestIncidentDetailShortIDNotFound(t *testing.T) {
104+
saveAndResetGlobals(t)
105+
stub := newGFStub(t)
106+
stub.dataForPath = func(path string, body map[string]any) any {
107+
if path == "/incident/list" {
108+
return incidentListData()
109+
}
110+
return nil
111+
}
112+
113+
_, err := execCommand("incident", "detail", testShortID)
114+
if err == nil || !strings.Contains(err.Error(), "no incident with short id") {
115+
t.Errorf("want not-found error, got %v", err)
116+
}
117+
}
118+
119+
// TestIncidentDetailFullIDSkipsResolve: a 24-hex id goes straight to
120+
// /incident/info with no resolve round-trip (existing behavior preserved).
121+
func TestIncidentDetailFullIDSkipsResolve(t *testing.T) {
122+
saveAndResetGlobals(t)
123+
stub := newGFStub(t)
124+
var paths []string
125+
stub.dataForPath = func(path string, body map[string]any) any {
126+
paths = append(paths, path)
127+
if path == "/incident/info" {
128+
return incidentItem(testFullID, testShortID, "kafka backlog")
129+
}
130+
return nil
131+
}
132+
133+
if _, err := execCommand("incident", "detail", testFullID); err != nil {
134+
t.Fatalf("execCommand: %v", err)
135+
}
136+
if want := []string{"/incident/info"}; !equalStrings(paths, want) {
137+
t.Fatalf("paths = %v, want %v (full id must not trigger a resolve)", paths, want)
138+
}
139+
if got := stub.bodies[0]["incident_id"]; got != testFullID {
140+
t.Errorf("info incident_id = %#v, want %q", got, testFullID)
141+
}
142+
}
143+
144+
// TestIncidentGetShortIDResolves: `get <6-hex>` resolves the short id, then
145+
// fetches by the resolved full id via incident_ids.
146+
func TestIncidentGetShortIDResolves(t *testing.T) {
147+
saveAndResetGlobals(t)
148+
stub := newGFStub(t)
149+
stub.dataForPath = func(path string, body map[string]any) any {
150+
// Both the resolve and the final fetch hit /incident/list; the canned
151+
// row is fine for either.
152+
return incidentListData(incidentItem(testFullID, testShortID, "kafka backlog"))
153+
}
154+
155+
if _, err := execCommand("incident", "get", testShortID); err != nil {
156+
t.Fatalf("execCommand: %v", err)
157+
}
158+
if stub.requests != 2 {
159+
t.Fatalf("requests = %d, want 2 (resolve + fetch)", stub.requests)
160+
}
161+
162+
// Resolve sent nums; final fetch sent the resolved full id via incident_ids.
163+
if _, ok := stub.bodies[0]["nums"]; !ok {
164+
t.Errorf("first request should carry nums, got %#v", stub.bodies[0])
165+
}
166+
ids, _ := stub.bodies[1]["incident_ids"].([]any)
167+
if len(ids) != 1 || ids[0] != testFullID {
168+
t.Errorf("fetch incident_ids = %#v, want [%q]", stub.bodies[1]["incident_ids"], testFullID)
169+
}
170+
}
171+
172+
// TestIncidentListNumsReachesWire: --nums is split and sent as the nums array.
173+
func TestIncidentListNumsReachesWire(t *testing.T) {
174+
saveAndResetGlobals(t)
175+
stub := newGFStub(t)
176+
stub.data = incidentListData()
177+
178+
if _, err := execCommand("incident", "list", "--nums", "311510,ABC123"); err != nil {
179+
t.Fatalf("execCommand: %v", err)
180+
}
181+
if stub.lastPath != "/incident/list" {
182+
t.Fatalf("path = %q, want /incident/list", stub.lastPath)
183+
}
184+
nums, _ := stub.lastBody["nums"].([]any)
185+
if len(nums) != 2 || nums[0] != "311510" || nums[1] != "ABC123" {
186+
t.Errorf("nums = %#v, want [311510 ABC123]", stub.lastBody["nums"])
187+
}
188+
}
189+
190+
func equalStrings(a, b []string) bool {
191+
if len(a) != len(b) {
192+
return false
193+
}
194+
for i := range a {
195+
if a[i] != b[i] {
196+
return false
197+
}
198+
}
199+
return true
200+
}

0 commit comments

Comments
 (0)