Repository navigation
feat(mcpx): make script.check canonical with legacy aliases #5660
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
| } | ||
| }) | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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"}, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
With a daemon running, Useful? React with 👍 / 👎. |
||
| 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", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an existing project config contains
{"script":{"typecheck":"on"}}, runningmcpx settings set script.typecheck off --persist projectagainst a running daemon now leaves the configuration with conflicting values.handleSettingsSetininternal/daemon/routes_settings.gowritesdecl.Path, nowscript.check, whileconfig.SetKeyonly updates that leaf without removingscript.typecheck. Subsequent invocations fail settings resolution becauseApplyFilereads both spellings andSet.Applyrejects their differing values at the same layer. Persistent writes need to replace or remove existing alias entries when writing the canonical setting.Useful? React with 👍 / 👎.