Merge pull request #160 from flashcatcloud/fix/incident-list-channel-flag

fix(cli): make incident list --channel match sibling list verbs

Author: ysyneu

internal/cli/incident.go

@@ -72,7 +72,7 @@ func pastIncidentColumns() []output.Column {
 }
 
 func newIncidentListCmd() *cobra.Command {
-	var progress, severity, query, since, until, nums, fields string
+	var progress, severity, query, since, until, nums, fields, channel string
 	var channelID int64
 	var limit, page int
 	defaultStructuredFields := []string{"incident_id", "title", "incident_severity", "progress", "start_time", "channel_id"}
@@ -101,7 +101,17 @@ func newIncidentListCmd() *cobra.Command {
 				}
 				req.Page = page
 				req.Limit = limit
-				if channelID != 0 {
+				if channel != "" {
+					channelIDs, err := parseIntSlice(channel)
+					if err != nil {
+						return fmt.Errorf("invalid --channel: %w", err)
+					}
+					req.ChannelIDs = channelIDs
+				} else if channelID != 0 {
+					// --channel-id is a deprecated single-ID alias kept for scripts
+					// written before --channel existed; --channel above is canonical
+					// and wins when both are set. parseIntSlice/--channel is exactly
+					// the pattern alert list and change list already use.
 					req.ChannelIDs = []int64{channelID}
 				}
 				if nums != "" {
@@ -144,9 +154,16 @@ func newIncidentListCmd() *cobra.Command {
 	cmd.Flags().StringVar(&severity, "severity", "", "Filter: Critical,Warning,Info")
 	registerEnumFlag(cmd, "progress", "Triggered", "Processing", "Closed")
 	registerEnumFlag(cmd, "severity", severityEnum...)
-	// --channel-id matches the sibling channel commands (channel info
-	// --channel-id, channel escalate-rule-list --channel-id).
-	cmd.Flags().Int64Var(&channelID, "channel-id", 0, "Filter by channel ID")
+	// --channel matches the sibling list verbs (alert list --channel,
+	// alert-event list --channel, change list --channel): comma-separated
+	// channel IDs, forwarded to the API's channel_ids ([]int64) field.
+	cmd.Flags().StringVar(&channel, "channel", "", "Comma-separated channel IDs")
+	// --channel-id is kept as a deprecated single-ID alias for existing
+	// scripts (this is a public CLI). MarkDeprecated hides it from --help
+	// and prints a runtime notice on use; no separate MarkHidden call is
+	// needed (pflag's MarkDeprecated already sets Flag.Hidden = true).
+	cmd.Flags().Int64Var(&channelID, "channel-id", 0, "Deprecated: use --channel instead")
+	_ = cmd.Flags().MarkDeprecated("channel-id", "use --channel instead")
 	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; --since→--until window must be < 31 days)")

internal/cli/incident_test.go

@@ -29,12 +29,62 @@ func TestCommandIncidentSimilarLimitReachesWire(t *testing.T) {
 	}
 }
 
-// TestCommandIncidentListChannelIDFlag verifies that `incident list` accepts
-// the canonical --channel-id flag (consistent with the sibling channel
-// commands, e.g. `channel info --channel-id`) and forwards it to /incident/list
-// as channel_ids. An agent that transferred --channel-id from those commands
-// previously hit "unknown flag: --channel-id" and wasted a turn.
-func TestCommandIncidentListChannelIDFlag(t *testing.T) {
+// TestCommandIncidentListChannelFlag verifies --channel is a string flag
+// (comma-separated IDs), matching the sibling list verbs (alert list
+// --channel, alert-event list --channel, change list --channel) — not the
+// singular int64 --channel-id this command used to require — and that
+// --channel-id is still registered but hidden+deprecated.
+func TestCommandIncidentListChannelFlag(t *testing.T) {
+	cmd := newIncidentListCmd()
+	flags := cmd.Flags()
+
+	f := flags.Lookup("channel")
+	if f == nil {
+		t.Fatal("flag --channel not registered")
+	}
+	if got := f.Value.Type(); got != "string" {
+		t.Errorf("--channel flag type = %q, want %q", got, "string")
+	}
+	if got := f.DefValue; got != "" {
+		t.Errorf("--channel default = %q, want %q", got, "")
+	}
+
+	idFlag := flags.Lookup("channel-id")
+	if idFlag == nil {
+		t.Fatal("flag --channel-id must still be registered (deprecated alias)")
+	}
+	if !idFlag.Hidden {
+		t.Error("--channel-id must be hidden now that --channel is canonical")
+	}
+	if idFlag.Deprecated == "" {
+		t.Error("--channel-id must carry a deprecation message")
+	}
+}
+
+// TestCommandIncidentListChannelForwardsMultipleIDs verifies a
+// comma-separated --channel value reaches /incident/list as channel_ids —
+// the same wire shape alert list / change list already use.
+func TestCommandIncidentListChannelForwardsMultipleIDs(t *testing.T) {
+	saveAndResetGlobals(t)
+	stub := newGFStub(t)
+
+	if _, err := execCommand("incident", "list", "--channel", "100,200"); err != nil {
+		t.Fatalf("execCommand --channel: %v", err)
+	}
+	if stub.lastPath != "/incident/list" {
+		t.Fatalf("path = %q, want /incident/list", stub.lastPath)
+	}
+	if got, want := fmt.Sprint(stub.lastBody["channel_ids"]), "[100 200]"; got != want {
+		t.Fatalf("channel_ids = %q, want %q", got, want)
+	}
+}
+
+// TestCommandIncidentListChannelIDFlagDeprecatedAlias verifies the
+// deprecated --channel-id alias still works and still forwards to
+// /incident/list as channel_ids, so scripts written before --channel existed
+// keep working. --channel is canonical now; see
+// TestCommandIncidentListChannelFlag above.
+func TestCommandIncidentListChannelIDFlagDeprecatedAlias(t *testing.T) {
 	saveAndResetGlobals(t)
 	stub := newGFStub(t)
 

internal/skilldoc/build.go

@@ -78,6 +78,9 @@ func command(c *cobra.Command, path []string) Command {
 		cmd.Group = path[0]
 	}
 	c.Flags().VisitAll(func(f *pflag.Flag) {
+		if f.Hidden {
+			return
+		}
 		cmd.Flags = append(cmd.Flags, Flag{
 			Name:     f.Name,
 			Type:     f.Value.Type(),

internal/skilldoc/build_test.go

@@ -50,6 +50,44 @@ func TestBuild_CapturesLeafWithFlagsAndRequired(t *testing.T) {
 	}
 }
 
+// TestBuild_ExcludesHiddenFlags verifies a flag hidden via cobra's
+// MarkDeprecated (which implies Hidden) is dropped from the card dump, so a
+// deprecated alias like incident list's --channel-id never resurfaces in a
+// generated skill card.
+func TestBuild_ExcludesHiddenFlags(t *testing.T) {
+	root := &cobra.Command{Use: "fduty"}
+	list := &cobra.Command{Use: "list", Short: "List things", Run: func(*cobra.Command, []string) {}}
+	list.Flags().String("channel", "", "Comma-separated channel IDs")
+	list.Flags().Int64("channel-id", 0, "Deprecated: use --channel instead")
+	_ = list.Flags().MarkDeprecated("channel-id", "use --channel instead")
+	root.AddCommand(list)
+
+	d := Build(root)
+	var got *Command
+	for i := range d.Commands {
+		if d.Commands[i].Path == "list" {
+			got = &d.Commands[i]
+		}
+	}
+	if got == nil {
+		t.Fatalf("missing list command")
+	}
+	for _, f := range got.Flags {
+		if f.Name == "channel-id" {
+			t.Fatalf("deprecated/hidden flag --channel-id must not appear in the dump: %+v", got.Flags)
+		}
+	}
+	var hasChannel bool
+	for _, f := range got.Flags {
+		if f.Name == "channel" {
+			hasChannel = true
+		}
+	}
+	if !hasChannel {
+		t.Fatalf("visible flag --channel missing from dump: %+v", got.Flags)
+	}
+}
+
 // runnableGroupTree mirrors internal/cli.newGroupCmd: a container command that
 // is Runnable (RunE just prints help, same as every group in the real tree —
 // alert, incident, oncall schedule, ...) purely so a mistyped subcommand fails

skills/flashduty/reference/incident.md

@@ -235,7 +235,7 @@ Get incident detail
 
 ### list
 List incidents
-- `--channel-id` int64
+- `--channel` string
 - `--fields` string
 - `--limit` int
 - `--nums` string