Skip to content

Commit a49b4fa

Browse files
JAORMXclaude
andauthored
Add thv upgrade check and list --check-upgrades (#5409)
* Add upgrade detection for registry workloads CLI and API users have no way to discover when a newer version of a registry-sourced MCP server is available; only Studio implements drift detection, in its frontend. Introduce a backend package that all clients can consume. Add pkg/workloads/upgrade with a Checker that compares a running workload's image tag against its registry entry (semver-aware, with a string fallback) and reports environment-variable and configuration (transport / permission-profile / network-isolation) drift. Comparison degrades safely to "unknown" for :latest, digest refs, repository changes, and non-registry-sourced workloads, so only a strictly-newer tag on the same repository yields "upgrade-available". This is the read-only detection core (RFC THV-0068, phase A); the apply path, API endpoints, and CLI follow in later changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address review feedback on upgrade detection - Lowercase an uppercase "V" tag prefix so semver comparison works; "V1.2.0" vs "V1.3.0" no longer falls through to undecidable and hides a real upgrade. - Drop the raw provider error from CheckResult.Reason (it is serialized into the API response and can leak internal addressing); log it at DEBUG and return a fixed string. Same for the CheckAll path. - Add a defensive default to the comparison switch so an unexpected value yields StatusUnknown rather than the least-safe StatusUpToDate. - Stop reporting network-isolation drift: the registry has no network-isolation field, so it fired for every isolated workload regardless of the candidate version. Remove the ConfigDrift field and the now-unused BoolChange type. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add upgrade-check REST endpoints for workloads CLI, Studio, and automation need a single backend source of truth for upgrade availability instead of each client reimplementing registry drift detection. Expose the Phase A checker over the existing workloads API. Add GET /api/v1beta/workloads/upgrade-check (batch) and GET /api/v1beta/workloads/{name}/upgrade-check (single). The batch handler reuses the exact group/all authorization scoping of the list endpoint and intersects it with the enumerated run configs, so it can never report a workload outside the caller's scope. Responses carry only non-sensitive CheckResult metadata; secret env-var defaults are cleared. Both routes are read-only and skip image pulls, so they use the standard timeout. Regenerate the OpenAPI spec. The dedicated apply endpoint (POST .../upgrade) follows once the Applier lands (RFC THV-0068, phase B). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Return 404 for unknown group in workload listing FilterByGroup returns an empty slice (nil error) for a group that does not exist, so a typo'd ?group= silently returned 200 with an empty list instead of the documented 404. Check group existence explicitly via the group manager before filtering, in both the upgrade-check and the listWorkloads handlers, so the advertised 404 is real. Add a bulk upgrade-check test covering the unknown-group path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Regenerate OpenAPI spec after dropping network-isolation drift The upgrade detection change removed the ConfigDrift.NetworkIsolation field and the BoolChange type, so regenerate the committed OpenAPI spec to drop the stale schema and property. Fixes the Verify Swagger Documentation CI check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Add thv upgrade check and list --check-upgrades CLI users had no way to see whether their registry-sourced MCP servers have newer versions available. Surface the upgrade checker on the command line. Add a thv upgrade command group with a check [name] subcommand: with a name it prints a verbose report (candidate image, new env vars, and permission/transport/network posture drift); with no name it prints a table for all workloads. Add an opt-in --check-upgrades flag to thv list that appends an upgrade column. Both reuse the pkg/workloads/upgrade checker and only format results; the default list path is unchanged and performs no registry lookup, so it stays offline-friendly. Bulk output is sorted by name to match thv list. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * 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> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 16ce897 commit a49b4fa

8 files changed

Lines changed: 561 additions & 16 deletions

File tree

cmd/thv/app/commands.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ func NewRootCmd(enableUpdates bool) *cobra.Command {
7676
rootCmd.AddCommand(skillCmd)
7777
rootCmd.AddCommand(statusCmd)
7878
rootCmd.AddCommand(tuiCmd)
79+
rootCmd.AddCommand(upgradeCmd)
7980

8081
// Silence printing the usage on error
8182
rootCmd.SilenceUsage = true

cmd/thv/app/list.go

Lines changed: 112 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package app
55

66
import (
7+
"context"
78
"encoding/json"
89
"fmt"
910
"log/slog"
@@ -13,7 +14,9 @@ import (
1314
"github.com/spf13/cobra"
1415

1516
"github.com/stacklok/toolhive/pkg/core"
17+
"github.com/stacklok/toolhive/pkg/runner"
1618
"github.com/stacklok/toolhive/pkg/workloads"
19+
"github.com/stacklok/toolhive/pkg/workloads/upgrade"
1720
)
1821

1922
var listCmd = &cobra.Command{
@@ -41,24 +44,41 @@ Examples:
4144
}
4245

4346
var (
44-
listAll bool
45-
listFormat string
46-
listLabelFilter []string
47-
listGroupFilter string
47+
listAll bool
48+
listFormat string
49+
listLabelFilter []string
50+
listGroupFilter string
51+
listCheckUpgrades bool
4852
)
4953

5054
func init() {
5155
AddAllFlag(listCmd, &listAll, true, "Show all workloads (default shows just running)")
5256
AddFormatFlag(listCmd, &listFormat, FormatJSON, FormatText, "mcpservers")
5357
listCmd.Flags().StringArrayVarP(&listLabelFilter, "label", "l", []string{}, "Filter workloads by labels (format: key=value)")
5458
AddGroupFlag(listCmd, &listGroupFilter, false)
59+
listCmd.Flags().BoolVar(&listCheckUpgrades, "check-upgrades", false,
60+
"Check each workload for available upgrades against its source registry (performs a registry lookup)")
5561

5662
listCmd.PreRunE = chainPreRunE(
5763
validateGroupFlag(),
5864
ValidateFormat(&listFormat, FormatJSON, FormatText, "mcpservers"),
65+
validateCheckUpgradesFormat(),
5966
)
6067
}
6168

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+
6282
func listCmdFunc(cmd *cobra.Command, _ []string) error {
6383
ctx := cmd.Context()
6484

@@ -81,10 +101,20 @@ func listCmdFunc(cmd *cobra.Command, _ []string) error {
81101
}
82102
}
83103

104+
// Optionally compute upgrade status for each workload. This is the only path
105+
// that performs a registry lookup; the default list stays offline-friendly.
106+
var upgrades map[string]*upgrade.CheckResult
107+
if listCheckUpgrades {
108+
upgrades, err = checkUpgradesForWorkloads(ctx, workloadList)
109+
if err != nil {
110+
return err
111+
}
112+
}
113+
84114
// Output based on format
85115
switch listFormat {
86116
case FormatJSON:
87-
return printJSONOutput(workloadList)
117+
return printJSONOutput(workloadList, upgrades)
88118
case "mcpservers":
89119
return printMCPServersOutput(workloadList)
90120
default:
@@ -97,13 +127,49 @@ func listCmdFunc(cmd *cobra.Command, _ []string) error {
97127
}
98128
return nil
99129
}
100-
printTextOutput(workloadList)
130+
printTextOutput(workloadList, upgrades)
101131
return nil
102132
}
103133
}
104134

105-
// printJSONOutput prints workload information in JSON format
106-
func printJSONOutput(workloadList []core.Workload) error {
135+
// checkUpgradesForWorkloads builds a single Checker, loads each workload's saved
136+
// RunConfig, and returns the upgrade result keyed by workload name. Workloads
137+
// whose config cannot be loaded are omitted from the map. The comparison logic
138+
// lives entirely in pkg/workloads/upgrade; this only collects inputs.
139+
func checkUpgradesForWorkloads(ctx context.Context, workloadList []core.Workload) (map[string]*upgrade.CheckResult, error) {
140+
checker, err := newUpgradeChecker()
141+
if err != nil {
142+
return nil, err
143+
}
144+
145+
configs := make([]*runner.RunConfig, 0, len(workloadList))
146+
for _, wl := range workloadList {
147+
cfg, err := runner.LoadState(ctx, wl.Name)
148+
if err != nil {
149+
slog.Debug("skipping upgrade check for workload with unloadable config", "workload", wl.Name, "error", err)
150+
continue
151+
}
152+
configs = append(configs, cfg)
153+
}
154+
155+
results := checker.CheckAll(ctx, configs)
156+
byName := make(map[string]*upgrade.CheckResult, len(results))
157+
for _, r := range results {
158+
byName[r.WorkloadName] = r
159+
}
160+
return byName, nil
161+
}
162+
163+
// workloadWithUpgrade augments a workload with its optional upgrade-check
164+
// result for JSON output when --check-upgrades is set.
165+
type workloadWithUpgrade struct {
166+
core.Workload
167+
Upgrade *upgrade.CheckResult `json:"upgrade,omitempty"`
168+
}
169+
170+
// printJSONOutput prints workload information in JSON format. When upgrades is
171+
// non-nil, each workload is augmented with its upgrade-check result.
172+
func printJSONOutput(workloadList []core.Workload, upgrades map[string]*upgrade.CheckResult) error {
107173
// Ensure we have a non-nil slice to avoid null in JSON output
108174
if workloadList == nil {
109175
workloadList = []core.Workload{}
@@ -112,13 +178,26 @@ func printJSONOutput(workloadList []core.Workload) error {
112178
// Sort workloads alphabetically by name for deterministic output
113179
core.SortWorkloadsByName(workloadList)
114180

115-
// Marshal to JSON
116-
jsonData, err := json.MarshalIndent(workloadList, "", " ")
181+
// Without upgrade data, marshal the workloads directly to preserve the
182+
// existing output shape.
183+
if upgrades == nil {
184+
jsonData, err := json.MarshalIndent(workloadList, "", " ")
185+
if err != nil {
186+
return fmt.Errorf("failed to marshal JSON: %w", err)
187+
}
188+
fmt.Println(string(jsonData))
189+
return nil
190+
}
191+
192+
augmented := make([]workloadWithUpgrade, 0, len(workloadList))
193+
for _, wl := range workloadList {
194+
augmented = append(augmented, workloadWithUpgrade{Workload: wl, Upgrade: upgrades[wl.Name]})
195+
}
196+
197+
jsonData, err := json.MarshalIndent(augmented, "", " ")
117198
if err != nil {
118199
return fmt.Errorf("failed to marshal JSON: %w", err)
119200
}
120-
121-
// Print JSON directly to stdout
122201
fmt.Println(string(jsonData))
123202
return nil
124203
}
@@ -150,14 +229,19 @@ func printMCPServersOutput(workloadList []core.Workload) error {
150229
return nil
151230
}
152231

153-
// printTextOutput prints workload information in text format
154-
func printTextOutput(workloadList []core.Workload) {
232+
// printTextOutput prints workload information in text format. When upgrades is
233+
// non-nil, an additional UPGRADE column reports each workload's upgrade status.
234+
func printTextOutput(workloadList []core.Workload, upgrades map[string]*upgrade.CheckResult) {
155235
// Sort workloads alphabetically by name for deterministic output
156236
core.SortWorkloadsByName(workloadList)
157237

158238
// Create a tabwriter for pretty output
159239
w := tabwriter.NewWriter(os.Stdout, 0, 0, 3, ' ', 0)
160-
if _, err := fmt.Fprintln(w, "NAME\tPACKAGE\tSTATUS\tURL\tPORT\tGROUP\tCREATED"); err != nil {
240+
header := "NAME\tPACKAGE\tSTATUS\tURL\tPORT\tGROUP\tCREATED"
241+
if upgrades != nil {
242+
header += "\tUPGRADE"
243+
}
244+
if _, err := fmt.Fprintln(w, header); err != nil {
161245
slog.Warn(fmt.Sprintf("Failed to write output header: %v", err))
162246
return
163247
}
@@ -168,7 +252,7 @@ func printTextOutput(workloadList []core.Workload) {
168252
status := workloadStatusIndicator(c.Status)
169253

170254
// Print workload information
171-
if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%d\t%s\t%s\n",
255+
if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%d\t%s\t%s",
172256
c.Name,
173257
c.Package,
174258
status,
@@ -179,6 +263,18 @@ func printTextOutput(workloadList []core.Workload) {
179263
); err != nil {
180264
slog.Debug(fmt.Sprintf("Failed to write workload information: %v", err))
181265
}
266+
if upgrades != nil {
267+
upgradeStatus := "-"
268+
if r, ok := upgrades[c.Name]; ok {
269+
upgradeStatus = string(r.Status)
270+
}
271+
if _, err := fmt.Fprintf(w, "\t%s", upgradeStatus); err != nil {
272+
slog.Debug(fmt.Sprintf("Failed to write upgrade status: %v", err))
273+
}
274+
}
275+
if _, err := fmt.Fprintln(w); err != nil {
276+
slog.Debug(fmt.Sprintf("Failed to write newline: %v", err))
277+
}
182278
}
183279

184280
// Flush the tabwriter

0 commit comments

Comments
 (0)