Move Skills and skill versions out of store into skills and skillpg

services/core/IMPLEMENTATION.md

@@ -27,6 +27,7 @@ Domain owners, each with its PostgreSQL adapter under `internal/persistence/post
 - `vaults` (`vaultpg`): Vaults and Credentials, the encryption of Credential secrets, OAuth access-token refresh, and the MCP credential selection that Session creation freezes and the Dispatcher's `Credentials` resolves into a bearer token.
 - `environmenttemplates` (`templatepg`): Environment Templates, their validation and default network, their sealed setup, initial files, Skills and Plugins, and the resolved Template that Session creation composes into its Environment.
 - `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.
 
 ## Request handling
 
@@ -42,7 +43,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, Skill versions) 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. The Skill version cursor lookup is tenant-wide so another Skill's version can be told apart from a missing one; another tenant's version stays missing.
+- 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.
 
 ## Source Files and Artifacts
 
@@ -60,7 +61,7 @@ Capture bytes into private large objects without a Session admission lock. Befor
 - The directory helper checks each requested path component with `Root.Lstat` below `os.OpenRoot(workspace)` and opens the final directory with `O_NOFOLLOW`. A missing component, a regular file or a symbolic link maps to the distinct `not_directory` result, which the daemon and gateway carry only for directory reads; Core turns it into an empty page. `not_found` (Claude SDK adapter reader), permission, transport and uncertain results keep their errors.
 - A Files.create write intent stores a digest of the path, size and content, not a path ledger, so Core cannot tell a file an earlier Files.create wrote from any other file; an existing regular file therefore gets the untracked-file message. Reserve the intent under the Session lock before dispatch. The daemon verifies the complete body's SHA-256 before calling the writer, and the writer creates parents with `Root.MkdirAll(0700)`, writes `.oac-write-<uuid>` in the workspace root and publishes it with `Root.Link`, which never replaces an existing entry. Known refusals return `write_rejected` with `reason` `destination_directory` or `unsafe_destination`; Core settles the intent as `rejected`, which leaves no committed receipt and releases the mutation owner. Only an exact committed or rejected receipt settles an intent; nothing settles an unknown one automatically.
 - Check the 5 MiB inline bound after path validation and Base64 decoding, and before the pending-hosted check, the source File lookup and execution. The JSON body limit still admits the Base64 form of 50 MiB so that oversized inline bodies up to that size get the official message.
-- Serialize Skill version uploads, default changes and version deletion on the owning Skill row lock. Deleting the default version deletes the Skill only when no other version row exists, through the same cascade as Skill deletion, so every encrypted version row goes in the same commit. `next_version` only increases.
+- `skillpg` serializes Skill version uploads, default changes and version deletion on the owning Skill row lock, and `skills` decides a version deletion over the locked Skill. Deleting the default version deletes the Skill only when no other version row exists, through the same cascade as Skill deletion, so every encrypted version row goes in the same commit. `next_version` only increases.
 
 ## Scheduling, preparation and pending input
 

services/core/cmd/server/http_routes_test.go

@@ -90,7 +90,8 @@ func daemonComposition(t testing.TB) http.Handler {
 		Projects:       trapProjects{keys: keys},
 		ModelProviders: struct{ api.ModelProviders }{}, ModelProvidersReader: struct{ api.ModelProvidersReader }{},
 		Vaults: struct{ api.Vaults }{}, VaultsReader: struct{ api.VaultsReader }{},
-		Files: struct{ api.Files }{}, FilesReader: struct{ api.FilesReader }{}, Skills: struct{ api.Skills }{},
+		Files: struct{ api.Files }{}, FilesReader: struct{ api.FilesReader }{},
+		Skills: struct{ api.Skills }{}, SkillsReader: struct{ api.SkillsReader }{},
 		Agents: struct{ api.Agents }{}, AgentsReader: struct{ api.AgentsReader }{}, Sessions: struct{ api.Sessions }{}, SessionEvents: struct{ api.SessionEvents }{},
 		EnvironmentTemplates: struct{ api.EnvironmentTemplates }{}, EnvironmentTemplatesReader: struct{ api.EnvironmentTemplatesReader }{},
 		SessionHistory: struct{ api.SessionHistory }{}, Subagents: struct{ api.Subagents }{}, Artifacts: struct{ api.Artifacts }{},

services/core/cmd/server/main.go

@@ -45,6 +45,7 @@ import (
 	"github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/filepg"
 	"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/skillpg"
 	"github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/templatepg"
 	"github.com/MiniMax-AI/OpenAgentCore/services/core/internal/persistence/postgres/vaultpg"
 	"github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtime"
@@ -54,6 +55,7 @@ import (
 	historystoreresolver "github.com/MiniMax-AI/OpenAgentCore/services/core/internal/runtimehistory/storeresolver"
 	"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/skills"
 	"github.com/MiniMax-AI/OpenAgentCore/services/core/internal/store"
 	"github.com/MiniMax-AI/OpenAgentCore/services/core/internal/vaults"
 	"github.com/jackc/pgx/v5/pgxpool"
@@ -139,6 +141,11 @@ func run() error {
 	if err != nil {
 		return err
 	}
+	skillStore := skillpg.New(units, credentialKey)
+	skillService, err := skills.NewService(skillStore, skillStore)
+	if err != nil {
+		return err
+	}
 	installation, err := installationFacts(public)
 	if err != nil {
 		return err
@@ -334,7 +341,7 @@ func run() error {
 		Projects:       executionStore,
 		ModelProviders: modelConfigurationService, ModelProvidersReader: modelConfigurationStore,
 		Vaults: vaultService, VaultsReader: vaultStore,
-		Skills:               executionStore,
+		Skills: skillService, SkillsReader: skillStore,
 		EnvironmentTemplates: environmentTemplates, EnvironmentTemplatesReader: templateStore,
 		Files: fileService, FilesReader: fileStore,
 		Agents: agentService, AgentsReader: agentStore,

services/core/internal/api/dependencies.go

@@ -37,6 +37,7 @@ type Dependencies struct {
 	Files                Files
 	FilesReader          FilesReader
 	Skills               Skills
+	SkillsReader         SkillsReader
 	EnvironmentTemplates EnvironmentTemplates
 	Agents               Agents
 	AgentsReader         AgentsReader
@@ -118,9 +119,9 @@ func (d Dependencies) validate() error {
 		field{"InstallationBindings", d.InstallationBindings}, field{"Projects", d.Projects},
 		field{"Vaults", d.Vaults}, field{"VaultsReader", d.VaultsReader},
 		field{"ModelProviders", d.ModelProviders}, field{"ModelProvidersReader", d.ModelProvidersReader},
-		field{"Skills", d.Skills},
 		field{"Files", d.Files}, field{"FilesReader", d.FilesReader},
 		field{"EnvironmentTemplates", d.EnvironmentTemplates}, field{"EnvironmentTemplatesReader", d.EnvironmentTemplatesReader},
+		field{"Skills", d.Skills}, field{"SkillsReader", d.SkillsReader},
 		field{"Agents", d.Agents}, field{"AgentsReader", d.AgentsReader},
 		field{"Sessions", d.Sessions},
 		field{"SessionEvents", d.SessionEvents}, field{"SessionHistory", d.SessionHistory}, field{"Subagents", d.Subagents},

services/core/internal/api/dependencies_test.go

@@ -23,6 +23,7 @@ type testFakes struct {
 	files                  *fakeFiles
 	filesReader            *fakeFilesReader
 	skills                 *fakeSkills
+	skillsReader           *fakeSkillsReader
 	environmentTemplates   *fakeEnvironmentTemplates
 	agents                 *fakeAgents
 	agentsReader           *fakeAgentsReader
@@ -62,9 +63,9 @@ func testDependencies(t testing.TB) (Dependencies, *testFakes) {
 		projects:       &fakeProjects{t: t},
 		modelProviders: &fakeModelProviders{t: t}, modelProvidersReader: &fakeModelProvidersReader{t: t},
 		vaults: &fakeVaults{t: t}, vaultsReader: &fakeVaultsReader{t: t},
-		skills:               &fakeSkills{t: t},
 		environmentTemplates: &fakeEnvironmentTemplates{t: t}, environmentTemplatesReader: &fakeEnvironmentTemplatesReader{t: t},
 		files: &fakeFiles{t: t}, filesReader: &fakeFilesReader{t: t},
+		skills: &fakeSkills{t: t}, skillsReader: &fakeSkillsReader{t: t},
 		agents: &fakeAgents{t: t}, agentsReader: &fakeAgentsReader{t: t},
 		sessions: &fakeSessions{t: t}, sessionEvents: &fakeSessionEvents{t: t},
 		sessionHistory: &fakeSessionHistory{t: t}, subagents: &fakeSubagents{t: t}, artifacts: &fakeArtifacts{t: t},
@@ -76,11 +77,12 @@ func testDependencies(t testing.TB) (Dependencies, *testFakes) {
 	}
 	return Dependencies{
 		Engine: "codex", CoreKeys: coreKeys(t, "admin"), InstallationBindings: f.installationBindings,
-		Projects: f.projects, Skills: f.skills,
+		Projects:       f.projects,
 		ModelProviders: f.modelProviders, ModelProvidersReader: f.modelProvidersReader,
 		Vaults: f.vaults, VaultsReader: f.vaultsReader,
 		Files: f.files, FilesReader: f.filesReader,
 		EnvironmentTemplates: f.environmentTemplates, EnvironmentTemplatesReader: f.environmentTemplatesReader,
+		Skills: f.skills, SkillsReader: f.skillsReader,
 		Agents: f.agents, AgentsReader: f.agentsReader,
 		Sessions: f.sessions, SessionEvents: f.sessionEvents,
 		SessionHistory: f.sessionHistory, Subagents: f.subagents, Artifacts: f.artifacts, SessionAdmin: f.sessionAdmin,

services/core/internal/api/errors.go

@@ -4,7 +4,6 @@ import (
 	"encoding/json"
 	"errors"
 	"net/http"
-	"strings"
 
 	v1 "github.com/MiniMax-AI/OpenAgentCore/contracts/agents-api/v1"
 	"github.com/MiniMax-AI/OpenAgentCore/internal/obs/log"
@@ -188,8 +187,6 @@ func writeStoreError(w http.ResponseWriter, r *http.Request, err error, notFound
 	case errors.Is(err, store.ErrRuntimeNodeUnavailable):
 		writeError(w, http.StatusServiceUnavailable, "runtime_node_unavailable", "The selected sandbox node is unavailable or has no capacity.")
 
-	case errors.Is(err, store.ErrDefaultSkillVersion):
-		writeError(w, http.StatusBadRequest, "invalid_value", "Cannot delete the default skill version.", "version")
 	case errors.Is(err, execution.ErrModelProviderRequired):
 		writeError(w, http.StatusBadRequest, "model_provider_required", "This Session was created without a model provider and cannot run. Create a new Session with x_agents_core.model_provider or an Agent that has one saved.")
 	case errors.Is(err, store.ErrHostedEnvironmentFailed):
@@ -206,20 +203,10 @@ func writeStoreError(w http.ResponseWriter, r *http.Request, err error, notFound
 	case errors.Is(err, execution.ErrExecutionUnavailable):
 		writeError(w, http.StatusServiceUnavailable, "execution_unavailable", "Execution is not available on this service.")
 	case errors.As(err, &cursor):
-		// Observed official fields for an unresolved list cursor: Skill versions
-		// use invalid_value on after, Beta lists invalid_request_error with a null param.
-		if listFamilyOf(r) == skillsList {
-			writeError(w, http.StatusBadRequest, "invalid_value", cursor.Message, "after")
-		} else {
-			writeError(w, http.StatusBadRequest, "invalid_request_error", cursor.Message)
-		}
+		// Observed official fields for an unresolved Beta list cursor, with a null param.
+		writeError(w, http.StatusBadRequest, "invalid_request_error", cursor.Message)
 	case errors.Is(err, store.ErrNotFound):
-		code := "not_found_error"
-		// Skills retain their non-beta error envelope.
-		if strings.HasPrefix(r.URL.Path, "/v1/skills/") || r.URL.Path == "/v1/skills" {
-			code = ""
-		}
-		writeError(w, http.StatusNotFound, code, "Resource not found.", notFoundParam...)
+		writeError(w, http.StatusNotFound, "not_found_error", "Resource not found.", notFoundParam...)
 	case errors.Is(err, store.ErrSessionNotIdle):
 		// Observed official status, type, code, null param and message.
 		writeError(w, http.StatusConflict, "conflict_error", "session must be durably idle or failed without required actions before deletion")

services/core/internal/api/errors_skills.go

@@ -0,0 +1,38 @@
+package api
+
+import (
+	"errors"
+	"net/http"
+
+	"github.com/MiniMax-AI/OpenAgentCore/services/core/internal/skills"
+)
+
+// writeSkillsError maps the skills domain's errors to the Skills responses.
+// The /v1 Skills routes keep their own observed error fields; the /core/v1
+// Project routes use the Beta ones.
+func writeSkillsError(w http.ResponseWriter, r *http.Request, err error) {
+	skillsRoute := listFamilyOf(r) == skillsList
+	var cursor *skills.CursorError
+	switch {
+	case errors.Is(err, skills.ErrDefaultVersion):
+		writeError(w, http.StatusBadRequest, "invalid_value", "Cannot delete the default skill version.", "version")
+	case errors.As(err, &cursor):
+		// Observed official fields for an unresolved version cursor.
+		if skillsRoute {
+			writeError(w, http.StatusBadRequest, "invalid_value", cursor.Message, "after")
+		} else {
+			writeError(w, http.StatusBadRequest, "invalid_request_error", cursor.Message)
+		}
+	case errors.Is(err, skills.ErrNotFound):
+		code := "not_found_error"
+		if skillsRoute {
+			code = ""
+		}
+		writeError(w, http.StatusNotFound, code, "Resource not found.")
+	case errors.Is(err, skills.ErrInvalidInput):
+		writeError(w, http.StatusBadRequest, "invalid_request", invalidInputMessage)
+	case writeAuditSourceError(w, r, err) || writeTextValueError(w, r, err) || writeCredentialUnavailableError(w, r, err):
+	default:
+		writeInternalError(w, r)
+	}
+}

services/core/internal/api/errors_skills_test.go

@@ -0,0 +1,57 @@
+package api
+
+import (
+	"errors"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/MiniMax-AI/OpenAgentCore/services/core/internal/credentialcrypto"
+	"github.com/MiniMax-AI/OpenAgentCore/services/core/internal/skills"
+	"github.com/MiniMax-AI/OpenAgentCore/services/core/internal/textvalue"
+	"github.com/MiniMax-AI/OpenAgentCore/services/core/internal/writeaudit"
+)
+
+// The /v1 Skills routes keep their observed error fields; the /core/v1 Project
+// Skills routes use the Beta ones.
+func TestWriteSkillsError(t *testing.T) {
+	const project = "/core/v1/projects/project/skills/skill_missing"
+	for _, test := range []struct {
+		name, path string
+		err        error
+		status     int
+		body       string
+	}{
+		{"default version", "/v1/skills/skill_missing/versions/1", skills.ErrDefaultVersion, http.StatusBadRequest,
+			`{"error":{"message":"Cannot delete the default skill version.","type":"invalid_request_error","code":"invalid_value","param":"version"}}`},
+		{"cursor", "/v1/skills/skill_missing/versions", &skills.CursorError{Message: "Skill version cursor does not match this skill."}, http.StatusBadRequest,
+			`{"error":{"message":"Skill version cursor does not match this skill.","type":"invalid_request_error","code":"invalid_value","param":"after"}}`},
+		{"project cursor", project + "/versions", &skills.CursorError{Message: "Skill version cursor does not match this skill."}, http.StatusBadRequest,
+			`{"error":{"message":"Skill version cursor does not match this skill.","type":"invalid_request_error","code":"invalid_request_error","param":null}}`},
+		{"list not found", "/v1/skills", skills.ErrNotFound, http.StatusNotFound,
+			`{"error":{"message":"Resource not found.","type":"invalid_request_error","code":null,"param":null}}`},
+		{"version not found", "/v1/skills/skill_missing/versions/1", fmt.Errorf("lookup: %w", skills.ErrNotFound), http.StatusNotFound,
+			`{"error":{"message":"Resource not found.","type":"invalid_request_error","code":null,"param":null}}`},
+		{"project not found", project, skills.ErrNotFound, http.StatusNotFound,
+			`{"error":{"message":"Resource not found.","type":"not_found_error","code":"not_found_error","param":null}}`},
+		{"invalid input", "/v1/skills", skills.ErrInvalidInput, http.StatusBadRequest,
+			`{"error":{"message":"Invalid resource identifier or request limits.","type":"invalid_request_error","code":"invalid_request","param":null}}`},
+		{"unstorable text", "/v1/skills", fmt.Errorf("create: %w", textvalue.ErrUnstorable), http.StatusBadRequest,
+			`{"error":{"message":"` + unstorableTextMessage + `","type":"invalid_request_error","code":"invalid_request_error","param":null}}`},
+		{"audit source", "/v1/skills", fmt.Errorf("record: %w", writeaudit.ErrInvalidSource), http.StatusBadRequest,
+			`{"error":{"message":"` + invalidInputMessage + `","type":"invalid_request_error","code":"invalid_request","param":null}}`},
+		{"credential key missing", "/v1/skills", credentialcrypto.ErrUnavailable, http.StatusServiceUnavailable,
+			`{"error":{"message":"Credential encryption is not configured on this service.","type":"server_error","code":"credential_storage_unavailable","param":null}}`},
+		{"unknown", "/v1/skills", errors.New("connection reset"), http.StatusInternalServerError,
+			`{"error":{"message":"The operation could not be completed.","type":"server_error","code":"internal_error","param":null}}`},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			response := httptest.NewRecorder()
+			writeSkillsError(response, httptest.NewRequest(http.MethodGet, test.path, nil), test.err)
+			if response.Code != test.status || response.Body.String() != test.body+"\n" {
+				t.Fatalf("%d %s", response.Code, response.Body)
+			}
+		})
+	}
+}