From 5ba793daba83935e9061dd9d7a4968c8f107fec6 Mon Sep 17 00:00:00 2001 From: saladday <1203511142@qq.com> Date: Wed, 30 Sep 2026 08:39:50 +0000 Subject: [PATCH] Validate Provider registrations before use --- CONTRIBUTING.md | 1 + docs/sandbox-provider.md | 13 ++ .../server/managed_setup_preflight_test.go | 37 ++++ .../core/cmd/specification-contract/main.go | 6 +- .../providers/deployment_contract_test.go | 5 +- .../internal/sandbox/providers/operations.go | 3 + .../sandbox/providers/operations_test.go | 3 +- .../sandbox/providers/registration.go | 60 ++++++ .../providers/registration_configuration.go | 75 +++++++ .../registration_configuration_test.go | 152 ++++++++++++++ .../sandbox/providers/registration_test.go | 189 ++++++++++++++++++ .../internal/sandbox/providers/registry.go | 14 +- .../sandbox/providers/registry_test.go | 18 +- .../sandbox/providers/specification.go | 6 +- 14 files changed, 561 insertions(+), 21 deletions(-) create mode 100644 services/core/internal/sandbox/providers/registration.go create mode 100644 services/core/internal/sandbox/providers/registration_configuration.go create mode 100644 services/core/internal/sandbox/providers/registration_configuration_test.go create mode 100644 services/core/internal/sandbox/providers/registration_test.go diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e851e87cd..9d0c8ed6b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -20,6 +20,7 @@ This guide owns how to work in the repository: documentation ownership, the repo | Harness qualification and acceptance | [Harness integration](contracts/agents-api/harnesses.md) | | Harness service qualification declarations and registration | [Explicit service qualification](contracts/agents-api/harness-onboarding.md#explicit-service-qualification) and `services/core/internal/engine/profile.go` | | Harness selection and Agent defaults | [Harness selection](contracts/agents-api/harness-selection.md) | +| Provider registration validation | [Sandbox Provider guide](docs/sandbox-provider.md#registration-validation) | | Provider selection, sandbox deployment and E2B setup | [Sandbox deployment](contracts/agents-api/sandbox-deployment.md) | | Hosted sandbox nodes | [Nodes guide](docs/getting-started/nodes.md) and [sandbox deployment contract](contracts/agents-api/sandbox-deployment.md) | | Claude private bridge and Runtime artifact | [Claude SDK adapter](packages/claude-sdk-adapter/README.md) | diff --git a/docs/sandbox-provider.md b/docs/sandbox-provider.md index f2bfba3b5..ba34ed313 100644 --- a/docs/sandbox-provider.md +++ b/docs/sandbox-provider.md @@ -208,6 +208,19 @@ mode, defaults, the adapter-owned operation declaration, and local or direct con direct adapters. Neither allocates compute. There is no init-time registration or runtime plugin loading. +### Registration validation + +`providers.ValidateRegistration` is the single wiring check. Lookup, constructor binding and the installer projection use it before configuration callbacks or constructors can run. Unknown provider names remain invalid input; malformed registered adapters return a safe `providercontract.ErrContract`, without including submitted configuration or native diagnostics. + +- A `nodes` registration requires only `BuildLocal`; a `direct` registration requires only `BuildDirect`. Missing, mixed or unknown modes are rejected. +- Specification/resource validators, the configuration adapter and the complete operation declaration are mandatory. An incomplete registration cannot publish a partial installer projection. +- The Runtime input policy must either accept the pinned Runtime or give the adapter's fixed reason for rejecting it; it cannot do both. +- Current checkpoint suspension requires node mode and positive idle/retention defaults that fit Runtime durations. The two durations are independent. Providers without checkpoint support must not configure suspension defaults. + +The configuration adapter must be non-nil, including its concrete value. Each `ConfigurationRequirements` field needs an explicit valid decision: `Credential` and `PublicOrigin` use `Required` or `NotRequired`; `Discovery` uses the shared supported/Unsupported declaration with its safe reason. `ConfigurationDiscoverer` must be implemented even when discovery is unsupported. New requirement fields or discovery methods require an explicit validation update; they cannot inherit an existing decision. Configuration discovery is distinct from resource selection discovery. Requiring a credential does not itself promise the resource operation `VerifyCredential`. + +These checks establish registration completeness, not the correctness of native SDK behavior. Constructor and adapter contract tests still apply. + For a new implementation: 1. Implement the operation contracts above in the adapter package and add native contract tests. diff --git a/services/core/cmd/server/managed_setup_preflight_test.go b/services/core/cmd/server/managed_setup_preflight_test.go index 351202791..cc9e98477 100644 --- a/services/core/cmd/server/managed_setup_preflight_test.go +++ b/services/core/cmd/server/managed_setup_preflight_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/execution" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sandbox" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sandbox/e2b" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" @@ -148,3 +149,39 @@ func TestInitialE2BPublicTemplateOutsideTeamIsRejected(t *testing.T) { t.Fatal("public readability accepted as team ownership", err) } } + +func TestManagedSetupRejectsUnknownRegistrationBeforePreparationOrRouting(t *testing.T) { + // No store or node hub is available: rejection must precede any use of them. + s := &managedSetup{installationID: "installation", publicURL: "http://127.0.0.1"} + setup := store.SandboxSetup{InstallationID: "installation", Provider: "missing-registration"} + for _, call := range []struct { + name string + run func() (execution.PreparedRuntimeDeployment, error) + }{ + {"prepare", func() (execution.PreparedRuntimeDeployment, error) { + return s.prepare(t.Context(), setup) + }}, + {"route", func() (execution.PreparedRuntimeDeployment, error) { + return s.routeGenerations(execution.PreparedRuntimeDeployment{}, setup) + }}, + } { + t.Run(call.name, func(t *testing.T) { + candidate, err := call.run() + if !errors.Is(err, sandbox.ErrInvalid) || candidate.Config != nil || s.selected.Load() != nil { + t.Fatalf("invalid registration reached preparation or publication: %v", err) + } + }) + } +} + +func TestManagedSetupRoutesProviderWithoutCredentialRequirement(t *testing.T) { + s := &managedSetup{} + config := &execution.RuntimeProvider{ProviderKind: "docker"} + candidate, err := s.routeGenerations( + execution.PreparedRuntimeDeployment{Config: config}, + store.SandboxSetup{Provider: "docker"}, + ) + if err != nil || candidate.Config != config || candidate.FenceCredential != nil || candidate.VerifyCredential != nil { + t.Fatalf("explicit no-credential provider required credential routing: %v", err) + } +} diff --git a/services/core/cmd/specification-contract/main.go b/services/core/cmd/specification-contract/main.go index 9d52abc27..3d02f3c97 100644 --- a/services/core/cmd/specification-contract/main.go +++ b/services/core/cmd/specification-contract/main.go @@ -12,7 +12,11 @@ import ( func main() { write := flag.Bool("write", false, "update deploy/install/node_spec.py from the repository root") flag.Parse() - projection := providers.PythonDeploymentContract() + projection, err := providers.PythonDeploymentContract() + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } if !*write { fmt.Println(projection) return diff --git a/services/core/internal/sandbox/providers/deployment_contract_test.go b/services/core/internal/sandbox/providers/deployment_contract_test.go index a3cd57f20..990c48f3c 100644 --- a/services/core/internal/sandbox/providers/deployment_contract_test.go +++ b/services/core/internal/sandbox/providers/deployment_contract_test.go @@ -14,7 +14,10 @@ func TestInstallerDeploymentProjectionIsCurrent(t *testing.T) { if err != nil { t.Fatal(err) } - expected := PythonDeploymentContract() + expected, err := PythonDeploymentContract() + if err != nil { + t.Fatal(err) + } if !strings.Contains(string(raw), expected) { t.Fatal("node_spec.py contract is stale; regenerate with go run ./services/core/cmd/specification-contract -write") } diff --git a/services/core/internal/sandbox/providers/operations.go b/services/core/internal/sandbox/providers/operations.go index 566f8d040..01e786689 100644 --- a/services/core/internal/sandbox/providers/operations.go +++ b/services/core/internal/sandbox/providers/operations.go @@ -8,6 +8,9 @@ import ( // ValidateBinding catches construction that disagrees with its registration. func ValidateBinding(adapter Adapter, provider sandbox.SandboxProvider) error { + if err := ValidateRegistration(adapter); err != nil { + return err + } if err := sandbox.ValidateProvider(provider); err != nil { return err } diff --git a/services/core/internal/sandbox/providers/operations_test.go b/services/core/internal/sandbox/providers/operations_test.go index 8f4455c42..b02ef060f 100644 --- a/services/core/internal/sandbox/providers/operations_test.go +++ b/services/core/internal/sandbox/providers/operations_test.go @@ -4,7 +4,6 @@ import ( "errors" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/providercontract" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sandbox/docker" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sandbox/e2b" "testing" ) @@ -16,7 +15,7 @@ func TestRegistrationRejectsMissingAndMismatchedDeclarations(t *testing.T) { } } delete(adapters, "invalid-contract-fixture") - if err := ValidateBinding(Adapter{Operations: e2b.Operations}, &docker.Provider{}); !errors.Is(err, providercontract.ErrContract) { + if err := ValidateBinding(adapters["e2b"], &docker.Provider{}); !errors.Is(err, providercontract.ErrContract) { t.Fatal("registration differs from instance", err) } } diff --git a/services/core/internal/sandbox/providers/registration.go b/services/core/internal/sandbox/providers/registration.go new file mode 100644 index 000000000..66ea85dde --- /dev/null +++ b/services/core/internal/sandbox/providers/registration.go @@ -0,0 +1,60 @@ +package providers + +import ( + "fmt" + "time" + + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/providercontract" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sandbox" +) + +// ValidateRegistration checks wiring before configuration parsing or construction. +// Only configuration requirements and operation declarations are read. +func ValidateRegistration(a Adapter) error { + invalid := func(field string) error { + return fmt.Errorf("%w: invalid registration %s", providercontract.ErrContract, field) + } + switch a.Mode { + case "nodes": + if a.BuildLocal == nil || a.BuildDirect != nil { + return invalid("node constructor") + } + case "direct": + if a.BuildDirect == nil || a.BuildLocal != nil { + return invalid("direct constructor") + } + default: + return invalid("mode") + } + if a.ValidateSpecification == nil { + return invalid("specification validator") + } + if a.ValidateResources == nil { + return invalid("resource validator") + } + if err := validateConfigurationAdapter(a.Configuration); err != nil { + return err + } + if a.Policy.Runtime == (a.Policy.RuntimeError != "") { + return invalid("Runtime input policy") + } + if a.Operations == nil { + return invalid("operation declaration") + } + operations := a.Operations() + if err := sandbox.ValidateOperations(operations); err != nil { + return err + } + // The current common lifecycle admits checkpoint suspension only on nodes, + // and creates its policy whenever checkpoint support is declared. + if operations["Initial"].State == providercontract.Supported { + const maximumSeconds = int64((1<<63 - 1) / time.Second) + if a.Mode != "nodes" || a.IdleSeconds < 1 || a.RetentionSeconds < 1 || + a.IdleSeconds > maximumSeconds || a.RetentionSeconds > maximumSeconds { + return invalid("checkpoint policy") + } + } else if a.IdleSeconds != 0 || a.RetentionSeconds != 0 { + return invalid("non-checkpoint policy") + } + return nil +} diff --git a/services/core/internal/sandbox/providers/registration_configuration.go b/services/core/internal/sandbox/providers/registration_configuration.go new file mode 100644 index 000000000..3d8a5dd54 --- /dev/null +++ b/services/core/internal/sandbox/providers/registration_configuration.go @@ -0,0 +1,75 @@ +package providers + +import ( + "errors" + "fmt" + "reflect" + + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/providercontract" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sandbox" +) + +func configurationRegistrationError() error { + return fmt.Errorf("%w: invalid configuration registration", providercontract.ErrContract) +} + +func validateConfigurationAdapter(configuration sandbox.ConfigurationAdapter) error { + if configuration == nil { + return configurationRegistrationError() + } + value := reflect.ValueOf(configuration) + switch value.Kind() { + case reflect.Chan, reflect.Func, reflect.Interface, reflect.Map, reflect.Pointer, reflect.Slice: + if value.IsNil() { + return configurationRegistrationError() + } + } + discovery := reflect.TypeFor[sandbox.ConfigurationDiscoverer]() + if !value.Type().Implements(discovery) { + return configurationRegistrationError() + } + if err := validateConfigurationDiscoveryInterface(discovery); err != nil { + return err + } + return validateConfigurationRequirements(reflect.ValueOf(configuration.Requirements())) +} + +// Every discovery method needs its own authored requirement. A future method +// cannot inherit the existing Discovery decision merely by being implemented. +func validateConfigurationDiscoveryInterface(discovery reflect.Type) error { + if discovery.NumMethod() != 1 { + return configurationRegistrationError() + } + if _, exists := discovery.MethodByName("DiscoverConfiguration"); !exists { + return configurationRegistrationError() + } + return nil +} + +// Check field names as well as values so new requirements cannot bypass the +// gate. This owns only configuration requirements, not resource operations. +func validateConfigurationRequirements(value reflect.Value) error { + if value.Kind() != reflect.Struct || value.NumField() != 3 { + return configurationRegistrationError() + } + for i := 0; i < value.NumField(); i++ { + switch value.Type().Field(i).Name { + case "Credential", "PublicOrigin": + requirement, ok := value.Field(i).Interface().(sandbox.Requirement) + if !ok || (requirement != sandbox.Required && requirement != sandbox.NotRequired) { + return configurationRegistrationError() + } + case "Discovery": + support, ok := value.Field(i).Interface().(providercontract.Support) + if !ok { + return configurationRegistrationError() + } + if err := support.Check("DiscoverConfiguration"); err != nil && !errors.Is(err, providercontract.ErrUnsupported) { + return configurationRegistrationError() + } + default: + return configurationRegistrationError() + } + } + return nil +} diff --git a/services/core/internal/sandbox/providers/registration_configuration_test.go b/services/core/internal/sandbox/providers/registration_configuration_test.go new file mode 100644 index 000000000..760b1f704 --- /dev/null +++ b/services/core/internal/sandbox/providers/registration_configuration_test.go @@ -0,0 +1,152 @@ +package providers + +import ( + "context" + "errors" + "reflect" + "testing" + + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/providercontract" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sandbox" +) + +// Only declaration reads are safe on this fixture. Any configuration or native +// call through the nil embedded interfaces fails the test immediately. +type registrationConfiguration struct { + sandbox.ConfigurationAdapter + sandbox.ConfigurationDiscoverer + requirements sandbox.ConfigurationRequirements +} + +func (a registrationConfiguration) Requirements() sandbox.ConfigurationRequirements { + return a.requirements +} + +// A declaration of Unsupported still requires an explicit rejection method. +type missingConfigurationDiscovery struct{ sandbox.ConfigurationAdapter } + +func TestConfigurationRegistrationRejectsNilAndMissingDiscovery(t *testing.T) { + a := adapters["docker"] + var typedNil *registrationConfiguration + for _, configuration := range []sandbox.ConfigurationAdapter{ + nil, typedNil, missingConfigurationDiscovery{a.Configuration}, + missingConfigurationDiscovery{adapters["e2b"].Configuration}, + } { + a.Configuration = configuration + if err := ValidateRegistration(a); !errors.Is(err, providercontract.ErrContract) { + t.Fatalf("%T: %v", configuration, err) + } + } +} + +func TestConfigurationRequirementsRejectEachOmission(t *testing.T) { + a := adapters["docker"] + original := a.Configuration.Requirements() + typ := reflect.TypeOf(original) + for i := 0; i < typ.NumField(); i++ { + t.Run(typ.Field(i).Name, func(t *testing.T) { + requirements := original + reflect.ValueOf(&requirements).Elem().Field(i).SetZero() + a.Configuration = registrationConfiguration{requirements: requirements} + if err := ValidateRegistration(a); !errors.Is(err, providercontract.ErrContract) { + t.Fatal(err) + } + }) + } +} + +func TestConfigurationRequirementsRejectInvalidDeclarations(t *testing.T) { + for _, change := range []func(*sandbox.ConfigurationRequirements){ + func(r *sandbox.ConfigurationRequirements) { r.Credential = "automatic" }, + func(r *sandbox.ConfigurationRequirements) { r.PublicOrigin = "private" }, + func(r *sandbox.ConfigurationRequirements) { r.Discovery.State = "unknown" }, + func(r *sandbox.ConfigurationRequirements) { r.Discovery.Reason = "" }, + func(r *sandbox.ConfigurationRequirements) { r.Discovery.Reason = "https://private:key@host" }, + func(r *sandbox.ConfigurationRequirements) { r.Discovery.State = providercontract.Supported }, + } { + a := adapters["docker"] + requirements := a.Configuration.Requirements() + change(&requirements) + a.Configuration = registrationConfiguration{requirements: requirements} + if err := ValidateRegistration(a); !errors.Is(err, providercontract.ErrContract) { + t.Fatal(err) + } + } +} + +func TestConfigurationRequirementsDoNotInventDependencies(t *testing.T) { + const kind = "configuration-requirement-test" + defer delete(adapters, kind) + // Required credentials are input policy. VerifyCredential is a separate + // resource operation that may be Unsupported for this provider. + for _, credential := range []sandbox.Requirement{sandbox.Required, sandbox.NotRequired} { + for _, public := range []sandbox.Requirement{sandbox.Required, sandbox.NotRequired} { + a := adapters["docker"] + requirements := a.Configuration.Requirements() + requirements.Credential, requirements.PublicOrigin = credential, public + a.Configuration = registrationConfiguration{requirements: requirements} + if err := ValidateRegistration(a); err != nil { + t.Fatal(err) + } + adapters[kind] = a + gotCredential, err := UsesCredential(kind) + if err != nil || gotCredential != (credential == sandbox.Required) { + t.Fatalf("credential %s: value=%v error=%v", credential, gotCredential, err) + } + gotPublic, err := RequiresPublicOrigin(kind) + if err != nil || gotPublic != (public == sandbox.Required) { + t.Fatalf("public origin %s: value=%v error=%v", public, gotPublic, err) + } + } + } +} + +type futureConfigurationDiscovery interface { + sandbox.ConfigurationDiscoverer + NextDiscovery(context.Context) error +} + +func TestFutureConfigurationRequirementAndMethodNeedExplicitHandling(t *testing.T) { + original := adapters["docker"].Configuration.Requirements() + fields := make([]reflect.StructField, 0, 4) + typ := reflect.TypeOf(original) + for i := 0; i < typ.NumField(); i++ { + fields = append(fields, typ.Field(i)) + } + fields = append(fields, reflect.StructField{Name: "FutureRequirement", Type: reflect.TypeFor[sandbox.Requirement]()}) + value := reflect.New(reflect.StructOf(fields)).Elem() + for i := 0; i < typ.NumField(); i++ { + value.Field(i).Set(reflect.ValueOf(original).Field(i)) + } + value.Field(typ.NumField()).Set(reflect.ValueOf(sandbox.NotRequired)) + if err := validateConfigurationRequirements(value); !errors.Is(err, providercontract.ErrContract) { + t.Fatal("new requirement silently inherited policy", err) + } + if err := validateConfigurationDiscoveryInterface(reflect.TypeFor[futureConfigurationDiscovery]()); !errors.Is(err, providercontract.ErrContract) { + t.Fatal("new discovery method inherited support", err) + } +} + +func TestUnsupportedConfigurationDiscoveryMatchesAuthoredReason(t *testing.T) { + for kind, a := range adapters { + support := a.Configuration.Requirements().Discovery + if support.State != providercontract.Unsupported { + continue + } + native := a.Configuration.(sandbox.ConfigurationDiscoverer) + for _, read := range []func() ([]byte, error){ + func() ([]byte, error) { + return native.DiscoverConfiguration(t.Context(), sandbox.ConfigurationDiscoveryInput{}) + }, + func() ([]byte, error) { + return DiscoverConfiguration(t.Context(), kind, sandbox.ConfigurationDiscoveryInput{}) + }, + } { + result, err := read() + reason, valid := providercontract.UnsupportedReason(err, "DiscoverConfiguration") + if result != nil || !valid || reason != support.Reason { + t.Fatalf("%s: result=%v reason=%s err=%v", kind, result, reason, err) + } + } + } +} diff --git a/services/core/internal/sandbox/providers/registration_test.go b/services/core/internal/sandbox/providers/registration_test.go new file mode 100644 index 000000000..764eee16a --- /dev/null +++ b/services/core/internal/sandbox/providers/registration_test.go @@ -0,0 +1,189 @@ +package providers + +import ( + "errors" + "strings" + "testing" + + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/providercontract" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sandbox" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sandbox/docker" + "github.com/google/uuid" +) + +func validRegistrationSpec() sandbox.DeploymentSpec { + return sandbox.DeploymentSpec{ + Resources: sandbox.Resources{CPUs: 2, MemoryMiB: 2048}, + Runtime: &sandbox.RuntimeRelease{ + SourceCommit: strings.Repeat("a", 40), ImageID: "sha256:" + strings.Repeat("b", 64), + ImageManifestDigest: "sha256:" + strings.Repeat("c", 64), + MicrosandboxRef: "oac-runtime@sha256:" + strings.Repeat("d", 64), + RuntimeSHA256: strings.Repeat("e", 64), FirmwareSHA256: strings.Repeat("f", 64), + }, + } +} + +func TestRegistrationRejectsBeforeCallbacksOrConstruction(t *testing.T) { + const kind = "registration-test-provider" + defer delete(adapters, kind) + for _, tc := range []struct { + name string + mutate func(*Adapter) + }{ + {"missing Runtime input policy", func(a *Adapter) { a.Policy = sandbox.DeploymentPolicy{} }}, + {"contradictory Runtime input policy", func(a *Adapter) { a.Policy.RuntimeError = "Runtime rejected" }}, + {"missing mode", func(a *Adapter) { a.Mode = "" }}, + {"unknown mode", func(a *Adapter) { a.Mode = "private-token" }}, + {"missing local constructor", func(a *Adapter) { a.BuildLocal = nil }}, + {"wrong local constructor", func(a *Adapter) { a.Mode = "direct" }}, + {"missing direct constructor", func(a *Adapter) { a.Mode = "direct"; a.BuildLocal = nil }}, + {"both constructors", func(a *Adapter) { + a.BuildDirect = func(DirectConfig) (sandbox.SandboxProvider, error) { + t.Fatal("called direct constructor") + return nil, nil + } + }}, + {"missing specification validator", func(a *Adapter) { a.ValidateSpecification = nil }}, + {"missing resource validator", func(a *Adapter) { a.ValidateResources = nil }}, + {"missing configuration", func(a *Adapter) { a.Configuration = nil }}, + {"typed nil configuration", func(a *Adapter) { var c *registrationConfiguration; a.Configuration = c }}, + {"missing discovery implementation", func(a *Adapter) { a.Configuration = missingConfigurationDiscovery{a.Configuration} }}, + {"missing configuration requirement", func(a *Adapter) { a.Configuration = registrationConfiguration{} }}, + {"invalid credential requirement", func(a *Adapter) { + r := a.Configuration.Requirements() + r.Credential = "private-token" + a.Configuration = registrationConfiguration{requirements: r} + }}, + {"invalid public origin requirement", func(a *Adapter) { + r := a.Configuration.Requirements() + r.PublicOrigin = "private-token" + a.Configuration = registrationConfiguration{requirements: r} + }}, + + {"missing operations", func(a *Adapter) { a.Operations = nil }}, + {"incomplete operations", func(a *Adapter) { a.Operations = func() providercontract.Operations { return nil } }}, + } { + t.Run(tc.name, func(t *testing.T) { + a := adapters["docker"] + a.BuildLocal = func(Config, *Built) (func(), error) { t.Fatal("called local constructor"); return nil, nil } + a.ValidateSpecification = func(sandbox.DeploymentSpec) error { t.Fatal("called specification validator"); return nil } + a.ValidateResources = func(sandbox.Resources) error { t.Fatal("called resource validator"); return nil } + a.Configuration = registrationConfiguration{requirements: a.Configuration.Requirements()} + tc.mutate(&a) + adapters[kind] = a + selection := sandbox.Selection{Provider: kind, DeploymentSpec: validRegistrationSpec()} + for _, entry := range []struct { + name string + call func() error + }{ + {"lookup", func() error { _, err := Lookup(kind); return err }}, + {"credential requirement", func() error { _, err := UsesCredential(kind); return err }}, + {"public origin requirement", func() error { _, err := RequiresPublicOrigin(kind); return err }}, + {"normalize", func() error { _, err := Normalize(selection); return err }}, + {"specification", func() error { return ValidateSpecification(kind, selection.DeploymentSpec) }}, + {"resources", func() error { return ValidateResources(kind, selection.Resources) }}, + {"description", func() error { _, err := Describe(kind, uuid.NewString()); return err }}, + {"decode input", func() error { _, err := DecodeInput(kind, nil, nil); return err }}, + {"encode", func() error { _, err := Encode(kind, nil); return err }}, + {"decode", func() error { _, err := Decode(kind, sandbox.ConfigurationRecord{}); return err }}, + {"equal", func() error { _, err := Equal(kind, nil, nil); return err }}, + {"discovery", func() error { + _, err := DiscoverConfiguration(t.Context(), kind, sandbox.ConfigurationDiscoveryInput{}) + return err + }}, + {"resolve change", func() error { _, err := ResolveChange(selection, selection); return err }}, + {"credential", func() error { _, err := WithCredential(selection, selection); return err }}, + {"local build", func() error { + _, _, err := Build(Config{Provider: kind, Generation: 1, InstallationID: uuid.NewString(), Specification: selection.DeploymentSpec}) + return err + }}, + {"direct build", func() error { _, err := BuildDirect(DirectConfig{Selection: selection}); return err }}, + {"binding", func() error { return ValidateBinding(a, &docker.Provider{}) }}, + {"projection", func() error { + text, err := PythonDeploymentContract() + if text != "" { + t.Fatal("partial invalid projection") + } + return err + }}, + } { + t.Run(entry.name, func(t *testing.T) { + err := entry.call() + if !errors.Is(err, providercontract.ErrContract) || strings.Contains(err.Error(), "private-token") { + t.Fatalf("expected safe registration error, got %v", err) + } + }) + } + }) + } +} + +func TestCompleteRegistrationsPreserveConstruction(t *testing.T) { + for kind, a := range adapters { + if err := ValidateRegistration(a); err != nil { + t.Fatalf("%s: %v", kind, err) + } + } + const kind = "new-test-provider" + defer delete(adapters, kind) + calls := 0 + a := adapters["docker"] + a.BuildLocal = func(_ Config, built *Built) (func(), error) { + calls++ + built.Provider = &docker.Provider{} + return func() {}, nil + } + adapters[kind] = a + built, closeProvider, err := Build(Config{Provider: kind, Generation: 1, InstallationID: uuid.NewString(), Specification: validRegistrationSpec()}) + if err != nil || built.Provider == nil || calls != 1 { + t.Fatalf("node build: %v calls=%d", err, calls) + } + closeProvider() + // Direct providers may legitimately need no remote credential or extra + // selection state; registration must not require irrelevant callback stubs. + a.Mode, a.BuildLocal = "direct", nil + a.BuildDirect = func(DirectConfig) (sandbox.SandboxProvider, error) { + calls++ + return &docker.Provider{}, nil + } + adapters[kind] = a + p, err := BuildDirect(DirectConfig{Selection: sandbox.Selection{Provider: kind}}) + if err != nil || p == nil || calls != 2 { + t.Fatalf("credential-free direct build: %v calls=%d", err, calls) + } + if _, err := PythonDeploymentContract(); err != nil { + t.Fatal(err) + } +} + +// Idle time is measured before suspension, retention after suspension. Neither +// duration needs to be greater than the other. +func TestRegistrationCheckpointPolicy(t *testing.T) { + for _, tc := range []struct { + name string + kind string + idle, retention int64 + direct, valid bool + }{ + {"negative idle", "microsandbox", -1, 20, false, false}, + {"missing idle", "microsandbox", 0, 20, false, false}, + {"missing retention", "microsandbox", 20, 0, false, false}, + {"overflow", "microsandbox", 1<<63 - 1, 20, false, false}, + {"direct suspension", "microsandbox", 20, 20, true, false}, + {"unsupported suspension", "docker", 20, 20, false, false}, + {"independent durations", "microsandbox", 300, 30, false, true}, + {"no suspension", "docker", 0, 0, false, true}, + } { + t.Run(tc.name, func(t *testing.T) { + a := adapters[tc.kind] + a.IdleSeconds, a.RetentionSeconds = tc.idle, tc.retention + if tc.direct { + a.Mode, a.BuildLocal, a.BuildDirect = "direct", nil, adapters["e2b"].BuildDirect + } + err := ValidateRegistration(a) + if (err == nil) != tc.valid || err != nil && !errors.Is(err, providercontract.ErrContract) { + t.Fatal(err) + } + }) + } +} diff --git a/services/core/internal/sandbox/providers/registry.go b/services/core/internal/sandbox/providers/registry.go index 1c4e09f49..f081704d6 100644 --- a/services/core/internal/sandbox/providers/registry.go +++ b/services/core/internal/sandbox/providers/registry.go @@ -50,10 +50,10 @@ var adapters = map[string]Adapter{ func Lookup(kind string) (Adapter, error) { a, ok := adapters[kind] - if !ok || a.Operations == nil { + if !ok { return Adapter{}, fmt.Errorf("%w: unsupported sandbox provider", sandbox.ErrInvalid) } - if err := sandbox.ValidateOperations(a.Operations()); err != nil { + if err := ValidateRegistration(a); err != nil { return Adapter{}, err } return a, nil @@ -110,10 +110,14 @@ func Describe(kind, installation string) (Description, error) { // PythonDeploymentContract projects the same registered adapter policies into // the node installer; no second provider list exists in another language. -func PythonDeploymentContract() string { +func PythonDeploymentContract() (string, error) { policies := make(map[string]sandbox.DeploymentPolicy, len(adapters)) - for kind, a := range adapters { + for kind := range adapters { + a, err := Lookup(kind) + if err != nil { + return "", err + } policies[kind] = a.Policy } - return sandbox.PythonDeploymentContract(policies) + return sandbox.PythonDeploymentContract(policies), nil } diff --git a/services/core/internal/sandbox/providers/registry_test.go b/services/core/internal/sandbox/providers/registry_test.go index 94c1cc78f..3d3639b13 100644 --- a/services/core/internal/sandbox/providers/registry_test.go +++ b/services/core/internal/sandbox/providers/registry_test.go @@ -2,14 +2,10 @@ package providers import ( "errors" - "testing" - - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sandbox/docker" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sandbox/e2b" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sandbox/microsandbox" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sandbox" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sandbox/e2b" "github.com/google/uuid" + "testing" ) func TestRegistrationOwnsDeploymentPolicy(t *testing.T) { @@ -67,9 +63,9 @@ func TestSelectionNormalizationAndCredentialInheritance(t *testing.T) { func TestNewRegistrationDoesNotNeedCoreDispatchChanges(t *testing.T) { const kind = "contract-test-provider" // Registration is test-local: production registrations are fixed, never plugins. - adapters[kind] = Adapter{Mode: "nodes", Operations: docker.Operations, ValidateSpecification: func(sandbox.DeploymentSpec) error { return nil }, Configuration: nodeConfigurationAdapter{validate: func(sandbox.DeploymentSpec) error { return nil }}} + adapters[kind] = adapters["docker"] defer delete(adapters, kind) - s, err := Normalize(sandbox.Selection{Provider: kind}) + s, err := Normalize(sandbox.Selection{Provider: kind, DeploymentSpec: validRegistrationSpec()}) if err != nil || s.Provider != kind || !IsNode(kind) || SupportsCheckpoint(kind) { t.Fatal("new entry did not follow shared boundary", err) } @@ -86,11 +82,11 @@ func TestRetainedLimitUsesRegisteredCapabilities(t *testing.T) { const kind = "capacity-test-provider" defer delete(adapters, kind) for _, checkpoint := range []bool{false, true} { - operations := docker.Operations + adapter := adapters["docker"] if checkpoint { - operations = microsandbox.Operations + adapter = adapters["microsandbox"] } - adapters[kind] = Adapter{Mode: "nodes", Operations: operations} + adapters[kind] = adapter for _, retained := range []int{0, 20} { want := 10 if checkpoint { diff --git a/services/core/internal/sandbox/providers/specification.go b/services/core/internal/sandbox/providers/specification.go index e80c0535c..414737e6a 100644 --- a/services/core/internal/sandbox/providers/specification.go +++ b/services/core/internal/sandbox/providers/specification.go @@ -7,7 +7,11 @@ import ( // Local paths belong to the node. Core owns reservation capacity, execution // resources and the immutable deployment release it enrolled with. func validateSpecification(c Config) error { - if !IsNode(c.Provider) { + adapter, err := Lookup(c.Provider) + if err != nil { + return err + } + if adapter.Mode != "nodes" { return errors.New("nodes support Docker or microsandbox; E2B is managed by Core") } if c.Generation == 0 {