perf: reduce redundant bitwarden CLI calls

This commit is contained in:
thibaud-lclr 2026-04-20 12:03:22 +02:00
parent 98bac84ab8
commit 2920f5980a
4 changed files with 89 additions and 31 deletions

View file

@ -1537,6 +1537,7 @@ import (
"os"
"path/filepath"
"strings"
"sync"
"gitea.lclr.dev/AI/mcp-framework/bootstrap"
"gitea.lclr.dev/AI/mcp-framework/cli"
@ -1816,14 +1817,16 @@ func (r Runtime) runConfigTest(ctx context.Context, inv bootstrap.Invocation) er
stdout = os.Stdout
}
secretStoreFactory := memoizeSecretStoreFactory(r.openSecretStore)
report := cli.RunDoctor(ctx, cli.DoctorOptions{
ConfigCheck: cli.NewConfigCheck(r.ConfigStore),
SecretStoreCheck: cli.SecretStoreAvailabilityCheck(r.openSecretStore),
SecretStoreCheck: cli.SecretStoreAvailabilityCheck(secretStoreFactory),
SecretBackendPolicy: r.activeBackendPolicy(),
RequiredSecrets: []cli.DoctorSecret{
{Name: r.SecretName, Label: "API token"},
},
SecretStoreFactory: r.openSecretStore,
SecretStoreFactory: secretStoreFactory,
ManifestCheck: r.manifestDoctorCheck(),
BitwardenOptions: cli.BitwardenDoctorOptions{
LookupEnv: os.LookupEnv,
@ -1841,6 +1844,25 @@ func (r Runtime) runConfigTest(ctx context.Context, inv bootstrap.Invocation) er
return nil
}
func memoizeSecretStoreFactory(factory func() (secretstore.Store, error)) func() (secretstore.Store, error) {
if factory == nil {
return nil
}
var (
once sync.Once
store secretstore.Store
err error
)
return func() (secretstore.Store, error) {
once.Do(func() {
store, err = factory()
})
return store, err
}
}
func (r Runtime) runUpdate(ctx context.Context, inv bootstrap.Invocation) error {
stdout := inv.Stdout
if stdout == nil {

View file

@ -76,6 +76,8 @@ func TestGenerateCreatesRecommendedSkeleton(t *testing.T) {
"ManifestSource",
"ManifestCheck: r.manifestDoctorCheck()",
"SecretBackendPolicy: r.activeBackendPolicy()",
"secretStoreFactory := memoizeSecretStoreFactory(r.openSecretStore)",
"func memoizeSecretStoreFactory(factory func() (secretstore.Store, error)) func() (secretstore.Store, error) {",
"cli.WriteSetupSecretVerified",
} {
if !strings.Contains(string(appGo), snippet) {

View file

@ -211,7 +211,7 @@ func detectShellFlavor(shellHint string) string {
func (s *bitwardenStore) SetSecret(name, label, secret string) error {
secretName := s.scopedName(name)
item, err := s.findItem(secretName, name)
item, payload, err := s.findItem(secretName, name)
switch {
case errors.Is(err, ErrNotFound):
template, err := s.itemTemplate()
@ -239,11 +239,6 @@ func (s *bitwardenStore) SetSecret(name, label, secret string) error {
return err
}
payload, err := s.itemByID(item.ID)
if err != nil {
return err
}
setBitwardenSecretPayload(payload, s.serviceName, name, secretName, label, secret)
encoded, err := s.encodePayload(payload)
if err != nil {
@ -266,12 +261,7 @@ func (s *bitwardenStore) SetSecret(name, label, secret string) error {
func (s *bitwardenStore) GetSecret(name string) (string, error) {
secretName := s.scopedName(name)
item, err := s.findItem(secretName, name)
if err != nil {
return "", err
}
payload, err := s.itemByID(item.ID)
_, payload, err := s.findItem(secretName, name)
if err != nil {
return "", err
}
@ -286,7 +276,7 @@ func (s *bitwardenStore) GetSecret(name string) (string, error) {
func (s *bitwardenStore) DeleteSecret(name string) error {
secretName := s.scopedName(name)
item, err := s.findItem(secretName, name)
item, _, err := s.findItem(secretName, name)
if errors.Is(err, ErrNotFound) {
return nil
}
@ -311,7 +301,12 @@ func (s *bitwardenStore) scopedName(name string) string {
return fmt.Sprintf("%s/%s", s.serviceName, name)
}
func (s *bitwardenStore) findItem(secretName, rawSecretName string) (bitwardenListItem, error) {
type bitwardenResolvedItem struct {
item bitwardenListItem
payload map[string]any
}
func (s *bitwardenStore) findItem(secretName, rawSecretName string) (bitwardenListItem, map[string]any, error) {
output, err := s.execute(
fmt.Sprintf("list bitwarden items for secret %q", secretName),
nil,
@ -321,15 +316,15 @@ func (s *bitwardenStore) findItem(secretName, rawSecretName string) (bitwardenLi
secretName,
)
if err != nil {
return bitwardenListItem{}, err
return bitwardenListItem{}, nil, err
}
if strings.TrimSpace(string(output)) == "" {
return bitwardenListItem{}, ErrNotFound
return bitwardenListItem{}, nil, ErrNotFound
}
var items []bitwardenListItem
if err := json.Unmarshal(output, &items); err != nil {
return bitwardenListItem{}, fmt.Errorf("decode bitwarden items for secret %q: %w", secretName, err)
return bitwardenListItem{}, nil, fmt.Errorf("decode bitwarden items for secret %q: %w", secretName, err)
}
matches := make([]bitwardenListItem, 0, len(items))
@ -344,42 +339,47 @@ func (s *bitwardenStore) findItem(secretName, rawSecretName string) (bitwardenLi
}
if len(matches) == 0 {
return bitwardenListItem{}, ErrNotFound
return bitwardenListItem{}, nil, ErrNotFound
}
markedMatches := make([]bitwardenListItem, 0, len(matches))
legacyMatches := make([]bitwardenListItem, 0, len(matches))
markedMatches := make([]bitwardenResolvedItem, 0, len(matches))
legacyMatches := make([]bitwardenResolvedItem, 0, len(matches))
for _, item := range matches {
payload, err := s.itemByID(item.ID)
if err != nil {
return bitwardenListItem{}, err
return bitwardenListItem{}, nil, err
}
resolved := bitwardenResolvedItem{
item: item,
payload: payload,
}
if bitwardenItemMatchesMarkers(payload, s.serviceName, rawSecretName) {
markedMatches = append(markedMatches, item)
markedMatches = append(markedMatches, resolved)
continue
}
legacyMatches = append(legacyMatches, item)
legacyMatches = append(legacyMatches, resolved)
}
switch len(markedMatches) {
case 0:
switch len(legacyMatches) {
case 0:
return bitwardenListItem{}, ErrNotFound
return bitwardenListItem{}, nil, ErrNotFound
case 1:
return legacyMatches[0], nil
return legacyMatches[0].item, legacyMatches[0].payload, nil
default:
return bitwardenListItem{}, fmt.Errorf(
return bitwardenListItem{}, nil, fmt.Errorf(
"multiple legacy bitwarden items match secret %q for service %q",
secretName,
s.serviceName,
)
}
case 1:
return markedMatches[0], nil
return markedMatches[0].item, markedMatches[0].payload, nil
default:
return bitwardenListItem{}, fmt.Errorf(
return bitwardenListItem{}, nil, fmt.Errorf(
"multiple bitwarden items share marker for secret %q and service %q",
secretName,
s.serviceName,

View file

@ -337,6 +337,38 @@ func TestBitwardenStoreFallsBackToSingleLegacyItemWithoutMarkers(t *testing.T) {
}
}
func TestBitwardenStoreGetSecretReadsSelectedItemOnlyOnce(t *testing.T) {
withBitwardenSession(t)
fakeCLI := newFakeBitwardenCLI("bw")
fakeCLI.itemsByID["item-1"] = fakeBitwardenItem{
ID: "item-1",
Name: "email-mcp/api-token",
Secret: "secret-v1",
MarkerService: "email-mcp",
MarkerSecretName: "api-token",
}
withBitwardenRunner(t, fakeCLI.run)
store, err := Open(Options{
ServiceName: "email-mcp",
BackendPolicy: BackendBitwardenCLI,
})
if err != nil {
t.Fatalf("Open returned error: %v", err)
}
value, err := store.GetSecret("api-token")
if err != nil {
t.Fatalf("GetSecret returned error: %v", err)
}
if value != "secret-v1" {
t.Fatalf("GetSecret = %q, want secret-v1", value)
}
if fakeCLI.getItemCalls != 1 {
t.Fatalf("bw get item count = %d, want 1", fakeCLI.getItemCalls)
}
}
func TestBitwardenFindItemTreatsEmptyListOutputAsNotFound(t *testing.T) {
store := &bitwardenStore{command: "bw", serviceName: "email-mcp"}
withBitwardenRunner(t, func(command string, stdin []byte, args ...string) ([]byte, error) {
@ -346,7 +378,7 @@ func TestBitwardenFindItemTreatsEmptyListOutputAsNotFound(t *testing.T) {
return nil, fmt.Errorf("unexpected args: %v", args)
})
_, err := store.findItem("email-mcp/api-token", "api-token")
_, _, err := store.findItem("email-mcp/api-token", "api-token")
if !errors.Is(err, ErrNotFound) {
t.Fatalf("error = %v, want ErrNotFound", err)
}
@ -527,6 +559,7 @@ type fakeBitwardenCLI struct {
status string
versionChecked bool
statusChecked bool
getItemCalls int
}
type fakeBitwardenItem struct {
@ -568,6 +601,7 @@ func (f *fakeBitwardenCLI) run(command string, stdin []byte, args ...string) ([]
case len(args) == 4 && args[0] == "list" && args[1] == "items" && args[2] == "--search":
return f.handleListItems(args[3])
case len(args) == 3 && args[0] == "get" && args[1] == "item":
f.getItemCalls++
return f.handleGetItem(args[2])
case len(args) == 3 && args[0] == "get" && args[1] == "template" && args[2] == "item":
return []byte(`{"type":2,"name":"","notes":"","secureNote":{"type":0},"fields":[]}`), nil