From ba42be8d779a2ed2de85d768ef791790826b47e7 Mon Sep 17 00:00:00 2001 From: thibaud-lclr Date: Tue, 14 Apr 2026 17:09:46 +0200 Subject: [PATCH] refactor(cli): upgrade mcp-framework to v1.3.1 and remove glue code --- go.mod | 2 +- go.sum | 4 +- internal/cli/app.go | 30 +++------------ internal/cli/app_test.go | 2 +- internal/cli/doctor.go | 78 ++++++++++++++++++--------------------- internal/cli/wire.go | 63 ++----------------------------- internal/cli/wire_test.go | 74 +++++++++++-------------------------- 7 files changed, 70 insertions(+), 183 deletions(-) diff --git a/go.mod b/go.mod index 1a77a58..e8006f3 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module email-mcp go 1.25.0 require ( - gitea.lclr.dev/AI/mcp-framework v1.3.0-rc3 + gitea.lclr.dev/AI/mcp-framework v1.3.1 github.com/emersion/go-imap/v2 v2.0.0-beta.8 github.com/emersion/go-message v0.18.2 github.com/godbus/dbus/v5 v5.2.2 diff --git a/go.sum b/go.sum index 5f917b0..cf8a62f 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -gitea.lclr.dev/AI/mcp-framework v1.3.0-rc3 h1:r++aJfZFsp+Bi9fUyqb/KjblciV6I3SdvHT3QbpmPZg= -gitea.lclr.dev/AI/mcp-framework v1.3.0-rc3/go.mod h1:kUVMrL3/UBYgjOsW7sJCs3V0pO0qoJJMpIpueoTsoA4= +gitea.lclr.dev/AI/mcp-framework v1.3.1 h1:GxT5bV22+hbLLUz2IMCscb3qLdkqX5u9d1dbHnUs7RI= +gitea.lclr.dev/AI/mcp-framework v1.3.1/go.mod h1:kUVMrL3/UBYgjOsW7sJCs3V0pO0qoJJMpIpueoTsoA4= github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 h1:/vQbFIOMbk2FiG/kXiLl8BRyzTWDw7gX/Hz7Dd5eDMs= github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4/go.mod h1:hN7oaIRCjzsZ2dE+yG5k+rsdt3qcwykqK6HVGcKwsw4= github.com/99designs/keyring v1.2.2 h1:pZd3neh/EmUzWONb35LxQfvuY7kiSXAq3HQd97+XBn0= diff --git a/internal/cli/app.go b/internal/cli/app.go index 2cf1873..2add796 100644 --- a/internal/cli/app.go +++ b/internal/cli/app.go @@ -538,31 +538,11 @@ func resolveCredentialFields(profile ProfileConfig, store secretStore, fields [] return frameworkcli.ResolveFields(frameworkcli.ResolveOptions{ Fields: fields, - Lookup: func(source frameworkcli.ValueSource, key string) (string, bool, error) { - switch source { - case frameworkcli.SourceEnv: - value, ok := os.LookupEnv(strings.TrimSpace(key)) - return value, ok, nil - case frameworkcli.SourceConfig: - value, ok := configValues[strings.TrimSpace(key)] - return value, ok, nil - case frameworkcli.SourceSecret: - if store == nil { - return "", false, nil - } - - value, err := store.GetSecret(strings.TrimSpace(key)) - if err != nil { - if errors.Is(err, frameworksecretstore.ErrNotFound) { - return "", false, nil - } - return "", false, err - } - return value, true, nil - default: - return "", false, nil - } - }, + Lookup: frameworkcli.ResolveLookup(frameworkcli.ResolveLookupOptions{ + Env: frameworkcli.EnvLookup(os.LookupEnv), + Config: frameworkcli.ConfigMap(configValues), + Secret: frameworkcli.SecretStore(store), + }), }) } diff --git a/internal/cli/app_test.go b/internal/cli/app_test.go index c120d54..1550ab6 100644 --- a/internal/cli/app_test.go +++ b/internal/cli/app_test.go @@ -963,7 +963,7 @@ base_url = "https://gitea.lclr.dev" text := output.String() for _, needle := range []string{ "[OK] config: config file is readable", - "[OK] profile: resolved profile is complete", + "[OK] profile: required profile values are resolved", "[OK] password: stored password is present", "[OK] connectivity: IMAP server is reachable", "Summary: 6 ok, 0 warning(s), 0 failure(s), 6 total", diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index 819bc90..9e6f729 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "os" "path/filepath" "runtime" "strings" @@ -40,7 +41,7 @@ func (a *App) runDoctor(ctx context.Context, args []string) error { }, ConnectivityCheck: a.doctorConnectivityCheck(profileFlag), ExtraChecks: []frameworkcli.DoctorCheck{ - a.doctorProfileCheck(profileFlag), + a.doctorRequiredProfileFieldsCheck(profileFlag), a.doctorPasswordCheck(profileFlag), }, }) @@ -65,53 +66,44 @@ func (a *App) doctorManifestDir() string { return filepath.Dir(executablePath) } -func (a *App) doctorProfileCheck(profileFlag string) frameworkcli.DoctorCheck { - return func(context.Context) frameworkcli.DoctorResult { - cfg, _, err := a.configStore.LoadDefault() - if err != nil { - return frameworkcli.DoctorResult{ - Name: "profile", - Status: frameworkcli.DoctorStatusFail, - Summary: "cannot load profile configuration", - Detail: err.Error(), - } - } +func (a *App) doctorRequiredProfileFieldsCheck(profileFlag string) frameworkcli.DoctorCheck { + var ( + profileValues map[string]string + loadErr error + ) - profileName := a.resolveProfileName(profileFlag, cfg.CurrentProfile) - resolution, err := resolveCredentialFields(cfg.Profiles[profileName], nil, profileFieldSpecs()) - if err != nil { - var missingErr *frameworkcli.MissingRequiredValuesError - if errors.As(err, &missingErr) { - return frameworkcli.DoctorResult{ - Name: "profile", - Status: frameworkcli.DoctorStatusFail, - Summary: "resolved profile is incomplete", - Detail: fmt.Sprintf("profile %q: missing %s", profileName, strings.Join(missingErr.Fields, ", ")), + check := frameworkcli.RequiredResolvedFieldsCheck(frameworkcli.ResolveOptions{ + Fields: profileFieldSpecs(), + Lookup: frameworkcli.ResolveLookup(frameworkcli.ResolveLookupOptions{ + Env: frameworkcli.EnvLookup(os.LookupEnv), + Config: func(key string) (string, bool, error) { + if loadErr != nil { + return "", false, loadErr } - } + if profileValues == nil { + cfg, _, err := a.configStore.LoadDefault() + if err != nil { + loadErr = err + return "", false, loadErr + } - return frameworkcli.DoctorResult{ - Name: "profile", - Status: frameworkcli.DoctorStatusFail, - Summary: "cannot resolve profile values", - Detail: err.Error(), - } - } + profileName := a.resolveProfileName(profileFlag, cfg.CurrentProfile) + profile := cfg.Profiles[profileName] + profileValues = map[string]string{ + "host": profile.Host, + "username": profile.Username, + } + } - host, _ := resolution.Get("host") - username, _ := resolution.Get("username") + return frameworkcli.MapLookup(profileValues)(key) + }, + }), + }) - return frameworkcli.DoctorResult{ - Name: "profile", - Status: frameworkcli.DoctorStatusOK, - Summary: "resolved profile is complete", - Detail: fmt.Sprintf( - "profile %q (host: %s, username: %s)", - profileName, - host.Source, - username.Source, - ), - } + return func(ctx context.Context) frameworkcli.DoctorResult { + result := check(ctx) + result.Name = "profile" + return result } } diff --git a/internal/cli/wire.go b/internal/cli/wire.go index e3d1d1e..1255ea7 100644 --- a/internal/cli/wire.go +++ b/internal/cli/wire.go @@ -2,10 +2,8 @@ package cli import ( "context" - "fmt" "io" "os" - "path/filepath" "strings" frameworkconfig "gitea.lclr.dev/AI/mcp-framework/config" @@ -68,14 +66,10 @@ func (f runtimeFactories) withDefaults() runtimeFactories { } if f.openSecretStore == nil { f.openSecretStore = func() (secretStore, error) { - policy, err := resolveSecretStorePolicy(f.loadManifest, f.resolveExecutable) - if err != nil { - return nil, err - } - - return frameworksecretstore.Open(frameworksecretstore.Options{ - ServiceName: "email-mcp", - BackendPolicy: policy, + return frameworksecretstore.OpenFromManifest(frameworksecretstore.OpenFromManifestOptions{ + ServiceName: "email-mcp", + ManifestLoader: frameworksecretstore.ManifestLoader(f.loadManifest), + ExecutableResolver: frameworksecretstore.ExecutableResolver(f.resolveExecutable), LookupEnv: func(name string) (string, bool) { trimmedName := strings.TrimSpace(name) if strings.HasPrefix(trimmedName, "imap-password/") { @@ -111,52 +105,3 @@ func (s staticCredentialStore) Save(_ context.Context, _ string, _ secretstore.C func (s staticCredentialStore) Load(_ context.Context, _ string) (secretstore.Credential, error) { return s.credential, nil } - -func resolveSecretStorePolicy(loadManifest manifestLoader, resolveExecutable executableResolver) (frameworksecretstore.BackendPolicy, error) { - if loadManifest == nil { - return frameworksecretstore.BackendAuto, nil - } - - searchDirs := []string{"."} - if resolveExecutable != nil { - executablePath, err := resolveExecutable() - if err == nil { - searchDirs = append([]string{filepath.Dir(executablePath)}, searchDirs...) - } - } - - seen := map[string]struct{}{} - for _, dir := range searchDirs { - trimmedDir := strings.TrimSpace(dir) - if trimmedDir == "" { - continue - } - if _, ok := seen[trimmedDir]; ok { - continue - } - seen[trimmedDir] = struct{}{} - - file, _, err := loadManifest(trimmedDir) - if err != nil { - continue - } - return parseSecretBackendPolicy(file.SecretStore.BackendPolicy) - } - - return frameworksecretstore.BackendAuto, nil -} - -func parseSecretBackendPolicy(raw string) (frameworksecretstore.BackendPolicy, error) { - switch strings.TrimSpace(raw) { - case "", string(frameworksecretstore.BackendAuto): - return frameworksecretstore.BackendAuto, nil - case string(frameworksecretstore.BackendKWalletOnly): - return frameworksecretstore.BackendKWalletOnly, nil - case string(frameworksecretstore.BackendKeyringAny): - return frameworksecretstore.BackendKeyringAny, nil - case string(frameworksecretstore.BackendEnvOnly): - return frameworksecretstore.BackendEnvOnly, nil - default: - return "", fmt.Errorf("invalid secret backend policy %q", strings.TrimSpace(raw)) - } -} diff --git a/internal/cli/wire_test.go b/internal/cli/wire_test.go index 1f0fdc3..75e7144 100644 --- a/internal/cli/wire_test.go +++ b/internal/cli/wire_test.go @@ -1,11 +1,10 @@ package cli import ( - "fmt" + "strings" "testing" frameworkmanifest "gitea.lclr.dev/AI/mcp-framework/manifest" - frameworksecretstore "gitea.lclr.dev/AI/mcp-framework/secretstore" ) func TestBuildAppReturnsConfiguredApp(t *testing.T) { @@ -33,56 +32,6 @@ func TestBuildAppReturnsConfiguredApp(t *testing.T) { } } -func TestResolveSecretStorePolicyUsesManifestValue(t *testing.T) { - policy, err := resolveSecretStorePolicy( - func(string) (frameworkmanifest.File, string, error) { - return frameworkmanifest.File{ - SecretStore: frameworkmanifest.SecretStore{ - BackendPolicy: "env-only", - }, - }, "/tmp/mcp.toml", nil - }, - func() (string, error) { return "/tmp/bin/email-mcp", nil }, - ) - if err != nil { - t.Fatalf("resolveSecretStorePolicy returned error: %v", err) - } - if policy != frameworksecretstore.BackendEnvOnly { - t.Fatalf("policy = %q, want %q", policy, frameworksecretstore.BackendEnvOnly) - } -} - -func TestResolveSecretStorePolicyReturnsErrorOnInvalidManifestValue(t *testing.T) { - _, err := resolveSecretStorePolicy( - func(string) (frameworkmanifest.File, string, error) { - return frameworkmanifest.File{ - SecretStore: frameworkmanifest.SecretStore{ - BackendPolicy: "invalid-policy", - }, - }, "/tmp/mcp.toml", nil - }, - func() (string, error) { return "/tmp/bin/email-mcp", nil }, - ) - if err == nil { - t.Fatal("expected invalid secret store policy error") - } -} - -func TestResolveSecretStorePolicyFallsBackToAutoWhenManifestMissing(t *testing.T) { - policy, err := resolveSecretStorePolicy( - func(string) (frameworkmanifest.File, string, error) { - return frameworkmanifest.File{}, "", fmt.Errorf("manifest missing") - }, - func() (string, error) { return "/tmp/bin/email-mcp", nil }, - ) - if err != nil { - t.Fatalf("resolveSecretStorePolicy returned error: %v", err) - } - if policy != frameworksecretstore.BackendAuto { - t.Fatalf("policy = %q, want %q", policy, frameworksecretstore.BackendAuto) - } -} - func TestBuildAppOpenSecretStoreMapsProfilePasswordToEnvironment(t *testing.T) { t.Setenv(passwordEnv, "env-secret") @@ -110,3 +59,24 @@ func TestBuildAppOpenSecretStoreMapsProfilePasswordToEnvironment(t *testing.T) { t.Fatalf("GetSecret = %q, want %q", value, "env-secret") } } + +func TestBuildAppOpenSecretStoreReturnsErrorOnInvalidManifestPolicy(t *testing.T) { + app := buildApp(nil, nil, nil, "dev", runtimeFactories{ + loadManifest: func(string) (frameworkmanifest.File, string, error) { + return frameworkmanifest.File{ + SecretStore: frameworkmanifest.SecretStore{ + BackendPolicy: "invalid-policy", + }, + }, "/tmp/mcp.toml", nil + }, + resolveExecutable: func() (string, error) { return "/tmp/bin/email-mcp", nil }, + }) + + _, err := app.openSecretStore() + if err == nil { + t.Fatal("expected invalid secret store policy error") + } + if !strings.Contains(err.Error(), "invalid secret_store.backend_policy") { + t.Fatalf("unexpected error: %v", err) + } +}