From 26d884c611d3d9cf517e7776662bfd7c23f7e209 Mon Sep 17 00:00:00 2001 From: "developing-today[bot]" <5454422+developing-today[bot]@users.noreply.github.com> Date: Wed, 4 Nov 2026 16:07:05 -0600 Subject: [PATCH] feat(mcpx): add script.check compatibility aliases --- pkgs/mcpx/docs/configuration.md | 2 +- pkgs/mcpx/docs/exec.md | 2 +- pkgs/mcpx/docs/scripts.md | 8 +- pkgs/mcpx/internal/api/ops_exec.go | 2 +- pkgs/mcpx/internal/cli/commands.go | 9 +- pkgs/mcpx/internal/cli/flagindex_generated.go | 4 +- pkgs/mcpx/internal/daemon/routes_exec.go | 2 +- pkgs/mcpx/internal/e2e/effect_test.go | 2 +- pkgs/mcpx/internal/execsvc/execsvc.go | 3 +- pkgs/mcpx/internal/preflight/preflight.go | 13 +-- .../mcpx/internal/preflight/preflight_test.go | 36 ++++++++ .../internal/settings/configuration_table.go | 3 + pkgs/mcpx/internal/settings/effect.go | 2 +- pkgs/mcpx/internal/settings/pathalias_test.go | 90 +++++++++++++++++++ pkgs/mcpx/internal/settings/registry.go | 22 +++-- pkgs/mcpx/internal/settings/schema.go | 50 +++++++++-- pkgs/mcpx/internal/settings/value.go | 7 +- 17 files changed, 218 insertions(+), 39 deletions(-) create mode 100644 pkgs/mcpx/internal/settings/pathalias_test.go diff --git a/pkgs/mcpx/docs/configuration.md b/pkgs/mcpx/docs/configuration.md index b03c5ebdb..5d7c99b7f 100644 --- a/pkgs/mcpx/docs/configuration.md +++ b/pkgs/mcpx/docs/configuration.md @@ -1012,6 +1012,7 @@ skipped launcher phases). | --- | --- | --- | --- | --- | --- | --- | --- | | `script.before` | source | *(empty)* | client | no | `--script-before` | `MCPX_SCRIPT_BEFORE` | runs first, ahead of the globals being installed | | `script.captureConsole` | bool | `true` | client | no | `--script-capture-console` | `MCPX_SCRIPT_CAPTURE_CONSOLE` | route console calls into the log | +| `script.check` | enum | `off` | client | no | `--script-check`, `--script-typecheck`, `--typecheck` | `MCPX_SCRIPT_CHECK`, `MCPX_SCRIPT_TYPECHECK` | check the generated program before running it; path alias: script.typecheck; one of off, types (also on), strict | | `script.cwd` | enum | `caller` | client | no | `--script-cwd` | `MCPX_SCRIPT_CWD` | where a script runs: caller, script, config or workdir | | `script.env` | list | *(empty)* | client | no | `--script-env`, `--env` | `MCPX_SCRIPT_ENV` | KEY=VALUE pairs added to the script's environment | | `script.imports` | enum | `report` | client | no | `--script-imports` | `MCPX_SCRIPT_IMPORTS` | resolve the script's import graph before running it | @@ -1027,7 +1028,6 @@ skipped launcher phases). | `script.runtimeOrder` | list | `deno,bun,node` | client | no | `--script-runtime-order` | `MCPX_SCRIPT_RUNTIME_ORDER` | the runtimes auto tries, in order | | `script.runtimes` | string | *(empty)* | client | no | `--script-runtimes` | `MCPX_SCRIPT_RUNTIMES` | named runtime binaries: {name: {kind, bin, args}} | | `script.suffix` | source | *(empty)* | client | no | `--script-suffix` | `MCPX_SCRIPT_SUFFIX` | runs last on both paths, like a finally | -| `script.typecheck` | enum | `off` | client | no | `--script-typecheck` | `MCPX_SCRIPT_TYPECHECK` | check the generated program before running it | ### search diff --git a/pkgs/mcpx/docs/exec.md b/pkgs/mcpx/docs/exec.md index 719e5c65a..3aa1bcc4c 100644 --- a/pkgs/mcpx/docs/exec.md +++ b/pkgs/mcpx/docs/exec.md @@ -110,7 +110,7 @@ type ExecOptions = { ns?: string[]; args?: string[]; export?: string; - typecheck?: "off" | "on" | "strict"; + typecheck?: "off" | "types" | "strict" | "on"; // on is a legacy alias for types launcher?: string; launcherName?: string; // template text, or "none" allowRepeat?: string[]; diff --git a/pkgs/mcpx/docs/scripts.md b/pkgs/mcpx/docs/scripts.md index dd096bdea..3370059e5 100644 --- a/pkgs/mcpx/docs/scripts.md +++ b/pkgs/mcpx/docs/scripts.md @@ -88,9 +88,11 @@ writer hits first: properties — that Node itself runs fine once types are transformed. An `enum` in a script is guarded by a test (`TestNodeRunsNonErasableTypeScript`), not a limitation. -- **Typechecking is Deno-only.** `--typecheck` / - `script.typecheck` runs the check (via `deno check`) before - the run and only applies when Deno is the runtime. +- **Typechecking is Deno-only.** `--typecheck` / `script.typecheck` + remain accepted as legacy spellings for `--script-check` / + `script.check`. `types` resolves imports and checks types with `deno check`, + and `strict` also checks declaration files. The old value `on` means `types`. + Non-Deno runtimes warn and continue because they strip types rather than checking them. ### Authority diff --git a/pkgs/mcpx/internal/api/ops_exec.go b/pkgs/mcpx/internal/api/ops_exec.go index 674c9dbed..e700e9568 100644 --- a/pkgs/mcpx/internal/api/ops_exec.go +++ b/pkgs/mcpx/internal/api/ops_exec.go @@ -135,7 +135,7 @@ const execOptionsSchema = `{"type":"object","properties":{ "ns":{"type":"array","items":{"type":"string"},"description":"restrict the generated client to these namespaces"}, "args":{"type":"array","items":{"type":"string"},"description":"arguments handed to the script"}, "export":{"type":"string","description":"call this exported function instead of the default one"}, -"typecheck":{"type":"string","enum":["off","on","strict"],"description":"check the program before running it; default from settings"}, +"typecheck":{"type":"string","enum":["off","types","strict","on"],"description":"check the program before running it; on is a legacy alias for types; default from settings"}, "launcher":{"type":"string","description":"launcher template text replacing the generated one, or none for no launcher"}, "launcherName":{"type":"string","description":"what to call the launcher in messages"}, "allowRepeat":{"type":"array","items":{"type":"string"},"description":"launcher placeholders permitted to resolve more than once"}, diff --git a/pkgs/mcpx/internal/cli/commands.go b/pkgs/mcpx/internal/cli/commands.go index 6d01e15f4..2ef249dfb 100644 --- a/pkgs/mcpx/internal/cli/commands.go +++ b/pkgs/mcpx/internal/cli/commands.go @@ -656,7 +656,7 @@ func (a *App) runScript(ctx context.Context, args []string, inline bool) error { noLauncher := fs.Bool("no-launcher", false, "run the script with no launcher: no globals, no capture, no wrapper") typecheck := fs.String("typecheck", "", - "check the program before running it: off, on, strict") + "check the program before running it: off, types, strict (on aliases types)") promptFlag := fs.String("prompt", "", "synthesize and execute a script from a natural language prompt") validateFlag := fs.Bool("validate", false, @@ -2416,10 +2416,9 @@ func (a *App) resolveScriptShape(cfg *config.Config, f scriptFlags) (scriptShape sh.onSuccess = cfg.ScriptPhase("onSuccess", sh.hooks.phaseValues("script.onSuccess", f.onSuccess)) sh.onError = cfg.ScriptPhase("onError", sh.hooks.phaseValues("script.onError", f.onError)) - // Read from the resolved set, so the value is whichever layer won: the - // short --typecheck spelling, the generated --script-typecheck, a - // variable, or the config file. - sh.typecheck = firstNonEmpty(f.typecheck, a.Settings().String("script.typecheck")) + // Read the canonical setting after resolution so legacy path, variable, + // and flag spellings all pass through the same value normalization. + sh.typecheck = a.Settings().String("script.check") // No short flag spelling: the registry value is the whole story, from // the generated --script-imports flag, a variable, or the config file. diff --git a/pkgs/mcpx/internal/cli/flagindex_generated.go b/pkgs/mcpx/internal/cli/flagindex_generated.go index 8b2508028..f1886aa09 100644 --- a/pkgs/mcpx/internal/cli/flagindex_generated.go +++ b/pkgs/mcpx/internal/cli/flagindex_generated.go @@ -103,7 +103,7 @@ var handFlagIndex = map[string][]HandFlag{ {Name: "on-error", Usage: "line to run when it throws; repeatable"}, {Name: "launcher", Usage: "replace the generated launcher with this source or file"}, {Name: "no-launcher", Usage: "run the script with no launcher: no globals, no capture, no wrapper"}, - {Name: "typecheck", Usage: "check the program before running it: off, on, strict"}, + {Name: "typecheck", Usage: "check the program before running it: off, types, strict (on aliases types)"}, {Name: "prompt", Usage: "synthesize and execute a script from a natural language prompt"}, {Name: "validate", Usage: "validate script structure, tool signatures, and syntax without executing"}, {Name: "repair", Usage: "automatically attempt script repair using configured model if execution fails"}, @@ -230,7 +230,7 @@ var handFlagIndex = map[string][]HandFlag{ {Name: "on-error", Usage: "line to run when it throws; repeatable"}, {Name: "launcher", Usage: "replace the generated launcher with this source or file"}, {Name: "no-launcher", Usage: "run the script with no launcher: no globals, no capture, no wrapper"}, - {Name: "typecheck", Usage: "check the program before running it: off, on, strict"}, + {Name: "typecheck", Usage: "check the program before running it: off, types, strict (on aliases types)"}, {Name: "prompt", Usage: "synthesize and execute a script from a natural language prompt"}, {Name: "validate", Usage: "validate script structure, tool signatures, and syntax without executing"}, {Name: "repair", Usage: "automatically attempt script repair using configured model if execution fails"}, diff --git a/pkgs/mcpx/internal/daemon/routes_exec.go b/pkgs/mcpx/internal/daemon/routes_exec.go index f48141fe7..4b4662986 100644 --- a/pkgs/mcpx/internal/daemon/routes_exec.go +++ b/pkgs/mcpx/internal/daemon/routes_exec.go @@ -202,7 +202,7 @@ func (s *Server) ExecService() *execsvc.Service { lim.Setup, lim.SetupErr = runner.ParseSetup(set.String("script.runtimes"), set.List("script.runtimeOrder"), set.String("script.profiles")) lim.CaptureConsole = set.Bool("script.captureConsole") - lim.Typecheck = set.String("script.typecheck") + lim.Typecheck = set.String("script.check") lim.Imports = set.String("script.imports") lim.ImportsOnDynamic = set.String("script.importsOnDynamic") lim.Delivery = set.String("artifacts.delivery") diff --git a/pkgs/mcpx/internal/e2e/effect_test.go b/pkgs/mcpx/internal/e2e/effect_test.go index 345a821c0..9aa904131 100644 --- a/pkgs/mcpx/internal/e2e/effect_test.go +++ b/pkgs/mcpx/internal/e2e/effect_test.go @@ -38,7 +38,7 @@ var implementedEffectProbes = map[string]string{ "script.interpreter": "TestScriptInterpreterIsReachableFromTheEnvironment", "script.env": "TestScriptEnvIsReachableFromTheEnvironment", "script.captureConsole": "TestScriptCaptureConsoleOffDropsConsoleOutput", - "script.typecheck": "TestScriptTypecheckOnRefusesATypeError", + "script.check": "TestScriptTypecheckOnRefusesATypeError", "logging.file": "TestLoggingFileOffWritesNoDurableLog", "plumbing.strictUnknownKeys": "TestStrictUnknownKeysRefusesATypo", "paths.state": "TestStateDirectoryIsReachableAsASetting", diff --git a/pkgs/mcpx/internal/execsvc/execsvc.go b/pkgs/mcpx/internal/execsvc/execsvc.go index a8cfb6911..3b7c2e491 100644 --- a/pkgs/mcpx/internal/execsvc/execsvc.go +++ b/pkgs/mcpx/internal/execsvc/execsvc.go @@ -152,7 +152,8 @@ type Options struct { // Phase lines and the launcher arrive as source: a path would name the // daemon's filesystem, and the caller meant theirs. - // TypeCheck is off, on or strict; empty takes the daemon's default. + // TypeCheck is off, syntax, types or strict; on remains an alias for types. + // Empty takes the daemon's default. TypeCheck string `json:"typecheck,omitempty"` // Imports is off or report; empty takes the daemon's default. Imports string `json:"imports,omitempty"` diff --git a/pkgs/mcpx/internal/preflight/preflight.go b/pkgs/mcpx/internal/preflight/preflight.go index 63eb4a4ad..17585ce7e 100644 --- a/pkgs/mcpx/internal/preflight/preflight.go +++ b/pkgs/mcpx/internal/preflight/preflight.go @@ -179,7 +179,8 @@ type TypeCheckMode string const ( TypeCheckOff TypeCheckMode = "off" - TypeCheckOn TypeCheckMode = "on" + TypeCheckTypes TypeCheckMode = "types" + TypeCheckOn TypeCheckMode = TypeCheckTypes // legacy name for callers TypeCheckStrict TypeCheckMode = "strict" ) @@ -226,8 +227,8 @@ func TypeCheckResolved(ctx context.Context, runtimeBin, entry string, mode TypeC // equivalent single command. Saying so is better than pretending the // check ran. r.Add(Problem{ - What: "type checking is only available under Deno; " + base + " strips types rather than checking them", - Where: "script.typecheck", + What: "script checking is only available under Deno; " + base + " strips types rather than checking them", + Where: "script.check", }) return r } @@ -256,14 +257,14 @@ func TypeCheckResolved(ctx context.Context, runtimeBin, entry string, mode TypeC if cctx.Err() != nil { r.Add(Problem{ What: "type checking did not finish within " + timeout.String(), - Where: "script.typecheck", - Fix: "raise the timeout, or set script.typecheck off", + Where: "script.check", + Fix: "raise the timeout, or set script.check off", }) return r } r.Add(Problem{ What: "the program does not type check:\n" + indent(strings.TrimSpace(string(out))), - Where: "script.typecheck", + Where: "script.check", Fatal: true, }) return r diff --git a/pkgs/mcpx/internal/preflight/preflight_test.go b/pkgs/mcpx/internal/preflight/preflight_test.go index a6a6f5fe9..4d95402fe 100644 --- a/pkgs/mcpx/internal/preflight/preflight_test.go +++ b/pkgs/mcpx/internal/preflight/preflight_test.go @@ -105,6 +105,42 @@ func TestTypeCheckIsSkippedWhenOff(t *testing.T) { } } +func TestScriptCheckModesSelectTypesAndStrictCommands(t *testing.T) { + dir := t.TempDir() + entry := filepath.Join(dir, "main.ts") + if err := os.WriteFile(entry, []byte("export {};\n"), 0o644); err != nil { + t.Fatal(err) + } + marker := filepath.Join(dir, "args") + stub := filepath.Join(dir, "deno-stub") + script := "#!/bin/sh\nprintf '%s\\n' \"$@\" > '" + marker + "'\n" + if err := os.WriteFile(stub, []byte(script), 0o755); err != nil { + t.Fatal(err) + } + + for _, tc := range []struct { + mode preflight.TypeCheckMode + want []string + }{ + {preflight.TypeCheckTypes, []string{"check", entry}}, + {preflight.TypeCheckStrict, []string{"check", "--all", entry}}, + } { + t.Run(string(tc.mode), func(t *testing.T) { + rep := preflight.TypeCheckResolved(context.Background(), stub, entry, tc.mode, 0, nil, dir) + if err := rep.Err(); err != nil { + t.Fatalf("check failed: %v", err) + } + got, err := os.ReadFile(marker) + if err != nil { + t.Fatal(err) + } + if fields := strings.Fields(string(got)); strings.Join(fields, "\x00") != strings.Join(tc.want, "\x00") { + t.Errorf("arguments = %q, want %q", fields, tc.want) + } + }) + } +} + func TestTypeCheckSaysSoWhenTheRuntimeCannotDoIt(t *testing.T) { // Node and Bun strip types rather than checking them. Saying so is // better than pretending the check ran. diff --git a/pkgs/mcpx/internal/settings/configuration_table.go b/pkgs/mcpx/internal/settings/configuration_table.go index d6b5410dd..145a22401 100644 --- a/pkgs/mcpx/internal/settings/configuration_table.go +++ b/pkgs/mcpx/internal/settings/configuration_table.go @@ -48,6 +48,9 @@ func ConfigurationTable(all []Setting) string { envs = append(envs, "`"+a+"`") } governs := set.Short + if len(set.PathAliases) > 0 { + governs += "; path alias: " + strings.Join(set.PathAliases, ", ") + } if len(set.EnumAliases) > 0 { // Only where aliases exist: the canonical names are what a // reader should write, and the aliases are what they may diff --git a/pkgs/mcpx/internal/settings/effect.go b/pkgs/mcpx/internal/settings/effect.go index ec63ef759..aad7cc5d9 100644 --- a/pkgs/mcpx/internal/settings/effect.go +++ b/pkgs/mcpx/internal/settings/effect.go @@ -45,7 +45,7 @@ var EffectProbes = map[string]string{ "logging.file": "TestLoggingFileOffWritesNoDurableLog", "script.captureConsole": "TestScriptCaptureConsoleOffDropsConsoleOutput", "script.prefix": "TestScriptPrefixIsReachableFromTheEnvironment", - "script.typecheck": "TestScriptTypecheckOnRefusesATypeError", + "script.check": "TestScriptTypecheckOnRefusesATypeError", "script.env": "TestScriptEnvIsReachableFromTheEnvironment", "paths.state": "TestStateDirectoryIsReachableAsASetting", "registry.url": "TestRegistryPageSizeChangesWhatTheCLIAsksFor", diff --git a/pkgs/mcpx/internal/settings/pathalias_test.go b/pkgs/mcpx/internal/settings/pathalias_test.go new file mode 100644 index 000000000..0f4cada7d --- /dev/null +++ b/pkgs/mcpx/internal/settings/pathalias_test.go @@ -0,0 +1,90 @@ +package settings_test + +import ( + "flag" + "strings" + "testing" + + "github.com/developing-today/code/pkgs/mcpx/internal/settings" +) + +func pathAliasSchema(t *testing.T) *settings.Schema { + t.Helper() + s, err := settings.New([]settings.Setting{{ + Path: "new.name", PathAliases: []string{"old.name"}, + Kind: settings.KindEnum, Enum: []string{"off", "types"}, + EnumAliases: map[string][]string{"types": {"on"}}, Default: "off", + FlagAliases: []string{"old-name"}, EnvAliases: []string{"MCPX_OLD_NAME"}, + Name: "Name", Short: "name", + }}) + if err != nil { + t.Fatalf("build alias schema: %v", err) + } + return s +} + +func TestPathAliasNormalizesAndStoresCanonicalPathFromEverySource(t *testing.T) { + s := pathAliasSchema(t) + sources := map[string]func(*settings.Set) error{ + "file": func(set *settings.Set) error { + return s.ApplyFile(set, map[string]any{"old": map[string]any{"name": "on"}}, "/x/.mcpx.json", 0) + }, + "env": func(set *settings.Set) error { + return s.ApplyEnv(set, []string{"MCPX_OLD_NAME=on"}) + }, + "flag": func(set *settings.Set) error { + fs := flag.NewFlagSet("run", flag.ContinueOnError) + fs.SetOutput(nopWriter{}) + b := s.Bind(fs, "run") + if err := fs.Parse([]string{"--old-name", "on"}); err != nil { + return err + } + return b.ApplyTo(set) + }, + "runtime": func(set *settings.Set) error { return set.SetRuntime("old.name", "on") }, + } + for name, apply := range sources { + t.Run(name, func(t *testing.T) { + set := settings.NewSet(s) + if err := apply(set); err != nil { + t.Fatalf("apply alias: %v", err) + } + if got := set.String("new.name"); got != "types" { + t.Errorf("canonical path = %q, want types", got) + } + if got := set.String("old.name"); got != "types" { + t.Errorf("alias lookup = %q, want types", got) + } + if value, ok := set.Value("old.name"); !ok || value.Path != "new.name" { + t.Errorf("value = %#v, %v; want canonical path", value, ok) + } + }) + } +} + +func TestPathAliasCanonicalizesCallOverrides(t *testing.T) { + s := pathAliasSchema(t) + base := settings.NewSet(s) + view := base.WithOverrides(map[string]string{"old.name": "types"}, "request") + if got := view.String("new.name"); got != "types" { + t.Errorf("canonical path = %q, want types", got) + } + if got := view.String("old.name"); got != "types" { + t.Errorf("alias lookup = %q, want types", got) + } +} + +func TestPathAliasesCannotCollideWithCanonicalOrOtherAliasPaths(t *testing.T) { + base := settings.Setting{Path: "new.name", PathAliases: []string{"old.name"}, Kind: settings.KindString, Name: "Name", Short: "name"} + for name, list := range map[string][]settings.Setting{ + "canonical path": {base, {Path: "old.name", Kind: settings.KindString, Name: "Old", Short: "old"}}, + "another alias": {base, {Path: "other.name", PathAliases: []string{"old.name"}, Kind: settings.KindString, Name: "Other", Short: "other"}}, + "empty alias": {{Path: "new.name", PathAliases: []string{""}, Kind: settings.KindString, Name: "Name", Short: "name"}}, + } { + t.Run(name, func(t *testing.T) { + if _, err := settings.New(list); err == nil || !strings.Contains(err.Error(), "path") { + t.Errorf("invalid alias declaration accepted: %v", err) + } + }) + } +} diff --git a/pkgs/mcpx/internal/settings/registry.go b/pkgs/mcpx/internal/settings/registry.go index e72303ed7..346b6ff57 100644 --- a/pkgs/mcpx/internal/settings/registry.go +++ b/pkgs/mcpx/internal/settings/registry.go @@ -349,14 +349,20 @@ func scriptSettings() []Setting { Long: srcLong, }, { - Path: "script.typecheck", Commands: runCommands, Kind: KindEnum, Default: "off", - Scope: ScopeClient, - Enum: []string{"off", "on", "strict"}, - Name: "Type check", - Short: "check the generated program before running it", - Long: "Resolves and checks every import without executing anything. Costs " + - "a second or two on a cold module cache, which is why it is off by " + - "default rather than on.", + Path: "script.check", PathAliases: []string{"script.typecheck"}, + Commands: runCommands, Kind: KindEnum, Default: "off", + Scope: ScopeClient, + Enum: []string{"off", "types", "strict"}, + EnumAliases: map[string][]string{"types": {"on"}}, + FlagAliases: []string{"script-typecheck", "typecheck"}, + EnvAliases: []string{"MCPX_SCRIPT_TYPECHECK"}, + Name: "Script check", + Short: "check the generated program before running it", + Long: "types resolves imports and checks types; strict also checks " + + "declaration files. Checking is off by default. Type checking is " + + "currently supported by Deno. The old " + + "script.typecheck path, MCPX_SCRIPT_TYPECHECK variable, --script-typecheck " + + "flag and --typecheck flag remain accepted; the old value on means types.", }, { Path: "script.imports", Commands: runCommands, Kind: KindEnum, Default: "report", diff --git a/pkgs/mcpx/internal/settings/schema.go b/pkgs/mcpx/internal/settings/schema.go index e0bcf5e85..7a92912af 100644 --- a/pkgs/mcpx/internal/settings/schema.go +++ b/pkgs/mcpx/internal/settings/schema.go @@ -168,6 +168,10 @@ type Setting struct { Env string EnvAliases []string + // PathAliases are older dotted configuration/runtime-setting paths that + // resolve to this setting. Values are always stored under Path. + PathAliases []string + // Enum lists the permitted values when Kind is KindEnum. Enum []string @@ -326,10 +330,11 @@ func isLower(r rune) bool { return r >= 'a' && r <= 'z' } // Schema is the whole set. type Schema struct { - settings []Setting - byPath map[string]*Setting - byFlag map[string]*Setting - byEnv map[string]*Setting + settings []Setting + byPath map[string]*Setting + byPathAlias map[string]*Setting + byFlag map[string]*Setting + byEnv map[string]*Setting } // New builds a schema, rejecting collisions. A duplicated flag or variable is @@ -337,9 +342,10 @@ type Schema struct { // discovering that one of two settings silently wins. func New(list []Setting) (*Schema, error) { s := &Schema{ - byPath: map[string]*Setting{}, - byFlag: map[string]*Setting{}, - byEnv: map[string]*Setting{}, + byPath: map[string]*Setting{}, + byPathAlias: map[string]*Setting{}, + byFlag: map[string]*Setting{}, + byEnv: map[string]*Setting{}, } // Sort before taking any pointers. Sorting afterwards moves the elements // out from under every pointer already handed to the lookup maps, which @@ -355,6 +361,18 @@ func New(list []Setting) (*Schema, error) { } p := &s.settings[i] s.byPath[set.Path] = p + for _, name := range set.PathAliases { + if name == "" || name == set.Path { + return nil, fmt.Errorf("settings: %q has invalid path alias %q", set.Path, name) + } + if prev, dup := s.byPath[name]; dup { + return nil, fmt.Errorf("settings: path %q is claimed by both %q and alias of %q", name, prev.Path, set.Path) + } + if prev, dup := s.byPathAlias[name]; dup { + return nil, fmt.Errorf("settings: path alias %q is claimed by both %q and %q", name, prev.Path, set.Path) + } + s.byPathAlias[name] = p + } for _, name := range append([]string{set.FlagName()}, set.FlagAliases...) { if prev, dup := s.byFlag[name]; dup { @@ -371,6 +389,13 @@ func New(list []Setting) (*Schema, error) { s.byEnv[name] = p } } + for _, set := range s.settings { + for _, alias := range set.PathAliases { + if prev, dup := s.byPath[alias]; dup { + return nil, fmt.Errorf("settings: path alias %q for %q collides with %q", alias, set.Path, prev.Path) + } + } + } for i := range s.settings { if err := checkEnumAliases(&s.settings[i]); err != nil { return nil, err @@ -388,9 +413,20 @@ func (s *Schema) All() []Setting { return append([]Setting(nil), s.settings...) // Lookup finds a setting by its dotted path. func (s *Schema) Lookup(path string) (*Setting, bool) { set, ok := s.byPath[path] + if !ok { + set, ok = s.byPathAlias[path] + } return set, ok } +// canonicalPath returns the declared spelling for a path alias. +func (s *Schema) canonicalPath(path string) string { + if set, ok := s.byPathAlias[path]; ok { + return set.Path + } + return path +} + // ByFlag finds a setting by any of its flag spellings. func (s *Schema) ByFlag(name string) (*Setting, bool) { set, ok := s.byFlag[strings.TrimLeft(name, "-")] diff --git a/pkgs/mcpx/internal/settings/value.go b/pkgs/mcpx/internal/settings/value.go index 2f22f1b7b..4c2db345d 100644 --- a/pkgs/mcpx/internal/settings/value.go +++ b/pkgs/mcpx/internal/settings/value.go @@ -137,7 +137,7 @@ func (s *Set) WithOverrides(ov map[string]string, detail string) *Set { } copied := make(map[string]string, len(ov)) for k, v := range ov { - copied[k] = v + copied[s.schema.canonicalPath(k)] = v } return &Set{schema: s.schema, base: s, override: copied, overrideDetail: detail} } @@ -148,6 +148,7 @@ func (s *Set) WithOverrides(ov map[string]string, detail string) *Set { // that skips the parser is the one way to get an invalid value into a running // process. func (s *Set) SetRuntime(path, raw string) error { + path = s.schema.canonicalPath(path) set, ok := s.schema.Lookup(path) if !ok { return fmt.Errorf("unknown setting %q", path) @@ -168,6 +169,7 @@ func (s *Set) SetRuntime(path, raw string) error { // ClearRuntime drops a runtime override, so the value falls back to whatever // the flags, environment and files said. Reports whether one was there. func (s *Set) ClearRuntime(path string) bool { + path = s.schema.canonicalPath(path) s.mu.Lock() defer s.mu.Unlock() _, had := s.override[path] @@ -178,6 +180,7 @@ func (s *Set) ClearRuntime(path string) bool { // RuntimeOverride reports the runtime override for path, if one is in force. // A caller that changes it and may have to undo the change reads it first. func (s *Set) RuntimeOverride(path string) (string, bool) { + path = s.schema.canonicalPath(path) s.mu.RLock() defer s.mu.RUnlock() v, ok := s.override[path] @@ -203,6 +206,7 @@ func (s *Set) RuntimeOverrides() map[string]string { // twice in one file. Across layers there is no conflict, only precedence, and // treating that as an error would make configuration files useless. func (s *Set) Apply(path, raw string, origin Origin) error { + path = s.schema.canonicalPath(path) set, ok := s.schema.Lookup(path) if !ok { return fmt.Errorf("unknown setting %q", path) @@ -239,6 +243,7 @@ func (s *Set) Apply(path, raw string, origin Origin) error { // to its ceiling when it has one (Setting.ClampedBy), so no reader can // observe a value above it. func (s *Set) Value(path string) (*Value, bool) { + path = s.schema.canonicalPath(path) v, ok := s.unclamped(path) if !ok { return v, ok