From 5d3b850e46f0dda5a79d76e9b9d79eb1d7545ca4 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 29 May 2026 20:39:17 +0000 Subject: [PATCH] Simplify inspect command handling --- internal/app/inspect.go | 54 +++++++++-------- internal/cli/root.go | 122 ++++++++++++++------------------------ internal/cli/root_test.go | 39 ++++++++++++ 3 files changed, 112 insertions(+), 103 deletions(-) diff --git a/internal/app/inspect.go b/internal/app/inspect.go index 38e171a..9fe89ac 100644 --- a/internal/app/inspect.go +++ b/internal/app/inspect.go @@ -40,59 +40,44 @@ func InspectReports(ctx context.Context, req InspectReportsRequest) ([]state.Rep } func InspectMetadata(ctx context.Context, req InspectRunRequest) (state.Metadata, error) { - store, err := defaultStore(req.Config) - if err != nil { - return state.Metadata{}, err - } - metadata, _, err := store.LoadMetadataByRunID(ctx, req.RunID) - return metadata, err + inspection, err := inspectRun(ctx, req) + return inspection.metadata, err } func InspectBriefing(ctx context.Context, req InspectRunRequest) (briefing.Package, error) { - store, err := defaultStore(req.Config) + inspection, err := inspectRun(ctx, req) if err != nil { return briefing.Package{}, err } - metadata, _, err := store.LoadMetadataByRunID(ctx, req.RunID) - if err != nil { - return briefing.Package{}, err - } - return store.LoadBriefing(ctx, metadata.BriefingPath) + return inspection.store.LoadBriefing(ctx, inspection.metadata.BriefingPath) } func InspectDataPackage(ctx context.Context, req InspectRunRequest) (promptinput.Package, error) { - store, err := defaultStore(req.Config) + inspection, err := inspectRun(ctx, req) if err != nil { return promptinput.Package{}, err } - metadata, _, err := store.LoadMetadataByRunID(ctx, req.RunID) - if err != nil { - return promptinput.Package{}, err - } - return store.LoadDataPackage(ctx, metadata.DataPackagePath) + return inspection.store.LoadDataPackage(ctx, inspection.metadata.DataPackagePath) } func InspectPriorSnapshot(ctx context.Context, req InspectRunRequest) (*state.PriorSnapshot, error) { - store, err := defaultStore(req.Config) + inspection, err := inspectRun(ctx, req) if err != nil { return nil, err } - metadata, _, err := store.LoadMetadataByRunID(ctx, req.RunID) + resolved, err := resolvedFromMetadata(inspection.metadata) if err != nil { return nil, err } - resolved, err := resolvedFromMetadata(metadata) - if err != nil { - return nil, err - } - return store.FindPriorSnapshot(ctx, resolved) + return inspection.store.FindPriorSnapshot(ctx, resolved) } func InspectSources(ctx context.Context, req InspectRunRequest) (SourceInspection, error) { - metadata, err := InspectMetadata(ctx, req) + inspection, err := inspectRun(ctx, req) if err != nil { return SourceInspection{}, err } + metadata := inspection.metadata return SourceInspection{ RunID: metadata.RunID, ReportID: metadata.ReportID, @@ -102,6 +87,23 @@ func InspectSources(ctx context.Context, req InspectRunRequest) (SourceInspectio }, nil } +type runInspection struct { + store *state.FilesystemStore + metadata state.Metadata +} + +func inspectRun(ctx context.Context, req InspectRunRequest) (runInspection, error) { + store, err := defaultStore(req.Config) + if err != nil { + return runInspection{}, err + } + metadata, _, err := store.LoadMetadataByRunID(ctx, req.RunID) + if err != nil { + return runInspection{}, err + } + return runInspection{store: store, metadata: metadata}, nil +} + func resolvedFromMetadata(metadata state.Metadata) (report.Resolved, error) { definition, err := report.DefaultRegistry().Lookup(metadata.ReportID) if err != nil { diff --git a/internal/cli/root.go b/internal/cli/root.go index f9e7af5..441eb97 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -72,7 +72,7 @@ func (r Runner) Run(ctx context.Context, args []string, stdout io.Writer, stderr result, err := app.RunBatchDetailed(ctx, req) if result != nil { writeRunLogs(stderr, result) - if encodeErr := writeRunSummary(stdout, result); encodeErr != nil { + if encodeErr := writeJSON(stdout, result); encodeErr != nil { return encodeErr } if result.Failed > 0 { @@ -108,6 +108,29 @@ type inspectOptions struct { RunID string } +type inspectRunCommand struct { + Name string + Inspect func(context.Context, app.InspectRunRequest) (any, error) +} + +var inspectRunCommands = []inspectRunCommand{ + {Name: "metadata", Inspect: func(ctx context.Context, req app.InspectRunRequest) (any, error) { + return app.InspectMetadata(ctx, req) + }}, + {Name: "briefing", Inspect: func(ctx context.Context, req app.InspectRunRequest) (any, error) { + return app.InspectBriefing(ctx, req) + }}, + {Name: "data-package", Inspect: func(ctx context.Context, req app.InspectRunRequest) (any, error) { + return app.InspectDataPackage(ctx, req) + }}, + {Name: "prior", Inspect: func(ctx context.Context, req app.InspectRunRequest) (any, error) { + return app.InspectPriorSnapshot(ctx, req) + }}, + {Name: "sources", Inspect: func(ctx context.Context, req app.InspectRunRequest) (any, error) { + return app.InspectSources(ctx, req) + }}, +} + func (r Runner) runInspect(ctx context.Context, args []string, stdout io.Writer) error { if len(args) == 0 { return fmt.Errorf("inspect requires a command") @@ -128,81 +151,32 @@ func (r Runner) runInspect(ctx context.Context, args []string, stdout io.Writer) return err } return writeJSON(stdout, records) - case "metadata": - opts, err := parseInspectRunFlags(command, args[1:]) - if err != nil { - return err - } - cfg, err := config.Load(config.LoadOptions{Path: opts.ConfigPath}) - if err != nil { - return err - } - metadata, err := app.InspectMetadata(ctx, app.InspectRunRequest{Config: cfg, RunID: opts.RunID}) - if err != nil { - return err - } - return writeJSON(stdout, metadata) - case "briefing": - opts, err := parseInspectRunFlags(command, args[1:]) - if err != nil { - return err - } - cfg, err := config.Load(config.LoadOptions{Path: opts.ConfigPath}) - if err != nil { - return err - } - pkg, err := app.InspectBriefing(ctx, app.InspectRunRequest{Config: cfg, RunID: opts.RunID}) - if err != nil { - return err - } - return writeJSON(stdout, pkg) - case "data-package": - opts, err := parseInspectRunFlags(command, args[1:]) - if err != nil { - return err - } - cfg, err := config.Load(config.LoadOptions{Path: opts.ConfigPath}) - if err != nil { - return err - } - pkg, err := app.InspectDataPackage(ctx, app.InspectRunRequest{Config: cfg, RunID: opts.RunID}) - if err != nil { - return err - } - return writeJSON(stdout, pkg) - case "prior": - opts, err := parseInspectRunFlags(command, args[1:]) - if err != nil { - return err - } - cfg, err := config.Load(config.LoadOptions{Path: opts.ConfigPath}) - if err != nil { - return err - } - prior, err := app.InspectPriorSnapshot(ctx, app.InspectRunRequest{Config: cfg, RunID: opts.RunID}) - if err != nil { - return err - } - return writeJSON(stdout, prior) - case "sources": - opts, err := parseInspectRunFlags(command, args[1:]) - if err != nil { - return err - } - cfg, err := config.Load(config.LoadOptions{Path: opts.ConfigPath}) - if err != nil { - return err - } - sources, err := app.InspectSources(ctx, app.InspectRunRequest{Config: cfg, RunID: opts.RunID}) - if err != nil { - return err - } - return writeJSON(stdout, sources) default: + for _, candidate := range inspectRunCommands { + if candidate.Name == command { + return runInspectRunCommand(ctx, stdout, candidate, args[1:]) + } + } return fmt.Errorf("unknown inspect command %q", command) } } +func runInspectRunCommand(ctx context.Context, stdout io.Writer, command inspectRunCommand, args []string) error { + opts, err := parseInspectRunFlags(command.Name, args) + if err != nil { + return err + } + cfg, err := config.Load(config.LoadOptions{Path: opts.ConfigPath}) + if err != nil { + return err + } + value, err := command.Inspect(ctx, app.InspectRunRequest{Config: cfg, RunID: opts.RunID}) + if err != nil { + return err + } + return writeJSON(stdout, value) +} + func (r Runner) resolveGenerate(args []string) (app.GenerateRequest, error) { if r.Clock == nil { r.Clock = timeutil.SystemClock{} @@ -371,12 +345,6 @@ func parseInspectRunFlags(command string, args []string) (inspectOptions, error) return opts, nil } -func writeRunSummary(stdout io.Writer, result *app.BatchResult) error { - encoder := json.NewEncoder(stdout) - encoder.SetIndent("", " ") - return encoder.Encode(result) -} - func writeJSON(stdout io.Writer, value any) error { encoder := json.NewEncoder(stdout) encoder.SetIndent("", " ") diff --git a/internal/cli/root_test.go b/internal/cli/root_test.go index 68b83c1..6bfb48c 100644 --- a/internal/cli/root_test.go +++ b/internal/cli/root_test.go @@ -581,6 +581,45 @@ func TestRunInspectMissingMetadata(t *testing.T) { } } +func TestRunInspectRunCommandsParseRunIDAndConfig(t *testing.T) { + tempDir := t.TempDir() + configPath := filepath.Join(tempDir, "config.yml") + configBody := "workspace:\n root: " + filepath.Join(tempDir, "workspace") + "\n" + if err := os.WriteFile(configPath, []byte(configBody), 0o600); err != nil { + t.Fatalf("write config: %v", err) + } + runner := Runner{Clock: fixedClock()} + commands := []string{"metadata", "briefing", "data-package", "prior", "sources"} + + for _, command := range commands { + t.Run(command+" requires run id", func(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + + err := runner.Run(context.Background(), []string{"inspect", command, "--config", configPath}, &stdout, &stderr) + if err == nil { + t.Fatal("Run() error = nil, want missing run id error") + } + if !strings.Contains(err.Error(), "requires a run id") { + t.Fatalf("error = %q, want missing run id context", err.Error()) + } + }) + + t.Run(command+" accepts config", func(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + + err := runner.Run(context.Background(), []string{"inspect", command, "--config", configPath, "missing"}, &stdout, &stderr) + if err == nil { + t.Fatal("Run() error = nil, want missing metadata error") + } + if !strings.Contains(err.Error(), "metadata for run id") { + t.Fatalf("error = %q, want missing metadata context", err.Error()) + } + }) + } +} + func TestResolveGenerateCommands(t *testing.T) { runner := Runner{Clock: fixedClock()} tests := []struct {