From 83f8f20e4f433c38d15e4e118ad42b70ce3426aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=83=9E=E3=82=A4=E3=83=8E=E3=83=AB?= <97069334+MY-RV@users.noreply.github.com> Date: Sun, 20 Sep 2026 15:42:05 -0600 Subject: [PATCH 1/3] feat!: add the engine block and the runner axis MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps, and they are the same gap. A catalog could say which script answers a set of tokens, but not how that script's body becomes a process — that was hardcoded. And there was nowhere to say what godo itself needs, so any such setting would have landed beside the scripts it reads. engine: is that place. scripts: is the catalog, the data; engine: is every dial godo turns while reading and running it — the binary it expects, the dialect, the runner, the plugins. dialect moves there; a top-level dialect: keeps working, because it shipped in 0.1 and 0.2 and there is no reason to break a file over where a key sits. A Runner carries per-invocation state (working directory, stdio), so the registry takes the name at Register rather than reading it off the type, and an unset runner resolves to the Runner injected into the Engine — which keeps NewEngine(cat, runner) working with nothing registered, and lets the CLI decide what the default is. A resolver hook answers for names registered nowhere, which is how a catalog names a shell without godo keeping a list of every shell anyone might have. Unknown runner names fail when the plan is built rather than at load: a dialect must resolve before a script can be matched at all, while a runner is only needed to execute, and the registry that answers for it belongs to the Engine. Both --preview and a plain run fail closed. Each plan step records its own runner, so a dep declaring @runner keeps it instead of inheriting the caller's. ArgsAwareRunner moves one more question to the runner. Reading the body for ${godo:args…} is a fact about shell templates: a plugin's body is a program, has no such placeholder, and would have every extra token rejected. A runner that does not implement it keeps the old policy, message included. engine.plugins is parsed and validated but loaded by nothing. Declaring the shape now is what makes the failure legible — a script asking for a runner a plugin provides names that plugin instead of reading as a typo. sha256 is required from the start: a plugin is third-party code that runs when someone types 'godo test', and a digest is the only thing that says it is the code that was reviewed. config is optional and entirely the plugin's; godo carries it across without reading it. --- internal/catalog/engine.go | 132 ++++++++++-- internal/catalog/engine_block.go | 110 ++++++++++ internal/catalog/engine_block_test.go | 186 ++++++++++++++++ internal/catalog/engine_test.go | 291 ++++++++++++++++++++++++++ internal/catalog/errors.go | 1 + internal/catalog/load.go | 64 ++++-- internal/catalog/runner.go | 102 +++++++++ internal/catalog/types.go | 3 + pkg.go | 12 ++ 9 files changed, 875 insertions(+), 26 deletions(-) create mode 100644 internal/catalog/engine_block.go create mode 100644 internal/catalog/engine_block_test.go create mode 100644 internal/catalog/runner.go diff --git a/internal/catalog/engine.go b/internal/catalog/engine.go index 7c6d8e4..c1d4094 100644 --- a/internal/catalog/engine.go +++ b/internal/catalog/engine.go @@ -13,16 +13,41 @@ type Runner interface { Run(command string) error } +// ArgsAwareRunner is a Runner that decides for itself whether a body takes the +// tokens left over after the match. +// +// The default policy reads the body for ${godo:args…} and rejects leftover +// tokens when it finds none. That is a fact about shell templates, not about +// godo: a plugin's body is a program, has no such placeholder, and would have +// every extra token rejected. "Does this body accept leftover args?" is the +// runner's question, and a runner that implements this answers it. +type ArgsAwareRunner interface { + Runner + AcceptsArgs(commands []string) bool +} + // Engine resolves matches, deps, expansion, and run/preview. type Engine struct { Catalog *Catalog Dialects *DialectRegistry - Runner Runner + Runners *RunnerRegistry + // Runner is the default: what a script that names no runner resolves to. + // Named runners live in Runners. + Runner Runner } // EngineOption configures NewEngine. type EngineOption func(*Engine) +// WithRunners overrides the named-runner registry. +func WithRunners(r *RunnerRegistry) EngineOption { + return func(e *Engine) { + if r != nil { + e.Runners = r + } + } +} + // WithDialects overrides the dialect registry. func WithDialects(r *DialectRegistry) EngineOption { return func(e *Engine) { @@ -37,6 +62,7 @@ func NewEngine(cat *Catalog, runner Runner, opts ...EngineOption) *Engine { e := &Engine{ Catalog: cat, Dialects: DefaultDialects(), + Runners: DefaultRunners(), Runner: runner, } for _, opt := range opts { @@ -78,6 +104,47 @@ func (e *Engine) Resolve(tokens []string) (*Match, error) { return nil, fmt.Errorf("%w: %v", ErrNoMatch, tokens) } +// runnerFor resolves the Runner that executes one step. +// +// The registry wins. Otherwise the unset name means the Runner injected into +// the Engine. +func (e *Engine) runnerFor(name RunnerName) (Runner, error) { + if r, err := e.Runners.Lookup(name); err == nil { + return r, nil + } + if name == "" { + if e.Runner == nil { + return nil, fmt.Errorf("nil runner") + } + return e.Runner, nil + } + return nil, fmt.Errorf("%w: %q", ErrUnknownRunner, name) +} + +// stepRunner returns the effective runner name for a script, rejecting names +// nothing answers to. +// +// This is a name check, not an instance check: BuildPlan must stay usable with +// a nil Runner, which is how PreviewLines expands without anything to execute. +func (e *Engine) stepRunner(s Script) (RunnerName, error) { + name := EffectiveRunner(s, e.Catalog.Runner) + if name == "" { + return name, nil + } + if _, err := e.Runners.Lookup(name); err != nil { + // A runner the catalog declared a plugin for is a different failure + // from a typo, and saying so saves the reader the hunt. + if p, ok := e.Catalog.Engine.ProviderOf("runner", string(name)); ok { + return "", fmt.Errorf("%w: script %q asks for runner %q, provided by plugin %s — this build cannot load plugins", + ErrUnknownRunner, s.Key, name, p.Source) + } + // Otherwise the registry's resolver knows why (unknown name, shell not + // installed); its message is the one worth reading. + return "", fmt.Errorf("script %q asks for runner %q: %w", s.Key, name, err) + } + return name, nil +} + // Plan is the expanded execution plan (deps + body). type Plan struct { Match *Match @@ -89,6 +156,7 @@ type PlanStep struct { Kind string // "dep" | "body" Source string Command string + Runner RunnerName // effective runner for this step } // BuildPlan expands deps + body without executing. @@ -102,7 +170,11 @@ func (e *Engine) BuildPlan(tokens []string) (*Plan, error) { if err != nil { return nil, err } - if err := validateArgs(m); err != nil { + runner, err := e.stepRunner(m.Script) + if err != nil { + return nil, err + } + if err := e.validateArgs(m, runner); err != nil { return nil, err } plan := &Plan{Match: m} @@ -116,17 +188,44 @@ func (e *Engine) BuildPlan(tokens []string) (*Plan, error) { return nil, err } for _, c := range cmds { - plan.Steps = append(plan.Steps, PlanStep{Kind: "body", Source: m.Script.Key, Command: c}) + plan.Steps = append(plan.Steps, PlanStep{Kind: "body", Source: m.Script.Key, Command: c, Runner: runner}) } return plan, nil } -func validateArgs(m *Match) error { - need := expand.CommandsNeedArgs(m.Script.Commands) - if !need && len(m.Args) > 0 { - return fmt.Errorf("%w: %v (script %q has no ${godo:args})", ErrUnexpectedArgs, m.Args, m.Script.Key) +// validateArgs rejects leftover tokens a script cannot take. +// +// Which tokens a script can take depends on the runner, so the check asks the +// runner when it has an opinion and falls back to the placeholder policy +// otherwise. +func (e *Engine) validateArgs(m *Match, runner RunnerName) error { + if len(m.Args) == 0 { + return nil } - return nil + if ar, ok := e.argsAwareRunner(runner); ok { + if ar.AcceptsArgs(m.Script.Commands) { + return nil + } + return fmt.Errorf("%w: %v (script %q takes no args under runner %q)", ErrUnexpectedArgs, m.Args, m.Script.Key, runner) + } + if expand.CommandsNeedArgs(m.Script.Commands) { + return nil + } + return fmt.Errorf("%w: %v (script %q has no ${godo:args})", ErrUnexpectedArgs, m.Args, m.Script.Key) +} + +// argsAwareRunner reports whether the runner for this step answers the args +// question itself. Registry first, then the injected default, like runnerFor. +func (e *Engine) argsAwareRunner(name RunnerName) (ArgsAwareRunner, bool) { + if r, err := e.Runners.Lookup(name); err == nil { + ar, ok := r.(ArgsAwareRunner) + return ar, ok + } + if name == "" { + ar, ok := e.Runner.(ArgsAwareRunner) + return ar, ok + } + return nil, false } // appendDeps walks m's dependencies depth-first. @@ -158,7 +257,11 @@ func (e *Engine) appendDeps(plan *Plan, m *Match, stack, done map[string]bool) e if err != nil { return fmt.Errorf("deps %q: %w", invKey, err) } - if err := validateArgs(depMatch); err != nil { + depRunner, err := e.stepRunner(depMatch.Script) + if err != nil { + return err + } + if err := e.validateArgs(depMatch, depRunner); err != nil { return err } if err := e.appendDeps(plan, depMatch, stack, done); err != nil { @@ -169,7 +272,7 @@ func (e *Engine) appendDeps(plan *Plan, m *Match, stack, done map[string]bool) e return err } for _, c := range cmds { - plan.Steps = append(plan.Steps, PlanStep{Kind: "dep", Source: invKey, Command: c}) + plan.Steps = append(plan.Steps, PlanStep{Kind: "dep", Source: invKey, Command: c, Runner: depRunner}) } delete(stack, invKey) done[invKey] = true @@ -183,11 +286,12 @@ func (e *Engine) Run(tokens []string) error { if err != nil { return err } - if e.Runner == nil { - return fmt.Errorf("nil runner") - } for _, step := range plan.Steps { - if err := e.Runner.Run(step.Command); err != nil { + runner, err := e.runnerFor(step.Runner) + if err != nil { + return err + } + if err := runner.Run(step.Command); err != nil { return err } } diff --git a/internal/catalog/engine_block.go b/internal/catalog/engine_block.go new file mode 100644 index 0000000..88b7af9 --- /dev/null +++ b/internal/catalog/engine_block.go @@ -0,0 +1,110 @@ +package catalog + +import ( + "fmt" + "strings" +) + +// EngineSpec is the `engine:` block: what godo itself needs in order to run +// this catalog. +// +// Everything else in the file describes the scripts — which one answers your +// tokens, how its body runs. This describes the tool: the binary a catalog +// expects, and the plugins it wants loaded. Keeping them apart is what lets +// the toolchain side grow (a lockfile, a package manager) without the script +// side growing with it. +// +// Named EngineSpec, not Engine, because Engine is already the thing that +// resolves and runs a catalog. +type EngineSpec struct { + // Version is the minimum godo binary, "0.3.0" or ">=0.3.0". + Version string `yaml:"version"` + // Dialect and Runner configure how godo reads and runs this file. They are + // here rather than at the top level because they are settings for the tool, + // not content of the catalog: scripts: is the data, engine: is the dial. + Dialect DialectName `yaml:"dialect"` + Runner RunnerName `yaml:"runner"` + Plugins []Plugin `yaml:"plugins"` +} + +// Plugin is one entry of `engine.plugins`. +// +// Nothing loads these yet. They are parsed and validated so the shape is +// settled and a catalog can already declare what it expects; a script asking +// for a runner a plugin provides fails with that plugin named, rather than +// with "unknown runner". +type Plugin struct { + Source string `yaml:"source"` + // SHA256 is required. A plugin is third-party code that runs when someone + // types `godo test`; without a digest there is nothing to verify it is the + // code that was reviewed. + SHA256 string `yaml:"sha256"` + // Provides entries are ":", kind being runner or dialect. + Provides []string `yaml:"provides"` + // Config is optional and entirely the plugin's: its keys, its meaning, its + // defaults. godo carries it across and does not read it. What a plugin + // treats as deny-all, or ignores, is the plugin's contract, not godo's. + Config map[string]any `yaml:"config"` +} + +// Minimum returns the minimum binary version the catalog asks for, and whether +// it asked at all. +func (e EngineSpec) Minimum() (string, bool) { + v := strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(e.Version), ">=")) + return v, v != "" +} + +// ProviderOf returns the plugin that says it provides ":". +func (e EngineSpec) ProviderOf(kind, name string) (Plugin, bool) { + want := kind + ":" + name + for _, p := range e.Plugins { + for _, got := range p.Provides { + if got == want { + return p, true + } + } + } + return Plugin{}, false +} + +// validateEngine checks the block at load time. +func validateEngine(e EngineSpec) error { + if v := strings.TrimSpace(e.Version); v != "" { + rest := strings.TrimSpace(strings.TrimPrefix(v, ">=")) + if rest == "" { + return fmt.Errorf("engine.version %q: want a version, as \"0.3.0\" or \">=0.3.0\"", e.Version) + } + if strings.ContainsAny(rest, "<>=~^ ") { + return fmt.Errorf("engine.version %q: only a minimum is supported, as \"0.3.0\" or \">=0.3.0\"", e.Version) + } + } + seen := map[string]string{} + for i, p := range e.Plugins { + where := fmt.Sprintf("engine.plugins[%d]", i) + if strings.TrimSpace(p.Source) == "" { + return fmt.Errorf("%s: source is required", where) + } + if strings.TrimSpace(p.SHA256) == "" { + return fmt.Errorf("%s (%s): sha256 is required", where, p.Source) + } + if len(p.Provides) == 0 { + return fmt.Errorf("%s (%s): provides is required, as [runner:name]", where, p.Source) + } + for _, entry := range p.Provides { + kind, name, ok := strings.Cut(entry, ":") + if !ok || name == "" { + return fmt.Errorf("%s (%s): provides %q: want \":\"", where, p.Source, entry) + } + switch kind { + case "runner", "dialect": + default: + return fmt.Errorf("%s (%s): provides %q: kind must be runner or dialect", where, p.Source, entry) + } + if prev, dup := seen[entry]; dup { + return fmt.Errorf("%s (%s): provides %q already provided by %s", where, p.Source, entry, prev) + } + seen[entry] = p.Source + } + } + return nil +} diff --git a/internal/catalog/engine_block_test.go b/internal/catalog/engine_block_test.go new file mode 100644 index 0000000..7195245 --- /dev/null +++ b/internal/catalog/engine_block_test.go @@ -0,0 +1,186 @@ +package catalog_test + +import ( + "errors" + "strings" + "testing" + + "github.com/my-rv/godo/internal/catalog" +) + +const pluginBlock = `version: "0.1" + +engine: + version: ">=0.3.0" + plugins: + - source: https://example.test/godo-micropy@v1.2.0 + sha256: deadbeef + provides: [runner:micropy] + config: + proc: {exec: true, spawn: false} + stdlib: [os, os.path] + +scripts: + # @runner micropy + t: print("hi") +` + +func TestParse_engineBlock(t *testing.T) { + cat, err := catalog.Parse([]byte(pluginBlock), "x") + if err != nil { + t.Fatal(err) + } + min, ok := cat.Engine.Minimum() + if !ok || min != "0.3.0" { + t.Fatalf("minimum=%q ok=%v", min, ok) + } + if len(cat.Engine.Plugins) != 1 { + t.Fatalf("plugins=%+v", cat.Engine.Plugins) + } + p := cat.Engine.Plugins[0] + if p.SHA256 != "deadbeef" || p.Source == "" { + t.Fatalf("plugin=%+v", p) + } + // Config is the plugin's own; godo carries it without interpreting it. + if _, ok := p.Config["proc"]; !ok { + t.Fatalf("config=%+v", p.Config) + } + got, ok := cat.Engine.ProviderOf("runner", "micropy") + if !ok || got.Source != p.Source { + t.Fatalf("ProviderOf=%+v %v", got, ok) + } + if _, ok := cat.Engine.ProviderOf("runner", "nope"); ok { + t.Fatal("ProviderOf matched a name nothing provides") + } +} + +func TestParse_engineMinimumAcceptsBareVersion(t *testing.T) { + cat, err := catalog.Parse([]byte("version: \"0.1\"\nengine:\n version: 0.3.0\nscripts:\n t: echo hi\n"), "x") + if err != nil { + t.Fatal(err) + } + if min, ok := cat.Engine.Minimum(); !ok || min != "0.3.0" { + t.Fatalf("minimum=%q ok=%v", min, ok) + } +} + +func TestParse_noEngineBlockAsksForNothing(t *testing.T) { + cat, err := catalog.Parse([]byte("version: \"0.1\"\nscripts:\n t: echo hi\n"), "x") + if err != nil { + t.Fatal(err) + } + if _, ok := cat.Engine.Minimum(); ok { + t.Fatal("a catalog with no engine block must ask for no version") + } + if len(cat.Engine.Plugins) != 0 { + t.Fatalf("plugins=%+v", cat.Engine.Plugins) + } +} + +func TestParse_engineRejects(t *testing.T) { + cases := []struct{ name, src, want string }{ + {"no source", "engine:\n plugins:\n - sha256: a\n provides: [runner:x]\n", "source is required"}, + {"no digest", "engine:\n plugins:\n - source: s\n provides: [runner:x]\n", "sha256 is required"}, + {"no provides", "engine:\n plugins:\n - source: s\n sha256: a\n", "provides is required"}, + {"bad kind", "engine:\n plugins:\n - source: s\n sha256: a\n provides: [wat:x]\n", "kind must be runner or dialect"}, + {"no name", "engine:\n plugins:\n - source: s\n sha256: a\n provides: [runner]\n", `want ":"`}, + {"range syntax", "engine:\n version: \"^1.2\"\n", "only a minimum is supported"}, + {"empty constraint", "engine:\n version: \">=\"\n", "want a version"}, + { + "duplicate provides", + "engine:\n plugins:\n - source: a\n sha256: x\n provides: [runner:m]\n" + + " - source: b\n sha256: y\n provides: [runner:m]\n", + "already provided by a", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := catalog.Parse([]byte("version: \"0.1\"\n"+tc.src+"scripts:\n t: echo hi\n"), "x") + if err == nil || !errors.Is(err, catalog.ErrInvalidCatalog) { + t.Fatalf("err=%v", err) + } + if !strings.Contains(err.Error(), tc.want) { + t.Fatalf("err=%v, want %q", err, tc.want) + } + }) + } +} + +// A runner a plugin declares fails by naming the plugin, not as a typo. +func TestBuildPlan_pluginRunnerNamesItsPlugin(t *testing.T) { + cat, err := catalog.Parse([]byte(pluginBlock), "x") + if err != nil { + t.Fatal(err) + } + eng := catalog.NewEngine(cat, runnerFunc(func(string) error { return nil })) + _, err = eng.BuildPlan([]string{"t"}) + if !errors.Is(err, catalog.ErrUnknownRunner) { + t.Fatalf("err=%v", err) + } + for _, want := range []string{"micropy", "godo-micropy", "cannot load plugins"} { + if !strings.Contains(err.Error(), want) { + t.Fatalf("err=%v, missing %q", err, want) + } + } +} + +// dialect and runner live in engine:. +func TestParse_engineDialectAndRunner(t *testing.T) { + src := "version: \"0.1\"\nengine:\n dialect: matcher\n runner: bash\nscripts:\n t ${M}: echo ${godo:argv[M]}\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + if cat.Dialect != catalog.DialectMatcher { + t.Fatalf("dialect=%q", cat.Dialect) + } + if cat.Runner != "bash" { + t.Fatalf("runner=%q", cat.Runner) + } +} + +// Top-level dialect: shipped in 0.1 and 0.2, so it keeps working. +func TestParse_legacyTopLevelDialect(t *testing.T) { + cat, err := catalog.Parse([]byte("version: \"0.1\"\ndialect: matcher\nscripts:\n t ${M}: echo hi\n"), "x") + if err != nil { + t.Fatal(err) + } + if cat.Dialect != catalog.DialectMatcher { + t.Fatalf("dialect=%q", cat.Dialect) + } +} + +// engine.dialect wins when both are present. +func TestParse_engineDialectBeatsLegacy(t *testing.T) { + src := "version: \"0.1\"\ndialect: package\nengine:\n dialect: matcher\nscripts:\n t ${M}: echo hi\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + if cat.Dialect != catalog.DialectMatcher { + t.Fatalf("dialect=%q", cat.Dialect) + } +} + +// config is the plugin's: optional, and godo carries it without reading it. +func TestParse_pluginConfigIsOptionalAndUninterpreted(t *testing.T) { + src := "version: \"0.1\"\nengine:\n plugins:\n" + + " - source: s\n sha256: a\n provides: [runner:x]\n" + + " - source: t\n sha256: b\n provides: [runner:y]\n" + + " config: {anything: [1, 2], nested: {deep: true}}\n" + + "scripts:\n t: echo hi\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + if cat.Engine.Plugins[0].Config != nil { + t.Fatalf("absent config should stay nil, got %+v", cat.Engine.Plugins[0].Config) + } + cfg := cat.Engine.Plugins[1].Config + if _, ok := cfg["anything"]; !ok { + t.Fatalf("config=%+v", cfg) + } + if _, ok := cfg["nested"]; !ok { + t.Fatalf("config=%+v", cfg) + } +} diff --git a/internal/catalog/engine_test.go b/internal/catalog/engine_test.go index 018cb26..84c61c9 100644 --- a/internal/catalog/engine_test.go +++ b/internal/catalog/engine_test.go @@ -392,3 +392,294 @@ func TestEngine_argsAreQuotedIntoTheShellLine(t *testing.T) { t.Fatalf("%q", ran) } } + +// --- runner axis ------------------------------------------------------------ + +// An unset runner stays unset and means "the Runner this engine was given", +// so an embedder's injected runner keeps working without naming anything. +func TestParse_unsetRunnerMeansTheInjectedOne(t *testing.T) { + cat, err := catalog.Parse([]byte("version: \"0.1\"\nscripts:\n t: echo ok\n"), "x") + if err != nil { + t.Fatal(err) + } + if cat.Runner != "" { + t.Fatalf("file runner=%q, want empty", cat.Runner) + } + if got := catalog.EffectiveRunner(cat.Scripts[0], cat.Runner); got != "" { + t.Fatalf("effective=%q, want empty", got) + } + var ran []string + eng := catalog.NewEngine(cat, runnerFunc(func(c string) error { ran = append(ran, c); return nil })) + if err := eng.Run([]string{"t"}); err != nil { + t.Fatal(err) + } + if len(ran) != 1 || ran[0] != "echo ok" { + t.Fatalf("ran=%q", ran) + } +} + +func TestParse_fileRunnerAndDecoratorOverride(t *testing.T) { + src := "version: \"0.1\"\nengine:\n runner: micropy\nscripts:\n" + + " inherits: echo a\n" + + " # @runner bash\n" + + " overrides: echo b\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + if cat.Runner != "micropy" { + t.Fatalf("file runner=%q", cat.Runner) + } + if got := catalog.EffectiveRunner(cat.Scripts[0], cat.Runner); got != "micropy" { + t.Fatalf("inherits=%q", got) + } + if got := catalog.EffectiveRunner(cat.Scripts[1], cat.Runner); got != "bash" { + t.Fatalf("overrides=%q", got) + } +} + +func TestParse_runnerDecoratorRequiresName(t *testing.T) { + src := "version: \"0.1\"\nscripts:\n # @runner\n t: echo ok\n" + _, err := catalog.Parse([]byte(src), "x") + if err == nil || !errors.Is(err, catalog.ErrInvalidCatalog) { + t.Fatalf("err=%v", err) + } + if !strings.Contains(err.Error(), "@runner requires a name") { + t.Fatalf("err=%v", err) + } +} + +// An older binary has no @runner, and its unknown-decorator rejection is what +// stops a body meant for another runner from reaching the host shell. +func TestParse_unknownDecoratorStillRejected(t *testing.T) { + src := "version: \"0.1\"\nscripts:\n # @executor exec\n t: echo ok\n" + _, err := catalog.Parse([]byte(src), "x") + if err == nil || !strings.Contains(err.Error(), "unknown decorator @executor") { + t.Fatalf("err=%v", err) + } +} + +func TestBuildPlan_unknownRunnerFailsClosed(t *testing.T) { + src := "version: \"0.1\"\nscripts:\n # @runner nope\n t: echo ok\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + eng := catalog.NewEngine(cat, runnerFunc(func(string) error { return nil })) + if _, err := eng.BuildPlan([]string{"t"}); !errors.Is(err, catalog.ErrUnknownRunner) { + t.Fatalf("err=%v", err) + } + // Nothing must execute, so --preview has to fail the same way. + if _, err := eng.PreviewLines([]string{"t"}); !errors.Is(err, catalog.ErrUnknownRunner) { + t.Fatalf("preview err=%v", err) + } +} + +func TestRun_dispatchesPerStepRunner(t *testing.T) { + src := "version: \"0.1\"\nscripts:\n" + + " # @runner other\n" + + " dep: echo dep\n" + + " # @deps dep\n" + + " body: echo body\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + var shell, other []string + reg := catalog.NewRunnerRegistry() + if err := reg.Register("other", runnerFunc(func(c string) error { + other = append(other, c) + return nil + })); err != nil { + t.Fatal(err) + } + eng := catalog.NewEngine(cat, runnerFunc(func(c string) error { + shell = append(shell, c) + return nil + }), catalog.WithRunners(reg)) + + if err := eng.Run([]string{"body"}); err != nil { + t.Fatal(err) + } + if len(other) != 1 || other[0] != "echo dep" { + t.Fatalf("other=%v", other) + } + if len(shell) != 1 || shell[0] != "echo body" { + t.Fatalf("shell=%v", shell) + } +} + +func TestBuildPlan_stepsRecordTheirRunner(t *testing.T) { + src := "version: \"0.1\"\nscripts:\n" + + " # @runner other\n" + + " dep: echo dep\n" + + " # @deps dep\n" + + " body: echo body\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + reg := catalog.NewRunnerRegistry() + _ = reg.Register("other", runnerFunc(func(string) error { return nil })) + eng := catalog.NewEngine(cat, nil, catalog.WithRunners(reg)) + + plan, err := eng.BuildPlan([]string{"body"}) + if err != nil { + t.Fatal(err) + } + if len(plan.Steps) != 2 { + t.Fatalf("steps=%+v", plan.Steps) + } + // The dep named one; the body named none, so it is the injected default. + if plan.Steps[0].Runner != "other" || plan.Steps[1].Runner != "" { + t.Fatalf("runners=%q %q", plan.Steps[0].Runner, plan.Steps[1].Runner) + } +} + +// A named runner comes from the registry, never from the injected default. +func TestRun_namedRunnerBeatsTheInjectedDefault(t *testing.T) { + cat, err := catalog.Parse([]byte("version: \"0.1\"\nengine:\n runner: inherit\nscripts:\n t: echo ok\n"), "x") + if err != nil { + t.Fatal(err) + } + var injected, registered int + reg := catalog.NewRunnerRegistry() + _ = reg.Register(catalog.RunnerInherit, runnerFunc(func(string) error { registered++; return nil })) + eng := catalog.NewEngine(cat, runnerFunc(func(string) error { injected++; return nil }), catalog.WithRunners(reg)) + if err := eng.Run([]string{"t"}); err != nil { + t.Fatal(err) + } + if registered != 1 || injected != 0 { + t.Fatalf("registered=%d injected=%d", registered, injected) + } +} + +func TestRun_nilShellRunnerStillErrors(t *testing.T) { + cat, err := catalog.Parse([]byte("version: \"0.1\"\nscripts:\n t: echo ok\n"), "x") + if err != nil { + t.Fatal(err) + } + if err := catalog.NewEngine(cat, nil).Run([]string{"t"}); err == nil || + !strings.Contains(err.Error(), "nil runner") { + t.Fatalf("err=%v", err) + } +} + +func TestRunnerRegistry_rejectsEmptyNameAndNilRunner(t *testing.T) { + reg := catalog.NewRunnerRegistry() + if err := reg.Register("", runnerFunc(func(string) error { return nil })); err == nil { + t.Fatal("want error for empty name") + } + if err := reg.Register("x", nil); err == nil { + t.Fatal("want error for nil runner") + } + if _, err := reg.Lookup("x"); !errors.Is(err, catalog.ErrUnknownRunner) { + t.Fatalf("err=%v", err) + } +} + +// --- args are the runner's question ----------------------------------------- + +// pluginRunner stands in for a plugin whose body is a program rather than a +// shell template: no ${godo:args} to find, and it takes args regardless. +type pluginRunner struct{ ran []string } + +func (r *pluginRunner) Run(c string) error { r.ran = append(r.ran, c); return nil } +func (r *pluginRunner) AcceptsArgs([]string) bool { return true } + +type strictRunner struct{} + +func (strictRunner) Run(string) error { return nil } +func (strictRunner) AcceptsArgs([]string) bool { return false } + +// Default policy, unchanged: no ${godo:args} in the body → extra tokens error. +func TestBuildPlan_defaultPolicyStillRejectsUnexpectedArgs(t *testing.T) { + cat, err := catalog.Parse([]byte("version: \"0.1\"\nscripts:\n t: echo hi\n"), "x") + if err != nil { + t.Fatal(err) + } + eng := catalog.NewEngine(cat, runnerFunc(func(string) error { return nil })) + _, err = eng.BuildPlan([]string{"t", "extra"}) + if !errors.Is(err, catalog.ErrUnexpectedArgs) { + t.Fatalf("err=%v", err) + } + if !strings.Contains(err.Error(), "${godo:args}") { + t.Fatalf("err=%v", err) + } +} + +// A plugin body has no placeholder, and still has to be able to take args. +func TestBuildPlan_argsAwareRunnerOverridesThePlaceholderPolicy(t *testing.T) { + src := "version: \"0.1\"\nscripts:\n # @runner micropy\n t: print(godo.args)\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + reg := catalog.NewRunnerRegistry() + pr := &pluginRunner{} + if err := reg.Register("micropy", pr); err != nil { + t.Fatal(err) + } + eng := catalog.NewEngine(cat, nil, catalog.WithRunners(reg)) + if err := eng.Run([]string{"t", "--flag", "a b"}); err != nil { + t.Fatalf("run: %v", err) + } + if len(pr.ran) != 1 || pr.ran[0] != "print(godo.args)" { + t.Fatalf("ran=%q", pr.ran) + } +} + +func TestBuildPlan_argsAwareRunnerCanRefuse(t *testing.T) { + src := "version: \"0.1\"\nscripts:\n # @runner strict\n t: echo ${godo:args}\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + reg := catalog.NewRunnerRegistry() + _ = reg.Register("strict", strictRunner{}) + eng := catalog.NewEngine(cat, nil, catalog.WithRunners(reg)) + _, err = eng.BuildPlan([]string{"t", "extra"}) + if !errors.Is(err, catalog.ErrUnexpectedArgs) { + t.Fatalf("err=%v", err) + } + if !strings.Contains(err.Error(), `runner "strict"`) { + t.Fatalf("err=%v", err) + } +} + +// Deps are matched on their own, so the check uses the dep's runner. +func TestBuildPlan_depArgsUseTheDepRunner(t *testing.T) { + src := "version: \"0.1\"\ndialect: matcher\nscripts:\n" + + " # @runner micropy\n" + + " seed ${ENV}: print(${godo:argv[ENV]})\n" + + " # @deps seed dev\n" + + " boot: echo up\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + reg := catalog.NewRunnerRegistry() + _ = reg.Register("micropy", &pluginRunner{}) + eng := catalog.NewEngine(cat, runnerFunc(func(string) error { return nil }), catalog.WithRunners(reg)) + plan, err := eng.BuildPlan([]string{"boot"}) + if err != nil { + t.Fatal(err) + } + if len(plan.Steps) != 2 || plan.Steps[0].Runner != "micropy" { + t.Fatalf("steps=%+v", plan.Steps) + } +} + +// An unknown runner is still caught before the args question is asked. +func TestBuildPlan_unknownRunnerBeatsUnexpectedArgs(t *testing.T) { + src := "version: \"0.1\"\nscripts:\n # @runner nope\n t: echo hi\n" + cat, err := catalog.Parse([]byte(src), "x") + if err != nil { + t.Fatal(err) + } + eng := catalog.NewEngine(cat, runnerFunc(func(string) error { return nil })) + _, err = eng.BuildPlan([]string{"t", "extra"}) + if !errors.Is(err, catalog.ErrUnknownRunner) { + t.Fatalf("err=%v", err) + } +} diff --git a/internal/catalog/errors.go b/internal/catalog/errors.go index 4e52f15..1bfbf2f 100644 --- a/internal/catalog/errors.go +++ b/internal/catalog/errors.go @@ -13,6 +13,7 @@ var ( ErrDependencyCycle = errors.New("dependency cycle") ErrInvalidCatalog = errors.New("invalid catalog") ErrInvalidCapture = errors.New("invalid capture name") + ErrUnknownRunner = errors.New("unknown runner") ) // ExitError carries a process exit code from a failed command. diff --git a/internal/catalog/load.go b/internal/catalog/load.go index 5bfa5ba..a9173bc 100644 --- a/internal/catalog/load.go +++ b/internal/catalog/load.go @@ -42,6 +42,7 @@ func Parse(data []byte, path string, dialects ...*DialectRegistry) (*Catalog, er } cat := &Catalog{Path: path} + var legacyDialect DialectName var scriptsNode *yaml.Node for i := 0; i < len(doc.Content); i += 2 { key := doc.Content[i] @@ -50,7 +51,13 @@ func Parse(data []byte, path string, dialects ...*DialectRegistry) (*Catalog, er case "version": cat.Version = strings.TrimSpace(scalarString(val)) case "dialect": - cat.Dialect = DialectName(strings.TrimSpace(scalarString(val))) + // Legacy position, kept because it shipped in 0.1 and 0.2. + // engine.dialect is the current one and wins. + legacyDialect = DialectName(strings.TrimSpace(scalarString(val))) + case "engine": + if err := val.Decode(&cat.Engine); err != nil { + return nil, fmt.Errorf("%w: engine: %v", ErrInvalidCatalog, err) + } case "scripts": scriptsNode = val } @@ -58,9 +65,26 @@ func Parse(data []byte, path string, dialects ...*DialectRegistry) (*Catalog, er if cat.Version == "" { return nil, fmt.Errorf("%w: version is required", ErrInvalidCatalog) } + if err := validateEngine(cat.Engine); err != nil { + return nil, fmt.Errorf("%w: %v", ErrInvalidCatalog, err) + } + cat.Dialect = cat.Engine.Dialect + if cat.Dialect == "" { + cat.Dialect = legacyDialect + } + cat.Runner = cat.Engine.Runner if cat.Dialect == "" { cat.Dialect = DialectPackage } + // An unset runner stays unset: it means "whatever this engine runs with", + // which is the Runner injected into it. Naming a default here would bake a + // choice that belongs to the caller — godo the CLI runs your own shell, + // while an embedder runs what it wired up. + // + // Runner names are not validated here either. A dialect has to resolve + // before a script can be matched at all, so an unknown one is a load error; + // a runner is only needed to execute, and the registry that could answer + // for it belongs to the Engine. An unknown runner fails at plan time. if _, err := reg.Lookup(cat.Dialect); err != nil { return nil, fmt.Errorf("%w: dialect %q not implemented", ErrInvalidCatalog, cat.Dialect) } @@ -90,13 +114,14 @@ func Parse(data []byte, path string, dialects ...*DialectRegistry) (*Catalog, er func scriptFromNodes(key, val *yaml.Node, fileDialect DialectName, reg *DialectRegistry) (Script, error) { s := Script{Key: key.Value} - doc, deps, dialect, err := parseDecorators(key.HeadComment) + dec, err := parseDecorators(key.HeadComment) if err != nil { return Script{}, fmt.Errorf("%w: script %q: %v", ErrInvalidCatalog, s.Key, err) } - s.Doc = doc - s.Deps = deps - s.Dialect = dialect + s.Doc = dec.doc + s.Deps = dec.deps + s.Dialect = dec.dialect + s.Runner = dec.runner if s.Dialect != "" { if _, err := reg.Lookup(DialectName(s.Dialect)); err != nil { return Script{}, fmt.Errorf("%w: script %q: @dialect %q not implemented", ErrInvalidCatalog, s.Key, s.Dialect) @@ -153,9 +178,18 @@ func validateMatcherKey(key string) error { return nil } -func parseDecorators(headComment string) (doc string, deps []string, dialect string, err error) { +// decorators is the parsed @-block above a script key. +type decorators struct { + doc string + deps []string + dialect string + runner string +} + +func parseDecorators(headComment string) (decorators, error) { + var dec decorators if headComment == "" { - return "", nil, "", nil + return dec, nil } var docLines []string for _, line := range strings.Split(headComment, "\n") { @@ -173,20 +207,26 @@ func parseDecorators(headComment string) (doc string, deps []string, dialect str switch fields[0] { case "@deps", "@dependencies": rest := strings.TrimSpace(strings.TrimPrefix(line, fields[0])) - deps = append(deps, splitDepsList(rest)...) + dec.deps = append(dec.deps, splitDepsList(rest)...) case "@dialect": if len(fields) < 2 || fields[1] == "" { - return "", nil, "", fmt.Errorf("@dialect requires a name") + return decorators{}, fmt.Errorf("@dialect requires a name") + } + dec.dialect = fields[1] + case "@runner": + if len(fields) < 2 || fields[1] == "" { + return decorators{}, fmt.Errorf("@runner requires a name") } - dialect = fields[1] + dec.runner = fields[1] default: - return "", nil, "", fmt.Errorf("unknown decorator %s", fields[0]) + return decorators{}, fmt.Errorf("unknown decorator %s", fields[0]) } continue } docLines = append(docLines, line) } - return strings.Join(docLines, "\n"), deps, dialect, nil + dec.doc = strings.Join(docLines, "\n") + return dec, nil } func splitDepsList(s string) []string { diff --git a/internal/catalog/runner.go b/internal/catalog/runner.go new file mode 100644 index 0000000..756a6cc --- /dev/null +++ b/internal/catalog/runner.go @@ -0,0 +1,102 @@ +package catalog + +import "fmt" + +// RunnerName identifies an execution strategy. +// +// Dialect answers "which script responds to these tokens"; runner answers +// "how does the resolved body become a process". The two are independent: a +// script may be matched by any dialect and run by any runner. +type RunnerName string + +const ( + // RunnerInherit runs the line through the shell the caller is already in. + // + // The CLI injects it as the default: the shell someone is using is their + // own business, and godo proxying to a different one was a choice that is + // not godo's to make. + // + // It is deliberately absent from DefaultRunners: internal/execshell imports + // this package, so it cannot be a default registry entry without an import + // cycle. An unset runner resolves to the Runner injected into the Engine, + // which is also what keeps NewEngine(cat, runner) working unchanged. + RunnerInherit RunnerName = "inherit" +) + +// RunnerRegistry maps runner names to implementations (Open/Closed). +// +// Unlike Dialect, a Runner carries per-invocation state (working directory, +// stdio), so the name is supplied at registration rather than being a method +// on the type: one implementation can be registered twice with different +// configuration. +type RunnerRegistry struct { + byName map[RunnerName]Runner + + // Resolve answers for names that were not registered ahead of time. + // + // It is how "# @runner bash" works without godo keeping a list of every + // shell anyone might have: the name is looked up when a catalog asks for + // it. godo does not manage those shells — it proxies to them. + Resolve func(RunnerName) (Runner, error) +} + +// NewRunnerRegistry returns an empty registry. +func NewRunnerRegistry() *RunnerRegistry { + return &RunnerRegistry{byName: make(map[RunnerName]Runner)} +} + +// DefaultRunners returns the registry a stock engine starts with. +// +// It is empty: an unset runner is the injected Runner. The function exists so +// engine wiring reads the same for both axes. +func DefaultRunners() *RunnerRegistry { return NewRunnerRegistry() } + +// Register adds a runner under name. +func (r *RunnerRegistry) Register(name RunnerName, run Runner) error { + if r.byName == nil { + r.byName = make(map[RunnerName]Runner) + } + if name == "" { + return fmt.Errorf("empty runner name") + } + if run == nil { + return fmt.Errorf("nil runner for %q", name) + } + r.byName[name] = run + return nil +} + +// Lookup returns a runner by name. +func (r *RunnerRegistry) Lookup(name RunnerName) (Runner, error) { + if r == nil { + return nil, fmt.Errorf("%w: %q", ErrUnknownRunner, name) + } + if r.byName == nil { + r.byName = make(map[RunnerName]Runner) + } + if run, ok := r.byName[name]; ok { + return run, nil + } + if r.Resolve == nil { + return nil, fmt.Errorf("%w: %q", ErrUnknownRunner, name) + } + run, err := r.Resolve(name) + if err != nil { + return nil, err + } + // Memoised: Lookup is called more than once per step (validate, then run). + r.byName[name] = run + return run, nil +} + +// EffectiveRunner resolves the @runner override or the file default. +// +// Empty means "the Runner this engine was given". godo the CLI injects the +// caller's own shell there, so a catalog that names nothing runs under the +// shell you are in; an embedder injects whatever it wants and gets that. +func EffectiveRunner(script Script, fileDefault RunnerName) RunnerName { + if script.Runner != "" { + return RunnerName(script.Runner) + } + return fileDefault +} diff --git a/internal/catalog/types.go b/internal/catalog/types.go index 98f4943..ccad344 100644 --- a/internal/catalog/types.go +++ b/internal/catalog/types.go @@ -21,13 +21,16 @@ type Script struct { Doc string // joined non-@ comment lines Deps []string // @deps / @dependencies entries (raw, before expand) Dialect string // @dialect override; empty → file dialect + Runner string // @runner override; empty → file runner } // Catalog is a loaded godo.yaml. type Catalog struct { Path string Version string + Engine EngineSpec Dialect DialectName + Runner RunnerName Scripts []Script // definition order } diff --git a/pkg.go b/pkg.go index d008cc0..d4a649d 100644 --- a/pkg.go +++ b/pkg.go @@ -20,13 +20,18 @@ const FileName = catalog.FileName type ( DialectName = catalog.DialectName + RunnerName = catalog.RunnerName + RunnerRegistry = catalog.RunnerRegistry Script = catalog.Script Catalog = catalog.Catalog + EngineSpec = catalog.EngineSpec + Plugin = catalog.Plugin Match = catalog.Match ExitError = catalog.ExitError Dialect = catalog.Dialect DialectRegistry = catalog.DialectRegistry Runner = catalog.Runner + ArgsAwareRunner = catalog.ArgsAwareRunner Engine = catalog.Engine EngineOption = catalog.EngineOption Plan = catalog.Plan @@ -36,6 +41,8 @@ type ( ) const ( + RunnerInherit = catalog.RunnerInherit + DialectPackage = catalog.DialectPackage DialectMatcher = catalog.DialectMatcher DialectNscript = catalog.DialectNscript @@ -49,6 +56,7 @@ var ( ErrDependencyCycle = catalog.ErrDependencyCycle ErrInvalidCatalog = catalog.ErrInvalidCatalog ErrInvalidCapture = catalog.ErrInvalidCapture + ErrUnknownRunner = catalog.ErrUnknownRunner ) var ( @@ -58,8 +66,12 @@ var ( NewDialectRegistry = catalog.NewDialectRegistry DefaultDialects = catalog.DefaultDialects EffectiveDialect = catalog.EffectiveDialect + NewRunnerRegistry = catalog.NewRunnerRegistry + DefaultRunners = catalog.DefaultRunners + EffectiveRunner = catalog.EffectiveRunner NewEngine = catalog.NewEngine WithDialects = catalog.WithDialects + WithRunners = catalog.WithRunners ExitCode = catalog.ExitCode IsCaptureName = catalog.IsCaptureName ) From 09ca69fa975b1aac21b6e0be3c848d4bed01c589 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=83=9E=E3=82=A4=E3=83=8E=E3=83=AB?= <97069334+MY-RV@users.noreply.github.com> Date: Sun, 20 Sep 2026 15:42:05 -0600 Subject: [PATCH 2/3] feat!: run scripts in the shell you are in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit godo always used sh -c / cmd /C, so a zsh, fish or PowerShell user ran their catalog under a shell they did not pick — and the divergence was silent rather than an error: 'arr=(a b c); echo ${arr[1]}' prints a in zsh and b in sh, because zsh indexes arrays from 1. The shell someone uses is their own business; godo proxying to a different one was a choice that was not godo's to make. The default is now the caller's own shell, and a catalog can name one outright (# @runner bash) when it wants the same shell for everyone. godo does not manage those shells: it resolves the name on PATH and hands the line over. The name is logical, never a path — cmd, not cmd.exe; pwsh, not ps1 — so the same godo.yaml reads the same everywhere. Names are checked against a list of known shells, because '# @runner git' would otherwise become 'git -c ' and fail in a way nobody could read. Consequence, and it is deliberate: a catalog is read by the shell of whoever runs it, so zsh syntax behaves differently for a teammate on bash. godo is a proxy and promises neither cross-OS nor cross-shell. Windows reads the parent process rather than the environment. PowerShell sets PSModulePath and everything it starts inherits it, so a cmd.exe opened from PowerShell would look like PowerShell — godo would have run PowerShell syntax under cmd, silently, which is the exact failure this removes. A parent that is not a shell falls through to %ComSpec%. It cross-compiles and vets for windows/amd64 but is unverified on a real Windows host, which is why GODO_SHELL wins over all of it. engine.version is enforced here too: a catalog needing a newer binary says so once and clearly, instead of failing later in whatever way the missing feature breaks. And godo -e runners prints what this machine actually has, what the catalog declares from plugins, and — since no command run inside a shell can report which shell started godo — how to check that in your own terminal. The old sh/cmd runner is removed rather than kept as a third name: it sat between 'the shell I am in' and 'this exact shell', and '# @runner sh' says it better through the same path as every other shell. --- internal/cli/app.go | 141 ++++++++++++++++++++- internal/cli/e2e_contract_test.go | 164 +++++++++++++++++++++++++ internal/execshell/inherit.go | 176 +++++++++++++++++++++++++++ internal/execshell/inherit_test.go | 140 +++++++++++++++++++++ internal/execshell/parent_other.go | 7 ++ internal/execshell/parent_windows.go | 52 ++++++++ internal/execshell/runner.go | 56 --------- internal/execshell/runner_test.go | 51 -------- 8 files changed, 678 insertions(+), 109 deletions(-) create mode 100644 internal/execshell/inherit.go create mode 100644 internal/execshell/inherit_test.go create mode 100644 internal/execshell/parent_other.go create mode 100644 internal/execshell/parent_windows.go delete mode 100644 internal/execshell/runner.go delete mode 100644 internal/execshell/runner_test.go diff --git a/internal/cli/app.go b/internal/cli/app.go index 8b2684f..cc8ac18 100644 --- a/internal/cli/app.go +++ b/internal/cli/app.go @@ -4,7 +4,9 @@ import ( "fmt" "io" "os" + "os/exec" "path/filepath" + "runtime" "strings" "github.com/my-rv/godo" @@ -56,6 +58,9 @@ func (a *App) Run(args []string) error { fmt.Fprintln(a.Stdout, godo.Version) return nil } + if mode == modeRunners { + return a.listRunners(a.nearestCatalog()) + } if mode == modeUpdate || mode == modeUpdateCheck { return a.runUpdate(mode == modeUpdateCheck) } @@ -72,11 +77,32 @@ func (a *App) Run(args []string) error { if err != nil { return err } + if err := requireEngineVersion(cat, godo.Version); err != nil { + return err + } + root := filepath.Dir(path) + // The default is the shell you are in. A catalog that names no runner gets + // this one, which is why godo stops running zsh users under sh. runner := a.Runner if runner == nil { - runner = execshell.Runner{Dir: filepath.Dir(path)} + runner = execshell.InheritRunner{Dir: root} } - eng := catalog.NewEngine(cat, runner) + // Registered here rather than in internal/catalog: internal/execshell + // imports it, so the engine cannot hold it without an import cycle. + runners := catalog.NewRunnerRegistry() + if err := runners.Register(catalog.RunnerInherit, execshell.InheritRunner{Dir: root}); err != nil { + return err + } + // Any other name is a shell the catalog asked for by name. godo does not + // manage those — it proxies to whatever is on PATH. + runners.Resolve = func(name catalog.RunnerName) (catalog.Runner, error) { + r, err := execshell.NativeShell(string(name), root) + if err != nil { + return nil, fmt.Errorf("%w: %v", catalog.ErrUnknownRunner, err) + } + return r, nil + } + eng := catalog.NewEngine(cat, runner, catalog.WithRunners(runners)) switch mode { case modeList: @@ -95,6 +121,27 @@ func (a *App) Run(args []string) error { } } +// requireEngineVersion enforces engine.version against the running binary. +// +// A catalog using something a older godo does not have should say so once, +// clearly, instead of failing later in whatever way that feature happens to +// break. A -dev build compares by its numeric part, so working on godo itself +// is not blocked by its own catalog. +func requireEngineVersion(cat *catalog.Catalog, binary string) error { + want, ok := cat.Engine.Minimum() + if !ok { + return nil + } + older, err := update.Newer(binary, want) + if err != nil { + return fmt.Errorf("engine.version: %w", err) + } + if older { + return fmt.Errorf("%s needs godo %s or newer; this is %s (godo -e update)", cat.Path, want, binary) + } + return nil +} + func (a *App) runUpdate(checkOnly bool) error { client := a.UpdateClient if client == nil { @@ -140,6 +187,84 @@ func (a *App) runUpdate(checkOnly bool) error { return nil } +// listRunners prints what a catalog may put in runner: / # @runner here. +// +// "here" is the point: the native shells are whatever this machine has, so the +// list is a fact about the machine, not about godo. +// nearestCatalog loads the catalog for -e runners, or nil. +// +// The listing is useful outside a repo, so a missing or broken catalog is not +// an error here — it only means there is nothing extra to say about plugins. +func (a *App) nearestCatalog() *catalog.Catalog { + cwd, err := a.cwd() + if err != nil { + return nil + } + path, err := catalog.FindFile(cwd) + if err != nil { + return nil + } + cat, err := catalog.LoadFile(path) + if err != nil { + return nil + } + return cat +} + +func (a *App) listRunners(cat *catalog.Catalog) error { + fmt.Fprintf(a.Stdout, "%-10s %s [default]\n", string(catalog.RunnerInherit), execshell.DetectShell()) + fmt.Fprintln(a.Stdout) + fmt.Fprintln(a.Stdout, "shells found here:") + found := false + for _, name := range execshell.KnownShells() { + path, err := exec.LookPath(name) + if err != nil { + continue + } + found = true + fmt.Fprintf(a.Stdout, " %-10s %s\n", name, path) + } + if !found { + fmt.Fprintln(a.Stdout, " (none)") + } + if cat != nil { + a.printDeclaredPlugins(cat) + } + a.printShellCheck() + return nil +} + +// printDeclaredPlugins names runners a catalog expects from plugins, so the +// listing does not read as though those names simply do not exist. +func (a *App) printDeclaredPlugins(cat *catalog.Catalog) { + if len(cat.Engine.Plugins) == 0 { + return + } + fmt.Fprintln(a.Stdout, "\ndeclared by this catalog (no plugin loader in this build):") + for _, p := range cat.Engine.Plugins { + fmt.Fprintf(a.Stdout, " %-10s %s\n", strings.Join(p.Provides, " "), p.Source) + } +} + +// printShellCheck tells the reader how to confirm which shell they are in. +// +// godo answers that from the parent process, which no command run inside a +// shell can report — running one would only describe the shell godo just +// started. So when the detected shell looks wrong, the check has to happen in +// the reader's own terminal, and this says how. +func (a *App) printShellCheck() { + fmt.Fprintln(a.Stdout, "\nNot the shell you expected? Run this in your terminal:") + if runtime.GOOS == "windows" { + fmt.Fprintln(a.Stdout, " PowerShell $PSVersionTable.PSVersion") + fmt.Fprintln(a.Stdout, " cmd echo %COMSPEC%") + fmt.Fprintln(a.Stdout, "\nThen: set GODO_SHELL=C:\\path\\to\\shell.exe") + return + } + fmt.Fprintln(a.Stdout, " echo $0 (sh, bash, zsh, dash, ksh)") + fmt.Fprintln(a.Stdout, " echo $version (fish)") + fmt.Fprintln(a.Stdout, "\nThen: GODO_SHELL=/path/to/shell") +} + func (a *App) list(eng *catalog.Engine, tokens []string) error { if len(tokens) == 0 { for _, s := range eng.ListAll() { @@ -162,6 +287,9 @@ func (a *App) list(eng *catalog.Engine, tokens []string) error { if s.Dialect != "" { fmt.Fprintf(a.Stdout, "@dialect %s\n", s.Dialect) } + if s.Runner != "" { + fmt.Fprintf(a.Stdout, "@runner %s\n", s.Runner) + } for _, d := range s.Deps { fmt.Fprintf(a.Stdout, "@deps %s\n", d) } @@ -190,6 +318,7 @@ const ( modePreview modeUpdate modeUpdateCheck + modeRunners ) // ParseFlags parses context flags before script tokens. @@ -241,6 +370,11 @@ func parseEngineCommand(tokens []string) (mode mode, rest []string, err error) { return 0, nil, fmt.Errorf("-e version: unexpected arguments %v", tokens[1:]) } return modeVersion, nil, nil + case "runners": + if len(tokens) > 1 { + return 0, nil, fmt.Errorf("-e runners: unexpected arguments %v", tokens[1:]) + } + return modeRunners, nil, nil case "update": switch { case len(tokens) == 1: @@ -276,10 +410,12 @@ Examples: godo test # script "test" from godo.yaml godo update # script "update" if defined godo -e version # binary version + godo -e runners # runners usable on this machine godo -e update # self-update from GitHub Releases godo -e update check # check only File: godo.yaml (walk-up). Dialects: package | matcher. +Runners: inherit (default) | a shell by name (see -e runners). Env: GODO_RELEASES_API overrides GitHub API base for update.`) } @@ -289,6 +425,7 @@ func EngineUsage(w io.Writer) { Built-in commands (not godo.yaml scripts): version print binary version + runners list runners usable here update download latest GitHub Release for this OS/arch update check report whether an update is available help this help diff --git a/internal/cli/e2e_contract_test.go b/internal/cli/e2e_contract_test.go index 77bd61e..a6ba2b2 100644 --- a/internal/cli/e2e_contract_test.go +++ b/internal/cli/e2e_contract_test.go @@ -2,8 +2,10 @@ package cli_test import ( "bytes" + "errors" "os" "path/filepath" + "runtime" "strings" "testing" @@ -120,3 +122,165 @@ scripts: t.Fatal("expected expand error") } } + +// Contract: "runner" — a catalog may name a native shell directly, and godo +// proxies to whatever is on PATH instead of managing a list of them. +func TestE2E_nativeShellRunnerByName(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("posix shells") + } + cwd := t.TempDir() + // BASH_VERSION only exists in bash, so the marker proves which shell ran. + writeGodoYAML(t, cwd, "version: \"0.1\"\nscripts:\n"+ + " # @runner bash\n"+ + " t: printf %s \"${BASH_VERSION:+is-bash}\" > marker\n") + app, _, _ := e2eApp(t, cwd) + // The injected runner covers "shell"; a named shell must not reach it. + app.Runner = runnerFunc(func(string) error { return errors.New("shell runner must not be used") }) + if err := app.Run([]string{"t"}); err != nil { + t.Fatalf("run: %v", err) + } + got, err := os.ReadFile(filepath.Join(cwd, "marker")) + if err != nil { + t.Fatal(err) + } + if string(got) != "is-bash" { + t.Fatalf("marker=%q — bash did not run the line", got) + } +} + +func TestE2E_unknownShellNameIsRefused(t *testing.T) { + cwd := t.TempDir() + writeGodoYAML(t, cwd, "version: \"0.1\"\nscripts:\n # @runner git\n t: echo hi\n") + app, _, _ := e2eApp(t, cwd) + err := app.Run([]string{"t"}) + if err == nil || !strings.Contains(err.Error(), "not a known shell") { + t.Fatalf("err=%v", err) + } +} + +func TestE2E_engineRunnersLists(t *testing.T) { + app, out, _ := e2eApp(t, t.TempDir()) + if err := app.Run([]string{"-e", "runners"}); err != nil { + t.Fatal(err) + } + got := out.String() + for _, want := range []string{"inherit", "[default]", "shells found here", "GODO_SHELL", "Not the shell you expected"} { + if !strings.Contains(got, want) { + t.Fatalf("missing %q in:\n%s", want, got) + } + } +} + +func TestE2E_engineRunnersRejectsArguments(t *testing.T) { + app, _, _ := e2eApp(t, t.TempDir()) + if err := app.Run([]string{"-e", "runners", "extra"}); err == nil { + t.Fatal("want error") + } +} + +// --ls shows the runner a script asked for, next to its dialect. +func TestE2E_lsShowsRunner(t *testing.T) { + cwd := t.TempDir() + writeGodoYAML(t, cwd, "version: \"0.1\"\nscripts:\n # @runner inherit\n t: echo hi\n") + app, out, _ := e2eApp(t, cwd) + if err := app.Run([]string{"--ls", "t"}); err != nil { + t.Fatal(err) + } + if !strings.Contains(out.String(), "@runner inherit") { + t.Fatalf("ls=%q", out.String()) + } +} + +// Contract: a catalog that names no runner runs under the shell you are in. +func TestE2E_defaultRunnerIsTheCallersShell(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("posix shells") + } + if _, err := os.Stat("/bin/zsh"); err != nil { + t.Skip("no zsh on this machine") + } + t.Setenv("GODO_SHELL", "/bin/zsh") + cwd := t.TempDir() + // $0 is the shell running the line, and every POSIX shell reports it. + // (Arrays would be a sharper example but dash has none, and /bin/sh is + // dash on Debian and bash on macOS.) + writeGodoYAML(t, cwd, "version: \"0.1\"\nscripts:\n"+ + " t: \"printf %s \\\"$0\\\" > marker\"\n") + app := cli.New() + app.Getwd = func() (string, error) { return cwd, nil } + if err := app.Run([]string{"t"}); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(filepath.Join(cwd, "marker")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(got), "zsh") { + t.Fatalf("marker=%q — default did not use the caller's shell", got) + } +} + +// Naming a shell pins it, whatever shell the caller is in. +func TestE2E_namedShellPinsItRegardlessOfCaller(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("posix shells") + } + t.Setenv("GODO_SHELL", "/bin/zsh") + cwd := t.TempDir() + writeGodoYAML(t, cwd, "version: \"0.1\"\nengine:\n runner: sh\nscripts:\n"+ + " t: \"printf %s \\\"$0\\\" > marker\"\n") + app := cli.New() + app.Getwd = func() (string, error) { return cwd, nil } + if err := app.Run([]string{"t"}); err != nil { + t.Fatal(err) + } + got, err := os.ReadFile(filepath.Join(cwd, "marker")) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(got), "zsh") { + t.Fatalf("marker=%q — runner: sh did not pin sh", got) + } +} + +// Contract: "engine" — a catalog that needs a newer binary says so once, +// clearly, instead of failing later in whatever way the missing feature breaks. +func TestE2E_engineVersionGate(t *testing.T) { + cwd := t.TempDir() + writeGodoYAML(t, cwd, "version: \"0.1\"\nengine:\n version: \">=99.0.0\"\nscripts:\n t: echo hi\n") + app, _, _ := e2eApp(t, cwd) + err := app.Run([]string{"t"}) + if err == nil || !strings.Contains(err.Error(), "needs godo 99.0.0 or newer") { + t.Fatalf("err=%v", err) + } +} + +func TestE2E_engineVersionSatisfied(t *testing.T) { + cwd := t.TempDir() + writeGodoYAML(t, cwd, "version: \"0.1\"\nengine:\n version: \">=0.0.1\"\nscripts:\n t: echo hi\n") + app, _, _ := e2eApp(t, cwd) + if err := app.Run([]string{"t"}); err != nil { + t.Fatalf("err=%v", err) + } +} + +// -e runners names what the catalog expects from plugins, so those runners do +// not read as though they simply did not exist. +func TestE2E_runnersListsDeclaredPlugins(t *testing.T) { + cwd := t.TempDir() + writeGodoYAML(t, cwd, "version: \"0.1\"\nengine:\n plugins:\n"+ + " - source: https://example.test/godo-micropy@v1\n"+ + " sha256: deadbeef\n"+ + " provides: [runner:micropy]\n"+ + "scripts:\n t: echo hi\n") + app, out, _ := e2eApp(t, cwd) + if err := app.Run([]string{"-e", "runners"}); err != nil { + t.Fatal(err) + } + for _, want := range []string{"declared by this catalog", "runner:micropy", "godo-micropy"} { + if !strings.Contains(out.String(), want) { + t.Fatalf("missing %q in:\n%s", want, out.String()) + } + } +} diff --git a/internal/execshell/inherit.go b/internal/execshell/inherit.go new file mode 100644 index 0000000..feb4ec5 --- /dev/null +++ b/internal/execshell/inherit.go @@ -0,0 +1,176 @@ +package execshell + +import ( + "fmt" + "os" + "os/exec" + "runtime" + "strings" + + "github.com/my-rv/godo/internal/catalog" +) + +// InheritRunner runs command lines through the shell the caller is already in. +// +// godo used to always use sh -c / cmd /C, which meant a zsh, fish or PowerShell +// user ran their catalog under a shell they did not choose — and the divergence +// was silent, not an error: "arr=(a b c); echo ${arr[1]}" prints a in zsh and b +// in sh, because zsh indexes arrays from 1. +// +// This does not make anything portable and is not meant to: if you are in +// PowerShell you write PowerShell. It makes godo honest about who runs the line. +// +// What it does not give you is your shell's *configuration*. Like sh -c, the +// shell is started non-interactively and does not read your rc file, so your +// aliases and functions are not there. It is your shell's grammar, not your +// shell's setup. +type InheritRunner struct { + Dir string + Stdout *os.File + Stderr *os.File + Stdin *os.File + + // Shell overrides detection. Empty → detect (see DetectShell). + Shell string +} + +// Run implements catalog.Runner. +func (r InheritRunner) Run(command string) error { + shell := r.Shell + if shell == "" { + shell = DetectShell() + } + name, args := shellCommand(shell, command) + cmd := exec.Command(name, args...) + if r.Dir != "" { + cmd.Dir = r.Dir + } + // Avoid typed-nil *os.File in io.Writer: interface != nil but Write fails. + if r.Stdout != nil { + cmd.Stdout = r.Stdout + } else { + cmd.Stdout = os.Stdout + } + if r.Stderr != nil { + cmd.Stderr = r.Stderr + } else { + cmd.Stderr = os.Stderr + } + if r.Stdin != nil { + cmd.Stdin = r.Stdin + } else { + cmd.Stdin = os.Stdin + } + err := cmd.Run() + if err == nil { + return nil + } + if ee, ok := err.(*exec.ExitError); ok { + return &catalog.ExitError{Code: ee.ExitCode(), Message: fmt.Sprintf("command failed: %s", command)} + } + return err +} + +// DetectShell returns the shell to run a catalog line with. +// +// GODO_SHELL wins, always. Otherwise $SHELL on Unix; on Windows, the parent +// process when it is a shell, else %ComSpec%. +// +// Windows gets the parent process rather than an environment variable because +// the environment cannot answer: PowerShell sets PSModulePath and everything it +// starts inherits it, so a cmd.exe opened from PowerShell looks like +// PowerShell. A parent that is not a shell — a build tool, an editor, CI — +// falls through to %ComSpec%, which is also what a user who disagrees with any +// of this overrides with GODO_SHELL. +// +// $SHELL on Unix is the login shell, not necessarily the one running right +// now: bash started inside zsh still reports zsh. That is the convention every +// other tool follows, and GODO_SHELL is the way to disagree with it too. +func DetectShell() string { + if s := strings.TrimSpace(os.Getenv("GODO_SHELL")); s != "" { + return s + } + if runtime.GOOS == "windows" { + if s := callerShell(); s != "" { + return s + } + if c := strings.TrimSpace(os.Getenv("ComSpec")); c != "" { + return c + } + return "cmd.exe" + } + if s := strings.TrimSpace(os.Getenv("SHELL")); s != "" { + return s + } + return "/bin/sh" +} + +// shellBase is the shell's name, lowercased and without a .exe suffix. +// +// filepath.Base is separator-aware per platform, and this has to recognise a +// Windows path even when the detection runs somewhere else (a test, a catalog +// pinning GODO_SHELL), so it splits on both separators itself. +func shellBase(shell string) string { + s := shell + if i := strings.LastIndexAny(s, `/\`); i >= 0 { + s = s[i+1:] + } + return strings.TrimSuffix(strings.ToLower(s), ".exe") +} + +// shellCommand returns the process and arguments that make shell run one +// command line, non-interactively and without reading a profile. +func shellCommand(shell, command string) (string, []string) { + switch shellBase(shell) { + case "cmd", "command": + return shell, []string{"/C", command} + case "powershell", "pwsh": + // -NoProfile matches what -c does on Unix: the rc file is not read. + return shell, []string{"-NoProfile", "-Command", command} + default: + // sh, bash, zsh, dash, ksh, fish, and anything else that follows the + // convention. -c is close to universal; a shell that does not take it + // is the case GODO_SHELL exists for. + return shell, []string{"-c", command} + } +} + +// KnownShells are the shell names a catalog may name directly with +// "# @runner ". +// +// It is a list rather than "anything on PATH" because a runner name is not an +// arbitrary command: "# @runner git" would otherwise run "git -c " and +// fail in a way nobody could read. A shell that is not here is reachable with +// GODO_SHELL and the default runner. +func KnownShells() []string { + return []string{ + "sh", "bash", "zsh", "dash", "ksh", "ash", "fish", "nu", + "cmd", "pwsh", "powershell", + } +} + +// NativeShell returns a runner for a shell named directly by a catalog. +// +// The name is logical, never a path: a catalog says "cmd" or "pwsh", not +// "cmd.exe" or a drive letter, so the same godo.yaml reads the same on every +// machine. The extension, if the platform wants one, is PATH's business. +func NativeShell(name, dir string) (InheritRunner, error) { + if !isKnownShell(name) { + return InheritRunner{}, fmt.Errorf("%q is not a known shell (%s); for another one set GODO_SHELL", + name, strings.Join(KnownShells(), ", ")) + } + path, err := exec.LookPath(name) + if err != nil { + return InheritRunner{}, fmt.Errorf("shell %q is not installed here", name) + } + return InheritRunner{Dir: dir, Shell: path}, nil +} + +func isKnownShell(name string) bool { + for _, s := range KnownShells() { + if s == name { + return true + } + } + return false +} diff --git a/internal/execshell/inherit_test.go b/internal/execshell/inherit_test.go new file mode 100644 index 0000000..5960e79 --- /dev/null +++ b/internal/execshell/inherit_test.go @@ -0,0 +1,140 @@ +package execshell + +import ( + "bytes" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/my-rv/godo/internal/catalog" +) + +func TestShellCommand_flagsPerShellFamily(t *testing.T) { + cases := []struct { + shell string + want []string + }{ + {"/bin/sh", []string{"-c"}}, + {"/bin/bash", []string{"-c"}}, + {"/bin/zsh", []string{"-c"}}, + {"/usr/local/bin/fish", []string{"-c"}}, + {"/opt/weird/myshell", []string{"-c"}}, + {`C:\Windows\System32\cmd.exe`, []string{"/C"}}, + {"cmd", []string{"/C"}}, + {"powershell.exe", []string{"-NoProfile", "-Command"}}, + {"pwsh", []string{"-NoProfile", "-Command"}}, + {`C:\Program Files\PowerShell\7\pwsh.exe`, []string{"-NoProfile", "-Command"}}, + } + for _, tc := range cases { + name, args := shellCommand(tc.shell, "echo hi") + if name != tc.shell { + t.Fatalf("%s: name=%q", tc.shell, name) + } + if len(args) != len(tc.want)+1 { + t.Fatalf("%s: args=%q", tc.shell, args) + } + for i, w := range tc.want { + if args[i] != w { + t.Fatalf("%s: args=%q want prefix %q", tc.shell, args, tc.want) + } + } + if args[len(args)-1] != "echo hi" { + t.Fatalf("%s: command not last: %q", tc.shell, args) + } + } +} + +// Detection is a heuristic; GODO_SHELL is the documented way out of it. +func TestDetectShell_godoShellWins(t *testing.T) { + t.Setenv("GODO_SHELL", "/opt/mine/sh") + t.Setenv("SHELL", "/bin/zsh") + if got := DetectShell(); got != "/opt/mine/sh" { + t.Fatalf("got %q", got) + } +} + +func TestDetectShell_usesSHELLOnUnix(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("unix branch") + } + t.Setenv("GODO_SHELL", "") + t.Setenv("SHELL", "/bin/zsh") + if got := DetectShell(); got != "/bin/zsh" { + t.Fatalf("got %q", got) + } + t.Setenv("SHELL", "") + if got := DetectShell(); got != "/bin/sh" { + t.Fatalf("fallback: got %q", got) + } +} + +// The point of the runner, as a test: the same line means different things in +// different shells, and godo must stop picking for you. +func TestInheritRunner_runsUnderTheCallersShell(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("posix shells") + } + if _, err := os.Stat("/bin/zsh"); err != nil { + t.Skip("no zsh on this machine") + } + // $0 is the shell running the line. Arrays would be a sharper example but + // dash has none, and /bin/sh is dash on Debian and bash on macOS. + const line = `printf %s "$0"` + + got := captureRun(t, InheritRunner{Shell: "/bin/zsh"}, line) + if !strings.Contains(got, "zsh") { + t.Fatalf("zsh ran the line but $0 was %q", got) + } + got = captureRun(t, InheritRunner{Shell: "/bin/sh"}, line) + if strings.Contains(got, "zsh") { + t.Fatalf("sh ran the line but $0 was %q", got) + } +} + +func TestInheritRunner_propagatesTheChildExitCode(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("posix shells") + } + err := InheritRunner{Shell: "/bin/sh"}.Run("exit 42") + if err == nil { + t.Fatal("want failure") + } + if got := catalog.ExitCode(err); got != 42 { + t.Fatalf("exit=%d (%v)", got, err) + } +} + +func TestInheritRunner_runsInTheCatalogDirectory(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("posix shells") + } + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "marker"), nil, 0o644); err != nil { + t.Fatal(err) + } + got := captureRun(t, InheritRunner{Dir: dir, Shell: "/bin/sh"}, "ls marker") + if got != "marker" { + t.Fatalf("got %q", got) + } +} + +// captureRun runs one line and returns its trimmed stdout. +func captureRun(t *testing.T, r InheritRunner, line string) string { + t.Helper() + f, err := os.CreateTemp(t.TempDir(), "out") + if err != nil { + t.Fatal(err) + } + defer f.Close() + r.Stdout = f + if err := r.Run(line); err != nil { + t.Fatalf("run %q: %v", line, err) + } + b, err := os.ReadFile(f.Name()) + if err != nil { + t.Fatal(err) + } + return strings.TrimSpace(string(bytes.TrimSpace(b))) +} diff --git a/internal/execshell/parent_other.go b/internal/execshell/parent_other.go new file mode 100644 index 0000000..a8b0426 --- /dev/null +++ b/internal/execshell/parent_other.go @@ -0,0 +1,7 @@ +//go:build !windows + +package execshell + +// callerShell is unused off Windows: $SHELL answers the question there, and +// reading the parent process would need a different syscall on every Unix. +func callerShell() string { return "" } diff --git a/internal/execshell/parent_windows.go b/internal/execshell/parent_windows.go new file mode 100644 index 0000000..43c8457 --- /dev/null +++ b/internal/execshell/parent_windows.go @@ -0,0 +1,52 @@ +//go:build windows + +package execshell + +import ( + "os" + "syscall" + "unsafe" +) + +// callerShell returns the shell that started this process, or "". +// +// Windows has no $SHELL, and the environment cannot answer the question: +// PSModulePath is set by PowerShell but inherited by everything it starts, so +// a cmd.exe opened from PowerShell looks exactly like PowerShell. The parent +// process is the only honest answer, so that is what this reads. +// +// Empty when the parent is not a shell — a build tool, an editor, a CI runner — +// which is the common case and why the caller falls back to %ComSpec%. +func callerShell() string { + name, err := processName(uint32(os.Getppid())) + if err != nil || name == "" { + return "" + } + if !isKnownShell(shellBase(name)) { + return "" + } + return name +} + +// processName returns the executable name of pid via the process snapshot. +func processName(pid uint32) (string, error) { + snap, err := syscall.CreateToolhelp32Snapshot(syscall.TH32CS_SNAPPROCESS, 0) + if err != nil { + return "", err + } + defer syscall.CloseHandle(snap) + + var e syscall.ProcessEntry32 + e.Size = uint32(unsafe.Sizeof(e)) + if err := syscall.Process32First(snap, &e); err != nil { + return "", err + } + for { + if e.ProcessID == pid { + return syscall.UTF16ToString(e.ExeFile[:]), nil + } + if err := syscall.Process32Next(snap, &e); err != nil { + return "", err + } + } +} diff --git a/internal/execshell/runner.go b/internal/execshell/runner.go deleted file mode 100644 index 07009aa..0000000 --- a/internal/execshell/runner.go +++ /dev/null @@ -1,56 +0,0 @@ -package execshell - -import ( - "fmt" - "os" - "os/exec" - "runtime" - - "github.com/my-rv/godo/internal/catalog" -) - -// Runner runs command lines via the system shell. -type Runner struct { - Dir string - Stdout *os.File - Stderr *os.File - Stdin *os.File -} - -// Run implements catalog.Runner. -func (r Runner) Run(command string) error { - var cmd *exec.Cmd - if runtime.GOOS == "windows" { - cmd = exec.Command("cmd", "/C", command) - } else { - cmd = exec.Command("sh", "-c", command) - } - if r.Dir != "" { - cmd.Dir = r.Dir - } - // Avoid typed-nil *os.File in io.Writer: interface != nil but Write panics/fails. - if r.Stdout != nil { - cmd.Stdout = r.Stdout - } else { - cmd.Stdout = os.Stdout - } - if r.Stderr != nil { - cmd.Stderr = r.Stderr - } else { - cmd.Stderr = os.Stderr - } - if r.Stdin != nil { - cmd.Stdin = r.Stdin - } else { - cmd.Stdin = os.Stdin - } - err := cmd.Run() - if err == nil { - return nil - } - if ee, ok := err.(*exec.ExitError); ok { - code := ee.ExitCode() - return &catalog.ExitError{Code: code, Message: fmt.Sprintf("command failed: %s", command)} - } - return err -} diff --git a/internal/execshell/runner_test.go b/internal/execshell/runner_test.go deleted file mode 100644 index 4f879db..0000000 --- a/internal/execshell/runner_test.go +++ /dev/null @@ -1,51 +0,0 @@ -package execshell_test - -import ( - "errors" - "os" - "path/filepath" - "runtime" - "testing" - - "github.com/my-rv/godo/internal/catalog" - "github.com/my-rv/godo/internal/execshell" -) - -func TestRunner_exitCode(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("shell exit differs") - } - dir := t.TempDir() - r := execshell.Runner{Dir: dir, Stdout: os.Stdout, Stderr: os.Stderr} - err := r.Run("exit 42") - var ee *catalog.ExitError - if !errors.As(err, &ee) || ee.Code != 42 { - t.Fatalf("%v", err) - } -} - -func TestRunner_usesDir(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("pwd") - } - dir := t.TempDir() - marker := filepath.Join(dir, "marker") - r := execshell.Runner{Dir: dir} - if err := r.Run("touch marker"); err != nil { - t.Fatal(err) - } - if _, err := os.Stat(marker); err != nil { - t.Fatal(err) - } -} - -func TestRunner_zeroValueStdioEcho(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("sh") - } - // Mirrors CLI: Runner{Dir: ...} with nil File fields (typed-nil trap). - r := execshell.Runner{Dir: t.TempDir()} - if err := r.Run(`echo "Hello, World!"`); err != nil { - t.Fatal(err) - } -} From 5cff8fe41e4010291d34c022b38ef9700c9f0087 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E3=83=9E=E3=82=A4=E3=83=8E=E3=83=AB?= <97069334+MY-RV@users.noreply.github.com> Date: Sun, 20 Sep 2026 15:42:05 -0600 Subject: [PATCH 3/3] docs: contract, runners guide, design note, and the v0.3 row The contract gains engine: and the runner axis. The guide gains a runners page in the shape the dialect pages have, with the zsh-vs-bash array example as the thing that makes the default concrete. The design note is premise-first, because the premise is what kept getting lost: godo is a proxy, it does not promise cross-OS, and cross-OS belongs to a plugin rather than to the core. It records what was deliberately not done and why, so the next person proposing an interpreter in the core finds the argument already written down. The roadmap gains the v0.3 row it was missing, marked in progress and held until plugin loading works, since engine: without a loader is half a promise. The repo's own godo.yaml names no runner on purpose, with a comment saying why: its scripts are plain commands, so the caller's shell makes no difference, and the gate dogfoods the new default instead of pinning around it. --- CHANGELOG.md | 73 +++++++++++++ docs/README.md | 1 + docs/contract.md | 188 ++++++++++++++++++++++++++++++-- docs/dev/README.md | 1 + docs/dev/runners-and-plugins.md | 93 ++++++++++++++++ docs/getting-started.md | 2 +- docs/guide/matcher-dialect.md | 3 +- docs/guide/placeholders.md | 6 +- docs/guide/runners.md | 132 ++++++++++++++++++++++ docs/reference/api.md | 12 +- docs/reference/cli.md | 1 + docs/roadmap.md | 32 ++++++ godo.yaml | 5 + 13 files changed, 534 insertions(+), 15 deletions(-) create mode 100644 docs/dev/runners-and-plugins.md create mode 100644 docs/guide/runners.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 333b08a..942771a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,12 +2,73 @@ ## [Unreleased] +Breaking. The shell that runs your scripts changed. + +### Changed +- **`dialect` moved to `engine.dialect`.** A top-level `dialect:` keeps + working — it shipped in 0.1 and 0.2 — and `engine.dialect` wins if both are + present. `runner` only ever existed as `engine.runner`. +- **The default runner is the shell you are already in**, not `sh -c` / + `cmd /C`. A zsh user gets zsh, a PowerShell user gets PowerShell. The shell + someone uses is their own business; godo proxying to a different one was a + choice that was not godo's to make. + *Consequence, and it is deliberate:* a catalog is read by the shell of + whoever runs it, so zsh syntax behaves differently for a teammate on bash. + godo is a proxy and promises neither cross-OS nor cross-shell. To pin one + shell for everyone, name it (`runner: sh`, `runner: bash`). + *Library:* an unset runner means the `Runner` injected into the engine, so + `NewEngine(cat, myRunner)` is unaffected. + +### Added +- **Runner axis.** `{file}.runner` and `# @runner` say *how* a script's body + becomes a process, the way `dialect` says *which* script answers the tokens. + It exists so a plugin's body — which is not a shell line at all — has a way + to say so; see the [design note](./docs/dev/runners-and-plugins.md). +- **`inherit`** (the default): the shell you are in. `GODO_SHELL` overrides + detection; otherwise `$SHELL` on Unix, and on Windows the parent process when + it is a shell, else `%ComSpec%`. Windows reads the parent because the + environment cannot answer — PowerShell sets `PSModulePath` and everything it + starts inherits it, so a `cmd.exe` opened from PowerShell would look like + PowerShell. +- **A shell by name:** `# @runner sh` / `bash` / `zsh` / `dash` / `ksh` / `ash` + / `fish` / `nu` / `cmd` / `pwsh` / `powershell`. godo does not manage these — + it resolves the name on `PATH` and hands the line over. The name is logical, + never a path: `cmd`, not `cmd.exe`; `pwsh`, not `ps1`. A name outside the + list is refused rather than run, so `# @runner git` cannot quietly become + `git -c `. +- **`engine:` block** — every dial godo turns while reading and running a + catalog, kept apart from `scripts:`, which is the data. Holds `version` + (minimum binary, enforced before anything runs), `dialect`, `runner`, and + `plugins`. `engine.plugins` takes `source`, a **required** `sha256`, + `provides: [runner:name]`, and an optional `config` that is entirely the + plugin's — godo carries it without reading it. Nothing loads plugins yet; + they are parsed and validated so the shape is settled, and a script asking + for a runner a plugin provides fails by naming that plugin instead of reading + as a typo. +- **`godo -e runners`** lists what is usable on the machine you are on, and how + to confirm which shell you are in when the detected one looks wrong. +- `--ls ` prints `@runner` beside `@dialect`. +- `ArgsAwareRunner`: whether a script takes the tokens left over after the + match is the runner's question. A shell keeps the `${godo:args…}` rule, + message included; a runner whose body is a program answers for itself. +- Public `RunnerName`, `RunnerInherit`, `RunnerRegistry`, `NewRunnerRegistry`, + `DefaultRunners`, `EffectiveRunner`, `WithRunners`, `ErrUnknownRunner`. + ### Fixed - A value shaped like `--flag=…` is quoted from the `=` onward, so `--preview` shows `git commit --am='two words'` instead of `git commit '--am=two words'`, which read as though the flag name were part of the message. Identical single argument to the shell — rendering only. +### Notes +- An unknown runner fails when the plan is built (`--preview` included), not at + load: a dialect must resolve before a script can be matched at all, a runner + only to execute. Nothing executes either way. +- A shell is started non-interactively and without a profile, so you get your + shell's grammar, not your shell's setup — aliases and functions are not there. +- The Windows detection cross-compiles and vets but is unverified on a real + Windows host. + ## [0.2.0] — 2026-09-19 Breaking. Placeholder syntax inside script bodies changed; catalogs need editing. @@ -44,6 +105,12 @@ Full rules in [docs/contract.md](./docs/contract.md). body. Both existed only to rescue host environment variables from being read as captures. +### Fixed +- A value shaped like `--flag=…` is quoted from the `=` onward, so `--preview` + shows `git commit --am='two words'` instead of `git commit '--am=two words'`, + which read as though the flag name were part of the message. Identical single + argument to the shell — rendering only. + ### Notes - The Windows quoting path is **untested**: CI runs Linux only, and the runner tests skip on Windows. `cmd.exe` also expands `%VAR%` before a command sees @@ -62,6 +129,12 @@ Full rules in [docs/contract.md](./docs/contract.md). - Product docs (overview, getting started, guides, reference, distribution, roadmap). - Origin story (EN/ES), SECURITY.md, CONTRIBUTING, GitHub issue/PR templates, CODEOWNERS. +### Fixed +- A value shaped like `--flag=…` is quoted from the `=` onward, so `--preview` + shows `git commit --am='two words'` instead of `git commit '--am=two words'`, + which read as though the flag name were part of the message. Identical single + argument to the shell — rendering only. + ### Notes - Product display name: **GoDo**; identifiers remain lowercase `godo`. - Pre-1.0: APIs and CLI may still change. Treat `v0.x` as evolving. diff --git a/docs/README.md b/docs/README.md index 6e67a58..795aded 100644 --- a/docs/README.md +++ b/docs/README.md @@ -25,6 +25,7 @@ One topic per file (copy-paste examples): | [Preview and ls](./guide/preview-and-ls.md) | `--preview`, `--ls` | | [Package dialect](./guide/package-dialect.md) | Exact names (default) | | [Matcher dialect](./guide/matcher-dialect.md) | Pattern keys + captures | +| [Runners](./guide/runners.md) | Which shell runs your line; `runner:` / `# @runner` | | [Deps](./guide/deps.md) | `# @deps` | | [Placeholders](./guide/placeholders.md) | `${…}` / `${godo:args…}` | | [Working directory](./guide/working-directory.md) | Catalog root, walk-up | diff --git a/docs/contract.md b/docs/contract.md index 9becfee..4d9ea65 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -56,16 +56,177 @@ version: "0.1" Contract version of the file (not the `godo` binary). +## Engine + +File field: `{file}.engine`. Optional. + +```yaml +engine: + version: ">=0.3.0" + dialect: matcher + runner: bash + plugins: + - source: https://github.com/my-rv/godo-micropy@v1.2.0 + sha256: "661471…" + provides: [runner:micropy] + config: + proc: {exec: true, spawn: false} + fs: {slink: true} +``` + +`scripts:` is the catalog — the data. `engine:` is every dial godo turns while +reading and running it: the binary it expects, the dialect, the runner, the +plugins. Keeping them apart is what lets the toolchain side grow without the +script side growing with it. + +| Field | | +|-------|--| +| `version` | Minimum `godo` binary | +| `dialect` | How keys match tokens. Default `package` | +| `runner` | How a body becomes a process. Default: the shell you are in | +| `plugins` | Declared plugins (nothing loads them yet) | + +### `engine.version` + +The minimum `godo` binary, as `"0.3.0"` or `">=0.3.0"`. Only a minimum — no +ranges, no `^`, no `~`. A binary below it refuses the catalog before running +anything: + +``` +godo: ./godo.yaml needs godo 0.3.0 or newer; this is 0.2.0 (godo -e update) +``` + +Comparison drops any pre-release suffix, so a `-dev` build is judged by its +numbers. + +### `engine.plugins` + +**No build loads plugins yet.** Entries are parsed and validated so the shape +is settled and a catalog can already declare what it expects. + +| Field | | +|-------|--| +| `source` | Required. Where the plugin comes from | +| `sha256` | **Required.** A plugin is third-party code that runs when someone types `godo test`; without a digest there is nothing to verify it is the code that was reviewed | +| `provides` | Required. `":"` entries, kind being `runner` or `dialect`. Two plugins may not provide the same one | +| `config` | Optional, and entirely the plugin's: its keys, its meaning, its defaults. godo carries it across without reading it | + +A script asking for a runner a plugin provides fails by naming that plugin: + +``` +godo: unknown runner: script "wt" asks for runner "micropy", provided by +plugin https://github.com/my-rv/godo-micropy@v1.2.0 — this build cannot load +plugins +``` + +`godo -e runners` lists what a catalog declares, beside what the machine has. + ## Dialect -File field: `{file}.dialect`. Optional; default `package`. +File field: `{file}.engine.dialect`. Optional; default `package`. ```yaml -dialect: package # omit → package +engine: + dialect: package # omit → package ``` File default. Scripts may override with `# @dialect`. +A top-level `dialect:` is still read — it shipped in 0.1 and 0.2 — and +`engine.dialect` wins if both are present. + +## Runner + +File field: `{file}.engine.runner`. Optional; default `inherit`. + +```yaml +engine: + runner: inherit # omit → inherit +``` + +File default. Scripts may override with `# @runner`. + +`dialect` answers *which script responds to these tokens*; `runner` answers +*how the resolved body becomes a process*. They are independent: any dialect +may be paired with any runner. + +| Runner | | +|--------|--| +| `inherit` | The shell you are already in. Default | +| *a shell name* | `sh`, `bash`, `zsh`, `dash`, `ksh`, `ash`, `fish`, `nu`, `cmd`, `pwsh`, `powershell` | + +`godo -e runners` lists what is usable on the machine you are on. + +godo does not manage these shells — it resolves the name on `PATH` and hands +the line over. The line is yours and the shell is yours; godo is the proxy. + +### `inherit` + +The default. Your shell runs your line: + +```yaml + arr: "arr=(a b c); echo ${arr[1]}" +``` + +``` +zsh → a (zsh indexes arrays from 1) +sh → b (sh indexes arrays from 0) +``` + +Selection, in order: + +| | | +|--|--| +| `GODO_SHELL` | Always wins | +| Unix | `$SHELL`, else `/bin/sh` | +| Windows | the parent process when it is a shell, else `%ComSpec%` | + +godo answers "which shell am I in" from the parent process, which is the only +thing that knows. No command run *inside* a shell can report it — it would only +describe the shell godo just started. To confirm it yourself, in your own +terminal: `echo $0` (sh, bash, zsh, dash, ksh), `echo $version` (fish), +`$PSVersionTable.PSVersion` (PowerShell), `echo %COMSPEC%` (cmd). `godo -e +runners` prints these too. + +Windows reads the parent process because the environment cannot answer: +PowerShell sets `PSModulePath` and everything it starts inherits it, so a +`cmd.exe` opened from PowerShell would look like PowerShell. On Unix, `$SHELL` +is the login shell rather than the one running right now — bash started inside +zsh still reports zsh. `GODO_SHELL` is how you disagree with either. + +The shell is started non-interactively and without a profile (`-c`, `/C`, or +`-NoProfile -Command`), so this gives you your shell's **grammar**, not your +shell's **setup** — your aliases and functions are not there. + +**Consequence:** a catalog is read by the shell of whoever runs it, so a script +written in zsh syntax behaves differently for a teammate on bash. That is +deliberate — godo is a proxy and promises neither cross-OS nor cross-shell. + +### A shell by name + +To pin one shell for everyone, name it: + +```yaml + # @runner bash + ci: shopt -s globstar && echo **/*.go +``` + +The name is **logical, never a path**: write `cmd`, not `cmd.exe`; `pwsh`, not +`pwsh.exe` or `ps1` (`.ps1` is a script extension, not the program). The +platform's extension is `PATH`'s business, so the same `godo.yaml` reads the +same everywhere. + +A name outside the list above is refused rather than run — otherwise +`# @runner git` would quietly become `git -c `. For a shell not on the +list, set `GODO_SHELL` and use `inherit`. + +A shell that is not installed here fails when the plan is built, so nothing +executes. + +Whether a script accepts the tokens left over after the match is also the +runner's question. A shell accepts them only when the body references +`${godo:args…}`; a runner whose body is a program answers for itself. + ## Bind Godo expands placeholders **in-process** before `exec` / preview, in two spaces: @@ -91,13 +252,17 @@ no escape syntax. `${godo:argv[i]}` with a number is an error pointing at `${godo:args[i]}`. `${godo:…}` inside a matcher key or a `@deps` entry is rejected. -Expanded values are **shell-quoted** for the host shell (`sh` / `cmd`): one -argument in is one argument out. `:raw` opts a single placeholder out. +Expanded values are **shell-quoted** (`sh` rules on POSIX, `cmd` rules on +Windows): one argument in is one argument out. `:raw` opts a single placeholder +out. A value shaped like `--flag=…` is quoted from the `=` onward, so a preview reads `--am='two words'` rather than `'--am=two words'`. Same single argument to the shell; the flag name is not part of the value. +Those rules also hold for bash, zsh, dash, ksh and fish. PowerShell quotes +differently, so a value containing a backtick or `$` may not survive there. + Windows caveat: `cmd.exe` expands `%VAR%` and `!VAR!` before a command sees its arguments, and no quoting on the command line fully suppresses that. @@ -118,6 +283,7 @@ YAML comment block **immediately above** the script key. Apply to scripts only. |------|--| | `# text` (no `@`) | Doc | | `# @dialect ` | Match this script with another dialect (override of `{file}.dialect`) | +| `# @runner ` | Run this script with another runner (override of `{file}.runner`) | | `# @deps a, b` | Run `a`, then `b`, then the value | | `# @dependencies a, b` | Alias of `@deps` | @@ -134,10 +300,12 @@ test ${MODULE}: go test ./${godo:argv[MODULE]}/... ``` - `@dialect` — per-script override; without it, uses `{file}.dialect` +- `@runner` — per-script override; without it, uses `{file}.runner` - `@deps` / `@dependencies` — invocation like `godo …` (space-separated tokens; entries separated by `,`) - literals and bare `${NAME}` from captures **already bound** by the match (godo space) - no `${godo:…}` in `@deps`; entries resolve to tokens, so a capture holding a space stays one token - order = list order; stop on first failure +- each step keeps **its own** runner: a dep declaring `@runner` runs under that runner, not the caller's - cycle → error (on the **expanded** invocation) - caller args are **not** forwarded to deps - diamond (A→B,C and B→C): C runs **once**, at its first (deepest-first) position @@ -155,7 +323,8 @@ scripts.: string | string[] ```yaml version: "0.1" -dialect: package +engine: + dialect: package scripts: # Unit tests @@ -178,7 +347,7 @@ scripts: - `string` — one command - `string[]` — in order; stop on first failure - `${godo:args}` / `${godo:args[i]}` / `${godo:args[i..j]}` — optional in the value -- without `${godo:args}` → extra tokens error +- without `${godo:args}` → extra tokens error (see **Runner**) - no captures in keys (that is `matcher`) ## Dialect `matcher` @@ -192,7 +361,8 @@ scripts.: string | string[] ```yaml version: "0.1" -dialect: matcher +engine: + dialect: matcher scripts: "${GRP} ${SCR}": go run -C scripts/${GRP}/${SCR} . ${godo:args} @@ -222,7 +392,7 @@ Reserved: cannot be used as `{file}.dialect` / `@dialect` until implemented and ## Common semantics 1. Resolve `godo.yaml` from cwd upward through parents. -2. Read `{file}.version` and `{file}.dialect` (omit dialect → `package`; dialect must be implemented). -3. Match → bind captures → expand `@deps` → run deps → expand value → execute (or `--preview` / `--ls`). +2. Read `{file}.version` and `{file}.engine` (omit `engine.dialect` → `package`, must be implemented; omit `engine.runner` → `inherit`). +3. Match → bind captures → expand `@deps` → run deps → expand value → execute with the step's runner (or `--preview` / `--ls`). 4. Exit code = of the command (or the first failure in a list / deps); catalog/match errors → `1`. 5. Exec cwd = directory of the `godo.yaml` (see **Exec cwd**). diff --git a/docs/dev/README.md b/docs/dev/README.md index 982786c..f30e276 100644 --- a/docs/dev/README.md +++ b/docs/dev/README.md @@ -7,6 +7,7 @@ Internals and contributor conventions. Product docs live in [`docs/`](../README. | [Architecture](./architecture.md) | Facade / internal / CLI | | [Standards](./standards.md) | Coding bar | | [Versioning](./versioning.md) | SemVer + binary vs file `version:` | +| [Runners and plugins](./runners-and-plugins.md) | Premise: godo is a proxy; cross-OS is a plugin's job | | [Archive](./archive/README.md) | Brainstorm notes (historical) | Start with root [`CONTRIBUTING.md`](../../CONTRIBUTING.md). diff --git a/docs/dev/runners-and-plugins.md b/docs/dev/runners-and-plugins.md new file mode 100644 index 0000000..e7f4661 --- /dev/null +++ b/docs/dev/runners-and-plugins.md @@ -0,0 +1,93 @@ +# Runners and plugins — design note + +Not the contract. [`contract.md`](../contract.md) is what we promise; nothing +here is in [`roadmap.md`](../roadmap.md). + +## Premise + +**godo is a proxy.** Same idea as `package.json` scripts: it names the chores +and hands the line to the shell. It does **not** promise to solve cross-OS. If +you are on Linux you write Linux (`&&`); on Windows you write Windows. Your +shell is your responsibility, and a catalog author who wants portability picks +portable commands. + +The first layer is that proxy plus flavored water: dialects, `@deps`, +`--preview`, `--ls`. Native, harmless, no ambition beyond naming things well. + +**Cross-OS is a plugin's job**, not the core's. A plugin like `micropy` — +MicroPython compiled to WASM — carries a body that runs the same everywhere +because it is not a shell line at all. + +## What follows from that + +Two axes, and only one of them was open: + +| Axis | Question | | +|------|----------|--| +| Dialect | which script answers these tokens? | already open | +| Runner | how does the body become a process? | hardcoded to `sh -c` / `cmd /C` | + +A MicroPython body has no shell that can run it. For `micropy` to exist at all, +godo needs a way for a script to say *this one is not a shell line*. That is +the whole reason the runner axis exists — `runner:` / `# @runner` and a registry, +with `shell` as the default. + +It is a socket, not a feature. What was added alongside it is not portability +work either: `shell` hardcoded `sh` / `cmd`, so a zsh or PowerShell user ran +their catalog under a shell they did not pick. `inherit` — now the default — uses +the one they are in, and a catalog can name a shell outright (`# @runner +bash`). Your shell is your responsibility; godo just stops lying about which +one it is, and proxies to whatever `PATH` has. + +A `shell` runner meaning "always `sh` / `cmd`" existed briefly and was removed: +it was a third thing between "your shell" and "this shell", it kept +`internal/execshell` carrying two runners, and almost nobody writes a line that +means the same in `sh` and in `cmd`. `# @runner sh` says it better. + +## Deliberately not done + +Things an earlier draft of this note argued for, dropped because they +contradict the premise: + +- **A built-in `exec` runner** (argv, no shell). Its only real gain was a + Windows `%VAR%` edge case; the quoting added in v0.2.0 already covers the + rest. And it moves the body from the shell's ownership to godo's, which is + how it ended up needing its own quote parser — a mini-shell inside godo, with + rules that are neither `sh`'s nor PowerShell's. Wrong direction. +- **A built-in POSIX interpreter** (`mvdan.cc/sh`). Same reason, plus a + dependency, and it is cross-OS work that belongs in a plugin. +- **A structured `Line` / `Part` expansion.** Only existed to serve `exec`. + +The reserved dialect names `nscript` / `matchns` stay reserved and unplanned. + +## Where a plugin is declared + +`engine:` — kept apart from the script fields on purpose. `dialect:` and +`runner:` say how to read and run the scripts; `engine:` says what godo itself +needs to do it. That line is what lets the toolchain side grow — a lockfile, a +package manager — without the script side growing with it. + +The block parses and validates today; nothing loads from it. `sha256` is +required from the start rather than added later, because a plugin is +third-party code that runs on `godo test` and a digest is the only thing that +says it is the code that was reviewed. + +## What is actually left for plugins + +None of this is built: + +1. Load a `.wasm` at runtime (`wazero`: pure Go, no cgo, one artifact for every + platform, sandboxed by default). +2. Ship MicroPython as that `.wasm`, with the host API from the prototype + (`godo.argv`, `godo.args`, `godo.proc`, `godo.fs`). +3. Fetch and pin it: `sha256` required, no auto-update, capabilities granted + deny-all through the plugin's own `config:`. + +Two constraints worth keeping when that work starts: + +- **Every effect goes through host functions.** Not because sandboxing is + fashionable, but because it is the only way `--preview` keeps working on a + body that is a program: a recording host can show what the script would run + without running it. +- **The plugin cannot police itself.** It declares which host function each + `config:` key unlocks; godo does the gating. diff --git a/docs/getting-started.md b/docs/getting-started.md index 799df6b..dda2674 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -31,7 +31,7 @@ scripts: check: go build -o app . ``` -`version` is required. `dialect` is optional and defaults to `package`. Script docs and `@deps` / `@dialect` are comments immediately above each key — start with [guide/scripts.md](./guide/scripts.md). +`version` is required. `engine.dialect` is optional and defaults to `package`. Script docs and `@deps` / `@dialect` are comments immediately above each key — start with [guide/scripts.md](./guide/scripts.md). ## 3. Run diff --git a/docs/guide/matcher-dialect.md b/docs/guide/matcher-dialect.md index f560803..312bf72 100644 --- a/docs/guide/matcher-dialect.md +++ b/docs/guide/matcher-dialect.md @@ -6,7 +6,8 @@ Keys are token patterns. First matching key (definition order) wins. ```yaml version: "0.1" -dialect: matcher +engine: + dialect: matcher scripts: test ${MODULE}: go test ./${godo:argv[MODULE]}/... diff --git a/docs/guide/placeholders.md b/docs/guide/placeholders.md index bafcdfb..ea99bb1 100644 --- a/docs/guide/placeholders.md +++ b/docs/guide/placeholders.md @@ -13,7 +13,8 @@ Because a body is shell text, godo claims **only** the `${godo:…}` namespace t and leaves every other `${…}` alone. A collision is not resolved — it cannot occur. ```yaml -dialect: matcher +engine: + dialect: matcher scripts: # @deps lint ${MODULE} # godo space @@ -27,7 +28,8 @@ scripts: Consumes a capture bound by the matcher key. ```yaml -dialect: matcher +engine: + dialect: matcher scripts: build ${name}: echo building ${godo:argv[name]} -- ${godo:args} diff --git a/docs/guide/runners.md b/docs/guide/runners.md new file mode 100644 index 0000000..3056711 --- /dev/null +++ b/docs/guide/runners.md @@ -0,0 +1,132 @@ +# Runners + +A **dialect** decides *which* script answers your tokens. A **runner** decides +*how* that script's body becomes a process. They are independent — any dialect +pairs with any runner. + +```yaml +version: "0.1" +engine: + runner: inherit # omit → inherit + +scripts: + test: go test ./... + + # @runner bash + ci: shopt -s globstar && echo **/*.go +``` + +| Runner | | +|--------|--| +| `inherit` | The shell you are already in. **Default** | +| a shell name | `sh`, `bash`, `zsh`, `dash`, `ksh`, `ash`, `fish`, `nu`, `cmd`, `pwsh`, `powershell` | + +godo does not manage these shells. It resolves the name on `PATH` and hands the +line over — the line is yours, the shell is yours, godo is the proxy. + +## `inherit` — your shell + +The default. Whatever shell you are in runs the line: + +```yaml +scripts: + arr: "arr=(a b c); echo ${arr[1]}" +``` + +``` +$ godo arr # from zsh +a +$ godo arr # from bash +b +``` + +Not a bug: zsh indexes arrays from 1, bash from 0. Same line, two shells, two +answers — and that is the point. Before this, godo always used `sh`, so a zsh +user got `b` without being told why. + +**Consequence, and it is deliberate:** a catalog is read by the shell of +whoever runs it. A script written in zsh syntax behaves differently for a +teammate on bash. godo is a proxy and promises neither cross-OS nor +cross-shell. If that matters for a script, name a shell. + +## Naming a shell + +Pins it for everyone, whatever shell they are in: + +```yaml +scripts: + # @runner bash + ci: shopt -s globstar && echo **/*.go + + # @runner pwsh + sign: Get-AuthenticodeSignature .\dist\godo.exe +``` + +The name is **logical, never a path**. Write `cmd`, not `cmd.exe`; `pwsh`, not +`pwsh.exe` or `ps1` — `.ps1` is a script extension, the program is `pwsh`. The +platform's extension is `PATH`'s business, so the same `godo.yaml` reads the +same on every machine. + +A name outside the list is refused rather than run: + +``` +$ godo deploy +godo: script "deploy" asks for runner "git": unknown runner: "git" is not a +known shell (sh, bash, zsh, …); for another one set GODO_SHELL +``` + +Otherwise `# @runner git` would quietly become `git -c `. + +A shell that is not installed fails when the plan is built, so nothing runs. + +## What godo picked, and how to disagree + +``` +$ godo -e runners +inherit /bin/zsh [default] + +shells found here: + sh /bin/sh + bash /bin/bash + zsh /bin/zsh + +Not the shell you expected? Run this in your terminal: + echo $0 (sh, bash, zsh, dash, ksh) + echo $version (fish) + +Then: GODO_SHELL=/path/to/shell +``` + +Selection order: + +| | | +|--|--| +| `GODO_SHELL` | Always wins | +| Unix | `$SHELL`, else `/bin/sh` | +| Windows | the parent process when it is a shell, else `%ComSpec%` | + +Windows reads the parent process because the environment cannot answer: +PowerShell sets `PSModulePath` and everything it starts inherits it, so a +`cmd.exe` opened from PowerShell would look like PowerShell. + +On Unix, `$SHELL` is your *login* shell, not necessarily the one running right +now — bash started inside zsh still reports zsh. Every other tool follows that +convention; `GODO_SHELL` is how you disagree with it. + +## What you do not get + +Your shell's **grammar**, not your shell's **setup**. The shell starts +non-interactively and without a profile (`-c`, `/C`, `-NoProfile -Command`), so +your aliases and functions are not there — exactly as `sh -c` always behaved. + +```yaml +scripts: + # your alias "gs" does not exist here + status: git status +``` + +## Plugins + +`# @runner` is also how a body that is **not a shell line** says so — a plugin +carrying its own interpreter. Nothing ships yet; see +[runners and plugins](../dev/runners-and-plugins.md). diff --git a/docs/reference/api.md b/docs/reference/api.md index 82be78e..4e27901 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -15,7 +15,15 @@ var Version string // default 0.1.0-dev; override with ldflags | `FileName` | `"godo.yaml"` | | `FindFile` | Walk cwd parents for catalog | | `LoadFile` / `Parse` | Load catalog | -| `NewEngine` / `WithDialects` | Runner | +| `NewEngine` / `WithDialects` / `WithRunners` | Engine wiring | +| `NewRunnerRegistry` / `DefaultRunners` / `EffectiveRunner` | Named runners (`RunnerInherit`) | + +A catalog that names no runner uses the `Runner` injected into the engine, so +`NewEngine(cat, myRunner)` keeps working with nothing registered. The CLI +injects the caller's own shell there, which is why `inherit` is the default for +a `godo.yaml`. +| `ArgsAwareRunner` | Runner decides whether a body takes leftover tokens | +| `EngineSpec` / `Plugin` | The `engine:` block: binary minimum + declared plugins | | `Engine.Run` / `PreviewLines` | Execute / expand | | `ExitCode` | Map errors to process codes | @@ -25,6 +33,6 @@ var Version string // default 0.1.0-dev; override with ldflags ## Errors -`ErrNoMatch`, `ErrNoTokens`, `ErrUnexpectedArgs`, `ErrDependencyCycle`, `ErrInvalidCatalog`, `ErrInvalidCapture`, … +`ErrNoMatch`, `ErrNoTokens`, `ErrUnexpectedArgs`, `ErrDependencyCycle`, `ErrInvalidCatalog`, `ErrInvalidCapture`, `ErrUnknownRunner`, … Pre-1.0: symbols may move; prefer pinning modules. [versioning.md](../dev/versioning.md). diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 53281ec..2d654fd 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -18,6 +18,7 @@ Normative source: [contract.md](../contract.md). | | | |--|--| +| `-e runners` | List runners usable on this machine | | `-e version` | Print binary version | | `-e update` | Install newer Release asset | | `-e update check` | Check only | diff --git a/docs/roadmap.md b/docs/roadmap.md index 545ab2a..bf675dc 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -16,12 +16,44 @@ This is what we **commit to communicate**. Pre-1.0 APIs can still change within **Not promised in v0.1:** Homebrew/Scoop installs, dialects `nscript`/`matchns`, stable Go API. +## v0.3 — in progress, unreleased + +Breaking: the shell that runs your scripts changes, and file fields move into +`engine:`. **Held until plugin loading works** — `engine:` without a loader is +half a promise, and this release is where the promise gets made. + +| Promise | | +|---------|--| +| Runner axis | `engine.runner` / `# @runner` pick how a body becomes a process | +| Default runner | The shell **you are in**, not `sh` / `cmd` | +| Shell by name | `sh`, `bash`, `zsh`, `dash`, `ksh`, `ash`, `fish`, `nu`, `cmd`, `pwsh`, `powershell` | +| `engine:` block | `version`, `dialect`, `runner`, `plugins` — what godo needs, apart from what the scripts are | +| `engine.version` | Minimum binary, enforced before anything runs | +| `godo -e runners` | What is usable here, and how to check which shell you are in | +| Compatibility | Top-level `dialect:` keeps working | + +### Landed + +Everything in the table above. + +### Still required before v0.3 ships + +- **Plugin loading.** `engine.plugins` parses and validates; nothing reads the + artifact yet. WASM via `wazero`, digest-pinned — see + [runners and plugins](./dev/runners-and-plugins.md). + +**Not promised in v0.3:** Windows shell detection is written from the +documented behavior of those shells; it compiles and vets for `windows/amd64` +but is unverified on a real Windows host. `GODO_SHELL` overrides it. + ## Post-v0.1 — intended (not promised dates) - Shared family packaging: `MY-RV/homebrew-tap` (`brew install --cask MY-RV/tap/godo`), `MY-RV/scoop-bucket` - Optional: winget (`MY-RV.Godo`), later choco / AUR / Nix as demand appears - Engine command registry polish; more e2e - Dialects backlog only if explicitly promoted here +- Plugin loading (WASM via wazero, digest-pinned) — see + [runners and plugins](./dev/runners-and-plugins.md) ## v1.0 — future promise diff --git a/godo.yaml b/godo.yaml index 7117aac..ec8f03f 100644 --- a/godo.yaml +++ b/godo.yaml @@ -1,5 +1,10 @@ version: "0.1" +# No runner: named, so every script below runs under the shell you are in. +# These are plain commands, so that is the same everywhere — see +# docs/guide/runners.md. A script needing one specific shell says so with +# "# @runner ". + scripts: test: go test ./... vet: go vet ./...