release: merge audit fixes for v1.3.22

internal/cli/fieldproject_test.go

@@ -17,6 +17,7 @@ func incidentRow() map[string]any {
 		"incident_severity": "Critical",
 		"progress":          "Triggered",
 		"start_time":        1712000000,
+		"channel_id":        12345,
 		"description":       "root volume at 98%",
 		"labels":            map[string]any{"service": "db", "env": "prod"},
 		"responders": []map[string]any{
@@ -41,19 +42,84 @@ func alertRow() map[string]any {
 	}
 }
 
-// TestFieldsProjectionDefaultUnchanged is the conductor constraint: with NO
-// --fields, the structured (toon and json) output must still be the full nested
-// record — the nested blobs the proposal deliberately preserves as the default.
-func TestFieldsProjectionDefaultUnchanged(t *testing.T) {
+// TestIncidentListStructuredDefaultUsesCompactProjection is the default agent
+// path: incident list in json/toon mode must not dump the full nested SDK row
+// when --fields is omitted, while an explicit --fields still wins.
+func TestIncidentListStructuredDefaultUsesCompactProjection(t *testing.T) {
+	t.Run("json default", func(t *testing.T) {
+		saveAndResetGlobals(t)
+		stub := newGFStub(t)
+		stub.data = map[string]any{"items": []any{incidentRow()}, "total": 1}
+
+		out, err := execCommand("incident", "list", "--output-format", "json")
+		if err != nil {
+			t.Fatalf("execCommand: %v", err)
+		}
+
+		assertProjectedJSONFields(t, out, []string{"incident_id", "title", "incident_severity", "progress", "start_time", "channel_id"})
+	})
+
+	t.Run("toon default", func(t *testing.T) {
+		saveAndResetGlobals(t)
+		stub := newGFStub(t)
+		stub.data = map[string]any{"items": []any{incidentRow()}, "total": 1}
+
+		out, err := execCommand("incident", "list", "--output-format", "toon")
+		if err != nil {
+			t.Fatalf("execCommand: %v", err)
+		}
+
+		for _, key := range []string{"incident_id", "title", "incident_severity", "progress", "start_time", "channel_id"} {
+			if !strings.Contains(out, key) {
+				t.Errorf("default toon output missing compact key %q, got:\n%s", key, out)
+			}
+		}
+		for _, key := range []string{"responders", "labels", "description"} {
+			if strings.Contains(out, key) {
+				t.Errorf("default toon output should not contain full-record key %q, got:\n%s", key, out)
+			}
+		}
+	})
+
+	t.Run("explicit fields win", func(t *testing.T) {
+		saveAndResetGlobals(t)
+		stub := newGFStub(t)
+		stub.data = map[string]any{"items": []any{incidentRow()}, "total": 1}
+
+		out, err := execCommand("incident", "list", "--fields", "incident_id,title", "--output-format", "json")
+		if err != nil {
+			t.Fatalf("execCommand: %v", err)
+		}
+
+		assertProjectedJSONFields(t, out, []string{"incident_id", "title"})
+	})
+
+	t.Run("explicit empty fields errors", func(t *testing.T) {
+		saveAndResetGlobals(t)
+		stub := newGFStub(t)
+		stub.data = map[string]any{"items": []any{incidentRow()}, "total": 1}
+
+		_, err := execCommand("incident", "list", "--fields", "", "--output-format", "json")
+		if err == nil {
+			t.Fatal("expected an error for empty --fields, got nil")
+		}
+		if !strings.Contains(err.Error(), "--fields") {
+			t.Errorf("error should name --fields, got: %v", err)
+		}
+	})
+}
+
+// TestAlertFieldsProjectionDefaultUnchanged is the conductor constraint for the
+// sibling command: with NO --fields, alert list structured output still emits
+// the full nested record. The compact default is incident-list-only.
+func TestAlertFieldsProjectionDefaultUnchanged(t *testing.T) {
 	cases := []struct {
 		name     string
 		cmd      []string
 		data     map[string]any
 		format   string
 		mustHave []string // nested keys that must survive in the full dump
 	}{
-		{"incident toon", []string{"incident", "list"}, incidentRow(), "toon", []string{"responders", "labels", "description"}},
-		{"incident json", []string{"incident", "list"}, incidentRow(), "json", []string{"responders", "labels", "description"}},
 		{"alert toon", []string{"alert", "list"}, alertRow(), "toon", []string{"events", "incident", "labels", "description"}},
 		{"alert json", []string{"alert", "list"}, alertRow(), "json", []string{"events", "incident", "labels", "description"}},
 	}
@@ -77,6 +143,27 @@ func TestFieldsProjectionDefaultUnchanged(t *testing.T) {
 	}
 }
 
+func assertProjectedJSONFields(t *testing.T, out string, fields []string) {
+	t.Helper()
+
+	var rows []map[string]json.RawMessage
+	if err := json.Unmarshal([]byte(strings.TrimSpace(out)), &rows); err != nil {
+		t.Fatalf("failed to parse projected json: %v\nraw:\n%s", err, out)
+	}
+	if len(rows) != 1 {
+		t.Fatalf("expected 1 projected row, got %d:\n%s", len(rows), out)
+	}
+	row := rows[0]
+	if len(row) != len(fields) {
+		t.Fatalf("expected exactly %d keys, got %d (%v)", len(fields), len(row), row)
+	}
+	for _, f := range fields {
+		if _, ok := row[f]; !ok {
+			t.Errorf("projected row missing key %q, got keys %v", f, row)
+		}
+	}
+}
+
 // TestFieldsProjectionTOON: --fields in toon mode emits exactly the requested
 // keys and drops everything else.
 func TestFieldsProjectionTOON(t *testing.T) {
@@ -154,22 +241,7 @@ func TestFieldsProjectionJSON(t *testing.T) {
 				t.Fatalf("execCommand: %v", err)
 			}
 
-			var rows []map[string]json.RawMessage
-			if err := json.Unmarshal([]byte(strings.TrimSpace(out)), &rows); err != nil {
-				t.Fatalf("failed to parse projected json: %v\nraw:\n%s", err, out)
-			}
-			if len(rows) != 1 {
-				t.Fatalf("expected 1 projected row, got %d:\n%s", len(rows), out)
-			}
-			row := rows[0]
-			if len(row) != len(tc.fields) {
-				t.Fatalf("expected exactly %d keys, got %d (%v)", len(tc.fields), len(row), row)
-			}
-			for _, f := range tc.fields {
-				if _, ok := row[f]; !ok {
-					t.Errorf("projected row missing key %q, got keys %v", f, row)
-				}
-			}
+			assertProjectedJSONFields(t, out, tc.fields)
 		})
 	}
 }

internal/cli/generic_table.go

@@ -20,6 +20,8 @@ const maxHeuristicColumns = 8
 // can't blow out the table width.
 const genericStringMaxWidth = 40
 
+const mcpPerUserOAuthNotice = "Note: registered but not usable until OAuth is completed in Flashduty Plugins -> MCP; tools will not appear until authorized."
+
 // instantLike mirrors go-flashduty's Timestamp/TimestampMilli (and the output
 // package's unexported instant) so the renderer can recognise timestamp fields
 // by reflection.
@@ -62,15 +64,18 @@ func renderGenericTable(ctx *RunContext, data any) error {
 		if rows, total, ok := listEnvelope(v); ok {
 			return renderRowTable(ctx, rows, total)
 		}
-		return renderVertical(ctx, v)
+		if err := renderVertical(ctx, v); err != nil {
+			return err
+		}
+		return renderMcpPerUserOAuthNotice(ctx, v)
 	default:
 		return jsonFallback(ctx, data)
 	}
 }
 
 // listEnvelope reports whether struct v is a paginated list envelope: exactly
-// one exported field that is a slice of structs (the rows), with the remaining
-// fields being pagination metadata. It returns the rows value and the total
+// one exported field that is a slice of structs (the rows), with any remaining
+// fields limited to pagination metadata. It returns the rows value and the total
 // (the int field named "Total" when present, else the row count).
 func listEnvelope(v reflect.Value) (rows reflect.Value, total int, ok bool) {
 	t := v.Type()
@@ -92,6 +97,9 @@ func listEnvelope(v reflect.Value) (rows reflect.Value, total int, ok bool) {
 		if total < 0 && f.Name == "Total" && fv.CanInt() {
 			total = int(fv.Int())
 		}
+		if !isListMetadataField(f, fv) {
+			return reflect.Value{}, 0, false
+		}
 	}
 	if !found {
 		return reflect.Value{}, 0, false
@@ -102,6 +110,22 @@ func listEnvelope(v reflect.Value) (rows reflect.Value, total int, ok bool) {
 	return rows, total, true
 }
 
+func isListMetadataField(f reflect.StructField, fv reflect.Value) bool {
+	if f.Anonymous && f.Name == "ListOptions" {
+		return true
+	}
+	switch f.Name {
+	case "Total":
+		return fv.CanInt()
+	case "HasNextPage":
+		return fv.Kind() == reflect.Bool
+	case "SearchAfterCtx", "NextCursor":
+		return fv.Kind() == reflect.String
+	default:
+		return false
+	}
+}
+
 // isRowSlice reports whether t is a slice whose element (after pointer deref) is
 // a struct — i.e. a table-able row collection.
 func isRowSlice(t reflect.Type) bool {
@@ -210,6 +234,22 @@ func renderVertical(ctx *RunContext, v reflect.Value) error {
 	return ctx.Printer.Print(rows, cols)
 }
 
+func renderMcpPerUserOAuthNotice(ctx *RunContext, v reflect.Value) error {
+	if !isMcpPerUserOAuth(v) {
+		return nil
+	}
+	_, err := fmt.Fprintln(ctx.Writer, mcpPerUserOAuthNotice)
+	return err
+}
+
+func isMcpPerUserOAuth(v reflect.Value) bool {
+	if v.Type().Name() != "McpServerItem" {
+		return false
+	}
+	auth := v.FieldByName("AuthMode")
+	return auth.IsValid() && auth.Kind() == reflect.String && auth.String() == "per_user_oauth"
+}
+
 // derefStruct dereferences pointer chains and returns the underlying struct
 // reflect.Value. The second return is false when item is nil, not a struct after
 // dereferencing, or any pointer in the chain is nil.

internal/cli/generic_table_test.go

@@ -144,6 +144,57 @@ func TestRenderGenericTable_DetailVertical(t *testing.T) {
 	}
 }
 
+func TestRenderGenericTable_McpServerItemWithEmptyToolsRendersDetail(t *testing.T) {
+	var buf bytes.Buffer
+	item := &flashduty.McpServerItem{
+		ServerID:    "mcp_test",
+		ServerName:  "github",
+		Description: "GitHub connector",
+		AuthMode:    "shared",
+		Status:      "enabled",
+		Transport:   "streamable-http",
+		URL:         "https://mcp.example.com/github",
+		Tools:       []flashduty.McpToolInfo{},
+	}
+	if err := renderGenericTable(tableCtx(&buf), item); err != nil {
+		t.Fatalf("render: %v", err)
+	}
+	got := buf.String()
+	if strings.Contains(got, "No results.") {
+		t.Fatalf("single MCP server item with empty tools was rendered as an empty list:\n%s", got)
+	}
+	for _, want := range []string{"FIELD", "VALUE", "SERVER_ID", "mcp_test", "SERVER_NAME", "github"} {
+		if !strings.Contains(got, want) {
+			t.Errorf("MCP server detail output missing %q\n---\n%s", want, got)
+		}
+	}
+}
+
+func TestRenderGenericTable_McpServerPerUserOAuthNotice(t *testing.T) {
+	var buf bytes.Buffer
+	item := &flashduty.McpServerItem{
+		ServerID:    "mcp_oauth",
+		ServerName:  "github",
+		Description: "GitHub connector",
+		AuthMode:    "per_user_oauth",
+		Status:      "enabled",
+		Transport:   "streamable-http",
+		URL:         "https://mcp.example.com/github",
+	}
+	if err := renderGenericTable(tableCtx(&buf), item); err != nil {
+		t.Fatalf("render: %v", err)
+	}
+	got := buf.String()
+	for _, want := range []string{
+		"registered but not usable until OAuth is completed in Flashduty Plugins -> MCP",
+		"tools will not appear until authorized",
+	} {
+		if !strings.Contains(got, want) {
+			t.Errorf("per-user OAuth notice missing %q\n---\n%s", want, got)
+		}
+	}
+}
+
 func TestPrintGenericResult_StructuredUnchanged(t *testing.T) {
 	resp := &fakeListResp{Items: []heuristicRow{{Name: "alpha", Count: 7}}, Total: 1}