Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 2 additions & 21 deletions internal/provider/catalog/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,11 +147,7 @@ func (s *Service) loadCatalog(ctx context.Context, identity provider.ProviderIde
}

func (s *Service) discoverAndPersist(ctx context.Context, input provider.CatalogInput) ([]providertypes.ModelDescriptor, error) {
supported, err := s.supportsModelDiscovery(input.Identity.Driver)
if err != nil {
return nil, err
}
if !supported {
if !s.registry.Supports(input.Identity.Driver) {

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

return nil, nil
}

Expand Down Expand Up @@ -190,8 +186,7 @@ func (s *Service) queueRefresh(input provider.CatalogInput) {
return
}

supported, err := s.supportsModelDiscovery(input.Identity.Driver)
if err != nil || !supported {
if !s.registry.Supports(input.Identity.Driver) {

This comment was marked as outdated.

return
}
identity := input.Identity
Expand Down Expand Up @@ -221,20 +216,6 @@ func (s *Service) queueRefresh(input provider.CatalogInput) {
}()
}

func (s *Service) supportsModelDiscovery(driverType string) (bool, error) {
driverType = strings.TrimSpace(driverType)
if !s.registry.Supports(driverType) {
return false, nil
}

caps, err := s.registry.DriverTransportCapabilities(driverType)
if err != nil {
return false, err
}

return caps.ModelDiscovery, nil
}

func shouldQueueRefresh(options queryOptions, snapshot catalogSnapshot, performedSyncRefresh bool) bool {
if !options.queueRefresh {
return false
Expand Down
148 changes: 43 additions & 105 deletions internal/provider/catalog/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -363,90 +363,6 @@ func TestListProviderModelsCachedUsesFreshCatalogWithoutDiscovery(t *testing.T)
}
}

func TestListProviderModelsSkipsDiscoveryWhenDriverDisablesModelDiscovery(t *testing.T) {
t.Setenv(testAPIKeyEnv, "")

var resolveCalls int32
var discoverCalls int32
registry := newRegistryWithCapabilities(
t,
openaicompat.DriverName,
provider.DriverTransportCapabilities{
Streaming: true,
ToolTransport: true,
ModelDiscovery: false,
ImageInputTransport: false,
},
func(ctx context.Context, cfg provider.RuntimeConfig) ([]providertypes.ModelDescriptor, error) {
atomic.AddInt32(&discoverCalls, 1)
return []providertypes.ModelDescriptor{{ID: "server-model", Name: "Server Model"}}, nil
},
)

service := NewService("", registry, newMemoryStore())
input := openAIProviderSource()
input.ResolveDiscoveryConfig = func() (provider.RuntimeConfig, error) {
atomic.AddInt32(&resolveCalls, 1)
return provider.RuntimeConfig{}, errors.New("resolver should not run")
}

models, err := service.ListProviderModels(context.Background(), input)
if err != nil {
t.Fatalf("ListProviderModels() error = %v", err)
}
if len(models) != 1 || models[0].ID != config.OpenAIDefaultModel {
t.Fatalf("expected builtin default model fallback, got %+v", models)
}
if atomic.LoadInt32(&resolveCalls) != 0 {
t.Fatalf("expected discovery config resolver to be skipped, got %d calls", resolveCalls)
}
if atomic.LoadInt32(&discoverCalls) != 0 {
t.Fatalf("expected discovery func to be skipped, got %d calls", discoverCalls)
}
}

func TestListProviderModelsCustomProviderSkipsDiscoveryWhenDriverDisablesModelDiscovery(t *testing.T) {
t.Setenv(testAPIKeyEnv, "")

var resolveCalls int32
var discoverCalls int32
registry := newRegistryWithCapabilities(
t,
openaicompat.DriverName,
provider.DriverTransportCapabilities{
Streaming: true,
ToolTransport: true,
ModelDiscovery: false,
ImageInputTransport: false,
},
func(ctx context.Context, cfg provider.RuntimeConfig) ([]providertypes.ModelDescriptor, error) {
atomic.AddInt32(&discoverCalls, 1)
return []providertypes.ModelDescriptor{{ID: "server-model", Name: "Server Model"}}, nil
},
)

service := NewService("", registry, newMemoryStore())
input := customGatewayProviderSource()
input.ResolveDiscoveryConfig = func() (provider.RuntimeConfig, error) {
atomic.AddInt32(&resolveCalls, 1)
return provider.RuntimeConfig{}, errors.New("resolver should not run")
}

models, err := service.ListProviderModels(context.Background(), input)
if err != nil {
t.Fatalf("ListProviderModels() error = %v", err)
}
if len(models) != 0 {
t.Fatalf("expected custom provider without discovery support to stay empty, got %+v", models)
}
if atomic.LoadInt32(&resolveCalls) != 0 {
t.Fatalf("expected discovery config resolver to be skipped, got %d calls", resolveCalls)
}
if atomic.LoadInt32(&discoverCalls) != 0 {
t.Fatalf("expected discovery func to be skipped, got %d calls", discoverCalls)
}
}

func TestDiscoverAndPersistFailurePaths(t *testing.T) {
t.Run("unsupported driver", func(t *testing.T) {
service := NewService("", provider.NewRegistry(), newMemoryStore())
Expand Down Expand Up @@ -558,33 +474,55 @@ func TestQueueRefreshDeduplicatesInFlightRequests(t *testing.T) {
t.Fatal("expected in-flight refresh marker to be cleared")
}

func newRegistry(t *testing.T, name string, discover provider.DiscoveryFunc) *provider.Registry {
return newRegistryWithCapabilities(
t,
name,
provider.DriverTransportCapabilities{
Streaming: true,
ToolTransport: true,
ModelDiscovery: true,
ImageInputTransport: false,
},
discover,
)
func TestQueueRefreshSkipsWhenDriverUnsupportedOrIdentityIncomplete(t *testing.T) {
t.Parallel()

t.Run("unsupported driver", func(t *testing.T) {
t.Parallel()

service := NewService("", provider.NewRegistry(), newMemoryStore())
input := openAIProviderSource()
input.Identity.Driver = "missing"

service.queueRefresh(input)

service.refreshMu.Lock()
defer service.refreshMu.Unlock()
if len(service.inFlightByID) != 0 {
t.Fatalf("expected no refresh to be queued, got %+v", service.inFlightByID)
}
})

t.Run("incomplete identity", func(t *testing.T) {
t.Parallel()

service := NewService("", newRegistry(t, openaicompat.DriverName, nil), newMemoryStore())
input := openAIProviderSource()
input.Identity.BaseURL = ""

service.queueRefresh(input)

service.refreshMu.Lock()
defer service.refreshMu.Unlock()
if len(service.inFlightByID) != 0 {
t.Fatalf("expected no refresh to be queued, got %+v", service.inFlightByID)
}
})
}

func newRegistryWithCapabilities(
t *testing.T,
name string,
capabilities provider.DriverTransportCapabilities,
discover provider.DiscoveryFunc,
) *provider.Registry {
func newRegistry(t *testing.T, name string, discover provider.DiscoveryFunc) *provider.Registry {
t.Helper()

if discover == nil {
discover = func(ctx context.Context, cfg provider.RuntimeConfig) ([]providertypes.ModelDescriptor, error) {
return nil, nil
}
}

registry := provider.NewRegistry()
if err := registry.Register(provider.DriverDefinition{
Name: name,
Discover: discover,
Capabilities: capabilities,
Name: name,
Discover: discover,
Build: func(ctx context.Context, cfg provider.RuntimeConfig) (provider.Provider, error) {
return catalogTestProvider{}, nil
},
Expand Down
8 changes: 0 additions & 8 deletions internal/provider/contracts.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,6 @@ type Provider interface {
Generate(ctx context.Context, req providertypes.GenerateRequest, events chan<- providertypes.StreamEvent) error
}

// DriverTransportCapabilities 描述 driver 层本身是否能传输某类运行时能力。
type DriverTransportCapabilities struct {
Streaming bool
ToolTransport bool
ModelDiscovery bool
ImageInputTransport bool
}

// CatalogInput 汇总 provider/catalog 查询、发现与缓存所需的最小输入。
type CatalogInput struct {
Identity ProviderIdentity
Expand Down
6 changes: 0 additions & 6 deletions internal/provider/openaicompat/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,5 @@ func driverDefinition(name string) provider.DriverDefinition {
return p.DiscoverModels(ctx)
},
ValidateCatalogIdentity: validateCatalogIdentity,
Capabilities: provider.DriverTransportCapabilities{
Streaming: true,
ToolTransport: true,
ModelDiscovery: true,
ImageInputTransport: false,
},
}
}
16 changes: 3 additions & 13 deletions internal/provider/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ type DriverDefinition struct {
Build Builder
Discover DiscoveryFunc
ValidateCatalogIdentity func(identity ProviderIdentity) error
Capabilities DriverTransportCapabilities
}

type Registry struct {
Expand All @@ -43,6 +42,9 @@ func (r *Registry) Register(driver DriverDefinition) error {
if driver.Build == nil {
return fmt.Errorf("provider: driver %q build func is nil", driver.Name)
}
if driver.Discover == nil {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hard-requiring Discover here removes the only way to represent a generate-only driver. After this change, callers have to register a stub discover function, and catalog.Service will treat the driver as discovery-capable on every cache miss/background refresh. That reintroduces ResolveDiscoveryConfig/API-key failures for providers that should have stayed on configured or default models, which is exactly the behavior the deleted catalog tests were protecting against.

return fmt.Errorf("provider: driver %q discover func is nil", driver.Name)
}
if _, exists := r.drivers[driverType]; exists {
return fmt.Errorf("%w: %s", ErrDriverAlreadyRegistered, driver.Name)
}
Expand All @@ -63,9 +65,6 @@ func (r *Registry) DiscoverModels(ctx context.Context, cfg RuntimeConfig) ([]pro
if err != nil {
return nil, err
}
if driver.Discover == nil {
return nil, nil
}
return driver.Discover(ctx, cfg)
}

Expand All @@ -74,15 +73,6 @@ func (r *Registry) Supports(driverType string) bool {
return err == nil
}

// DriverTransportCapabilities 返回指定 driver 的传输能力声明;driver 不存在时返回对应错误。
func (r *Registry) DriverTransportCapabilities(driverType string) (DriverTransportCapabilities, error) {
driver, err := r.driver(driverType)
if err != nil {
return DriverTransportCapabilities{}, err
}
return driver.Capabilities, nil
}

// ValidateCatalogIdentity 在读取 catalog 快照或执行默认模型回退前执行无需密钥的静态校验,避免无效配置被误判为可用。
func (r *Registry) ValidateCatalogIdentity(identity ProviderIdentity) error {
if r == nil {
Expand Down
70 changes: 22 additions & 48 deletions internal/provider/registry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ func stubDriver(driverType string) provider.DriverDefinition {
Build: func(ctx context.Context, cfg provider.RuntimeConfig) (provider.Provider, error) {
return stubProvider{}, nil
},
Discover: func(ctx context.Context, cfg provider.RuntimeConfig) ([]providertypes.ModelDescriptor, error) {
return nil, nil
},
}
}

Expand Down Expand Up @@ -111,30 +114,6 @@ func TestRegistryDiscoverModels(t *testing.T) {
}
})

t.Run("driver without discovery function", func(t *testing.T) {
t.Parallel()

registry := provider.NewRegistry()
driver := provider.DriverDefinition{
Name: "test-driver",
Build: func(ctx context.Context, cfg provider.RuntimeConfig) (provider.Provider, error) {
return stubProvider{}, nil
},
Discover: nil,
}
if err := registry.Register(driver); err != nil {
t.Fatalf("Register() error = %v", err)
}

got, err := registry.DiscoverModels(context.Background(), provider.RuntimeConfig{Driver: "test-driver"})
if err != nil {
t.Fatalf("DiscoverModels() error = %v", err)
}
if got != nil {
t.Fatalf("expected nil models, got %v", got)
}
})

t.Run("unknown driver", func(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -176,30 +155,6 @@ func TestRegistrySupports(t *testing.T) {
}
}

func TestRegistryDriverTransportCapabilities(t *testing.T) {
t.Parallel()

registry := newTestRegistry(t)
got, err := registry.DriverTransportCapabilities("OPENAICOMPAT")
if err != nil {
t.Fatalf("DriverTransportCapabilities() error = %v", err)
}
if !got.Streaming {
t.Fatalf("expected openaicompat driver to support streaming, got %+v", got)
}
if !got.ToolTransport {
t.Fatalf("expected openaicompat driver to support tool transport, got %+v", got)
}
if !got.ModelDiscovery {
t.Fatalf("expected openaicompat driver to support model discovery, got %+v", got)
}

_, err = registry.DriverTransportCapabilities("missing")
if !errors.Is(err, provider.ErrDriverNotFound) {
t.Fatalf("expected ErrDriverNotFound for missing driver, got %v", err)
}
}

func TestRegistryValidateCatalogIdentity(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -277,6 +232,25 @@ func TestRegistryRegisterErrors(t *testing.T) {
t.Fatalf("unexpected error: %v", err)
}
})

t.Run("nil discover function", func(t *testing.T) {
t.Parallel()

registry := provider.NewRegistry()
err := registry.Register(provider.DriverDefinition{
Name: "test",
Build: func(ctx context.Context, cfg provider.RuntimeConfig) (provider.Provider, error) {
return nil, nil
},
Discover: nil,
})
if err == nil {
t.Fatal("expected error for nil discover function")
}
if err.Error() != `provider: driver "test" discover func is nil` {
t.Fatalf("unexpected error: %v", err)
}
})
}

func TestRegistryDriverWithNilMap(t *testing.T) {
Expand Down
3 changes: 0 additions & 3 deletions internal/runtime/compact_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,6 @@ func (g *compactSummaryGenerator) Generate(ctx context.Context, input contextcom
strings.TrimSpace(g.providerConfig.APIKey) == "" {
return "", errors.New("runtime: compact summary generator provider config is incomplete")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The same regression exists in compact generation: without the old streaming capability check, we now invoke the provider and only fail after the call returns, often as a generic stream/protocol error. That makes unsupported drivers look like flaky runtime failures instead of a clear configuration/contract problem.

}
if err := ensureDriverTransportCapabilities(g.providerFactory, g.providerConfig, true, false); err != nil {
return "", err
}

prompt := agentcontext.BuildCompactPrompt(agentcontext.CompactPromptInput{
Mode: string(input.Mode),
Expand Down
Loading
Loading