From 8896412204d4a4c033c865691a66086135c10f4a Mon Sep 17 00:00:00 2001 From: Daniel De Vera Date: Thu, 7 May 2026 09:49:32 -0300 Subject: [PATCH 1/2] plan: align display across plan-family commands; surface CreatedBy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The plan-family commands had drifted from each other in several small ways since the previous PR. Audit + fixes: - plan tag get rendered the embedded plan as a thin metadata blurb (ID, Steps count, prompt first line, Created) — the shape we replaced in plan get during the previous PR. It now embeds the full plan body (Inputs / Steps / Outputs sections) using the same arrow form plan get uses. Plan-level metadata is omitted: ID and SelectionHint already appear in the tag header above; the rest is one signadot plan get away. The tag header also gains a Selection Hint: line so plan tag list and plan tag get agree on what summarises a tag. To make this shareable without a circular import (plan imports plantag for tag-application after compile/create), the plan-detail printer moved into the planshared package as PrintPlanDetails alongside a PrintPlanBody variant that skips the top-level header for embedded use. plan/printers.go collapses to a thin wrapper. - Surface PlanStatus.CreatedBy (added in the latest go-sdk drop) as a Created By: row in the plan header — used by plan get, compile, create, recompile, run, and the embedded view in plan tag get (where it renders as Plan Created By: to disambiguate against the tag-level Created/Updated fields). Resolution by ActorType: user → email, api_key → "api key" + masked id, system → "system". - plan action get's Outputs table no longer carries REQUIRED / DEFAULT columns. Those are input concepts that always rendered as false / empty for outputs (review feedback on PR #325 that hadn't landed). Separate row type with just NAME / SCHEMA. - planrunnergroup/printers.go: migrate inline time.Parse + timeago.NoMax to utils.TimeAgo, matching the cleanup we'd already done in plantag and planexec. Drops the time and timeago imports. Co-Authored-By: Claude Opus 4.7 (1M context) --- go.mod | 2 +- go.sum | 4 +- internal/command/plan/printers.go | 221 +---------------- internal/command/planaction/printers.go | 42 +++- internal/command/planrunnergroup/printers.go | 9 +- internal/command/planshared/plan.go | 244 +++++++++++++++++++ internal/command/planshared/render.go | 37 +++ internal/command/plantag/printers.go | 36 +-- 8 files changed, 339 insertions(+), 256 deletions(-) create mode 100644 internal/command/planshared/plan.go diff --git a/go.mod b/go.mod index 99e9fb7..7335bba 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/nsf/jsondiff v0.0.0-20210926074059-1e845ec5d249 github.com/oklog/run v1.1.0 github.com/panta/machineid v1.0.2 - github.com/signadot/go-sdk v0.3.8-0.20260505200838-3bae35b0d0d8 + github.com/signadot/go-sdk v0.3.8-0.20260507124501-af142d7ec85c github.com/signadot/libconnect v0.1.1-0.20260424105947-336dce43da75 github.com/spf13/cobra v1.8.1 github.com/spf13/viper v1.11.0 diff --git a/go.sum b/go.sum index c17e40e..e436abd 100644 --- a/go.sum +++ b/go.sum @@ -452,8 +452,8 @@ github.com/segmentio/encoding v0.5.4 h1:OW1VRern8Nw6ITAtwSZ7Idrl3MXCFwXHPgqESYfv github.com/segmentio/encoding v0.5.4/go.mod h1:HS1ZKa3kSN32ZHVZ7ZLPLXWvOVIiZtyJnO1gPH1sKt0= github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3 h1:n661drycOFuPLCN3Uc8sB6B/s6Z4t2xvBgU1htSHuq8= github.com/sergi/go-diff v1.3.2-0.20230802210424-5b0b94c5c0d3/go.mod h1:A0bzQcvG0E7Rwjx0REVgAGH58e96+X0MeOfepqsbeW4= -github.com/signadot/go-sdk v0.3.8-0.20260505200838-3bae35b0d0d8 h1:KFayX3D9o1XaDA8JNIngOVEDc0t35Y5pERsAn3Tti8A= -github.com/signadot/go-sdk v0.3.8-0.20260505200838-3bae35b0d0d8/go.mod h1:8CfvBQ/AQ3LPruaQZoflmpBjoZTwXfhBt2OtS1eet+Q= +github.com/signadot/go-sdk v0.3.8-0.20260507124501-af142d7ec85c h1:1FYXrhR0NXcxC9uwwrZVU4A58mImiJ1yUACVUfoNJyY= +github.com/signadot/go-sdk v0.3.8-0.20260507124501-af142d7ec85c/go.mod h1:8CfvBQ/AQ3LPruaQZoflmpBjoZTwXfhBt2OtS1eet+Q= github.com/signadot/libconnect v0.1.1-0.20260424105947-336dce43da75 h1:LZJrEqJeSb0CGsENOAQsvDeEO2YyMY1/ir2Nz4apvbI= github.com/signadot/libconnect v0.1.1-0.20260424105947-336dce43da75/go.mod h1:cAsgAummH9Q9DrLQ7+S3mqrBv/+ZYKVSEXjR/WfoUJM= github.com/sirupsen/logrus v1.7.0/go.mod h1:yWOB1SBYBC5VeMP7gHvWumXLIWorT60ONWic61uBYv0= diff --git a/internal/command/plan/printers.go b/internal/command/plan/printers.go index 6c03f62..188da38 100644 --- a/internal/command/plan/printers.go +++ b/internal/command/plan/printers.go @@ -1,231 +1,12 @@ package plan import ( - "fmt" "io" - "sort" - "text/tabwriter" "github.com/signadot/cli/internal/command/planshared" - "github.com/signadot/cli/internal/print" - "github.com/signadot/cli/internal/utils" "github.com/signadot/go-sdk/models" ) func printPlanDetails(out io.Writer, p *models.RunnablePlan) error { - tw := tabwriter.NewWriter(out, 0, 0, 3, ' ', 0) - - fmt.Fprintf(tw, "ID:\t%s\n", p.ID) - if p.Spec != nil { - if p.Spec.Prompt != "" { - fmt.Fprintf(tw, "Prompt:\t%s\n", print.FirstLine(p.Spec.Prompt)) - } - if p.Spec.Runner != "" { - fmt.Fprintf(tw, "Runner:\t%s\n", p.Spec.Runner) - } - if c := p.Spec.Cluster; c != nil { - switch { - case c.FromCluster != "": - fmt.Fprintf(tw, "Cluster:\tfrom param %q\n", c.FromCluster) - case c.FromSandbox != "": - fmt.Fprintf(tw, "Cluster:\tfrom sandbox param %q\n", c.FromSandbox) - case c.FromRouteGroup != "": - fmt.Fprintf(tw, "Cluster:\tfrom route group param %q\n", c.FromRouteGroup) - case c.Pattern != "": - fmt.Fprintf(tw, "Cluster:\tpattern %q\n", c.Pattern) - } - } - } - if p.Status != nil { - fmt.Fprintf(tw, "Created:\t%s\n", utils.FormatTimestamp(p.Status.CreatedAt)) - if p.Status.CompiledFrom != "" { - fmt.Fprintf(tw, "Compiled From:\t%s\n", p.Status.CompiledFrom) - } - if p.Status.Executions > 0 { - fmt.Fprintf(tw, "Executions:\t%d\n", p.Status.Executions) - } - } - if err := tw.Flush(); err != nil { - return err - } - - if p.Spec == nil { - return nil - } - - if len(p.Spec.Params) > 0 { - fmt.Fprintln(out) - fmt.Fprintln(out, "Inputs:") - printPlanParams(out, p.Spec.Params) - } - - if len(p.Spec.Steps) > 0 { - fmt.Fprintln(out) - fmt.Fprintln(out, "Steps:") - printPlanSteps(out, p.Spec.Steps) - } - - if len(p.Spec.Output) > 0 { - fmt.Fprintln(out) - fmt.Fprintln(out, "Outputs:") - printPlanOutputs(out, p.Spec.Output) - } - - return nil -} - -// printPlanParams renders each declared param. Defaults render with -// the arrow form used elsewhere; required params with no default get -// a "(required)" tag, optional ones with no default a "(optional)". -func printPlanParams(out io.Writer, params []*models.PlanField) { - maxName := 0 - for _, p := range params { - if p != nil && len(p.Name) > maxName { - maxName = len(p.Name) - } - } - for _, p := range params { - if p == nil { - continue - } - var detail, via string - switch { - case p.Default != nil: - detail = planshared.FormatValue(p.Default) - via = "default" - case p.Required: - via = "required" - default: - via = "optional" - } - planshared.PrintInputLine(out, " ", maxName, p.Name, detail, via) - } -} - -func printPlanOutputs(out io.Writer, outputs map[string]string) { - names := make([]string, 0, len(outputs)) - for k := range outputs { - names = append(names, k) - } - sort.Strings(names) - maxName := 0 - for _, n := range names { - if len(n) > maxName { - maxName = len(n) - } - } - // Plan outputs are always refs (mappings to step outputs), so we - // drop the trailing (ref) tag — there's no other variant to - // disambiguate against. - for _, n := range names { - fmt.Fprintf(out, " %-*s ← %s\n", maxName, n, outputs[n]) - } -} - -func printPlanSteps(out io.Writer, steps []*models.PlanStep) { - for i, s := range steps { - if s == nil { - continue - } - if i > 0 { - fmt.Fprintln(out) - } - fmt.Fprintf(out, " %s\n", s.ID) - if s.Action != nil { - line := " action: " + planshared.ActionLabel(s.Action) - if s.Action.Revision > 0 { - line += fmt.Sprintf(" (revision %d)", s.Action.Revision) - } - fmt.Fprintln(out, line) - if img := planshared.FormatImage(s.Action.Image); img != "" { - fmt.Fprintf(out, " image: %s\n", img) - } - if s.Action.Timeout != "" { - fmt.Fprintf(out, " timeout: %s\n", s.Action.Timeout) - } else if s.Action.TimeoutInputName != "" { - fmt.Fprintf(out, " timeout: (from input %q)\n", s.Action.TimeoutInputName) - } - } - if s.Condition != "" { - fmt.Fprintf(out, " when: %s\n", s.Condition) - } - printStepInputs(out, s) - printStepOutputs(out, s) - } -} - -// printStepInputs lists the inputs the plan author wired for this -// step. Values from step.Args.Values render as "set in plan"; refs -// from step.Args.Refs render as "ref". Inputs that were neither set -// nor wired (will fall back to defaults or be unbound at run time) -// aren't shown here; that resolution is per-execution and lives in -// plan x get. -func printStepInputs(out io.Writer, s *models.PlanStep) { - values := planshared.ParamsAsMap(planshared.StepArgsValues(s)) - var refs map[string]string - if s.Args != nil { - refs = s.Args.Refs - } - if len(values) == 0 && len(refs) == 0 { - return - } - - type wired struct { - name, detail, via string - } - var inputs []wired - for name, v := range values { - inputs = append(inputs, wired{name, planshared.FormatValue(v), "set in plan"}) - } - for name, r := range refs { - inputs = append(inputs, wired{name, r, "ref"}) - } - sort.Slice(inputs, func(i, j int) bool { return inputs[i].name < inputs[j].name }) - - fmt.Fprintln(out, " inputs:") - maxName := 0 - for _, in := range inputs { - if len(in.name) > maxName { - maxName = len(in.name) - } - } - for _, in := range inputs { - planshared.PrintInputLine(out, " ", maxName, in.name, in.detail, in.via) - } -} - -// printStepOutputs lists the step's declared extra_outputs (the step- -// level extension over the action's declared outputs) with their -// schema summary. The action's own outputs are inherent to the action -// and visible via signadot plan action get. -func printStepOutputs(out io.Writer, s *models.PlanStep) { - if len(s.ExtraOutputs) == 0 { - return - } - fmt.Fprintln(out, " outputs:") - maxName := 0 - for _, o := range s.ExtraOutputs { - if o != nil && len(o.Name) > maxName { - maxName = len(o.Name) - } - } - for _, o := range s.ExtraOutputs { - if o == nil { - continue - } - schema := formatFieldSchema(o) - fmt.Fprintf(out, " %-*s %s\n", maxName, o.Name, schema) - } -} - -func formatFieldSchema(f *models.PlanField) string { - if f.SchemaRef != "" { - return fmt.Sprintf("schema: %s", f.SchemaRef) - } - if m, ok := f.Schema.(map[string]any); ok { - if t, ok := m["type"].(string); ok && t != "" { - return fmt.Sprintf("schema: %s", t) - } - } - return "" + return planshared.PrintPlanDetails(out, p) } diff --git a/internal/command/planaction/printers.go b/internal/command/planaction/printers.go index 6c00c3c..e3f8635 100644 --- a/internal/command/planaction/printers.go +++ b/internal/command/planaction/printers.go @@ -81,10 +81,10 @@ func printActionDetails(out io.Writer, a *models.PlanAction) error { } if a.Status != nil { - if err := printFields(out, "Inputs", a.Status.BodyParams); err != nil { + if err := printInputFields(out, a.Status.BodyParams); err != nil { return err } - if err := printFields(out, "Outputs", a.Status.BodyOutputs); err != nil { + if err := printOutputFields(out, a.Status.BodyOutputs); err != nil { return err } } @@ -141,23 +141,26 @@ func enabled(a *models.PlanAction) string { return fmt.Sprintf("%t", a.Status.Enabled) } -type fieldRow struct { +type inputFieldRow struct { Name string `sdtab:"NAME"` Required string `sdtab:"REQUIRED"` Default string `sdtab:"DEFAULT"` Schema string `sdtab:"SCHEMA"` } -func printFields(out io.Writer, label string, fields []*models.PlanField) error { +// printInputFields renders an action's declared input parameters. +// Inputs carry name, required-ness, default, and schema — all four +// columns are meaningful here. +func printInputFields(out io.Writer, fields []*models.PlanField) error { if len(fields) == 0 { return nil } fmt.Fprintln(out) - fmt.Fprintf(out, "%s:\n", label) - t := sdtab.New[fieldRow](out) + fmt.Fprintln(out, "Inputs:") + t := sdtab.New[inputFieldRow](out) t.AddHeader() for _, f := range fields { - t.AddRow(fieldRow{ + t.AddRow(inputFieldRow{ Name: f.Name, Required: fmt.Sprintf("%t", f.Required), Default: formatAny(f.Default), @@ -167,6 +170,31 @@ func printFields(out io.Writer, label string, fields []*models.PlanField) error return t.Flush() } +type outputFieldRow struct { + Name string `sdtab:"NAME"` + Schema string `sdtab:"SCHEMA"` +} + +// printOutputFields renders an action's declared output fields. +// Outputs don't have required-ness or defaults (those are inputs +// concepts) — only name and schema apply. +func printOutputFields(out io.Writer, fields []*models.PlanField) error { + if len(fields) == 0 { + return nil + } + fmt.Fprintln(out) + fmt.Fprintln(out, "Outputs:") + t := sdtab.New[outputFieldRow](out) + t.AddHeader() + for _, f := range fields { + t.AddRow(outputFieldRow{ + Name: f.Name, + Schema: formatSchema(f), + }) + } + return t.Flush() +} + // formatAny renders a PlanField default for the table column. Scalars // pass through fmt.Sprintf so a string default reads as its bare text; // objects and arrays go through json.Marshal so they render as diff --git a/internal/command/planrunnergroup/printers.go b/internal/command/planrunnergroup/printers.go index 04ea162..d93b1b5 100644 --- a/internal/command/planrunnergroup/printers.go +++ b/internal/command/planrunnergroup/printers.go @@ -4,12 +4,10 @@ import ( "fmt" "io" "text/tabwriter" - "time" "github.com/signadot/cli/internal/sdtab" "github.com/signadot/cli/internal/utils" "github.com/signadot/go-sdk/models" - "github.com/xeonx/timeago" ) type planRunnerGroupRow struct { @@ -23,15 +21,10 @@ func printPlanRunnerGroupTable(out io.Writer, prgs []*models.PlanRunnerGroup) er t := sdtab.New[planRunnerGroupRow](out) t.AddHeader() for _, prg := range prgs { - createdAt, err := time.Parse(time.RFC3339, prg.CreatedAt) - if err != nil { - return err - } - t.AddRow(planRunnerGroupRow{ Name: prg.Name, Cluster: prg.Spec.Cluster, - Created: timeago.NoMax(timeago.English).Format(createdAt), + Created: utils.TimeAgo(prg.CreatedAt), Status: readiness(prg), }) } diff --git a/internal/command/planshared/plan.go b/internal/command/planshared/plan.go new file mode 100644 index 0000000..b1ffaf0 --- /dev/null +++ b/internal/command/planshared/plan.go @@ -0,0 +1,244 @@ +package planshared + +import ( + "fmt" + "io" + "sort" + "text/tabwriter" + + "github.com/signadot/cli/internal/print" + "github.com/signadot/cli/internal/utils" + "github.com/signadot/go-sdk/models" +) + +// PrintPlanDetails renders a plan's full detail view: a top-level +// metadata header followed by the Inputs / Steps / Outputs sections. +// Used by plan get, compile, create, recompile, and run. +func PrintPlanDetails(out io.Writer, p *models.RunnablePlan) error { + if err := printPlanHeader(out, p); err != nil { + return err + } + PrintPlanBody(out, p) + return nil +} + +// PrintPlanBody renders only the Inputs / Steps / Outputs sections of +// a plan, skipping the top-level metadata header. Used when the plan +// is embedded under a parent block (e.g. plan tag get) where the +// parent already covers the plan's identity and timestamps. +func PrintPlanBody(out io.Writer, p *models.RunnablePlan) { + if p == nil || p.Spec == nil { + return + } + if len(p.Spec.Params) > 0 { + fmt.Fprintln(out) + fmt.Fprintln(out, "Inputs:") + printPlanParams(out, p.Spec.Params) + } + if len(p.Spec.Steps) > 0 { + fmt.Fprintln(out) + fmt.Fprintln(out, "Steps:") + printPlanSteps(out, p.Spec.Steps) + } + if len(p.Spec.Output) > 0 { + fmt.Fprintln(out) + fmt.Fprintln(out, "Outputs:") + printPlanOutputs(out, p.Spec.Output) + } +} + +func printPlanHeader(out io.Writer, p *models.RunnablePlan) error { + tw := tabwriter.NewWriter(out, 0, 0, 3, ' ', 0) + fmt.Fprintf(tw, "ID:\t%s\n", p.ID) + if p.Spec != nil { + if p.Spec.SelectionHint != "" { + fmt.Fprintf(tw, "Selection Hint:\t%s\n", p.Spec.SelectionHint) + } + if p.Spec.Prompt != "" { + fmt.Fprintf(tw, "Prompt:\t%s\n", print.FirstLine(p.Spec.Prompt)) + } + if p.Spec.Runner != "" { + fmt.Fprintf(tw, "Runner:\t%s\n", p.Spec.Runner) + } + if c := p.Spec.Cluster; c != nil { + switch { + case c.FromCluster != "": + fmt.Fprintf(tw, "Cluster:\tfrom param %q\n", c.FromCluster) + case c.FromSandbox != "": + fmt.Fprintf(tw, "Cluster:\tfrom sandbox param %q\n", c.FromSandbox) + case c.FromRouteGroup != "": + fmt.Fprintf(tw, "Cluster:\tfrom route group param %q\n", c.FromRouteGroup) + case c.Pattern != "": + fmt.Fprintf(tw, "Cluster:\tpattern %q\n", c.Pattern) + } + } + } + if p.Status != nil { + fmt.Fprintf(tw, "Created:\t%s\n", utils.FormatTimestamp(p.Status.CreatedAt)) + if by := FormatCreatedBy(p.Status.CreatedBy); by != "" { + fmt.Fprintf(tw, "Created By:\t%s\n", by) + } + if p.Status.CompiledFrom != "" { + fmt.Fprintf(tw, "Compiled From:\t%s\n", p.Status.CompiledFrom) + } + if p.Status.Executions > 0 { + fmt.Fprintf(tw, "Executions:\t%d\n", p.Status.Executions) + } + } + return tw.Flush() +} + +// printPlanParams renders each declared param. Defaults render with +// the arrow form used elsewhere; required params with no default get +// a "(required)" tag, optional ones with no default a "(optional)". +func printPlanParams(out io.Writer, params []*models.PlanField) { + maxName := 0 + for _, p := range params { + if p != nil && len(p.Name) > maxName { + maxName = len(p.Name) + } + } + for _, p := range params { + if p == nil { + continue + } + var detail, via string + switch { + case p.Default != nil: + detail = FormatValue(p.Default) + via = "default" + case p.Required: + via = "required" + default: + via = "optional" + } + PrintInputLine(out, " ", maxName, p.Name, detail, via) + } +} + +// Plan outputs are always refs (mappings to step outputs), so we +// drop the trailing (ref) tag — there's no other variant to +// disambiguate against. +func printPlanOutputs(out io.Writer, outputs map[string]string) { + names := make([]string, 0, len(outputs)) + for k := range outputs { + names = append(names, k) + } + sort.Strings(names) + maxName := 0 + for _, n := range names { + if len(n) > maxName { + maxName = len(n) + } + } + for _, n := range names { + fmt.Fprintf(out, " %-*s ← %s\n", maxName, n, outputs[n]) + } +} + +func printPlanSteps(out io.Writer, steps []*models.PlanStep) { + for i, s := range steps { + if s == nil { + continue + } + if i > 0 { + fmt.Fprintln(out) + } + fmt.Fprintf(out, " %s\n", s.ID) + if s.Action != nil { + line := " action: " + ActionLabel(s.Action) + if s.Action.Revision > 0 { + line += fmt.Sprintf(" (revision %d)", s.Action.Revision) + } + fmt.Fprintln(out, line) + if img := FormatImage(s.Action.Image); img != "" { + fmt.Fprintf(out, " image: %s\n", img) + } + if s.Action.Timeout != "" { + fmt.Fprintf(out, " timeout: %s\n", s.Action.Timeout) + } else if s.Action.TimeoutInputName != "" { + fmt.Fprintf(out, " timeout: (from input %q)\n", s.Action.TimeoutInputName) + } + } + if s.Condition != "" { + fmt.Fprintf(out, " when: %s\n", s.Condition) + } + printStepInputs(out, s) + printStepOutputs(out, s) + } +} + +// printStepInputs lists the inputs the plan author wired for this +// step. Values from step.Args.Values render as "set in plan"; refs +// from step.Args.Refs render as "ref". Inputs that were neither set +// nor wired (will fall back to defaults or be unbound at run time) +// aren't shown here; that resolution is per-execution and lives in +// plan x get. +func printStepInputs(out io.Writer, s *models.PlanStep) { + values := ParamsAsMap(StepArgsValues(s)) + var refs map[string]string + if s.Args != nil { + refs = s.Args.Refs + } + if len(values) == 0 && len(refs) == 0 { + return + } + + type wired struct { + name, detail, via string + } + var inputs []wired + for name, v := range values { + inputs = append(inputs, wired{name, FormatValue(v), "set in plan"}) + } + for name, r := range refs { + inputs = append(inputs, wired{name, r, "ref"}) + } + sort.Slice(inputs, func(i, j int) bool { return inputs[i].name < inputs[j].name }) + + fmt.Fprintln(out, " inputs:") + maxName := 0 + for _, in := range inputs { + if len(in.name) > maxName { + maxName = len(in.name) + } + } + for _, in := range inputs { + PrintInputLine(out, " ", maxName, in.name, in.detail, in.via) + } +} + +// printStepOutputs lists the step's declared extra_outputs (the step- +// level extension over the action's declared outputs) with their +// schema summary. The action's own outputs are inherent to the action +// and visible via signadot plan action get. +func printStepOutputs(out io.Writer, s *models.PlanStep) { + if len(s.ExtraOutputs) == 0 { + return + } + fmt.Fprintln(out, " outputs:") + maxName := 0 + for _, o := range s.ExtraOutputs { + if o != nil && len(o.Name) > maxName { + maxName = len(o.Name) + } + } + for _, o := range s.ExtraOutputs { + if o == nil { + continue + } + fmt.Fprintf(out, " %-*s %s\n", maxName, o.Name, formatFieldSchema(o)) + } +} + +func formatFieldSchema(f *models.PlanField) string { + if f.SchemaRef != "" { + return fmt.Sprintf("schema: %s", f.SchemaRef) + } + if m, ok := f.Schema.(map[string]any); ok { + if t, ok := m["type"].(string); ok && t != "" { + return fmt.Sprintf("schema: %s", t) + } + } + return "" +} diff --git a/internal/command/planshared/render.go b/internal/command/planshared/render.go index 4f9ba18..08a08d1 100644 --- a/internal/command/planshared/render.go +++ b/internal/command/planshared/render.go @@ -97,6 +97,43 @@ func FormatImage(ref *models.PlanImageRef) string { return "" } +// FormatCreatedBy renders a PlanStatus.CreatedBy actor for the plan +// header. Resolution by ActorType: +// +// user → UserEmail (e.g. daniel@signadot.com) +// api_key → "api key " + APIKeyMasked +// system → "system" +// +// Returns "" when the actor is nil or unrecognised with no usable +// fields, so the caller can suppress the row. +func FormatCreatedBy(c *models.CreatedBy) string { + if c == nil { + return "" + } + switch c.ActorType { + case "user": + return c.UserEmail + case "api_key": + if c.APIKeyMasked != "" { + return "api key " + c.APIKeyMasked + } + return "api key" + case "system": + return "system" + } + // Unknown actor type — fall back to whatever the server returned + // so future server-side additions surface rather than getting + // silently dropped. + switch { + case c.UserEmail != "": + return c.UserEmail + case c.APIKeyMasked != "": + return c.APIKeyMasked + default: + return c.ActorType + } +} + // ActionLabel renders the step's action as "name (id)" when the // server returns both, name alone when the ID is empty, or ID alone // when the name isn't populated yet (older plans compiled before the diff --git a/internal/command/plantag/printers.go b/internal/command/plantag/printers.go index 2dab7ad..fdbcd5f 100644 --- a/internal/command/plantag/printers.go +++ b/internal/command/plantag/printers.go @@ -5,6 +5,7 @@ import ( "io" "text/tabwriter" + "github.com/signadot/cli/internal/command/planshared" "github.com/signadot/cli/internal/print" "github.com/signadot/cli/internal/sdtab" "github.com/signadot/cli/internal/utils" @@ -55,33 +56,32 @@ func printTagDetails(out io.Writer, tag *models.PlanTag) error { if tag.Spec != nil { fmt.Fprintf(tw, "Plan ID:\t%s\n", tag.Spec.PlanID) } + if tag.Plan != nil && tag.Plan.Spec != nil && tag.Plan.Spec.SelectionHint != "" { + fmt.Fprintf(tw, "Selection Hint:\t%s\n", tag.Plan.Spec.SelectionHint) + } + if tag.Plan != nil && tag.Plan.Status != nil { + if by := planshared.FormatCreatedBy(tag.Plan.Status.CreatedBy); by != "" { + // "Plan Created By" rather than "Created By" so it's + // unambiguous against the tag-level Created/Updated fields + // just below. + fmt.Fprintf(tw, "Plan Created By:\t%s\n", by) + } + } if tag.Status != nil { fmt.Fprintf(tw, "Created:\t%s\n", utils.FormatTimestamp(tag.Status.CreatedAt)) fmt.Fprintf(tw, "Updated:\t%s\n", utils.FormatTimestamp(tag.Status.UpdatedAt)) } - if err := tw.Flush(); err != nil { return err } - // If the tag has an inlined plan, show its details. + // If the tag has an inlined plan, render its body (Inputs / Steps + // / Outputs) with the same shape plan get uses. Plan-level + // metadata (ID, prompt, runner, ...) is omitted: ID and + // SelectionHint are already at the tag level above; the rest is + // one signadot plan get away. if tag.Plan != nil { - fmt.Fprintln(out) - fmt.Fprintln(out, "Plan:") - tw = tabwriter.NewWriter(out, 0, 0, 3, ' ', 0) - fmt.Fprintf(tw, " ID:\t%s\n", tag.Plan.ID) - if tag.Plan.Spec != nil { - fmt.Fprintf(tw, " Steps:\t%d\n", len(tag.Plan.Spec.Steps)) - if tag.Plan.Spec.Prompt != "" { - fmt.Fprintf(tw, " Prompt:\t%s\n", print.FirstLine(tag.Plan.Spec.Prompt)) - } - } - if tag.Plan.Status != nil { - fmt.Fprintf(tw, " Created:\t%s\n", utils.FormatTimestamp(tag.Plan.Status.CreatedAt)) - } - if err := tw.Flush(); err != nil { - return err - } + planshared.PrintPlanBody(out, tag.Plan) } // Print tag history if present. From d03f3c03ae7eeffc5115a4f5161df67c8135fec4 Mon Sep 17 00:00:00 2001 From: Daniel De Vera Date: Thu, 7 May 2026 10:40:05 -0300 Subject: [PATCH 2/2] planshared: render empty strings as the literal "" in FormatValue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Empty-string values previously collapsed to the same shape as nil/missing values: name (set in plan) with no detail. That hides the distinction between "value not provided" and "deliberately empty," which matters when an action interprets the empty string specifically — e.g. run-image treats image:"" as "run on the host runner without a container," a meaningful authoring choice the reader needs to see. Now an empty string renders as the literal "" (with quotes): image ← "" (set in plan) and an unset/nil value still renders as no-detail: image (set in plan) Same disambiguation applies wherever FormatValue is used (plan get's plan-level Inputs, plan x get's resolved inputs, plan get's declared param defaults, output rows). Scalars still pass through unchanged; objects and arrays continue to round-trip through json.Marshal. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/command/planshared/render.go | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/internal/command/planshared/render.go b/internal/command/planshared/render.go index 08a08d1..57fae8a 100644 --- a/internal/command/planshared/render.go +++ b/internal/command/planshared/render.go @@ -45,21 +45,28 @@ func PrintInputLine(out io.Writer, indent string, nameWidth int, name, detail, v // rendering as Go's map[a:1] / [1 2] forms. Multi-line and very long // values are summarised so they don't break the inline arrow form; // users who want the full value can use -o yaml. +// +// Empty strings render as the literal "" (with quotes) so they're +// distinguishable from nil/missing values. An explicit empty string +// can be a meaningful value the action interprets specifically (e.g. +// run-image treats image:"" as "run on the host runner without a +// container"); collapsing it to the same shape as "value not +// provided" would hide that intent from the reader. func FormatValue(v any) string { if v == nil { return "" } - var raw string if s, ok := v.(string); ok { - raw = s - } else { - b, err := json.Marshal(v) - if err != nil { - return fmt.Sprintf("%v", v) + if s == "" { + return `""` } - raw = string(b) + return truncateForDisplay(s) + } + b, err := json.Marshal(v) + if err != nil { + return fmt.Sprintf("%v", v) } - return truncateForDisplay(raw) + return truncateForDisplay(string(b)) } // truncateForDisplay collapses a value into a single line suitable