Skip to content

Commit 1029c5c

Browse files
JAORMXclaude
andcommitted
Address review feedback on upgrade CLI
- Reject --check-upgrades together with --format mcpservers: that format has no upgrade column, so the combination performed a registry lookup per workload and discarded the result. Fail loudly in PreRunE instead. - Guard the bulk-result loop against nil entries so it stays robust if CheckAll's contract ever changes. - Drop the network-isolation line from the posture-drift report; the checker no longer reports it (the registry has no such field). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 4e726d2 commit 1029c5c

2 files changed

Lines changed: 19 additions & 3 deletions

File tree

cmd/thv/app/list.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,23 @@ func init() {
6262
listCmd.PreRunE = chainPreRunE(
6363
validateGroupFlag(),
6464
ValidateFormat(&listFormat, FormatJSON, FormatText, "mcpservers"),
65+
validateCheckUpgradesFormat(),
6566
)
6667
}
6768

69+
// validateCheckUpgradesFormat rejects --check-upgrades with --format mcpservers.
70+
// The mcpservers format emits client configuration and has no upgrade column, so
71+
// the flag combination would perform a registry lookup per workload and then
72+
// discard the result. Fail loudly rather than do hidden, wasted work.
73+
func validateCheckUpgradesFormat() func(*cobra.Command, []string) error {
74+
return func(_ *cobra.Command, _ []string) error {
75+
if listCheckUpgrades && listFormat == "mcpservers" {
76+
return fmt.Errorf("--check-upgrades is not supported with --format mcpservers; use --format text or json")
77+
}
78+
return nil
79+
}
80+
}
81+
6882
func listCmdFunc(cmd *cobra.Command, _ []string) error {
6983
ctx := cmd.Context()
7084

cmd/thv/app/upgrade.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,11 @@ func printUpgradeTable(results []*upgrade.CheckResult) {
175175
}
176176

177177
for _, r := range results {
178+
if r == nil {
179+
// CheckAll does not return nil entries today; guard so this stays
180+
// robust if that ever changes.
181+
continue
182+
}
178183
if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%d\t%s\n",
179184
r.WorkloadName,
180185
r.Status,
@@ -229,9 +234,6 @@ func printUpgradeDetail(r *upgrade.CheckResult) {
229234
if c := r.ConfigDrift.Transport; c != nil {
230235
fmt.Printf(" ⚠ transport: %s -> %s\n", c.From, c.To)
231236
}
232-
if c := r.ConfigDrift.NetworkIsolation; c != nil {
233-
fmt.Printf(" ⚠ network isolation: %t -> %t\n", c.From, c.To)
234-
}
235237
if c := r.ConfigDrift.PermissionProfile; c != nil {
236238
fmt.Printf(" ⚠ permission profile: %s -> %s\n", c.From, c.To)
237239
}

0 commit comments

Comments
 (0)