Skip to content

Commit df25898

Browse files
committed
fix(cli): make incident list --channel match sibling list verbs
incident list was the only list verb that filtered by channel via --channel-id (a single int64), while alert list, alert-event list, and change list all use --channel (a comma-separated string parsed into multiple channel IDs). The underlying API (ListIncidentsRequest.ChannelIDs []int64) already supports multiple channel IDs, so incident list's flag was an unnecessary outlier rather than a capability gap. Add --channel string to incident list, parsed with the same parseIntSlice helper alert list and change list already use, and forward it as ChannelIDs. Keep --channel-id working as a deprecated, hidden single-ID alias via cobra's MarkDeprecated so existing scripts do not break; --channel wins when both are set. internal/skilldoc/build.go's command() walked every flag via Flags().VisitAll without skipping hidden ones, so the generated skill card for incident list would have kept showing the now-hidden --channel-id. Filter out hidden flags there, matching the existing hidden/deprecated skip one level up for commands themselves, then regenerate skills/flashduty/reference/incident.md via 'make gen-cards'. Verified: - go build ./... and go test ./... pass - 'incident list --help' shows --channel and no longer shows --channel-id - 'incident list --channel-id 1 --help' still exits 0 and prints pflag's deprecation notice on stderr - 'make check-cards' passes, confirming the skill card and CLI flag set are back in lockstep
1 parent d6be655 commit df25898

5 files changed

Lines changed: 120 additions & 12 deletions

File tree

internal/cli/incident.go

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func pastIncidentColumns() []output.Column {
7272
}
7373

7474
func newIncidentListCmd() *cobra.Command {
75-
var progress, severity, query, since, until, nums, fields string
75+
var progress, severity, query, since, until, nums, fields, channel string
7676
var channelID int64
7777
var limit, page int
7878
defaultStructuredFields := []string{"incident_id", "title", "incident_severity", "progress", "start_time", "channel_id"}
@@ -101,7 +101,17 @@ func newIncidentListCmd() *cobra.Command {
101101
}
102102
req.Page = page
103103
req.Limit = limit
104-
if channelID != 0 {
104+
if channel != "" {
105+
channelIDs, err := parseIntSlice(channel)
106+
if err != nil {
107+
return fmt.Errorf("invalid --channel: %w", err)
108+
}
109+
req.ChannelIDs = channelIDs
110+
} else if channelID != 0 {
111+
// --channel-id is a deprecated single-ID alias kept for scripts
112+
// written before --channel existed; --channel above is canonical
113+
// and wins when both are set. parseIntSlice/--channel is exactly
114+
// the pattern alert list and change list already use.
105115
req.ChannelIDs = []int64{channelID}
106116
}
107117
if nums != "" {
@@ -144,9 +154,16 @@ func newIncidentListCmd() *cobra.Command {
144154
cmd.Flags().StringVar(&severity, "severity", "", "Filter: Critical,Warning,Info")
145155
registerEnumFlag(cmd, "progress", "Triggered", "Processing", "Closed")
146156
registerEnumFlag(cmd, "severity", severityEnum...)
147-
// --channel-id matches the sibling channel commands (channel info
148-
// --channel-id, channel escalate-rule-list --channel-id).
149-
cmd.Flags().Int64Var(&channelID, "channel-id", 0, "Filter by channel ID")
157+
// --channel matches the sibling list verbs (alert list --channel,
158+
// alert-event list --channel, change list --channel): comma-separated
159+
// channel IDs, forwarded to the API's channel_ids ([]int64) field.
160+
cmd.Flags().StringVar(&channel, "channel", "", "Comma-separated channel IDs")
161+
// --channel-id is kept as a deprecated single-ID alias for existing
162+
// scripts (this is a public CLI). MarkDeprecated hides it from --help
163+
// and prints a runtime notice on use; no separate MarkHidden call is
164+
// needed (pflag's MarkDeprecated already sets Flag.Hidden = true).
165+
cmd.Flags().Int64Var(&channelID, "channel-id", 0, "Deprecated: use --channel instead")
166+
_ = cmd.Flags().MarkDeprecated("channel-id", "use --channel instead")
150167
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)")
151168
cmd.Flags().StringVar(&nums, "nums", "", "Comma-separated short incident ids (num, the 6-char id shown in the UI) to filter by")
152169
cmd.Flags().StringVar(&since, "since", "24h", "Start time (duration, date, datetime, or unix timestamp; --since→--until window must be < 31 days)")

internal/cli/incident_test.go

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,12 +29,62 @@ func TestCommandIncidentSimilarLimitReachesWire(t *testing.T) {
2929
}
3030
}
3131

32-
// TestCommandIncidentListChannelIDFlag verifies that `incident list` accepts
33-
// the canonical --channel-id flag (consistent with the sibling channel
34-
// commands, e.g. `channel info --channel-id`) and forwards it to /incident/list
35-
// as channel_ids. An agent that transferred --channel-id from those commands
36-
// previously hit "unknown flag: --channel-id" and wasted a turn.
37-
func TestCommandIncidentListChannelIDFlag(t *testing.T) {
32+
// TestCommandIncidentListChannelFlag verifies --channel is a string flag
33+
// (comma-separated IDs), matching the sibling list verbs (alert list
34+
// --channel, alert-event list --channel, change list --channel) — not the
35+
// singular int64 --channel-id this command used to require — and that
36+
// --channel-id is still registered but hidden+deprecated.
37+
func TestCommandIncidentListChannelFlag(t *testing.T) {
38+
cmd := newIncidentListCmd()
39+
flags := cmd.Flags()
40+
41+
f := flags.Lookup("channel")
42+
if f == nil {
43+
t.Fatal("flag --channel not registered")
44+
}
45+
if got := f.Value.Type(); got != "string" {
46+
t.Errorf("--channel flag type = %q, want %q", got, "string")
47+
}
48+
if got := f.DefValue; got != "" {
49+
t.Errorf("--channel default = %q, want %q", got, "")
50+
}
51+
52+
idFlag := flags.Lookup("channel-id")
53+
if idFlag == nil {
54+
t.Fatal("flag --channel-id must still be registered (deprecated alias)")
55+
}
56+
if !idFlag.Hidden {
57+
t.Error("--channel-id must be hidden now that --channel is canonical")
58+
}
59+
if idFlag.Deprecated == "" {
60+
t.Error("--channel-id must carry a deprecation message")
61+
}
62+
}
63+
64+
// TestCommandIncidentListChannelForwardsMultipleIDs verifies a
65+
// comma-separated --channel value reaches /incident/list as channel_ids —
66+
// the same wire shape alert list / change list already use.
67+
func TestCommandIncidentListChannelForwardsMultipleIDs(t *testing.T) {
68+
saveAndResetGlobals(t)
69+
stub := newGFStub(t)
70+
71+
if _, err := execCommand("incident", "list", "--channel", "100,200"); err != nil {
72+
t.Fatalf("execCommand --channel: %v", err)
73+
}
74+
if stub.lastPath != "/incident/list" {
75+
t.Fatalf("path = %q, want /incident/list", stub.lastPath)
76+
}
77+
if got, want := fmt.Sprint(stub.lastBody["channel_ids"]), "[100 200]"; got != want {
78+
t.Fatalf("channel_ids = %q, want %q", got, want)
79+
}
80+
}
81+
82+
// TestCommandIncidentListChannelIDFlagDeprecatedAlias verifies the
83+
// deprecated --channel-id alias still works and still forwards to
84+
// /incident/list as channel_ids, so scripts written before --channel existed
85+
// keep working. --channel is canonical now; see
86+
// TestCommandIncidentListChannelFlag above.
87+
func TestCommandIncidentListChannelIDFlagDeprecatedAlias(t *testing.T) {
3888
saveAndResetGlobals(t)
3989
stub := newGFStub(t)
4090

internal/skilldoc/build.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,9 @@ func command(c *cobra.Command, path []string) Command {
7878
cmd.Group = path[0]
7979
}
8080
c.Flags().VisitAll(func(f *pflag.Flag) {
81+
if f.Hidden {
82+
return
83+
}
8184
cmd.Flags = append(cmd.Flags, Flag{
8285
Name: f.Name,
8386
Type: f.Value.Type(),

internal/skilldoc/build_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,44 @@ func TestBuild_CapturesLeafWithFlagsAndRequired(t *testing.T) {
5050
}
5151
}
5252

53+
// TestBuild_ExcludesHiddenFlags verifies a flag hidden via cobra's
54+
// MarkDeprecated (which implies Hidden) is dropped from the card dump, so a
55+
// deprecated alias like incident list's --channel-id never resurfaces in a
56+
// generated skill card.
57+
func TestBuild_ExcludesHiddenFlags(t *testing.T) {
58+
root := &cobra.Command{Use: "fduty"}
59+
list := &cobra.Command{Use: "list", Short: "List things", Run: func(*cobra.Command, []string) {}}
60+
list.Flags().String("channel", "", "Comma-separated channel IDs")
61+
list.Flags().Int64("channel-id", 0, "Deprecated: use --channel instead")
62+
_ = list.Flags().MarkDeprecated("channel-id", "use --channel instead")
63+
root.AddCommand(list)
64+
65+
d := Build(root)
66+
var got *Command
67+
for i := range d.Commands {
68+
if d.Commands[i].Path == "list" {
69+
got = &d.Commands[i]
70+
}
71+
}
72+
if got == nil {
73+
t.Fatalf("missing list command")
74+
}
75+
for _, f := range got.Flags {
76+
if f.Name == "channel-id" {
77+
t.Fatalf("deprecated/hidden flag --channel-id must not appear in the dump: %+v", got.Flags)
78+
}
79+
}
80+
var hasChannel bool
81+
for _, f := range got.Flags {
82+
if f.Name == "channel" {
83+
hasChannel = true
84+
}
85+
}
86+
if !hasChannel {
87+
t.Fatalf("visible flag --channel missing from dump: %+v", got.Flags)
88+
}
89+
}
90+
5391
// runnableGroupTree mirrors internal/cli.newGroupCmd: a container command that
5492
// is Runnable (RunE just prints help, same as every group in the real tree —
5593
// alert, incident, oncall schedule, ...) purely so a mistyped subcommand fails

skills/flashduty/reference/incident.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ Get incident detail
235235

236236
### list
237237
List incidents
238-
- `--channel-id` int64
238+
- `--channel` string
239239
- `--fields` string
240240
- `--limit` int
241241
- `--nums` string

0 commit comments

Comments
 (0)