Skip to content

fix(cli): require explicit source for unknown-server fallback #7

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.

Sign up for GitHub

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

Merged
lydakis merged 2 commits into from
Mar 12, 2026
Merged

fix(cli): require explicit source for unknown-server fallback #7

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e94e50e
fix(cli): require explicit source for unknown-server fallback
lydakis Mar 12, 2026
02c69f4
fix(cli): accept existing bare file sources
lydakis Mar 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion internal/cli/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ var (
}
resolveSourceFn = bootstrap.Resolve
checkPrereqsFn = bootstrap.CheckPrerequisites
statSourceFn = os.Stat
)

// Run is the main CLI entry point. Returns an exit code.
Expand Down Expand Up @@ -756,6 +757,9 @@ func sendServerRequestWithEphemeralFallback(client daemonRequester, req *ipc.Req
if effectiveReq.Ephemeral != nil || !isUnknownServerResponse(resp, effectiveReq.Server) {
return resp, nil
}
if !looksLikeExplicitEphemeralSource(effectiveReq.Server) {
return resp, nil
Comment on lines +760 to +761
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Treat bare relative file paths as explicit sources

This new guard skips fallback unless looksLikeExplicitEphemeralSource is true, but that helper does not recognize valid relative file paths without / or a .json/.toml suffix (for example mcpx manifest ping {} when ./manifest is a JSON/TOML manifest). Before this commit those sources resolved via bootstrap.Resolve (which reads arbitrary file paths), so this change regresses legitimate ephemeral-source invocations into unknown server errors.

Useful? React with 👍 / 👎.

}

ephemeral, resolveResp := resolveEphemeralSource(effectiveReq.Server)
if resolveResp != nil {
Expand Down Expand Up @@ -856,7 +860,12 @@ func looksLikeExplicitEphemeralSource(source string) bool {
if strings.Contains(lower, "?config=") {
return true
}
return false

info, err := statSourceFn(filepath.Clean(source))
if err != nil {
return false
}
return info.Mode().IsRegular()
Comment on lines +864 to +868
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep typo handling from probing bare local files

The new os.Stat fallback makes any existing regular file look like an explicit source, so an invocation like mcpx ghub ping {} now enters ephemeral resolution whenever ./ghub exists, even if the user meant a server name and that file is unrelated. In this case parse/validation failures are surfaced as resolving source ... errors instead of the previous unknown server response, which regresses typo UX and reintroduces local-file influence for bare names in normal project directories.

Useful? React with 👍 / 👎.

}

func isUnknownServerResponse(resp *ipc.Response, server string) bool {
Expand Down
167 changes: 149 additions & 18 deletions internal/cli/root_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1469,7 +1469,7 @@ func TestRunUnknownServerHelpUsesDaemonRuntimeEphemeralWithoutResolvingSource(t
}
}

func TestRunUnknownServerToolCallKeepsUsageErrorForNonExplicitResolveError(t *testing.T) {
func TestRunUnknownServerToolCallKeepsUsageErrorForMissingBareRelativeSource(t *testing.T) {
tmp := t.TempDir()
xdgConfigHome := filepath.Join(tmp, "xdg-config")
configDir := filepath.Join(xdgConfigHome, "mcpx")
Expand All @@ -1484,7 +1484,22 @@ func TestRunUnknownServerToolCallKeepsUsageErrorForNonExplicitResolveError(t *te
t.Setenv("HOME", tmp)
t.Setenv("XDG_RUNTIME_DIR", t.TempDir())

const source = "go.mod"
projectDir := filepath.Join(tmp, "project")
if err := os.MkdirAll(projectDir, 0o755); err != nil {
t.Fatalf("MkdirAll(projectDir): %v", err)
}
oldWD, err := os.Getwd()
if err != nil {
t.Fatalf("Getwd(): %v", err)
}
if err := os.Chdir(projectDir); err != nil {
t.Fatalf("Chdir(projectDir): %v", err)
}
defer func() {
_ = os.Chdir(oldWD)
}()

const source = "manifest"

oldSpawn := spawnOrConnectFn
oldClient := newDaemonClient
Expand All @@ -1498,14 +1513,12 @@ func TestRunUnknownServerToolCallKeepsUsageErrorForNonExplicitResolveError(t *te
}()

spawnOrConnectFn = func() (string, error) { return "nonce", nil }
resolveSourceFn = func(_ context.Context, got string, _ bootstrap.ResolveOptions) (bootstrap.ResolvedServer, error) {
if got != source {
return bootstrap.ResolvedServer{}, errors.New("unexpected source")
}
return bootstrap.ResolvedServer{}, errors.New("payload is not a supported JSON or TOML server manifest")
resolveSourceFn = func(context.Context, string, bootstrap.ResolveOptions) (bootstrap.ResolvedServer, error) {
t.Fatal("resolveSourceFn should not be called for missing bare relative source")
return bootstrap.ResolvedServer{}, nil
}
checkPrereqsFn = func(config.ServerConfig) error {
t.Fatal("checkPrereqsFn should not be called when resolve fails")
t.Fatal("checkPrereqsFn should not be called when resolve is skipped")
return nil
}

Expand Down Expand Up @@ -1989,6 +2002,130 @@ func TestRunUnknownServerToolCallCanonicalizesRelativeSourceKey(t *testing.T) {
}
}

func TestRunUnknownServerToolCallRetriesWithBareRelativeFileSource(t *testing.T) {
tmp := t.TempDir()
xdgConfigHome := filepath.Join(tmp, "xdg-config")
configDir := filepath.Join(xdgConfigHome, "mcpx")
if err := os.MkdirAll(configDir, 0o755); err != nil {
t.Fatalf("MkdirAll(configDir): %v", err)
}
if err := os.WriteFile(filepath.Join(configDir, "config.toml"), []byte(""), 0o600); err != nil {
t.Fatalf("WriteFile(config.toml): %v", err)
}

projectDir := filepath.Join(tmp, "project")
if err := os.MkdirAll(projectDir, 0o755); err != nil {
t.Fatalf("MkdirAll(projectDir): %v", err)
}
manifestPath := filepath.Join(projectDir, "manifest")
if err := os.WriteFile(manifestPath, []byte(`{"mcpServers":{"demo":{"command":"echo","args":["ok"]}}}`), 0o600); err != nil {
t.Fatalf("WriteFile(manifest): %v", err)
}

oldWD, err := os.Getwd()
if err != nil {
t.Fatalf("Getwd(): %v", err)
}
if err := os.Chdir(projectDir); err != nil {
t.Fatalf("Chdir(projectDir): %v", err)
}
defer func() {
_ = os.Chdir(oldWD)
}()

t.Setenv("XDG_CONFIG_HOME", xdgConfigHome)
t.Setenv("HOME", tmp)
t.Setenv("XDG_RUNTIME_DIR", t.TempDir())

oldSpawn := spawnOrConnectFn
oldClient := newDaemonClient
oldResolve := resolveSourceFn
oldCheck := checkPrereqsFn
defer func() {
spawnOrConnectFn = oldSpawn
newDaemonClient = oldClient
resolveSourceFn = oldResolve
checkPrereqsFn = oldCheck
}()

spawnOrConnectFn = func() (string, error) { return "nonce", nil }
expectedServerKey := sourceRequestServerKey("manifest", callerWorkingDirectory())
resolveSourceFn = func(_ context.Context, got string, _ bootstrap.ResolveOptions) (bootstrap.ResolvedServer, error) {
if got != expectedServerKey {
return bootstrap.ResolvedServer{}, fmt.Errorf("unexpected source %q", got)
}
return bootstrap.ResolvedServer{
Server: config.ServerConfig{Command: "echo", Args: []string{"ok"}},
}, nil
}
checkPrereqsFn = func(config.ServerConfig) error { return nil }

var calls int
newDaemonClient = func(_, _ string) daemonRequester {
return stubDaemonClient{
sendFn: func(req *ipc.Request) (*ipc.Response, error) {
calls++
switch calls {
case 1:
if req.Type != "call_tool" {
return nil, errors.New("unexpected request type")
}
if req.Server != expectedServerKey {
return nil, fmt.Errorf("unexpected server key: %q", req.Server)
}
if req.Ephemeral != nil {
return nil, errors.New("unexpected ephemeral payload on first request")
}
return &ipc.Response{
ExitCode: ipc.ExitUsageErr,
ErrorCode: ipc.ErrorCodeUnknownServer,
}, nil
case 2:
if req.Type != "call_tool" {
return nil, errors.New("unexpected retry request type")
}
if req.Server != expectedServerKey {
return nil, fmt.Errorf("unexpected retry server key: %q", req.Server)
}
if req.Ephemeral == nil {
return nil, errors.New("missing ephemeral payload on retry")
}
if req.Ephemeral.Server.Command != "echo" {
return nil, errors.New("unexpected ephemeral command")
}
return &ipc.Response{ExitCode: ipc.ExitOK, Content: []byte("ok\n")}, nil
default:
return nil, errors.New("unexpected extra request")
}
},
}
}

oldOut := rootStdout
oldErr := rootStderr
defer func() {
rootStdout = oldOut
rootStderr = oldErr
}()
var out bytes.Buffer
var errOut bytes.Buffer
rootStdout = &out
rootStderr = &errOut

if code := Run([]string{"manifest", "ping", "{}"}); code != ipc.ExitOK {
t.Fatalf("Run([manifest ping {}]) = %d, want %d (stdout=%q stderr=%q)", code, ipc.ExitOK, out.String(), errOut.String())
}
if out.String() != "ok\n" {
t.Fatalf("stdout = %q, want %q", out.String(), "ok\\n")
}
if errOut.Len() != 0 {
t.Fatalf("stderr = %q, want empty", errOut.String())
}
if calls != 2 {
t.Fatalf("daemon requests = %d, want 2", calls)
}
}

func TestRunUnknownServerToolCallRetriesWithEphemeralSource(t *testing.T) {
tmp := t.TempDir()
xdgConfigHome := filepath.Join(tmp, "xdg-config")
Expand Down Expand Up @@ -2111,18 +2248,12 @@ func TestRunUnknownServerToolCallKeepsUsageErrorForTypoServerName(t *testing.T)
}()

spawnOrConnectFn = func() (string, error) { return "nonce", nil }
resolveSourceFn = func(_ context.Context, source string, _ bootstrap.ResolveOptions) (bootstrap.ResolvedServer, error) {
if source != typoServer {
return bootstrap.ResolvedServer{}, errors.New("unexpected source")
}
return bootstrap.Resolve(context.Background(), source, bootstrap.ResolveOptions{
ReadFile: func(string) ([]byte, error) {
return nil, os.ErrNotExist
},
})
resolveSourceFn = func(context.Context, string, bootstrap.ResolveOptions) (bootstrap.ResolvedServer, error) {
t.Fatal("resolveSourceFn should not be called for implicit unknown-server fallback")
return bootstrap.ResolvedServer{}, nil
}
checkPrereqsFn = func(config.ServerConfig) error {
t.Fatal("checkPrereqsFn should not be called when resolve fails")
t.Fatal("checkPrereqsFn should not be called when resolve is skipped")
return nil
}

Expand Down
Loading