From d3e2ac6d075a3680c8ca0693a865935475a20876 Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Wed, 30 Sep 2026 20:47:43 +0000 Subject: [PATCH] Move Item, Subagent and Artifact reads and Artifact staging into sessions and sessionpg The pooled sessions.Service and sessions.Reader skeleton lands with the Artifact family: StageTurnArtifacts and DeleteSessionArtifact are Service use cases on sessionpg.Store, and the Item, Subagent and Artifact reads are sessionpg.Store methods behind sessions.Reader. Execution stages a Turn's export through the required Dispatcher.Sessions; StartWorker also requires Dispatcher.SessionsReader. cmd/server builds one sessionpg.Store for the Dispatcher and the api Items, Subagents, Artifacts and ArtifactsReader fields. Staging keeps its single pooled transaction: the Environment is authorized before the export is read, content streams into large objects that roll back with a rejected or failed export, and the Session lock is taken through sessionpg.LockSession only after the transfer to recheck the Turn. LoadPublicSubagent and LoadChildTurn move to sessionpg/subagents.go with their store signatures. The store read, capture and cursor files are deleted, and the capture rejection test moves to sessionpg. --- services/core/IMPLEMENTATION.md | 5 +- services/core/cmd/server/http_routes_test.go | 1 + services/core/cmd/server/main.go | 16 +- services/core/internal/api/dependencies.go | 5 +- .../core/internal/api/dependencies_test.go | 14 +- services/core/internal/api/fakes_test.go | 32 ++- services/core/internal/api/items.go | 2 +- .../native_classification_integration_test.go | 2 +- .../core/internal/api/resource_query_test.go | 6 +- .../core/internal/api/session_artifacts.go | 24 +- .../internal/api/session_artifacts_test.go | 11 +- .../session_diagnostics_public_compat_test.go | 14 +- .../internal/api/session_diagnostics_test.go | 2 +- services/core/internal/api/subagent_turns.go | 6 +- services/core/internal/api/subagents.go | 6 +- services/core/internal/execution/artifacts.go | 2 +- .../core/internal/execution/dispatcher.go | 5 + .../core/internal/execution/owner_test.go | 42 ++- services/core/internal/execution/worker.go | 6 + .../postgres/sessionpg/artifacts.go | 255 +++++++++++++++++ .../postgres/sessionpg/artifacts_test.go | 264 ++++++++++++++++++ .../persistence/postgres/sessionpg/doc.go | 4 +- .../postgres/sessionpg}/item_reads.go | 9 +- .../postgres/sessionpg/public_session.go | 28 ++ .../persistence/postgres/sessionpg/store.go | 18 ++ .../postgres/sessionpg/subagent_reads.go | 230 +++++++++++++++ .../postgres/sessionpg/subagents.go | 49 ++++ .../providers/configuration_flow_test.go | 7 +- services/core/internal/sessions/artifacts.go | 213 ++++++++++++++ .../core/internal/sessions/artifacts_test.go | 256 +++++++++++++++++ services/core/internal/sessions/doc.go | 3 +- services/core/internal/sessions/items.go | 22 ++ services/core/internal/sessions/reader.go | 9 + services/core/internal/sessions/reads.go | 26 +- services/core/internal/sessions/service.go | 20 ++ services/core/internal/sessions/subagents.go | 32 +++ .../internal/store/admin_delete_audit_test.go | 10 +- .../store/admin_session_archive_test.go | 6 +- .../store/archive_cancellation_test.go | 6 +- .../core/internal/store/artifact_capture.go | 125 --------- .../internal/store/command_output_test.go | 6 +- services/core/internal/store/dispatch_test.go | 6 +- .../store/environment_write_audit_test.go | 8 +- .../core/internal/store/fixture_db_test.go | 11 +- .../store/function_execution_native_test.go | 2 +- .../store/function_item_events_test.go | 4 +- .../core/internal/store/item_order_test.go | 6 +- .../core/internal/store/item_reads_test.go | 18 +- .../internal/store/item_serialization_test.go | 4 +- .../internal/store/list_cursor_public_test.go | 23 +- services/core/internal/store/list_cursors.go | 16 -- .../store/local_artifact_export_test.go | 2 +- .../store/local_environment_worker_test.go | 2 +- .../internal/store/native_environment_test.go | 2 +- .../internal/store/prepared_dispatch_test.go | 2 +- .../internal/store/public_execution_test.go | 2 +- .../store/public_handler_fixture_test.go | 20 +- .../internal/store/runtime_idle_clock_test.go | 2 +- .../store/sandbox_deployment_switch_test.go | 4 +- .../store/self_hosted_cancel_public_test.go | 4 +- .../core/internal/store/session_artifacts.go | 146 ---------- .../store/session_artifacts_public_test.go | 30 +- .../internal/store/session_artifacts_test.go | 96 ++----- .../internal/store/session_deletion_test.go | 2 +- .../internal/store/session_events_test.go | 4 +- .../store/session_initial_input_test.go | 2 +- .../internal/store/sessions_fixture_test.go | 20 ++ services/core/internal/store/sessions_test.go | 16 ++ .../store/subagent_item_projection.go | 3 +- .../internal/store/subagent_item_reads.go | 76 ----- .../core/internal/store/subagent_lifecycle.go | 2 +- .../store/subagent_native_outputs_test.go | 6 +- .../core/internal/store/subagent_reads.go | 182 ------------ .../internal/store/subagent_resources_test.go | 28 +- services/core/internal/store/turn_reads.go | 2 +- 75 files changed, 1749 insertions(+), 803 deletions(-) create mode 100644 services/core/internal/persistence/postgres/sessionpg/artifacts.go create mode 100644 services/core/internal/persistence/postgres/sessionpg/artifacts_test.go rename services/core/internal/{store => persistence/postgres/sessionpg}/item_reads.go (96%) create mode 100644 services/core/internal/persistence/postgres/sessionpg/public_session.go create mode 100644 services/core/internal/persistence/postgres/sessionpg/store.go create mode 100644 services/core/internal/persistence/postgres/sessionpg/subagent_reads.go create mode 100644 services/core/internal/persistence/postgres/sessionpg/subagents.go create mode 100644 services/core/internal/sessions/artifacts.go create mode 100644 services/core/internal/sessions/artifacts_test.go create mode 100644 services/core/internal/sessions/items.go create mode 100644 services/core/internal/sessions/reader.go create mode 100644 services/core/internal/sessions/service.go create mode 100644 services/core/internal/sessions/subagents.go delete mode 100644 services/core/internal/store/artifact_capture.go delete mode 100644 services/core/internal/store/list_cursors.go delete mode 100644 services/core/internal/store/session_artifacts.go create mode 100644 services/core/internal/store/sessions_fixture_test.go delete mode 100644 services/core/internal/store/subagent_item_reads.go delete mode 100644 services/core/internal/store/subagent_reads.go diff --git a/services/core/IMPLEMENTATION.md b/services/core/IMPLEMENTATION.md index 54b1fd32d..8772f79c1 100644 --- a/services/core/IMPLEMENTATION.md +++ b/services/core/IMPLEMENTATION.md @@ -34,6 +34,7 @@ Domain owners, each with its PostgreSQL adapter under `internal/persistence/post - `modelconfiguration` (`modelconfigurationpg`): each Harness's deployment default model configuration and its last-use observations. - `skills` (`skillpg`): Skills and their immutable versions: archive checks, the default and latest pointers, version selection and deletion, and each version's sealed archive. Session creation freezes selected versions inside its `store` transaction with the `skills` rules. - `deployment` (`deploymentpg`): the sandbox deployment and its nodes: provider configuration and the sealed credential, the specification and retained generations, setup, update and switch, node enrollment, identity and authentication, generation configuration, capacity, presence, status and host history. It interprets Sandbox Provider declarations through the `providers.Registry` it is given, whose lookups return typed errors. `cmd/server` builds that registry and calls it only to build a direct Provider and to discover a Provider's configuration; it takes each setup's mode and declared operations from the `Setup` that `deployment` returns. The only other reader is `store`, whose own `providers.Builtin()` validates the stored specification at Session admission and reads the public-origin requirement at placement. Deployment changes run through `deployment.ExecutionOperations` on `deploymentpg.NewExecution(lease, …)`, which the Worker receives as `execution.Owner.Deployment`; the Worker reads the deployment and prepares a selection's setup through the pooled `deployment.Service` in `execution.Dispatcher.Deployment`, and node management and reads use the pooled `deploymentpg.Store`, which `cmd/server` also reads the owner epoch from and runs the host-history sampler on. Provider calls run outside transactions, and the final transaction rechecks the expected generation. Allocations, placement and reset stay in `store`. +- `sessions` (`sessionpg`): Session use cases and reads as they leave `store`. The pooled `sessions.Service` runs the use cases on `sessionpg.Store`, which implements `sessions.Storage`, and plain reads use `sessions.Reader`, which `sessionpg.Store` also implements, directly. So far these cover root Items and Subagents (their reads) and Session Artifacts (their reads, deletion and the staging of a Turn's export). `cmd/server` builds one `sessionpg.Store` and wires the Service and the Store into `execution.Dispatcher.Sessions` and `SessionsReader` and into the api fields; the Worker stages Artifacts through the Service. ## Request handling @@ -49,7 +50,7 @@ On the Beta group, the OpenAI-Beta check (exactly one `agents=v1` value) runs be - Stored strings other than metadata rely on PostgreSQL rejecting U+0000 and invalid UTF-8: `pgunit.IsUnstorableText` detects SQLSTATE `22021` (text parameter) and `22P05` (`\u0000` in jsonb), and the adapter returns `textvalue.ErrUnstorable`, the 400 unstorable-text error, including for query filters such as `agent_id`. The failing statement aborts its transaction, so keep each request's writes in one transaction. - An `after` cursor that cannot name a resource on a lookup list (Agents, Sessions, Turns, Templates, Vaults, Credentials) resolves through `pgunit.LookupCursor` to the never-assigned maximum UUID and runs the normal lookup, so storage failures and missing rows behave as for a well-formed cursor. Resolve every cursor only inside its already resolved parent and tenant. -- Lists whose parent and cursor lookups are separate statements (Artifacts, and Skill versions in `skills`) re-check the parent before reporting a cursor 400, so a parent deleted in between still returns its 404. Item and Subagent lists read both inside one locked Session transaction. `skillpg` looks up a Skill version cursor tenant-wide so `skills` can tell another Skill's version from a missing one; another tenant's version stays missing. +- Lists whose parent and cursor lookups are separate statements (Artifacts in `sessionpg`, and Skill versions in `skills`) re-check the parent before reporting a cursor 400, so a parent deleted in between still returns its 404. `sessionpg` reads Item and Subagent lists and their cursors inside one locked Session transaction. `skillpg` looks up a Skill version cursor tenant-wide so `skills` can tell another Skill's version from a missing one; another tenant's version stays missing. ## Source Files and Artifacts @@ -57,7 +58,7 @@ Source Files are Project resources with a lifecycle independent of copied worksp Session Artifacts are immutable published copies, separate from live workspace files and source Files. The private output exporter reuses the authorized workspace path boundary and streams bounded bytes; publication requires complete capture and confirmed helper and transport success, not merely valid archive syntax. The daemon owns and drains the exporter's stdout pipe separately from child reaping, so pull-transport backpressure cannot consume the process-exit I/O deadline; after helper exit, each pipe read has one second, reset after consumer delays, which rejects inherited pipes that never close. Cancellation closes the owned reader and the dispatch consumer, then waits for the child. Never extract an output archive into Core's filesystem or hold the execution lease through a large transfer. -Capture bytes into private large objects without a Session admission lock. Before capture, seal native input under that lock with the private capture marker; later messages reuse the Environment input reservation and wait for the next Turn, and the public Turn stays in progress until publication settles. Directory reads during capture use an independent read-only preparation, not the released native Run. After a confirmed export, lock and recheck the live Turn, then publish the metadata in the same transaction as the Turn's completion. `sessions.EndTurn` decides Artifact publication and `sessionpg` applies it in the Turn's terminal transaction, not the capture transaction: capture commits and releases the Session lock before the Turn completes, and the terminal transaction holds the Session lock that also orders Artifact deletion. Drop staged rows whose SHA-256 equals the newest remaining published Artifact for the path, ordered by the producing Turn's creation time and then ID (publication time can come from the Runtime and does not order Turns), and unlink their large objects in the same transaction. Published rows are never modified. Failed and cancelled Turns discard private objects, and Session deletion removes private and published copies. The exporter skips output symlinks by their `lstat` type without following them; hard links, other special files, device crossings and concurrent changes reject the capture. Stored reads are authorized independently of Environment availability, so published Artifacts outlive the Environment. +Capture bytes into private large objects without a Session admission lock. Before capture, seal native input under that lock with the private capture marker; later messages reuse the Environment input reservation and wait for the next Turn, and the public Turn stays in progress until publication settles. Directory reads during capture use an independent read-only preparation, not the released native Run. `sessions.StageTurnArtifacts` stages a confirmed export as a pooled use case, never on the execution lease: it authorizes the Environment before reading the export, admits only distinct regular files under `outputs/` within the capture limits followed by nothing but zero padding, and after the transfer takes the Session lock through `sessionpg.LockSession` and rechecks that the Turn is in progress without a cancellation request. `sessionpg` streams each file into a large object in that one transaction, so a rejected or failed export rolls back its content. The metadata is published in the same transaction as the Turn's completion. `sessions.EndTurn` decides Artifact publication and `sessionpg` applies it in the Turn's terminal transaction, not the capture transaction: capture commits and releases the Session lock before the Turn completes, and the terminal transaction holds the Session lock that also orders Artifact deletion. Drop staged rows whose SHA-256 equals the newest remaining published Artifact for the path, ordered by the producing Turn's creation time and then ID (publication time can come from the Runtime and does not order Turns), and unlink their large objects in the same transaction. Published rows are never modified. Failed and cancelled Turns discard private objects, and Session deletion removes private and published copies. The exporter skips output symlinks by their `lstat` type without following them; hard links, other special files, device crossings and concurrent changes reject the capture. `sessionpg` serves stored reads authorized independently of Environment availability, so published Artifacts outlive the Environment, and reads content from one snapshot, so a read admitted before a deletion finishes. Deletion takes the Session lock, unlinks the content and records the write audit in one transaction. - A malformed `environment_id` Artifact filter resolves to the never-assigned maximum UUID, so it matches nothing without a text comparison. diff --git a/services/core/cmd/server/http_routes_test.go b/services/core/cmd/server/http_routes_test.go index c3c40e46a..4c9b2503e 100644 --- a/services/core/cmd/server/http_routes_test.go +++ b/services/core/cmd/server/http_routes_test.go @@ -102,6 +102,7 @@ func daemonComposition(t testing.TB) http.Handler { Items: struct{ api.Items }{}, Subagents: struct{ api.Subagents }{}, Artifacts: struct{ api.Artifacts }{}, + ArtifactsReader: struct{ api.ArtifactsReader }{}, 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 }{}, RuntimeObservations: struct{ api.RuntimeObservations }{}, RuntimeHistory: struct{ api.RuntimeHistory }{}, diff --git a/services/core/cmd/server/main.go b/services/core/cmd/server/main.go index 7964810a1..3466196eb 100644 --- a/services/core/cmd/server/main.go +++ b/services/core/cmd/server/main.go @@ -48,6 +48,7 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/modelconfigurationpg" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/projectpg" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/sessionpg" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/skillpg" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/templatepg" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/vaultpg" @@ -60,6 +61,7 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtimeobs" observationstoreresolver "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtimeobs/storeresolver" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sandbox/providers" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/skills" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/vaults" @@ -162,6 +164,11 @@ func run() error { if err != nil { return err } + sessionStore := sessionpg.New(units) + sessionService, err := sessions.NewService(sessionStore) + if err != nil { + return err + } installation, err := installationFacts(public) if err != nil { return err @@ -283,6 +290,8 @@ func run() error { if registry != nil { dispatcher := &execution.Dispatcher{Store: executionStore, Registry: registry, Credentials: vaultService, Observer: modelConfigurationStore, Deployment: deploymentService, + Sessions: sessionService, + SessionsReader: sessionStore, ManagedRuntimes: managed, MaxConcurrentExecutions: concurrency} lease, err := pgunit.AcquireLease(ctx, pool) if err != nil { @@ -373,9 +382,10 @@ func run() error { SessionCreation: executionStore, SessionEvents: executionStore, Turns: executionStore, - Items: executionStore, - Subagents: executionStore, - Artifacts: executionStore, + Items: sessionStore, + Subagents: sessionStore, + Artifacts: sessionService, + ArtifactsReader: sessionStore, SessionAdmin: executionStore, Environments: executionStore, ExecutorConnections: executorConnections{store: executionStore, registry: registry}, Admin: executionStore, AdminAudit: auditStore, WriteAudit: auditStore, Metrics: metrics, diff --git a/services/core/internal/api/dependencies.go b/services/core/internal/api/dependencies.go index cfc368596..f5250ee05 100644 --- a/services/core/internal/api/dependencies.go +++ b/services/core/internal/api/dependencies.go @@ -49,6 +49,7 @@ type Dependencies struct { Items Items Subagents Subagents Artifacts Artifacts + ArtifactsReader ArtifactsReader SessionAdmin SessionAdmin Environments Environments ExecutorConnections ExecutorConnections @@ -136,7 +137,9 @@ func (d Dependencies) validate() error { field{"Turns", d.Turns}, field{"Items", d.Items}, field{"Subagents", d.Subagents}, - field{"Artifacts", d.Artifacts}, field{"SessionAdmin", d.SessionAdmin}, field{"Environments", d.Environments}, + field{"Artifacts", d.Artifacts}, + field{"ArtifactsReader", d.ArtifactsReader}, + field{"SessionAdmin", d.SessionAdmin}, field{"Environments", d.Environments}, field{"ExecutorConnections", d.ExecutorConnections}, field{"Admin", d.Admin}, field{"AdminAudit", d.AdminAudit}, field{"WriteAudit", d.WriteAudit}, field{"Metrics", d.Metrics}, field{"RuntimeObservations", d.RuntimeObservations}, field{"RuntimeHistory", d.RuntimeHistory}, ); err != nil { diff --git a/services/core/internal/api/dependencies_test.go b/services/core/internal/api/dependencies_test.go index e8c3d00b1..246d1f931 100644 --- a/services/core/internal/api/dependencies_test.go +++ b/services/core/internal/api/dependencies_test.go @@ -35,6 +35,7 @@ type testFakes struct { items *fakeItems subagents *fakeSubagents artifacts *fakeArtifacts + artifactsReader *fakeArtifactsReader sessionAdmin *fakeSessionAdmin environments *fakeEnvironments executorConnections *fakeExecutorConnections @@ -78,8 +79,10 @@ func testDependencies(t testing.TB) (Dependencies, *testFakes) { sessionEvents: &fakeSessionEvents{t: t}, turns: &fakeTurns{t: t}, items: &fakeItems{t: t}, - subagents: &fakeSubagents{t: t}, artifacts: &fakeArtifacts{t: t}, - sessionAdmin: &fakeSessionAdmin{t: t}, environments: &fakeEnvironments{t: t}, executorConnections: &fakeExecutorConnections{t: t}, + subagents: &fakeSubagents{t: t}, + artifacts: &fakeArtifacts{t: t}, + artifactsReader: &fakeArtifactsReader{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}, runtimeObservations: &fakeRuntimeObservations{t: t}, runtimeHistory: &fakeRuntimeHistory{t: t}, installationBindings: &fakeInstallationBindings{t: t}, sessionAdmission: &fakeSessionAdmission{t: t}, @@ -103,8 +106,11 @@ func testDependencies(t testing.TB) (Dependencies, *testFakes) { SessionEvents: f.sessionEvents, Turns: f.turns, Items: f.items, - Subagents: f.subagents, Artifacts: f.artifacts, SessionAdmin: f.sessionAdmin, - Environments: f.environments, ExecutorConnections: f.executorConnections, Admin: f.admin, AdminAudit: f.adminAudit, WriteAudit: f.writeAudit, + Subagents: f.subagents, + Artifacts: f.artifacts, + ArtifactsReader: f.artifactsReader, + 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, }, f } diff --git a/services/core/internal/api/fakes_test.go b/services/core/internal/api/fakes_test.go index 7379141f5..d50f85999 100644 --- a/services/core/internal/api/fakes_test.go +++ b/services/core/internal/api/fakes_test.go @@ -131,40 +131,44 @@ func (f *fakeAgentsReader) GetAgentWithModelProvider(a0 context.Context, a1 stri type fakeArtifacts struct { t testing.TB - getSessionArtifact func(context.Context, string, string, string) (sessions.Artifact, error) - listSessionArtifacts func(context.Context, string, string, string, string, int, bool) (sessions.ArtifactPage, error) - readSessionArtifact func(context.Context, string, string, string, func(sessions.Artifact, io.Reader) error) error - deleteSessionArtifact func(context.Context, string, string, string) error + deleteSessionArtifact func(context.Context, sessions.DeleteSessionArtifactCommand) error } -func (f *fakeArtifacts) GetSessionArtifact(a0 context.Context, a1 string, a2 string, a3 string) (sessions.Artifact, error) { +func (f *fakeArtifacts) DeleteSessionArtifact(a0 context.Context, a1 sessions.DeleteSessionArtifactCommand) error { + if f.deleteSessionArtifact == nil { + unexpectedCall(f.t, "DeleteSessionArtifact") + } + return f.deleteSessionArtifact(a0, a1) +} + +type fakeArtifactsReader struct { + t testing.TB + getSessionArtifact func(context.Context, string, string, string) (sessions.Artifact, error) + listSessionArtifacts func(context.Context, string, string, string, string, int, bool) (sessions.ArtifactPage, error) + readSessionArtifact func(context.Context, string, string, string, func(sessions.Artifact, io.Reader) error) error +} + +func (f *fakeArtifactsReader) GetSessionArtifact(a0 context.Context, a1 string, a2 string, a3 string) (sessions.Artifact, error) { if f.getSessionArtifact == nil { unexpectedCall(f.t, "GetSessionArtifact") } return f.getSessionArtifact(a0, a1, a2, a3) } -func (f *fakeArtifacts) ListSessionArtifacts(a0 context.Context, a1 string, a2 string, a3 string, a4 string, a5 int, a6 bool) (sessions.ArtifactPage, error) { +func (f *fakeArtifactsReader) ListSessionArtifacts(a0 context.Context, a1 string, a2 string, a3 string, a4 string, a5 int, a6 bool) (sessions.ArtifactPage, error) { if f.listSessionArtifacts == nil { unexpectedCall(f.t, "ListSessionArtifacts") } return f.listSessionArtifacts(a0, a1, a2, a3, a4, a5, a6) } -func (f *fakeArtifacts) ReadSessionArtifact(a0 context.Context, a1 string, a2 string, a3 string, a4 func(sessions.Artifact, io.Reader) error) error { +func (f *fakeArtifactsReader) ReadSessionArtifact(a0 context.Context, a1 string, a2 string, a3 string, a4 func(sessions.Artifact, io.Reader) error) error { if f.readSessionArtifact == nil { unexpectedCall(f.t, "ReadSessionArtifact") } return f.readSessionArtifact(a0, a1, a2, a3, a4) } -func (f *fakeArtifacts) DeleteSessionArtifact(a0 context.Context, a1 string, a2 string, a3 string) error { - if f.deleteSessionArtifact == nil { - unexpectedCall(f.t, "DeleteSessionArtifact") - } - return f.deleteSessionArtifact(a0, a1, a2, a3) -} - type fakeConfigurationDiscovery struct { t testing.TB discoverConfiguration func(ctx context.Context, provider string, input sandbox.ConfigurationDiscoveryInput) (json.RawMessage, error) diff --git a/services/core/internal/api/items.go b/services/core/internal/api/items.go index 5c294c89a..cc2ddf834 100644 --- a/services/core/internal/api/items.go +++ b/services/core/internal/api/items.go @@ -34,7 +34,7 @@ func (h *Handler) listItems(w http.ResponseWriter, r *http.Request) { } page, err := h.Items.ListItems(r.Context(), tenantID(r), chi.URLParam(r, "session_id"), options.after, options.limit, options.ascending) if err != nil { - writeStoreError(w, r, err) + writeSessionsError(w, r, err) return } writeJSON(w, http.StatusOK, itemListResponse(page.Items, page.HasMore)) diff --git a/services/core/internal/api/native_classification_integration_test.go b/services/core/internal/api/native_classification_integration_test.go index 64cb83480..e9dcde9df 100644 --- a/services/core/internal/api/native_classification_integration_test.go +++ b/services/core/internal/api/native_classification_integration_test.go @@ -15,7 +15,7 @@ import ( func TestNativeClassificationPostgresRoundTripAndPublicPrivacy(t *testing.T) { s, pool := diagnosticDatabase(t) - h, _, tenant := adminTestHandler(t, databaseSessionReads(s)) + h, _, tenant := adminTestHandler(t, databaseSessionReads(s, pool)) for _, code := range []string{"authentication_error", "connection_failed", "secret-canary"} { t.Run(code, func(t *testing.T) { session, err := s.CreateSession(t.Context(), tenant, sessions.CreateSession{Creator: identity.Subject{Kind: "service_account", ID: "native-classification"}, Engine: "codex", IdempotencyKey: uuid.NewString(), Configuration: json.RawMessage(`{"agent":{"id":"agent_root","model":"test"},"environment":{"type":"none"}}`)}) diff --git a/services/core/internal/api/resource_query_test.go b/services/core/internal/api/resource_query_test.go index 7c6f96359..9e4cf7408 100644 --- a/services/core/internal/api/resource_query_test.go +++ b/services/core/internal/api/resource_query_test.go @@ -254,9 +254,9 @@ type ownedArtifactStore struct { deleted int } -func (s *ownedArtifactStore) DeleteSessionArtifact(_ context.Context, tenant, session, id string) error { - s.tenants = append(s.tenants, tenant) - if tenant != s.owner || session != "session" || id != "artifact" { +func (s *ownedArtifactStore) DeleteSessionArtifact(_ context.Context, command sessions.DeleteSessionArtifactCommand) error { + s.tenants = append(s.tenants, command.TenantID) + if command.TenantID != s.owner || command.SessionID != "session" || command.ArtifactID != "artifact" { return sessions.ErrNotFound } s.deleted++ diff --git a/services/core/internal/api/session_artifacts.go b/services/core/internal/api/session_artifacts.go index cdc6400ac..70ee53e9c 100644 --- a/services/core/internal/api/session_artifacts.go +++ b/services/core/internal/api/session_artifacts.go @@ -11,12 +11,16 @@ import ( "github.com/go-chi/chi/v5" ) -// Artifacts reads, streams and deletes a Session's published Artifacts. +// Artifacts deletes a Session's published Artifacts. type Artifacts interface { + DeleteSessionArtifact(context.Context, sessions.DeleteSessionArtifactCommand) error +} + +// ArtifactsReader reads and streams a Session's published Artifacts. +type ArtifactsReader interface { GetSessionArtifact(context.Context, string, string, string) (sessions.Artifact, error) ListSessionArtifacts(context.Context, string, string, string, string, int, bool) (sessions.ArtifactPage, error) ReadSessionArtifact(context.Context, string, string, string, func(sessions.Artifact, io.Reader) error) error - DeleteSessionArtifact(context.Context, string, string, string) error } // @Summary List immutable Session artifacts @@ -38,9 +42,9 @@ func (h *Handler) listSessionArtifacts(w http.ResponseWriter, r *http.Request) { if !ok { return } - page, err := h.Artifacts.ListSessionArtifacts(r.Context(), tenantID(r), chi.URLParam(r, "session_id"), r.URL.Query().Get("environment_id"), options.after, options.limit, options.ascending) + page, err := h.ArtifactsReader.ListSessionArtifacts(r.Context(), tenantID(r), chi.URLParam(r, "session_id"), r.URL.Query().Get("environment_id"), options.after, options.limit, options.ascending) if err != nil { - writeStoreError(w, r, err) + writeSessionsError(w, r, err) return } data := make([]v1.SessionArtifact, 0, len(page.Artifacts)) @@ -62,9 +66,9 @@ func (h *Handler) listSessionArtifacts(w http.ResponseWriter, r *http.Request) { // @Failure 400,401,404,500,503 {object} v1.ErrorResponse // @Router /agents/sessions/{session_id}/artifacts/{artifact_id} [get] func (h *Handler) getSessionArtifact(w http.ResponseWriter, r *http.Request) { - artifact, err := h.Artifacts.GetSessionArtifact(r.Context(), tenantID(r), chi.URLParam(r, "session_id"), chi.URLParam(r, "artifact_id")) + artifact, err := h.ArtifactsReader.GetSessionArtifact(r.Context(), tenantID(r), chi.URLParam(r, "session_id"), chi.URLParam(r, "artifact_id")) if err != nil { - writeStoreError(w, r, err) + writeSessionsError(w, r, err) return } writeJSON(w, http.StatusOK, artifactResponse(artifact)) @@ -83,8 +87,8 @@ func (h *Handler) getSessionArtifact(w http.ResponseWriter, r *http.Request) { // @Router /agents/sessions/{session_id}/artifacts/{artifact_id} [delete] func (h *Handler) deleteSessionArtifact(w http.ResponseWriter, r *http.Request) { id := chi.URLParam(r, "artifact_id") - if err := h.Artifacts.DeleteSessionArtifact(r.Context(), tenantID(r), chi.URLParam(r, "session_id"), id); err != nil { - writeStoreError(w, r, err) + if err := h.Artifacts.DeleteSessionArtifact(r.Context(), sessions.DeleteSessionArtifactCommand{TenantID: tenantID(r), SessionID: chi.URLParam(r, "session_id"), ArtifactID: id}); err != nil { + writeSessionsError(w, r, err) return } writeJSON(w, http.StatusOK, v1.SessionArtifactDeleted{ID: id, Object: "agent.session.artifact.deleted", Deleted: true}) @@ -103,12 +107,12 @@ func (h *Handler) deleteSessionArtifact(w http.ResponseWriter, r *http.Request) // @Router /agents/sessions/{session_id}/artifacts/{artifact_id}/content [get] func (h *Handler) sessionArtifactContent(w http.ResponseWriter, r *http.Request) { err := serveStoredContent(w, r, func(ctx context.Context, consume func(string, int64, io.Reader) error) error { - return h.Artifacts.ReadSessionArtifact(ctx, tenantID(r), chi.URLParam(r, "session_id"), chi.URLParam(r, "artifact_id"), func(a sessions.Artifact, body io.Reader) error { + return h.ArtifactsReader.ReadSessionArtifact(ctx, tenantID(r), chi.URLParam(r, "session_id"), chi.URLParam(r, "artifact_id"), func(a sessions.Artifact, body io.Reader) error { return consume(path.Base(a.Path), a.SizeBytes, body) }) }) if err != nil { - writeStoreError(w, r, err) + writeSessionsError(w, r, err) } } diff --git a/services/core/internal/api/session_artifacts_test.go b/services/core/internal/api/session_artifacts_test.go index f8ba1a65e..52bdd2316 100644 --- a/services/core/internal/api/session_artifacts_test.go +++ b/services/core/internal/api/session_artifacts_test.go @@ -48,15 +48,16 @@ func (f *artifactFixture) ReadSessionArtifact(ctx context.Context, tenant, sessi return consume(a, bytes.NewReader([]byte{0, 255, 1})) } -func (f *artifactFixture) DeleteSessionArtifact(ctx context.Context, tenant, session, id string) error { - _, err := f.GetSessionArtifact(ctx, tenant, session, id) +func (f *artifactFixture) DeleteSessionArtifact(ctx context.Context, command sessions.DeleteSessionArtifactCommand) error { + _, err := f.GetSessionArtifact(ctx, command.TenantID, command.SessionID, command.ArtifactID) return err } -// wire serves the Artifacts area from f. +// wire serves the Artifacts and ArtifactsReader areas from f. func (f *artifactFixture) wire(_ *Dependencies, fakes *testFakes) { - fakes.artifacts.getSessionArtifact, fakes.artifacts.listSessionArtifacts = f.GetSessionArtifact, f.ListSessionArtifacts - fakes.artifacts.readSessionArtifact, fakes.artifacts.deleteSessionArtifact = f.ReadSessionArtifact, f.DeleteSessionArtifact + fakes.artifactsReader.getSessionArtifact, fakes.artifactsReader.listSessionArtifacts = f.GetSessionArtifact, f.ListSessionArtifacts + fakes.artifactsReader.readSessionArtifact = f.ReadSessionArtifact + fakes.artifacts.deleteSessionArtifact = f.DeleteSessionArtifact } type artifactResponseRecorder struct{ *httptest.ResponseRecorder } diff --git a/services/core/internal/api/session_diagnostics_public_compat_test.go b/services/core/internal/api/session_diagnostics_public_compat_test.go index b2e4acef6..63694ea55 100644 --- a/services/core/internal/api/session_diagnostics_public_compat_test.go +++ b/services/core/internal/api/session_diagnostics_public_compat_test.go @@ -9,6 +9,8 @@ import ( "testing" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/identity" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/sessionpg" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store" "github.com/MiniMax-AI/OpenAgentCore/services/core/migrations" @@ -23,7 +25,7 @@ func TestDiagnosticPublicCompatibility(t *testing.T) { key := callerBinding() deps, fakes := testDependencies(t) fakes.projectsReader.resolveAPIKey = projectKeys(t, key).ResolveAPIKey - databaseSessionReads(s)(&deps, fakes) + databaseSessionReads(s, pool)(&deps, fakes) h := newTestHandler(t, deps) session, err := s.CreateSession(t.Context(), key.TenantID, sessions.CreateSession{Creator: identity.Subject{Kind: "service_account", ID: "compat-test"}, Engine: "codex", IdempotencyKey: uuid.NewString(), Configuration: json.RawMessage(`{"agent":{"id":"agent_root","model":"test"},"environment":{"type":"none"}}`)}) if err != nil { @@ -60,9 +62,13 @@ func diagnosticRequest(handler http.Handler, path, token string) *httptest.Respo return w } -// databaseSessionReads serves Session, Turn, Item and diagnostic reads from s. -func databaseSessionReads(s *store.Store) func(*Dependencies, *testFakes) { - return func(d *Dependencies, _ *testFakes) { d.Sessions, d.Turns, d.Items, d.SessionAdmin = s, s, s, s } +// databaseSessionReads serves Session, Turn and diagnostic reads from s, and +// Item reads from the Session adapter on pool. +func databaseSessionReads(s *store.Store, pool *pgxpool.Pool) func(*Dependencies, *testFakes) { + return func(d *Dependencies, _ *testFakes) { + d.Sessions, d.Turns, d.SessionAdmin = s, s, s + d.Items = sessionpg.New(pgunit.NewPool(pool)) + } } func diagnosticDatabase(t *testing.T) (*store.Store, *pgxpool.Pool) { diff --git a/services/core/internal/api/session_diagnostics_test.go b/services/core/internal/api/session_diagnostics_test.go index 98bdf78a4..572cf8c1a 100644 --- a/services/core/internal/api/session_diagnostics_test.go +++ b/services/core/internal/api/session_diagnostics_test.go @@ -14,7 +14,7 @@ import ( func TestDiagnosticsCoreHandlerDatabaseBoundary(t *testing.T) { s, pool := diagnosticDatabase(t) - h, _, tenant := adminTestHandler(t, databaseSessionReads(s)) + h, _, tenant := adminTestHandler(t, databaseSessionReads(s, pool)) session, err := s.CreateSession(t.Context(), tenant, sessions.CreateSession{Creator: identity.Subject{Kind: "service_account", ID: "diagnostic-test"}, Engine: "codex", IdempotencyKey: "diagnostics", Configuration: json.RawMessage(`{"agent":{"id":"agent_root","model":"test"},"environment":{"type":"none"}}`)}) if err != nil { t.Fatal(err) diff --git a/services/core/internal/api/subagent_turns.go b/services/core/internal/api/subagent_turns.go index ed3e863b3..6ad9c4d45 100644 --- a/services/core/internal/api/subagent_turns.go +++ b/services/core/internal/api/subagent_turns.go @@ -21,7 +21,7 @@ import ( func (h *Handler) getSubagentTurn(w http.ResponseWriter, r *http.Request) { value, err := h.Subagents.GetSubagentTurn(r.Context(), tenantID(r), chi.URLParam(r, "session_id"), chi.URLParam(r, "subagent_id"), chi.URLParam(r, "turn_id")) if err != nil { - writeStoreError(w, r, err) + writeSessionsError(w, r, err) return } writeJSON(w, http.StatusOK, value) @@ -48,7 +48,7 @@ func (h *Handler) listSubagentTurns(w http.ResponseWriter, r *http.Request) { } page, err := h.Subagents.ListSubagentTurns(r.Context(), tenantID(r), chi.URLParam(r, "session_id"), chi.URLParam(r, "subagent_id"), options.after, options.limit, options.ascending) if err != nil { - writeStoreError(w, r, err) + writeSessionsError(w, r, err) return } writeJSON(w, http.StatusOK, turnListResponse(page.Data, page.HasMore)) @@ -76,7 +76,7 @@ func (h *Handler) listSubagentTurnItems(w http.ResponseWriter, r *http.Request) } page, err := h.Subagents.ListSubagentTurnItems(r.Context(), tenantID(r), chi.URLParam(r, "session_id"), chi.URLParam(r, "subagent_id"), chi.URLParam(r, "turn_id"), options.after, options.limit, options.ascending) if err != nil { - writeStoreError(w, r, err) + writeSessionsError(w, r, err) return } writeJSON(w, http.StatusOK, itemListResponse(page.Data, page.HasMore)) diff --git a/services/core/internal/api/subagents.go b/services/core/internal/api/subagents.go index 0b8cbffa5..101f84027 100644 --- a/services/core/internal/api/subagents.go +++ b/services/core/internal/api/subagents.go @@ -43,7 +43,7 @@ func (h *Handler) registerSubagentRoutes(r chi.Router) { func (h *Handler) getSubagent(w http.ResponseWriter, r *http.Request) { value, err := h.Subagents.GetSubagent(r.Context(), tenantID(r), chi.URLParam(r, "session_id"), chi.URLParam(r, "subagent_id")) if err != nil { - writeStoreError(w, r, err) + writeSessionsError(w, r, err) return } writeJSON(w, http.StatusOK, value) @@ -69,7 +69,7 @@ func (h *Handler) listSubagents(w http.ResponseWriter, r *http.Request) { } page, err := h.Subagents.ListSubagents(r.Context(), tenantID(r), chi.URLParam(r, "session_id"), options.after, options.limit, options.ascending) if err != nil { - writeStoreError(w, r, err) + writeSessionsError(w, r, err) return } writeJSON(w, http.StatusOK, subagentListResponse(page.Data, page.HasMore)) @@ -96,7 +96,7 @@ func (h *Handler) listSubagentItems(w http.ResponseWriter, r *http.Request) { } page, err := h.Subagents.ListSubagentItems(r.Context(), tenantID(r), chi.URLParam(r, "session_id"), chi.URLParam(r, "subagent_id"), options.after, options.limit, options.ascending) if err != nil { - writeStoreError(w, r, err) + writeSessionsError(w, r, err) return } writeJSON(w, http.StatusOK, itemListResponse(page.Data, page.HasMore)) diff --git a/services/core/internal/execution/artifacts.go b/services/core/internal/execution/artifacts.go index fd9fe9f9a..203f5ada1 100644 --- a/services/core/internal/execution/artifacts.go +++ b/services/core/internal/execution/artifacts.go @@ -21,7 +21,7 @@ func (d *Dispatcher) captureCompletedArtifacts(ctx context.Context, peer *runtim if err == nil { err = d.withPreparedWorkspace(owner, peer, session, environment, bound, func(ctx context.Context, handle string) error { return peer.ExportWorkspaceOutputs(ctx, proto.WorkspaceExportPayload{Handle: handle, EnvironmentID: environment.ID}, func(body io.Reader) error { - return d.Store.StageTurnArtifacts(ctx, session.TenantID, session.ID, turnID, environment.ID, body) + return d.Sessions.StageTurnArtifacts(ctx, sessions.StageTurnArtifactsCommand{TenantID: session.TenantID, SessionID: session.ID, TurnID: turnID, EnvironmentID: environment.ID, Export: body}) }) }) } diff --git a/services/core/internal/execution/dispatcher.go b/services/core/internal/execution/dispatcher.go index e5ca6b691..7dd709059 100644 --- a/services/core/internal/execution/dispatcher.go +++ b/services/core/internal/execution/dispatcher.go @@ -40,6 +40,11 @@ type Dispatcher struct { // Deployment reads the sandbox deployment and prepares a selection's setup. // It is required; deployment changes go through Owner.Deployment. Deployment *deployment.Service + // Sessions runs the pooled Session use cases, such as staging a Turn's + // Artifacts. It is required. + Sessions *sessions.Service + // SessionsReader serves the plain Session reads. It is required. + SessionsReader sessions.Reader // ManagedRuntimes is optional internal provisioning; it does not admit hosted API requests. ManagedRuntimes *RuntimeProvider // MaxConcurrentExecutions bounds work admitted by this Core execution owner. diff --git a/services/core/internal/execution/owner_test.go b/services/core/internal/execution/owner_test.go index 2d703c5df..5dae77e5c 100644 --- a/services/core/internal/execution/owner_test.go +++ b/services/core/internal/execution/owner_test.go @@ -10,6 +10,7 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/modelconfiguration" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtimegateway" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" "github.com/google/uuid" ) @@ -52,6 +53,17 @@ func (l *closeCountingLease) Close(ctx context.Context) error { return l.inner.Close(ctx) } +// unusedSessions returns the Session service and reader for Workers that run +// no Turn. Any call through them panics. +func unusedSessions(t *testing.T) (*sessions.Service, sessions.Reader) { + t.Helper() + service, err := sessions.NewService(struct{ sessions.Storage }{}) + if err != nil { + t.Fatal(err) + } + return service, struct{ sessions.Reader }{} +} + // unusedObserver fails the test on any observation. The Workers it serves run // no Turn. type unusedObserver struct{ t *testing.T } @@ -89,21 +101,35 @@ func TestStartWorkerFailureClosesLeaseOnce(t *testing.T) { _, err := StartWorker(canceled, &Dispatcher{Credentials: &recordingCredentials{}, Observer: unusedObserver{t}}, Owner{Lease: lease}) return err }, - "missing Store": func(t *testing.T, lease *closeCountingLease) error { + "missing Session service": func(t *testing.T, lease *closeCountingLease) error { _, _, deployments := resetManagerStore(t) _, err := StartWorker(canceled, &Dispatcher{Credentials: &recordingCredentials{}, Observer: unusedObserver{t}, Deployment: deployments}, Owner{Lease: lease}) return err }, + "missing Session reader": func(t *testing.T, lease *closeCountingLease) error { + _, _, deployments := resetManagerStore(t) + service, _ := unusedSessions(t) + _, err := StartWorker(canceled, &Dispatcher{Credentials: &recordingCredentials{}, Observer: unusedObserver{t}, Deployment: deployments, Sessions: service}, Owner{Lease: lease}) + return err + }, + "missing Store": func(t *testing.T, lease *closeCountingLease) error { + _, _, deployments := resetManagerStore(t) + service, reader := unusedSessions(t) + _, err := StartWorker(canceled, &Dispatcher{Credentials: &recordingCredentials{}, Observer: unusedObserver{t}, Deployment: deployments, Sessions: service, SessionsReader: reader}, Owner{Lease: lease}) + return err + }, "missing deployment": func(t *testing.T, lease *closeCountingLease) error { _, owner, deployments := resetManagerStore(t) - _, err := StartWorker(canceled, &Dispatcher{Credentials: &recordingCredentials{}, Observer: unusedObserver{t}, Deployment: deployments}, Owner{Lease: lease, Store: owner.Store}) + service, reader := unusedSessions(t) + _, err := StartWorker(canceled, &Dispatcher{Credentials: &recordingCredentials{}, Observer: unusedObserver{t}, Deployment: deployments, Sessions: service, SessionsReader: reader}, Owner{Lease: lease, Store: owner.Store}) return err }, "deployment claim": func(t *testing.T, lease *closeCountingLease) error { s, owner, deployments := resetManagerStore(t) lease.inner = owner.Lease id := uuid.NewString() - dispatcher := &Dispatcher{Store: s, Registry: runtimegateway.NewRegistry(), Credentials: &recordingCredentials{}, Observer: unusedObserver{t}, Deployment: deployments, ManagedRuntimes: NewDeferredRuntimeProvider(id, func(context.Context) (*RuntimeProvider, error) { return nil, nil })} + service, reader := unusedSessions(t) + dispatcher := &Dispatcher{Store: s, Registry: runtimegateway.NewRegistry(), Credentials: &recordingCredentials{}, Observer: unusedObserver{t}, Deployment: deployments, Sessions: service, SessionsReader: reader, ManagedRuntimes: NewDeferredRuntimeProvider(id, func(context.Context) (*RuntimeProvider, error) { return nil, nil })} _, err := StartWorker(canceled, dispatcher, Owner{Lease: lease, Store: owner.Store, Deployment: owner.Deployment}) if ping := owner.Lease.CheckOwnership(t.Context()); !errors.Is(ping, pgunit.ErrLeaseClosed) { t.Error("failed start kept the database lease", ping) @@ -127,6 +153,7 @@ func TestStartWorkerFailureClosesLeaseOnce(t *testing.T) { func TestStartWorkerChecksDeploymentAfterItsDependencies(t *testing.T) { _, owner, deployments := resetManagerStore(t) credentials, observer := &recordingCredentials{}, unusedObserver{t} + service, reader := unusedSessions(t) for _, test := range []struct { name string dispatcher Dispatcher @@ -136,8 +163,10 @@ func TestStartWorkerChecksDeploymentAfterItsDependencies(t *testing.T) { {"missing Credentials", Dispatcher{Observer: observer}, true, "execution worker requires MCP Credentials"}, {"missing observer", Dispatcher{Credentials: credentials}, true, "execution requires a model configuration observer"}, {"missing deployment service", Dispatcher{Credentials: credentials, Observer: observer}, true, "execution worker requires the deployment service"}, - {"missing Store", Dispatcher{Credentials: credentials, Observer: observer, Deployment: deployments}, false, "execution worker requires the execution Store"}, - {"missing deployment", Dispatcher{Credentials: credentials, Observer: observer, Deployment: deployments}, true, "execution worker requires the deployment execution operations"}, + {"missing Session service", Dispatcher{Credentials: credentials, Observer: observer, Deployment: deployments}, true, "execution worker requires the Session service"}, + {"missing Session reader", Dispatcher{Credentials: credentials, Observer: observer, Deployment: deployments, Sessions: service}, true, "execution worker requires the Session reader"}, + {"missing Store", Dispatcher{Credentials: credentials, Observer: observer, Deployment: deployments, Sessions: service, SessionsReader: reader}, false, "execution worker requires the execution Store"}, + {"missing deployment", Dispatcher{Credentials: credentials, Observer: observer, Deployment: deployments, Sessions: service, SessionsReader: reader}, true, "execution worker requires the deployment execution operations"}, } { t.Run(test.name, func(t *testing.T) { lease := &closeCountingLease{t: t} @@ -157,7 +186,8 @@ func TestWorkerRunClosesLeaseAfterDrain(t *testing.T) { s, owner, deployments := resetManagerStore(t) lease := &closeCountingLease{t: t, inner: owner.Lease} id := uuid.NewString() - dispatcher := &Dispatcher{Store: s, Registry: runtimegateway.NewRegistry(), Credentials: &recordingCredentials{}, Observer: unusedObserver{t}, Deployment: deployments, ManagedRuntimes: NewDeferredRuntimeProvider(id, func(context.Context) (*RuntimeProvider, error) { return nil, nil })} + service, reader := unusedSessions(t) + dispatcher := &Dispatcher{Store: s, Registry: runtimegateway.NewRegistry(), Credentials: &recordingCredentials{}, Observer: unusedObserver{t}, Deployment: deployments, Sessions: service, SessionsReader: reader, ManagedRuntimes: NewDeferredRuntimeProvider(id, func(context.Context) (*RuntimeProvider, error) { return nil, nil })} worker, err := StartWorker(t.Context(), dispatcher, Owner{Lease: lease, Store: owner.Store, Deployment: owner.Deployment}) if err != nil { t.Fatal(err) diff --git a/services/core/internal/execution/worker.go b/services/core/internal/execution/worker.go index 731fe7419..668eb6a27 100644 --- a/services/core/internal/execution/worker.go +++ b/services/core/internal/execution/worker.go @@ -59,6 +59,12 @@ func StartWorker(ctx context.Context, dispatcher *Dispatcher, owner Owner) (_ *W if dispatcher.Deployment == nil { return nil, errors.New("execution worker requires the deployment service") } + if dispatcher.Sessions == nil { + return nil, errors.New("execution worker requires the Session service") + } + if dispatcher.SessionsReader == nil { + return nil, errors.New("execution worker requires the Session reader") + } if owner.Store == nil { return nil, errors.New("execution worker requires the execution Store") } diff --git a/services/core/internal/persistence/postgres/sessionpg/artifacts.go b/services/core/internal/persistence/postgres/sessionpg/artifacts.go new file mode 100644 index 000000000..a5ec9a935 --- /dev/null +++ b/services/core/internal/persistence/postgres/sessionpg/artifacts.go @@ -0,0 +1,255 @@ +package sessionpg + +import ( + "context" + "errors" + "fmt" + "io" + + "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/persistence/postgres/auditpg" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" +) + +var ( + _ sessions.ArtifactReader = (*Store)(nil) + _ sessions.ArtifactStorage = (*Store)(nil) +) + +func (s *Store) GetSessionArtifact(ctx context.Context, tenantID, sessionID, artifactID string) (sessions.Artifact, error) { + lookup, err := artifactLookup(tenantID, sessionID, artifactID) + if err != nil { + return sessions.Artifact{}, err + } + row, err := loadArtifact(ctx, s.units.Queries(), lookup) + if err != nil { + return sessions.Artifact{}, err + } + return artifactFromRow(row), nil +} + +func (s *Store) ListSessionArtifacts(ctx context.Context, tenantID, sessionID, environmentID, cursor string, limit int, ascending bool) (sessions.ArtifactPage, error) { + if limit < 1 || limit > 100 { + return sessions.ArtifactPage{}, sessions.ErrInvalidInput + } + tenant, err := parseID(tenantID) + if err != nil { + return sessions.ArtifactPage{}, err + } + session := pgunit.PathID(sessionID) + q := s.units.Queries() + if err := visibleSession(ctx, q, tenant, session); err != nil { + return sessions.ArtifactPage{}, err + } + params := sqlc.ListSessionArtifactsParams{TenantID: tenant, SessionID: session, PageLimit: int32(limit + 1), Ascending: ascending, AfterID: pgtype.UUID{Valid: true}} + if environmentID != "" { + // A malformed filter matches nothing, like another Environment's ID (HE-56). + params.EnvironmentID = pgunit.PathID(environmentID) + } + if cursor != "" { + // Any cursor that is not an Artifact of this Session, including a + // malformed one, is an invalid cursor rather than a missing resource. + after, err := loadArtifact(ctx, q, sqlc.GetSessionArtifactParams{TenantID: tenant, SessionID: session, ID: pgunit.PathID(cursor)}) + if errors.Is(err, sessions.ErrNotFound) { + // The Session lookup above is a separate statement: a Session deleted + // since then stays not found. Deleted Sessions never reappear, so an + // existing one here also existed when the cursor was read. + if err := visibleSession(ctx, q, tenant, session); err != nil { + return sessions.ArtifactPage{}, err + } + return sessions.ArtifactPage{}, sessions.ErrArtifactCursor + } + if err != nil { + return sessions.ArtifactPage{}, err + } + params.AfterCreated = after.CreatedAt + params.AfterID = after.ID + } + rows, err := q.ListSessionArtifacts(ctx, params) + if err != nil { + return sessions.ArtifactPage{}, err + } + page := sessions.ArtifactPage{Artifacts: make([]sessions.Artifact, 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 { + page.Artifacts = append(page.Artifacts, artifactFromRow(row)) + } + return page, nil +} + +// ReadSessionArtifact keeps an admitted snapshot available across concurrent +// deletion. +func (s *Store) ReadSessionArtifact(ctx context.Context, tenantID, sessionID, artifactID string, consume func(sessions.Artifact, io.Reader) error) error { + lookup, err := artifactLookup(tenantID, sessionID, artifactID) + if err != nil { + return err + } + if consume == nil { + return sessions.ErrInvalidInput + } + return s.units.Snapshot(ctx, func(ctx context.Context, tx pgx.Tx) error { + row, err := loadArtifact(ctx, sqlc.New(tx), lookup) + if err != nil { + return err + } + objects := tx.LargeObjects() + body, err := objects.Open(ctx, row.BodyOid.Uint32, pgx.LargeObjectModeRead) + if err != nil { + return err + } + if err := consume(artifactFromRow(row), body); err != nil { + return err + } + return body.Close() + }) +} + +func (s *Store) DeleteSessionArtifact(ctx context.Context, tenantID, sessionID, artifactID string) error { + lookup, err := artifactLookup(tenantID, sessionID, artifactID) + if err != nil { + return err + } + return s.units.Transaction(ctx, func(ctx context.Context, tx pgx.Tx) error { + q := sqlc.New(tx) + // Use the same lock order as whole-Session deletion and Turn publication. + locked, err := LockSession(ctx, q, lookup.TenantID, lookup.SessionID) + if err != nil { + return err + } + if err := locked.Public(); err != nil { + return err + } + oid, err := q.DeleteSessionArtifact(ctx, sqlc.DeleteSessionArtifactParams(lookup)) + if errors.Is(err, pgx.ErrNoRows) { + return sessions.ErrNotFound + } + if err != nil { + return err + } + objects := tx.LargeObjects() + if err := objects.Unlink(ctx, oid.Uint32); err != nil { + return err + } + return auditpg.RecordWriteAudit(ctx, q, tenantID, "delete", "artifact", uuid.UUID(lookup.ID.Bytes).String(), uuid.UUID(lookup.SessionID.Bytes).String()) + }) +} + +// WithArtifactStaging runs the staging in one pooled transaction, not on the +// execution lease: the transfer can outlast a lease transaction, and staged +// Artifacts stay private until the lease-bound completion publishes or +// discards them. Large objects the staging created are removed with the +// rolled-back transaction. +func (s *Store) WithArtifactStaging(ctx context.Context, key sessions.ArtifactStagingKey, stage func(context.Context, sessions.ArtifactStagingTx) error) error { + turn, err := TurnLookup(key.TenantID, key.SessionID, key.TurnID) + if err != nil { + return err + } + environment, err := parseID(key.EnvironmentID) + if err != nil { + return err + } + return s.units.Transaction(ctx, func(ctx context.Context, tx pgx.Tx) error { + return stage(ctx, &artifactStaging{tx: tx, q: sqlc.New(tx), turn: turn, environment: environment}) + }) +} + +// artifactStaging is one staging transaction. It keeps the content put so far +// until StageArtifacts records it. +type artifactStaging struct { + tx pgx.Tx + q *sqlc.Queries + turn sqlc.GetTurnParams + environment pgtype.UUID + staged []sqlc.StageSessionArtifactParams +} + +func (t *artifactStaging) LoadEnvironment(ctx context.Context) (sessions.Environment, error) { + row, err := t.q.GetSessionEnvironment(ctx, sqlc.GetSessionEnvironmentParams{TenantID: t.turn.TenantID, ID: t.turn.SessionID}) + return EnvironmentFromRow(row.Environment, row.TenantID, row.Configuration, err) +} + +func (t *artifactStaging) PutArtifactContent(ctx context.Context, path string, size int64, content io.Reader) error { + writer, err := pgunit.CreateLargeObject(ctx, t.tx) + if err != nil { + return err + } + if _, err := io.CopyN(writer, content, size); err != nil { + return err + } + body, err := writer.Close() + if err != nil { + return err + } + t.staged = append(t.staged, sqlc.StageSessionArtifactParams{ID: pgtype.UUID{Bytes: uuid.New(), Valid: true}, Path: path, SizeBytes: body.Size, BodyOid: pgtype.Uint32{Uint32: body.OID, Valid: true}, Sha256: body.SHA256}) + return nil +} + +func (t *artifactStaging) LockSession(ctx context.Context) (sessions.LockedSession, error) { + return LockSession(ctx, t.q, t.turn.TenantID, t.turn.SessionID) +} + +func (t *artifactStaging) LoadTurn(ctx context.Context) (sessions.Turn, error) { + row, err := t.q.GetTurn(ctx, t.turn) + if errors.Is(err, pgx.ErrNoRows) { + return sessions.Turn{}, sessions.ErrNotFound + } + if err != nil { + return sessions.Turn{}, err + } + return TurnFromRow(row), nil +} + +func (t *artifactStaging) StageArtifacts(ctx context.Context) error { + for _, row := range t.staged { + row.SessionID, row.TurnID, row.EnvironmentID = t.turn.SessionID, t.turn.ID, t.environment + if err := t.q.StageSessionArtifact(ctx, row); err != nil { + return err + } + } + return nil +} + +// artifactLookup resolves a public Artifact path: a malformed tenant is +// sessions.ErrInvalidInput, and a malformed Session or Artifact ID resolves as +// missing. +func artifactLookup(tenantID, sessionID, artifactID string) (sqlc.GetSessionArtifactParams, error) { + tenant, err := parseID(tenantID) + return sqlc.GetSessionArtifactParams{TenantID: tenant, SessionID: pgunit.PathID(sessionID), ID: pgunit.PathID(artifactID)}, err +} + +// loadArtifact loads a published Artifact of a visible Session, or +// sessions.ErrNotFound. +func loadArtifact(ctx context.Context, q *sqlc.Queries, lookup sqlc.GetSessionArtifactParams) (sqlc.SessionArtifact, error) { + row, err := q.GetSessionArtifact(ctx, lookup) + if errors.Is(err, pgx.ErrNoRows) { + return row, sessions.ErrNotFound + } + return row, err +} + +// visibleSession reports whether the tenant's Session exists and is not +// publicly deleted, as sessions.ErrNotFound. +func visibleSession(ctx context.Context, q *sqlc.Queries, tenant, session pgtype.UUID) error { + _, err := q.GetSession(ctx, sqlc.GetSessionParams{TenantID: tenant, ID: session}) + if errors.Is(err, pgx.ErrNoRows) { + return sessions.ErrNotFound + } + if err != nil { + return fmt.Errorf("get session: %w", err) + } + return nil +} + +func artifactFromRow(row sqlc.SessionArtifact) sessions.Artifact { + return sessions.Artifact{ID: uuid.UUID(row.ID.Bytes).String(), SessionID: uuid.UUID(row.SessionID.Bytes).String(), + TurnID: uuid.UUID(row.TurnID.Bytes).String(), EnvironmentID: uuid.UUID(row.EnvironmentID.Bytes).String(), + Path: row.Path, SizeBytes: row.SizeBytes, CreatedAt: row.CreatedAt.Time} +} diff --git a/services/core/internal/persistence/postgres/sessionpg/artifacts_test.go b/services/core/internal/persistence/postgres/sessionpg/artifacts_test.go new file mode 100644 index 000000000..abdded5e0 --- /dev/null +++ b/services/core/internal/persistence/postgres/sessionpg/artifacts_test.go @@ -0,0 +1,264 @@ +package sessionpg + +import ( + "archive/tar" + "bytes" + "errors" + "io" + "testing" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "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/sessions" +) + +func uuidText(id pgtype.UUID) string { return uuid.UUID(id.Bytes).String() } + +// stagingTurn stores a fresh tenant's Session with a connected Environment of +// kind and an in-progress Turn, and returns their IDs. +func stagingTurn(t *testing.T, pool *pgxpool.Pool, kind string) (tenant, session, environment, turn string) { + t.Helper() + tenantID, sessionID, environmentID := newEnvironment(t, pool, kind, "connected") + turnID := uuid.New() + exec(t, pool, `INSERT INTO turns(id, session_id, status) VALUES ($1, $2, 'in_progress')`, turnID, sessionID) + return uuidText(tenantID), uuidText(sessionID), uuidText(environmentID), turnID.String() +} + +// artifactExport is a Turn export of files in order. +func artifactExport(t *testing.T, files ...[2]string) []byte { + t.Helper() + var data bytes.Buffer + w := tar.NewWriter(&data) + for _, file := range files { + if err := w.WriteHeader(&tar.Header{Name: file[0], Mode: 0600, Size: int64(len(file[1])), Typeflag: tar.TypeReg}); err != nil { + t.Fatal(err) + } + if _, err := w.Write([]byte(file[1])); err != nil { + t.Fatal(err) + } + } + if err := w.Close(); err != nil { + t.Fatal(err) + } + return data.Bytes() +} + +func largeObjects(t *testing.T, pool *pgxpool.Pool) int { + t.Helper() + var count int + if err := pool.QueryRow(t.Context(), "SELECT count(*) FROM pg_largeobject_metadata").Scan(&count); err != nil { + t.Fatal(err) + } + return count +} + +func stagedRows(t *testing.T, pool *pgxpool.Pool, turn string) int { + t.Helper() + var count int + if err := pool.QueryRow(t.Context(), "SELECT count(*) FROM session_artifacts WHERE turn_id = $1", turn).Scan(&count); err != nil { + t.Fatal(err) + } + return count +} + +func stagingService(t *testing.T, pool *pgxpool.Pool) (*Store, *sessions.Service) { + t.Helper() + store := New(pgunit.NewPool(pool)) + service, err := sessions.NewService(store) + if err != nil { + t.Fatal(err) + } + return store, service +} + +func stage(t *testing.T, service *sessions.Service, tenant, session, turn, environment string, export io.Reader) error { + return service.StageTurnArtifacts(t.Context(), sessions.StageTurnArtifactsCommand{TenantID: tenant, SessionID: session, TurnID: turn, EnvironmentID: environment, Export: export}) +} + +type artifactReadError struct{} + +func (artifactReadError) Read([]byte) (int, error) { return 0, io.ErrUnexpectedEOF } + +// A rejected or unauthorized export leaves no content and no Artifact. The +// database is isolated because the check counts every large object. +func TestArtifactStagingStoresNothingItRejects(t *testing.T) { + pool := pgtest.OpenIsolated(t, nil) + _, service := stagingService(t, pool) + for _, kind := range []string{"openai_hosted", "self_hosted"} { + t.Run(kind, func(t *testing.T) { + tenant, session, environment, turn := stagingTurn(t, pool, kind) + before := largeObjects(t, pool) + valid := artifactExport(t, [2]string{"outputs/a", "data"}) + for name, body := range map[string]io.Reader{ + "transport-failure-after-valid-tar": io.MultiReader(bytes.NewReader(valid), artifactReadError{}), + "truncated-body": bytes.NewReader(valid[:513]), + "trailing-data": io.MultiReader(bytes.NewReader(valid), bytes.NewReader([]byte("not archive padding"))), + "traversal": bytes.NewReader(artifactExport(t, [2]string{"outputs/../secret", "no"})), + "private-root": bytes.NewReader(artifactExport(t, [2]string{"outputs/a", "data"}, [2]string{"secrets/key", "no"})), + } { + t.Run(name, func(t *testing.T) { + if err := stage(t, service, tenant, session, turn, environment, body); err == nil { + t.Fatal("invalid capture accepted") + } + if count := largeObjects(t, pool); count != before { + t.Fatalf("rollback leaked objects: %d -> %d", before, count) + } + if rows := stagedRows(t, pool, turn); rows != 0 { + t.Fatalf("rollback kept %d Artifacts", rows) + } + }) + } + // Authorization precedes reading the export. + for _, ids := range [][4]string{{uuid.NewString(), session, turn, environment}, {tenant, session, turn, uuid.NewString()}} { + if err := stage(t, service, ids[0], ids[1], ids[2], ids[3], artifactReadError{}); !errors.Is(err, sessions.ErrNotFound) { + t.Fatalf("unauthorized capture reached the export: %v", err) + } + } + }) + } +} + +// Staged content stays private, and a Turn that stopped running or a deleted +// Session stages nothing. +func TestArtifactStagingRequiresTheRunningTurn(t *testing.T) { + pool := pgtest.OpenIsolated(t, nil) + store, service := stagingService(t, pool) + tenant, session, environment, turn := stagingTurn(t, pool, "openai_hosted") + before := largeObjects(t, pool) + body := artifactExport(t, [2]string{"outputs/a.txt", "alpha"}, [2]string{"outputs/empty", ""}) + if err := stage(t, service, tenant, session, turn, environment, bytes.NewReader(body)); err != nil { + t.Fatal(err) + } + var private, total int + if err := pool.QueryRow(t.Context(), `SELECT count(*) FILTER (WHERE created_at IS NULL AND session_id = $2 AND environment_id = $3), count(*) + FROM session_artifacts WHERE turn_id = $1`, turn, session, environment).Scan(&private, &total); err != nil || private != 2 || total != 2 { + t.Fatalf("staged %d private of %d: %v", private, total, err) + } + if count := largeObjects(t, pool); count != before+2 { + t.Fatalf("staged content: %d -> %d", before, count) + } + if page, err := store.ListSessionArtifacts(t.Context(), tenant, session, "", "", 100, true); err != nil || len(page.Artifacts) != 0 { + t.Fatalf("private capture visible: %+v %v", page, err) + } + + exec(t, pool, `UPDATE turns SET cancel_requested_at = clock_timestamp() WHERE id = $1`, turn) + late := artifactExport(t, [2]string{"outputs/late", "late"}) + if err := stage(t, service, tenant, session, turn, environment, bytes.NewReader(late)); !errors.Is(err, sessions.ErrTurnConflict) { + t.Fatalf("capture after a cancel request: %v", err) + } + exec(t, pool, `UPDATE sessions SET deleted_at = clock_timestamp() WHERE id = $1`, session) + if err := stage(t, service, tenant, session, turn, environment, bytes.NewReader(late)); !errors.Is(err, sessions.ErrNotFound) { + t.Fatalf("capture for a deleted Session: %v", err) + } + if count := largeObjects(t, pool); count != before+2 { + t.Fatalf("rejected capture leaked objects: %d -> %d", before+2, count) + } + if rows := stagedRows(t, pool, turn); rows != 2 { + t.Fatalf("rejected capture changed Artifacts: %d", rows) + } +} + +// The Artifact list pages in publication order in both directions, cursors +// stay within the Session, and a read admitted before a deletion finishes. +func TestSessionArtifactReadsPageReadAndDelete(t *testing.T) { + pool := pgtest.Open(t) + store, service := stagingService(t, pool) + tenant, session, environment, turn := stagingTurn(t, pool, "openai_hosted") + body := artifactExport(t, [2]string{"outputs/a", "alpha"}, [2]string{"outputs/b", "bravo"}, [2]string{"outputs/c", "charlie"}) + if err := stage(t, service, tenant, session, turn, environment, bytes.NewReader(body)); err != nil { + t.Fatal(err) + } + // Publication belongs to Turn completion; publish in path order here. + exec(t, pool, `UPDATE session_artifacts SET created_at = timestamptz '2026-01-01' + (ascii(right(path, 1)) * interval '1 second') WHERE turn_id = $1`, turn) + otherTenant, otherSession, otherEnvironment, otherTurn := stagingTurn(t, pool, "openai_hosted") + if err := stage(t, service, otherTenant, otherSession, otherTurn, otherEnvironment, bytes.NewReader(artifactExport(t, [2]string{"outputs/x", "x"}))); err != nil { + t.Fatal(err) + } + exec(t, pool, `UPDATE session_artifacts SET created_at = clock_timestamp() WHERE turn_id = $1`, otherTurn) + foreign, err := store.ListSessionArtifacts(t.Context(), otherTenant, otherSession, "", "", 100, true) + if err != nil || len(foreign.Artifacts) != 1 { + t.Fatal(foreign, err) + } + + for _, test := range []struct { + ascending bool + want string + }{{true, "abc"}, {false, "cba"}} { + var got string + cursor := "" + for pages := 0; ; pages++ { + page, err := store.ListSessionArtifacts(t.Context(), tenant, session, environment, cursor, 1, test.ascending) + if err != nil || len(page.Artifacts) != 1 || pages > 3 { + t.Fatal(page, err) + } + got += page.Artifacts[0].Path[len("/workspace/outputs/"):] + if cursor = page.NextCursor; cursor == "" { + break + } + } + if got != test.want { + t.Fatalf("ascending=%v paged %q, want %q", test.ascending, got, test.want) + } + } + for _, limit := range []int{0, 101} { + if _, err := store.ListSessionArtifacts(t.Context(), tenant, session, "", "", limit, true); !errors.Is(err, sessions.ErrInvalidInput) { + t.Fatalf("limit %d: %v", limit, err) + } + } + for _, cursor := range []string{"not-an-id", foreign.Artifacts[0].ID} { + if _, err := store.ListSessionArtifacts(t.Context(), tenant, session, "", cursor, 10, true); !errors.Is(err, sessions.ErrArtifactCursor) { + t.Fatalf("cursor %q: %v", cursor, err) + } + } + if _, err := store.ListSessionArtifacts(t.Context(), otherTenant, session, "", "", 10, true); !errors.Is(err, sessions.ErrNotFound) { + t.Fatalf("foreign tenant: %v", err) + } + if page, err := store.ListSessionArtifacts(t.Context(), tenant, session, "not-an-id", "", 10, true); err != nil || len(page.Artifacts) != 0 { + t.Fatalf("malformed environment filter: %+v %v", page, err) + } + + page, err := store.ListSessionArtifacts(t.Context(), tenant, session, "", "", 10, true) + if err != nil || len(page.Artifacts) != 3 { + t.Fatal(page, err) + } + first := page.Artifacts[0] + if got, err := store.GetSessionArtifact(t.Context(), tenant, session, first.ID); err != nil || got.ID != first.ID || got.Path != "/workspace/outputs/a" { + t.Fatalf("get: %+v %v", got, err) + } + if _, err := store.GetSessionArtifact(t.Context(), otherTenant, session, first.ID); !errors.Is(err, sessions.ErrNotFound) { + t.Fatalf("foreign get: %v", err) + } + var oid uint32 + if err := pool.QueryRow(t.Context(), "SELECT body_oid FROM session_artifacts WHERE id = $1", first.ID).Scan(&oid); err != nil { + t.Fatal(err) + } + if err := service.DeleteSessionArtifact(t.Context(), sessions.DeleteSessionArtifactCommand{TenantID: otherTenant, SessionID: session, ArtifactID: first.ID}); !errors.Is(err, sessions.ErrNotFound) { + t.Fatalf("foreign delete: %v", err) + } + if err := store.ReadSessionArtifact(t.Context(), tenant, session, first.ID, func(read sessions.Artifact, content io.Reader) error { + if err := service.DeleteSessionArtifact(t.Context(), sessions.DeleteSessionArtifactCommand{TenantID: tenant, SessionID: session, ArtifactID: first.ID}); err != nil { + return err + } + data, err := io.ReadAll(content) + if err != nil || read.ID != first.ID || string(data) != "alpha" { + t.Fatalf("admitted read: %+v %q %v", read, data, err) + } + return nil + }); err != nil { + t.Fatal(err) + } + if _, err := store.GetSessionArtifact(t.Context(), tenant, session, first.ID); !errors.Is(err, sessions.ErrNotFound) { + t.Fatalf("deleted Artifact: %v", err) + } + var content bool + if err := pool.QueryRow(t.Context(), "SELECT EXISTS (SELECT 1 FROM pg_largeobject_metadata WHERE oid = $1)", oid).Scan(&content); err != nil || content { + t.Fatalf("deleted content kept: %v %v", content, err) + } + if err := service.DeleteSessionArtifact(t.Context(), sessions.DeleteSessionArtifactCommand{TenantID: tenant, SessionID: session, ArtifactID: first.ID}); !errors.Is(err, sessions.ErrNotFound) { + t.Fatalf("repeated delete: %v", err) + } +} diff --git a/services/core/internal/persistence/postgres/sessionpg/doc.go b/services/core/internal/persistence/postgres/sessionpg/doc.go index 4709ade9d..3ccc9cc8d 100644 --- a/services/core/internal/persistence/postgres/sessionpg/doc.go +++ b/services/core/internal/persistence/postgres/sessionpg/doc.go @@ -6,7 +6,9 @@ // sequence positions, event IDs, Item positions and output indexes, and write // the public change journal, Items, Turn usage, Artifact settlement, // Environment state, input reservations and devices. SessionTx binds a Session -// to its caller's transaction for the sessions procedures. +// to its caller's transaction for the sessions procedures. Store is the pooled +// adapter: it stores the sessions.Service use cases and serves the Session +// reads. package sessionpg import ( diff --git a/services/core/internal/store/item_reads.go b/services/core/internal/persistence/postgres/sessionpg/item_reads.go similarity index 96% rename from services/core/internal/store/item_reads.go rename to services/core/internal/persistence/postgres/sessionpg/item_reads.go index facbfed05..6156742dc 100644 --- a/services/core/internal/store/item_reads.go +++ b/services/core/internal/persistence/postgres/sessionpg/item_reads.go @@ -1,18 +1,21 @@ -package store +package sessionpg import ( "context" "encoding/json" "errors" + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + 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/persistence/postgres/pgunit" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" - "github.com/jackc/pgx/v5" - "github.com/jackc/pgx/v5/pgtype" ) +var _ sessions.ItemReader = (*Store)(nil) + func (s *Store) ListItems(ctx context.Context, tenantID, sessionID, cursor string, limit int, ascending bool) (sessions.ItemPage, error) { if limit < 1 || limit > 100 { return sessions.ItemPage{}, sessions.ErrInvalidInput diff --git a/services/core/internal/persistence/postgres/sessionpg/public_session.go b/services/core/internal/persistence/postgres/sessionpg/public_session.go new file mode 100644 index 000000000..76766cd7c --- /dev/null +++ b/services/core/internal/persistence/postgres/sessionpg/public_session.go @@ -0,0 +1,28 @@ +package sessionpg + +import ( + "context" + + "github.com/jackc/pgx/v5/pgtype" + + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/db/sqlc" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" +) + +// withPublicSession runs apply in the Session transaction of the tenant's +// visible Session. A malformed tenant is sessions.ErrInvalidInput; a malformed +// Session ID resolves as a missing Session. +func (s *Store) withPublicSession(ctx context.Context, tenantID, sessionID string, apply func(context.Context, *sqlc.Queries, pgtype.UUID) error) error { + tenant, err := parseID(tenantID) + if err != nil { + return err + } + session := pgunit.PathID(sessionID) + return WithSession(ctx, s.units, tenant, session, func(ctx context.Context, q *sqlc.Queries, locked sessions.LockedSession) error { + if err := locked.Public(); err != nil { + return err + } + return apply(ctx, q, session) + }) +} diff --git a/services/core/internal/persistence/postgres/sessionpg/store.go b/services/core/internal/persistence/postgres/sessionpg/store.go new file mode 100644 index 000000000..fb32417b5 --- /dev/null +++ b/services/core/internal/persistence/postgres/sessionpg/store.go @@ -0,0 +1,18 @@ +package sessionpg + +import ( + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" +) + +// Store is the pooled Session adapter: the storage of the Session use cases +// and the Session reads. +type Store struct{ units *pgunit.Pool } + +// New builds the pooled Session adapter on units. +func New(units *pgunit.Pool) *Store { return &Store{units: units} } + +var ( + _ sessions.Storage = (*Store)(nil) + _ sessions.Reader = (*Store)(nil) +) diff --git a/services/core/internal/persistence/postgres/sessionpg/subagent_reads.go b/services/core/internal/persistence/postgres/sessionpg/subagent_reads.go new file mode 100644 index 000000000..8b8fc91de --- /dev/null +++ b/services/core/internal/persistence/postgres/sessionpg/subagent_reads.go @@ -0,0 +1,230 @@ +package sessionpg + +import ( + "context" + "encoding/json" + "errors" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + + 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/persistence/postgres/pgunit" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" +) + +var _ sessions.SubagentReader = (*Store)(nil) + +func (s *Store) GetSubagent(ctx context.Context, tenantID, sessionID, subagentID string) (v1.Subagent, error) { + var result v1.Subagent + err := s.withPublicSession(ctx, tenantID, sessionID, func(ctx context.Context, q *sqlc.Queries, session pgtype.UUID) error { + var err error + result, err = LoadPublicSubagent(ctx, q, session, subagentID) + return err + }) + return result, err +} + +func (s *Store) ListSubagents(ctx context.Context, tenantID, sessionID, cursor string, limit int, ascending bool) (v1.SubagentList, error) { + result := v1.SubagentList{Data: []v1.Subagent{}} + if limit < 1 || limit > 100 { + return result, sessions.ErrInvalidInput + } + err := s.withPublicSession(ctx, tenantID, sessionID, func(ctx context.Context, q *sqlc.Queries, session pgtype.UUID) error { + p := sqlc.ListPublicSubagentsParams{SessionID: session, Ascending: ascending, PageLimit: int32(limit + 1), AfterID: pgtype.UUID{Valid: true}} + if cursor != "" { + // Any cursor that is not a Subagent of this Session, including a + // malformed one, is an invalid cursor rather than a missing resource. + after, err := LoadPublicSubagent(ctx, q, session, cursor) + if err != nil { + return unresolvedCursor(err, sessions.ErrResourceCursor) + } + p.AfterOpened = pgtype.Int8{Int64: after.OpenedAt, Valid: true} + p.AfterID, _ = parseID(cursor) + } + rows, err := q.ListPublicSubagents(ctx, p) + if err != nil { + return err + } + result.HasMore = len(rows) > limit + if result.HasMore { + rows = rows[:limit] + } + for _, id := range rows { + value, err := LoadPublicSubagent(ctx, q, session, uuid.UUID(id.Bytes).String()) + if err != nil { + return err + } + result.Data = append(result.Data, value) + } + return nil + }) + return result, err +} + +func (s *Store) GetSubagentTurn(ctx context.Context, tenantID, sessionID, subagentID, turnID string) (v1.Turn, error) { + var result v1.Turn + err := s.withPublicSession(ctx, tenantID, sessionID, func(ctx context.Context, q *sqlc.Queries, session pgtype.UUID) error { + if _, err := LoadPublicSubagent(ctx, q, session, subagentID); err != nil { + return err + } + row, err := LoadChildTurn(ctx, q, session, subagentID, turnID) + if err != nil { + return err + } + agent, err := sessionAgentID(ctx, q, session) + if err == nil { + result = publicChildTurn(row, agent) + } + return err + }) + return result, err +} + +func (s *Store) ListSubagentTurns(ctx context.Context, tenantID, sessionID, subagentID, cursor string, limit int, ascending bool) (v1.TurnList, error) { + result := v1.TurnList{Data: []v1.Turn{}} + if limit < 1 || limit > 100 { + return result, sessions.ErrInvalidInput + } + err := s.withPublicSession(ctx, tenantID, sessionID, func(ctx context.Context, q *sqlc.Queries, session pgtype.UUID) error { + if _, err := LoadPublicSubagent(ctx, q, session, subagentID); err != nil { + return err + } + child, _ := parseID(subagentID) + p := sqlc.ListChildTurnsParams{SessionID: session, SubagentID: child, Ascending: ascending, PageLimit: int32(limit + 1), AfterID: pgtype.UUID{Valid: true}} + if cursor != "" { + // Root Turns and other children's Turns are outside this list. + row, err := LoadChildTurn(ctx, q, session, subagentID, cursor) + if err != nil { + return unresolvedCursor(err, sessions.ErrResourceCursor) + } + p.AfterCreated = row.CreatedAt + p.AfterID = row.ID + } + rows, err := q.ListChildTurns(ctx, p) + if err != nil { + return err + } + agent, err := sessionAgentID(ctx, q, session) + if err != nil { + return err + } + result.HasMore = len(rows) > limit + if result.HasMore { + rows = rows[:limit] + } + for _, row := range rows { + result.Data = append(result.Data, publicChildTurn(row, agent)) + } + return nil + }) + return result, err +} + +func (s *Store) ListSubagentItems(ctx context.Context, tenantID, sessionID, subagentID, cursor string, limit int, ascending bool) (v1.ItemList, error) { + return s.listChildItems(ctx, tenantID, sessionID, subagentID, "", cursor, limit, ascending) +} + +func (s *Store) ListSubagentTurnItems(ctx context.Context, tenantID, sessionID, subagentID, turnID, cursor string, limit int, ascending bool) (v1.ItemList, error) { + if turnID == "" { + return v1.ItemList{}, sessions.ErrInvalidInput + } + return s.listChildItems(ctx, tenantID, sessionID, subagentID, turnID, cursor, limit, ascending) +} + +// listChildItems lists the Items of the Subagent, or of one of its Turns when +// turnID is not empty. +func (s *Store) listChildItems(ctx context.Context, tenantID, sessionID, subagentID, turnID, cursor string, limit int, ascending bool) (v1.ItemList, error) { + result := v1.ItemList{Data: []v1.Item{}} + if limit < 1 || limit > 100 { + return result, sessions.ErrInvalidInput + } + err := s.withPublicSession(ctx, tenantID, sessionID, func(ctx context.Context, q *sqlc.Queries, session pgtype.UUID) error { + if _, err := LoadPublicSubagent(ctx, q, session, subagentID); err != nil { + return err + } + child, _ := parseID(subagentID) + p := sqlc.ListChildItemsParams{SessionID: session, SubagentID: child, Ascending: ascending, PageLimit: int32(limit + 1), AfterID: pgtype.UUID{Valid: true}, AfterTurn: pgtype.UUID{Valid: true}} + if turnID != "" { + row, err := LoadChildTurn(ctx, q, session, subagentID, turnID) + if err != nil { + return err + } + p.TurnID = row.ID + } + if cursor != "" { + // Any cursor outside this child (and Turn) scope, including a + // malformed one, uses the Session Item cursor error. + row, err := q.GetChildItem(ctx, sqlc.GetChildItemParams{SessionID: session, SubagentID: child, ID: pgunit.PathID(cursor)}) + if errors.Is(err, pgx.ErrNoRows) || (err == nil && p.TurnID.Valid && p.TurnID != row.TurnID) { + return sessions.ErrItemCursor + } + if err != nil { + return err + } + p.AfterID = row.ID + p.AfterCreated = row.TurnCreatedAt + p.AfterTurn = row.TurnID + p.AfterPosition = row.Position + } + rows, err := q.ListChildItems(ctx, p) + if err != nil { + return err + } + result.HasMore = len(rows) > limit + if result.HasMore { + rows = rows[:limit] + } + for _, raw := range rows { + var item v1.Item + if err := json.Unmarshal(raw, &item); err != nil { + return err + } + result.Data = append(result.Data, item) + } + return nil + }) + return result, err +} + +// sessionAgentID returns the Session's Agent ID, which is the agent_id of every +// Turn in the Session, including child Turns. The official service projects a +// direct child's Turn this way; nested children follow the same rule unobserved. +func sessionAgentID(ctx context.Context, q *sqlc.Queries, session pgtype.UUID) (string, error) { + agent, err := q.SubagentRootAgent(ctx, session) + if err == nil && agent == "" { + err = errors.New("missing stored agent identity") + } + return agent, err +} + +// publicChildTurn identifies the child through subagent_id; agent_id is the +// Session's Agent ID. +func publicChildTurn(row sqlc.SubagentTurn, agent string) v1.Turn { + child := uuid.UUID(row.SubagentID.Bytes).String() + value := v1.Turn{ID: uuid.UUID(row.ID.Bytes).String(), SessionID: uuid.UUID(row.SessionID.Bytes).String(), AgentID: agent, SubagentID: &child, Object: "agent.session.turn", Status: row.Status, CreatedAt: row.CreatedAt.Time.Unix()} + if row.StartedAt.Valid { + seconds := row.StartedAt.Time.Unix() + value.StartedAt = &seconds + } + if row.CompletedAt.Valid { + seconds := row.CompletedAt.Time.Unix() + value.CompletedAt = &seconds + } + _ = json.Unmarshal(row.TokenUsage, &value.Usage) + if row.Status == sessions.TurnFailed { + value.Error = &v1.TurnError{Code: "internal_error", Message: "The execution could not complete."} + } + return value +} + +// unresolvedCursor replaces a missing cursor resource with the list's cursor +// error and keeps every other failure. +func unresolvedCursor(err, cursor error) error { + if errors.Is(err, sessions.ErrNotFound) { + return cursor + } + return err +} diff --git a/services/core/internal/persistence/postgres/sessionpg/subagents.go b/services/core/internal/persistence/postgres/sessionpg/subagents.go new file mode 100644 index 000000000..594132f66 --- /dev/null +++ b/services/core/internal/persistence/postgres/sessionpg/subagents.go @@ -0,0 +1,49 @@ +package sessionpg + +import ( + "context" + "errors" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + + 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/persistence/postgres/pgunit" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" +) + +// LoadPublicSubagent loads a Subagent of the Session as the public API shows +// it. A missing or malformed ID is sessions.ErrNotFound. +func LoadPublicSubagent(ctx context.Context, q *sqlc.Queries, session pgtype.UUID, id string) (v1.Subagent, error) { + row, err := q.GetPublicSubagent(ctx, sqlc.GetPublicSubagentParams{SessionID: session, ID: pgunit.PathID(id)}) + if errors.Is(err, pgx.ErrNoRows) { + return v1.Subagent{}, sessions.ErrNotFound + } + if err != nil { + return v1.Subagent{}, err + } + result := v1.Subagent{ID: id, SessionID: uuid.UUID(session.Bytes).String(), ParentAgentID: row.ParentAgentID, Object: "agent.session.subagent", Status: row.Status, OpenedAt: row.NativeCreatedAt} + if row.Name.Valid { + result.Name = &row.Name.String + } + if row.Instructions.Valid { + result.Instructions = []v1.AgentContent{{Type: "output_text", Text: &row.Instructions.String}} + } + if row.ClosedAtMs.Valid { + seconds := row.ClosedAtMs.Int64 / 1000 + result.ClosedAt = &seconds + } + return result, nil +} + +// LoadChildTurn loads a Turn of the Session's Subagent child. A missing or +// malformed ID, a root Turn and another child's Turn are sessions.ErrNotFound. +func LoadChildTurn(ctx context.Context, q *sqlc.Queries, session pgtype.UUID, child, id string) (sqlc.SubagentTurn, error) { + row, err := q.GetChildTurn(ctx, sqlc.GetChildTurnParams{SessionID: session, ID: pgunit.PathID(id)}) + if errors.Is(err, pgx.ErrNoRows) || (err == nil && uuid.UUID(row.SubagentID.Bytes).String() != child) { + return row, sessions.ErrNotFound + } + return row, err +} diff --git a/services/core/internal/sandbox/providers/configuration_flow_test.go b/services/core/internal/sandbox/providers/configuration_flow_test.go index 6802289f4..144a3647a 100644 --- a/services/core/internal/sandbox/providers/configuration_flow_test.go +++ b/services/core/internal/sandbox/providers/configuration_flow_test.go @@ -132,9 +132,10 @@ func TestAdditionalConfigurationProviderUsesCommonAPIAndStore(t *testing.T) { SessionCreation: s, SessionEvents: s, Turns: s, - Items: s, - Subagents: s, - Artifacts: s, + Items: struct{ api.Items }{}, + Subagents: struct{ api.Subagents }{}, + Artifacts: struct{ api.Artifacts }{}, + ArtifactsReader: struct{ api.ArtifactsReader }{}, SessionAdmin: s, Environments: s, Admin: s, AdminAudit: struct{ api.AdminAudit }{}, WriteAudit: struct{ api.WriteAudit }{}, ExecutorConnections: struct{ api.ExecutorConnections }{}, diff --git a/services/core/internal/sessions/artifacts.go b/services/core/internal/sessions/artifacts.go new file mode 100644 index 000000000..f7d6178cf --- /dev/null +++ b/services/core/internal/sessions/artifacts.go @@ -0,0 +1,213 @@ +package sessions + +import ( + "archive/tar" + "context" + "io" + "io/fs" + "strings" + "time" +) + +const ( + // maxArtifactBytes bounds the content of one captured Artifact. + maxArtifactBytes int64 = 200 << 20 + // maxArtifactBatchBytes bounds the content of one Turn's export. + maxArtifactBatchBytes int64 = 500 << 20 + // maxArtifactFiles bounds the files of one Turn's export. + maxArtifactFiles = 4096 + // maxArtifactNameBytes bounds an exported file's archive name. + maxArtifactNameBytes = 4096 + // maxExportPadding bounds the zero padding after the archive trailer. + maxExportPadding = 32768 +) + +// Artifact is a file a completed Turn published from its Environment's +// outputs directory, immutable once published. +type Artifact struct { + ID string + SessionID string + TurnID string + EnvironmentID string + Path string + SizeBytes int64 + CreatedAt time.Time +} + +// ArtifactPage is one page of a Session's published Artifacts. NextCursor is +// empty on the last page. +type ArtifactPage struct { + Artifacts []Artifact + NextCursor string +} + +// ArtifactReader reads a Session's published Artifacts, independently of its +// Environment's availability. +type ArtifactReader interface { + // GetSessionArtifact returns a published Artifact of the visible Session, + // or ErrNotFound. + GetSessionArtifact(ctx context.Context, tenantID, sessionID, artifactID string) (Artifact, error) + // ListSessionArtifacts returns one page of the visible Session's published + // Artifacts in publication-time and ID order. A non-empty environmentID + // keeps the Artifacts that Environment produced; a malformed one matches + // nothing. A limit outside 1..100 is ErrInvalidInput. A cursor that is not + // a published Artifact of the Session, including a malformed one, is + // ErrArtifactCursor. + ListSessionArtifacts(ctx context.Context, tenantID, sessionID, environmentID, cursor string, limit int, ascending bool) (ArtifactPage, error) + // ReadSessionArtifact passes consume a published Artifact and its content + // from one snapshot, so a concurrent deletion never truncates a read it + // admitted. It returns consume's error unchanged. + ReadSessionArtifact(ctx context.Context, tenantID, sessionID, artifactID string, consume func(Artifact, io.Reader) error) error +} + +// ArtifactStorage persists a Session's Artifacts. +type ArtifactStorage interface { + // WithArtifactStaging runs stage in one transaction that commits only when + // stage returns nil, so a failed staging leaves no content and no + // Artifact. A malformed identifier in key is ErrInvalidInput. + WithArtifactStaging(ctx context.Context, key ArtifactStagingKey, stage func(context.Context, ArtifactStagingTx) error) error + // DeleteSessionArtifact removes a published Artifact of the visible + // Session and its content under the Session lock, and records the write + // audit, or returns ErrNotFound. + DeleteSessionArtifact(ctx context.Context, tenantID, sessionID, artifactID string) error +} + +// ArtifactStagingKey names the Turn whose export is staged and the +// Environment that produced it. +type ArtifactStagingKey struct { + TenantID string + SessionID string + TurnID string + EnvironmentID string +} + +// ArtifactStagingTx is the transaction StageTurnArtifacts runs in. It takes +// the Session lock only when LockSession is called. +type ArtifactStagingTx interface { + // LoadEnvironment reads the visible Session's Environment without a lock. + LoadEnvironment(ctx context.Context) (Environment, error) + // PutArtifactContent stores size bytes of content for path. The content + // stays private until StageArtifacts records it. + PutArtifactContent(ctx context.Context, path string, size int64, content io.Reader) error + // LockSession locks the Session, including a publicly deleted one. + LockSession(ctx context.Context) (LockedSession, error) + // LoadTurn reads the key's Turn under the Session lock. + LoadTurn(ctx context.Context) (Turn, error) + // StageArtifacts records every content put in this transaction as a + // private Artifact of the key's Turn and Environment. + StageArtifacts(ctx context.Context) error +} + +// StageTurnArtifactsCommand stages what a completed Turn exported from its +// Session's Environment. +type StageTurnArtifactsCommand struct { + TenantID string + SessionID string + TurnID string + EnvironmentID string + // Export is a tar archive of regular files under outputs/, followed by + // nothing but zero padding until the transport ends. + Export io.Reader +} + +// StageTurnArtifacts stores a complete export as private Artifacts of the +// Turn, which the Turn's completion publishes or discards. It authorizes the +// Environment before reading the export or allocating storage, and takes the +// Session lock only after the transfer, so the export never holds admission. +// Under the lock the Turn must still be in progress without a cancellation +// request, or the export is ErrTurnConflict. A rejected export stores nothing. +func (s *Service) StageTurnArtifacts(ctx context.Context, command StageTurnArtifactsCommand) error { + if command.Export == nil { + return ErrInvalidInput + } + key := ArtifactStagingKey{TenantID: command.TenantID, SessionID: command.SessionID, TurnID: command.TurnID, EnvironmentID: command.EnvironmentID} + return s.storage.WithArtifactStaging(ctx, key, func(ctx context.Context, tx ArtifactStagingTx) error { + environment, err := tx.LoadEnvironment(ctx) + if err != nil { + return err + } + if environment.ID != command.EnvironmentID { + return ErrNotFound + } + if _, err := EnvironmentType(environment.Configuration); err != nil { + return ErrInvalidInput + } + if err := readArtifactExport(command.Export, func(path string, size int64, content io.Reader) error { + return tx.PutArtifactContent(ctx, path, size, content) + }); err != nil { + return err + } + locked, err := tx.LockSession(ctx) + if err != nil { + return err + } + if err := locked.Public(); err != nil { + return err + } + turn, err := tx.LoadTurn(ctx) + if err != nil { + return err + } + if turn.Status != TurnInProgress || !turn.CancelRequestedAt.IsZero() { + return ErrTurnConflict + } + return tx.StageArtifacts(ctx) + }) +} + +// DeleteSessionArtifactCommand deletes one published Artifact. +type DeleteSessionArtifactCommand struct { + TenantID string + SessionID string + ArtifactID string +} + +// DeleteSessionArtifact deletes the published copy and its content. The +// workspace file it was captured from stays, and reads admitted before the +// deletion finish. +func (s *Service) DeleteSessionArtifact(ctx context.Context, command DeleteSessionArtifactCommand) error { + return s.storage.DeleteSessionArtifact(ctx, command.TenantID, command.SessionID, command.ArtifactID) +} + +// readArtifactExport passes put each file of a workspace export in archive +// order, as its /workspace path, size and content. Only distinct regular files +// with valid names under outputs/ are admitted, within the file count and the +// per-file and batch size bounds. After the archive trailer the transport must +// end, carrying at most maxExportPadding zero bytes. Anything else is +// ErrInvalidInput; a malformed archive or a failed read returns its own error. +func readArtifactExport(export io.Reader, put func(path string, size int64, content io.Reader) error) error { + archive := tar.NewReader(export) + seen := make(map[string]bool) + var total int64 + for { + header, err := archive.Next() + if err == io.EOF { + break + } + if err != nil { + return err + } + if header.Typeflag != tar.TypeReg || !strings.HasPrefix(header.Name, "outputs/") || !fs.ValidPath(header.Name) || strings.ContainsAny(header.Name, "\\\x00\r\n") || len(header.Name) > maxArtifactNameBytes || header.Size < 0 || header.Size > maxArtifactBytes || header.Size > maxArtifactBatchBytes-total || len(seen) >= maxArtifactFiles || seen[header.Name] { + return ErrInvalidInput + } + seen[header.Name] = true + total += header.Size + if err := put("/workspace/"+header.Name, header.Size, archive); err != nil { + return err + } + } + // Require transport EOF after the archive trailer, including confirmed helper exit. + padding, err := io.ReadAll(io.LimitReader(export, maxExportPadding+1)) + if err != nil { + return err + } + if len(padding) > maxExportPadding { + return ErrInvalidInput + } + for _, b := range padding { + if b != 0 { + return ErrInvalidInput + } + } + return nil +} diff --git a/services/core/internal/sessions/artifacts_test.go b/services/core/internal/sessions/artifacts_test.go new file mode 100644 index 000000000..d36891a3d --- /dev/null +++ b/services/core/internal/sessions/artifacts_test.go @@ -0,0 +1,256 @@ +package sessions + +import ( + "archive/tar" + "bytes" + "context" + "errors" + "fmt" + "io" + "strings" + "testing" + "time" +) + +// exportFile is one entry of a test export. +type exportFile struct { + name string + body string + kind byte +} + +// export builds a tar export of files in order, followed by padding. +func export(t *testing.T, padding []byte, files ...exportFile) []byte { + t.Helper() + var data bytes.Buffer + w := tar.NewWriter(&data) + for _, file := range files { + kind := file.kind + if kind == 0 { + kind = tar.TypeReg + } + header := &tar.Header{Name: file.name, Mode: 0600, Typeflag: kind} + if kind == tar.TypeReg { + header.Size = int64(len(file.body)) + } + if kind == tar.TypeSymlink { + header.Linkname = "/etc/passwd" + } + if err := w.WriteHeader(header); err != nil { + t.Fatal(err) + } + if _, err := w.Write([]byte(file.body)); err != nil { + t.Fatal(err) + } + } + if err := w.Close(); err != nil { + t.Fatal(err) + } + return append(data.Bytes(), padding...) +} + +// oversized is an export whose only header declares size bytes; the +// declared content never follows. +func oversized(t *testing.T, size int64) []byte { + t.Helper() + var data bytes.Buffer + if err := tar.NewWriter(&data).WriteHeader(&tar.Header{Name: "outputs/big", Mode: 0600, Size: size, Typeflag: tar.TypeReg}); err != nil { + t.Fatal(err) + } + return data.Bytes() +} + +type failingRead struct{} + +func (failingRead) Read([]byte) (int, error) { return 0, errors.New("transport failed") } + +func TestReadArtifactExport(t *testing.T) { + var put []string + valid := export(t, make([]byte, maxExportPadding), exportFile{name: "outputs/a.txt", body: "alpha"}, exportFile{name: "outputs/nested/empty"}) + if err := readArtifactExport(bytes.NewReader(valid), func(path string, size int64, content io.Reader) error { + body, err := io.ReadAll(io.LimitReader(content, size)) + put = append(put, fmt.Sprintf("%s %d %s", path, size, body)) + return err + }); err != nil { + t.Fatal(err) + } + if want := []string{"/workspace/outputs/a.txt 5 alpha", "/workspace/outputs/nested/empty 0 "}; strings.Join(put, "|") != strings.Join(want, "|") { + t.Fatalf("put %q, want %q", put, want) + } + + many := make([]exportFile, maxArtifactFiles+1) + for i := range many { + many[i] = exportFile{name: fmt.Sprintf("outputs/%d", i)} + } + for name, body := range map[string][]byte{ + "directory": export(t, nil, exportFile{name: "outputs/dir/", kind: tar.TypeDir}), + "symlink": export(t, nil, exportFile{name: "outputs/link", kind: tar.TypeSymlink}), + "private root": export(t, nil, exportFile{name: "secrets/key", body: "no"}), + "traversal": export(t, nil, exportFile{name: "outputs/../secret", body: "no"}), + "backslash": export(t, nil, exportFile{name: `outputs/a\b`, body: "no"}), + "newline": export(t, nil, exportFile{name: "outputs/a\nb", body: "no"}), + "long name": export(t, nil, exportFile{name: "outputs/" + strings.Repeat("a", maxArtifactNameBytes), body: "no"}), + "duplicate": export(t, nil, exportFile{name: "outputs/a", body: "1"}, exportFile{name: "outputs/a", body: "2"}), + "too many files": export(t, nil, many...), + "oversized file": oversized(t, maxArtifactBytes+1), + "trailing data": export(t, []byte("not archive padding"), exportFile{name: "outputs/a", body: "data"}), + "excess padding": export(t, make([]byte, maxExportPadding+1), exportFile{name: "outputs/a", body: "data"}), + "nonzero padding": export(t, append(make([]byte, 10), 1), exportFile{name: "outputs/a", body: "data"}), + "unterminated tar": export(t, nil, exportFile{name: "outputs/a", body: "data"})[:512], + } { + t.Run(name, func(t *testing.T) { + err := readArtifactExport(bytes.NewReader(body), func(_ string, size int64, content io.Reader) error { + _, err := io.CopyN(io.Discard, content, size) + return err + }) + if err == nil { + t.Fatal("export accepted") + } + if name != "unterminated tar" && !errors.Is(err, ErrInvalidInput) { + t.Fatalf("got %v, want ErrInvalidInput", err) + } + }) + } + + // A failed transfer after a valid archive returns its own error. + transport := io.MultiReader(bytes.NewReader(export(t, nil, exportFile{name: "outputs/a", body: "data"})), failingRead{}) + if err := readArtifactExport(transport, func(_ string, size int64, content io.Reader) error { + _, err := io.CopyN(io.Discard, content, size) + return err + }); err == nil || errors.Is(err, ErrInvalidInput) { + t.Fatalf("transport failure: %v", err) + } + // put's error stops the export. + if err := readArtifactExport(bytes.NewReader(valid), func(string, int64, io.Reader) error { return errStorage }); !errors.Is(err, errStorage) { + t.Fatalf("put failure: %v", err) + } +} + +// fakeStaging is a strict artifact staging transaction. Each method records its +// call, then runs its func; a method whose func is unset fails the test. +type fakeStaging struct { + t *testing.T + calls []string + + loadEnvironment func() (Environment, error) + putContent func(io.Reader, int64) error + lockSession func() (LockedSession, error) + loadTurn func() (Turn, error) + stageArtifacts func() error +} + +func (f *fakeStaging) record(name string, set bool, detail ...string) { + f.t.Helper() + if !set { + f.t.Fatalf("unexpected call to %s", name) + } + f.calls = append(f.calls, strings.Join(append([]string{name}, detail...), " ")) +} + +func (f *fakeStaging) LoadEnvironment(context.Context) (Environment, error) { + f.record("LoadEnvironment", f.loadEnvironment != nil) + return f.loadEnvironment() +} + +func (f *fakeStaging) PutArtifactContent(_ context.Context, path string, size int64, content io.Reader) error { + f.record("PutArtifactContent", f.putContent != nil, path) + return f.putContent(content, size) +} + +func (f *fakeStaging) LockSession(context.Context) (LockedSession, error) { + f.record("LockSession", f.lockSession != nil) + return f.lockSession() +} + +func (f *fakeStaging) LoadTurn(context.Context) (Turn, error) { + f.record("LoadTurn", f.loadTurn != nil) + return f.loadTurn() +} + +func (f *fakeStaging) StageArtifacts(context.Context) error { + f.record("StageArtifacts", f.stageArtifacts != nil) + return f.stageArtifacts() +} + +// fakeArtifactStorage runs each staging in tx and records the key; any other +// call fails the test. +type fakeArtifactStorage struct { + t *testing.T + tx *fakeStaging + keys []ArtifactStagingKey +} + +func (s *fakeArtifactStorage) WithArtifactStaging(ctx context.Context, key ArtifactStagingKey, stage func(context.Context, ArtifactStagingTx) error) error { + s.keys = append(s.keys, key) + return stage(ctx, s.tx) +} + +func (s *fakeArtifactStorage) DeleteSessionArtifact(context.Context, string, string, string) error { + s.t.Fatal("unexpected call to DeleteSessionArtifact") + return nil +} + +func discard(content io.Reader, size int64) error { + _, err := io.CopyN(io.Discard, content, size) + return err +} + +func TestStageTurnArtifacts(t *testing.T) { + hosted := Environment{ID: "environment", Configuration: []byte(`{"type":"openai_hosted"}`)} + running := Turn{ID: "turn", Status: TurnInProgress} + command := func(body []byte) StageTurnArtifactsCommand { + return StageTurnArtifactsCommand{TenantID: "tenant", SessionID: "session", TurnID: "turn", EnvironmentID: "environment", Export: bytes.NewReader(body)} + } + valid := export(t, nil, exportFile{name: "outputs/a", body: "alpha"}, exportFile{name: "outputs/b", body: "bravo"}) + transfer := []string{"LoadEnvironment", "PutArtifactContent /workspace/outputs/a", "PutArtifactContent /workspace/outputs/b", "LockSession", "LoadTurn"} + for _, test := range []struct { + name string + tx fakeStaging + body []byte + want error + calls []string + }{ + {"stages after the lock", fakeStaging{loadEnvironment: returns(hosted), putContent: discard, lockSession: returns(LockedSession{}), loadTurn: returns(running), stageArtifacts: done}, valid, nil, append(transfer, "StageArtifacts")}, + {"another Environment", fakeStaging{loadEnvironment: returns(Environment{ID: "other", Configuration: hosted.Configuration})}, valid, ErrNotFound, []string{"LoadEnvironment"}}, + {"no captured Environment type", fakeStaging{loadEnvironment: returns(Environment{ID: "environment", Configuration: []byte(`{"type":"none"}`)})}, valid, ErrInvalidInput, []string{"LoadEnvironment"}}, + {"invalid export", fakeStaging{loadEnvironment: returns(hosted), putContent: discard}, export(t, nil, exportFile{name: "outputs/a", body: "alpha"}, exportFile{name: "secrets/key", body: "no"}), ErrInvalidInput, []string{"LoadEnvironment", "PutArtifactContent /workspace/outputs/a"}}, + {"deleted Session", fakeStaging{loadEnvironment: returns(hosted), putContent: discard, lockSession: returns(LockedSession{Deleted: true})}, valid, ErrNotFound, transfer[:4]}, + {"settled Turn", fakeStaging{loadEnvironment: returns(hosted), putContent: discard, lockSession: returns(LockedSession{}), loadTurn: returns(Turn{ID: "turn", Status: TurnCompleted})}, valid, ErrTurnConflict, transfer}, + {"cancel requested", fakeStaging{loadEnvironment: returns(hosted), putContent: discard, lockSession: returns(LockedSession{}), loadTurn: returns(Turn{ID: "turn", Status: TurnInProgress, CancelRequestedAt: time.Unix(1, 0)})}, valid, ErrTurnConflict, transfer}, + } { + t.Run(test.name, func(t *testing.T) { + tx := test.tx + tx.t = t + storage := &fakeArtifactStorage{t: t, tx: &tx} + service, err := NewService(storage) + if err != nil { + t.Fatal(err) + } + err = service.StageTurnArtifacts(t.Context(), command(test.body)) + if test.want == nil && err != nil || test.want != nil && !errors.Is(err, test.want) { + t.Fatalf("got %v, want %v", err, test.want) + } + if strings.Join(tx.calls, "\n") != strings.Join(test.calls, "\n") { + t.Fatalf("calls:\n%s\nwant:\n%s", strings.Join(tx.calls, "\n"), strings.Join(test.calls, "\n")) + } + if want := (ArtifactStagingKey{TenantID: "tenant", SessionID: "session", TurnID: "turn", EnvironmentID: "environment"}); len(storage.keys) != 1 || storage.keys[0] != want { + t.Fatalf("staging keys %+v", storage.keys) + } + }) + } + + // Without an export nothing reaches storage. + service, err := NewService(&fakeArtifactStorage{t: t}) + if err != nil { + t.Fatal(err) + } + if err := service.StageTurnArtifacts(t.Context(), StageTurnArtifactsCommand{TenantID: "tenant", SessionID: "session", TurnID: "turn", EnvironmentID: "environment"}); !errors.Is(err, ErrInvalidInput) { + t.Fatal(err) + } +} + +func TestNewServiceRequiresStorage(t *testing.T) { + if _, err := NewService(nil); err == nil { + t.Fatal("service built without storage") + } +} diff --git a/services/core/internal/sessions/doc.go b/services/core/internal/sessions/doc.go index b3f93cc48..690c0ac9a 100644 --- a/services/core/internal/sessions/doc.go +++ b/services/core/internal/sessions/doc.go @@ -7,5 +7,6 @@ // The Session writes that several operations share are procedures here, over // the transaction interfaces declared beside them: cancelling work, failing // and terminating an Environment, tracking input activity and the admission -// gates. +// gates. Service runs the pooled Session use cases, such as staging a Turn's +// Artifacts, over Storage, and Reader declares the Session reads. package sessions diff --git a/services/core/internal/sessions/items.go b/services/core/internal/sessions/items.go new file mode 100644 index 000000000..f6447c639 --- /dev/null +++ b/services/core/internal/sessions/items.go @@ -0,0 +1,22 @@ +package sessions + +import ( + "context" + + v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" +) + +// ItemPage is one page of a Session's root Items. +type ItemPage struct { + Items []v1.Item + HasMore bool +} + +// ItemReader reads a Session's root Items. +type ItemReader interface { + // ListItems returns one page of the visible Session's root Items in + // first-observation order, ascending or descending. A limit outside 1..100 + // is ErrInvalidInput. A cursor that is not an Item of the Session, + // including a malformed one, is ErrItemCursor. + ListItems(ctx context.Context, tenantID, sessionID, cursor string, limit int, ascending bool) (ItemPage, error) +} diff --git a/services/core/internal/sessions/reader.go b/services/core/internal/sessions/reader.go new file mode 100644 index 000000000..17b8357c0 --- /dev/null +++ b/services/core/internal/sessions/reader.go @@ -0,0 +1,9 @@ +package sessions + +// Reader reads a Session's public resources, one family per line. Callers use +// it directly; no use case forwards a read. +type Reader interface { + ArtifactReader + ItemReader + SubagentReader +} diff --git a/services/core/internal/sessions/reads.go b/services/core/internal/sessions/reads.go index 522012181..f8fb70a74 100644 --- a/services/core/internal/sessions/reads.go +++ b/services/core/internal/sessions/reads.go @@ -1,30 +1,6 @@ package sessions -import ( - "time" - - v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" -) - -type ItemPage struct { - Items []v1.Item - HasMore bool -} - -type Artifact struct { - ID string - SessionID string - TurnID string - EnvironmentID string - Path string - SizeBytes int64 - CreatedAt time.Time -} - -type ArtifactPage struct { - Artifacts []Artifact - NextCursor string -} +import "time" // ItemDiagnosticTiming records Core database receipt and settlement, never native // execution duration. Historical terminal Items can have unknown settlement. diff --git a/services/core/internal/sessions/service.go b/services/core/internal/sessions/service.go new file mode 100644 index 000000000..823a52804 --- /dev/null +++ b/services/core/internal/sessions/service.go @@ -0,0 +1,20 @@ +package sessions + +import "errors" + +// Service runs the pooled Session use cases: those that need no execution +// fencing, even when only execution calls them. Reads go through Reader. +type Service struct{ storage Storage } + +// NewService builds the Session use cases on storage. +func NewService(storage Storage) (*Service, error) { + if storage == nil { + return nil, errors.New("sessions: storage is required") + } + return &Service{storage: storage}, nil +} + +// Storage persists the pooled Session use cases, one family per line. +type Storage interface { + ArtifactStorage +} diff --git a/services/core/internal/sessions/subagents.go b/services/core/internal/sessions/subagents.go new file mode 100644 index 000000000..ef19b6537 --- /dev/null +++ b/services/core/internal/sessions/subagents.go @@ -0,0 +1,32 @@ +package sessions + +import ( + "context" + + v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1" +) + +// SubagentReader reads the Subagents a visible Session observed, with their +// Turns and Items. A Subagent, Turn or Item outside the named Session or +// Subagent is ErrNotFound. A list limit outside 1..100 is ErrInvalidInput. A +// Subagent or Subagent Turn cursor outside its list, including a malformed +// one, is ErrResourceCursor; an Item cursor is ErrItemCursor. +type SubagentReader interface { + // GetSubagent returns one Subagent of the Session, including a nested or + // closed one. + GetSubagent(ctx context.Context, tenantID, sessionID, subagentID string) (v1.Subagent, error) + // ListSubagents returns one page of the Session's Subagents in opening + // order. + ListSubagents(ctx context.Context, tenantID, sessionID, cursor string, limit int, ascending bool) (v1.SubagentList, error) + // ListSubagentItems returns one page of the Subagent's Items. + ListSubagentItems(ctx context.Context, tenantID, sessionID, subagentID, cursor string, limit int, ascending bool) (v1.ItemList, error) + // GetSubagentTurn returns one of the Subagent's Turns. Its agent_id is the + // Session's Agent ID and its subagent_id names the child. + GetSubagentTurn(ctx context.Context, tenantID, sessionID, subagentID, turnID string) (v1.Turn, error) + // ListSubagentTurns returns one page of the Subagent's Turns in creation + // order. + ListSubagentTurns(ctx context.Context, tenantID, sessionID, subagentID, cursor string, limit int, ascending bool) (v1.TurnList, error) + // ListSubagentTurnItems returns one page of the Items of one of the + // Subagent's Turns. An empty Turn ID is ErrInvalidInput. + ListSubagentTurnItems(ctx context.Context, tenantID, sessionID, subagentID, turnID, cursor string, limit int, ascending bool) (v1.ItemList, error) +} diff --git a/services/core/internal/store/admin_delete_audit_test.go b/services/core/internal/store/admin_delete_audit_test.go index 6aa52cd03..8650556f2 100644 --- a/services/core/internal/store/admin_delete_audit_test.go +++ b/services/core/internal/store/admin_delete_audit_test.go @@ -152,11 +152,11 @@ func prepareAdminHistoryDelete(t *testing.T, s *Store, name string) (string, res tenant, session, environment, turn := artifactTurn(t, s, "self_hosted") body := bytes.Repeat([]byte("admin-private-body"), 500) archive := artifactArchive(t, map[string][]byte{"outputs/private.txt": body, "outputs/other.txt": []byte("second body")}) - if err := s.StageTurnArtifacts(t.Context(), tenant, session, turn, environment, bytes.NewReader(archive)); err != nil { + if err := stageTurnArtifacts(t.Context(), s, tenant, session, turn, environment, bytes.NewReader(archive)); err != nil { t.Fatal(err) } transition(t, s, tenant, session, turn, sessions.TurnInProgress, sessions.TurnCompleted) - page, err := s.ListSessionArtifacts(t.Context(), tenant, session, "", "", 100, true) + page, err := sessionAdapter(s).ListSessionArtifacts(t.Context(), tenant, session, "", "", 100, true) if err != nil || len(page.Artifacts) != 2 { t.Fatal("artifact fixture", err) } @@ -169,7 +169,7 @@ func prepareAdminHistoryDelete(t *testing.T, s *Store, name string) (string, res if artifact.Path == "/workspace/outputs/other.txt" { want = []byte("second body") } - if err := s.ReadSessionArtifact(t.Context(), tenant, session, artifact.ID, func(_ sessions.Artifact, r io.Reader) error { + if err := sessionAdapter(s).ReadSessionArtifact(t.Context(), tenant, session, artifact.ID, func(_ sessions.Artifact, r io.Reader) error { got, err := io.ReadAll(r) if !bytes.Equal(got, want) { t.Error("artifact large-object bytes were not restored") @@ -185,7 +185,7 @@ func prepareAdminHistoryDelete(t *testing.T, s *Store, name string) (string, res } artifact := page.Artifacts[0] return tenant, resourceAuditMutation{action: "delete", kind: "artifact", parent: session, run: func(ctx context.Context) (string, error) { - return artifact.ID, s.DeleteSessionArtifact(ctx, tenant, session, artifact.ID) + return artifact.ID, sessionAdapter(s).DeleteSessionArtifact(ctx, tenant, session, artifact.ID) }}, verify, 1 } @@ -196,7 +196,7 @@ func assertAdminDeletedResource(t *testing.T, s *Store, tenant string, mutation case "session": _, err = s.GetSession(t.Context(), tenant, id) case "artifact": - _, err = s.GetSessionArtifact(t.Context(), tenant, mutation.parent, id) + _, err = sessionAdapter(s).GetSessionArtifact(t.Context(), tenant, mutation.parent, id) default: t.Fatal("unsupported delete fixture") } diff --git a/services/core/internal/store/admin_session_archive_test.go b/services/core/internal/store/admin_session_archive_test.go index 78396305a..10727b4cd 100644 --- a/services/core/internal/store/admin_session_archive_test.go +++ b/services/core/internal/store/admin_session_archive_test.go @@ -140,7 +140,7 @@ func TestManagedSessionArchiveRetainsHistoryAndSettledResources(t *testing.T) { t.Fatal(err) } body := []byte("retained artifact") - if err := s.StageTurnArtifacts(t.Context(), tenant, session.ID, input.TurnID, session.Environment.ID, bytes.NewReader(artifactArchive(t, map[string][]byte{"outputs/result.txt": body}))); err != nil { + if err := stageTurnArtifacts(t.Context(), s, tenant, session.ID, input.TurnID, session.Environment.ID, bytes.NewReader(artifactArchive(t, map[string][]byte{"outputs/result.txt": body}))); err != nil { t.Fatal(err) } transition(t, w, tenant, session.ID, input.TurnID, sessions.TurnInProgress, sessions.TurnCompleted) @@ -180,11 +180,11 @@ func TestManagedSessionArchiveRetainsHistoryAndSettledResources(t *testing.T) { if result, err := s.GetManagedSessionArchive(t.Context(), tenant, session.ID); err != nil || result.State != "released" { t.Fatal("release not reflected", result, err) } - page, err := s.ListSessionArtifacts(t.Context(), tenant, session.ID, "", "", 100, true) + page, err := sessionAdapter(s).ListSessionArtifacts(t.Context(), tenant, session.ID, "", "", 100, true) if err != nil || len(page.Artifacts) != 1 { t.Fatal(page, err) } - if err := s.ReadSessionArtifact(t.Context(), tenant, session.ID, page.Artifacts[0].ID, func(_ sessions.Artifact, r io.Reader) error { + if err := sessionAdapter(s).ReadSessionArtifact(t.Context(), tenant, session.ID, page.Artifacts[0].ID, func(_ sessions.Artifact, r io.Reader) error { got, err := io.ReadAll(r) if !bytes.Equal(got, body) { t.Error("archive damaged published artifact bytes") diff --git a/services/core/internal/store/archive_cancellation_test.go b/services/core/internal/store/archive_cancellation_test.go index 3545bd280..93aeb7fb5 100644 --- a/services/core/internal/store/archive_cancellation_test.go +++ b/services/core/internal/store/archive_cancellation_test.go @@ -100,7 +100,11 @@ func TestArchiveWaitingCancellationReceipts(t *testing.T) { t.Fatal(err) } t.Cleanup(func() { conn.Close() }) - h := &dispatchHarness{t: t, s: s, db: db, lease: leased.Lease, tenant: project.TenantID, session: session, conn: conn, registry: registry, d: &execution.Dispatcher{Store: writer, Registry: registry, Observer: modelconfigurationpg.New(pgunit.NewPool(db.pool), db.cipher)}} + sessionStore, sessionService, err := fixtureSessions(db) + if err != nil { + t.Fatal(err) + } + h := &dispatchHarness{t: t, s: s, db: db, lease: leased.Lease, tenant: project.TenantID, session: session, conn: conn, registry: registry, d: &execution.Dispatcher{Store: writer, Registry: registry, Observer: modelconfigurationpg.New(pgunit.NewPool(db.pool), db.cipher), Sessions: sessionService, SessionsReader: sessionStore}} capabilities := workerEnvironmentCapabilities() capabilities.FunctionTools = proto.CapabilitySupported h.write("", proto.TypeHeartbeat, proto.HeartbeatPayload{SupportedAgentKinds: []proto.SupportedAgentKind{{Kind: "codex", Available: true, Capabilities: capabilities}}}) diff --git a/services/core/internal/store/artifact_capture.go b/services/core/internal/store/artifact_capture.go deleted file mode 100644 index c27754f8c..000000000 --- a/services/core/internal/store/artifact_capture.go +++ /dev/null @@ -1,125 +0,0 @@ -package store - -import ( - "archive/tar" - "context" - "encoding/json" - "io" - "io/fs" - "strings" - - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/db/sqlc" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/sessionpg" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" - "github.com/google/uuid" - "github.com/jackc/pgx/v5" - "github.com/jackc/pgx/v5/pgtype" -) - -const MaxArtifactBytes int64 = 200 << 20 -const MaxArtifactBatchBytes int64 = 500 << 20 - -// StageTurnArtifacts stores a complete export privately without locking admission during transfer. -func (s *Store) StageTurnArtifacts(ctx context.Context, tenantID, sessionID, turnID, environmentID string, input io.Reader) error { - lookup, err := sessionpg.TurnLookup(tenantID, sessionID, turnID) - if err != nil { - return err - } - environment, err := parseID(environmentID) - if err != nil || input == nil { - return sessions.ErrInvalidInput - } - // Authorize before reading caller-controlled bytes or allocating storage. - owned, err := s.GetSessionEnvironment(ctx, tenantID, sessionID) - if err != nil { - return err - } - if owned.ID != environmentID { - return sessions.ErrNotFound - } - var configuration struct { - Type string `json:"type"` - } - if err := json.Unmarshal(owned.Configuration, &configuration); err != nil { - return err - } - if configuration.Type != "openai_hosted" && configuration.Type != "self_hosted" { - return sessions.ErrInvalidInput - } - return s.pooled.Transaction(ctx, func(ctx context.Context, tx pgx.Tx) error { - rows, err := captureArtifactArchive(ctx, tx, input) - if err != nil { - return err - } - q := s.queries.WithTx(tx) - locked, err := sessionpg.LockSession(ctx, q, lookup.TenantID, lookup.SessionID) - if err != nil { - return err - } - if err := locked.Public(); err != nil { - return err - } - turn, err := q.GetTurn(ctx, lookup) - if err != nil { - return err - } - if turn.Status != sessions.TurnInProgress || turn.CancelRequestedAt.Valid { - return sessions.ErrTurnConflict - } - for _, row := range rows { - row.SessionID, row.TurnID, row.EnvironmentID = lookup.SessionID, lookup.ID, environment - if err := q.StageSessionArtifact(ctx, row); err != nil { - return err - } - } - return nil - }) -} - -func captureArtifactArchive(ctx context.Context, tx pgx.Tx, input io.Reader) ([]sqlc.StageSessionArtifactParams, error) { - archive := tar.NewReader(input) - rows := make([]sqlc.StageSessionArtifactParams, 0) - seen := make(map[string]bool) - var total int64 - for { - header, err := archive.Next() - if err == io.EOF { - break - } - if err != nil { - return nil, err - } - if header.Typeflag != tar.TypeReg || !strings.HasPrefix(header.Name, "outputs/") || !fs.ValidPath(header.Name) || strings.ContainsAny(header.Name, "\\\x00\r\n") || len(header.Name) > 4096 || header.Size < 0 || header.Size > MaxArtifactBytes || header.Size > MaxArtifactBatchBytes-total || len(rows) >= 4096 || seen[header.Name] { - return nil, sessions.ErrInvalidInput - } - seen[header.Name] = true - total += header.Size - writer, err := pgunit.CreateLargeObject(ctx, tx) - if err != nil { - return nil, err - } - if _, err := io.CopyN(writer, archive, header.Size); err != nil { - return nil, err - } - body, err := writer.Close() - if err != nil { - return nil, err - } - rows = append(rows, sqlc.StageSessionArtifactParams{ID: pgtype.UUID{Bytes: uuid.New(), Valid: true}, Path: "/workspace/" + header.Name, SizeBytes: body.Size, BodyOid: pgtype.Uint32{Uint32: body.OID, Valid: true}, Sha256: body.SHA256}) - } - // Require transport EOF after the archive trailer, including confirmed helper exit. - padding, err := io.ReadAll(io.LimitReader(input, 32769)) - if err != nil { - return nil, err - } - if len(padding) > 32768 { - return nil, sessions.ErrInvalidInput - } - for _, b := range padding { - if b != 0 { - return nil, sessions.ErrInvalidInput - } - } - return rows, nil -} diff --git a/services/core/internal/store/command_output_test.go b/services/core/internal/store/command_output_test.go index 08c87b0eb..ead53987f 100644 --- a/services/core/internal/store/command_output_test.go +++ b/services/core/internal/store/command_output_test.go @@ -54,7 +54,7 @@ func TestCommandOutputCommitsFragmentsSnapshotsAndRecovery(t *testing.T) { if before != after { t.Fatal("rollback published output") } - page, err := store.New(pool).ListItems(ctx, tenant, session.ID, "", 100, true) + page, err := sessionReads(pool).ListItems(ctx, tenant, session.ID, "", 100, true) if err != nil || len(page.Items) != 2 { t.Fatalf("read draft: %+v %v", page, err) } @@ -76,7 +76,7 @@ func TestCommandOutputCommitsFragmentsSnapshotsAndRecovery(t *testing.T) { // Reopening the Store recovers committed Items without creating events. reopened := store.New(pool) before, _ = s.SessionEventCursor(ctx, tenant, session.ID) - page, err = reopened.ListItems(ctx, tenant, session.ID, "", 100, true) + page, err = sessionReads(pool).ListItems(ctx, tenant, session.ID, "", 100, true) if err != nil || len(page.Items) != 3 { t.Fatalf("recovery: %+v %v", page, err) } @@ -145,7 +145,7 @@ func TestExecutionJournalsCommandOutputBeforeCancellation(t *testing.T) { } h.write(input.TurnID, proto.TypeInteractionDecisionAck, proto.InteractionDecisionAckPayload{DeliveryID: cancel.DeliveryID, Applied: true, Outcome: &proto.DonePayload{}}) h.finished(result, sessions.TurnCancelled) - page, err := h.s.ListItems(ctx, h.tenant, h.session.ID, "", 100, true) + page, err := sessionReads(h.db.pool).ListItems(ctx, h.tenant, h.session.ID, "", 100, true) if err != nil || len(page.Items) != 2 || page.Items[1].Status != "incomplete" || page.Items[1].Output != "partial" { t.Fatalf("journal/cancellation lost partial output: %+v %v", page, err) } diff --git a/services/core/internal/store/dispatch_test.go b/services/core/internal/store/dispatch_test.go index 5596580c1..274948591 100644 --- a/services/core/internal/store/dispatch_test.go +++ b/services/core/internal/store/dispatch_test.go @@ -114,7 +114,11 @@ func newDispatchHarnessForSession(t *testing.T, configuration []byte, local bool } time.Sleep(10 * time.Millisecond) } - h.d = &execution.Dispatcher{Store: s, Registry: h.registry, Observer: modelconfigurationpg.New(pgunit.NewPool(db.pool), db.cipher)} + sessionStore, sessionService, err := fixtureSessions(db) + if err != nil { + t.Fatal(err) + } + h.d = &execution.Dispatcher{Store: s, Registry: h.registry, Observer: modelconfigurationpg.New(pgunit.NewPool(db.pool), db.cipher), Sessions: sessionService, SessionsReader: sessionStore} return h } diff --git a/services/core/internal/store/environment_write_audit_test.go b/services/core/internal/store/environment_write_audit_test.go index f681bd6a1..4603cda5b 100644 --- a/services/core/internal/store/environment_write_audit_test.go +++ b/services/core/internal/store/environment_write_audit_test.go @@ -81,11 +81,11 @@ func TestArtifactDeleteWriteAuditAndRollback(t *testing.T) { s, _ := testStore(t) tenant, session, env, turn := artifactTurn(t, s, "self_hosted") archive := artifactArchive(t, map[string][]byte{"outputs/private.txt": []byte("secret-file-body")}) - if err := s.StageTurnArtifacts(t.Context(), tenant, session, turn, env, bytes.NewReader(archive)); err != nil { + if err := stageTurnArtifacts(t.Context(), s, tenant, session, turn, env, bytes.NewReader(archive)); err != nil { t.Fatal(err) } transition(t, s, tenant, session, turn, sessions.TurnInProgress, sessions.TurnCompleted) - page, err := s.ListSessionArtifacts(t.Context(), tenant, session, "", "", 100, true) + page, err := sessionAdapter(s).ListSessionArtifacts(t.Context(), tenant, session, "", "", 100, true) if err != nil || len(page.Artifacts) != 1 { t.Fatal(page, err) } @@ -94,12 +94,12 @@ func TestArtifactDeleteWriteAuditAndRollback(t *testing.T) { if fail { rejectSessionAudit(t, s, ctx) } - err = s.DeleteSessionArtifact(ctx, tenant, session, artifact.ID) + err = sessionAdapter(s).DeleteSessionArtifact(ctx, tenant, session, artifact.ID) if fail { if err == nil { t.Fatal("artifact deletion bypassed audit failure") } - if err := s.ReadSessionArtifact(t.Context(), tenant, session, artifact.ID, func(_ sessions.Artifact, r io.Reader) error { + if err := sessionAdapter(s).ReadSessionArtifact(t.Context(), tenant, session, artifact.ID, func(_ sessions.Artifact, r io.Reader) error { body, err := io.ReadAll(r) if string(body) != "secret-file-body" { t.Error("large object did not roll back") diff --git a/services/core/internal/store/fixture_db_test.go b/services/core/internal/store/fixture_db_test.go index 6cecd4e75..743c9f77a 100644 --- a/services/core/internal/store/fixture_db_test.go +++ b/services/core/internal/store/fixture_db_test.go @@ -43,8 +43,9 @@ func newManagedTestStoreDB(t *testing.T) (*store.Store, fixtureDB) { // startWorker starts the execution Worker as cmd/server does: it acquires the // execution lease on db and hands it, with the execution writer built on it, to // the Worker, which closes it when Run exits. The Worker opens MCP bearer tokens -// through the vaults service on db and records model configuration observations -// through the model configuration adapter on db. +// through the vaults service on db, records model configuration observations +// through the model configuration adapter on db and runs Session use cases +// and reads through the Session service and adapter on db. func startWorker(t testing.TB, ctx context.Context, db fixtureDB, dispatcher *execution.Dispatcher) *execution.Worker { t.Helper() worker, err := startWorkerErr(ctx, db, dispatcher) @@ -60,6 +61,10 @@ func startWorkerErr(ctx context.Context, db fixtureDB, dispatcher *execution.Dis if err != nil { return nil, err } + sessionStore, sessionService, err := fixtureSessions(db) + if err != nil { + return nil, err + } lease, err := pgunit.AcquireLease(ctx, db.pool) if err != nil { return nil, err @@ -72,6 +77,8 @@ func startWorkerErr(ctx context.Context, db fixtureDB, dispatcher *execution.Dis owned.Credentials = credentials owned.Observer = modelconfigurationpg.New(pgunit.NewPool(db.pool), db.cipher) owned.Deployment = deployments + owned.Sessions = sessionService + owned.SessionsReader = sessionStore return execution.StartWorker(ctx, &owned, execution.Owner{ Lease: lease, Store: store.NewExecution(dispatcher.Store, lease), diff --git a/services/core/internal/store/function_execution_native_test.go b/services/core/internal/store/function_execution_native_test.go index 1ee45d958..1e71b44fb 100644 --- a/services/core/internal/store/function_execution_native_test.go +++ b/services/core/internal/store/function_execution_native_test.go @@ -53,7 +53,7 @@ func TestNativeFunctionExecutionPersistsCallsResultsAndContinuity(t *testing.T) if err != nil || !saved.Applied { t.Fatal(saved, err) } - page, err := h.s.ListItems(ctx, h.tenant, h.session.ID, "", 100, true) + page, err := sessionReads(h.db.pool).ListItems(ctx, h.tenant, h.session.ID, "", 100, true) if err != nil { t.Fatal(err) } diff --git a/services/core/internal/store/function_item_events_test.go b/services/core/internal/store/function_item_events_test.go index e5b601950..7d8d63597 100644 --- a/services/core/internal/store/function_item_events_test.go +++ b/services/core/internal/store/function_item_events_test.go @@ -40,7 +40,7 @@ func TestFunctionResultEventsAreInputs(t *testing.T) { t.Fatal("function result consumed an output index", event) } } - page, err := s.ListItems(t.Context(), tenant, session.ID, "", 100, true) + page, err := sessionAdapter(s).ListItems(t.Context(), tenant, session.ID, "", 100, true) if err != nil || results != 1 { t.Fatal(page, results, err) } @@ -96,7 +96,7 @@ func TestFunctionResultItemsRetainSubmittedFields(t *testing.T) { } } } - page, err := s.ListItems(t.Context(), tenant, session.ID, "", 100, true) + page, err := sessionAdapter(s).ListItems(t.Context(), tenant, session.ID, "", 100, true) if err != nil { t.Fatal(err) } diff --git a/services/core/internal/store/item_order_test.go b/services/core/internal/store/item_order_test.go index 60e4e3ee3..7f18caa6f 100644 --- a/services/core/internal/store/item_order_test.go +++ b/services/core/internal/store/item_order_test.go @@ -30,7 +30,7 @@ func TestItemObservationOrderSurvivesTiesUpdatesRetriesAndRecovery(t *testing.T) if err != nil { t.Fatal(err) } - page, err := s.ListItems(ctx, tenant, session.ID, "", 100, true) + page, err := sessionReads(pool).ListItems(ctx, tenant, session.ID, "", 100, true) if err != nil || len(page.Items) != 1 { t.Fatal(page, err) } @@ -54,7 +54,7 @@ func TestItemObservationOrderSurvivesTiesUpdatesRetriesAndRecovery(t *testing.T) if _, err = s.SubmitMessage(ctx, tenant, session.ID, "steer", json.RawMessage(`{"text":"continue"}`)); err != nil { t.Fatal(err) } - page, err = s.ListItems(ctx, tenant, session.ID, "", 100, true) + page, err = sessionReads(pool).ListItems(ctx, tenant, session.ID, "", 100, true) if err != nil || len(page.Items) != 5 { t.Fatal(page, err) } @@ -87,7 +87,7 @@ func TestItemObservationOrderSurvivesTiesUpdatesRetriesAndRecovery(t *testing.T) var got []string cursor := "" for { - page, err := store.New(pool).ListItems(ctx, tenant, session.ID, cursor, 2, asc) + page, err := sessionReads(pool).ListItems(ctx, tenant, session.ID, cursor, 2, asc) if err != nil { t.Fatal(err) } diff --git a/services/core/internal/store/item_reads_test.go b/services/core/internal/store/item_reads_test.go index 1f2bdf2b1..ac5fc8767 100644 --- a/services/core/internal/store/item_reads_test.go +++ b/services/core/internal/store/item_reads_test.go @@ -46,7 +46,7 @@ func TestItemsRecoverSnapshotsPartialResultsPaginationAndIsolation(t *testing.T) t.Fatal(err) } } - page, err := s.ListItems(ctx, tenant, session.ID, "", 100, true) + page, err := sessionReads(pool).ListItems(ctx, tenant, session.ID, "", 100, true) if err != nil { t.Fatal(err) } @@ -60,7 +60,7 @@ func TestItemsRecoverSnapshotsPartialResultsPaginationAndIsolation(t *testing.T) if err != nil { t.Fatal(err) } - reopened := store.New(pool) + reopened := sessionReads(pool) page, err = reopened.ListItems(ctx, tenant, session.ID, "", 100, true) if err != nil { t.Fatal(err) @@ -104,13 +104,13 @@ func TestItemsRecoverSnapshotsPartialResultsPaginationAndIsolation(t *testing.T) } other, _ := s.CreateSession(ctx, tenant, sessions.CreateSession{Creator: store.FixtureCreator(), Engine: "codex", IdempotencyKey: "other"}) // A foreign parent is not found before the cursor is read. - if _, err = s.ListItems(ctx, uuid.NewString(), session.ID, page.Items[0].ID, 20, true); !errors.Is(err, sessions.ErrNotFound) { + if _, err = sessionReads(pool).ListItems(ctx, uuid.NewString(), session.ID, page.Items[0].ID, 20, true); !errors.Is(err, sessions.ErrNotFound) { t.Fatal(err) } // Another Session's Item is an invalid cursor here, like a missing or malformed one. for _, cursor := range []string{page.Items[0].ID, uuid.NewString(), "not-a-uuid"} { var invalid *sessions.CursorError - if _, err = s.ListItems(ctx, tenant, other.ID, cursor, 20, true); !errors.As(err, &invalid) || invalid.Message != "Invalid session item ID in `after`" { + if _, err = sessionReads(pool).ListItems(ctx, tenant, other.ID, cursor, 20, true); !errors.As(err, &invalid) || invalid.Message != "Invalid session item ID in `after`" { t.Fatal(cursor, err) } } @@ -118,7 +118,7 @@ func TestItemsRecoverSnapshotsPartialResultsPaginationAndIsolation(t *testing.T) func TestItemProjectionFailureRollsBackJournalAndAggregateRecovers(t *testing.T) { ctx := context.Background() - s, _ := store.NewTestStore(t) + s, pool := store.NewTestStore(t) tenant := uuid.NewString() session, _ := s.CreateSession(ctx, tenant, sessions.CreateSession{Creator: store.FixtureCreator(), Engine: "codex", IdempotencyKey: "legacy"}) input, err := s.SubmitMessage(ctx, tenant, session.ID, "input", json.RawMessage(`{"text":"test"}`)) @@ -137,7 +137,7 @@ func TestItemProjectionFailureRollsBackJournalAndAggregateRecovers(t *testing.T) if err != nil || len(events) != 0 { t.Fatal(events, err) } - page, err := s.ListItems(ctx, tenant, session.ID, "", 100, true) + page, err := sessionReads(pool).ListItems(ctx, tenant, session.ID, "", 100, true) if err != nil || len(page.Items) != 1 { t.Fatal(page, err) } @@ -145,7 +145,7 @@ func TestItemProjectionFailureRollsBackJournalAndAggregateRecovers(t *testing.T) if err != nil { t.Fatal(err) } - current, err := s.ListItems(ctx, tenant, session.ID, "", 100, true) + current, err := sessionReads(pool).ListItems(ctx, tenant, session.ID, "", 100, true) if err != nil || len(current.Items) != 2 || *current.Items[1].Content[0].Text != "legacy answer" { t.Fatal(current, err) } @@ -175,7 +175,7 @@ func TestReceiptOnlyTextRecoversWithoutInventingCompletion(t *testing.T) { if err != nil { t.Fatal(err) } - page, err := store.New(pool).ListItems(ctx, tenant, session.ID, "", 100, true) + page, err := sessionReads(pool).ListItems(ctx, tenant, session.ID, "", 100, true) if err != nil || len(page.Items) != 2 || page.Items[1].Status != "incomplete" || *page.Items[1].Content[0].Text != "retained cancellation text" { t.Fatal(page, err) } @@ -212,7 +212,7 @@ func TestLegacyFailureRetainsPartialAnswerAcrossRecovery(t *testing.T) { if err != nil { t.Fatal(err) } - page, err := store.New(pool).ListItems(ctx, tenant, session.ID, "", 100, true) + page, err := sessionReads(pool).ListItems(ctx, tenant, session.ID, "", 100, true) if err != nil || len(page.Items) != 4 { t.Fatal(page, err) } diff --git a/services/core/internal/store/item_serialization_test.go b/services/core/internal/store/item_serialization_test.go index 8a04344b3..1378b9eeb 100644 --- a/services/core/internal/store/item_serialization_test.go +++ b/services/core/internal/store/item_serialization_test.go @@ -115,7 +115,7 @@ func TestAssistantMessageEventsFollowOfficialSequence(t *testing.T) { if !reflect.DeepEqual(kinds, want) || !reflect.DeepEqual(deltas, test.deltas) || done != test.final { t.Fatalf("sequence %v deltas %q done %q", kinds, deltas, done) } - page, err := s.ListItems(t.Context(), tenant, session.ID, "", 100, true) + page, err := sessionAdapter(s).ListItems(t.Context(), tenant, session.ID, "", 100, true) if err != nil || len(page.Items) != 2 || *page.Items[1].Content[0].Text != test.final { t.Fatalf("stored answer changed: %+v %v", page, err) } @@ -144,7 +144,7 @@ func TestInputItemEventsCarryNullOutputIndexAndPhase(t *testing.T) { t.Fatalf("user item.added: %v %s", event.fields, event.fields["item"]) } } - page, err := s.ListItems(t.Context(), tenant, session.ID, "", 100, true) + page, err := sessionAdapter(s).ListItems(t.Context(), tenant, session.ID, "", 100, true) if err != nil || added != 1 || len(page.Items) != 1 { t.Fatal(page, added, err) } diff --git a/services/core/internal/store/list_cursor_public_test.go b/services/core/internal/store/list_cursor_public_test.go index 3d3a52026..c14a5d669 100644 --- a/services/core/internal/store/list_cursor_public_test.go +++ b/services/core/internal/store/list_cursor_public_test.go @@ -34,7 +34,7 @@ type cursorFixture struct { file string } -func seedCursorFixture(t *testing.T, s *store.Store, writer *store.Store, skillService *skills.Service, client pathIDClient, token, tenant, label string) cursorFixture { +func seedCursorFixture(t *testing.T, s *store.Store, writer *store.Store, skillService *skills.Service, artifacts *sessions.Service, client pathIDClient, token, tenant, label string) cursorFixture { t.Helper() ctx := t.Context() var f cursorFixture @@ -76,10 +76,10 @@ func seedCursorFixture(t *testing.T, s *store.Store, writer *store.Store, skillS artifactSession, environment := hostedArtifactSession(t, s, tenant, label+"-artifacts") f.artifactSession = artifactSession - f.artifactTurn = completeArtifactTurn(t, s, tenant, artifactSession, environment, label+"-artifact-turn", map[string]string{"a.txt": "alpha", "c.txt": "charlie"}) + f.artifactTurn = completeArtifactTurn(t, s, artifacts, tenant, artifactSession, environment, label+"-artifact-turn", map[string]string{"a.txt": "alpha", "c.txt": "charlie"}) f.artifact = first("/v1/agents/sessions/" + artifactSession + "/artifacts") otherArtifactSession, otherEnvironment := hostedArtifactSession(t, s, tenant, label+"-other-artifacts") - completeArtifactTurn(t, s, tenant, otherArtifactSession, otherEnvironment, label+"-other-artifact-turn", map[string]string{"b.txt": "bravo"}) + completeArtifactTurn(t, s, artifacts, tenant, otherArtifactSession, otherEnvironment, label+"-other-artifact-turn", map[string]string{"b.txt": "bravo"}) f.otherArtifact = first("/v1/agents/sessions/" + otherArtifactSession + "/artifacts") // Subagent history is seeded through the execution lease, as a daemon would. @@ -137,9 +137,10 @@ func seedCursorFixture(t *testing.T, s *store.Store, writer *store.Store, skillS f.subSession, f.rootTurn = seedSubagents(label + "-subagents") f.rootItem = first("/v1/agents/sessions/" + f.subSession + "/items") f.child, f.sibling = subagent(f.subSession, "child"), subagent(f.subSession, "sibling") - childTurns, err := s.ListSubagentTurns(ctx, tenant, f.subSession, f.child, "", 10, true) - if err != nil || len(childTurns.Data) != 2 { - t.Fatal("fixture child Turns", childTurns, err) + status, raw := client.do(token, http.MethodGet, "/v1/agents/sessions/"+f.subSession+"/subagents/"+f.child+"/turns?order=asc", "", nil) + var childTurns struct{ Data []struct{ ID string } } + if status != http.StatusOK || json.Unmarshal([]byte(raw), &childTurns) != nil || len(childTurns.Data) != 2 { + t.Fatal("fixture child Turns", status, raw) } f.childTurn, f.laterChildTurn = childTurns.Data[0].ID, childTurns.Data[1].ID f.childItem = first("/v1/agents/sessions/" + f.subSession + "/subagents/" + f.child + "/turns/" + f.childTurn + "/items") @@ -199,7 +200,7 @@ func seedCursorFixture(t *testing.T, s *store.Store, writer *store.Store, skillS if err := form.Close(); err != nil { t.Fatal(err) } - status, raw := client.do(token, http.MethodPost, "/v1/files", form.FormDataContentType(), upload.Bytes()) + status, raw = client.do(token, http.MethodPost, "/v1/files", form.FormDataContentType(), upload.Bytes()) var file struct{ ID string } if status != http.StatusOK || json.Unmarshal([]byte(raw), &file) != nil || file.ID == "" { t.Fatalf("fixture File: %d %s", status, raw) @@ -240,8 +241,12 @@ func TestListCursorErrorsPostgres(t *testing.T) { client := pathIDClient{t: t, server: server} writer := executionOwner(t, db, s).Store skillService := store.SkillService(t, db.pool, db.cipher) - a := seedCursorFixture(t, s, writer, skillService, client, owner, ownerTenant, "a") - b := seedCursorFixture(t, s, writer, skillService, client, foreign, foreignTenant, "b") + _, sessionService, err := fixtureSessions(db) + if err != nil { + t.Fatal(err) + } + a := seedCursorFixture(t, s, writer, skillService, sessionService, client, owner, ownerTenant, "a") + b := seedCursorFixture(t, s, writer, skillService, sessionService, client, foreign, foreignTenant, "b") text := func(value string) *string { return &value } var ( diff --git a/services/core/internal/store/list_cursors.go b/services/core/internal/store/list_cursors.go deleted file mode 100644 index 270a634c0..000000000 --- a/services/core/internal/store/list_cursors.go +++ /dev/null @@ -1,16 +0,0 @@ -package store - -import ( - "errors" - - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" -) - -// unresolvedCursor replaces a missing cursor resource with the list's cursor -// error and keeps every other failure. -func unresolvedCursor(err, cursor error) error { - if errors.Is(err, sessions.ErrNotFound) { - return cursor - } - return err -} diff --git a/services/core/internal/store/local_artifact_export_test.go b/services/core/internal/store/local_artifact_export_test.go index d505e062e..cc45e09e5 100644 --- a/services/core/internal/store/local_artifact_export_test.go +++ b/services/core/internal/store/local_artifact_export_test.go @@ -41,7 +41,7 @@ func completeLocalArtifactExport(t *testing.T, h *dispatchHarness, worker *execu if next.DecodePayload(&export) != nil || export.Step != "next" || export.Offset != int64(data.Len()) { t.Fatal("export did not await native completion") } - page, err := h.s.ListSessionArtifacts(t.Context(), h.tenant, h.session.ID, "", "", 20, false) + page, err := sessionReads(h.db.pool).ListSessionArtifacts(t.Context(), h.tenant, h.session.ID, "", "", 20, false) if err != nil || len(page.Artifacts) != 0 { t.Fatal("capture published before native completion", err) } diff --git a/services/core/internal/store/local_environment_worker_test.go b/services/core/internal/store/local_environment_worker_test.go index e81318b9d..8a029669c 100644 --- a/services/core/internal/store/local_environment_worker_test.go +++ b/services/core/internal/store/local_environment_worker_test.go @@ -152,7 +152,7 @@ func TestLocalEnvironmentWorkerSchedulesPreparationWithoutRemoteResolver(t *test if err != nil || bound.Device.EnvironmentID != environment.ID || bound.NativeSessionID != "local-native-history" { t.Fatal("local native identity was not retained", err) } - artifacts, err := h.s.ListSessionArtifacts(t.Context(), h.tenant, h.session.ID, environment.ID, "", 20, false) + artifacts, err := sessionReads(h.db.pool).ListSessionArtifacts(t.Context(), h.tenant, h.session.ID, environment.ID, "", 20, false) if err != nil || len(artifacts.Artifacts) != 1 || artifacts.Artifacts[0].Path != "/workspace/outputs/result.bin" || artifacts.Artifacts[0].TurnID != start.RunID || artifacts.Artifacts[0].SizeBytes != 3 { t.Fatalf("completed turn did not publish output: %+v %v", artifacts, err) } diff --git a/services/core/internal/store/native_environment_test.go b/services/core/internal/store/native_environment_test.go index e83773c85..69f6f3856 100644 --- a/services/core/internal/store/native_environment_test.go +++ b/services/core/internal/store/native_environment_test.go @@ -124,7 +124,7 @@ func TestNativeNoExecutionEnvironment(t *testing.T) { if _, err := os.Stat(marker); !os.IsNotExist(err) { t.Fatalf("forbidden command may have executed: %v", err) } - page, err := h.s.ListItems(ctx, h.tenant, h.session.ID, "", 100, true) + page, err := sessionReads(h.db.pool).ListItems(ctx, h.tenant, h.session.ID, "", 100, true) if err != nil { t.Fatal(err) } diff --git a/services/core/internal/store/prepared_dispatch_test.go b/services/core/internal/store/prepared_dispatch_test.go index ce7f87c0d..0e0b31158 100644 --- a/services/core/internal/store/prepared_dispatch_test.go +++ b/services/core/internal/store/prepared_dispatch_test.go @@ -86,7 +86,7 @@ func TestPreparedDispatchPromotesOriginalBatchAndPersistsCompletion(t *testing.T if err != nil || session.LastTurn != nil { t.Fatal("preparation created work before readiness", session, err) } - items, err := h.s.ListItems(t.Context(), h.tenant, h.session.ID, "", 100, true) + items, err := sessionReads(h.db.pool).ListItems(t.Context(), h.tenant, h.session.ID, "", 100, true) if err != nil || len(items.Items) != 0 { t.Fatal("preparation published input history", items, err) } diff --git a/services/core/internal/store/public_execution_test.go b/services/core/internal/store/public_execution_test.go index b34755d39..4e10aa83e 100644 --- a/services/core/internal/store/public_execution_test.go +++ b/services/core/internal/store/public_execution_test.go @@ -74,7 +74,7 @@ func TestExecutionWorkerAdmissionBindingAndRecovery(t *testing.T) { } h.write(request.ID, proto.TypeDone, proto.DonePayload{Content: "Answer", Metadata: map[string]any{proto.DoneMetaAgentSessionID: "worker-native"}}) waitTurn(t, h, receipts[0].TurnID, sessions.TurnCompleted) - items, err := h.s.ListItems(ctx, h.tenant, h.session.ID, "", 100, true) + items, err := sessionReads(h.db.pool).ListItems(ctx, h.tenant, h.session.ID, "", 100, true) if err != nil || len(items.Items) != 3 { t.Fatal(items, err) } diff --git a/services/core/internal/store/public_handler_fixture_test.go b/services/core/internal/store/public_handler_fixture_test.go index b28c1c8d6..6300ed783 100644 --- a/services/core/internal/store/public_handler_fixture_test.go +++ b/services/core/internal/store/public_handler_fixture_test.go @@ -37,10 +37,10 @@ const testExecutorURL = "wss://core.example/api/v1/agent-daemon/ws" // publicHandler serves s through api.NewHandler. s backs every area the Store // implements, and db is the database and credential key that built s; the -// audit reads, Agents, Files and Vaults come from db. keys authenticate as Project -// keys and "admin" as the Core key. Metrics, Runtime observation and history, -// and executor connections are strict stand-ins. Execution and Sandboxes stay -// disabled unless configure sets them. +// audit reads, Agents, Files, Vaults, Items, Subagents and Artifacts come from +// db. keys authenticate as Project keys and "admin" as the Core key. Metrics, +// Runtime observation and history, and executor connections are strict +// stand-ins. Execution and Sandboxes stay disabled unless configure sets them. func publicHandler(t testing.TB, s *store.Store, db fixtureDB, keys fixtureKeyResolver, engine string, configure ...func(*api.Dependencies)) (http.Handler, error) { t.Helper() admin, err := api.NewDeploymentAuthenticator([]string{runtimedevice.HashCredential("admin")}) @@ -71,6 +71,10 @@ func publicHandler(t testing.TB, s *store.Store, db fixtureDB, keys fixtureKeyRe return nil, err } projectStore, projectService := fixtureProjects(t, db) + sessionStore, sessionService, err := fixtureSessions(db) + if err != nil { + return nil, err + } deps := api.Dependencies{ Engine: engine, CoreKeys: admin, InstallationBindings: s, Projects: projectService, ProjectsReader: fixtureProjectsReader{Reader: projectStore, keys: keys}, @@ -84,9 +88,11 @@ func publicHandler(t testing.TB, s *store.Store, db fixtureDB, keys fixtureKeyRe SessionCreation: s, SessionEvents: s, Turns: s, - Items: s, - Subagents: s, - Artifacts: s, SessionAdmin: s, Environments: s, Admin: s, AdminAudit: audit, WriteAudit: audit, + Items: sessionStore, + Subagents: sessionStore, + Artifacts: sessionService, + ArtifactsReader: sessionStore, + SessionAdmin: s, Environments: s, Admin: s, AdminAudit: audit, WriteAudit: audit, ExecutorConnections: strict, Metrics: strict, RuntimeObservations: strict, RuntimeHistory: strict, } for _, c := range configure { diff --git a/services/core/internal/store/runtime_idle_clock_test.go b/services/core/internal/store/runtime_idle_clock_test.go index d1315d1e7..22eeb216b 100644 --- a/services/core/internal/store/runtime_idle_clock_test.go +++ b/services/core/internal/store/runtime_idle_clock_test.go @@ -142,7 +142,7 @@ func TestManagedIdleClockIgnoresChildHostSkewAndReplay(t *testing.T) { t.Fatal("replayed child completion reset idle", unchanged, err) } verifyManagedIdleClock(t, s, w, owner, before, after) - page, err := s.ListSubagentTurns(t.Context(), owner.TenantID, owner.SessionID, child, "", 10, true) + page, err := sessionAdapter(s).ListSubagentTurns(t.Context(), owner.TenantID, owner.SessionID, child, "", 10, true) if err != nil || len(page.Data) != 1 { t.Fatal(page, err) } diff --git a/services/core/internal/store/sandbox_deployment_switch_test.go b/services/core/internal/store/sandbox_deployment_switch_test.go index 5d35a2f71..024cf4da6 100644 --- a/services/core/internal/store/sandbox_deployment_switch_test.go +++ b/services/core/internal/store/sandbox_deployment_switch_test.go @@ -323,7 +323,7 @@ func TestSandboxSwitchPreservesReleasedAllocationAndItemHistory(t *testing.T) { if _, err := w.TransitionTurn(t.Context(), tenant, history.ID, input.TurnID, sessions.TurnTransition{ExpectedStatus: sessions.TurnInProgress, Status: sessions.TurnCompleted}); err != nil { t.Fatal(err) } - items, err := s.ListItems(t.Context(), tenant, history.ID, "", 100, true) + items, err := sessionAdapter(s).ListItems(t.Context(), tenant, history.ID, "", 100, true) if err != nil || len(items.Items) != 2 { t.Fatal("history fixture", err) } @@ -338,7 +338,7 @@ func TestSandboxSwitchPreservesReleasedAllocationAndItemHistory(t *testing.T) { if _, err := resetAndSelect(t, w, installation, 1, sandbox.Selection{DeploymentSpec: SandboxDeploymentTestSpec("docker"), Provider: "docker"}); err != nil { t.Fatal(err) } - items, err = s.ListItems(t.Context(), tenant, history.ID, "", 100, true) + items, err = sessionAdapter(s).ListItems(t.Context(), tenant, history.ID, "", 100, true) if err != nil { t.Fatal(err) } diff --git a/services/core/internal/store/self_hosted_cancel_public_test.go b/services/core/internal/store/self_hosted_cancel_public_test.go index e03b3062c..3f33f202e 100644 --- a/services/core/internal/store/self_hosted_cancel_public_test.go +++ b/services/core/internal/store/self_hosted_cancel_public_test.go @@ -144,7 +144,7 @@ func TestSelfHostedCancellationOfficialClient(t *testing.T) { if snapshot(created.ID) != before { t.Fatal("idle cancellation replay or rejected input changed active work") } - itemsBefore, err := s.ListItems(t.Context(), tenant, created.ID, "", 100, true) + itemsBefore, err := sessionReads(db.pool).ListItems(t.Context(), tenant, created.ID, "", 100, true) if err != nil || len(itemsBefore.Items) != 2 { t.Fatal("controlled partial output was not recorded", err) } @@ -158,7 +158,7 @@ func TestSelfHostedCancellationOfficialClient(t *testing.T) { if err != nil || turn.Status != sessions.TurnInProgress || turn.CancelRequestedAt.IsZero() || !turn.CompletedAt.IsZero() { t.Fatal("202 must admit cancellation without fabricating native completion", err) } - itemsAfter, err := s.ListItems(t.Context(), tenant, created.ID, "", 100, true) + itemsAfter, err := sessionReads(db.pool).ListItems(t.Context(), tenant, created.ID, "", 100, true) if err != nil || !reflect.DeepEqual(itemsBefore, itemsAfter) { t.Fatal("cancellation admission changed partial history", err) } diff --git a/services/core/internal/store/session_artifacts.go b/services/core/internal/store/session_artifacts.go deleted file mode 100644 index d0c880770..000000000 --- a/services/core/internal/store/session_artifacts.go +++ /dev/null @@ -1,146 +0,0 @@ -package store - -import ( - "context" - "errors" - "io" - - "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/persistence/postgres/sessionpg" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" - "github.com/google/uuid" - "github.com/jackc/pgx/v5" - "github.com/jackc/pgx/v5/pgtype" -) - -func (s *Store) GetSessionArtifact(ctx context.Context, tenantID, sessionID, artifactID string) (sessions.Artifact, error) { - lookup, err := artifactLookup(tenantID, sessionID, artifactID) - if err != nil { - return sessions.Artifact{}, err - } - row, err := s.queries.GetSessionArtifact(ctx, lookup) - if errors.Is(err, pgx.ErrNoRows) { - return sessions.Artifact{}, sessions.ErrNotFound - } - return artifactFromRow(row), err -} - -func (s *Store) ListSessionArtifacts(ctx context.Context, tenantID, sessionID, environmentID, cursor string, limit int, ascending bool) (sessions.ArtifactPage, error) { - if limit < 1 || limit > 100 { - return sessions.ArtifactPage{}, sessions.ErrInvalidInput - } - if _, err := s.GetSession(ctx, tenantID, sessionID); err != nil { - return sessions.ArtifactPage{}, err - } - tenant, _ := parseID(tenantID) - session, _ := parseID(sessionID) - params := sqlc.ListSessionArtifactsParams{TenantID: tenant, SessionID: session, PageLimit: int32(limit + 1), Ascending: ascending, AfterID: pgtype.UUID{Valid: true}} - if environmentID != "" { - // A malformed filter matches nothing, like another Environment's ID (HE-56). - params.EnvironmentID = pgunit.PathID(environmentID) - } - if cursor != "" { - // Any cursor that is not an Artifact of this Session, including a - // malformed one, is an invalid cursor rather than a missing resource. - after, err := s.GetSessionArtifact(ctx, tenantID, sessionID, cursor) - if errors.Is(err, sessions.ErrNotFound) { - // The Session lookup above is a separate statement: a Session deleted - // since then stays not found. Deleted Sessions never reappear, so an - // existing one here also existed when the cursor was read. - if _, err := s.GetSession(ctx, tenantID, sessionID); err != nil { - return sessions.ArtifactPage{}, err - } - return sessions.ArtifactPage{}, sessions.ErrArtifactCursor - } - if err != nil { - return sessions.ArtifactPage{}, err - } - params.AfterCreated = pgtype.Timestamptz{Time: after.CreatedAt, Valid: true} - params.AfterID, _ = parseID(after.ID) - } - rows, err := s.queries.ListSessionArtifacts(ctx, params) - if err != nil { - return sessions.ArtifactPage{}, err - } - page := sessions.ArtifactPage{Artifacts: make([]sessions.Artifact, 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 { - page.Artifacts = append(page.Artifacts, artifactFromRow(row)) - } - return page, nil -} - -// ReadSessionArtifact keeps an admitted snapshot available across concurrent deletion. -func (s *Store) ReadSessionArtifact(ctx context.Context, tenantID, sessionID, artifactID string, consume func(sessions.Artifact, io.Reader) error) error { - lookup, err := artifactLookup(tenantID, sessionID, artifactID) - if err != nil { - return err - } - if consume == nil { - return sessions.ErrInvalidInput - } - return s.pooled.Snapshot(ctx, func(ctx context.Context, tx pgx.Tx) error { - row, err := s.queries.WithTx(tx).GetSessionArtifact(ctx, lookup) - if errors.Is(err, pgx.ErrNoRows) { - return sessions.ErrNotFound - } - if err != nil { - return err - } - objects := tx.LargeObjects() - body, err := objects.Open(ctx, row.BodyOid.Uint32, pgx.LargeObjectModeRead) - if err != nil { - return err - } - if err := consume(artifactFromRow(row), body); err != nil { - return err - } - return body.Close() - }) -} - -func (s *Store) DeleteSessionArtifact(ctx context.Context, tenantID, sessionID, artifactID string) error { - lookup, err := artifactLookup(tenantID, sessionID, artifactID) - if err != nil { - return err - } - return s.pooled.Transaction(ctx, func(ctx context.Context, tx pgx.Tx) error { - q := s.queries.WithTx(tx) - // Use the same lock order as whole-Session deletion and Turn publication. - locked, err := sessionpg.LockSession(ctx, q, lookup.TenantID, lookup.SessionID) - if err != nil { - return err - } - if err := locked.Public(); err != nil { - return err - } - oid, err := q.DeleteSessionArtifact(ctx, sqlc.DeleteSessionArtifactParams(lookup)) - if errors.Is(err, pgx.ErrNoRows) { - return sessions.ErrNotFound - } - if err != nil { - return err - } - objects := tx.LargeObjects() - if err := objects.Unlink(ctx, oid.Uint32); err != nil { - return err - } - return auditpg.RecordWriteAudit(ctx, q, tenantID, "delete", "artifact", uuid.UUID(lookup.ID.Bytes).String(), uuid.UUID(lookup.SessionID.Bytes).String()) - }) -} - -func artifactLookup(tenantID, sessionID, artifactID string) (sqlc.GetSessionArtifactParams, error) { - ids, err := publicTurnLookup(tenantID, sessionID, artifactID) - return sqlc.GetSessionArtifactParams{TenantID: ids.TenantID, SessionID: ids.SessionID, ID: ids.ID}, err -} - -func artifactFromRow(row sqlc.SessionArtifact) sessions.Artifact { - return sessions.Artifact{ID: uuid.UUID(row.ID.Bytes).String(), SessionID: uuid.UUID(row.SessionID.Bytes).String(), - TurnID: uuid.UUID(row.TurnID.Bytes).String(), EnvironmentID: uuid.UUID(row.EnvironmentID.Bytes).String(), - Path: row.Path, SizeBytes: row.SizeBytes, CreatedAt: row.CreatedAt.Time} -} diff --git a/services/core/internal/store/session_artifacts_public_test.go b/services/core/internal/store/session_artifacts_public_test.go index 9249b2f9f..55c5f54eb 100644 --- a/services/core/internal/store/session_artifacts_public_test.go +++ b/services/core/internal/store/session_artifacts_public_test.go @@ -33,9 +33,9 @@ func hostedArtifactSession(t *testing.T, s *store.Store, tenant, key string) (se return created.ID, created.Environment.ID } -// completeArtifactTurn runs one Turn whose complete outputs tree is captured -// and settled as completed, and returns the Turn ID. -func completeArtifactTurn(t *testing.T, s *store.Store, tenant, session, environment, key string, outputs map[string]string) string { +// completeArtifactTurn runs one Turn whose complete outputs tree is staged +// through artifacts and settled as completed, and returns the Turn ID. +func completeArtifactTurn(t *testing.T, s *store.Store, artifacts *sessions.Service, tenant, session, environment, key string, outputs map[string]string) string { t.Helper() receipt, err := s.SubmitMessage(t.Context(), tenant, session, key, json.RawMessage(`{"input":[{"role":"user","content":[{"type":"input_text","text":"publish outputs"}]}]}`)) if err != nil { @@ -61,7 +61,7 @@ func completeArtifactTurn(t *testing.T, s *store.Store, tenant, session, environ if err := w.Close(); err != nil { t.Fatal(err) } - if err := s.StageTurnArtifacts(t.Context(), tenant, session, receipt.TurnID, environment, &archive); err != nil { + if err := artifacts.StageTurnArtifacts(t.Context(), sessions.StageTurnArtifactsCommand{TenantID: tenant, SessionID: session, TurnID: receipt.TurnID, EnvironmentID: environment, Export: &archive}); err != nil { t.Fatal(err) } transition(sessions.TurnInProgress, sessions.TurnCompleted) @@ -91,14 +91,18 @@ func artifactHTTPServer(t *testing.T, s *store.Store, db fixtureDB) (server *htt // without weakening tenant or Session scoping. func TestSessionArtifactListEnvelopeAndEnvironmentFilterPostgres(t *testing.T) { s, db := newTestStoreDB(t) + _, sessionService, err := fixtureSessions(db) + if err != nil { + t.Fatal(err) + } server, owner, ownerTenant, foreign, foreignTenant := artifactHTTPServer(t, s, db) client := pathIDClient{t: t, server: server} session, environment := hostedArtifactSession(t, s, ownerTenant, "artifact-list") - completeArtifactTurn(t, s, ownerTenant, session, environment, "artifact-list-turn", map[string]string{"a.txt": "alpha", "b.txt": "bravo"}) + completeArtifactTurn(t, s, sessionService, ownerTenant, session, environment, "artifact-list-turn", map[string]string{"a.txt": "alpha", "b.txt": "bravo"}) idle, otherEnvironment := hostedArtifactSession(t, s, ownerTenant, "artifact-idle") foreignSession, foreignEnvironment := hostedArtifactSession(t, s, foreignTenant, "artifact-foreign") - completeArtifactTurn(t, s, foreignTenant, foreignSession, foreignEnvironment, "artifact-foreign-turn", map[string]string{"a.txt": "alpha"}) + completeArtifactTurn(t, s, sessionService, foreignTenant, foreignSession, foreignEnvironment, "artifact-foreign-turn", map[string]string{"a.txt": "alpha"}) type envelope struct { Object *string `json:"object"` @@ -211,25 +215,29 @@ func TestSessionArtifactsOfficialClientPostgres(t *testing.T) { t.Skip("pinned official Python SDK required") } s, db := newTestStoreDB(t) + sessionStore, sessionService, err := fixtureSessions(db) + if err != nil { + t.Fatal(err) + } server, owner, ownerTenant, foreign, _ := artifactHTTPServer(t, s, db) session, environment := hostedArtifactSession(t, s, ownerTenant, "artifact-sdk") outputs := map[string]string{"a.txt": "alpha", "sub/b.txt": "bravo", "empty.txt": ""} - first := completeArtifactTurn(t, s, ownerTenant, session, environment, "artifact-sdk-1", outputs) - page, err := s.ListSessionArtifacts(t.Context(), ownerTenant, session, "", "", 100, true) + first := completeArtifactTurn(t, s, sessionService, ownerTenant, session, environment, "artifact-sdk-1", outputs) + page, err := sessionStore.ListSessionArtifacts(t.Context(), ownerTenant, session, "", "", 100, true) if err != nil { t.Fatal(err) } for _, artifact := range page.Artifacts { if artifact.Path == "/workspace/outputs/a.txt" { - if err := s.DeleteSessionArtifact(t.Context(), ownerTenant, session, artifact.ID); err != nil { + if err := sessionService.DeleteSessionArtifact(t.Context(), sessions.DeleteSessionArtifactCommand{TenantID: ownerTenant, SessionID: session, ArtifactID: artifact.ID}); err != nil { t.Fatal(err) } } } outputs["c.txt"] = "charlie" - second := completeArtifactTurn(t, s, ownerTenant, session, environment, "artifact-sdk-2", outputs) + second := completeArtifactTurn(t, s, sessionService, ownerTenant, session, environment, "artifact-sdk-2", outputs) outputs["sub/b.txt"] = "bravo-v2" - third := completeArtifactTurn(t, s, ownerTenant, session, environment, "artifact-sdk-3", outputs) + third := completeArtifactTurn(t, s, sessionService, ownerTenant, session, environment, "artifact-sdk-3", outputs) hex := func(files map[string]string) map[string]string { out := make(map[string]string, len(files)) for name, body := range files { diff --git a/services/core/internal/store/session_artifacts_test.go b/services/core/internal/store/session_artifacts_test.go index f7cec36fd..f37d5e744 100644 --- a/services/core/internal/store/session_artifacts_test.go +++ b/services/core/internal/store/session_artifacts_test.go @@ -15,6 +15,7 @@ import ( "time" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/db/sqlc" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" "github.com/google/uuid" "github.com/jackc/pgx/v5/pgxpool" @@ -75,15 +76,15 @@ func testSessionArtifactsPublishVersionScopeAndLifetime(t *testing.T, kind strin before := largeObjectCount(t, pool) data := bytes.Repeat([]byte("immutable\x00"), 100000) archive := artifactArchive(t, map[string][]byte{"outputs/a.bin": data, "outputs/nested/empty": {}}) - if err := s.StageTurnArtifacts(t.Context(), tenant, session, turn, environment, bytes.NewReader(archive)); err != nil { + if err := stageTurnArtifacts(t.Context(), s, tenant, session, turn, environment, bytes.NewReader(archive)); err != nil { t.Fatal(err) } - page, err := s.ListSessionArtifacts(t.Context(), tenant, session, "", "", 100, true) + page, err := sessionAdapter(s).ListSessionArtifacts(t.Context(), tenant, session, "", "", 100, true) if err != nil || len(page.Artifacts) != 0 { t.Fatalf("private capture visible: %+v %v", page, err) } completed := transition(t, s, tenant, session, turn, sessions.TurnInProgress, sessions.TurnCompleted) - page, err = s.ListSessionArtifacts(t.Context(), tenant, session, environment, "", 100, true) + page, err = sessionAdapter(s).ListSessionArtifacts(t.Context(), tenant, session, environment, "", 100, true) if err != nil || len(page.Artifacts) != 2 { t.Fatalf("published capture: %+v %v", page, err) } @@ -92,36 +93,36 @@ func testSessionArtifactsPublishVersionScopeAndLifetime(t *testing.T, kind strin t.Fatalf("publication metadata: %+v", a) } foreign := uuid.NewString() - if _, err := s.GetSessionArtifact(t.Context(), foreign, session, a.ID); !errors.Is(err, sessions.ErrNotFound) { + if _, err := sessionAdapter(s).GetSessionArtifact(t.Context(), foreign, session, a.ID); !errors.Is(err, sessions.ErrNotFound) { t.Fatalf("foreign metadata: %v", err) } - if _, err := s.GetSessionArtifact(t.Context(), tenant, uuid.NewString(), a.ID); !errors.Is(err, sessions.ErrNotFound) { + if _, err := sessionAdapter(s).GetSessionArtifact(t.Context(), tenant, uuid.NewString(), a.ID); !errors.Is(err, sessions.ErrNotFound) { t.Fatalf("wrong session metadata: %v", err) } - if err := s.DeleteSessionArtifact(t.Context(), foreign, session, a.ID); !errors.Is(err, sessions.ErrNotFound) { + if err := sessionAdapter(s).DeleteSessionArtifact(t.Context(), foreign, session, a.ID); !errors.Is(err, sessions.ErrNotFound) { t.Fatalf("foreign delete: %v", err) } - if err := s.ReadSessionArtifact(t.Context(), foreign, session, a.ID, func(sessions.Artifact, io.Reader) error { + if err := sessionAdapter(s).ReadSessionArtifact(t.Context(), foreign, session, a.ID, func(sessions.Artifact, io.Reader) error { t.Error("foreign read reached content") return nil }); !errors.Is(err, sessions.ErrNotFound) { t.Fatalf("foreign read: %v", err) } } - if _, err := s.ListSessionArtifacts(t.Context(), uuid.NewString(), session, "", "", 100, false); !errors.Is(err, sessions.ErrNotFound) { + if _, err := sessionAdapter(s).ListSessionArtifacts(t.Context(), uuid.NewString(), session, "", "", 100, false); !errors.Is(err, sessions.ErrNotFound) { t.Fatalf("foreign list: %v", err) } - if empty, err := s.ListSessionArtifacts(t.Context(), tenant, session, uuid.NewString(), "", 100, false); err != nil || len(empty.Artifacts) != 0 { + if empty, err := sessionAdapter(s).ListSessionArtifacts(t.Context(), tenant, session, uuid.NewString(), "", 100, false); err != nil || len(empty.Artifacts) != 0 { t.Fatalf("environment filter: %+v %v", empty, err) } // A later completed Turn publishes another immutable version of the same path. next := submitMessage(t, s, tenant, session, "version-two") transition(t, s, tenant, session, next.TurnID, sessions.TurnQueued, sessions.TurnInProgress) - if err := s.StageTurnArtifacts(t.Context(), tenant, session, next.TurnID, environment, bytes.NewReader(artifactArchive(t, map[string][]byte{"outputs/a.bin": []byte("new")}))); err != nil { + if err := stageTurnArtifacts(t.Context(), s, tenant, session, next.TurnID, environment, bytes.NewReader(artifactArchive(t, map[string][]byte{"outputs/a.bin": []byte("new")}))); err != nil { t.Fatal(err) } transition(t, s, tenant, session, next.TurnID, sessions.TurnInProgress, sessions.TurnCompleted) - all, err := s.ListSessionArtifacts(t.Context(), tenant, session, "", "", 100, true) + all, err := sessionAdapter(s).ListSessionArtifacts(t.Context(), tenant, session, "", "", 100, true) if err != nil || len(all.Artifacts) != 3 { t.Fatal(all, err) } @@ -129,7 +130,7 @@ func testSessionArtifactsPublishVersionScopeAndLifetime(t *testing.T, kind strin var got []sessions.Artifact cursor := "" for { - part, err := s.ListSessionArtifacts(t.Context(), tenant, session, environment, cursor, 1, asc) + part, err := sessionAdapter(s).ListSessionArtifacts(t.Context(), tenant, session, environment, cursor, 1, asc) if err != nil { t.Fatal(err) } @@ -155,8 +156,8 @@ func testSessionArtifactsPublishVersionScopeAndLifetime(t *testing.T, kind strin t.Fatal(err) } for _, a := range page.Artifacts { - if err := New(pool).ReadSessionArtifact(t.Context(), tenant, session, a.ID, func(meta sessions.Artifact, r io.Reader) error { - if err := s.DeleteSessionArtifact(t.Context(), tenant, session, a.ID); err != nil { + if err := sessionAdapter(New(pool)).ReadSessionArtifact(t.Context(), tenant, session, a.ID, func(meta sessions.Artifact, r io.Reader) error { + if err := sessionAdapter(s).DeleteSessionArtifact(t.Context(), tenant, session, a.ID); err != nil { return err } body, err := io.ReadAll(r) @@ -171,7 +172,7 @@ func testSessionArtifactsPublishVersionScopeAndLifetime(t *testing.T, kind strin }); err != nil { t.Fatal(err) } - if _, err := s.GetSessionArtifact(t.Context(), tenant, session, a.ID); !errors.Is(err, sessions.ErrNotFound) { + if _, err := sessionAdapter(s).GetSessionArtifact(t.Context(), tenant, session, a.ID); !errors.Is(err, sessions.ErrNotFound) { t.Fatalf("deleted metadata retained: %v", err) } } @@ -183,44 +184,6 @@ func testSessionArtifactsPublishVersionScopeAndLifetime(t *testing.T, kind strin } } -type artifactReadError struct{} - -func (artifactReadError) Read([]byte) (int, error) { return 0, io.ErrUnexpectedEOF } - -func TestSessionArtifactsRejectIncompleteAndUnownedCapture(t *testing.T) { - for _, kind := range []string{"openai_hosted", "self_hosted"} { - t.Run(kind, func(t *testing.T) { testSessionArtifactsRejectIncompleteAndUnownedCapture(t, kind) }) - } -} - -func testSessionArtifactsRejectIncompleteAndUnownedCapture(t *testing.T, kind string) { - s, pool := testStore(t) - tenant, session, environment, turn := artifactTurn(t, s, kind) - before := largeObjectCount(t, pool) - valid := artifactArchive(t, map[string][]byte{"outputs/a": []byte("data")}) - for name, body := range map[string]io.Reader{ - "transport-failure-after-valid-tar": io.MultiReader(bytes.NewReader(valid), artifactReadError{}), - "truncated-body": bytes.NewReader(valid[:513]), - "trailing-data": io.MultiReader(bytes.NewReader(valid), bytes.NewReader([]byte("not archive padding"))), - "traversal": bytes.NewReader(artifactArchive(t, map[string][]byte{"outputs/../secret": []byte("no")})), - "private-root": bytes.NewReader(artifactArchive(t, map[string][]byte{"secrets/key": []byte("no")})), - } { - t.Run(name, func(t *testing.T) { - if err := s.StageTurnArtifacts(t.Context(), tenant, session, turn, environment, body); err == nil { - t.Fatal("invalid capture accepted") - } - if count := largeObjectCount(t, pool); count != before { - t.Fatalf("rollback leaked objects: %d -> %d", before, count) - } - }) - } - for _, ids := range [][4]string{{uuid.NewString(), session, turn, environment}, {tenant, session, turn, uuid.NewString()}} { - if err := s.StageTurnArtifacts(t.Context(), ids[0], ids[1], ids[2], ids[3], artifactReadError{}); !errors.Is(err, sessions.ErrNotFound) { - t.Fatalf("unauthorized capture reached reader: %v", err) - } - } -} - func TestSessionArtifactsDiscardTerminalPrivateCapture(t *testing.T) { for _, status := range []string{sessions.TurnFailed, sessions.TurnCancelled} { t.Run(status, func(t *testing.T) { @@ -228,14 +191,14 @@ func TestSessionArtifactsDiscardTerminalPrivateCapture(t *testing.T) { tenant, session, environment, turn := artifactTurn(t, s, "openai_hosted") before := largeObjectCount(t, pool) body := artifactArchive(t, map[string][]byte{"outputs/a": []byte("private")}) - if err := s.StageTurnArtifacts(t.Context(), tenant, session, turn, environment, bytes.NewReader(body)); err != nil { + if err := stageTurnArtifacts(t.Context(), s, tenant, session, turn, environment, bytes.NewReader(body)); err != nil { t.Fatal(err) } transition(t, s, tenant, session, turn, sessions.TurnInProgress, status) if count := largeObjectCount(t, pool); count != before { t.Fatalf("terminal capture leaked objects: %d -> %d", before, count) } - if err := s.StageTurnArtifacts(t.Context(), tenant, session, turn, environment, bytes.NewReader(body)); !errors.Is(err, sessions.ErrTurnConflict) { + if err := stageTurnArtifacts(t.Context(), s, tenant, session, turn, environment, bytes.NewReader(body)); !errors.Is(err, sessions.ErrTurnConflict) { t.Fatalf("late capture accepted: %v", err) } if count := largeObjectCount(t, pool); count != before { @@ -255,7 +218,7 @@ func TestSessionArtifactTransferDoesNotBlockDeletionOrCancellation(t *testing.T) defer reader.Close() defer writer.Close() result := make(chan error, 1) - go func() { result <- s.StageTurnArtifacts(t.Context(), tenant, session, turn, environment, reader) }() + go func() { result <- stageTurnArtifacts(t.Context(), s, tenant, session, turn, environment, reader) }() // A complete TAR arrives, but transport has not acknowledged success yet. if _, err := writer.Write(artifactArchive(t, map[string][]byte{"outputs/a": []byte("partial")})); err != nil { t.Fatal(err) @@ -303,7 +266,7 @@ func stageArtifactOutputs(t *testing.T, s *Store, tenant, session, environment, for name, body := range files { archive["outputs/"+name] = []byte(body) } - if err := s.StageTurnArtifacts(t.Context(), tenant, session, turn, environment, bytes.NewReader(artifactArchive(t, archive))); err != nil { + if err := stageTurnArtifacts(t.Context(), s, tenant, session, turn, environment, bytes.NewReader(artifactArchive(t, archive))); err != nil { t.Fatal(err) } } @@ -311,7 +274,7 @@ func stageArtifactOutputs(t *testing.T, s *Store, tenant, session, environment, // publishedByTurn returns the Artifacts one Turn published, keyed by outputs-relative path. func publishedByTurn(t *testing.T, s *Store, tenant, session, turn string) map[string]sessions.Artifact { t.Helper() - page, err := s.ListSessionArtifacts(t.Context(), tenant, session, "", "", 100, true) + page, err := sessionAdapter(s).ListSessionArtifacts(t.Context(), tenant, session, "", "", 100, true) if err != nil || page.NextCursor != "" { t.Fatalf("list: %+v %v", page, err) } @@ -327,7 +290,7 @@ func publishedByTurn(t *testing.T, s *Store, tenant, session, turn string) map[s func artifactBytes(t *testing.T, s *Store, tenant, session, id string) string { t.Helper() var body []byte - if err := s.ReadSessionArtifact(t.Context(), tenant, session, id, func(_ sessions.Artifact, r io.Reader) error { + if err := sessionAdapter(s).ReadSessionArtifact(t.Context(), tenant, session, id, func(_ sessions.Artifact, r io.Reader) error { var err error body, err = io.ReadAll(r) return err @@ -377,7 +340,7 @@ func TestSessionArtifactsRepublishOnlyNewChangedOrDeletedPaths(t *testing.T) { unchanged := func(artifacts ...sessions.Artifact) { t.Helper() for _, artifact := range artifacts { - if got, err := s.GetSessionArtifact(t.Context(), tenant, session, artifact.ID); err != nil || got != artifact { + if got, err := sessionAdapter(s).GetSessionArtifact(t.Context(), tenant, session, artifact.ID); err != nil || got != artifact { t.Fatalf("existing Artifact changed: %+v -> %+v %v", artifact, got, err) } } @@ -396,7 +359,7 @@ func TestSessionArtifactsRepublishOnlyNewChangedOrDeletedPaths(t *testing.T) { // The first Turn behaves as before: every regular output is published. outputs := map[string]string{"a.txt": "alpha", "sub/b.txt": "bravo", "empty.txt": ""} one := run(outputs, "a.txt", "sub/b.txt", "empty.txt") - if err := s.DeleteSessionArtifact(t.Context(), tenant, session, one["a.txt"].ID); err != nil { + if err := sessionAdapter(s).DeleteSessionArtifact(t.Context(), tenant, session, one["a.txt"].ID); err != nil { t.Fatal(err) } // New c.txt and deleted-then-unchanged a.txt; unchanged paths keep their IDs. @@ -420,7 +383,7 @@ func TestSessionArtifactsRepublishOnlyNewChangedOrDeletedPaths(t *testing.T) { unchanged(one["sub/b.txt"], one["empty.txt"], two["a.txt"], two["c.txt"], three["sub/b.txt"], four["sub/b.txt"]) objects() // Deletion leaves no tombstone: the newest remaining version is the comparison base. - if err := s.DeleteSessionArtifact(t.Context(), tenant, session, four["sub/b.txt"].ID); err != nil { + if err := sessionAdapter(s).DeleteSessionArtifact(t.Context(), tenant, session, four["sub/b.txt"].ID); err != nil { t.Fatal(err) } outputs["sub/b.txt"] = "bravo-v2" @@ -432,7 +395,7 @@ func TestSessionArtifactsRepublishOnlyNewChangedOrDeletedPaths(t *testing.T) { // by the completion transaction, so the same Turn republishes the path. turn := startArtifactTurn(t, s, tenant, session, "artifact-turn-delete-during-capture") stageArtifactOutputs(t, s, tenant, session, environment, turn, outputs) - if err := s.DeleteSessionArtifact(t.Context(), tenant, session, two["a.txt"].ID); err != nil { + if err := sessionAdapter(s).DeleteSessionArtifact(t.Context(), tenant, session, two["a.txt"].ID); err != nil { t.Fatal(err) } transition(t, s, tenant, session, turn, sessions.TurnInProgress, sessions.TurnCompleted) @@ -528,15 +491,16 @@ func TestSessionArtifactsCompletionWaitsForConcurrentDeletion(t *testing.T) { t.Fatal(err) } defer tx.Rollback(context.Background()) - lookup, err := artifactLookup(tenant, session, newest.ID) + tenantID, err := pgunit.ParseID(tenant) if err != nil { t.Fatal(err) } + lookup := sqlc.DeleteSessionArtifactParams{TenantID: tenantID, SessionID: pgunit.PathID(session), ID: pgunit.PathID(newest.ID)} q := s.queries.WithTx(tx) if _, err := q.LockSession(t.Context(), sqlc.LockSessionParams{TenantID: lookup.TenantID, ID: lookup.SessionID}); err != nil { t.Fatal(err) } - oid, err := q.DeleteSessionArtifact(t.Context(), sqlc.DeleteSessionArtifactParams(lookup)) + oid, err := q.DeleteSessionArtifact(t.Context(), lookup) if err != nil { t.Fatal(err) } @@ -582,7 +546,7 @@ func TestSessionArtifactsCompletionWaitsForConcurrentDeletion(t *testing.T) { if got := publishedPaths(published); strings.Join(got, ",") != "a.txt" || artifactBytes(t, s, tenant, session, published["a.txt"].ID) != "alpha" { t.Fatalf("concurrent deletion was not republished: %v", got) } - if _, err := s.GetSessionArtifact(t.Context(), tenant, session, newest.ID); !errors.Is(err, sessions.ErrNotFound) { + if _, err := sessionAdapter(s).GetSessionArtifact(t.Context(), tenant, session, newest.ID); !errors.Is(err, sessions.ErrNotFound) { t.Fatalf("deleted Artifact remains: %v", err) } if count := largeObjectCount(t, pool); count != before+1 { diff --git a/services/core/internal/store/session_deletion_test.go b/services/core/internal/store/session_deletion_test.go index c2e3668c8..84b0d53f5 100644 --- a/services/core/internal/store/session_deletion_test.go +++ b/services/core/internal/store/session_deletion_test.go @@ -145,7 +145,7 @@ func TestSessionDeletionWaitsForSettledTurnAndRejectsAdmission(t *testing.T) { if _, err := fresh.RequestCancel(ctx, tenant, session.ID, "late-cancel"); !errors.Is(err, sessions.ErrNotFound) { t.Fatal(err) } - if _, err := fresh.ListItems(ctx, tenant, session.ID, "", 20, true); !errors.Is(err, sessions.ErrNotFound) { + if _, err := sessionAdapter(fresh).ListItems(ctx, tenant, session.ID, "", 20, true); !errors.Is(err, sessions.ErrNotFound) { t.Fatal(err) } turn, err := fresh.GetTurn(ctx, tenant, session.ID, receipt.TurnID) diff --git a/services/core/internal/store/session_events_test.go b/services/core/internal/store/session_events_test.go index 179b84102..ff0984160 100644 --- a/services/core/internal/store/session_events_test.go +++ b/services/core/internal/store/session_events_test.go @@ -142,7 +142,7 @@ func TestSessionEventsCommitSnapshotsRetriesAndIsolation(t *testing.T) { t.Fatal("foreign event access", err) } before, _ = s.SessionEventCursor(ctx, tenant, session.ID) - if _, err = s.ListItems(ctx, tenant, session.ID, "", 100, true); err != nil { + if _, err = sessionReads(pool).ListItems(ctx, tenant, session.ID, "", 100, true); err != nil { t.Fatal(err) } after, _ = s.SessionEventCursor(ctx, tenant, session.ID) @@ -195,7 +195,7 @@ func TestSessionEventsRetentionAndQueuedCancellation(t *testing.T) { if _, err = pool.Exec(ctx, "UPDATE session_events SET payload=jsonb_build_object('padding',repeat('x',524288)) WHERE session_id=$1", session.ID); err != nil { t.Fatal(err) } - if _, err = s.ListItems(ctx, tenant, session.ID, "", 1, true); err != nil { + if _, err = sessionReads(pool).ListItems(ctx, tenant, session.ID, "", 1, true); err != nil { t.Fatal(err) } var bytes int64 diff --git a/services/core/internal/store/session_initial_input_test.go b/services/core/internal/store/session_initial_input_test.go index 2a66829b8..e581f228d 100644 --- a/services/core/internal/store/session_initial_input_test.go +++ b/services/core/internal/store/session_initial_input_test.go @@ -123,7 +123,7 @@ func TestInitialInputFailureRollsBackSessionAndWork(t *testing.T) { if err != nil || got.LastTurn == nil { t.Fatal("retry after rollback failed", got, err) } - items, err := s.ListItems(ctx, tenant, got.ID, "", 100, true) + items, err := sessionAdapter(s).ListItems(ctx, tenant, got.ID, "", 100, true) if err != nil || len(items.Items) != 2 { t.Fatal(items, err) } diff --git a/services/core/internal/store/sessions_fixture_test.go b/services/core/internal/store/sessions_fixture_test.go new file mode 100644 index 000000000..7b25eb1b1 --- /dev/null +++ b/services/core/internal/store/sessions_fixture_test.go @@ -0,0 +1,20 @@ +package store_test + +import ( + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgunit" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/sessionpg" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" +) + +// sessionReads is the Session adapter on pool. It serves the Item, Subagent +// and Artifact reads. +func sessionReads(pool *pgxpool.Pool) *sessionpg.Store { return sessionpg.New(pgunit.NewPool(pool)) } + +// fixtureSessions builds the Session adapter and service on db, as cmd/server does. +func fixtureSessions(db fixtureDB) (*sessionpg.Store, *sessions.Service, error) { + sessionStore := sessionReads(db.pool) + sessionService, err := sessions.NewService(sessionStore) + return sessionStore, sessionService, err +} diff --git a/services/core/internal/store/sessions_test.go b/services/core/internal/store/sessions_test.go index 7ae996b03..cb3976fe9 100644 --- a/services/core/internal/store/sessions_test.go +++ b/services/core/internal/store/sessions_test.go @@ -3,6 +3,7 @@ package store import ( "context" "errors" + "io" "reflect" "sync" "testing" @@ -11,6 +12,7 @@ import ( "github.com/jackc/pgx/v5/pgxpool" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/pgtest" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/sessionpg" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" ) @@ -20,6 +22,20 @@ func testStore(t *testing.T) (*Store, *pgxpool.Pool) { return New(pool), pool } +// sessionAdapter is the Session adapter on s's database. It serves the Item, +// Subagent and Artifact reads and deletes Artifacts. +func sessionAdapter(s *Store) *sessionpg.Store { return sessionpg.New(s.pooled) } + +// stageTurnArtifacts stages export as the Turn's Artifacts through the Session +// service on s's database, as the execution Worker does. +func stageTurnArtifacts(ctx context.Context, s *Store, tenant, session, turn, environment string, export io.Reader) error { + service, err := sessions.NewService(sessionAdapter(s)) + if err != nil { + return err + } + return service.StageTurnArtifacts(ctx, sessions.StageTurnArtifactsCommand{TenantID: tenant, SessionID: session, TurnID: turn, EnvironmentID: environment, Export: export}) +} + func TestSessionsPersistAndStayTenantScoped(t *testing.T) { s, pool := testStore(t) ctx := context.Background() diff --git a/services/core/internal/store/subagent_item_projection.go b/services/core/internal/store/subagent_item_projection.go index f486860ea..324ae43bb 100644 --- a/services/core/internal/store/subagent_item_projection.go +++ b/services/core/internal/store/subagent_item_projection.go @@ -9,6 +9,7 @@ import ( "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/db/sqlc" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/items" + "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/sessionpg" "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" "github.com/google/uuid" "github.com/jackc/pgx/v5" @@ -35,7 +36,7 @@ func projectSubagentItem(ctx context.Context, q *sqlc.Queries, session pgtype.UU return sessions.ErrInvalidInput } turnID := items.Identity(uuid.UUID(child.ID.Bytes).String(), "turn:"+p.TurnID) - turn, err := childTurn(ctx, q, session, uuid.UUID(child.ID.Bytes).String(), turnID) + turn, err := sessionpg.LoadChildTurn(ctx, q, session, uuid.UUID(child.ID.Bytes).String(), turnID) if err != nil { return err } diff --git a/services/core/internal/store/subagent_item_reads.go b/services/core/internal/store/subagent_item_reads.go deleted file mode 100644 index ad447755d..000000000 --- a/services/core/internal/store/subagent_item_reads.go +++ /dev/null @@ -1,76 +0,0 @@ -package store - -import ( - "context" - "encoding/json" - "errors" - - 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/persistence/postgres/pgunit" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" - "github.com/jackc/pgx/v5" - "github.com/jackc/pgx/v5/pgtype" -) - -func (s *Store) ListSubagentItems(ctx context.Context, tenant, session, child, after string, limit int, asc bool) (v1.ItemList, error) { - return s.listChildItems(ctx, tenant, session, child, "", after, limit, asc) -} -func (s *Store) ListSubagentTurnItems(ctx context.Context, tenant, session, child, turn, after string, limit int, asc bool) (v1.ItemList, error) { - if turn == "" { - return v1.ItemList{}, sessions.ErrInvalidInput - } - return s.listChildItems(ctx, tenant, session, child, turn, after, limit, asc) -} -func (s *Store) listChildItems(ctx context.Context, tenant, session, child, turn, after string, limit int, asc bool) (v1.ItemList, error) { - result := v1.ItemList{Data: []v1.Item{}} - if limit < 1 || limit > 100 { - return result, sessions.ErrInvalidInput - } - err := s.withPublicSession(ctx, tenant, session, func(ctx context.Context, q *sqlc.Queries, sid pgtype.UUID) error { - if _, err := publicSubagent(ctx, q, sid, child); err != nil { - return err - } - childID, _ := parseID(child) - p := sqlc.ListChildItemsParams{SessionID: sid, SubagentID: childID, Ascending: asc, PageLimit: int32(limit + 1), AfterID: pgtype.UUID{Valid: true}, AfterTurn: pgtype.UUID{Valid: true}} - if turn != "" { - row, err := childTurn(ctx, q, sid, child, turn) - if err != nil { - return err - } - p.TurnID = row.ID - } - if after != "" { - // Any cursor outside this child (and Turn) scope, including a - // malformed one, uses the Session Item cursor error. - row, err := q.GetChildItem(ctx, sqlc.GetChildItemParams{SessionID: sid, SubagentID: childID, ID: pgunit.PathID(after)}) - if errors.Is(err, pgx.ErrNoRows) || (err == nil && p.TurnID.Valid && p.TurnID != row.TurnID) { - return sessions.ErrItemCursor - } - if err != nil { - return err - } - p.AfterID = row.ID - p.AfterCreated = row.TurnCreatedAt - p.AfterTurn = row.TurnID - p.AfterPosition = row.Position - } - rows, err := q.ListChildItems(ctx, p) - if err != nil { - return err - } - result.HasMore = len(rows) > limit - if result.HasMore { - rows = rows[:limit] - } - for _, raw := range rows { - var item v1.Item - if err := json.Unmarshal(raw, &item); err != nil { - return err - } - result.Data = append(result.Data, item) - } - return nil - }) - return result, err -} diff --git a/services/core/internal/store/subagent_lifecycle.go b/services/core/internal/store/subagent_lifecycle.go index 5cc94074c..3a49fb0f6 100644 --- a/services/core/internal/store/subagent_lifecycle.go +++ b/services/core/internal/store/subagent_lifecycle.go @@ -20,7 +20,7 @@ func validNativeIdentity(value string) bool { return value != "" && len(value) <= 512 && strings.TrimSpace(value) == value && !strings.ContainsAny(value, "\x00\r\n") } func recordSubagentChange(ctx context.Context, q *sqlc.Queries, session, id pgtype.UUID, kind string) error { - value, err := publicSubagent(ctx, q, session, uuid.UUID(id.Bytes).String()) + value, err := sessionpg.LoadPublicSubagent(ctx, q, session, uuid.UUID(id.Bytes).String()) if err != nil { return err } diff --git a/services/core/internal/store/subagent_native_outputs_test.go b/services/core/internal/store/subagent_native_outputs_test.go index 4a880f941..6db0b6e68 100644 --- a/services/core/internal/store/subagent_native_outputs_test.go +++ b/services/core/internal/store/subagent_native_outputs_test.go @@ -46,7 +46,7 @@ func TestSubagentNativeFunctionResultDoesNotConsumeOutputIndex(t *testing.T) { if err != nil { t.Fatal(err) } - items, err := s.ListSubagentItems(t.Context(), tenant, session.ID, child.ID, "", 20, true) + items, err := sessionAdapter(s).ListSubagentItems(t.Context(), tenant, session.ID, child.ID, "", 20, true) if err != nil || len(items.Data) != 3 { t.Fatal(items, err) } @@ -119,11 +119,11 @@ func TestSubagentCancelledPartialMessageSurvivesHistoryReplay(t *testing.T) { if err != nil { t.Fatal(err) } - items, err := s.ListSubagentItems(t.Context(), tenant, session.ID, child.ID, "", 20, true) + items, err := sessionAdapter(s).ListSubagentItems(t.Context(), tenant, session.ID, child.ID, "", 20, true) if err != nil || len(items.Data) != 1 || items.Data[0].Status != "incomplete" || *items.Data[0].Content[0].Text != "Partial native answer" { t.Fatal(items, err) } - turns, err := s.ListSubagentTurns(t.Context(), tenant, session.ID, child.ID, "", 20, true) + turns, err := sessionAdapter(s).ListSubagentTurns(t.Context(), tenant, session.ID, child.ID, "", 20, true) if err != nil || len(turns.Data) != 1 || turns.Data[0].Status != sessions.TurnCancelled { t.Fatal(turns, err) } diff --git a/services/core/internal/store/subagent_reads.go b/services/core/internal/store/subagent_reads.go deleted file mode 100644 index a8cfcf03a..000000000 --- a/services/core/internal/store/subagent_reads.go +++ /dev/null @@ -1,182 +0,0 @@ -package store - -import ( - "context" - "encoding/json" - "errors" - - 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/persistence/postgres/pgunit" - "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/sessions" - "github.com/google/uuid" - "github.com/jackc/pgx/v5" - "github.com/jackc/pgx/v5/pgtype" -) - -func publicSubagent(ctx context.Context, q *sqlc.Queries, session pgtype.UUID, id string) (v1.Subagent, error) { - row, err := q.GetPublicSubagent(ctx, sqlc.GetPublicSubagentParams{SessionID: session, ID: pgunit.PathID(id)}) - if errors.Is(err, pgx.ErrNoRows) { - return v1.Subagent{}, sessions.ErrNotFound - } - if err != nil { - return v1.Subagent{}, err - } - result := v1.Subagent{ID: id, SessionID: uuid.UUID(session.Bytes).String(), ParentAgentID: row.ParentAgentID, Object: "agent.session.subagent", Status: row.Status, OpenedAt: row.NativeCreatedAt} - if row.Name.Valid { - result.Name = &row.Name.String - } - if row.Instructions.Valid { - result.Instructions = []v1.AgentContent{{Type: "output_text", Text: &row.Instructions.String}} - } - if row.ClosedAtMs.Valid { - seconds := row.ClosedAtMs.Int64 / 1000 - result.ClosedAt = &seconds - } - return result, nil -} - -func (s *Store) GetSubagent(ctx context.Context, tenant, session, id string) (v1.Subagent, error) { - var result v1.Subagent - err := s.withPublicSession(ctx, tenant, session, func(ctx context.Context, q *sqlc.Queries, sid pgtype.UUID) error { - var err error - result, err = publicSubagent(ctx, q, sid, id) - return err - }) - return result, err -} - -func (s *Store) ListSubagents(ctx context.Context, tenant, session, after string, limit int, asc bool) (v1.SubagentList, error) { - result := v1.SubagentList{Data: []v1.Subagent{}} - if limit < 1 || limit > 100 { - return result, sessions.ErrInvalidInput - } - err := s.withPublicSession(ctx, tenant, session, func(ctx context.Context, q *sqlc.Queries, sid pgtype.UUID) error { - p := sqlc.ListPublicSubagentsParams{SessionID: sid, Ascending: asc, PageLimit: int32(limit + 1), AfterID: pgtype.UUID{Valid: true}} - if after != "" { - // Any cursor that is not a Subagent of this Session, including a - // malformed one, is an invalid cursor rather than a missing resource. - cursor, err := publicSubagent(ctx, q, sid, after) - if err != nil { - return unresolvedCursor(err, sessions.ErrResourceCursor) - } - p.AfterOpened = pgtype.Int8{Int64: cursor.OpenedAt, Valid: true} - p.AfterID, _ = parseID(after) - } - rows, err := q.ListPublicSubagents(ctx, p) - if err != nil { - return err - } - result.HasMore = len(rows) > limit - if result.HasMore { - rows = rows[:limit] - } - for _, id := range rows { - value, err := publicSubagent(ctx, q, sid, uuid.UUID(id.Bytes).String()) - if err != nil { - return err - } - result.Data = append(result.Data, value) - } - return nil - }) - return result, err -} - -func childTurn(ctx context.Context, q *sqlc.Queries, session pgtype.UUID, child, id string) (sqlc.SubagentTurn, error) { - row, err := q.GetChildTurn(ctx, sqlc.GetChildTurnParams{SessionID: session, ID: pgunit.PathID(id)}) - if errors.Is(err, pgx.ErrNoRows) || (err == nil && uuid.UUID(row.SubagentID.Bytes).String() != child) { - return row, sessions.ErrNotFound - } - return row, err -} - -// sessionAgentID returns the Session's Agent ID, which is the agent_id of every -// Turn in the Session, including child Turns. The official service projects a -// direct child's Turn this way; nested children follow the same rule unobserved. -func sessionAgentID(ctx context.Context, q *sqlc.Queries, session pgtype.UUID) (string, error) { - agent, err := q.SubagentRootAgent(ctx, session) - if err == nil && agent == "" { - err = errors.New("missing stored agent identity") - } - return agent, err -} - -// publicChildTurn identifies the child through subagent_id; agent_id is the -// Session's Agent ID. -func publicChildTurn(row sqlc.SubagentTurn, agent string) v1.Turn { - child := uuid.UUID(row.SubagentID.Bytes).String() - value := v1.Turn{ID: uuid.UUID(row.ID.Bytes).String(), SessionID: uuid.UUID(row.SessionID.Bytes).String(), AgentID: agent, SubagentID: &child, Object: "agent.session.turn", Status: row.Status, CreatedAt: row.CreatedAt.Time.Unix()} - if row.StartedAt.Valid { - seconds := row.StartedAt.Time.Unix() - value.StartedAt = &seconds - } - if row.CompletedAt.Valid { - seconds := row.CompletedAt.Time.Unix() - value.CompletedAt = &seconds - } - _ = json.Unmarshal(row.TokenUsage, &value.Usage) - if row.Status == sessions.TurnFailed { - value.Error = &v1.TurnError{Code: "internal_error", Message: "The execution could not complete."} - } - return value -} - -func (s *Store) GetSubagentTurn(ctx context.Context, tenant, session, child, id string) (v1.Turn, error) { - var result v1.Turn - err := s.withPublicSession(ctx, tenant, session, func(ctx context.Context, q *sqlc.Queries, sid pgtype.UUID) error { - if _, err := publicSubagent(ctx, q, sid, child); err != nil { - return err - } - row, err := childTurn(ctx, q, sid, child, id) - if err != nil { - return err - } - agent, err := sessionAgentID(ctx, q, sid) - if err == nil { - result = publicChildTurn(row, agent) - } - return err - }) - return result, err -} - -func (s *Store) ListSubagentTurns(ctx context.Context, tenant, session, child, after string, limit int, asc bool) (v1.TurnList, error) { - result := v1.TurnList{Data: []v1.Turn{}} - if limit < 1 || limit > 100 { - return result, sessions.ErrInvalidInput - } - err := s.withPublicSession(ctx, tenant, session, func(ctx context.Context, q *sqlc.Queries, sid pgtype.UUID) error { - if _, err := publicSubagent(ctx, q, sid, child); err != nil { - return err - } - childID, _ := parseID(child) - p := sqlc.ListChildTurnsParams{SessionID: sid, SubagentID: childID, Ascending: asc, PageLimit: int32(limit + 1), AfterID: pgtype.UUID{Valid: true}} - if after != "" { - // Root Turns and other children's Turns are outside this list. - row, err := childTurn(ctx, q, sid, child, after) - if err != nil { - return unresolvedCursor(err, sessions.ErrResourceCursor) - } - p.AfterCreated = row.CreatedAt - p.AfterID = row.ID - } - rows, err := q.ListChildTurns(ctx, p) - if err != nil { - return err - } - agent, err := sessionAgentID(ctx, q, sid) - if err != nil { - return err - } - result.HasMore = len(rows) > limit - if result.HasMore { - rows = rows[:limit] - } - for _, row := range rows { - result.Data = append(result.Data, publicChildTurn(row, agent)) - } - return nil - }) - return result, err -} diff --git a/services/core/internal/store/subagent_resources_test.go b/services/core/internal/store/subagent_resources_test.go index f3149f9d6..520f42e92 100644 --- a/services/core/internal/store/subagent_resources_test.go +++ b/services/core/internal/store/subagent_resources_test.go @@ -50,7 +50,7 @@ func TestSubagentResourcesNativeOwnershipLifecycleAndRecovery(t *testing.T) { ordinal += int32(len(facts)) } appendFacts(subagentIdentityEvent("child", "root", 100), subagentIdentityEvent("nested", "child", 101), subagentIdentityEvent("sibling", "root", 100)) - page, err := s.ListSubagents(ctx, tenant, session.ID, "", 100, true) + page, err := sessionAdapter(s).ListSubagents(ctx, tenant, session.ID, "", 100, true) if err != nil || len(page.Data) != 3 { t.Fatal(page, err) } @@ -59,7 +59,7 @@ func TestSubagentResourcesNativeOwnershipLifecycleAndRecovery(t *testing.T) { t.Fatal(err) } nested, _ := s.GetSubagentIdentity(ctx, tenant, session.ID, "nested") - resource, err := s.GetSubagent(ctx, tenant, session.ID, nested.ID) + resource, err := sessionAdapter(s).GetSubagent(ctx, tenant, session.ID, nested.ID) if err != nil || resource.ParentAgentID != child.ID { t.Fatal(resource, err) } @@ -76,7 +76,7 @@ func TestSubagentResourcesNativeOwnershipLifecycleAndRecovery(t *testing.T) { appendFacts(subagentFact(proto.TypeSubagentTurn, turn)) // Later history reads preserve the terminal Turn and all existing Items. appendFacts(subagentFact(proto.TypeSubagentItem, item), subagentFact(proto.TypeSubagentTurn, turn)) - turns, err := s.ListSubagentTurns(ctx, tenant, session.ID, child.ID, "", 20, true) + turns, err := sessionAdapter(s).ListSubagentTurns(ctx, tenant, session.ID, child.ID, "", 20, true) if err != nil || len(turns.Data) != 1 { t.Fatal(turns, err) } @@ -85,7 +85,7 @@ func TestSubagentResourcesNativeOwnershipLifecycleAndRecovery(t *testing.T) { if turns.Data[0].AgentID != "agent_root" || turns.Data[0].SubagentID == nil || *turns.Data[0].SubagentID != child.ID || turns.Data[0].Usage != nil { t.Fatal(turns) } - if same, err := s.GetSubagentTurn(ctx, tenant, session.ID, child.ID, tid); err != nil || !reflect.DeepEqual(same, turns.Data[0]) { + if same, err := sessionAdapter(s).GetSubagentTurn(ctx, tenant, session.ID, child.ID, tid); err != nil || !reflect.DeepEqual(same, turns.Data[0]) { t.Fatal(same, err) } // Session Turn reads carry root work only: a child Turn ID is missing there. @@ -102,24 +102,24 @@ func TestSubagentResourcesNativeOwnershipLifecycleAndRecovery(t *testing.T) { // Nested children follow the same agent_id rule (unobserved officially). nestedTurn := proto.SubagentTurnPayload{NativeID: "nested", TurnID: "native-nested-turn", Status: sessions.TurnInProgress, CreatedAtMS: opened, StartedAtMS: &opened} appendFacts(subagentFact(proto.TypeSubagentTurn, nestedTurn)) - nestedTurns, err := s.ListSubagentTurns(ctx, tenant, session.ID, nested.ID, "", 20, true) + nestedTurns, err := sessionAdapter(s).ListSubagentTurns(ctx, tenant, session.ID, nested.ID, "", 20, true) if err != nil || len(nestedTurns.Data) != 1 || nestedTurns.Data[0].AgentID != "agent_root" || *nestedTurns.Data[0].SubagentID != nested.ID { t.Fatal(nestedTurns, err) } - if _, err = s.GetSubagentTurn(ctx, uuid.NewString(), session.ID, child.ID, tid); !errors.Is(err, sessions.ErrNotFound) { + if _, err = sessionAdapter(s).GetSubagentTurn(ctx, uuid.NewString(), session.ID, child.ID, tid); !errors.Is(err, sessions.ErrNotFound) { t.Fatal("foreign tenant child Turn", err) } - if _, err = s.ListSubagentTurns(ctx, uuid.NewString(), session.ID, child.ID, "", 20, true); !errors.Is(err, sessions.ErrNotFound) { + if _, err = sessionAdapter(s).ListSubagentTurns(ctx, uuid.NewString(), session.ID, child.ID, "", 20, true); !errors.Is(err, sessions.ErrNotFound) { t.Fatal("foreign tenant child Turns", err) } if allTurns, err = s.ListTurns(ctx, tenant, session.ID, "", 100, true); err != nil || len(allTurns.Turns) != 1 { t.Fatal(allTurns, err) } - own, err := s.ListSubagentTurnItems(ctx, tenant, session.ID, child.ID, tid, "", 20, true) + own, err := sessionAdapter(s).ListSubagentTurnItems(ctx, tenant, session.ID, child.ID, tid, "", 20, true) if err != nil || len(own.Data) != 1 || own.Data[0].TurnID != tid { t.Fatal(own, err) } - rootItems, err := s.ListItems(ctx, tenant, session.ID, "", 100, true) + rootItems, err := sessionAdapter(s).ListItems(ctx, tenant, session.ID, "", 100, true) if err != nil { t.Fatal(err) } @@ -128,24 +128,24 @@ func TestSubagentResourcesNativeOwnershipLifecycleAndRecovery(t *testing.T) { t.Fatal("child Item leaked into root Items") } } - if _, err = s.GetSubagentTurn(ctx, tenant, session.ID, nested.ID, tid); !errors.Is(err, sessions.ErrNotFound) { + if _, err = sessionAdapter(s).GetSubagentTurn(ctx, tenant, session.ID, nested.ID, tid); !errors.Is(err, sessions.ErrNotFound) { t.Fatal("nested ownership", err) } - if _, err = s.ListSubagentItems(ctx, tenant, session.ID, nested.ID, own.Data[0].ID, 20, true); !errors.Is(err, sessions.ErrItemCursor) { + if _, err = sessionAdapter(s).ListSubagentItems(ctx, tenant, session.ID, nested.ID, own.Data[0].ID, 20, true); !errors.Is(err, sessions.ErrItemCursor) { t.Fatal("another child's Item cursor", err) } - if _, err = s.GetSubagent(ctx, uuid.NewString(), session.ID, child.ID); !errors.Is(err, sessions.ErrNotFound) { + if _, err = sessionAdapter(s).GetSubagent(ctx, uuid.NewString(), session.ID, child.ID); !errors.Is(err, sessions.ErrNotFound) { t.Fatal("foreign tenant", err) } closed := proto.SubagentLifecyclePayload{NativeID: "child", EffectID: "native-close-1", Status: "closed", OccurredAtMS: 103000} resumed := proto.SubagentLifecyclePayload{NativeID: "child", EffectID: "native-resume-1", Status: "active", OccurredAtMS: 104000} appendFacts(subagentFact(proto.TypeSubagentLifecycle, closed), subagentFact(proto.TypeSubagentLifecycle, closed)) - value, err := s.GetSubagent(ctx, tenant, session.ID, child.ID) + value, err := sessionAdapter(s).GetSubagent(ctx, tenant, session.ID, child.ID) if err != nil || value.Status != "closed" || value.ClosedAt == nil || *value.ClosedAt != 103 { t.Fatal(value, err) } appendFacts(subagentFact(proto.TypeSubagentLifecycle, resumed), subagentFact(proto.TypeSubagentLifecycle, resumed)) - reopened := New(pool) + reopened := sessionAdapter(New(pool)) value, err = reopened.GetSubagent(ctx, tenant, session.ID, child.ID) if err != nil || value.Status != "active" || value.ClosedAt != nil || value.OpenedAt != 100 { t.Fatal(value, err) diff --git a/services/core/internal/store/turn_reads.go b/services/core/internal/store/turn_reads.go index cdff33971..85b5c1793 100644 --- a/services/core/internal/store/turn_reads.go +++ b/services/core/internal/store/turn_reads.go @@ -13,7 +13,7 @@ import ( ) // ListTurns pages a Session's root Turns. Subagent Turns are not Session Turns; -// ListSubagentTurns reads them. +// sessionpg.Store.ListSubagentTurns reads them. func (s *Store) ListTurns(ctx context.Context, tenantID, sessionID, cursor string, limit int, ascending bool) (sessions.TurnPage, error) { if limit < 1 || limit > 100 { return sessions.TurnPage{}, fmt.Errorf("%w: page size must be 1..100", sessions.ErrInvalidInput)