From 087069a8992b8d0fea745d7de2bd975519245465 Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Wed, 30 Sep 2026 16:27:27 +0000 Subject: [PATCH] Move saved Agents into the agents domain and agentpg adapter agents owns the saved Agent rules: configuration normalization and bounds, update merging over the locked Agent, metadata and the model provider bundle checks. agentpg persists them on pgunit, seals the bundle, records write audit in the same transaction and translates database outcomes into the domain errors. api reaches Agents through the Agents service and the AgentsReader, and cmd/server wires both. The store Agent code and its tests move to the new owners. --- services/core/IMPLEMENTATION.md | 6 +- services/core/cmd/server/http_routes_test.go | 2 +- services/core/cmd/server/main.go | 9 +- services/core/internal/agents/agent.go | 56 ++ .../core/internal/agents/configuration.go | 149 ++++ .../internal/agents/configuration_test.go | 143 ++++ services/core/internal/agents/doc.go | 5 + services/core/internal/agents/errors.go | 12 + services/core/internal/agents/service.go | 123 ++++ services/core/internal/agents/service_test.go | 167 +++++ services/core/internal/agents/storage.go | 62 ++ .../core/internal/api/admin_resources_test.go | 15 +- services/core/internal/api/agents.go | 53 +- services/core/internal/api/agents_delete.go | 11 +- services/core/internal/api/agents_list.go | 7 +- services/core/internal/api/agents_update.go | 46 +- .../api/configuration_validation_test.go | 20 +- services/core/internal/api/dependencies.go | 4 +- .../core/internal/api/dependencies_test.go | 5 +- services/core/internal/api/errors_agents.go | 25 + services/core/internal/api/fakes_test.go | 63 +- services/core/internal/api/handler.go | 2 +- services/core/internal/api/handler_test.go | 10 +- .../internal/api/resource_creation_test.go | 7 +- .../core/internal/api/resource_query_test.go | 21 +- services/core/internal/api/routing_test.go | 31 +- .../core/internal/api/saved_configuration.go | 31 +- .../core/internal/api/saved_provider_test.go | 33 +- .../internal/api/session_model_defaults.go | 16 +- .../internal/api/validation_errors_test.go | 11 +- services/core/internal/api/vaults_test.go | 7 +- .../db/queries/agent_model_execution.sql | 2 +- .../db/sqlc/agent_model_execution.sql.go | 12 +- .../persistence/postgres/agentpg/store.go | 298 ++++++++ .../postgres/agentpg/store_test.go | 669 ++++++++++++++++++ .../internal/store/admin_delete_audit_test.go | 14 +- .../internal/store/admin_key_audit_test.go | 10 +- .../internal/store/agent_model_execution.go | 147 ---- .../store/agent_model_execution_test.go | 207 ------ services/core/internal/store/agents.go | 115 --- services/core/internal/store/agents_delete.go | 41 -- .../internal/store/agents_fixture_test.go | 24 + services/core/internal/store/agents_list.go | 52 -- .../core/internal/store/agents_list_test.go | 96 --- services/core/internal/store/agents_test.go | 108 --- services/core/internal/store/agents_update.go | 111 --- .../core/internal/store/agents_update_test.go | 67 -- .../configuration_validation_public_test.go | 4 +- .../internal/store/project_api_keys_test.go | 12 +- .../store/public_handler_fixture_test.go | 16 +- .../store/saved_web_search_public_test.go | 19 +- .../store/write_audit_resources_test.go | 55 +- 52 files changed, 2010 insertions(+), 1221 deletions(-) create mode 100644 services/core/internal/agents/agent.go create mode 100644 services/core/internal/agents/configuration.go create mode 100644 services/core/internal/agents/configuration_test.go create mode 100644 services/core/internal/agents/doc.go create mode 100644 services/core/internal/agents/errors.go create mode 100644 services/core/internal/agents/service.go create mode 100644 services/core/internal/agents/service_test.go create mode 100644 services/core/internal/agents/storage.go create mode 100644 services/core/internal/api/errors_agents.go create mode 100644 services/core/internal/persistence/postgres/agentpg/store.go create mode 100644 services/core/internal/persistence/postgres/agentpg/store_test.go delete mode 100644 services/core/internal/store/agent_model_execution.go delete mode 100644 services/core/internal/store/agent_model_execution_test.go delete mode 100644 services/core/internal/store/agents.go delete mode 100644 services/core/internal/store/agents_delete.go create mode 100644 services/core/internal/store/agents_fixture_test.go delete mode 100644 services/core/internal/store/agents_list.go delete mode 100644 services/core/internal/store/agents_list_test.go delete mode 100644 services/core/internal/store/agents_test.go delete mode 100644 services/core/internal/store/agents_update.go delete mode 100644 services/core/internal/store/agents_update_test.go diff --git a/services/core/IMPLEMENTATION.md b/services/core/IMPLEMENTATION.md index bcb109794..981323a67 100644 --- a/services/core/IMPLEMENTATION.md +++ b/services/core/IMPLEMENTATION.md @@ -18,6 +18,10 @@ Shared vocabulary has one owner each, and domains use it rather than copy it. `i `cmd/server` owns the execution lease. It acquires one `pgunit.Lease`, builds every lease-bound adapter on it, and passes the lease and those adapters together as one `execution.Owner` to `execution.StartWorker`. If anything fails before that call, `cmd/server` closes the lease. From that call the Worker owns cleanup: a failed start closes the lease before it returns, and a started Worker closes it after `Run` has cancelled and drained its work. Each close runs under its own bounded deadline, independent of the cancelled request or run. Lease-bound adapters and the store writer borrow the lease and never close it, and the Worker uses the lease only through `Owner.Lease`, never through an adapter. Store integration tests start the Worker the same way through `startWorker`. +Domain owners, each with its PostgreSQL adapter under `internal/persistence/postgres`: + +- `agents` (`agentpg`): saved Agents, their configuration merge and bounds, and the encrypted model-provider bundle bound to each Agent. + ## Request handling Every Agents API JSON route reads its body through `readJSONObject` before decoding, validation or lookup. The gate requires a JSON Content-Type, applies the route's body limit and rejects invalid UTF-8, malformed JSON (including unpaired surrogate escapes), repeated keys and non-object roots with the official messages; an empty body or `null` becomes `{}`. DELETE, multipart, Core extension and internal routes keep their own readers. Member names match exactly: decode request objects with `decodeInputObject`, or check `inexactMember` before another decoder, so `encoding/json` never matches a case variant. @@ -92,7 +96,7 @@ Session status and last activity use the public projection in [`internal/api/ses ## Agents and model providers -Reusable Agents are tenant-scoped rows independent of Session snapshots and engine bindings. The store persists caller-validated configuration without applying harness restrictions or model defaults, with internal limits of 512 KiB for configuration and the `metadata.Encode` bound of 64 KiB for metadata. An update locks the Agent row while merging the supplied fields and enforcing the configuration bound, then commits configuration, metadata and update time together, so a stale full snapshot never overwrites another update. An empty update preserves the saved fields and advances `updated_at` through the same SQL update. Deletion is one tenant-scoped `DELETE … RETURNING id`. A Session copies the saved configuration into its immutable snapshot and never looks up its source again. +Reusable Agents are tenant-scoped rows independent of Session snapshots and engine bindings. `agents` accepts caller-validated configuration without applying harness restrictions or model defaults, with internal limits of 512 KiB for configuration and the `metadata.Encode` bound of 64 KiB for metadata. An update runs inside `agentpg`'s Agent row lock: `agents` merges the supplied fields over the locked Agent and enforces the configuration bound, then `agentpg` commits configuration, metadata and update time together, so a stale full snapshot never overwrites another update. An empty update preserves the saved fields and advances `updated_at` through the same SQL update. Deletion is one tenant-scoped `DELETE … RETURNING id`. A Session copies the saved configuration into its immutable snapshot and never looks up its source again. Saved execution defaults keep a model-provider bundle whole at every replacement boundary: endpoint, key, protocol and limits are never inherited separately. Agent JSON holds only the safe provider fields and an output-only configured flag; the complete bundle is encrypted separately with a tenant and Agent binding and its own purpose, and configuration and secret changes commit together under the Agent row lock. Model-only edits need no key. Merged harness, protocol and limits are validated without reading keys. Session creation reads safe defaults and ciphertext in one snapshot, and a complete Session override does not decrypt the inherited bundle. The resolved bundle is frozen in an encrypted Session-owned row, and dispatch fails closed when that snapshot is missing or cannot be decrypted; later Agent edits, default changes, restarts and suspension never resolve it again. diff --git a/services/core/cmd/server/http_routes_test.go b/services/core/cmd/server/http_routes_test.go index e64d696ec..ee574a376 100644 --- a/services/core/cmd/server/http_routes_test.go +++ b/services/core/cmd/server/http_routes_test.go @@ -89,7 +89,7 @@ func daemonComposition(t testing.TB) http.Handler { Engine: "codex", CoreKeys: admin, InstallationBindings: struct{ api.InstallationBindings }{}, Projects: trapProjects{keys: keys}, Vaults: struct{ api.Vaults }{}, ModelProviders: struct{ api.ModelProviders }{}, Files: struct{ api.Files }{}, Skills: struct{ api.Skills }{}, EnvironmentTemplates: struct{ api.EnvironmentTemplates }{}, - Agents: struct{ api.Agents }{}, Sessions: struct{ api.Sessions }{}, SessionEvents: struct{ api.SessionEvents }{}, + Agents: struct{ api.Agents }{}, AgentsReader: struct{ api.AgentsReader }{}, Sessions: struct{ api.Sessions }{}, SessionEvents: struct{ api.SessionEvents }{}, SessionHistory: struct{ api.SessionHistory }{}, Subagents: struct{ api.Subagents }{}, Artifacts: struct{ api.Artifacts }{}, SessionAdmin: struct{ api.SessionAdmin }{}, Environments: struct{ api.Environments }{}, ExecutorConnections: struct{ api.ExecutorConnections }{}, Admin: struct{ api.Admin }{}, AdminAudit: struct{ api.AdminAudit }{}, WriteAudit: struct{ api.WriteAudit }{}, Metrics: struct{ api.Metrics }{}, diff --git a/services/core/cmd/server/main.go b/services/core/cmd/server/main.go index 126aac7b5..986219268 100644 --- a/services/core/cmd/server/main.go +++ b/services/core/cmd/server/main.go @@ -31,11 +31,13 @@ import ( "time" "github.com/MiniMax-AI/OpenAgentCore/internal/obs/log" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/api" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/coremetrics" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/databaseurl" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/execution" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/nativeinstaller" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/agentpg" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/auditpg" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtime" @@ -109,6 +111,11 @@ func run() error { executionStore.SetPublicURL(public) units := pgunit.NewPool(pool) auditStore := auditpg.New(units) + agentStore := agentpg.New(units, credentialKey) + agentService, err := agents.NewService(agentStore) + if err != nil { + return err + } installation, err := installationFacts(public) if err != nil { return err @@ -296,7 +303,7 @@ func run() error { Engine: engine, Harnesses: kinds, CoreKeys: keyAdmin, Installation: installation, InstallationBindings: executionStore, Projects: executionStore, Vaults: executionStore, ModelProviders: executionStore, Files: executionStore, - Skills: executionStore, EnvironmentTemplates: executionStore, Agents: executionStore, + Skills: executionStore, EnvironmentTemplates: executionStore, Agents: agentService, AgentsReader: agentStore, Sessions: executionStore, SessionEvents: executionStore, SessionHistory: executionStore, Subagents: executionStore, Artifacts: executionStore, SessionAdmin: executionStore, Environments: executionStore, ExecutorConnections: executorConnections{store: executionStore, registry: registry}, diff --git a/services/core/internal/agents/agent.go b/services/core/internal/agents/agent.go new file mode 100644 index 000000000..ae5ad1610 --- /dev/null +++ b/services/core/internal/agents/agent.go @@ -0,0 +1,56 @@ +package agents + +import ( + "encoding/json" + "fmt" + "time" + + v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" +) + +// Agent is saved configuration owned by a tenant. It has no Harness binding or +// live execution state; Session snapshots are separate objects. +type Agent struct { + ID string + TenantID string + Metadata map[string]string + Configuration json.RawMessage + CreatedAt time.Time + UpdatedAt time.Time +} + +// MaxConfigurationBytes bounds a saved configuration and an update's patch. +const MaxConfigurationBytes = 512 * 1024 + +// MaxPageSize bounds one page of ListAgents. +const MaxPageSize = 100 + +// ListQuery selects one page of a tenant's Agents in creation order. After is +// the last Agent of the previous page; an After that names no Agent of the +// tenant is ErrNotFound. +type ListQuery struct { + TenantID string + After string + Limit int + Ascending bool +} + +// Validate checks the page size. +func (q ListQuery) Validate() error { + if q.Limit < 1 || q.Limit > MaxPageSize { + return fmt.Errorf("%w: page size must be 1..%d", ErrInvalidInput, MaxPageSize) + } + return nil +} + +// Page is one page of Agents. NextCursor is empty on the last page. +type Page struct { + Agents []Agent + NextCursor string +} + +// ModelProviderChange replaces an Agent's saved model provider bundle. A nil +// Provider removes it. +type ModelProviderChange struct { + Provider *v1.ModelProviderInput +} diff --git a/services/core/internal/agents/configuration.go b/services/core/internal/agents/configuration.go new file mode 100644 index 000000000..209e98998 --- /dev/null +++ b/services/core/internal/agents/configuration.go @@ -0,0 +1,149 @@ +package agents + +import ( + "encoding/json" + "fmt" + + v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/jsonobject" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/metadata" +) + +// encodeMetadata encodes caller metadata for storage; nil is the empty object. +func encodeMetadata(values map[string]string) (json.RawMessage, error) { + encoded, err := metadata.Encode(values) + if err != nil { + return nil, fmt.Errorf("%w: metadata: %v", ErrInvalidInput, err) + } + return encoded, nil +} + +// createConfiguration normalizes the configuration of a new Agent. +func createConfiguration(raw json.RawMessage) (json.RawMessage, error) { + if len(raw) == 0 || len(raw) > MaxConfigurationBytes { + return nil, fmt.Errorf("%w: configuration must be an object of at most 512 KiB", ErrInvalidInput) + } + return normalizeConfiguration(raw) +} + +// decodePatch normalizes an update's supplied fields; empty supplies none. +func decodePatch(raw json.RawMessage) (map[string]json.RawMessage, error) { + if len(raw) == 0 { + raw = json.RawMessage(`{}`) + } + if len(raw) > MaxConfigurationBytes { + return nil, fmt.Errorf("%w: configuration patch exceeds 512 KiB", ErrInvalidInput) + } + normalized, err := normalizeConfiguration(raw) + if err != nil { + return nil, err + } + var patch map[string]json.RawMessage + if err := json.Unmarshal(normalized, &patch); err != nil { + return nil, fmt.Errorf("%w: configuration patch: %v", ErrInvalidInput, err) + } + return patch, nil +} + +func normalizeConfiguration(raw json.RawMessage) (json.RawMessage, error) { + normalized, err := jsonobject.Normalize(raw) + if err != nil { + return nil, fmt.Errorf("%w: configuration: %v", ErrInvalidInput, err) + } + return normalized, nil +} + +// validateModelExecution checks the saved Harness configuration and that the +// model provider bundle suits the saved Harness. provider is the bundle being +// saved, if any. +func validateModelExecution(configuration json.RawMessage, provider *v1.ModelProviderInput) error { + if provider != nil { + if err := provider.Validate(); err != nil { + return fmt.Errorf("%w: %s", ErrInvalidInput, err) + } + } + var config struct { + Core *v1.SavedAgentCore `json:"x_agents_core"` + } + if err := json.Unmarshal(configuration, &config); err != nil { + return fmt.Errorf("%w: x_agents_core: %v", ErrInvalidInput, err) + } + if config.Core != nil { + if err := v1.ValidateHarnessConfig(config.Core.Harness, config.Core.HarnessConfig); err != nil { + return fmt.Errorf("%w: %s", ErrInvalidInput, err) + } + } + if config.Core == nil || config.Core.ModelProvider == nil || config.Core.Harness == "" { + return nil + } + if err := config.Core.ModelProvider.ValidateHarness(config.Core.Harness); err != nil { + return fmt.Errorf("%w: %s", ErrInvalidInput, err) + } + return nil +} + +// mergeConfiguration applies an update's supplied fields to the saved +// configuration. Supplied top-level fields replace saved ones; x_agents_core +// subfields merge one by one, and a null x_agents_core removes it. Changing the +// model, the model provider or the Harness without supplying harness_config +// resets harness_config to {}, because native options belong to one model and +// Harness. +func mergeConfiguration(saved json.RawMessage, patch map[string]json.RawMessage) (json.RawMessage, error) { + configuration := map[string]json.RawMessage{} + if err := json.Unmarshal(saved, &configuration); err != nil { + return nil, fmt.Errorf("decode saved Agent configuration: %w", err) + } + coreNull := string(patch["x_agents_core"]) == "null" + var corePatch map[string]json.RawMessage + if raw := patch["x_agents_core"]; len(raw) > 0 && !coreNull { + if err := json.Unmarshal(raw, &corePatch); err != nil { + return nil, fmt.Errorf("%w: x_agents_core: %v", ErrInvalidInput, err) + } + } + _, modelChanged := patch["model"] + _, providerChanged := corePatch["model_provider"] + _, harnessChanged := corePatch["harness"] + _, nativeSupplied := corePatch["harness_config"] + if (modelChanged || providerChanged || harnessChanged) && !nativeSupplied && !coreNull { + if corePatch == nil { + corePatch = map[string]json.RawMessage{} + } + corePatch["harness_config"] = json.RawMessage(`{}`) + } + for field, value := range patch { + if field != "x_agents_core" { + configuration[field] = value + } + } + switch { + case coreNull: + configuration["x_agents_core"] = patch["x_agents_core"] + case corePatch != nil: + core := map[string]json.RawMessage{} + if old := configuration["x_agents_core"]; len(old) != 0 && string(old) != "null" { + if err := json.Unmarshal(old, &core); err != nil { + return nil, fmt.Errorf("decode saved Agent configuration: %w", err) + } + } + for key, replacement := range corePatch { + core[key] = replacement + } + merged, err := json.Marshal(core) + if err != nil { + return nil, err + } + configuration["x_agents_core"] = merged + } + merged, err := json.Marshal(configuration) + if err != nil { + return nil, err + } + merged, err = normalizeConfiguration(merged) + if err != nil { + return nil, err + } + if len(merged) > MaxConfigurationBytes { + return nil, fmt.Errorf("%w: configuration exceeds 512 KiB", ErrInvalidInput) + } + return merged, nil +} diff --git a/services/core/internal/agents/configuration_test.go b/services/core/internal/agents/configuration_test.go new file mode 100644 index 000000000..79a67304d --- /dev/null +++ b/services/core/internal/agents/configuration_test.go @@ -0,0 +1,143 @@ +package agents + +import ( + "encoding/json" + "errors" + "strings" + "testing" + + v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" +) + +// testProvider is a saved provider view with its keys in normalized order. +const testProvider = `{"api_key_configured":true,"base_url":"https://example.test/v1","protocol":"responses"}` + +func TestMergeConfiguration(t *testing.T) { + for _, tc := range []struct { + name, saved, patch, want string + }{ + {"top-level field replaces", `{"model":"a","name":"old"}`, `{"name":"new"}`, `{"model":"a","name":"new"}`}, + {"empty patch keeps", `{"model":"a","x_agents_core":{"harness":"codex","harness_config":{"k":1}}}`, `{}`, + `{"model":"a","x_agents_core":{"harness":"codex","harness_config":{"k":1}}}`}, + {"model change resets harness_config", `{"model":"a","x_agents_core":{"harness":"codex","harness_config":{"k":1}}}`, `{"model":"b"}`, + `{"model":"b","x_agents_core":{"harness":"codex","harness_config":{}}}`}, + {"model change without saved extension", `{"model":"a"}`, `{"model":"b"}`, `{"model":"b","x_agents_core":{"harness_config":{}}}`}, + {"supplied harness_config wins", `{"model":"a","x_agents_core":{"harness":"codex","harness_config":{"k":1}}}`, + `{"model":"b","x_agents_core":{"harness_config":{"k":2}}}`, `{"model":"b","x_agents_core":{"harness":"codex","harness_config":{"k":2}}}`}, + {"harness change merges subfields", `{"model":"a","x_agents_core":{"harness":"codex","model_provider":` + testProvider + `,"harness_config":{"k":1}}}`, + `{"x_agents_core":{"harness":"claude_sdk"}}`, + `{"model":"a","x_agents_core":{"harness":"claude_sdk","harness_config":{},"model_provider":` + testProvider + `}}`}, + {"provider change resets harness_config", `{"model":"a","x_agents_core":{"harness":"codex","harness_config":{"k":1}}}`, + `{"x_agents_core":{"model_provider":null}}`, `{"model":"a","x_agents_core":{"harness":"codex","harness_config":{},"model_provider":null}}`}, + {"null extension replaces it", `{"model":"a","x_agents_core":{"harness":"codex"}}`, `{"model":"b","x_agents_core":null}`, + `{"model":"b","x_agents_core":null}`}, + {"extension onto null", `{"model":"a","x_agents_core":null}`, `{"x_agents_core":{"harness":"codex"}}`, + `{"model":"a","x_agents_core":{"harness":"codex","harness_config":{}}}`}, + {"large numbers survive", `{"model":"a","x":12345678901234567890}`, `{"y":1.50}`, `{"model":"a","x":12345678901234567890,"y":1.50}`}, + } { + t.Run(tc.name, func(t *testing.T) { + patch := decodeTestPatch(t, tc.patch) + before, _ := json.Marshal(patch) + got, err := mergeConfiguration(json.RawMessage(tc.saved), patch) + if err != nil || string(got) != tc.want { + t.Fatalf("merge = %s, %v; want %s", got, err, tc.want) + } + if after, _ := json.Marshal(patch); string(after) != string(before) { + t.Fatalf("merge changed the patch: %s", after) + } + }) + } +} + +func TestMergeConfigurationRejects(t *testing.T) { + big := `"` + strings.Repeat("x", 400*1024) + `"` + for _, tc := range []struct { + name, saved, patch string + invalid bool + }{ + {"merged size bound", `{"model":"a","instructions":` + big + `}`, `{"name":` + `"` + strings.Repeat("y", 150*1024) + `"}`, true}, + {"extension not an object", `{"model":"a"}`, `{"x_agents_core":[]}`, true}, + {"corrupt saved configuration", `[]`, `{}`, false}, + } { + t.Run(tc.name, func(t *testing.T) { + _, err := mergeConfiguration(json.RawMessage(tc.saved), decodeTestPatch(t, tc.patch)) + if err == nil || errors.Is(err, ErrInvalidInput) != tc.invalid { + t.Fatalf("merge error = %v", err) + } + }) + } +} + +func TestCreateConfigurationAndPatchBounds(t *testing.T) { + oversized := json.RawMessage(`{"model":"` + strings.Repeat("x", MaxConfigurationBytes) + `"}`) + for _, raw := range []string{"", "null", "[]", "true", `{"model":"x"`, `{} {}`, string(oversized)} { + if _, err := createConfiguration(json.RawMessage(raw)); !errors.Is(err, ErrInvalidInput) { + t.Errorf("createConfiguration(%.20q) = %v", raw, err) + } + } + if got, err := createConfiguration(json.RawMessage(`{ "model" : "x", "n": 1.0 }`)); err != nil || string(got) != `{"model":"x","n":1.0}` { + t.Fatalf("createConfiguration = %s, %v", got, err) + } + for _, raw := range []string{"[]", "null", `{} {}`, string(oversized)} { + if _, err := decodePatch(json.RawMessage(raw)); !errors.Is(err, ErrInvalidInput) { + t.Errorf("decodePatch(%.20q) = %v", raw, err) + } + } + if patch, err := decodePatch(nil); err != nil || len(patch) != 0 { + t.Fatalf("empty patch = %v, %v", patch, err) + } +} + +func TestValidateModelExecution(t *testing.T) { + bundle := &v1.ModelProviderInput{Protocol: "responses", BaseURL: "https://example.test/v1", APIKey: "secret"} + for _, tc := range []struct { + name, configuration string + provider *v1.ModelProviderInput + valid bool + }{ + {"no extension", `{"model":"a"}`, nil, true}, + {"native provider", `{"model":"a","x_agents_core":{"harness":"codex","model_provider":` + testProvider + `}}`, bundle, true}, + {"provider without harness", `{"model":"a","x_agents_core":{"model_provider":` + strings.Replace(testProvider, "responses", "anthropic", 1) + `}}`, nil, true}, + {"incompatible saved provider", `{"model":"a","x_agents_core":{"harness":"claude_sdk","model_provider":` + testProvider + `}}`, nil, false}, + {"invalid bundle", `{"model":"a"}`, &v1.ModelProviderInput{Protocol: "responses"}, false}, + {"invalid harness_config", `{"model":"a","x_agents_core":{"harness":"codex","harness_config":{"unknown_option":true}}}`, nil, false}, + {"extension not an object", `{"model":"a","x_agents_core":[]}`, nil, false}, + } { + t.Run(tc.name, func(t *testing.T) { + err := validateModelExecution(json.RawMessage(tc.configuration), tc.provider) + if (err == nil) != tc.valid || err != nil && !errors.Is(err, ErrInvalidInput) { + t.Fatalf("validateModelExecution = %v", err) + } + }) + } +} + +func TestListQueryValidate(t *testing.T) { + for limit, valid := range map[int]bool{-1: false, 0: false, 1: true, MaxPageSize: true, MaxPageSize + 1: false} { + if err := (ListQuery{Limit: limit}).Validate(); (err == nil) != valid || err != nil && !errors.Is(err, ErrInvalidInput) { + t.Errorf("limit %d: %v", limit, err) + } + } +} + +func TestReviseKeepsOrReplacesMetadata(t *testing.T) { + current := Agent{Metadata: map[string]string{"team": "core"}, Configuration: json.RawMessage(`{"model":"a"}`)} + kept, err := revise(current, map[string]json.RawMessage{}, nil, nil) + if err != nil || string(kept.Metadata) != `{"team":"core"}` || kept.ModelProvider != nil { + t.Fatalf("kept = %+v, %v", kept, err) + } + change := &ModelProviderChange{} + replaced, err := revise(current, map[string]json.RawMessage{}, json.RawMessage(`{}`), change) + if err != nil || string(replaced.Metadata) != `{}` || replaced.ModelProvider != change { + t.Fatalf("replaced = %+v, %v", replaced, err) + } +} + +func decodeTestPatch(t *testing.T, raw string) map[string]json.RawMessage { + t.Helper() + patch, err := decodePatch(json.RawMessage(raw)) + if err != nil { + t.Fatal(err) + } + return patch +} diff --git a/services/core/internal/agents/doc.go b/services/core/internal/agents/doc.go new file mode 100644 index 000000000..c277e6143 --- /dev/null +++ b/services/core/internal/agents/doc.go @@ -0,0 +1,5 @@ +// Package agents owns saved Agents: reusable configuration a tenant keeps +// independently of Sessions, with the optional model provider bundle that its +// x_agents_core.model_provider describes. It validates and merges saved +// configuration; Session snapshots and execution belong elsewhere. +package agents diff --git a/services/core/internal/agents/errors.go b/services/core/internal/agents/errors.go new file mode 100644 index 000000000..2f852c34e --- /dev/null +++ b/services/core/internal/agents/errors.go @@ -0,0 +1,12 @@ +package agents + +import "errors" + +var ( + // ErrNotFound reports an Agent the tenant does not have, including an + // identifier that cannot name one. + ErrNotFound = errors.New("agent not found") + // ErrInvalidInput reports configuration, metadata or a request limit that + // the saved Agent rules reject. + ErrInvalidInput = errors.New("invalid agent input") +) diff --git a/services/core/internal/agents/service.go b/services/core/internal/agents/service.go new file mode 100644 index 000000000..c41b7fa3e --- /dev/null +++ b/services/core/internal/agents/service.go @@ -0,0 +1,123 @@ +package agents + +import ( + "context" + "encoding/json" + "errors" + + v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" +) + +// Service runs the Agent write use cases. Reads go through Reader. +type Service struct { + storage Storage +} + +func NewService(storage Storage) (*Service, error) { + if storage == nil { + return nil, errors.New("agents: storage is required") + } + return &Service{storage: storage}, nil +} + +// CreateCommand saves a new Agent. Configuration is the resolved saved +// configuration; ModelProvider is the bundle its x_agents_core.model_provider +// describes. Each call creates a new Agent. +type CreateCommand struct { + TenantID string + Metadata map[string]string + Configuration json.RawMessage + ModelProvider *v1.ModelProviderInput +} + +func (s *Service) Create(ctx context.Context, cmd CreateCommand) (Agent, error) { + metadata, err := encodeMetadata(cmd.Metadata) + if err != nil { + return Agent{}, err + } + configuration, err := createConfiguration(cmd.Configuration) + if err != nil { + return Agent{}, err + } + if err := validateModelExecution(configuration, cmd.ModelProvider); err != nil { + return Agent{}, err + } + return s.storage.CreateAgent(ctx, NewAgent{TenantID: cmd.TenantID, Metadata: metadata, Configuration: configuration, ModelProvider: cmd.ModelProvider}) +} + +// UpdateCommand changes the supplied fields of an Agent. Configuration holds +// the supplied top-level fields only (see mergeConfiguration). Metadata nil +// keeps the saved map; a supplied map replaces it. ModelProvider nil keeps the +// saved bundle. +type UpdateCommand struct { + TenantID string + AgentID string + Configuration json.RawMessage + Metadata *map[string]string + ModelProvider *ModelProviderChange +} + +// Update validates the supplied fields before it looks the Agent up, so an +// invalid request is rejected the same way for a missing Agent. An update that +// supplies nothing still advances UpdatedAt. +func (s *Service) Update(ctx context.Context, cmd UpdateCommand) (Agent, error) { + patch, err := decodePatch(cmd.Configuration) + if err != nil { + return Agent{}, err + } + var metadata json.RawMessage + if cmd.Metadata != nil { + if metadata, err = encodeMetadata(*cmd.Metadata); err != nil { + return Agent{}, err + } + } + var updated Agent + err = s.storage.WithAgentUpdate(ctx, cmd.TenantID, cmd.AgentID, func(tx UpdateTx) error { + current, err := tx.LoadAgent() + if err != nil { + return err + } + revision, err := revise(current, patch, metadata, cmd.ModelProvider) + if err != nil { + return err + } + updated, err = tx.ApplyRevision(revision) + return err + }) + if err != nil { + return Agent{}, err + } + return updated, nil +} + +// revise decides an Agent's next state. metadata nil keeps the saved map. +func revise(current Agent, patch map[string]json.RawMessage, metadata json.RawMessage, change *ModelProviderChange) (Revision, error) { + configuration, err := mergeConfiguration(current.Configuration, patch) + if err != nil { + return Revision{}, err + } + var provider *v1.ModelProviderInput + if change != nil { + provider = change.Provider + } + if err := validateModelExecution(configuration, provider); err != nil { + return Revision{}, err + } + if metadata == nil { + if metadata, err = encodeMetadata(current.Metadata); err != nil { + return Revision{}, err + } + } + return Revision{Metadata: metadata, Configuration: configuration, ModelProvider: change}, nil +} + +// DeleteCommand removes an Agent. Session snapshots taken from it stay. +type DeleteCommand struct { + TenantID string + AgentID string +} + +// Delete returns the deleted Agent's ID. +func (s *Service) Delete(ctx context.Context, cmd DeleteCommand) (string, error) { + return s.storage.DeleteAgent(ctx, cmd.TenantID, cmd.AgentID) +} diff --git a/services/core/internal/agents/service_test.go b/services/core/internal/agents/service_test.go new file mode 100644 index 000000000..e11d07a6b --- /dev/null +++ b/services/core/internal/agents/service_test.go @@ -0,0 +1,167 @@ +package agents + +import ( + "context" + "encoding/json" + "errors" + "strings" + "testing" + + v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" +) + +// fakeStorage is a strict Storage fake: a call whose func is nil fails the test. +type fakeStorage struct { + t *testing.T + createAgent func(context.Context, NewAgent) (Agent, error) + withAgentUpdate func(context.Context, string, string, func(UpdateTx) error) error + deleteAgent func(context.Context, string, string) (string, error) +} + +func (f *fakeStorage) CreateAgent(ctx context.Context, agent NewAgent) (Agent, error) { + if f.createAgent == nil { + f.t.Fatal("unexpected call to CreateAgent") + } + return f.createAgent(ctx, agent) +} + +func (f *fakeStorage) WithAgentUpdate(ctx context.Context, tenantID, agentID string, decide func(UpdateTx) error) error { + if f.withAgentUpdate == nil { + f.t.Fatal("unexpected call to WithAgentUpdate") + } + return f.withAgentUpdate(ctx, tenantID, agentID, decide) +} + +func (f *fakeStorage) DeleteAgent(ctx context.Context, tenantID, agentID string) (string, error) { + if f.deleteAgent == nil { + f.t.Fatal("unexpected call to DeleteAgent") + } + return f.deleteAgent(ctx, tenantID, agentID) +} + +// fakeUpdateTx serves one locked Agent and records the applied revision. +type fakeUpdateTx struct { + current Agent + loadErr error + applied *Revision +} + +func (tx *fakeUpdateTx) LoadAgent() (Agent, error) { return tx.current, tx.loadErr } + +func (tx *fakeUpdateTx) ApplyRevision(revision Revision) (Agent, error) { + tx.applied = &revision + return Agent{ID: tx.current.ID, Configuration: revision.Configuration}, nil +} + +func newTestService(t *testing.T, storage *fakeStorage) *Service { + t.Helper() + storage.t = t + service, err := NewService(storage) + if err != nil { + t.Fatal(err) + } + return service +} + +func TestNewServiceRequiresStorage(t *testing.T) { + if service, err := NewService(nil); err == nil || service != nil { + t.Fatalf("NewService(nil) = %v, %v", service, err) + } +} + +func TestCreateValidatesBeforeStorage(t *testing.T) { + service := newTestService(t, &fakeStorage{}) + for _, command := range []CreateCommand{ + {Configuration: json.RawMessage(`[]`)}, + {Configuration: json.RawMessage(`{"model":"a"}`), Metadata: map[string]string{"k": strings.Repeat("v", 70*1024)}}, + {Configuration: json.RawMessage(`{"model":"a","x_agents_core":{"harness":"claude_sdk","model_provider":` + testProvider + `}}`)}, + } { + if _, err := service.Create(context.Background(), command); !errors.Is(err, ErrInvalidInput) { + t.Errorf("Create = %v", err) + } + } +} + +func TestCreatePassesNormalizedAgent(t *testing.T) { + provider := &v1.ModelProviderInput{Protocol: "responses", BaseURL: "https://example.test/v1", APIKey: "secret"} + var saved NewAgent + service := newTestService(t, &fakeStorage{createAgent: func(_ context.Context, agent NewAgent) (Agent, error) { + saved = agent + return Agent{ID: "agent"}, nil + }}) + if _, err := service.Create(context.Background(), CreateCommand{TenantID: "tenant", Configuration: json.RawMessage(`{ "model": "a" }`), ModelProvider: provider}); err != nil { + t.Fatal(err) + } + if saved.TenantID != "tenant" || string(saved.Configuration) != `{"model":"a"}` || string(saved.Metadata) != `{}` || saved.ModelProvider != provider { + t.Fatalf("saved = %+v", saved) + } +} + +func TestUpdateValidatesBeforeLookup(t *testing.T) { + service := newTestService(t, &fakeStorage{}) + large := map[string]string{"k": strings.Repeat("v", 70*1024)} + for _, command := range []UpdateCommand{ + {Configuration: json.RawMessage(`[]`)}, + {Configuration: json.RawMessage(`{} {}`)}, + {Metadata: &large}, + } { + if _, err := service.Update(context.Background(), command); !errors.Is(err, ErrInvalidInput) { + t.Errorf("Update = %v", err) + } + } +} + +func TestUpdateDecidesOverLockedAgent(t *testing.T) { + current := Agent{ID: "agent", Metadata: map[string]string{"team": "core"}, + Configuration: json.RawMessage(`{"model":"a","x_agents_core":{"harness":"codex","harness_config":{"k":1}}}`)} + change := &ModelProviderChange{} + for _, tc := range []struct { + name string + command UpdateCommand + tx *fakeUpdateTx + wantErr error + want string + }{ + {"merged revision", UpdateCommand{TenantID: "tenant", AgentID: "agent", Configuration: json.RawMessage(`{"model":"b"}`), ModelProvider: change}, + &fakeUpdateTx{current: current}, nil, `{"model":"b","x_agents_core":{"harness":"codex","harness_config":{}}}`}, + {"missing Agent", UpdateCommand{TenantID: "tenant", AgentID: "agent"}, &fakeUpdateTx{loadErr: ErrNotFound}, ErrNotFound, ""}, + {"invalid merged state is not applied", UpdateCommand{TenantID: "tenant", AgentID: "agent", Configuration: json.RawMessage(`{"x_agents_core":{"harness":"codex","harness_config":{"unknown_option":true}}}`)}, + &fakeUpdateTx{current: current}, ErrInvalidInput, ""}, + } { + t.Run(tc.name, func(t *testing.T) { + service := newTestService(t, &fakeStorage{withAgentUpdate: func(_ context.Context, tenantID, agentID string, decide func(UpdateTx) error) error { + if tenantID != "tenant" || agentID != "agent" { + t.Fatalf("locked %s/%s", tenantID, agentID) + } + return decide(tc.tx) + }}) + updated, err := service.Update(context.Background(), tc.command) + if !errors.Is(err, tc.wantErr) { + t.Fatalf("Update = %v, want %v", err, tc.wantErr) + } + if tc.wantErr != nil { + if tc.tx.applied != nil { + t.Fatal("a rejected update was applied") + } + return + } + applied := tc.tx.applied + if applied == nil || string(applied.Configuration) != tc.want || string(updated.Configuration) != tc.want || + string(applied.Metadata) != `{"team":"core"}` || applied.ModelProvider != change { + t.Fatalf("applied = %+v", applied) + } + }) + } +} + +func TestDeletePassesThrough(t *testing.T) { + service := newTestService(t, &fakeStorage{deleteAgent: func(_ context.Context, tenantID, agentID string) (string, error) { + if tenantID != "tenant" { + t.Fatalf("tenant = %s", tenantID) + } + return agentID, nil + }}) + if id, err := service.Delete(context.Background(), DeleteCommand{TenantID: "tenant", AgentID: "agent"}); err != nil || id != "agent" { + t.Fatalf("Delete = %s, %v", id, err) + } +} diff --git a/services/core/internal/agents/storage.go b/services/core/internal/agents/storage.go new file mode 100644 index 000000000..16969e870 --- /dev/null +++ b/services/core/internal/agents/storage.go @@ -0,0 +1,62 @@ +package agents + +import ( + "context" + "encoding/json" + + v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" +) + +// Storage persists Agent writes. Each method runs in one transaction that also +// records the write audit. An agent ID that cannot name an Agent is a missing +// Agent. +type Storage interface { + // CreateAgent assigns the Agent's ID and saves it with its sealed model + // provider bundle. Sealing without a credential key is + // credentialcrypto.ErrUnavailable. + CreateAgent(context.Context, NewAgent) (Agent, error) + // WithAgentUpdate locks the tenant's Agent and runs update; the revision + // it applies commits only when update returns nil. + WithAgentUpdate(ctx context.Context, tenantID, agentID string, update func(UpdateTx) error) error + // DeleteAgent removes the Agent and its model provider bundle and returns + // the deleted ID. + DeleteAgent(ctx context.Context, tenantID, agentID string) (string, error) +} + +// UpdateTx is one locked Agent inside WithAgentUpdate. +type UpdateTx interface { + // LoadAgent returns the locked Agent, or ErrNotFound. + LoadAgent() (Agent, error) + // ApplyRevision stores the revision and returns the updated Agent. + ApplyRevision(Revision) (Agent, error) +} + +// NewAgent is a validated Agent to create. +type NewAgent struct { + TenantID string + // Metadata is the encoded metadata object. + Metadata json.RawMessage + Configuration json.RawMessage + // ModelProvider is sealed beside the Agent; nil saves none. + ModelProvider *v1.ModelProviderInput +} + +// Revision is the complete next state of an Agent. +type Revision struct { + // Metadata is the encoded metadata object. + Metadata json.RawMessage + Configuration json.RawMessage + // ModelProvider nil keeps the saved bundle. + ModelProvider *ModelProviderChange +} + +// Reader reads saved Agents of one tenant. An agent ID or list cursor that +// cannot name an Agent is a missing Agent: ErrNotFound. +type Reader interface { + GetAgent(ctx context.Context, tenantID, agentID string) (Agent, error) + ListAgents(context.Context, ListQuery) (Page, error) + // GetAgentWithModelProvider reads the Agent and its opened model provider + // bundle from one snapshot. The bundle is nil when the Agent has none; a + // bundle that cannot be opened is credentialcrypto.ErrUnavailable. + GetAgentWithModelProvider(ctx context.Context, tenantID, agentID string) (Agent, *v1.ModelProviderInput, error) +} diff --git a/services/core/internal/api/admin_resources_test.go b/services/core/internal/api/admin_resources_test.go index 151806cc9..b8f7cb365 100644 --- a/services/core/internal/api/admin_resources_test.go +++ b/services/core/internal/api/admin_resources_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/adminaudit" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/identity" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" ) @@ -58,23 +59,23 @@ type adminReadFixture struct { administrative, impersonated bool } -func (s *adminReadFixture) ListAgents(ctx context.Context, tenant, after string, limit int, ascending bool) (store.AgentPage, error) { - s.seenTenant = tenant +func (s *adminReadFixture) ListAgents(ctx context.Context, query agents.ListQuery) (agents.Page, error) { + s.seenTenant = query.TenantID _, s.administrative = adminaudit.FromContext(ctx) s.impersonated = ctx.Value(principalContextKey{}) != nil - return store.AgentPage{Agents: []store.SavedAgent{}}, nil + return agents.Page{Agents: []agents.Agent{}}, nil } -func (s *adminReadFixture) DeleteAgent(ctx context.Context, tenant, id string) (string, error) { - s.seenTenant = tenant +func (s *adminReadFixture) DeleteAgent(ctx context.Context, command agents.DeleteCommand) (string, error) { + s.seenTenant = command.TenantID _, s.administrative = adminaudit.FromContext(ctx) s.impersonated = ctx.Value(principalContextKey{}) != nil - return id, nil + return command.AgentID, nil } func TestAdminResourcesHaveExplicitTargetWithoutCallerImpersonation(t *testing.T) { key := callerBinding() deps, fakes := managementFakes(t, key) resources := &adminReadFixture{} - fakes.agents.listAgents, fakes.agents.deleteAgent = resources.ListAgents, resources.DeleteAgent + fakes.agentsReader.listAgents, fakes.agents.delete = resources.ListAgents, resources.DeleteAgent h := newTestHandler(t, deps) base := "/core/v1/projects/" + managementProjectID for _, test := range []struct { diff --git a/services/core/internal/api/agents.go b/services/core/internal/api/agents.go index 04d9e480e..f2f1cb4af 100644 --- a/services/core/internal/api/agents.go +++ b/services/core/internal/api/agents.go @@ -6,21 +6,23 @@ import ( "net/http" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/go-chi/chi/v5" - "github.com/google/uuid" ) -// Agents manages saved Agents. GetAgentForSession reads a saved Agent for -// Session creation together with its decrypted model provider when the -// Session inherits it. +// Agents runs the saved Agent writes. type Agents interface { - DeleteAgent(context.Context, string, string) (string, error) - UpdateAgent(context.Context, string, string, store.UpdateAgentInput) (store.SavedAgent, error) - ListAgents(context.Context, string, string, int, bool) (store.AgentPage, error) - CreateAgent(context.Context, string, store.CreateAgentInput) (store.SavedAgent, error) - GetAgent(context.Context, string, string) (store.SavedAgent, error) - GetAgentForSession(context.Context, string, string, bool) (store.SavedAgent, *v1.ModelProviderInput, error) + Create(context.Context, agents.CreateCommand) (agents.Agent, error) + Update(context.Context, agents.UpdateCommand) (agents.Agent, error) + Delete(context.Context, agents.DeleteCommand) (string, error) +} + +// AgentsReader reads saved Agents. Session creation reads an Agent with +// GetAgentWithModelProvider when the Session inherits its model provider. +type AgentsReader interface { + GetAgent(ctx context.Context, tenantID, agentID string) (agents.Agent, error) + ListAgents(context.Context, agents.ListQuery) (agents.Page, error) + GetAgentWithModelProvider(ctx context.Context, tenantID, agentID string) (agents.Agent, *v1.ModelProviderInput, error) } // @Summary Create a reusable Agent @@ -51,16 +53,17 @@ func (h *Handler) createAgent(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "invalid_request", "Request must be a JSON object containing supported fields.") return } - input, err := resolveSavedAgent(request) + command, err := resolveSavedAgent(request) if err != nil { if !writeFieldError(w, err) { writeError(w, http.StatusBadRequest, "unsupported_or_invalid_configuration", err.Error()) } return } - agent, err := h.Agents.CreateAgent(r.Context(), tenantID(r), input) + command.TenantID = tenantID(r) + agent, err := h.Agents.Create(r.Context(), command) if err != nil { - writeStoreError(w, r, err) + writeAgentsError(w, r, err) return } h.respondAgentStatus(w, r, agent, http.StatusCreated) @@ -77,33 +80,28 @@ func (h *Handler) createAgent(w http.ResponseWriter, r *http.Request) { // @Failure 400,401,404,500 {object} v1.ErrorResponse // @Router /agents/{agent_id} [get] func (h *Handler) getAgent(w http.ResponseWriter, r *http.Request) { - id := chi.URLParam(r, "agent_id") - if !validAgentID(id) { - writeStoreError(w, r, store.ErrNotFound) - return - } - agent, err := h.Agents.GetAgent(r.Context(), tenantID(r), id) + agent, err := h.AgentsReader.GetAgent(r.Context(), tenantID(r), chi.URLParam(r, "agent_id")) if err != nil { - writeStoreError(w, r, err) + writeAgentsError(w, r, err) return } h.respondAgent(w, r, agent) } -func (h *Handler) respondAgent(w http.ResponseWriter, r *http.Request, agent store.SavedAgent) { +func (h *Handler) respondAgent(w http.ResponseWriter, r *http.Request, agent agents.Agent) { h.respondAgentStatus(w, r, agent, http.StatusOK) } -func (h *Handler) respondAgentStatus(w http.ResponseWriter, r *http.Request, agent store.SavedAgent, status int) { +func (h *Handler) respondAgentStatus(w http.ResponseWriter, r *http.Request, agent agents.Agent, status int) { response, err := agentResponse(agent) if err != nil { - writeStoreError(w, r, err) + writeAgentsError(w, r, err) return } writeJSON(w, status, response) } -func agentResponse(agent store.SavedAgent) (v1.SavedAgent, error) { +func agentResponse(agent agents.Agent) (v1.SavedAgent, error) { var response v1.SavedAgent if err := json.Unmarshal(agent.Configuration, &response.SavedAgentConfiguration); err != nil { return response, err @@ -113,8 +111,3 @@ func agentResponse(agent store.SavedAgent) (v1.SavedAgent, error) { response.CreatedAt, response.UpdatedAt = agent.CreatedAt.Unix(), agent.UpdatedAt.Unix() return response, nil } - -func validAgentID(id string) bool { - parsed, err := uuid.Parse(id) - return err == nil && parsed != uuid.Nil -} diff --git a/services/core/internal/api/agents_delete.go b/services/core/internal/api/agents_delete.go index b50f148dd..f0fbe30f1 100644 --- a/services/core/internal/api/agents_delete.go +++ b/services/core/internal/api/agents_delete.go @@ -5,7 +5,7 @@ import ( "net/http" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/go-chi/chi/v5" ) @@ -28,14 +28,9 @@ func (h *Handler) deleteAgent(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "unsupported_parameter", "Agent deletion does not accept a request body.") return } - id := chi.URLParam(r, "agent_id") - if !validAgentID(id) { - writeStoreError(w, r, store.ErrNotFound) - return - } - deleted, err := h.Agents.DeleteAgent(r.Context(), tenantID(r), id) + deleted, err := h.Agents.Delete(r.Context(), agents.DeleteCommand{TenantID: tenantID(r), AgentID: chi.URLParam(r, "agent_id")}) if err != nil { - writeStoreError(w, r, err) + writeAgentsError(w, r, err) return } writeJSON(w, http.StatusOK, v1.AgentDeleted{ID: deleted, Object: "agent.deleted", Deleted: true}) diff --git a/services/core/internal/api/agents_list.go b/services/core/internal/api/agents_list.go index 699a1b03c..3698cb1be 100644 --- a/services/core/internal/api/agents_list.go +++ b/services/core/internal/api/agents_list.go @@ -4,6 +4,7 @@ import ( "net/http" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" ) // @Summary List reusable Agents @@ -23,16 +24,16 @@ func (h *Handler) listAgents(w http.ResponseWriter, r *http.Request) { if !ok { return } - page, err := h.Agents.ListAgents(r.Context(), tenantID(r), options.after, options.limit, options.ascending) + page, err := h.AgentsReader.ListAgents(r.Context(), agents.ListQuery{TenantID: tenantID(r), After: options.after, Limit: options.limit, Ascending: options.ascending}) if err != nil { - writeStoreError(w, r, err) + writeAgentsError(w, r, err) return } response := v1.SavedAgentList{Object: "list", Data: make([]v1.SavedAgent, 0, len(page.Agents)), HasMore: page.NextCursor != ""} for _, agent := range page.Agents { item, err := agentResponse(agent) if err != nil { - writeStoreError(w, r, err) + writeAgentsError(w, r, err) return } response.Data = append(response.Data, item) diff --git a/services/core/internal/api/agents_update.go b/services/core/internal/api/agents_update.go index ffd9e756c..03843821c 100644 --- a/services/core/internal/api/agents_update.go +++ b/services/core/internal/api/agents_update.go @@ -6,7 +6,7 @@ import ( "net/http" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/go-chi/chi/v5" ) @@ -27,49 +27,51 @@ func (h *Handler) updateAgent(w http.ResponseWriter, r *http.Request) { if !ok { return } - input, err := resolveAgentUpdate(raw) + command, err := resolveAgentUpdate(raw) if err != nil { if !writeFieldError(w, err) { writeError(w, http.StatusBadRequest, "unsupported_or_invalid_configuration", err.Error()) } return } - updated, err := h.Agents.UpdateAgent(r.Context(), tenantID(r), chi.URLParam(r, "agent_id"), input) + command.TenantID, command.AgentID = tenantID(r), chi.URLParam(r, "agent_id") + updated, err := h.Agents.Update(r.Context(), command) if err != nil { - writeStoreError(w, r, err) + writeAgentsError(w, r, err) return } h.respondAgent(w, r, updated) } -func resolveAgentUpdate(raw []byte) (store.UpdateAgentInput, error) { +// resolveAgentUpdate returns the command without its tenant and Agent. +func resolveAgentUpdate(raw []byte) (agents.UpdateCommand, error) { if err := metadataTypeError(raw); err != nil { - return store.UpdateAgentInput{}, err + return agents.UpdateCommand{}, err } if err := validateSavedAgentBody(raw, savedAgentUpdate); err != nil { - return store.UpdateAgentInput{}, err + return agents.UpdateCommand{}, err } if err := validateSavedCoreInput(raw); err != nil { - return store.UpdateAgentInput{}, err + return agents.UpdateCommand{}, err } var request v1.UpdateAgentRequest if decodeInputObject(raw, &request, "model", "name", "instructions", "metadata", "multi_agent", "reasoning", "service_tier", "text", "tools", "x_agents_core") != nil { - return store.UpdateAgentInput{}, errors.New("Request must be a JSON object containing supported fields.") + return agents.UpdateCommand{}, errors.New("Request must be a JSON object containing supported fields.") } var fields map[string]json.RawMessage if err := json.Unmarshal(raw, &fields); err != nil { - return store.UpdateAgentInput{}, err + return agents.UpdateCommand{}, err } if _, supplied := fields["model"]; supplied && request.Model == nil { - return store.UpdateAgentInput{}, errors.New("model must be a string when supplied.") + return agents.UpdateCommand{}, errors.New("model must be a string when supplied.") } normalized, err := resolveSavedFields(v1.CreateAgentRequest(request)) if err != nil { - return store.UpdateAgentInput{}, err + return agents.UpdateCommand{}, err } var patch map[string]json.RawMessage if err := json.Unmarshal(normalized.Configuration, &patch); err != nil { - return store.UpdateAgentInput{}, err + return agents.UpdateCommand{}, err } if _, supplied := fields["x_agents_core"]; supplied && request.XAgentsCore == nil { patch["x_agents_core"] = json.RawMessage(`null`) @@ -79,19 +81,21 @@ func resolveAgentUpdate(raw []byte) (store.UpdateAgentInput, error) { delete(patch, field) } } - result := store.UpdateAgentInput{ModelProvider: normalized.ModelProvider} + var result agents.UpdateCommand if extension, supplied := fields["x_agents_core"]; supplied { - if request.XAgentsCore == nil { - result.ModelProviderSet = true - } else { - _, coreFields := orderedMembers(extension) - _, result.ModelProviderSet = coreFields["model_provider"] - if result.ModelProviderSet && request.XAgentsCore.ModelProvider == nil { + _, coreFields := orderedMembers(extension) + _, providerSupplied := coreFields["model_provider"] + switch { + case request.XAgentsCore == nil: + result.ModelProvider = &agents.ModelProviderChange{} + case providerSupplied: + result.ModelProvider = &agents.ModelProviderChange{Provider: normalized.ModelProvider} + if request.XAgentsCore.ModelProvider == nil { _, corePatch := orderedMembers(patch["x_agents_core"]) corePatch["model_provider"] = json.RawMessage(`null`) patch["x_agents_core"], err = json.Marshal(corePatch) if err != nil { - return store.UpdateAgentInput{}, err + return agents.UpdateCommand{}, err } } } diff --git a/services/core/internal/api/configuration_validation_test.go b/services/core/internal/api/configuration_validation_test.go index fc1c63964..2297b7b32 100644 --- a/services/core/internal/api/configuration_validation_test.go +++ b/services/core/internal/api/configuration_validation_test.go @@ -9,7 +9,7 @@ import ( "testing" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/google/uuid" ) @@ -20,26 +20,26 @@ type savedConfigurationStore struct { agents map[string]string } -func (s *savedConfigurationStore) GetAgent(_ context.Context, tenant, id string) (store.SavedAgent, error) { +func (s *savedConfigurationStore) GetAgent(_ context.Context, tenant, id string) (agents.Agent, error) { configuration, ok := s.agents[id] if !ok { - return store.SavedAgent{}, store.ErrNotFound + return agents.Agent{}, agents.ErrNotFound } - return store.SavedAgent{ID: id, TenantID: tenant, Configuration: json.RawMessage(configuration), Metadata: map[string]string{}}, nil + return agents.Agent{ID: id, TenantID: tenant, Configuration: json.RawMessage(configuration), Metadata: map[string]string{}}, nil } -// GetAgentForSession reads the saved Agent for Session creation; these records -// carry no model provider. -func (s *savedConfigurationStore) GetAgentForSession(ctx context.Context, tenant, id string, _ bool) (store.SavedAgent, *v1.ModelProviderInput, error) { +// GetAgentWithModelProvider reads the saved Agent for Session creation; these +// records carry no model provider. +func (s *savedConfigurationStore) GetAgentWithModelProvider(ctx context.Context, tenant, id string) (agents.Agent, *v1.ModelProviderInput, error) { agent, err := s.GetAgent(ctx, tenant, id) return agent, nil, err } -func configurationHandler(t *testing.T, agents map[string]string) (http.Handler, *savedConfigurationStore) { +func configurationHandler(t *testing.T, saved map[string]string) (http.Handler, *savedConfigurationStore) { t.Helper() - s := &savedConfigurationStore{validationStore: &validationStore{}, agents: agents} + s := &savedConfigurationStore{validationStore: &validationStore{}, agents: saved} h, _, _ := testHandler(t, s.serve, func(_ *Dependencies, f *testFakes) { - f.agents.getAgent, f.agents.getAgentForSession = s.GetAgent, s.GetAgentForSession + f.agentsReader.getAgent, f.agentsReader.getAgentWithModelProvider = s.GetAgent, s.GetAgentWithModelProvider }) return h, s } diff --git a/services/core/internal/api/dependencies.go b/services/core/internal/api/dependencies.go index 94227f306..97f01b944 100644 --- a/services/core/internal/api/dependencies.go +++ b/services/core/internal/api/dependencies.go @@ -36,6 +36,7 @@ type Dependencies struct { Skills Skills EnvironmentTemplates EnvironmentTemplates Agents Agents + AgentsReader AgentsReader Sessions Sessions SessionEvents SessionEvents SessionHistory SessionHistory @@ -111,7 +112,8 @@ func (d Dependencies) validate() error { if err := required( field{"InstallationBindings", d.InstallationBindings}, field{"Projects", d.Projects}, field{"Vaults", d.Vaults}, field{"ModelProviders", d.ModelProviders}, field{"Files", d.Files}, field{"Skills", d.Skills}, - field{"EnvironmentTemplates", d.EnvironmentTemplates}, field{"Agents", d.Agents}, field{"Sessions", d.Sessions}, + field{"EnvironmentTemplates", d.EnvironmentTemplates}, field{"Agents", d.Agents}, field{"AgentsReader", d.AgentsReader}, + field{"Sessions", d.Sessions}, field{"SessionEvents", d.SessionEvents}, field{"SessionHistory", d.SessionHistory}, field{"Subagents", d.Subagents}, field{"Artifacts", d.Artifacts}, field{"SessionAdmin", d.SessionAdmin}, field{"Environments", d.Environments}, field{"ExecutorConnections", d.ExecutorConnections}, field{"Admin", d.Admin}, field{"AdminAudit", d.AdminAudit}, field{"WriteAudit", d.WriteAudit}, diff --git a/services/core/internal/api/dependencies_test.go b/services/core/internal/api/dependencies_test.go index 2d09e6b9f..b0a6d4b3d 100644 --- a/services/core/internal/api/dependencies_test.go +++ b/services/core/internal/api/dependencies_test.go @@ -22,6 +22,7 @@ type testFakes struct { skills *fakeSkills environmentTemplates *fakeEnvironmentTemplates agents *fakeAgents + agentsReader *fakeAgentsReader sessions *fakeSessions sessionEvents *fakeSessionEvents sessionHistory *fakeSessionHistory @@ -55,7 +56,7 @@ func testDependencies(t testing.TB) (Dependencies, *testFakes) { f := &testFakes{ projects: &fakeProjects{t: t}, vaults: &fakeVaults{t: t}, modelProviders: &fakeModelProviders{t: t}, files: &fakeFiles{t: t}, skills: &fakeSkills{t: t}, environmentTemplates: &fakeEnvironmentTemplates{t: t}, - agents: &fakeAgents{t: t}, sessions: &fakeSessions{t: t}, sessionEvents: &fakeSessionEvents{t: t}, + agents: &fakeAgents{t: t}, agentsReader: &fakeAgentsReader{t: t}, sessions: &fakeSessions{t: t}, sessionEvents: &fakeSessionEvents{t: t}, sessionHistory: &fakeSessionHistory{t: t}, subagents: &fakeSubagents{t: t}, artifacts: &fakeArtifacts{t: t}, sessionAdmin: &fakeSessionAdmin{t: t}, environments: &fakeEnvironments{t: t}, executorConnections: &fakeExecutorConnections{t: t}, admin: &fakeAdmin{t: t}, adminAudit: &fakeAdminAudit{t: t}, writeAudit: &fakeWriteAudit{t: t}, metrics: &fakeMetrics{t: t}, @@ -66,7 +67,7 @@ func testDependencies(t testing.TB) (Dependencies, *testFakes) { return Dependencies{ Engine: "codex", CoreKeys: coreKeys(t, "admin"), InstallationBindings: f.installationBindings, Projects: f.projects, Vaults: f.vaults, ModelProviders: f.modelProviders, Files: f.files, Skills: f.skills, - EnvironmentTemplates: f.environmentTemplates, Agents: f.agents, Sessions: f.sessions, SessionEvents: f.sessionEvents, + EnvironmentTemplates: f.environmentTemplates, Agents: f.agents, AgentsReader: f.agentsReader, Sessions: f.sessions, SessionEvents: f.sessionEvents, SessionHistory: f.sessionHistory, Subagents: f.subagents, Artifacts: f.artifacts, SessionAdmin: f.sessionAdmin, Environments: f.environments, ExecutorConnections: f.executorConnections, Admin: f.admin, AdminAudit: f.adminAudit, WriteAudit: f.writeAudit, Metrics: f.metrics, RuntimeObservations: f.runtimeObservations, RuntimeHistory: f.runtimeHistory, diff --git a/services/core/internal/api/errors_agents.go b/services/core/internal/api/errors_agents.go new file mode 100644 index 000000000..1e891cdc3 --- /dev/null +++ b/services/core/internal/api/errors_agents.go @@ -0,0 +1,25 @@ +package api + +import ( + "errors" + "net/http" + + "github.com/MiniMax-AI/OpenAgentCore/internal/obs/log" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" +) + +// writeAgentsError reports an error of the Agent operations. +func writeAgentsError(w http.ResponseWriter, r *http.Request, err error) { + if writeTextValueError(w, r, err) || writeAuditSourceError(w, r, err) || writeCredentialUnavailableError(w, r, err) { + return + } + switch { + case errors.Is(err, agents.ErrNotFound): + writeError(w, http.StatusNotFound, "not_found_error", "Resource not found.") + case errors.Is(err, agents.ErrInvalidInput): + writeError(w, http.StatusBadRequest, "invalid_request", "Invalid resource identifier or request limits.") + default: + log.Ctx(r.Context()).Error("oac-core persistence operation failed") + writeError(w, http.StatusInternalServerError, "internal_error", "The operation could not be completed.") + } +} diff --git a/services/core/internal/api/fakes_test.go b/services/core/internal/api/fakes_test.go index e9a20ed72..ffd7b3925 100644 --- a/services/core/internal/api/fakes_test.go +++ b/services/core/internal/api/fakes_test.go @@ -9,6 +9,7 @@ import ( v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/adminaudit" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/coremetrics" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/environmentconfig" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/identity" @@ -92,55 +93,59 @@ func (f *fakeAdmission) SubmitInputs(a0 context.Context, a1 string, a2 string, a } type fakeAgents struct { - t testing.TB - deleteAgent func(context.Context, string, string) (string, error) - updateAgent func(context.Context, string, string, store.UpdateAgentInput) (store.SavedAgent, error) - listAgents func(context.Context, string, string, int, bool) (store.AgentPage, error) - createAgent func(context.Context, string, store.CreateAgentInput) (store.SavedAgent, error) - getAgent func(context.Context, string, string) (store.SavedAgent, error) - getAgentForSession func(context.Context, string, string, bool) (store.SavedAgent, *v1.ModelProviderInput, error) + t testing.TB + create func(context.Context, agents.CreateCommand) (agents.Agent, error) + update func(context.Context, agents.UpdateCommand) (agents.Agent, error) + delete func(context.Context, agents.DeleteCommand) (string, error) } -func (f *fakeAgents) DeleteAgent(a0 context.Context, a1 string, a2 string) (string, error) { - if f.deleteAgent == nil { - unexpectedCall(f.t, "DeleteAgent") +func (f *fakeAgents) Create(a0 context.Context, a1 agents.CreateCommand) (agents.Agent, error) { + if f.create == nil { + unexpectedCall(f.t, "Create") } - return f.deleteAgent(a0, a1, a2) + return f.create(a0, a1) } -func (f *fakeAgents) UpdateAgent(a0 context.Context, a1 string, a2 string, a3 store.UpdateAgentInput) (store.SavedAgent, error) { - if f.updateAgent == nil { - unexpectedCall(f.t, "UpdateAgent") +func (f *fakeAgents) Update(a0 context.Context, a1 agents.UpdateCommand) (agents.Agent, error) { + if f.update == nil { + unexpectedCall(f.t, "Update") } - return f.updateAgent(a0, a1, a2, a3) + return f.update(a0, a1) } -func (f *fakeAgents) ListAgents(a0 context.Context, a1 string, a2 string, a3 int, a4 bool) (store.AgentPage, error) { - if f.listAgents == nil { - unexpectedCall(f.t, "ListAgents") +func (f *fakeAgents) Delete(a0 context.Context, a1 agents.DeleteCommand) (string, error) { + if f.delete == nil { + unexpectedCall(f.t, "Delete") } - return f.listAgents(a0, a1, a2, a3, a4) + return f.delete(a0, a1) } -func (f *fakeAgents) CreateAgent(a0 context.Context, a1 string, a2 store.CreateAgentInput) (store.SavedAgent, error) { - if f.createAgent == nil { - unexpectedCall(f.t, "CreateAgent") - } - return f.createAgent(a0, a1, a2) +type fakeAgentsReader struct { + t testing.TB + getAgent func(context.Context, string, string) (agents.Agent, error) + listAgents func(context.Context, agents.ListQuery) (agents.Page, error) + getAgentWithModelProvider func(context.Context, string, string) (agents.Agent, *v1.ModelProviderInput, error) } -func (f *fakeAgents) GetAgent(a0 context.Context, a1 string, a2 string) (store.SavedAgent, error) { +func (f *fakeAgentsReader) GetAgent(a0 context.Context, a1 string, a2 string) (agents.Agent, error) { if f.getAgent == nil { unexpectedCall(f.t, "GetAgent") } return f.getAgent(a0, a1, a2) } -func (f *fakeAgents) GetAgentForSession(a0 context.Context, a1 string, a2 string, a3 bool) (store.SavedAgent, *v1.ModelProviderInput, error) { - if f.getAgentForSession == nil { - unexpectedCall(f.t, "GetAgentForSession") +func (f *fakeAgentsReader) ListAgents(a0 context.Context, a1 agents.ListQuery) (agents.Page, error) { + if f.listAgents == nil { + unexpectedCall(f.t, "ListAgents") + } + return f.listAgents(a0, a1) +} + +func (f *fakeAgentsReader) GetAgentWithModelProvider(a0 context.Context, a1 string, a2 string) (agents.Agent, *v1.ModelProviderInput, error) { + if f.getAgentWithModelProvider == nil { + unexpectedCall(f.t, "GetAgentWithModelProvider") } - return f.getAgentForSession(a0, a1, a2, a3) + return f.getAgentWithModelProvider(a0, a1, a2) } type fakeArtifacts struct { diff --git a/services/core/internal/api/handler.go b/services/core/internal/api/handler.go index b430f8421..e6a4f3001 100644 --- a/services/core/internal/api/handler.go +++ b/services/core/internal/api/handler.go @@ -180,7 +180,7 @@ func (h *Handler) createSession(w http.ResponseWriter, r *http.Request) { saved, inheritedProvider, err := h.sessionAgentDefaults(r.Context(), tenantID(r), input) if err != nil { if !h.recoverSessionCreation(w, r, key, creationRequest, input.Stream) { - writeStoreError(w, r, err) + writeAgentsError(w, r, err) } return } diff --git a/services/core/internal/api/handler_test.go b/services/core/internal/api/handler_test.go index df21cf040..330dcd6a1 100644 --- a/services/core/internal/api/handler_test.go +++ b/services/core/internal/api/handler_test.go @@ -12,6 +12,7 @@ import ( "time" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/identity" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" "github.com/google/uuid" @@ -154,7 +155,14 @@ func TestHTTPRejectsUntrustedOrUnsupportedRequests(t *testing.T) { } { t.Run(test.name, func(t *testing.T) { unavailable := 0 - h, s, _ := testHandler(t, func(_ *Dependencies, f *testFakes) { f.metrics.recordUnavailable = func() { unavailable++ } }) + h, s, _ := testHandler(t, func(_ *Dependencies, f *testFakes) { + f.metrics.recordUnavailable = func() { unavailable++ } + if test.name == "unknown saved agent" { + f.agentsReader.getAgentWithModelProvider = func(context.Context, string, string) (agents.Agent, *v1.ModelProviderInput, error) { + return agents.Agent{}, nil, agents.ErrNotFound + } + } + }) r := httptest.NewRequest(http.MethodPost, test.path, strings.NewReader(test.body)) r.Header.Set("Authorization", test.auth) r.Header.Set("OpenAI-Beta", test.beta) diff --git a/services/core/internal/api/resource_creation_test.go b/services/core/internal/api/resource_creation_test.go index 86498b662..c4412a9c2 100644 --- a/services/core/internal/api/resource_creation_test.go +++ b/services/core/internal/api/resource_creation_test.go @@ -6,14 +6,15 @@ import ( "net/http" "testing" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" "github.com/google/uuid" ) type resourceCreationStore struct{} -func (*resourceCreationStore) CreateAgent(_ context.Context, tenant string, input store.CreateAgentInput) (store.SavedAgent, error) { - return store.SavedAgent{ID: uuid.NewString(), TenantID: tenant, Configuration: input.Configuration, Metadata: input.Metadata}, nil +func (*resourceCreationStore) CreateAgent(_ context.Context, command agents.CreateCommand) (agents.Agent, error) { + return agents.Agent{ID: uuid.NewString(), TenantID: command.TenantID, Configuration: command.Configuration, Metadata: command.Metadata}, nil } func (*resourceCreationStore) CreateEnvironmentTemplate(context.Context, string, store.EnvironmentTemplateInput) (store.EnvironmentTemplate, error) { @@ -23,7 +24,7 @@ func (*resourceCreationStore) CreateEnvironmentTemplate(context.Context, string, func TestAgentAndTemplateCreationStatus(t *testing.T) { s := &resourceCreationStore{} h, _, _ := testHandler(t, func(_ *Dependencies, f *testFakes) { - f.agents.createAgent, f.environmentTemplates.createEnvironmentTemplate = s.CreateAgent, s.CreateEnvironmentTemplate + f.agents.create, f.environmentTemplates.createEnvironmentTemplate = s.CreateAgent, s.CreateEnvironmentTemplate }) for _, tc := range []struct{ path, body, object string }{ {"/v1/agents", `{"model":"resource-model"}`, "agent"}, diff --git a/services/core/internal/api/resource_query_test.go b/services/core/internal/api/resource_query_test.go index 09fb5453a..e94672e50 100644 --- a/services/core/internal/api/resource_query_test.go +++ b/services/core/internal/api/resource_query_test.go @@ -13,6 +13,7 @@ import ( "testing" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtimedevice" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" "github.com/google/uuid" @@ -29,16 +30,22 @@ func (s *missingResourceStore) missing(tenant string) error { return store.ErrNotFound } -func (s *missingResourceStore) GetAgent(_ context.Context, tenant, _ string) (store.SavedAgent, error) { - return store.SavedAgent{}, s.missing(tenant) +// missingAgent records the tenant and reports the Agent as missing. +func (s *missingResourceStore) missingAgent(tenant string) error { + s.tenants = append(s.tenants, tenant) + return agents.ErrNotFound } -func (s *missingResourceStore) DeleteAgent(_ context.Context, tenant, _ string) (string, error) { - return "", s.missing(tenant) +func (s *missingResourceStore) GetAgent(_ context.Context, tenant, _ string) (agents.Agent, error) { + return agents.Agent{}, s.missingAgent(tenant) +} + +func (s *missingResourceStore) DeleteAgent(_ context.Context, command agents.DeleteCommand) (string, error) { + return "", s.missingAgent(command.TenantID) } -func (s *missingResourceStore) UpdateAgent(_ context.Context, tenant, _ string, _ store.UpdateAgentInput) (store.SavedAgent, error) { - return store.SavedAgent{}, s.missing(tenant) +func (s *missingResourceStore) UpdateAgent(_ context.Context, command agents.UpdateCommand) (agents.Agent, error) { + return agents.Agent{}, s.missingAgent(command.TenantID) } func (s *missingResourceStore) GetSession(_ context.Context, tenant, _ string) (store.Session, error) { @@ -67,7 +74,7 @@ func (s *missingResourceStore) DeleteEnvironmentTemplate(_ context.Context, tena // wire serves the Agent, Session and Environment template lookups from s. func (s *missingResourceStore) wire(_ *Dependencies, f *testFakes) { - f.agents.getAgent, f.agents.deleteAgent, f.agents.updateAgent = s.GetAgent, s.DeleteAgent, s.UpdateAgent + f.agentsReader.getAgent, f.agents.delete, f.agents.update = s.GetAgent, s.DeleteAgent, s.UpdateAgent f.sessions.getSession, f.sessions.deleteSession, f.sessions.updateSessionMetadata = s.GetSession, s.DeleteSession, s.UpdateSessionMetadata f.environmentTemplates.getEnvironmentTemplate, f.environmentTemplates.updateEnvironmentTemplate, f.environmentTemplates.deleteEnvironmentTemplate = s.GetEnvironmentTemplate, s.UpdateEnvironmentTemplate, s.DeleteEnvironmentTemplate } diff --git a/services/core/internal/api/routing_test.go b/services/core/internal/api/routing_test.go index b97b97a62..d369ec35a 100644 --- a/services/core/internal/api/routing_test.go +++ b/services/core/internal/api/routing_test.go @@ -19,6 +19,7 @@ import ( "time" "github.com/MiniMax-AI/OpenAgentCore/internal/obs/log" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtimedevice" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" "github.com/go-chi/chi/v5" @@ -39,32 +40,32 @@ var requestIDPattern = regexp.MustCompile(`^req_[0-9a-f]{32}$`) // routingStore serves one saved Agent and records Agent lookups. type routingStore struct { tenant string - agent store.SavedAgent + agent agents.Agent lookups, updates []string } -func (s *routingStore) GetAgent(_ context.Context, tenant, id string) (store.SavedAgent, error) { +func (s *routingStore) GetAgent(_ context.Context, tenant, id string) (agents.Agent, error) { s.lookups = append(s.lookups, id) if tenant != s.tenant || id != s.agent.ID { - return store.SavedAgent{}, store.ErrNotFound + return agents.Agent{}, agents.ErrNotFound } return s.agent, nil } -func (s *routingStore) ListAgents(_ context.Context, tenant, _ string, _ int, _ bool) (store.AgentPage, error) { - if tenant != s.tenant { - return store.AgentPage{}, nil +func (s *routingStore) ListAgents(_ context.Context, query agents.ListQuery) (agents.Page, error) { + if query.TenantID != s.tenant { + return agents.Page{}, nil } - return store.AgentPage{Agents: []store.SavedAgent{s.agent}}, nil + return agents.Page{Agents: []agents.Agent{s.agent}}, nil } -func (s *routingStore) UpdateAgent(_ context.Context, tenant, id string, input store.UpdateAgentInput) (store.SavedAgent, error) { - s.updates = append(s.updates, id) - if tenant != s.tenant || id != s.agent.ID { - return store.SavedAgent{}, store.ErrNotFound +func (s *routingStore) Update(_ context.Context, command agents.UpdateCommand) (agents.Agent, error) { + s.updates = append(s.updates, command.AgentID) + if command.TenantID != s.tenant || command.AgentID != s.agent.ID { + return agents.Agent{}, agents.ErrNotFound } - if input.Metadata != nil { - s.agent.Metadata = *input.Metadata + if command.Metadata != nil { + s.agent.Metadata = *command.Metadata } return s.agent, nil } @@ -86,11 +87,11 @@ func routingFixture(t *testing.T) (http.Handler, *chi.Mux, *routingStore) { tenant := uuid.NewString() keys := projectKeys(t, APIKey{OrganizationID: "test-org", ProjectID: "test-project", SubjectKind: "service_account", SubjectID: "test-runner", TokenSHA256: runtimedevice.HashCredential(routingKey), TenantID: tenant}) keys[runtimedevice.HashCredential(routingDerivedKey)] = keys[runtimedevice.HashCredential(routingKey)] - s := &routingStore{tenant: tenant, agent: store.SavedAgent{ID: uuid.NewString(), TenantID: tenant, Metadata: map[string]string{}, + s := &routingStore{tenant: tenant, agent: agents.Agent{ID: uuid.NewString(), TenantID: tenant, Metadata: map[string]string{}, Configuration: json.RawMessage(`{"model":"fixture"}`), CreatedAt: time.Unix(1700000000, 0), UpdatedAt: time.Unix(1700000000, 0)}} deps, fakes := testDependencies(trapTB{t}) fakes.projects.resolveProjectAPIKey = keys.ResolveProjectAPIKey - fakes.agents.getAgent, fakes.agents.listAgents, fakes.agents.updateAgent = s.GetAgent, s.ListAgents, s.UpdateAgent + fakes.agentsReader.getAgent, fakes.agentsReader.listAgents, fakes.agents.update = s.GetAgent, s.ListAgents, s.Update fakes.files.getSourceFile = func(context.Context, string, string) (store.SourceFile, error) { return store.SourceFile{}, store.ErrNotFound } diff --git a/services/core/internal/api/saved_configuration.go b/services/core/internal/api/saved_configuration.go index a9d2ec2f3..a73036000 100644 --- a/services/core/internal/api/saved_configuration.go +++ b/services/core/internal/api/saved_configuration.go @@ -9,33 +9,34 @@ import ( "unicode/utf8" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/metadata" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" ) -func resolveSavedAgent(input v1.CreateAgentRequest) (store.CreateAgentInput, error) { +// resolveSavedAgent returns the command without its tenant. +func resolveSavedAgent(input v1.CreateAgentRequest) (agents.CreateCommand, error) { if input.Model == nil { - return store.CreateAgentInput{}, errors.New("model is required and must be a string.") + return agents.CreateCommand{}, errors.New("model is required and must be a string.") } return resolveSavedFields(input) } // Update requests reuse field validation without requiring an omitted model. -func resolveSavedFields(input v1.CreateAgentRequest) (store.CreateAgentInput, error) { +func resolveSavedFields(input v1.CreateAgentRequest) (agents.CreateCommand, error) { if input.Name != nil { if length := utf8.RuneCountInString(*input.Name); length > 128 { - return store.CreateAgentInput{}, &fieldError{param: "name", message: fmt.Sprintf("Invalid 'name': string too long. Expected a string with maximum length 128, but got a string with length %d instead.", length)} + return agents.CreateCommand{}, &fieldError{param: "name", message: fmt.Sprintf("Invalid 'name': string too long. Expected a string with maximum length 128, but got a string with length %d instead.", length)} } } values, err := stringMetadata(input.Metadata) if err != nil { - return store.CreateAgentInput{}, err + return agents.CreateCommand{}, err } if err := metadataFieldError(metadata.Validate(values)); err != nil { - return store.CreateAgentInput{}, err + return agents.CreateCommand{}, err } if err := input.XAgentsCore.Validate(); err != nil { - return store.CreateAgentInput{}, err + return agents.CreateCommand{}, err } cfg := v1.SavedAgentConfiguration{XAgentsCore: input.XAgentsCore.SafeView(), Name: input.Name, Instructions: input.Instructions, ServiceTier: "auto"} if input.Model != nil { @@ -43,35 +44,35 @@ func resolveSavedFields(input v1.CreateAgentRequest) (store.CreateAgentInput, er } cfg.MultiAgent, err = resolveSavedMultiAgent(input.MultiAgent) if err != nil { - return store.CreateAgentInput{}, err + return agents.CreateCommand{}, err } if input.ServiceTier != nil { if !slices.Contains([]string{"auto", "default", "flex", "priority", "fast"}, *input.ServiceTier) { - return store.CreateAgentInput{}, errors.New("service_tier must be auto, default, flex, priority or fast.") + return agents.CreateCommand{}, errors.New("service_tier must be auto, default, flex, priority or fast.") } cfg.ServiceTier = *input.ServiceTier } if input.Reasoning != nil { cfg.Reasoning = *input.Reasoning if cfg.Reasoning.Effort != nil && !slices.Contains([]string{"none", "minimal", "low", "medium", "high", "xhigh", "max"}, *cfg.Reasoning.Effort) { - return store.CreateAgentInput{}, errors.New("reasoning.effort is not a supported protocol value.") + return agents.CreateCommand{}, errors.New("reasoning.effort is not a supported protocol value.") } if cfg.Reasoning.Summary != nil && !slices.Contains([]string{"concise", "detailed", "auto"}, *cfg.Reasoning.Summary) { - return store.CreateAgentInput{}, errors.New("reasoning.summary must be concise, detailed or auto.") + return agents.CreateCommand{}, errors.New("reasoning.summary must be concise, detailed or auto.") } } // Model-derived effort resolution is a recorded gap. Do not manufacture a // default from the operator's execution engine or another model's catalog. cfg.Text, err = resolveSavedText(input.Text) if err != nil { - return store.CreateAgentInput{}, err + return agents.CreateCommand{}, err } cfg.Tools, err = resolveSavedTools(input.Tools) if err != nil { - return store.CreateAgentInput{}, err + return agents.CreateCommand{}, err } configuration, err := json.Marshal(cfg) - result := store.CreateAgentInput{Metadata: values, Configuration: configuration} + result := agents.CreateCommand{Metadata: values, Configuration: configuration} if input.XAgentsCore != nil { result.ModelProvider = input.XAgentsCore.ModelProvider } diff --git a/services/core/internal/api/saved_provider_test.go b/services/core/internal/api/saved_provider_test.go index 69be54e76..5624721c0 100644 --- a/services/core/internal/api/saved_provider_test.go +++ b/services/core/internal/api/saved_provider_test.go @@ -8,40 +8,43 @@ import ( "testing" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/google/uuid" ) const savedProviderFixture = `{"protocol":"responses","base_url":"https://example.test/v1","api_key":"saved-provider-secret","context_window":100000,"max_output_tokens":8000}` type savedProviderStore struct { - saved store.SavedAgent + saved agents.Agent provider *v1.ModelProviderInput } -func (s *savedProviderStore) CreateAgent(_ context.Context, tenant string, input store.CreateAgentInput) (store.SavedAgent, error) { - s.provider = input.ModelProvider - s.saved = store.SavedAgent{ID: uuid.NewString(), TenantID: tenant, Configuration: input.Configuration, Metadata: input.Metadata} +func (s *savedProviderStore) CreateAgent(_ context.Context, command agents.CreateCommand) (agents.Agent, error) { + s.provider = command.ModelProvider + s.saved = agents.Agent{ID: uuid.NewString(), TenantID: command.TenantID, Configuration: command.Configuration, Metadata: command.Metadata} return s.saved, nil } -func (s *savedProviderStore) UpdateAgent(_ context.Context, _, _ string, input store.UpdateAgentInput) (store.SavedAgent, error) { - s.provider = input.ModelProvider - s.saved.Configuration = input.Configuration +func (s *savedProviderStore) UpdateAgent(_ context.Context, command agents.UpdateCommand) (agents.Agent, error) { + s.provider = nil + if command.ModelProvider != nil { + s.provider = command.ModelProvider.Provider + } + s.saved.Configuration = command.Configuration return s.saved, nil } -func (s *savedProviderStore) GetAgent(context.Context, string, string) (store.SavedAgent, error) { +func (s *savedProviderStore) GetAgent(context.Context, string, string) (agents.Agent, error) { return s.saved, nil } -func (s *savedProviderStore) ListAgents(context.Context, string, string, int, bool) (store.AgentPage, error) { - return store.AgentPage{Agents: []store.SavedAgent{s.saved}}, nil +func (s *savedProviderStore) ListAgents(context.Context, agents.ListQuery) (agents.Page, error) { + return agents.Page{Agents: []agents.Agent{s.saved}}, nil } // serve answers the saved Agent operations from s. func (s *savedProviderStore) serve(_ *Dependencies, f *testFakes) { - f.agents.createAgent, f.agents.updateAgent, f.agents.getAgent, f.agents.listAgents = s.CreateAgent, s.UpdateAgent, s.GetAgent, s.ListAgents + f.agents.create, f.agents.update, f.agentsReader.getAgent, f.agentsReader.listAgents = s.CreateAgent, s.UpdateAgent, s.GetAgent, s.ListAgents } func TestSavedProviderReadRedaction(t *testing.T) { @@ -104,8 +107,10 @@ func TestSavedProviderUpdatePresence(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { input, err := resolveAgentUpdate([]byte(tc.body)) - if err != nil || input.ModelProviderSet != tc.set || (input.ModelProvider != nil) != tc.provider { - t.Fatalf("set=%v provider=%v err=%v", input.ModelProviderSet, input.ModelProvider != nil, err) + set := input.ModelProvider != nil + provider := set && input.ModelProvider.Provider != nil + if err != nil || set != tc.set || provider != tc.provider { + t.Fatalf("set=%v provider=%v err=%v", set, provider, err) } if tc.patch != "" && string(input.Configuration) != tc.patch { t.Fatalf("patch=%s", input.Configuration) diff --git a/services/core/internal/api/session_model_defaults.go b/services/core/internal/api/session_model_defaults.go index 2a37bdf46..c897bd7c7 100644 --- a/services/core/internal/api/session_model_defaults.go +++ b/services/core/internal/api/session_model_defaults.go @@ -6,20 +6,26 @@ import ( "errors" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/credentialcrypto" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" "github.com/google/uuid" ) +// sessionAgentDefaults reads the Session's saved Agent. A Session that +// inherits the Agent's model provider reads the opened bundle with it. func (h *Handler) sessionAgentDefaults(ctx context.Context, tenant string, input sessionRequest) (*v1.SavedAgent, *v1.ModelProviderInput, error) { if input.AgentID == nil { return nil, nil, nil } - if !validAgentID(*input.AgentID) { - return nil, nil, store.ErrNotFound - } inherit := input.XAgentsCore == nil || input.XAgentsCore.ModelProvider == nil - resource, provider, err := h.Agents.GetAgentForSession(ctx, tenant, *input.AgentID, inherit) + var resource agents.Agent + var provider *v1.ModelProviderInput + var err error + if inherit { + resource, provider, err = h.AgentsReader.GetAgentWithModelProvider(ctx, tenant, *input.AgentID) + } else { + resource, err = h.AgentsReader.GetAgent(ctx, tenant, *input.AgentID) + } if err != nil { return nil, nil, err } diff --git a/services/core/internal/api/validation_errors_test.go b/services/core/internal/api/validation_errors_test.go index b67552671..8c0410ba4 100644 --- a/services/core/internal/api/validation_errors_test.go +++ b/services/core/internal/api/validation_errors_test.go @@ -11,6 +11,7 @@ import ( "time" v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" "github.com/google/uuid" "github.com/jackc/pgx/v5/pgconn" @@ -22,14 +23,14 @@ type validationStore struct { writes int } -func (s *validationStore) CreateAgent(_ context.Context, tenant string, input store.CreateAgentInput) (store.SavedAgent, error) { +func (s *validationStore) CreateAgent(_ context.Context, command agents.CreateCommand) (agents.Agent, error) { s.writes++ - return store.SavedAgent{ID: uuid.NewString(), TenantID: tenant, Configuration: input.Configuration, Metadata: input.Metadata, CreatedAt: time.Unix(1700000000, 0), UpdatedAt: time.Unix(1700000000, 0)}, nil + return agents.Agent{ID: uuid.NewString(), TenantID: command.TenantID, Configuration: command.Configuration, Metadata: command.Metadata, CreatedAt: time.Unix(1700000000, 0), UpdatedAt: time.Unix(1700000000, 0)}, nil } -func (s *validationStore) UpdateAgent(_ context.Context, tenant, id string, input store.UpdateAgentInput) (store.SavedAgent, error) { +func (s *validationStore) UpdateAgent(_ context.Context, command agents.UpdateCommand) (agents.Agent, error) { s.writes++ - return store.SavedAgent{ID: id, TenantID: tenant, Configuration: json.RawMessage(`{"model":"validation-model"}`), Metadata: map[string]string{}}, nil + return agents.Agent{ID: command.AgentID, TenantID: command.TenantID, Configuration: json.RawMessage(`{"model":"validation-model"}`), Metadata: map[string]string{}}, nil } func (s *validationStore) CreateVault(_ context.Context, tenant string, input store.CreateVaultInput) (store.Vault, error) { @@ -62,7 +63,7 @@ func (s *validationStore) UpdateEnvironmentTemplate(_ context.Context, _, id str func (s *validationStore) serve(d *Dependencies, f *testFakes) { d.Execution = f.execution() f.admission.createSession = s.CreateSession - f.agents.createAgent, f.agents.updateAgent = s.CreateAgent, s.UpdateAgent + f.agents.create, f.agents.update = s.CreateAgent, s.UpdateAgent f.vaults.createVault = s.CreateVault f.sessions.getSession, f.sessions.updateSessionMetadata = nil, s.UpdateSessionMetadata f.environmentTemplates.createEnvironmentTemplate, f.environmentTemplates.updateEnvironmentTemplate = s.CreateEnvironmentTemplate, s.UpdateEnvironmentTemplate diff --git a/services/core/internal/api/vaults_test.go b/services/core/internal/api/vaults_test.go index a28c1552c..3b84c48d1 100644 --- a/services/core/internal/api/vaults_test.go +++ b/services/core/internal/api/vaults_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtimedevice" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" "github.com/google/uuid" @@ -39,8 +40,8 @@ func (f *vaultResourceFixture) GetVault(_ context.Context, tenant, id string) (s // Vaults are not Agents: /v1/agents/vaults updates an unknown Agent ID, which // resolves as a missing Agent after body validation. -func (f *vaultResourceFixture) UpdateAgent(context.Context, string, string, store.UpdateAgentInput) (store.SavedAgent, error) { - return store.SavedAgent{}, store.ErrNotFound +func (f *vaultResourceFixture) UpdateAgent(context.Context, agents.UpdateCommand) (agents.Agent, error) { + return agents.Agent{}, agents.ErrNotFound } func vaultResourceHandler(t *testing.T) (http.Handler, *vaultResourceFixture) { @@ -50,7 +51,7 @@ func vaultResourceHandler(t *testing.T) (http.Handler, *vaultResourceFixture) { deps.Engine = "fake_alpha" fakes.projects.resolveProjectAPIKey = projectKeys(t, APIKey{OrganizationID: "vault-org", ProjectID: "vault-project", SubjectKind: "user", SubjectID: "vault-owner", TokenSHA256: runtimedevice.HashCredential("vault-key"), TenantID: f.vault.TenantID}).ResolveProjectAPIKey fakes.vaults.createVault, fakes.vaults.getVault, fakes.vaults.listVaults, fakes.vaults.deleteVault = f.CreateVault, f.GetVault, f.ListVaults, f.DeleteVault - fakes.agents.updateAgent = f.UpdateAgent + fakes.agents.update = f.UpdateAgent return newTestHandler(t, deps), f } diff --git a/services/core/internal/db/queries/agent_model_execution.sql b/services/core/internal/db/queries/agent_model_execution.sql index e90e0f1b9..42710c6e1 100644 --- a/services/core/internal/db/queries/agent_model_execution.sql +++ b/services/core/internal/db/queries/agent_model_execution.sql @@ -5,7 +5,7 @@ ON CONFLICT (agent_id) DO UPDATE SET encrypted_config = EXCLUDED.encrypted_confi -- name: DeleteAgentModelExecution :exec DELETE FROM agent_model_execution WHERE agent_id = @agent_id; --- name: GetAgentForSession :one +-- name: GetAgentWithModelExecution :one SELECT a.*, e.encrypted_config FROM agents a LEFT JOIN agent_model_execution e ON e.agent_id = a.id WHERE a.tenant_id = @tenant_id AND a.id = @agent_id; diff --git a/services/core/internal/db/sqlc/agent_model_execution.sql.go b/services/core/internal/db/sqlc/agent_model_execution.sql.go index 0051b6768..7fd6bcac0 100644 --- a/services/core/internal/db/sqlc/agent_model_execution.sql.go +++ b/services/core/internal/db/sqlc/agent_model_execution.sql.go @@ -20,18 +20,18 @@ func (q *Queries) DeleteAgentModelExecution(ctx context.Context, agentID pgtype. return err } -const getAgentForSession = `-- name: GetAgentForSession :one +const getAgentWithModelExecution = `-- name: GetAgentWithModelExecution :one SELECT a.id, a.tenant_id, a.metadata, a.configuration, a.created_at, a.updated_at, e.encrypted_config FROM agents a LEFT JOIN agent_model_execution e ON e.agent_id = a.id WHERE a.tenant_id = $1 AND a.id = $2 ` -type GetAgentForSessionParams struct { +type GetAgentWithModelExecutionParams struct { TenantID pgtype.UUID `json:"tenant_id"` AgentID pgtype.UUID `json:"agent_id"` } -type GetAgentForSessionRow struct { +type GetAgentWithModelExecutionRow struct { ID pgtype.UUID `json:"id"` TenantID pgtype.UUID `json:"tenant_id"` Metadata []byte `json:"metadata"` @@ -41,9 +41,9 @@ type GetAgentForSessionRow struct { EncryptedConfig []byte `json:"encrypted_config"` } -func (q *Queries) GetAgentForSession(ctx context.Context, arg GetAgentForSessionParams) (GetAgentForSessionRow, error) { - row := q.db.QueryRow(ctx, getAgentForSession, arg.TenantID, arg.AgentID) - var i GetAgentForSessionRow +func (q *Queries) GetAgentWithModelExecution(ctx context.Context, arg GetAgentWithModelExecutionParams) (GetAgentWithModelExecutionRow, error) { + row := q.db.QueryRow(ctx, getAgentWithModelExecution, arg.TenantID, arg.AgentID) + var i GetAgentWithModelExecutionRow err := row.Scan( &i.ID, &i.TenantID, diff --git a/services/core/internal/persistence/postgres/agentpg/store.go b/services/core/internal/persistence/postgres/agentpg/store.go new file mode 100644 index 000000000..2978328a0 --- /dev/null +++ b/services/core/internal/persistence/postgres/agentpg/store.go @@ -0,0 +1,298 @@ +// Package agentpg stores saved Agents in PostgreSQL. An Agent's model provider +// bundle is sealed with the Core credential key, bound to its tenant and +// Agent, in the Agent's transaction. +package agentpg + +import ( + "context" + "encoding/json" + "errors" + "fmt" + + v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/credentialcrypto" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/db/sqlc" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/auditpg" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/textvalue" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/writeaudit" + "github.com/google/uuid" + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" +) + +type Store struct { + pool *pgunit.Pool + cipher *credentialcrypto.Cipher +} + +// New returns the Agent store. cipher is nil when Core has no credential key; +// saving or opening a model provider bundle then fails with +// credentialcrypto.ErrUnavailable, as does opening a bundle the key cannot open. +func New(pool *pgunit.Pool, cipher *credentialcrypto.Cipher) *Store { + return &Store{pool: pool, cipher: cipher} +} + +var ( + _ agents.Storage = (*Store)(nil) + _ agents.Reader = (*Store)(nil) +) + +func (s *Store) CreateAgent(ctx context.Context, input agents.NewAgent) (agents.Agent, error) { + tenant, err := parseTenant(input.TenantID) + if err != nil { + return agents.Agent{}, err + } + var created agents.Agent + err = s.pool.Transaction(ctx, func(ctx context.Context, tx pgx.Tx) error { + q := sqlc.New(tx) + row, err := q.CreateAgent(ctx, sqlc.CreateAgentParams{ + ID: pgtype.UUID{Bytes: uuid.New(), Valid: true}, TenantID: tenant, + Metadata: input.Metadata, Configuration: input.Configuration, + }) + if err != nil { + return err + } + if input.ModelProvider != nil { + if err := s.saveModelProvider(ctx, q, row.TenantID, row.ID, input.ModelProvider); err != nil { + return err + } + } + if created, err = agentFromRow(row); err != nil { + return err + } + return auditpg.RecordWriteAudit(ctx, q, input.TenantID, "create", "agent", created.ID, "", writeaudit.Resource{Type: "agent", ID: created.ID}) + }) + if err != nil { + return agents.Agent{}, translate(err) + } + return created, nil +} + +// WithAgentUpdate treats an agentID that cannot name an Agent as a missing one. +func (s *Store) WithAgentUpdate(ctx context.Context, tenantID, agentID string, update func(agents.UpdateTx) error) error { + tenant, err := parseTenant(tenantID) + if err != nil { + return err + } + return s.pool.Transaction(ctx, func(ctx context.Context, tx pgx.Tx) error { + return update(&updateTx{ctx: ctx, q: sqlc.New(tx), store: s, tenantID: tenantID, tenant: tenant, id: pgunit.PathID(agentID)}) + }) +} + +// updateTx runs on the transaction's context, which bounds every statement. +type updateTx struct { + ctx context.Context + q *sqlc.Queries + store *Store + tenantID string + tenant pgtype.UUID + id pgtype.UUID +} + +func (t *updateTx) LoadAgent() (agents.Agent, error) { + row, err := t.q.LockAgent(t.ctx, sqlc.LockAgentParams{TenantID: t.tenant, ID: t.id}) + if errors.Is(err, pgx.ErrNoRows) { + return agents.Agent{}, agents.ErrNotFound + } + if err != nil { + return agents.Agent{}, err + } + return agentFromRow(row) +} + +func (t *updateTx) ApplyRevision(revision agents.Revision) (agents.Agent, error) { + updated, err := t.apply(revision) + if err != nil { + return agents.Agent{}, translate(err) + } + return updated, nil +} + +func (t *updateTx) apply(revision agents.Revision) (agents.Agent, error) { + if change := revision.ModelProvider; change != nil { + if change.Provider == nil { + if err := t.q.DeleteAgentModelExecution(t.ctx, t.id); err != nil { + return agents.Agent{}, err + } + } else if err := t.store.saveModelProvider(t.ctx, t.q, t.tenant, t.id, change.Provider); err != nil { + return agents.Agent{}, err + } + } + row, err := t.q.UpdateAgent(t.ctx, sqlc.UpdateAgentParams{TenantID: t.tenant, ID: t.id, Configuration: revision.Configuration, Metadata: revision.Metadata}) + if errors.Is(err, pgx.ErrNoRows) { + return agents.Agent{}, agents.ErrNotFound + } + if err != nil { + return agents.Agent{}, err + } + updated, err := agentFromRow(row) + if err != nil { + return agents.Agent{}, err + } + return updated, auditpg.RecordWriteAudit(t.ctx, t.q, t.tenantID, "update", "agent", updated.ID, "") +} + +// DeleteAgent treats an agentID that cannot name an Agent as a missing one. +// The model provider bundle goes with the Agent by foreign-key cascade. +func (s *Store) DeleteAgent(ctx context.Context, tenantID, agentID string) (string, error) { + tenant, err := parseTenant(tenantID) + if err != nil { + return "", err + } + var deleted string + err = s.pool.Transaction(ctx, func(ctx context.Context, tx pgx.Tx) error { + q := sqlc.New(tx) + id, err := q.DeleteAgent(ctx, sqlc.DeleteAgentParams{TenantID: tenant, ID: pgunit.PathID(agentID)}) + if errors.Is(err, pgx.ErrNoRows) { + return agents.ErrNotFound + } + if err != nil { + return err + } + deleted = uuid.UUID(id.Bytes).String() + return auditpg.RecordWriteAudit(ctx, q, tenantID, "delete", "agent", deleted, "") + }) + if err != nil { + return "", err + } + return deleted, nil +} + +// GetAgent treats an agentID that cannot name an Agent as a missing one. +func (s *Store) GetAgent(ctx context.Context, tenantID, agentID string) (agents.Agent, error) { + tenant, err := parseTenant(tenantID) + if err != nil { + return agents.Agent{}, err + } + row, err := s.pool.Queries().GetAgent(ctx, sqlc.GetAgentParams{TenantID: tenant, ID: pgunit.PathID(agentID)}) + if errors.Is(err, pgx.ErrNoRows) { + return agents.Agent{}, agents.ErrNotFound + } + if err != nil { + return agents.Agent{}, err + } + return agentFromRow(row) +} + +// ListAgents reads the cursor Agent and the page from one snapshot. A cursor +// that cannot name an Agent is a missing one. +func (s *Store) ListAgents(ctx context.Context, query agents.ListQuery) (agents.Page, error) { + if err := query.Validate(); err != nil { + return agents.Page{}, err + } + tenant, err := parseTenant(query.TenantID) + if err != nil { + return agents.Page{}, err + } + var page agents.Page + err = s.pool.Snapshot(ctx, func(ctx context.Context, tx pgx.Tx) error { + q := sqlc.New(tx) + params := sqlc.ListAgentsParams{TenantID: tenant, PageLimit: int32(query.Limit + 1), AfterID: pgtype.UUID{Valid: true}, Ascending: query.Ascending} + if query.After != "" { + after, err := q.GetAgent(ctx, sqlc.GetAgentParams{TenantID: tenant, ID: pgunit.PathID(query.After)}) + if errors.Is(err, pgx.ErrNoRows) { + return agents.ErrNotFound + } + if err != nil { + return err + } + params.AfterCreated, params.AfterID = after.CreatedAt, after.ID + } + rows, err := q.ListAgents(ctx, params) + if err != nil { + return err + } + page = agents.Page{Agents: make([]agents.Agent, 0, min(query.Limit, len(rows)))} + if len(rows) > query.Limit { + page.NextCursor = uuid.UUID(rows[query.Limit-1].ID.Bytes).String() + rows = rows[:query.Limit] + } + for _, row := range rows { + agent, err := agentFromRow(row) + if err != nil { + return err + } + page.Agents = append(page.Agents, agent) + } + return nil + }) + if err != nil { + return agents.Page{}, err + } + return page, nil +} + +// GetAgentWithModelProvider reads the Agent and its sealed bundle with one +// statement and opens the bundle. An agentID that cannot name an Agent is a +// missing one. +func (s *Store) GetAgentWithModelProvider(ctx context.Context, tenantID, agentID string) (agents.Agent, *v1.ModelProviderInput, error) { + tenant, err := parseTenant(tenantID) + if err != nil { + return agents.Agent{}, nil, err + } + row, err := s.pool.Queries().GetAgentWithModelExecution(ctx, sqlc.GetAgentWithModelExecutionParams{TenantID: tenant, AgentID: pgunit.PathID(agentID)}) + if errors.Is(err, pgx.ErrNoRows) { + return agents.Agent{}, nil, agents.ErrNotFound + } + if err != nil { + return agents.Agent{}, nil, err + } + agent, err := agentFromRow(sqlc.Agent{ID: row.ID, TenantID: row.TenantID, Metadata: row.Metadata, Configuration: row.Configuration, CreatedAt: row.CreatedAt, UpdatedAt: row.UpdatedAt}) + if err != nil { + return agents.Agent{}, nil, err + } + if row.EncryptedConfig == nil { + return agent, nil, nil + } + raw, err := s.cipher.OpenAgentModelExecution(row.EncryptedConfig, agent.TenantID, agent.ID) + if err != nil { + return agents.Agent{}, nil, credentialcrypto.ErrUnavailable + } + var provider v1.ModelProviderInput + if json.Unmarshal(raw, &provider) != nil || provider.Validate() != nil { + return agents.Agent{}, nil, credentialcrypto.ErrUnavailable + } + return agent, &provider, nil +} + +func (s *Store) saveModelProvider(ctx context.Context, q *sqlc.Queries, tenant, agent pgtype.UUID, provider *v1.ModelProviderInput) error { + raw, err := json.Marshal(provider) + if err != nil { + return err + } + sealed, err := s.cipher.SealAgentModelExecution(raw, uuid.UUID(tenant.Bytes).String(), uuid.UUID(agent.Bytes).String()) + if err != nil { + return credentialcrypto.ErrUnavailable + } + return q.SaveAgentModelExecution(ctx, sqlc.SaveAgentModelExecutionParams{AgentID: agent, EncryptedConfig: sealed}) +} + +func parseTenant(value string) (pgtype.UUID, error) { + tenant, err := pgunit.ParseID(value) + if err != nil { + return pgtype.UUID{}, fmt.Errorf("%w: tenant ID", agents.ErrInvalidInput) + } + return tenant, nil +} + +func agentFromRow(row sqlc.Agent) (agents.Agent, error) { + agent := agents.Agent{ + ID: uuid.UUID(row.ID.Bytes).String(), TenantID: uuid.UUID(row.TenantID.Bytes).String(), + Configuration: row.Configuration, CreatedAt: row.CreatedAt.Time, UpdatedAt: row.UpdatedAt.Time, + } + if err := json.Unmarshal(row.Metadata, &agent.Metadata); err != nil { + return agents.Agent{}, fmt.Errorf("decode agent metadata: %w", err) + } + return agent, nil +} + +// translate reports text PostgreSQL cannot store as textvalue.ErrUnstorable +// and returns any other error as it is. +func translate(err error) error { + if pgunit.IsUnstorableText(err) { + return textvalue.ErrUnstorable + } + return err +} diff --git a/services/core/internal/persistence/postgres/agentpg/store_test.go b/services/core/internal/persistence/postgres/agentpg/store_test.go new file mode 100644 index 000000000..7e8ad43b4 --- /dev/null +++ b/services/core/internal/persistence/postgres/agentpg/store_test.go @@ -0,0 +1,669 @@ +package agentpg_test + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "fmt" + "reflect" + "slices" + "strings" + "sync" + "testing" + "time" + + v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/adminaudit" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/credentialcrypto" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/agentpg" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgtest" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/textvalue" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/writeaudit" + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgxpool" +) + +// open returns the adapter and the Agent service over pool. +func open(t *testing.T, pool *pgxpool.Pool, cipher *credentialcrypto.Cipher) (*agentpg.Store, *agents.Service) { + t.Helper() + store := agentpg.New(pgunit.NewPool(pool), cipher) + service, err := agents.NewService(store) + if err != nil { + t.Fatal(err) + } + return store, service +} + +func testCipher(t *testing.T, seed byte) *credentialcrypto.Cipher { + t.Helper() + cipher, err := credentialcrypto.New(bytes.Repeat([]byte{seed}, 32)) + if err != nil { + t.Fatal(err) + } + return cipher +} + +func providerFixture(sequence int) *v1.ModelProviderInput { + return &v1.ModelProviderInput{Protocol: "responses", BaseURL: fmt.Sprintf("https://provider-%d.example/v1", sequence), APIKey: fmt.Sprintf("private-agent-canary-%d", sequence)} +} + +func providerConfiguration(t *testing.T, provider *v1.ModelProviderInput, harness string) json.RawMessage { + t.Helper() + raw, err := json.Marshal(map[string]any{"model": "actual-model", "x_agents_core": v1.SavedAgentCore{Harness: harness, ModelProvider: provider.SafeView()}}) + if err != nil { + t.Fatal(err) + } + return raw +} + +func count(t *testing.T, pool *pgxpool.Pool, query string, args ...any) int { + t.Helper() + var n int + if err := pool.QueryRow(t.Context(), query, args...).Scan(&n); err != nil { + t.Fatal(err) + } + return n +} + +func TestAgentsPersistIndependentlyAndStayTenantScoped(t *testing.T) { + pool := pgtest.Open(t) + store, service := open(t, pool, nil) + ctx := t.Context() + tenantA, tenantB := uuid.NewString(), uuid.NewString() + // Configuration is preserved without applying one harness's capabilities. + command := agents.CreateCommand{ + TenantID: tenantA, Metadata: map[string]string{"purpose": "保存 configuration"}, + Configuration: []byte(`{"model":" caller-model ","name":null,"instructions":" keep whitespace ","multi_agent":{"enabled":true,"max_concurrent_subagents":6},"tools":[{"type":"function","name":"lookup","description":"","defer_loading":true,"parameters":{"type":"object","properties":{"number":{"const":9007199254740993}}}}]}`), + } + before := time.Now().Add(-time.Second) + first, err := service.Create(ctx, command) + if err != nil { + t.Fatal(err) + } + var want, got any + if json.Unmarshal(command.Configuration, &want) != nil || json.Unmarshal(first.Configuration, &got) != nil || !reflect.DeepEqual(got, want) || + !bytes.Contains(first.Configuration, []byte("9007199254740993")) { + t.Fatalf("configuration changed: %s", first.Configuration) + } + if first.TenantID != tenantA || !reflect.DeepEqual(first.Metadata, command.Metadata) || + first.CreatedAt.Before(before) || first.CreatedAt.After(time.Now().Add(time.Second)) || !first.CreatedAt.Equal(first.UpdatedAt) { + t.Fatalf("unexpected saved agent: %+v", first) + } + // Identical configurations are distinct resources; storage invents no public + // create-idempotency contract or shared identity across callers. + for _, tenant := range []string{tenantA, tenantB} { + command.TenantID = tenant + other, err := service.Create(ctx, command) + if err != nil || other.ID == first.ID || other.TenantID != tenant { + t.Fatalf("distinct create: %+v, %v", other, err) + } + } + for _, lookup := range []struct{ tenant, id string }{{tenantB, first.ID}, {tenantA, uuid.NewString()}, {tenantA, "not-an-id"}, {tenantA, uuid.Nil.String()}} { + if _, err := store.GetAgent(ctx, lookup.tenant, lookup.id); !errors.Is(err, agents.ErrNotFound) { + t.Fatalf("unowned/absent agent read: %v", err) + } + } + reopened, _ := open(t, pgtest.Open(t), nil) + pool.Close() + durable, err := reopened.GetAgent(ctx, tenantA, first.ID) + if err != nil || !reflect.DeepEqual(durable, first) { + t.Fatalf("durable read: %+v, %v; want %+v", durable, err, first) + } +} + +func TestAgentsRejectInvalidTenantsAndEmptyMetadataIsAMap(t *testing.T) { + pool := pgtest.Open(t) + store, service := open(t, pool, nil) + ctx := t.Context() + tenant := uuid.NewString() + valid := agents.CreateCommand{Configuration: []byte(`{"model":"x"}`)} + for _, invalid := range []string{"", "not-a-uuid", uuid.Nil.String()} { + valid.TenantID = invalid + if _, err := service.Create(ctx, valid); !errors.Is(err, agents.ErrInvalidInput) { + t.Fatalf("invalid tenant accepted: %v", err) + } + if _, err := store.GetAgent(ctx, invalid, uuid.NewString()); !errors.Is(err, agents.ErrInvalidInput) { + t.Fatalf("invalid read tenant accepted: %v", err) + } + if _, err := store.ListAgents(ctx, agents.ListQuery{TenantID: invalid, Limit: 1}); !errors.Is(err, agents.ErrInvalidInput) { + t.Fatalf("invalid list tenant accepted: %v", err) + } + } + if n := count(t, pool, "SELECT count(*) FROM agents WHERE tenant_id = $1", tenant); n != 0 { + t.Fatalf("rejected input wrote %d rows", n) + } + valid.TenantID = tenant + empty, err := service.Create(ctx, valid) + if err != nil || empty.Metadata == nil || len(empty.Metadata) != 0 { + t.Fatalf("empty metadata: %+v, %v", empty, err) + } +} + +func TestAgentListPaginationIsolationAndReconnect(t *testing.T) { + pool := pgtest.Open(t) + store, service := open(t, pool, nil) + ctx := t.Context() + tenant, other := uuid.NewString(), uuid.NewString() + empty, err := store.ListAgents(ctx, agents.ListQuery{TenantID: tenant, Limit: 2}) + if err != nil || empty.Agents == nil || len(empty.Agents) != 0 || empty.NextCursor != "" { + t.Fatalf("empty page: %+v, %v", empty, err) + } + command := agents.CreateCommand{TenantID: tenant, Configuration: []byte(`{"model":"unchanged","tools":[{"parameters":{"const":9007199254740993}}]}`), Metadata: map[string]string{"scope": "same-tenant"}} + ids := []string{} + for range 5 { + agent, err := service.Create(ctx, command) + if err != nil { + t.Fatal(err) + } + ids = append(ids, agent.ID) + } + command.TenantID = other + foreign, err := service.Create(ctx, command) + if err != nil { + t.Fatal(err) + } + stamp := time.Unix(1700000000, 0).UTC() + if _, err := pool.Exec(ctx, "UPDATE agents SET created_at=$1, updated_at=$1 WHERE tenant_id=$2", stamp, tenant); err != nil { + t.Fatal(err) + } + slices.Sort(ids) + read := func(s *agentpg.Store, ascending bool) []string { + t.Helper() + var actual []string + cursor := "" + for { + page, err := s.ListAgents(ctx, agents.ListQuery{TenantID: tenant, After: cursor, Limit: 2, Ascending: ascending}) + if err != nil { + t.Fatal(err) + } + if len(page.Agents) == 0 || len(page.Agents) > 2 { + t.Fatalf("bad page: %+v", page) + } + for _, agent := range page.Agents { + original, err := s.GetAgent(ctx, tenant, agent.ID) + if err != nil || !reflect.DeepEqual(agent, original) || agent.TenantID != tenant { + t.Fatalf("resource changed: %+v, %v", agent, err) + } + actual = append(actual, agent.ID) + } + if page.NextCursor == "" { + break + } + if page.NextCursor != page.Agents[len(page.Agents)-1].ID || len(actual) > len(ids) { + t.Fatal("invalid/repeating continuation") + } + cursor = page.NextCursor + } + return actual + } + if got := read(store, true); !slices.Equal(got, ids) { + t.Fatalf("ascending equal timestamps: %v", got) + } + reverse := slices.Clone(ids) + slices.Reverse(reverse) + if got := read(store, false); !slices.Equal(got, reverse) { + t.Fatalf("descending equal timestamps: %v", got) + } + // A malformed cursor follows the missing-cursor path (ERR-01). + for _, after := range []string{foreign.ID, uuid.NewString(), "not-an-id"} { + if _, err := store.ListAgents(ctx, agents.ListQuery{TenantID: tenant, After: after, Limit: 2, Ascending: true}); !errors.Is(err, agents.ErrNotFound) { + t.Fatalf("unowned/unknown/malformed cursor accepted: %v", err) + } + } + tail, err := store.ListAgents(ctx, agents.ListQuery{TenantID: tenant, After: ids[len(ids)-1], Limit: 2, Ascending: true}) + if err != nil || tail.Agents == nil || len(tail.Agents) != 0 || tail.NextCursor != "" { + t.Fatalf("end page: %+v, %v", tail, err) + } + foreignPage, err := store.ListAgents(ctx, agents.ListQuery{TenantID: other, Limit: 100}) + if err != nil || len(foreignPage.Agents) != 1 || foreignPage.Agents[0].ID != foreign.ID { + t.Fatalf("tenant isolation: %+v, %v", foreignPage, err) + } + restored, _ := open(t, pgtest.Open(t), nil) + pool.Close() + if got := read(restored, true); !slices.Equal(got, ids) { + t.Fatalf("pagination changed after reconnect: %v", got) + } +} + +func TestAgentUpdateRollbackAndCompleteSizeBound(t *testing.T) { + pool := pgtest.Open(t) + store, service := open(t, pool, nil) + ctx := t.Context() + tenant := uuid.NewString() + configuration, err := json.Marshal(map[string]any{"model": "original", "instructions": strings.Repeat("x", 400*1024), "number": json.Number("9007199254740993")}) + if err != nil { + t.Fatal(err) + } + original, err := service.Create(ctx, agents.CreateCommand{TenantID: tenant, Configuration: configuration, Metadata: map[string]string{"keep": "original"}}) + if err != nil { + t.Fatal(err) + } + metadata := map[string]string{"replace": "not-committed"} + oversized, err := json.Marshal(map[string]string{"name": strings.Repeat("y", 150*1024)}) + if err != nil { + t.Fatal(err) + } + // The merged configuration exceeds the bound only after the locked read. + if _, err := service.Update(ctx, agents.UpdateCommand{TenantID: tenant, AgentID: original.ID, Configuration: oversized, Metadata: &metadata}); !errors.Is(err, agents.ErrInvalidInput) { + t.Fatalf("oversized merged update: %v", err) + } + if unchanged, err := store.GetAgent(ctx, tenant, original.ID); err != nil || !reflect.DeepEqual(unchanged, original) { + t.Fatalf("partial failed update: %v", err) + } + for _, id := range []string{original.ID, "not-an-id"} { + _, err = service.Update(ctx, agents.UpdateCommand{TenantID: uuid.NewString(), AgentID: id, Configuration: []byte(`{"model":"foreign"}`), Metadata: &metadata}) + if !errors.Is(err, agents.ErrNotFound) { + t.Fatalf("foreign or malformed update: %v", err) + } + } + // A failure releases the lock; a later valid patch preserves unrelated large values and numbers. + updated, err := service.Update(ctx, agents.UpdateCommand{TenantID: tenant, AgentID: original.ID, Configuration: []byte(`{"model":"updated"}`)}) + if err != nil { + t.Fatal(err) + } + var fields map[string]json.RawMessage + if err := json.Unmarshal(updated.Configuration, &fields); err != nil { + t.Fatal(err) + } + if string(fields["number"]) != "9007199254740993" || string(fields["model"]) != `"updated"` || !reflect.DeepEqual(updated.Metadata, original.Metadata) { + t.Fatal("unrelated configuration or metadata lost") + } + if !updated.CreatedAt.Equal(original.CreatedAt) || updated.UpdatedAt.Before(original.UpdatedAt) { + t.Fatal("resource timestamps changed incorrectly") + } + unchanged, err := service.Update(ctx, agents.UpdateCommand{TenantID: tenant, AgentID: original.ID}) + if err != nil || !unchanged.UpdatedAt.After(updated.UpdatedAt) { + t.Fatalf("empty update did not advance timestamp: %v", err) + } + updated.UpdatedAt = unchanged.UpdatedAt + if !reflect.DeepEqual(unchanged, updated) { + t.Fatal("empty update changed saved configuration") + } +} + +func TestAgentDeleteIsTenantScoped(t *testing.T) { + pool := pgtest.Open(t) + store, service := open(t, pool, nil) + ctx := t.Context() + tenant := uuid.NewString() + agent, err := service.Create(ctx, agents.CreateCommand{TenantID: tenant, Configuration: []byte(`{"model":"x"}`)}) + if err != nil { + t.Fatal(err) + } + for _, target := range []agents.DeleteCommand{{TenantID: uuid.NewString(), AgentID: agent.ID}, {TenantID: tenant, AgentID: "not-an-id"}} { + if _, err := service.Delete(ctx, target); !errors.Is(err, agents.ErrNotFound) { + t.Fatalf("foreign or malformed delete: %v", err) + } + } + if id, err := service.Delete(ctx, agents.DeleteCommand{TenantID: tenant, AgentID: agent.ID}); err != nil || id != agent.ID { + t.Fatalf("delete = %s, %v", id, err) + } + if _, err := store.GetAgent(ctx, tenant, agent.ID); !errors.Is(err, agents.ErrNotFound) { + t.Fatalf("deleted agent read: %v", err) + } + if _, err := service.Delete(ctx, agents.DeleteCommand{TenantID: tenant, AgentID: agent.ID}); !errors.Is(err, agents.ErrNotFound) { + t.Fatalf("repeated delete: %v", err) + } +} + +func TestAgentModelExecutionAtomicEncryptedSnapshot(t *testing.T) { + pool := pgtest.Open(t) + c := testCipher(t, 31) + store, service := open(t, pool, c) + ctx, tenant := t.Context(), uuid.NewString() + provider := providerFixture(0) + create := agents.CreateCommand{TenantID: tenant, Configuration: providerConfiguration(t, provider, "codex"), ModelProvider: provider} + agent, err := service.Create(ctx, create) + if err != nil { + t.Fatal(err) + } + var encrypted []byte + if err := pool.QueryRow(ctx, "SELECT encrypted_config FROM agent_model_execution WHERE agent_id=$1", agent.ID).Scan(&encrypted); err != nil { + t.Fatal(err) + } + if bytes.Contains(encrypted, []byte(provider.APIKey)) || bytes.Contains(agent.Configuration, []byte(provider.APIKey)) { + t.Fatal("provider secret exposed") + } + if _, err := c.OpenAgentModelExecution(encrypted, uuid.NewString(), agent.ID); err == nil { + t.Fatal("ciphertext was not tenant bound") + } + if _, err := c.OpenAgentModelExecution(encrypted, tenant, uuid.NewString()); err == nil { + t.Fatal("ciphertext was not Agent bound") + } + if _, _, err := store.GetAgentWithModelProvider(ctx, uuid.NewString(), agent.ID); !errors.Is(err, agents.ErrNotFound) { + t.Fatal("foreign tenant lookup succeeded") + } + snapshot := func(s *agentpg.Store) (agents.Agent, *v1.ModelProviderInput) { + t.Helper() + current, inherited, err := s.GetAgentWithModelProvider(ctx, tenant, agent.ID) + if err != nil { + t.Fatal(err) + } + return current, inherited + } + if _, inherited := snapshot(store); inherited == nil || *inherited != *provider { + t.Fatal("provider snapshot mismatch") + } + // Omitted provider updates and plain reads do not require the encryption key. + keylessStore, keyless := open(t, pool, nil) + if _, err := keyless.Update(ctx, agents.UpdateCommand{TenantID: tenant, AgentID: agent.ID, Configuration: []byte(`{"model":"new-model"}`)}); err != nil { + t.Fatal(err) + } + if _, err := keylessStore.GetAgent(ctx, tenant, agent.ID); err != nil { + t.Fatal("plain read required Agent decryption", err) + } + for name, s := range map[string]*agentpg.Store{"missing key": keylessStore, "wrong key": agentpg.New(pgunit.NewPool(pool), testCipher(t, 99))} { + if _, _, err := s.GetAgentWithModelProvider(ctx, tenant, agent.ID); !errors.Is(err, credentialcrypto.ErrUnavailable) { + t.Fatalf("%s opened the bundle: %v", name, err) + } + } + if _, err := keyless.Create(ctx, create); !errors.Is(err, credentialcrypto.ErrUnavailable) { + t.Fatal("unencrypted Agent create accepted", err) + } + replacement := providerFixture(1) + replace := func(tenant string) agents.UpdateCommand { + return agents.UpdateCommand{TenantID: tenant, AgentID: agent.ID, Configuration: providerConfiguration(t, replacement, "codex"), ModelProvider: &agents.ModelProviderChange{Provider: replacement}} + } + if _, err := service.Update(ctx, replace(uuid.NewString())); !errors.Is(err, agents.ErrNotFound) { + t.Fatal("foreign tenant replacement accepted", err) + } + if _, err := keyless.Update(ctx, replace(tenant)); !errors.Is(err, credentialcrypto.ErrUnavailable) { + t.Fatal("unencrypted replacement accepted", err) + } + if current, inherited := snapshot(store); inherited == nil || *inherited != *provider || !bytes.Contains(current.Configuration, []byte("new-model")) { + t.Fatal("failed replacement changed snapshot") + } + // A database rejection after secret replacement rolls both writes back. + invalidPatch, _ := json.Marshal(map[string]any{"model": "invalid\x00model", "x_agents_core": v1.SavedAgentCore{Harness: "codex", ModelProvider: replacement.SafeView()}}) + if _, err := service.Update(ctx, agents.UpdateCommand{TenantID: tenant, AgentID: agent.ID, Configuration: invalidPatch, ModelProvider: &agents.ModelProviderChange{Provider: replacement}}); !errors.Is(err, textvalue.ErrUnstorable) { + t.Fatal("unstorable configuration accepted", err) + } + if current, inherited := snapshot(store); inherited == nil || *inherited != *provider || !bytes.Contains(current.Configuration, []byte("new-model")) { + t.Fatal("database rejection left a partial replacement") + } + if n := count(t, pool, "SELECT count(*) FROM agents WHERE tenant_id=$1", tenant); n != 1 { + t.Fatal("failed create persisted partial Agent", n) + } + if _, err := service.Update(ctx, agents.UpdateCommand{TenantID: tenant, AgentID: agent.ID, Configuration: []byte(`{"x_agents_core":{"harness":"claude_sdk"}}`)}); !errors.Is(err, agents.ErrInvalidInput) { + t.Fatal("incompatible Harness-only update accepted", err) + } + // Provider-only replacement preserves the existing harness. + patch, _ := json.Marshal(map[string]any{"x_agents_core": map[string]any{"model_provider": replacement.SafeView()}}) + if _, err := service.Update(ctx, agents.UpdateCommand{TenantID: tenant, AgentID: agent.ID, Configuration: patch, ModelProvider: &agents.ModelProviderChange{Provider: replacement}}); err != nil { + t.Fatal(err) + } + if current, inherited := snapshot(store); inherited == nil || *inherited != *replacement || !bytes.Contains(current.Configuration, []byte(`"harness": "codex"`)) { + t.Fatal("provider-only replacement failed") + } + if _, err := keyless.Update(ctx, agents.UpdateCommand{TenantID: tenant, AgentID: agent.ID, Configuration: []byte(`{"x_agents_core":{"harness":"codex"}}`)}); err != nil { + t.Fatal(err) + } + if _, inherited := snapshot(store); inherited == nil || *inherited != *replacement { + t.Fatal("harness-only update lost provider") + } + if _, err := keyless.Update(ctx, agents.UpdateCommand{TenantID: tenant, AgentID: agent.ID, Configuration: []byte(`{"x_agents_core":{"model_provider":null}}`), ModelProvider: &agents.ModelProviderChange{}}); err != nil { + t.Fatal(err) + } + if current, inherited := snapshot(store); inherited != nil || !bytes.Contains(current.Configuration, []byte(`"harness": "codex"`)) { + t.Fatal("clear lost harness or retained provider") + } + if _, err := service.Update(ctx, replace(tenant)); err != nil { + t.Fatal(err) + } + if _, err := service.Update(ctx, agents.UpdateCommand{TenantID: tenant, AgentID: agent.ID, Configuration: []byte(`{"x_agents_core":null}`), ModelProvider: &agents.ModelProviderChange{}}); err != nil { + t.Fatal(err) + } + if n := count(t, pool, "SELECT count(*) FROM agent_model_execution WHERE agent_id=$1", agent.ID); n != 0 { + t.Fatal("extension clear retained secret") + } + if _, err := service.Update(ctx, replace(tenant)); err != nil { + t.Fatal(err) + } + if _, err := service.Delete(ctx, agents.DeleteCommand{TenantID: tenant, AgentID: agent.ID}); err != nil { + t.Fatal(err) + } + if n := count(t, pool, "SELECT count(*) FROM agent_model_execution WHERE agent_id=$1", agent.ID); n != 0 { + t.Fatal("Agent delete retained secret") + } +} + +func TestAgentModelExecutionConcurrentSnapshots(t *testing.T) { + pool := pgtest.Open(t) + store, service := open(t, pool, testCipher(t, 32)) + ctx, tenant := t.Context(), uuid.NewString() + p := providerFixture(0) + agent, err := service.Create(ctx, agents.CreateCommand{TenantID: tenant, Configuration: providerConfiguration(t, p, "codex"), ModelProvider: p}) + if err != nil { + t.Fatal(err) + } + configurations := make([]json.RawMessage, 31) + for i := range configurations { + configurations[i] = providerConfiguration(t, providerFixture(i), "codex") + } + var wg sync.WaitGroup + failures := make(chan error, 2) + wg.Add(2) + go func() { + defer wg.Done() + for i := 1; i <= 30; i++ { + change := &agents.ModelProviderChange{Provider: providerFixture(i)} + if _, err := service.Update(ctx, agents.UpdateCommand{TenantID: tenant, AgentID: agent.ID, Configuration: configurations[i], ModelProvider: change}); err != nil { + failures <- err + return + } + } + }() + go func() { + defer wg.Done() + for i := 0; i < 80; i++ { + a, p, err := store.GetAgentWithModelProvider(ctx, tenant, agent.ID) + if err != nil { + failures <- err + return + } + var config struct { + Core v1.SavedAgentCore `json:"x_agents_core"` + } + if json.Unmarshal(a.Configuration, &config) != nil || p == nil || config.Core.ModelProvider == nil || config.Core.ModelProvider.BaseURL != p.BaseURL { + failures <- errors.New("concurrent read mixed safe and secret snapshots") + return + } + } + }() + wg.Wait() + close(failures) + for err := range failures { + t.Error(err) + } +} + +func auditContext(ctx context.Context, tenant, request, key string) context.Context { + return writeaudit.WithSource(ctx, writeaudit.Source{ + KeyID: "static:" + strings.Repeat(key, 64), Name: "agent audit fixture", Prefix: strings.Repeat(key, 8), + Kind: "static", TenantID: tenant, RequestID: request, TraceID: "agent-audit-trace", + }) +} + +// A trigger fails the audit insertion after each real mutation; comparing the +// tenant's rows proves the Agent, its sealed bundle and the audit roll back +// together. An administrator delete records administrator audit only. +func TestAgentWritesAuditInTheirTransaction(t *testing.T) { + pool := pgtest.OpenIsolated(t, nil) + _, service := open(t, pool, testCipher(t, 91)) + ctx := t.Context() + if _, err := pool.Exec(ctx, `CREATE FUNCTION reject_agent_audit_fixture() RETURNS trigger LANGUAGE plpgsql AS $$ + BEGIN IF NEW.request_id = 'reject-agent-audit' THEN RAISE EXCEPTION 'forced audit insertion failure'; END IF; RETURN NEW; END $$; + CREATE TRIGGER reject_agent_audit_fixture BEFORE INSERT ON write_audit_operations FOR EACH ROW EXECUTE FUNCTION reject_agent_audit_fixture(); + CREATE TRIGGER reject_agent_admin_audit_fixture BEFORE INSERT ON admin_audit_log FOR EACH ROW EXECUTE FUNCTION reject_agent_audit_fixture()`); err != nil { + t.Fatal(err) + } + snapshot := func(tenant string) string { + t.Helper() + var rows string + if err := pool.QueryRow(ctx, `SELECT concat_ws('|', + (SELECT COALESCE(jsonb_agg(to_jsonb(a) ORDER BY a.id)::text, '') FROM agents a WHERE a.tenant_id=$1), + (SELECT COALESCE(jsonb_agg(to_jsonb(m) ORDER BY m.agent_id)::text, '') FROM agent_model_execution m JOIN agents a ON a.id=m.agent_id WHERE a.tenant_id=$1), + (SELECT count(*)::text FROM write_audit_operations WHERE tenant_id=$1), + (SELECT count(*)::text FROM write_audit_owners WHERE tenant_id=$1), + (SELECT count(*)::text FROM admin_audit_log WHERE tenant_id=$1))`, tenant).Scan(&rows); err != nil { + t.Fatal(err) + } + return rows + } + provider := providerFixture(0) + for _, action := range []string{"create", "update", "delete"} { + t.Run(action, func(t *testing.T) { + tenant := uuid.NewString() + create := agents.CreateCommand{TenantID: tenant, Configuration: providerConfiguration(t, provider, "codex"), ModelProvider: provider} + run := func(ctx context.Context) (string, error) { + a, err := service.Create(ctx, create) + return a.ID, err + } + owners := 1 + if action != "create" { + existing, err := service.Create(ctx, create) + if err != nil { + t.Fatal(err) + } + owners = 0 + run = func(ctx context.Context) (string, error) { + if action == "delete" { + return service.Delete(ctx, agents.DeleteCommand{TenantID: tenant, AgentID: existing.ID}) + } + change := &agents.ModelProviderChange{Provider: providerFixture(1)} + a, err := service.Update(ctx, agents.UpdateCommand{TenantID: tenant, AgentID: existing.ID, Configuration: providerConfiguration(t, change.Provider, "codex"), ModelProvider: change}) + return a.ID, err + } + } + before := snapshot(tenant) + if _, err := run(auditContext(ctx, tenant, "reject-agent-audit", "a")); err == nil { + t.Fatal("audit failure was accepted") + } + if after := snapshot(tenant); after != before { + t.Fatal("audit failure left business or audit changes") + } + request := uuid.NewString() + id, err := run(auditContext(ctx, tenant, request, "a")) + if err != nil { + t.Fatal(err) + } + var gotAction, kind, gotID string + var parent *string + if err := pool.QueryRow(ctx, `SELECT action,resource_type,resource_id,parent_id FROM write_audit_operations WHERE tenant_id=$1 AND request_id=$2`, tenant, request).Scan(&gotAction, &kind, &gotID, &parent); err != nil { + t.Fatal(err) + } + if gotAction != action || kind != "agent" || gotID != id || parent != nil && *parent != "" { + t.Fatalf("wrong operation identity: %s %s %s %v", gotAction, kind, gotID, parent) + } + if n := count(t, pool, `SELECT count(*) FROM write_audit_owners WHERE tenant_id=$1`, tenant); n != owners { + t.Fatalf("ownership count %d, want %d", n, owners) + } + if n := count(t, pool, `SELECT count(*) FROM write_audit_operations WHERE tenant_id=$1 AND to_jsonb(write_audit_operations)::text LIKE '%private-agent-canary%'`, tenant); n != 0 { + t.Fatal("audit contains secret") + } + }) + } + t.Run("administrator delete", func(t *testing.T) { + tenant := uuid.NewString() + // Administrator audit names the Project that owns the tenant. + if _, err := pool.Exec(ctx, "INSERT INTO execution_project_scopes(tenant_id,organization_id,project_id) VALUES($1,'agent-admin',$2)", tenant, tenant); err != nil { + t.Fatal(err) + } + if _, err := pool.Exec(ctx, "INSERT INTO projects(id,name,tenant_id,subject_kind,subject_id) VALUES($1,'Agent fixture',$1,'service_account',$2)", tenant, "project:"+tenant); err != nil { + t.Fatal(err) + } + agent, err := service.Create(ctx, agents.CreateCommand{TenantID: tenant, Configuration: providerConfiguration(t, provider, "codex"), ModelProvider: provider}) + if err != nil { + t.Fatal(err) + } + // Administrator provenance takes precedence over an inherited public one. + adminContext := func(request string) context.Context { + return adminaudit.WithSource(auditContext(ctx, tenant, request, "a"), adminaudit.Source{ + CredentialID: "87654321", ActorLabel: "administrator fixture", ProjectID: tenant, RequestID: request, TraceID: "admin-trace", + }) + } + before := snapshot(tenant) + if _, err := service.Delete(adminContext("reject-agent-audit"), agents.DeleteCommand{TenantID: tenant, AgentID: agent.ID}); err == nil { + t.Fatal("administrator audit failure was accepted") + } + if snapshot(tenant) != before { + t.Fatal("administrator audit failure left business or audit changes") + } + request := uuid.NewString() + if _, err := service.Delete(adminContext(request), agents.DeleteCommand{TenantID: tenant, AgentID: agent.ID}); err != nil { + t.Fatal(err) + } + var action, kind, id, raw string + if err := pool.QueryRow(ctx, `SELECT action,resource_type,resource_id,to_jsonb(a)::text FROM admin_audit_log a WHERE tenant_id=$1 AND request_id=$2`, tenant, request).Scan(&action, &kind, &id, &raw); err != nil { + t.Fatal(err) + } + if action != "delete" || kind != "agent" || id != agent.ID || strings.Contains(raw, "private-agent-canary") { + t.Fatalf("administrator audit = %s", raw) + } + if n := count(t, pool, `SELECT count(*) FROM write_audit_operations WHERE tenant_id=$1`, tenant); n != 0 { + t.Fatal("administrator impersonated public-key provenance") + } + if n := count(t, pool, `SELECT count(*) FROM agent_model_execution WHERE agent_id=$1`, agent.ID); n != 0 { + t.Fatal("administrator delete retained the sealed bundle") + } + }) +} + +// Reads and failed writes record nothing, and the creator stays the owner +// after another key updates and deletes the Agent. +func TestAgentAuditReadsFailuresAndStableOwnership(t *testing.T) { + pool := pgtest.Open(t) + store, service := open(t, pool, nil) + tenant := uuid.NewString() + first, err := service.Create(auditContext(t.Context(), tenant, uuid.NewString(), "a"), agents.CreateCommand{TenantID: tenant, Configuration: []byte(`{"model":"fixture"}`)}) + if err != nil { + t.Fatal(err) + } + owner := func() string { + var row string + if err := pool.QueryRow(t.Context(), `SELECT to_jsonb(o)::text FROM write_audit_owners o WHERE tenant_id=$1 AND resource_id=$2`, tenant, first.ID).Scan(&row); err != nil { + t.Fatal("creator disappeared", err) + } + return row + } + before := owner() + readCtx := auditContext(t.Context(), tenant, uuid.NewString(), "a") + if _, err := store.GetAgent(readCtx, tenant, first.ID); err != nil { + t.Fatal(err) + } + if _, err := store.ListAgents(readCtx, agents.ListQuery{TenantID: tenant, Limit: 1}); err != nil { + t.Fatal(err) + } + if _, err := service.Update(readCtx, agents.UpdateCommand{TenantID: tenant, AgentID: uuid.NewString()}); !errors.Is(err, agents.ErrNotFound) { + t.Fatal(err) + } + if _, err := service.Update(auditContext(t.Context(), tenant, uuid.NewString(), "b"), agents.UpdateCommand{TenantID: tenant, AgentID: first.ID}); err != nil { + t.Fatal(err) + } + if _, err := service.Delete(auditContext(t.Context(), tenant, uuid.NewString(), "b"), agents.DeleteCommand{TenantID: tenant, AgentID: first.ID}); err != nil { + t.Fatal(err) + } + if owner() != before { + t.Fatal("creator changed") + } + if n := count(t, pool, `SELECT count(*) FROM write_audit_operations WHERE tenant_id=$1`, tenant); n != 3 { + t.Fatal("read or failure audit", n) + } +} + +// Malformed supplied provenance fails the write closed. +func TestAgentWriteRejectsInvalidAuditSource(t *testing.T) { + pool := pgtest.Open(t) + _, service := open(t, pool, nil) + tenant := uuid.NewString() + ctx := writeaudit.WithSource(t.Context(), writeaudit.Source{KeyID: "static:" + strings.Repeat("a", 64), Prefix: "aaaaaaaa", Kind: "static", TenantID: tenant, RequestID: uuid.NewString()}) + if _, err := service.Create(ctx, agents.CreateCommand{TenantID: tenant, Configuration: []byte(`{"model":"x"}`)}); !errors.Is(err, writeaudit.ErrInvalidSource) { + t.Fatalf("invalid source accepted: %v", err) + } + if n := count(t, pool, "SELECT count(*) FROM agents WHERE tenant_id=$1", tenant); n != 0 { + t.Fatal("invalid source wrote an Agent") + } +} diff --git a/services/core/internal/store/admin_delete_audit_test.go b/services/core/internal/store/admin_delete_audit_test.go index 4dedf9484..4e98ffcba 100644 --- a/services/core/internal/store/admin_delete_audit_test.go +++ b/services/core/internal/store/admin_delete_audit_test.go @@ -105,7 +105,7 @@ func TestAdminDeleteResourceAuditTransactions(t *testing.T) { rejectAdminAuditInsert(t, s) archive := skillArchive(t, "admin-private-archive") tables := []string{"agents", "agent_model_execution", "environment_templates", "skills", "skill_versions", "source_files", "vaults", "vault_credentials", "sessions", "turns", "environments", "session_artifacts", "admin_audit_log", "write_audit_operations", "write_audit_owners", "pg_largeobject_metadata", "pg_largeobject"} - for _, name := range []string{"agent_delete", "template_delete", "skill_delete", "version_delete", "version_delete_last", "file_delete", "vault_delete", "credential_delete", "oauth_delete", "session_delete", "artifact_delete"} { + for _, name := range []string{"template_delete", "skill_delete", "version_delete", "version_delete_last", "file_delete", "vault_delete", "credential_delete", "oauth_delete", "session_delete", "artifact_delete"} { t.Run(name, func(t *testing.T) { tenant := uuid.NewString() var mutation resourceAuditMutation @@ -118,16 +118,6 @@ func TestAdminDeleteResourceAuditTransactions(t *testing.T) { if name == "file_delete" { removedObjects = 1 } - if name == "agent_delete" { - var id string - if err := pool.QueryRow(t.Context(), "SELECT id FROM agents WHERE tenant_id=$1", tenant).Scan(&id); err != nil { - t.Fatal(err) - } - provider := agentProviderFixture(94) - if _, err := s.UpdateAgent(t.Context(), tenant, id, UpdateAgentInput{Configuration: agentProviderConfiguration(t, provider, "codex"), ModelProvider: provider, ModelProviderSet: true}); err != nil { - t.Fatal(err) - } - } if name == "template_delete" { var id string if err := pool.QueryRow(t.Context(), "SELECT id FROM environment_templates WHERE tenant_id=$1", tenant).Scan(&id); err != nil { @@ -214,8 +204,6 @@ func assertAdminDeletedResource(t *testing.T, s *Store, tenant string, mutation t.Helper() var err error switch mutation.kind { - case "agent": - _, err = s.GetAgent(t.Context(), tenant, id) case "environment_template": _, err = s.GetEnvironmentTemplate(t.Context(), tenant, id) case "skill": diff --git a/services/core/internal/store/admin_key_audit_test.go b/services/core/internal/store/admin_key_audit_test.go index 6a615d1d6..364776977 100644 --- a/services/core/internal/store/admin_key_audit_test.go +++ b/services/core/internal/store/admin_key_audit_test.go @@ -7,18 +7,20 @@ import ( "testing" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/adminaudit" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/google/uuid" ) func TestAdminProjectAndKeyDatabaseAuditTransactions(t *testing.T) { - s, _ := newManagedTestStore(t) + s, pool := newManagedTestStore(t) rejectAdminAuditInsert(t, s) + agentStore, agentService := testAgents(t, pool, fixtureCipher) for _, action := range []string{"project_create", "rename", "archive", "key_create", "revoke"} { t.Run(action, func(t *testing.T) { projectID, keyID := uuid.NewString(), uuid.NewString() var p Project var issued IssuedProjectAPIKey - var asset SavedAgent + var asset agents.Agent ctxFor := func(request string) context.Context { return adminaudit.WithSource(t.Context(), adminaudit.Source{CredentialID: "87654321", ActorLabel: "administrator fixture", ProjectID: projectID, RequestID: request, TraceID: "admin-mutation-trace"}) } @@ -28,7 +30,7 @@ func TestAdminProjectAndKeyDatabaseAuditTransactions(t *testing.T) { if err != nil { t.Fatal(err) } - asset, err = s.CreateAgent(t.Context(), p.TenantID, CreateAgentInput{Configuration: []byte(`{"model":"fixture"}`)}) + asset, err = agentService.Create(t.Context(), agents.CreateCommand{TenantID: p.TenantID, Configuration: []byte(`{"model":"fixture"}`)}) if err != nil { t.Fatal(err) } @@ -112,7 +114,7 @@ func TestAdminProjectAndKeyDatabaseAuditTransactions(t *testing.T) { } } if action != "project_create" { - if _, err := s.GetAgent(t.Context(), p.TenantID, asset.ID); err != nil { + if _, err := agentStore.GetAgent(t.Context(), p.TenantID, asset.ID); err != nil { t.Fatal("management mutation removed assets", err) } } diff --git a/services/core/internal/store/agent_model_execution.go b/services/core/internal/store/agent_model_execution.go deleted file mode 100644 index 5a6cb0567..000000000 --- a/services/core/internal/store/agent_model_execution.go +++ /dev/null @@ -1,147 +0,0 @@ -package store - -import ( - "context" - "encoding/json" - "errors" - "fmt" - - v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/credentialcrypto" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/db/sqlc" - "github.com/google/uuid" - "github.com/jackc/pgx/v5" - "github.com/jackc/pgx/v5/pgtype" -) - -func (s *Store) saveAgentModelExecution(ctx context.Context, q *sqlc.Queries, tenant string, agent pgtype.UUID, provider *v1.ModelProviderInput) error { - if provider == nil { - return q.DeleteAgentModelExecution(ctx, agent) - } - raw, err := json.Marshal(provider) - if err != nil { - return err - } - encrypted, err := s.credentialCipher.SealAgentModelExecution(raw, tenant, uuid.UUID(agent.Bytes).String()) - if err != nil { - return credentialcrypto.ErrUnavailable - } - return q.SaveAgentModelExecution(ctx, sqlc.SaveAgentModelExecutionParams{AgentID: agent, EncryptedConfig: encrypted}) -} - -// GetAgentForSession reads the safe configuration and secret from one database -// snapshot. Explicit Session provider overrides never require Agent decryption. -func (s *Store) GetAgentForSession(ctx context.Context, tenantID, agentID string, inheritProvider bool) (SavedAgent, *v1.ModelProviderInput, error) { - tenant, err := parseID(tenantID) - if err != nil { - return SavedAgent{}, nil, err - } - id, err := parseID(agentID) - if err != nil { - return SavedAgent{}, nil, err - } - row, err := s.queries.GetAgentForSession(ctx, sqlc.GetAgentForSessionParams{TenantID: tenant, AgentID: id}) - if errors.Is(err, pgx.ErrNoRows) { - return SavedAgent{}, nil, ErrNotFound - } - if err != nil { - return SavedAgent{}, nil, fmt.Errorf("get agent for session: %w", err) - } - agent, err := agentFromRow(sqlc.Agent{ID: row.ID, TenantID: row.TenantID, Metadata: row.Metadata, Configuration: row.Configuration, CreatedAt: row.CreatedAt, UpdatedAt: row.UpdatedAt}) - if err != nil || !inheritProvider { - return agent, nil, err - } - var config struct { - Core *v1.SavedAgentCore `json:"x_agents_core"` - } - if err := json.Unmarshal(agent.Configuration, &config); err != nil { - return SavedAgent{}, nil, ErrInvalidInput - } - if config.Core == nil || config.Core.ModelProvider == nil { - return agent, nil, nil - } - raw, err := s.credentialCipher.OpenAgentModelExecution(row.EncryptedConfig, uuid.UUID(tenant.Bytes).String(), uuid.UUID(id.Bytes).String()) - if err != nil { - return SavedAgent{}, nil, credentialcrypto.ErrUnavailable - } - var provider v1.ModelProviderInput - if json.Unmarshal(raw, &provider) != nil || provider.Validate() != nil { - return SavedAgent{}, nil, credentialcrypto.ErrUnavailable - } - return agent, &provider, nil -} - -func validateAgentModelExecution(configuration []byte, provider *v1.ModelProviderInput) error { - if provider != nil { - if err := provider.Validate(); err != nil { - return fmt.Errorf("%w: %s", ErrInvalidInput, err) - } - } - var config struct { - Core *v1.SavedAgentCore `json:"x_agents_core"` - } - if err := json.Unmarshal(configuration, &config); err != nil { - return ErrInvalidInput - } - if config.Core != nil { - if err := v1.ValidateHarnessConfig(config.Core.Harness, config.Core.HarnessConfig); err != nil { - return fmt.Errorf("%w: %s", ErrInvalidInput, err) - } - } - if config.Core == nil || config.Core.ModelProvider == nil || config.Core.Harness == "" { - return nil - } - if err := config.Core.ModelProvider.ValidateHarness(config.Core.Harness); err != nil { - return fmt.Errorf("%w: %s", ErrInvalidInput, err) - } - return nil -} - -func mergeAgentConfiguration(configuration, patch map[string]json.RawMessage) error { - _, modelChanged := patch["model"] - var corePatch map[string]json.RawMessage - if raw := patch["x_agents_core"]; len(raw) > 0 && string(raw) != "null" { - if err := json.Unmarshal(raw, &corePatch); err != nil { - return err - } - } - _, providerChanged := corePatch["model_provider"] - _, harnessChanged := corePatch["harness"] - _, nativeSupplied := corePatch["harness_config"] - if (modelChanged || providerChanged || harnessChanged) && !nativeSupplied && string(patch["x_agents_core"]) != "null" { - if corePatch == nil { - corePatch = map[string]json.RawMessage{} - } - corePatch["harness_config"] = json.RawMessage(`{}`) - raw, err := json.Marshal(corePatch) - if err != nil { - return err - } - patch["x_agents_core"] = raw - } - - for field, value := range patch { - if field == "x_agents_core" && string(value) != "null" { - core := map[string]json.RawMessage{} - if old := configuration[field]; len(old) != 0 && string(old) != "null" { - if err := json.Unmarshal(old, &core); err != nil { - return err - } - } - var changes map[string]json.RawMessage - if err := json.Unmarshal(value, &changes); err != nil { - return err - } - for key, replacement := range changes { - core[key] = replacement - } - merged, err := json.Marshal(core) - if err != nil { - return err - } - value = merged - } - configuration[field] = value - } - return nil -} diff --git a/services/core/internal/store/agent_model_execution_test.go b/services/core/internal/store/agent_model_execution_test.go deleted file mode 100644 index a6d0dfcf0..000000000 --- a/services/core/internal/store/agent_model_execution_test.go +++ /dev/null @@ -1,207 +0,0 @@ -package store - -import ( - "bytes" - "encoding/json" - "errors" - "fmt" - "sync" - "testing" - - v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/credentialcrypto" - "github.com/google/uuid" -) - -func agentProviderFixture(sequence int) *v1.ModelProviderInput { - return &v1.ModelProviderInput{Protocol: "responses", BaseURL: fmt.Sprintf("https://provider-%d.example/v1", sequence), APIKey: fmt.Sprintf("private-agent-canary-%d", sequence)} -} - -func agentProviderConfiguration(t *testing.T, provider *v1.ModelProviderInput, harness string) json.RawMessage { - t.Helper() - raw, err := json.Marshal(map[string]any{"model": "actual-model", "x_agents_core": v1.SavedAgentCore{Harness: harness, ModelProvider: provider.SafeView()}}) - if err != nil { - t.Fatal(err) - } - return raw -} - -func TestAgentModelExecutionAtomicEncryptedSnapshot(t *testing.T) { - _, pool := testStore(t) - c, err := credentialcrypto.New(bytes.Repeat([]byte{31}, 32)) - if err != nil { - t.Fatal(err) - } - s := NewWithCredentialCipher(pool, c) - ctx, tenant := t.Context(), uuid.NewString() - provider := agentProviderFixture(0) - input := CreateAgentInput{Configuration: agentProviderConfiguration(t, provider, "codex"), ModelProvider: provider} - agent, err := s.CreateAgent(ctx, tenant, input) - if err != nil { - t.Fatal(err) - } - var encrypted []byte - if err := pool.QueryRow(ctx, "SELECT encrypted_config FROM agent_model_execution WHERE agent_id=$1", agent.ID).Scan(&encrypted); err != nil { - t.Fatal(err) - } - if bytes.Contains(encrypted, []byte(provider.APIKey)) || bytes.Contains(agent.Configuration, []byte(provider.APIKey)) { - t.Fatal("provider secret exposed") - } - if _, err := c.OpenAgentModelExecution(encrypted, uuid.NewString(), agent.ID); err == nil { - t.Fatal("ciphertext was not tenant bound") - } - if _, err := c.OpenAgentModelExecution(encrypted, tenant, uuid.NewString()); err == nil { - t.Fatal("ciphertext was not Agent bound") - } - if _, _, err := s.GetAgentForSession(ctx, uuid.NewString(), agent.ID, true); !errors.Is(err, ErrNotFound) { - t.Fatal("foreign tenant lookup succeeded") - } - _, inherited, err := s.GetAgentForSession(ctx, tenant, agent.ID, true) - if err != nil || inherited == nil || *inherited != *provider { - t.Fatal("provider snapshot mismatch", err) - } - session, err := s.CreateSession(ctx, tenant, CreateSessionInput{Creator: FixtureCreator(), Engine: "codex", IdempotencyKey: uuid.NewString(), Configuration: []byte(`{"agent":{"model":"actual-model"},"environment":{"type":"openai_hosted"}}`), ModelProvider: inherited}) - if err != nil { - t.Fatal(err) - } - // Omitted provider updates do not require access to the encryption key. - withoutKey := New(pool) - if _, err := withoutKey.UpdateAgent(ctx, tenant, agent.ID, UpdateAgentInput{Configuration: []byte(`{"model":"new-model"}`)}); err != nil { - t.Fatal(err) - } - if _, _, err := withoutKey.GetAgentForSession(ctx, tenant, agent.ID, false); err != nil { - t.Fatal("explicit override required Agent decryption", err) - } - if _, _, err := withoutKey.GetAgentForSession(ctx, tenant, agent.ID, true); !errors.Is(err, credentialcrypto.ErrUnavailable) { - t.Fatal("missing cipher accepted", err) - } - if _, err := withoutKey.CreateAgent(ctx, tenant, input); !errors.Is(err, credentialcrypto.ErrUnavailable) { - t.Fatal("unencrypted Agent create accepted", err) - } - replacement := agentProviderFixture(1) - if _, err := s.UpdateAgent(ctx, uuid.NewString(), agent.ID, UpdateAgentInput{Configuration: agentProviderConfiguration(t, replacement, "codex"), ModelProvider: replacement, ModelProviderSet: true}); !errors.Is(err, ErrNotFound) { - t.Fatal("foreign tenant replacement accepted", err) - } - if _, err := withoutKey.UpdateAgent(ctx, tenant, agent.ID, UpdateAgentInput{Configuration: agentProviderConfiguration(t, replacement, "codex"), ModelProvider: replacement, ModelProviderSet: true}); !errors.Is(err, credentialcrypto.ErrUnavailable) { - t.Fatal("unencrypted replacement accepted", err) - } - current, inherited, err := s.GetAgentForSession(ctx, tenant, agent.ID, true) - if err != nil || inherited == nil || *inherited != *provider || !bytes.Contains(current.Configuration, []byte("new-model")) { - t.Fatal("failed replacement changed snapshot", err) - } - // A database rejection after secret replacement rolls both writes back. - invalidPatch, _ := json.Marshal(map[string]any{"model": "invalid\x00model", "x_agents_core": v1.SavedAgentCore{Harness: "codex", ModelProvider: replacement.SafeView()}}) - if _, err := s.UpdateAgent(ctx, tenant, agent.ID, UpdateAgentInput{Configuration: invalidPatch, ModelProvider: replacement, ModelProviderSet: true}); err == nil { - t.Fatal("unstorable configuration accepted") - } - current, inherited, err = s.GetAgentForSession(ctx, tenant, agent.ID, true) - if err != nil || inherited == nil || *inherited != *provider || !bytes.Contains(current.Configuration, []byte("new-model")) { - t.Fatal("database rejection left a partial replacement", err) - } - var count int - if err := pool.QueryRow(ctx, "SELECT count(*) FROM agents WHERE tenant_id=$1", tenant).Scan(&count); err != nil || count != 1 { - t.Fatal("failed create persisted partial Agent", err) - } - if _, err := s.UpdateAgent(ctx, tenant, agent.ID, UpdateAgentInput{Configuration: []byte(`{"x_agents_core":{"harness":"claude_sdk"}}`)}); !errors.Is(err, ErrInvalidInput) { - t.Fatal("incompatible Harness-only update accepted", err) - } - // Provider-only replacement preserves the existing harness. - patch, _ := json.Marshal(map[string]any{"x_agents_core": map[string]any{"model_provider": replacement.SafeView()}}) - if _, err := s.UpdateAgent(ctx, tenant, agent.ID, UpdateAgentInput{Configuration: patch, ModelProvider: replacement, ModelProviderSet: true}); err != nil { - t.Fatal(err) - } - current, inherited, err = s.GetAgentForSession(ctx, tenant, agent.ID, true) - if err != nil || inherited == nil || *inherited != *replacement || !bytes.Contains(current.Configuration, []byte(`"harness": "codex"`)) { - t.Fatal("provider-only replacement failed", err) - } - if _, err := withoutKey.UpdateAgent(ctx, tenant, agent.ID, UpdateAgentInput{Configuration: []byte(`{"x_agents_core":{"harness":"codex"}}`)}); err != nil { - t.Fatal(err) - } - _, inherited, err = s.GetAgentForSession(ctx, tenant, agent.ID, true) - if err != nil || inherited == nil || *inherited != *replacement { - t.Fatal("harness-only update lost provider", err) - } - if _, err := withoutKey.UpdateAgent(ctx, tenant, agent.ID, UpdateAgentInput{Configuration: []byte(`{"x_agents_core":{"model_provider":null}}`), ModelProviderSet: true}); err != nil { - t.Fatal(err) - } - current, inherited, err = s.GetAgentForSession(ctx, tenant, agent.ID, true) - if err != nil || inherited != nil || !bytes.Contains(current.Configuration, []byte(`"harness": "codex"`)) { - t.Fatal("clear lost harness or retained provider", err) - } - if _, err := s.UpdateAgent(ctx, tenant, agent.ID, UpdateAgentInput{Configuration: agentProviderConfiguration(t, replacement, "codex"), ModelProvider: replacement, ModelProviderSet: true}); err != nil { - t.Fatal(err) - } - if _, err := s.UpdateAgent(ctx, tenant, agent.ID, UpdateAgentInput{Configuration: []byte(`{"x_agents_core":null}`), ModelProviderSet: true}); err != nil { - t.Fatal(err) - } - if err := pool.QueryRow(ctx, "SELECT count(*) FROM agent_model_execution WHERE agent_id=$1", agent.ID).Scan(&count); err != nil || count != 0 { - t.Fatal("extension clear retained secret", err) - } - if _, err := s.UpdateAgent(ctx, tenant, agent.ID, UpdateAgentInput{Configuration: agentProviderConfiguration(t, replacement, "codex"), ModelProvider: replacement, ModelProviderSet: true}); err != nil { - t.Fatal(err) - } - if _, err := s.DeleteAgent(ctx, tenant, agent.ID); err != nil { - t.Fatal(err) - } - if err := pool.QueryRow(ctx, "SELECT count(*) FROM agent_model_execution WHERE agent_id=$1", agent.ID).Scan(&count); err != nil || count != 0 { - t.Fatal("Agent delete retained secret", err) - } - frozen, err := s.SessionModelExecution(ctx, tenant, session.ID) - if err != nil || frozen == nil || *frozen != *provider { - t.Fatal("Agent mutation changed Session snapshot", err) - } -} - -func TestAgentModelExecutionConcurrentSnapshots(t *testing.T) { - _, pool := testStore(t) - c, err := credentialcrypto.New(bytes.Repeat([]byte{32}, 32)) - if err != nil { - t.Fatal(err) - } - s := NewWithCredentialCipher(pool, c) - ctx, tenant := t.Context(), uuid.NewString() - p := agentProviderFixture(0) - agent, err := s.CreateAgent(ctx, tenant, CreateAgentInput{Configuration: agentProviderConfiguration(t, p, "codex"), ModelProvider: p}) - if err != nil { - t.Fatal(err) - } - configurations := make([]json.RawMessage, 31) - for i := range configurations { - configurations[i] = agentProviderConfiguration(t, agentProviderFixture(i), "codex") - } - var wg sync.WaitGroup - failures := make(chan error, 2) - wg.Add(2) - go func() { - defer wg.Done() - for i := 1; i <= 30; i++ { - p := agentProviderFixture(i) - if _, err := s.UpdateAgent(ctx, tenant, agent.ID, UpdateAgentInput{Configuration: configurations[i], ModelProvider: p, ModelProviderSet: true}); err != nil { - failures <- err - return - } - } - }() - go func() { - defer wg.Done() - for i := 0; i < 80; i++ { - a, p, err := s.GetAgentForSession(ctx, tenant, agent.ID, true) - if err != nil { - failures <- err - return - } - var config struct { - Core v1.SavedAgentCore `json:"x_agents_core"` - } - if json.Unmarshal(a.Configuration, &config) != nil || p == nil || config.Core.ModelProvider == nil || config.Core.ModelProvider.BaseURL != p.BaseURL { - failures <- errors.New("concurrent read mixed safe and secret snapshots") - return - } - } - }() - wg.Wait() - close(failures) - for err := range failures { - t.Error(err) - } -} diff --git a/services/core/internal/store/agents.go b/services/core/internal/store/agents.go deleted file mode 100644 index 46419c728..000000000 --- a/services/core/internal/store/agents.go +++ /dev/null @@ -1,115 +0,0 @@ -package store - -import ( - "context" - "encoding/json" - "errors" - "fmt" - "time" - - v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" - "github.com/google/uuid" - "github.com/jackc/pgx/v5" - "github.com/jackc/pgx/v5/pgtype" - - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/db/sqlc" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/jsonobject" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/metadata" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/auditpg" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/writeaudit" -) - -// SavedAgent is reusable configuration owned by an execution tenant. It has no -// engine binding or live execution state; Session snapshots are separate objects. -type SavedAgent struct { - ID string - TenantID string - Metadata map[string]string - Configuration json.RawMessage - CreatedAt time.Time - UpdatedAt time.Time -} - -type CreateAgentInput struct { - ModelProvider *v1.ModelProviderInput - Metadata map[string]string - Configuration json.RawMessage -} - -// CreateAgent stores caller-validated, credential-free configuration. Public -// defaults and schema validation belong to the API, not an execution adapter. -// Each call creates a new resource; this primitive supplies no retry semantics. -func (s *Store) CreateAgent(ctx context.Context, tenantID string, input CreateAgentInput) (SavedAgent, error) { - tenant, err := parseID(tenantID) - if err != nil { - return SavedAgent{}, err - } - encodedMetadata, err := metadata.Encode(input.Metadata) - if err != nil { - return SavedAgent{}, fmt.Errorf("%w: %w", ErrInvalidInput, err) - } - if len(input.Configuration) == 0 || len(input.Configuration) > 512*1024 { - return SavedAgent{}, fmt.Errorf("%w: configuration must be an object of at most 512 KiB", ErrInvalidInput) - } - configuration, err := jsonobject.Normalize(input.Configuration) - if err != nil { - return SavedAgent{}, fmt.Errorf("%w: %w", ErrInvalidInput, err) - } - if err := validateAgentModelExecution(configuration, input.ModelProvider); err != nil { - return SavedAgent{}, err - } - var created SavedAgent - err = s.pooled.Transaction(ctx, func(ctx context.Context, tx pgx.Tx) error { - q := s.queries.WithTx(tx) - row, err := q.CreateAgent(ctx, sqlc.CreateAgentParams{ - ID: pgtype.UUID{Bytes: uuid.New(), Valid: true}, TenantID: tenant, - Metadata: encodedMetadata, Configuration: configuration, - }) - if err != nil { - return err - } - if err := s.saveAgentModelExecution(ctx, q, uuid.UUID(tenant.Bytes).String(), row.ID, input.ModelProvider); err != nil { - return err - } - created, err = agentFromRow(row) - if err != nil { - return err - } - return auditpg.RecordWriteAudit(ctx, q, tenantID, "create", "agent", created.ID, "", writeaudit.Resource{Type: "agent", ID: created.ID}) - }) - if err != nil { - return SavedAgent{}, fmt.Errorf("create agent: %w", err) - } - return created, nil -} - -// GetAgent scopes every lookup to the authenticated caller's tenant. -func (s *Store) GetAgent(ctx context.Context, tenantID, agentID string) (SavedAgent, error) { - tenant, err := parseID(tenantID) - if err != nil { - return SavedAgent{}, err - } - id, err := parseID(agentID) - if err != nil { - return SavedAgent{}, err - } - row, err := s.queries.GetAgent(ctx, sqlc.GetAgentParams{TenantID: tenant, ID: id}) - if errors.Is(err, pgx.ErrNoRows) { - return SavedAgent{}, ErrNotFound - } - if err != nil { - return SavedAgent{}, fmt.Errorf("get agent: %w", err) - } - return agentFromRow(row) -} - -func agentFromRow(row sqlc.Agent) (SavedAgent, error) { - agent := SavedAgent{ - ID: uuid.UUID(row.ID.Bytes).String(), TenantID: uuid.UUID(row.TenantID.Bytes).String(), - Configuration: row.Configuration, CreatedAt: row.CreatedAt.Time, UpdatedAt: row.UpdatedAt.Time, - } - if err := json.Unmarshal(row.Metadata, &agent.Metadata); err != nil { - return SavedAgent{}, fmt.Errorf("decode agent metadata: %w", err) - } - return agent, nil -} diff --git a/services/core/internal/store/agents_delete.go b/services/core/internal/store/agents_delete.go deleted file mode 100644 index 1d0cb6050..000000000 --- a/services/core/internal/store/agents_delete.go +++ /dev/null @@ -1,41 +0,0 @@ -package store - -import ( - "context" - "errors" - "fmt" - - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/db/sqlc" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/auditpg" - "github.com/google/uuid" - "github.com/jackc/pgx/v5" -) - -// DeleteAgent removes a saved resource independently of execution snapshots. -func (s *Store) DeleteAgent(ctx context.Context, tenantID, agentID string) (string, error) { - tenant, err := parseID(tenantID) - if err != nil { - return "", err - } - id, err := parseID(agentID) - if err != nil { - return "", err - } - var deletedID string - err = s.pooled.Transaction(ctx, func(ctx context.Context, tx pgx.Tx) error { - q := s.queries.WithTx(tx) - deleted, err := q.DeleteAgent(ctx, sqlc.DeleteAgentParams{TenantID: tenant, ID: id}) - if err != nil { - return err - } - deletedID = uuid.UUID(deleted.Bytes).String() - return auditpg.RecordWriteAudit(ctx, q, tenantID, "delete", "agent", deletedID, "") - }) - if errors.Is(err, pgx.ErrNoRows) { - return "", ErrNotFound - } - if err != nil { - return "", fmt.Errorf("delete agent: %w", err) - } - return deletedID, nil -} diff --git a/services/core/internal/store/agents_fixture_test.go b/services/core/internal/store/agents_fixture_test.go new file mode 100644 index 000000000..037c87f0d --- /dev/null +++ b/services/core/internal/store/agents_fixture_test.go @@ -0,0 +1,24 @@ +package store + +import ( + "testing" + + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/credentialcrypto" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/agentpg" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" + "github.com/jackc/pgx/v5/pgxpool" +) + +// testAgents builds the Agent adapter and service on the database and +// credential key that built the test's Store, for tests that need saved Agents +// as fixtures. +func testAgents(t testing.TB, pool *pgxpool.Pool, cipher *credentialcrypto.Cipher) (*agentpg.Store, *agents.Service) { + t.Helper() + agentStore := agentpg.New(pgunit.NewPool(pool), cipher) + agentService, err := agents.NewService(agentStore) + if err != nil { + t.Fatal(err) + } + return agentStore, agentService +} diff --git a/services/core/internal/store/agents_list.go b/services/core/internal/store/agents_list.go deleted file mode 100644 index f8c59e252..000000000 --- a/services/core/internal/store/agents_list.go +++ /dev/null @@ -1,52 +0,0 @@ -package store - -import ( - "context" - "fmt" - - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/db/sqlc" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" - "github.com/google/uuid" - "github.com/jackc/pgx/v5/pgtype" -) - -type AgentPage struct { - Agents []SavedAgent - NextCursor string -} - -func (s *Store) ListAgents(ctx context.Context, tenantID, cursor string, limit int, ascending bool) (AgentPage, error) { - tenant, err := parseID(tenantID) - if err != nil { - return AgentPage{}, err - } - if limit < 1 || limit > 100 { - return AgentPage{}, fmt.Errorf("%w: internal page size must be 1..100", ErrInvalidInput) - } - params := sqlc.ListAgentsParams{TenantID: tenant, PageLimit: int32(limit + 1), AfterID: pgtype.UUID{Valid: true}, Ascending: ascending} - if cursor != "" { - after, err := s.GetAgent(ctx, tenantID, pgunit.LookupCursor(cursor)) - if err != nil { - return AgentPage{}, err - } - params.AfterCreated = pgtype.Timestamptz{Time: after.CreatedAt, Valid: true} - params.AfterID, _ = parseID(after.ID) - } - rows, err := s.queries.ListAgents(ctx, params) - if err != nil { - return AgentPage{}, fmt.Errorf("list agents: %w", err) - } - page := AgentPage{Agents: make([]SavedAgent, 0, min(limit, len(rows)))} - if len(rows) > limit { - page.NextCursor = uuid.UUID(rows[limit-1].ID.Bytes).String() - rows = rows[:limit] - } - for _, row := range rows { - agent, err := agentFromRow(row) - if err != nil { - return AgentPage{}, err - } - page.Agents = append(page.Agents, agent) - } - return page, nil -} diff --git a/services/core/internal/store/agents_list_test.go b/services/core/internal/store/agents_list_test.go deleted file mode 100644 index f6c6b058b..000000000 --- a/services/core/internal/store/agents_list_test.go +++ /dev/null @@ -1,96 +0,0 @@ -package store - -import ( - "context" - "errors" - "reflect" - "slices" - "testing" - "time" - - "github.com/google/uuid" -) - -func TestAgentListPaginationIsolationAndReconnect(t *testing.T) { - s, pool := testStore(t) - ctx := context.Background() - tenant, other := uuid.NewString(), uuid.NewString() - empty, err := s.ListAgents(ctx, tenant, "", 2, false) - if err != nil || empty.Agents == nil || len(empty.Agents) != 0 || empty.NextCursor != "" { - t.Fatalf("empty page: %+v, %v", empty, err) - } - input := CreateAgentInput{Configuration: []byte(`{"model":"unchanged","tools":[{"parameters":{"const":9007199254740993}}]}`), Metadata: map[string]string{"scope": "same-tenant"}} - ids := []string{} - for range 5 { - agent, err := s.CreateAgent(ctx, tenant, input) - if err != nil { - t.Fatal(err) - } - ids = append(ids, agent.ID) - } - foreign, err := s.CreateAgent(ctx, other, input) - if err != nil { - t.Fatal(err) - } - stamp := time.Unix(1700000000, 0).UTC() - if _, err := pool.Exec(ctx, "UPDATE agents SET created_at=$1, updated_at=$1 WHERE tenant_id=$2", stamp, tenant); err != nil { - t.Fatal(err) - } - slices.Sort(ids) - read := func(s *Store, ascending bool) []string { - t.Helper() - var actual []string - cursor := "" - for { - page, err := s.ListAgents(ctx, tenant, cursor, 2, ascending) - if err != nil { - t.Fatal(err) - } - if len(page.Agents) == 0 || len(page.Agents) > 2 { - t.Fatalf("bad page: %+v", page) - } - for _, agent := range page.Agents { - original, err := s.GetAgent(ctx, tenant, agent.ID) - if err != nil || !reflect.DeepEqual(agent, original) || agent.TenantID != tenant { - t.Fatalf("resource changed: %+v, %v", agent, err) - } - actual = append(actual, agent.ID) - } - if page.NextCursor == "" { - break - } - if page.NextCursor != page.Agents[len(page.Agents)-1].ID || len(actual) > len(ids) { - t.Fatal("invalid/repeating continuation") - } - cursor = page.NextCursor - } - return actual - } - if got := read(s, true); !slices.Equal(got, ids) { - t.Fatalf("ascending equal timestamps: %v", got) - } - reverse := slices.Clone(ids) - slices.Reverse(reverse) - if got := read(s, false); !slices.Equal(got, reverse) { - t.Fatalf("descending equal timestamps: %v", got) - } - // A malformed cursor follows the missing-cursor path (ERR-01). - for _, after := range []string{foreign.ID, uuid.NewString(), "not-an-id"} { - if _, err := s.ListAgents(ctx, tenant, after, 2, true); !errors.Is(err, ErrNotFound) { - t.Fatalf("unowned/unknown/malformed cursor accepted: %v", err) - } - } - tail, err := s.ListAgents(ctx, tenant, ids[len(ids)-1], 2, true) - if err != nil || tail.Agents == nil || len(tail.Agents) != 0 || tail.NextCursor != "" { - t.Fatalf("end page: %+v, %v", tail, err) - } - foreignPage, err := s.ListAgents(ctx, other, "", 100, false) - if err != nil || len(foreignPage.Agents) != 1 || foreignPage.Agents[0].ID != foreign.ID { - t.Fatalf("tenant isolation: %+v, %v", foreignPage, err) - } - pool.Close() - restored, _ := testStore(t) - if got := read(restored, true); !slices.Equal(got, ids) { - t.Fatalf("pagination changed after reconnect: %v", got) - } -} diff --git a/services/core/internal/store/agents_test.go b/services/core/internal/store/agents_test.go deleted file mode 100644 index ac21b1177..000000000 --- a/services/core/internal/store/agents_test.go +++ /dev/null @@ -1,108 +0,0 @@ -package store - -import ( - "context" - "errors" - "reflect" - "strings" - "testing" - "time" - - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/jsonobject" - "github.com/google/uuid" -) - -func TestSavedAgentsPersistIndependentlyAndStayTenantScoped(t *testing.T) { - s, pool := testStore(t) - ctx := context.Background() - tenantA, tenantB := uuid.NewString(), uuid.NewString() - session, err := s.CreateSession(ctx, tenantA, CreateSessionInput{Creator: FixtureCreator(), Engine: "codex", IdempotencyKey: "session"}) - if err != nil { - t.Fatal(err) - } - // Configuration is preserved without applying one harness's capabilities. - input := CreateAgentInput{ - Metadata: map[string]string{"purpose": "保存 configuration"}, - Configuration: []byte(`{"model":" caller-model ","name":null,"instructions":" keep whitespace ","multi_agent":{"enabled":true,"max_concurrent_subagents":6},"tools":[{"type":"function","name":"lookup","description":"","defer_loading":true,"parameters":{"type":"object","properties":{"number":{"const":9007199254740993}}}}]}`), - } - before := time.Now().Add(-time.Second) - first, err := s.CreateAgent(ctx, tenantA, input) - if err != nil { - t.Fatal(err) - } - expectedConfig, err := jsonobject.Normalize(input.Configuration) - if err != nil { - t.Fatal(err) - } - gotConfig, err := jsonobject.Normalize(first.Configuration) - if err != nil || string(gotConfig) != string(expectedConfig) { - t.Fatalf("configuration changed: %s, %v", first.Configuration, err) - } - if first.ID == session.ID || first.TenantID != tenantA || !reflect.DeepEqual(first.Metadata, input.Metadata) || - first.CreatedAt.Before(before) || first.CreatedAt.After(time.Now().Add(time.Second)) || !first.CreatedAt.Equal(first.UpdatedAt) { - t.Fatalf("unexpected saved agent: %+v", first) - } - // Identical configurations are distinct resources; storage invents no public - // create-idempotency contract or shared identity across callers. - for _, tenant := range []string{tenantA, tenantB} { - other, err := s.CreateAgent(ctx, tenant, input) - if err != nil || other.ID == first.ID || other.TenantID != tenant { - t.Fatalf("distinct create: %+v, %v", other, err) - } - } - for _, lookup := range []struct{ tenant, id string }{ - {tenantB, first.ID}, {tenantA, uuid.NewString()}, {tenantA, session.ID}, - } { - if _, err := s.GetAgent(ctx, lookup.tenant, lookup.id); !errors.Is(err, ErrNotFound) { - t.Fatalf("unowned/absent agent read: %v", err) - } - } - if _, err := s.GetSession(ctx, tenantA, first.ID); !errors.Is(err, ErrNotFound) { - t.Fatalf("saved agent became an execution session: %v", err) - } - unchanged, err := s.GetSession(ctx, tenantA, session.ID) - if err != nil || !reflect.DeepEqual(unchanged, session) { - t.Fatalf("saved Agent storage changed Session: %+v, %v", unchanged, err) - } - pool.Close() - recovered, _ := testStore(t) - got, err := recovered.GetAgent(ctx, tenantA, first.ID) - if err != nil || !reflect.DeepEqual(got, first) { - t.Fatalf("durable read: %+v, %v; want %+v", got, err, first) - } - emptyMetadata, err := recovered.CreateAgent(ctx, tenantA, CreateAgentInput{Configuration: []byte(`{"model":"another-model"}`)}) - if err != nil || emptyMetadata.Metadata == nil || len(emptyMetadata.Metadata) != 0 { - t.Fatalf("empty metadata: %+v, %v", emptyMetadata, err) - } -} - -func TestSavedAgentsRejectInvalidStoreInput(t *testing.T) { - s, _ := testStore(t) - ctx := context.Background() - tenant := uuid.NewString() - for _, raw := range []string{"", "null", "[]", "true", `{"model":"x"`, `{} {}`, `{"x":"` + strings.Repeat("x", 512*1024) + `"}`} { - if _, err := s.CreateAgent(ctx, tenant, CreateAgentInput{Configuration: []byte(raw)}); !errors.Is(err, ErrInvalidInput) { - t.Fatalf("invalid configuration length %d: %v", len(raw), err) - } - } - valid := CreateAgentInput{Configuration: []byte(`{"model":"x"}`)} - for _, invalid := range []string{"", "not-a-uuid", uuid.Nil.String()} { - if _, err := s.CreateAgent(ctx, invalid, valid); !errors.Is(err, ErrInvalidInput) { - t.Fatalf("invalid tenant accepted: %v", err) - } - if _, err := s.GetAgent(ctx, invalid, uuid.NewString()); !errors.Is(err, ErrInvalidInput) { - t.Fatalf("invalid read tenant accepted: %v", err) - } - if _, err := s.GetAgent(ctx, tenant, invalid); !errors.Is(err, ErrInvalidInput) { - t.Fatalf("invalid ID accepted: %v", err) - } - } - valid.Metadata = map[string]string{"large": strings.Repeat("x", 64*1024)} - if _, err := s.CreateAgent(ctx, tenant, valid); !errors.Is(err, ErrInvalidInput) { - t.Fatalf("oversized metadata accepted: %v", err) - } - var count int - if err := s.pool.QueryRow(ctx, "SELECT count(*) FROM agents WHERE tenant_id = $1", tenant).Scan(&count); err != nil || count != 0 { - t.Fatalf("rejected input wrote %d rows: %v", count, err) - } -} diff --git a/services/core/internal/store/agents_update.go b/services/core/internal/store/agents_update.go deleted file mode 100644 index da49eb654..000000000 --- a/services/core/internal/store/agents_update.go +++ /dev/null @@ -1,111 +0,0 @@ -package store - -import ( - "context" - "encoding/json" - "errors" - "fmt" - - v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/db/sqlc" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/jsonobject" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/metadata" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/auditpg" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" - "github.com/google/uuid" - "github.com/jackc/pgx/v5" -) - -// UpdateAgentInput contains validated field replacements, not a full snapshot. -// Core extension subfields merge independently. A nil metadata pointer preserves -// the existing map; a supplied map replaces it. ModelProviderSet distinguishes -// omission from replacement or an explicit nil provider, which clears the secret. -type UpdateAgentInput struct { - ModelProvider *v1.ModelProviderInput - ModelProviderSet bool - Configuration json.RawMessage - Metadata *map[string]string -} - -func (s *Store) UpdateAgent(ctx context.Context, tenantID, agentID string, input UpdateAgentInput) (SavedAgent, error) { - tenant, err := parseID(tenantID) - if err != nil { - return SavedAgent{}, err - } - id := pgunit.PathID(agentID) - raw := input.Configuration - if len(raw) == 0 { - raw = json.RawMessage(`{}`) - } - if len(raw) > 512*1024 { - return SavedAgent{}, ErrInvalidInput - } - raw, err = jsonobject.Normalize(raw) - if err != nil { - return SavedAgent{}, fmt.Errorf("%w: %w", ErrInvalidInput, err) - } - var patch map[string]json.RawMessage - if err := json.Unmarshal(raw, &patch); err != nil { - return SavedAgent{}, err - } - var encodedMetadata []byte - if input.Metadata != nil { - encodedMetadata, err = metadata.Encode(*input.Metadata) - if err != nil { - return SavedAgent{}, fmt.Errorf("%w: %w", ErrInvalidInput, err) - } - } - var updated SavedAgent - err = s.pooled.Transaction(ctx, func(ctx context.Context, tx pgx.Tx) error { - q := s.queries.WithTx(tx) - row, err := q.LockAgent(ctx, sqlc.LockAgentParams{TenantID: tenant, ID: id}) - if errors.Is(err, pgx.ErrNoRows) { - return ErrNotFound - } - if err != nil { - return err - } - var configuration map[string]json.RawMessage - if err := json.Unmarshal(row.Configuration, &configuration); err != nil { - return err - } - if err := mergeAgentConfiguration(configuration, patch); err != nil { - return err - } - merged, err := json.Marshal(configuration) - if err != nil { - return err - } - merged, err = jsonobject.Normalize(merged) - if err != nil { - return fmt.Errorf("%w: %w", ErrInvalidInput, err) - } - if len(merged) > 512*1024 { - return ErrInvalidInput - } - if err := validateAgentModelExecution(merged, input.ModelProvider); err != nil { - return err - } - if input.ModelProviderSet { - if err := s.saveAgentModelExecution(ctx, q, uuid.UUID(tenant.Bytes).String(), id, input.ModelProvider); err != nil { - return err - } - } - if input.Metadata == nil { - encodedMetadata = row.Metadata - } - row, err = q.UpdateAgent(ctx, sqlc.UpdateAgentParams{TenantID: tenant, ID: id, Configuration: merged, Metadata: encodedMetadata}) - if err != nil { - return err - } - updated, err = agentFromRow(row) - if err != nil { - return err - } - return auditpg.RecordWriteAudit(ctx, q, tenantID, "update", "agent", updated.ID, "") - }) - if err != nil { - return SavedAgent{}, fmt.Errorf("update agent: %w", err) - } - return updated, nil -} diff --git a/services/core/internal/store/agents_update_test.go b/services/core/internal/store/agents_update_test.go deleted file mode 100644 index f60c4b73a..000000000 --- a/services/core/internal/store/agents_update_test.go +++ /dev/null @@ -1,67 +0,0 @@ -package store - -import ( - "encoding/json" - "errors" - "reflect" - "strings" - "testing" - - "github.com/google/uuid" -) - -func TestAgentUpdateRollbackAndCompleteSizeBound(t *testing.T) { - s, _ := testStore(t) - ctx := t.Context() - tenant := uuid.NewString() - configuration, err := json.Marshal(map[string]any{"model": "original", "instructions": strings.Repeat("x", 400*1024), "number": json.Number("9007199254740993")}) - if err != nil { - t.Fatal(err) - } - original, err := s.CreateAgent(ctx, tenant, CreateAgentInput{Configuration: configuration, Metadata: map[string]string{"keep": "original"}}) - if err != nil { - t.Fatal(err) - } - metadata := map[string]string{"replace": "not-committed"} - oversized, err := json.Marshal(map[string]string{"name": strings.Repeat("y", 150*1024)}) - if err != nil { - t.Fatal(err) - } - for _, patch := range []json.RawMessage{oversized, []byte(`[]`), []byte(`{"name":"bad"} {}`)} { - _, err := s.UpdateAgent(ctx, tenant, original.ID, UpdateAgentInput{Configuration: patch, Metadata: &metadata}) - if !errors.Is(err, ErrInvalidInput) { - t.Fatalf("invalid update: %v", err) - } - unchanged, err := s.GetAgent(ctx, tenant, original.ID) - if err != nil || !reflect.DeepEqual(unchanged, original) { - t.Fatalf("partial failed update: %v", err) - } - } - _, err = s.UpdateAgent(ctx, uuid.NewString(), original.ID, UpdateAgentInput{Configuration: []byte(`{"model":"foreign"}`), Metadata: &metadata}) - if !errors.Is(err, ErrNotFound) { - t.Fatalf("foreign update: %v", err) - } - // A failure releases the lock; a later valid patch preserves unrelated large values and numbers. - updated, err := s.UpdateAgent(ctx, tenant, original.ID, UpdateAgentInput{Configuration: []byte(`{"model":"updated"}`)}) - if err != nil { - t.Fatal(err) - } - var fields map[string]json.RawMessage - if err := json.Unmarshal(updated.Configuration, &fields); err != nil { - t.Fatal(err) - } - if string(fields["number"]) != "9007199254740993" || string(fields["model"]) != `"updated"` || !reflect.DeepEqual(updated.Metadata, original.Metadata) { - t.Fatal("unrelated configuration or metadata lost") - } - if !updated.CreatedAt.Equal(original.CreatedAt) || updated.UpdatedAt.Before(original.UpdatedAt) { - t.Fatal("resource timestamps changed incorrectly") - } - unchanged, err := s.UpdateAgent(ctx, tenant, original.ID, UpdateAgentInput{}) - if err != nil || !unchanged.UpdatedAt.After(updated.UpdatedAt) { - t.Fatalf("empty update did not advance timestamp: %v", err) - } - updated.UpdatedAt = unchanged.UpdatedAt - if !reflect.DeepEqual(unchanged, updated) { - t.Fatal("empty update changed saved configuration") - } -} diff --git a/services/core/internal/store/configuration_validation_public_test.go b/services/core/internal/store/configuration_validation_public_test.go index c7e242adb..ed523ef5b 100644 --- a/services/core/internal/store/configuration_validation_public_test.go +++ b/services/core/internal/store/configuration_validation_public_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/credentialcrypto" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtimedevice" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" @@ -43,7 +44,8 @@ func TestAgentConfigurationValidationRejectsWithoutWritesPostgres(t *testing.T) saved := func(tools string) string { return `{"model":"config-model","name":null,"instructions":null,"multi_agent":{"enabled":false,"max_concurrent_subagents":null},"reasoning":{},"service_tier":"auto","text":{"format":{"type":"text"},"verbosity":"medium"},"tools":` + tools + `}` } - legacy, err := s.CreateAgent(t.Context(), ownerTenant, store.CreateAgentInput{Metadata: map[string]string{}, Configuration: json.RawMessage(saved(`[{"type":"function","name":"lookup","description":"","parameters":{"type":"string"},"defer_loading":false}]`))}) + _, agentService := fixtureAgents(t, db) + legacy, err := agentService.Create(t.Context(), agents.CreateCommand{TenantID: ownerTenant, Metadata: map[string]string{}, Configuration: json.RawMessage(saved(`[{"type":"function","name":"lookup","description":"","parameters":{"type":"string"},"defer_loading":false}]`))}) if err != nil { t.Fatal(err) } diff --git a/services/core/internal/store/project_api_keys_test.go b/services/core/internal/store/project_api_keys_test.go index 67e059d4e..f82c5519c 100644 --- a/services/core/internal/store/project_api_keys_test.go +++ b/services/core/internal/store/project_api_keys_test.go @@ -10,6 +10,7 @@ import ( "testing" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/adminaudit" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/identity" "github.com/google/uuid" ) @@ -34,7 +35,7 @@ func createTestProject(t *testing.T, s *Store) Project { return p } func TestProjectKeysShareIdentityAndArchiveRetainsAssets(t *testing.T) { - s, _ := testStore(t) + s, pool := testStore(t) p := createTestProject(t, s) other := createTestProject(t, s) first, err := s.CreateProjectAPIKey(keyAdminContext(t.Context(), p.ID), p.ID, uuid.NewString(), "first") @@ -53,14 +54,15 @@ func TestProjectKeysShareIdentityAndArchiveRetainsAssets(t *testing.T) { if err != nil || a.Principal != b.Principal || a.Principal.SubjectID != "project:"+p.ID || a.Principal.ProjectID != "proj_"+p.ID { t.Fatal("Project keys do not share the stable Project principal", err) } - asset, err := s.CreateAgent(t.Context(), a.Principal.TenantID, CreateAgentInput{Configuration: []byte(`{"model":"test"}`)}) + agentStore, agentService := testAgents(t, pool, nil) + asset, err := agentService.Create(t.Context(), agents.CreateCommand{TenantID: a.Principal.TenantID, Configuration: []byte(`{"model":"test"}`)}) if err != nil { t.Fatal(err) } - if _, err := s.GetAgent(t.Context(), b.Principal.TenantID, asset.ID); err != nil { + if _, err := agentStore.GetAgent(t.Context(), b.Principal.TenantID, asset.ID); err != nil { t.Fatal("peer key cannot read shared asset", err) } - if _, err := s.GetAgent(t.Context(), other.TenantID, asset.ID); !errors.Is(err, ErrNotFound) { + if _, err := agentStore.GetAgent(t.Context(), other.TenantID, asset.ID); !errors.Is(err, agents.ErrNotFound) { t.Fatal("foreign Project read asset", err) } renamed, err := s.RenameProject(keyAdminContext(t.Context(), p.ID), p.ID, "renamed") @@ -101,7 +103,7 @@ func TestProjectKeysShareIdentityAndArchiveRetainsAssets(t *testing.T) { if _, err := s.CreateProjectAPIKey(keyAdminContext(t.Context(), p.ID), p.ID, uuid.NewString(), "late"); !errors.Is(err, ErrProjectArchived) { t.Fatal("archived Project admitted new key", err) } - if _, err := s.GetAgent(t.Context(), p.TenantID, asset.ID); err != nil { + if _, err := agentStore.GetAgent(t.Context(), p.TenantID, asset.ID); err != nil { t.Fatal("archive removed assets", err) } if err := s.ValidateProjectKeySeparation(t.Context(), []string{projectKeyDigest(second.Key)}); err == nil { diff --git a/services/core/internal/store/public_handler_fixture_test.go b/services/core/internal/store/public_handler_fixture_test.go index 4d96803ab..8fcf324ee 100644 --- a/services/core/internal/store/public_handler_fixture_test.go +++ b/services/core/internal/store/public_handler_fixture_test.go @@ -7,9 +7,11 @@ import ( "testing" "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/api" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/coremetrics" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/execution" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/agentpg" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/auditpg" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtimedevice" @@ -37,10 +39,11 @@ func publicHandler(t testing.TB, s *store.Store, db fixtureDB, keys fixtureKeyRe } strict := strictStandIn{t} audit := auditpg.New(pgunit.NewPool(db.pool)) + agentStore, agentService := fixtureAgents(t, db) deps := api.Dependencies{ Engine: engine, CoreKeys: admin, InstallationBindings: s, Projects: fixtureProjects{Store: s, keys: keys}, Vaults: s, ModelProviders: s, Files: s, Skills: s, - EnvironmentTemplates: s, Agents: s, Sessions: s, SessionEvents: s, SessionHistory: s, Subagents: s, + EnvironmentTemplates: s, Agents: agentService, AgentsReader: agentStore, Sessions: s, SessionEvents: s, SessionHistory: s, Subagents: s, Artifacts: s, SessionAdmin: s, Environments: s, Admin: s, AdminAudit: audit, WriteAudit: audit, ExecutorConnections: strict, Metrics: strict, RuntimeObservations: strict, RuntimeHistory: strict, } @@ -50,6 +53,17 @@ func publicHandler(t testing.TB, s *store.Store, db fixtureDB, keys fixtureKeyRe return api.NewHandler(deps) } +// fixtureAgents builds the Agent adapter and service on db. +func fixtureAgents(t testing.TB, db fixtureDB) (*agentpg.Store, *agents.Service) { + t.Helper() + agentStore := agentpg.New(pgunit.NewPool(db.pool), db.cipher) + agentService, err := agents.NewService(agentStore) + if err != nil { + t.Fatal(err) + } + return agentStore, agentService +} + // fixtureProjects serves Projects from the Store and resolves Project keys from // the test's fixture keys. type fixtureProjects struct { diff --git a/services/core/internal/store/saved_web_search_public_test.go b/services/core/internal/store/saved_web_search_public_test.go index e560ec781..437248c35 100644 --- a/services/core/internal/store/saved_web_search_public_test.go +++ b/services/core/internal/store/saved_web_search_public_test.go @@ -8,8 +8,8 @@ import ( "strings" "testing" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/agents" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtimedevice" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" "github.com/google/uuid" ) @@ -75,7 +75,7 @@ func TestSavedWebSearchPostgres(t *testing.T) { live := `{"type":"web_search","mode":"live",` + defaults // W1: Agent create with every pinned mode (official create 201). - agents := map[string]string{} + agentIDs := map[string]string{} for _, tc := range []struct{ name, tool, official string }{ {"type-only", `{"type":"web_search"}`, live}, {"mode-null", `{"type":"web_search","mode":null}`, live}, @@ -96,7 +96,7 @@ func TestSavedWebSearchPostgres(t *testing.T) { var created struct{ ID string } _ = json.Unmarshal([]byte(body), &created) readBack(tc.name, created.ID, body) - agents[tc.name] = created.ID + agentIDs[tc.name] = created.ID } // W2: update replaces tools with each observed form (official update 200). @@ -115,7 +115,8 @@ func TestSavedWebSearchPostgres(t *testing.T) { // A disabled record saved before this batch reads unchanged and is admitted with // the same frozen Session tool (W7). - legacy, err := s.CreateAgent(t.Context(), ownerTenant, store.CreateAgentInput{Metadata: map[string]string{}, Configuration: json.RawMessage( + _, agentService := fixtureAgents(t, db) + legacy, err := agentService.Create(t.Context(), agents.CreateCommand{TenantID: ownerTenant, Metadata: map[string]string{}, Configuration: json.RawMessage( `{"model":"search-model","name":null,"instructions":null,"multi_agent":{"enabled":false,"max_concurrent_subagents":null},"reasoning":{},"service_tier":"auto","text":{"format":{"type":"text"},"verbosity":"medium"},"tools":[{"type":"web_search","mode":"disabled","context_size":"medium","allowed_domains":[],"location":null}]}`)}) if err != nil { t.Fatal(err) @@ -146,7 +147,7 @@ func TestSavedWebSearchPostgres(t *testing.T) { // A same-key retry of a Session that recorded its creation request recovers it // after the Agent enables search. retry := func(key string) (int, string) { - request, err := http.NewRequest(http.MethodPost, server.URL+"/v1/agents/sessions", strings.NewReader(`{"agent_id":"`+agents["mode-disabled"]+`","environment":{"type":"none"},"input":"hi"}`)) + request, err := http.NewRequest(http.MethodPost, server.URL+"/v1/agents/sessions", strings.NewReader(`{"agent_id":"`+agentIDs["mode-disabled"]+`","environment":{"type":"none"},"input":"hi"}`)) if err != nil { t.Fatal(err) } @@ -167,8 +168,8 @@ func TestSavedWebSearchPostgres(t *testing.T) { if status != http.StatusCreated || original == "" { t.Fatalf("retry fixture: %d", status) } - client.created(owner, "/v1/agents/"+agents["mode-disabled"], `{"tools":[{"type":"web_search","mode":"live"}]}`) - enabledTools := map[string]string{"mode-live": agents["mode-live"], "mode-cached": agents["mode-cached"], "type-only": agents["type-only"], "updated-to-live": agents["mode-disabled"]} + client.created(owner, "/v1/agents/"+agentIDs["mode-disabled"], `{"tools":[{"type":"web_search","mode":"live"}]}`) + enabledTools := map[string]string{"mode-live": agentIDs["mode-live"], "mode-cached": agentIDs["mode-cached"], "type-only": agentIDs["type-only"], "updated-to-live": agentIDs["mode-disabled"]} // W4: every creation mode rejects enabled saved search without writes. before := databaseDigest(t, db.pool) @@ -214,7 +215,7 @@ func TestSavedWebSearchPostgres(t *testing.T) { send := func(token, id string) (int, string) { return client.do(token, request.method, strings.ReplaceAll(request.path, "%s", id), "application/json", []byte(strings.ReplaceAll(request.body, "%s", id))) } - foreignStatus, foreignBody := send(foreign, agents["mode-live"]) + foreignStatus, foreignBody := send(foreign, agentIDs["mode-live"]) missingStatus, missingBody := send(owner, missing) if foreignStatus != http.StatusNotFound || foreignStatus != missingStatus || foreignBody != missingBody { t.Errorf("%s %s: foreign %d %s, missing %d %s", request.method, request.path, foreignStatus, foreignBody, missingStatus, missingBody) @@ -241,7 +242,7 @@ func TestSavedWebSearchPostgres(t *testing.T) { // W5: a per-Session tools replacement admits the saved Agent without its search. for _, replacement := range []string{`[]`, `[{"type":"web_search","mode":"disabled"}]`} { - session := client.created(owner, "/v1/agents/sessions", `{"agent_id":"`+agents["mode-live"]+`","agent":{"tools":`+replacement+`},"environment":{"type":"none"},"input":"hi"}`) + session := client.created(owner, "/v1/agents/sessions", `{"agent_id":"`+agentIDs["mode-live"]+`","agent":{"tools":`+replacement+`},"environment":{"type":"none"},"input":"hi"}`) if got := sessionTool("replacement", session); got != "" && got != disabled { t.Fatalf("replacement %s: Session tool %s", replacement, got) } diff --git a/services/core/internal/store/write_audit_resources_test.go b/services/core/internal/store/write_audit_resources_test.go index c9d543b5c..05f7adbda 100644 --- a/services/core/internal/store/write_audit_resources_test.go +++ b/services/core/internal/store/write_audit_resources_test.go @@ -3,8 +3,6 @@ package store import ( "bytes" "context" - "encoding/json" - "errors" "reflect" "strings" "testing" @@ -48,7 +46,7 @@ func TestWriteAuditStandaloneResourceTransactions(t *testing.T) { } archive := skillArchive(t, "audit-private-archive") for _, name := range []string{ - "agent_create", "agent_update", "agent_delete", "template_create", "template_update", "template_delete", + "template_create", "template_update", "template_delete", "skill_create", "skill_upload_version", "skill_update_default", "skill_delete", "version_delete", "version_delete_last", "file_create", "file_delete", "vault_create", "vault_delete", "credential_create", "credential_update", "credential_delete", "oauth_create", "oauth_update", "oauth_delete", @@ -120,21 +118,6 @@ func prepareResourceAuditMutation(t *testing.T, s *Store, tenant, name string, a t.Fatal(err) } } - if strings.HasPrefix(name, "agent_") { - input := CreateAgentInput{Configuration: json.RawMessage(`{"model":"fixture"}`)} - if name == "agent_create" { - return resourceAuditMutation{action: "create", kind: "agent", owners: 1, run: func(ctx context.Context) (string, error) { v, e := s.CreateAgent(ctx, tenant, input); return v.ID, e }} - } - a, err := s.CreateAgent(ctx, tenant, input) - must(err) - if name == "agent_update" { - return resourceAuditMutation{action: "update", kind: "agent", run: func(ctx context.Context) (string, error) { - v, e := s.UpdateAgent(ctx, tenant, a.ID, UpdateAgentInput{Configuration: json.RawMessage(`{"model":"replacement"}`)}) - return v.ID, e - }} - } - return resourceAuditMutation{action: "delete", kind: "agent", run: func(ctx context.Context) (string, error) { return s.DeleteAgent(ctx, tenant, a.ID) }} - } if strings.HasPrefix(name, "template_") { if name == "template_create" { return resourceAuditMutation{action: "create", kind: "environment_template", owners: 1, run: func(ctx context.Context) (string, error) { @@ -246,42 +229,6 @@ func prepareResourceAuditMutation(t *testing.T, s *Store, tenant, name string, a return resourceAuditMutation{action: "delete", kind: "credential", parent: vault.ID, run: func(ctx context.Context) (string, error) { return s.DeleteCredential(ctx, tenant, vault.ID, v.ID) }} } -func TestWriteAuditResourceReadsFailuresAndStableOwnership(t *testing.T) { - s, pool := testStore(t) - tenant := uuid.NewString() - first, err := s.CreateAgent(resourceAuditContext(t.Context(), tenant, uuid.NewString()), tenant, CreateAgentInput{Configuration: json.RawMessage(`{"model":"fixture"}`)}) - if err != nil { - t.Fatal(err) - } - var before string - if err := pool.QueryRow(t.Context(), `SELECT to_jsonb(o)::text FROM write_audit_owners o WHERE tenant_id=$1 AND resource_id=$2`, tenant, first.ID).Scan(&before); err != nil { - t.Fatal(err) - } - readCtx := resourceAuditContext(t.Context(), tenant, uuid.NewString()) - if _, err := s.GetAgent(readCtx, tenant, first.ID); err != nil { - t.Fatal(err) - } - if _, err := s.UpdateAgent(readCtx, tenant, uuid.NewString(), UpdateAgentInput{}); !errors.Is(err, ErrNotFound) { - t.Fatal(err) - } - other, _ := writeaudit.FromContext(resourceAuditContext(t.Context(), tenant, uuid.NewString())) - other.KeyID, other.Prefix = "static:"+strings.Repeat("b", 64), "bbbbbbbb" - if _, err := s.UpdateAgent(writeaudit.WithSource(t.Context(), other), tenant, first.ID, UpdateAgentInput{}); err != nil { - t.Fatal(err) - } - if _, err := s.DeleteAgent(resourceAuditContext(t.Context(), tenant, uuid.NewString()), tenant, first.ID); err != nil { - t.Fatal(err) - } - var after string - var count int - if err := pool.QueryRow(t.Context(), `SELECT to_jsonb(o)::text FROM write_audit_owners o WHERE tenant_id=$1 AND resource_id=$2`, tenant, first.ID).Scan(&after); err != nil || after != before { - t.Fatal("creator changed or disappeared", err) - } - if err := pool.QueryRow(t.Context(), `SELECT count(*) FROM write_audit_operations WHERE tenant_id=$1`, tenant).Scan(&count); err != nil || count != 3 { - t.Fatal("read or failure audit", count, err) - } -} - // A public mutation owns its transaction, so inject database failures through // the connection configuration rather than replacing a Store query wrapper. func readOnlyResourceStore(t *testing.T, pool *pgxpool.Pool) *Store {