diff --git a/cmd/thv/app/skill_helpers.go b/cmd/thv/app/skill_helpers.go index 06f0bdc6f4..deaa640ae0 100644 --- a/cmd/thv/app/skill_helpers.go +++ b/cmd/thv/app/skill_helpers.go @@ -10,6 +10,7 @@ import ( "github.com/spf13/cobra" + tclient "github.com/stacklok/toolhive/pkg/client" "github.com/stacklok/toolhive/pkg/skills" skillclient "github.com/stacklok/toolhive/pkg/skills/client" ) @@ -65,3 +66,18 @@ func validateProjectRootForScope(scopeVar, projectRootVar *string) func(*cobra.C return nil } } + +// resolveProjectRoot returns explicit unchanged when non-empty. Otherwise it +// auto-detects the project root by walking up from the current working +// directory looking for a .git directory or file, matching the lock file's +// commit-to-git-root assumption used by "thv skill sync" and "thv skill upgrade". +func resolveProjectRoot(explicit string) (string, error) { + if explicit != "" { + return explicit, nil + } + root, err := tclient.DetectProjectRoot("") + if err != nil { + return "", fmt.Errorf("%w; run inside a git repository or pass --project-root explicitly", err) + } + return root, nil +} diff --git a/cmd/thv/app/skill_sync.go b/cmd/thv/app/skill_sync.go new file mode 100644 index 0000000000..2ee9680b46 --- /dev/null +++ b/cmd/thv/app/skill_sync.go @@ -0,0 +1,102 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package app + +import ( + "encoding/json" + "fmt" + + "github.com/spf13/cobra" + + "github.com/stacklok/toolhive/pkg/skills" +) + +var ( + skillSyncProjectRoot string + skillSyncClientsRaw string + skillSyncPrune bool + skillSyncFormat string +) + +var skillSyncCmd = &cobra.Command{ + Use: "sync", + Short: "Install skills exactly as pinned in the project's lock file", + Long: `Sync installs every skill pinned in toolhive.lock.yaml at its exact +recorded digest, restoring skills that are missing or have drifted from the +pinned state. Project-scoped skills installed outside of the lock file are +reported as unmanaged, or removed with --prune. + +The project root is auto-detected from the current directory (nearest +enclosing git repository) unless --project-root is given.`, + PreRunE: chainPreRunE(ValidateFormat(&skillSyncFormat)), + RunE: skillSyncCmdFunc, +} + +func init() { + skillCmd.AddCommand(skillSyncCmd) + + skillSyncCmd.Flags().StringVar(&skillSyncClientsRaw, "clients", "", + `Comma-separated target client apps (e.g. claude-code,opencode), or "all" for every available client`) + skillSyncCmd.Flags().BoolVar(&skillSyncPrune, "prune", false, + "Uninstall project-scoped skills that are not present in the lock file") + skillSyncCmd.Flags().StringVar(&skillSyncProjectRoot, "project-root", "", + "Project root path (auto-detected from the current directory if omitted)") + AddFormatFlag(skillSyncCmd, &skillSyncFormat) +} + +func skillSyncCmdFunc(cmd *cobra.Command, _ []string) error { + projectRoot, err := resolveProjectRoot(skillSyncProjectRoot) + if err != nil { + return err + } + + c := newSkillClient(cmd.Context()) + result, err := c.Sync(cmd.Context(), skills.SyncOptions{ + ProjectRoot: projectRoot, + Clients: parseSkillInstallClients(skillSyncClientsRaw), + Prune: skillSyncPrune, + }) + if err != nil { + return formatSkillError("sync skills", err) + } + + switch skillSyncFormat { + case FormatJSON: + data, jsonErr := json.MarshalIndent(result, "", " ") + if jsonErr != nil { + return fmt.Errorf("failed to marshal JSON: %w", jsonErr) + } + fmt.Println(string(data)) + default: + printSyncResultText(result) + } + return nil +} + +func printSyncResultText(result *skills.SyncResult) { + printSkillNameList("Installed", result.Installed) + printSkillNameList("Up to date", result.UpToDate) + printSkillNameList("Unmanaged (not in lock file; re-run with --prune to remove)", result.Unmanaged) + printSkillNameList("Pruned", result.Pruned) + if len(result.Failed) > 0 { + fmt.Println("Failed:") + for _, f := range result.Failed { + fmt.Printf(" %s: %s\n", f.Name, f.Error) + } + } + if len(result.Installed) == 0 && len(result.UpToDate) == 0 && len(result.Unmanaged) == 0 && + len(result.Pruned) == 0 && len(result.Failed) == 0 { + fmt.Println("Nothing to sync: lock file is empty") + } +} + +func printSkillNameList(label string, names []string) { + if len(names) == 0 { + return + } + fmt.Printf("%s:\n", label) + for _, name := range names { + fmt.Printf(" %s\n", name) + } +} diff --git a/cmd/thv/app/skill_sync_test.go b/cmd/thv/app/skill_sync_test.go new file mode 100644 index 0000000000..63669f1cc8 --- /dev/null +++ b/cmd/thv/app/skill_sync_test.go @@ -0,0 +1,79 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package app + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// nolint:paralleltest // subtests change the process-wide working directory via os.Chdir +func TestResolveProjectRoot(t *testing.T) { + t.Run("explicit value passes through without touching the filesystem", func(t *testing.T) { + got, err := resolveProjectRoot("/explicit/path") + require.NoError(t, err) + assert.Equal(t, "/explicit/path", got) + }) + + t.Run("empty value auto-detects from the current directory", func(t *testing.T) { + repoRoot := t.TempDir() + resolved, err := filepath.EvalSymlinks(repoRoot) + require.NoError(t, err) + require.NoError(t, os.MkdirAll(filepath.Join(resolved, ".git"), 0o755)) + + nested := filepath.Join(resolved, "a", "b") + require.NoError(t, os.MkdirAll(nested, 0o755)) + + orig, err := os.Getwd() + require.NoError(t, err) + t.Cleanup(func() { _ = os.Chdir(orig) }) + require.NoError(t, os.Chdir(nested)) + + got, err := resolveProjectRoot("") + require.NoError(t, err) + assert.Equal(t, resolved, got) + }) + + t.Run("empty value outside a git repo returns a helpful error", func(t *testing.T) { + dir := t.TempDir() + resolved, err := filepath.EvalSymlinks(dir) + require.NoError(t, err) + + orig, err := os.Getwd() + require.NoError(t, err) + t.Cleanup(func() { _ = os.Chdir(orig) }) + require.NoError(t, os.Chdir(resolved)) + + _, err = resolveProjectRoot("") + require.Error(t, err) + assert.Contains(t, err.Error(), "--project-root") + }) +} + +func TestShortDigest(t *testing.T) { + t.Parallel() + tests := []struct { + name string + in string + want string + }{ + {name: "empty", in: "", want: ""}, + {name: "short digest unchanged", in: "sha256:abc", want: "sha256:abc"}, + { + name: "long digest truncated to 19 chars", + in: "sha256:abcdef0123456789fedcba9876543210", + want: "sha256:abcdef012345", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.want, shortDigest(tt.in)) + }) + } +} diff --git a/cmd/thv/app/skill_upgrade.go b/cmd/thv/app/skill_upgrade.go new file mode 100644 index 0000000000..561bf74154 --- /dev/null +++ b/cmd/thv/app/skill_upgrade.go @@ -0,0 +1,110 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package app + +import ( + "encoding/json" + "fmt" + "os" + "text/tabwriter" + + "github.com/spf13/cobra" + + "github.com/stacklok/toolhive/pkg/skills" +) + +var ( + skillUpgradeProjectRoot string + skillUpgradeClientsRaw string + skillUpgradeDryRun bool + skillUpgradeFormat string +) + +var skillUpgradeCmd = &cobra.Command{ + Use: "upgrade [skill-name...]", + Short: "Check for and install newer content for locked skills", + Long: `Upgrade re-resolves each lock file entry's original source (registry +name, OCI reference, or git reference) and installs any newer content it +finds, updating toolhive.lock.yaml. Entries pinned to an immutable reference +(an OCI digest or a full git commit hash) are reported as not upgradable. + +With no arguments, every entry in the lock file is checked. Use --dry-run to +see what would change without installing anything. + +The project root is auto-detected from the current directory (nearest +enclosing git repository) unless --project-root is given.`, + PreRunE: chainPreRunE(ValidateFormat(&skillUpgradeFormat)), + RunE: skillUpgradeCmdFunc, +} + +func init() { + skillCmd.AddCommand(skillUpgradeCmd) + + skillUpgradeCmd.Flags().StringVar(&skillUpgradeClientsRaw, "clients", "", + `Comma-separated target client apps (e.g. claude-code,opencode), or "all" for every available client`) + skillUpgradeCmd.Flags().BoolVar(&skillUpgradeDryRun, "dry-run", false, + "Report what would change without installing anything") + skillUpgradeCmd.Flags().StringVar(&skillUpgradeProjectRoot, "project-root", "", + "Project root path (auto-detected from the current directory if omitted)") + AddFormatFlag(skillUpgradeCmd, &skillUpgradeFormat) +} + +func skillUpgradeCmdFunc(cmd *cobra.Command, args []string) error { + projectRoot, err := resolveProjectRoot(skillUpgradeProjectRoot) + if err != nil { + return err + } + + c := newSkillClient(cmd.Context()) + result, err := c.Upgrade(cmd.Context(), skills.UpgradeOptions{ + ProjectRoot: projectRoot, + Names: args, + DryRun: skillUpgradeDryRun, + Clients: parseSkillInstallClients(skillUpgradeClientsRaw), + }) + if err != nil { + return formatSkillError("upgrade skills", err) + } + + switch skillUpgradeFormat { + case FormatJSON: + data, jsonErr := json.MarshalIndent(result, "", " ") + if jsonErr != nil { + return fmt.Errorf("failed to marshal JSON: %w", jsonErr) + } + fmt.Println(string(data)) + default: + printUpgradeResultText(result) + } + return nil +} + +func printUpgradeResultText(result *skills.UpgradeResult) { + if len(result.Outcomes) == 0 { + fmt.Println("Nothing to upgrade: lock file is empty") + return + } + w := tabwriter.NewWriter(os.Stdout, 0, 0, 3, ' ', 0) + _, _ = fmt.Fprintln(w, "NAME\tSTATUS\tOLD DIGEST\tNEW DIGEST") + for _, o := range result.Outcomes { + _, _ = fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", o.Name, o.Status, shortDigest(o.OldDigest), shortDigest(o.NewDigest)) + } + _ = w.Flush() + + for _, o := range result.Outcomes { + if o.Status == skills.UpgradeStatusFailed && o.Error != "" { + fmt.Printf(" %s: %s\n", o.Name, o.Error) + } + } +} + +// shortDigest truncates a digest to "sha256:" plus 12 hex characters for +// compact table display, matching the convention used by container tooling. +func shortDigest(d string) string { + const shortLen = 19 // len("sha256:") + 12 + if len(d) > shortLen { + return d[:shortLen] + } + return d +} diff --git a/docs/arch/12-skills-system.md b/docs/arch/12-skills-system.md index e0021eb346..18ff828724 100644 --- a/docs/arch/12-skills-system.md +++ b/docs/arch/12-skills-system.md @@ -37,11 +37,13 @@ graph TB Packager[SkillPackager
Build OCI artifacts] Installer[Installer
Extract + validate] Store[SkillStore
SQLite persistence] + Lock[Lockfile
pkg/skills/lockfile] end subgraph "Client Filesystem" UserSkills["~/.claude/skills/
(user scope)"] ProjectSkills[".claude/skills/
(project scope)"] + LockFile["toolhive.lock.yaml
(project root)"] end subgraph "Access Layer" @@ -65,14 +67,17 @@ graph TB SVC --> Packager SVC --> Installer SVC --> Store + SVC --> Lock Installer --> UserSkills Installer --> ProjectSkills + Lock --> LockFile style SVC fill:#90caf9,stroke:#1565c0,stroke-width:2px style Store fill:#e3f2fd style UserSkills fill:#c8e6c9,stroke:#2e7d32,stroke-width:2px style ProjectSkills fill:#c8e6c9,stroke:#2e7d32,stroke-width:2px + style LockFile fill:#c8e6c9,stroke:#2e7d32,stroke-width:2px style CLI fill:#fff9c4 style API fill:#fff9c4 ``` @@ -258,6 +263,43 @@ Removes the skill files from the filesystem, deletes the database record, and re **Implementation:** `pkg/skills/skillsvc/uninstall.go` (Uninstall), `pkg/groups/skills.go` (RemoveSkillFromAllGroups) +## Project Lock File + +Every `--scope project` install writes an entry to a `toolhive.lock.yaml` file at the project root, alongside `.git`. The lock file pins the exact **name**, **version**, **source**, **resolved reference**, and **digest** of each project-scoped skill so a team can commit it to version control, restore an identical set of skills on another machine, and later check for newer content — the same role `package-lock.json` or `go.sum` play for other package managers. User-scope installs never touch the lock file. + +```yaml +version: 1 +skills: + - name: code-review + version: 1.0.0 + source: code-review # what the user/registry resolver originally typed + resolvedReference: ghcr.io/org/code-review:1.0.0 + digest: sha256:9f2b1e... +``` + +The lock file is **client-agnostic**: it pins skill content, not which client applications installed it. `thv skill sync` installs pinned skills for whichever clients are targeted (all detected clients by default, or `--clients`), independent of what was targeted at install time. + +### Sync + +```bash +thv skill sync # restore every pinned skill at its exact digest +thv skill sync --prune # also uninstall project-scoped skills absent from the lock file +``` + +For each lock entry, `Sync` compares the currently installed digest (if any) against the pinned digest. A mismatch or missing install triggers a fresh install pinned to `resolvedReference@digest` — an OCI digest reference, or a git reference pinned to the exact commit hash — so the installed content is byte-for-byte reproducible regardless of what a mutable tag or branch currently points to. Project-scoped skills installed outside the lock file are reported as unmanaged, or removed when `--prune` is set. A sync never rewrites the lock file itself: it only makes the filesystem and database match what is already pinned. + +### Upgrade + +```bash +thv skill upgrade # check every locked skill for newer content +thv skill upgrade code-review # check specific skills only +thv skill upgrade --dry-run # report without installing or writing the lock file +``` + +`Upgrade` re-resolves each entry's original `source` exactly as a fresh `thv skill install ` would (registry name -> catalog lookup, git branch/tag -> current head, OCI tag -> current digest) and compares the result against the pinned digest. A different digest installs the new content and rewrites the entry's `resolvedReference`, `digest`, and `version` — `source` never changes, so future upgrades keep re-resolving the same catalog name or ref. Entries already pinned to an immutable reference (an OCI `@sha256:...` digest or a full 40-character git commit hash) are reported as `not-upgradable` without contacting the network, since re-resolving them can never surface different content. + +**Implementation:** `pkg/skills/lockfile/` (schema, file-locked read/write/upsert/remove), `pkg/skills/skillsvc/sync.go` (Sync), `pkg/skills/skillsvc/upgrade.go` (Upgrade), `pkg/skills/skillsvc/pin.go` (digest-pinning and immutability helpers) + ## Git-Based Skill Resolution Skills can be installed directly from git repositories using the `git://` scheme: @@ -342,6 +384,8 @@ oci_tags table (reserved; not currently populated) | `GET` | `/builds` | List local builds | | `DELETE` | `/builds/{tag}` | Delete a local build | | `GET` | `/content` | Get a skill's SKILL.md body and file listing for a reference | +| `POST` | `/sync` | Install pinned skills from a project's lock file | +| `POST` | `/upgrade` | Re-resolve locked skills and install newer content, if any | **Implementation:** `pkg/api/v1/skills.go` @@ -366,7 +410,9 @@ thv skill ├── build [path] Build skill to OCI artifact ├── push [reference] Push built skill to registry ├── builds List locally-built OCI artifacts -└── builds remove [tag] Delete a locally-built artifact +├── builds remove [tag] Delete a locally-built artifact +├── sync Install skills exactly as pinned in toolhive.lock.yaml +└── upgrade [name...] Check for and install newer content for locked skills ``` **Implementation:** `cmd/thv/app/skill*.go` @@ -448,6 +494,7 @@ ToolHive owns the installation lifecycle, scoping model, CLI/API interfaces, and | Parsing | `pkg/skills/parser.go` | | Extraction | `pkg/skills/installer.go` | | Git resolution | `pkg/skills/gitresolver/` | +| Project lock file | `pkg/skills/lockfile/` | | Storage interface | `pkg/storage/interfaces.go` | | SQLite backend | `pkg/storage/sqlite/skill_store.go` | | REST API | `pkg/api/v1/skills.go` | diff --git a/docs/cli/thv_skill.md b/docs/cli/thv_skill.md index 3a93a62f77..23f10db802 100644 --- a/docs/cli/thv_skill.md +++ b/docs/cli/thv_skill.md @@ -38,6 +38,8 @@ The skill command provides subcommands to manage skills. * [thv skill install](thv_skill_install.md) - Install a skill * [thv skill list](thv_skill_list.md) - List installed skills * [thv skill push](thv_skill_push.md) - Push a built skill +* [thv skill sync](thv_skill_sync.md) - Install skills exactly as pinned in the project's lock file * [thv skill uninstall](thv_skill_uninstall.md) - Uninstall a skill +* [thv skill upgrade](thv_skill_upgrade.md) - Check for and install newer content for locked skills * [thv skill validate](thv_skill_validate.md) - Validate a skill definition diff --git a/docs/cli/thv_skill_sync.md b/docs/cli/thv_skill_sync.md new file mode 100644 index 0000000000..57d217bad6 --- /dev/null +++ b/docs/cli/thv_skill_sync.md @@ -0,0 +1,49 @@ +--- +title: thv skill sync +hide_title: true +description: Reference for ToolHive CLI command `thv skill sync` +last_update: + author: autogenerated +slug: thv_skill_sync +mdx: + format: md +--- + +## thv skill sync + +Install skills exactly as pinned in the project's lock file + +### Synopsis + +Sync installs every skill pinned in toolhive.lock.yaml at its exact +recorded digest, restoring skills that are missing or have drifted from the +pinned state. Project-scoped skills installed outside of the lock file are +reported as unmanaged, or removed with --prune. + +The project root is auto-detected from the current directory (nearest +enclosing git repository) unless --project-root is given. + +``` +thv skill sync [flags] +``` + +### Options + +``` + --clients string Comma-separated target client apps (e.g. claude-code,opencode), or "all" for every available client + --format string Output format (json, text) (default "text") + -h, --help help for sync + --project-root string Project root path (auto-detected from the current directory if omitted) + --prune Uninstall project-scoped skills that are not present in the lock file +``` + +### Options inherited from parent commands + +``` + --debug Enable debug mode +``` + +### SEE ALSO + +* [thv skill](thv_skill.md) - Manage skills + diff --git a/docs/cli/thv_skill_upgrade.md b/docs/cli/thv_skill_upgrade.md new file mode 100644 index 0000000000..4e7d058813 --- /dev/null +++ b/docs/cli/thv_skill_upgrade.md @@ -0,0 +1,52 @@ +--- +title: thv skill upgrade +hide_title: true +description: Reference for ToolHive CLI command `thv skill upgrade` +last_update: + author: autogenerated +slug: thv_skill_upgrade +mdx: + format: md +--- + +## thv skill upgrade + +Check for and install newer content for locked skills + +### Synopsis + +Upgrade re-resolves each lock file entry's original source (registry +name, OCI reference, or git reference) and installs any newer content it +finds, updating toolhive.lock.yaml. Entries pinned to an immutable reference +(an OCI digest or a full git commit hash) are reported as not upgradable. + +With no arguments, every entry in the lock file is checked. Use --dry-run to +see what would change without installing anything. + +The project root is auto-detected from the current directory (nearest +enclosing git repository) unless --project-root is given. + +``` +thv skill upgrade [skill-name...] [flags] +``` + +### Options + +``` + --clients string Comma-separated target client apps (e.g. claude-code,opencode), or "all" for every available client + --dry-run Report what would change without installing anything + --format string Output format (json, text) (default "text") + -h, --help help for upgrade + --project-root string Project root path (auto-detected from the current directory if omitted) +``` + +### Options inherited from parent commands + +``` + --debug Enable debug mode +``` + +### SEE ALSO + +* [thv skill](thv_skill.md) - Manage skills + diff --git a/docs/server/docs.go b/docs/server/docs.go index 5895881675..27c31dbd22 100644 --- a/docs/server/docs.go +++ b/docs/server/docs.go @@ -1812,6 +1812,117 @@ const docTemplate = `{ }, "type": "object" }, + "github_com_stacklok_toolhive_pkg_skills.SyncFailure": { + "properties": { + "error": { + "description": "Error is a human-readable description of the failure.", + "type": "string" + }, + "name": { + "description": "Name is the skill name that failed.", + "type": "string" + } + }, + "type": "object" + }, + "github_com_stacklok_toolhive_pkg_skills.SyncResult": { + "properties": { + "failed": { + "description": "Failed lists skills that could not be synced, with the reason for each.", + "items": { + "$ref": "#/components/schemas/github_com_stacklok_toolhive_pkg_skills.SyncFailure" + }, + "type": "array", + "uniqueItems": false + }, + "installed": { + "description": "Installed lists skills that were installed or reinstalled to match the lock file.", + "items": { + "type": "string" + }, + "type": "array", + "uniqueItems": false + }, + "pruned": { + "description": "Pruned lists unmanaged skills that were uninstalled because Prune was set.", + "items": { + "type": "string" + }, + "type": "array", + "uniqueItems": false + }, + "unmanaged": { + "description": "Unmanaged lists project-scoped skills present on disk but absent from the lock file.", + "items": { + "type": "string" + }, + "type": "array", + "uniqueItems": false + }, + "up_to_date": { + "description": "UpToDate lists skills that already matched the lock file's pinned digest.", + "items": { + "type": "string" + }, + "type": "array", + "uniqueItems": false + } + }, + "type": "object" + }, + "github_com_stacklok_toolhive_pkg_skills.UpgradeOutcome": { + "properties": { + "error": { + "description": "Error is a human-readable description of the failure, set only when Status is UpgradeStatusFailed.", + "type": "string" + }, + "name": { + "description": "Name is the skill name.", + "type": "string" + }, + "new_digest": { + "description": "NewDigest is the digest the source currently resolves to. Equal to\nOldDigest when Status is UpgradeStatusUpToDate.", + "type": "string" + }, + "old_digest": { + "description": "OldDigest is the digest pinned in the lock file before this operation.", + "type": "string" + }, + "status": { + "$ref": "#/components/schemas/github_com_stacklok_toolhive_pkg_skills.UpgradeStatus" + } + }, + "type": "object" + }, + "github_com_stacklok_toolhive_pkg_skills.UpgradeResult": { + "properties": { + "outcomes": { + "description": "Outcomes contains one entry per skill considered for upgrade.", + "items": { + "$ref": "#/components/schemas/github_com_stacklok_toolhive_pkg_skills.UpgradeOutcome" + }, + "type": "array", + "uniqueItems": false + } + }, + "type": "object" + }, + "github_com_stacklok_toolhive_pkg_skills.UpgradeStatus": { + "description": "Status is the outcome of the upgrade attempt.", + "enum": [ + "upgraded", + "up-to-date", + "not-upgradable", + "failed" + ], + "type": "string", + "x-enum-varnames": [ + "UpgradeStatusUpgraded", + "UpgradeStatusUpToDate", + "UpgradeStatusNotUpgradable", + "UpgradeStatusFailed" + ] + }, "github_com_stacklok_toolhive_pkg_skills.ValidationResult": { "properties": { "errors": { @@ -3405,6 +3516,28 @@ const docTemplate = `{ }, "type": "object" }, + "pkg_api_v1.syncSkillsRequest": { + "description": "Request to sync a project's installed skills to match its lock file", + "properties": { + "clients": { + "description": "Clients lists target client identifiers, or omit for every detected client", + "items": { + "type": "string" + }, + "type": "array", + "uniqueItems": false + }, + "project_root": { + "description": "ProjectRoot is the project root path whose lock file should be synced", + "type": "string" + }, + "prune": { + "description": "Prune removes project-scoped skills that are installed but not present in the lock file", + "type": "boolean" + } + }, + "type": "object" + }, "pkg_api_v1.toolOverride": { "description": "Tool override", "properties": { @@ -3605,6 +3738,36 @@ const docTemplate = `{ }, "type": "object" }, + "pkg_api_v1.upgradeSkillsRequest": { + "description": "Request to check for and install newer content for locked skills", + "properties": { + "clients": { + "description": "Clients lists target client identifiers, or omit for every detected client", + "items": { + "type": "string" + }, + "type": "array", + "uniqueItems": false + }, + "dry_run": { + "description": "DryRun reports what would change without installing anything", + "type": "boolean" + }, + "names": { + "description": "Names restricts the upgrade to specific skill names, or omit for every locked skill", + "items": { + "type": "string" + }, + "type": "array", + "uniqueItems": false + }, + "project_root": { + "description": "ProjectRoot is the project root path whose lock file should be upgraded", + "type": "string" + } + }, + "type": "object" + }, "pkg_api_v1.validateSkillRequest": { "description": "Request to validate a skill definition", "properties": { @@ -6209,6 +6372,138 @@ const docTemplate = `{ ] } }, + "/api/v1beta/skills/sync": { + "post": { + "description": "Install the exact pinned digest for every lock file entry, restoring drift", + "requestBody": { + "content": { + "application/json": { + "schema": { + "oneOf": [ + { + "type": "object" + }, + { + "$ref": "#/components/schemas/pkg_api_v1.syncSkillsRequest", + "summary": "request", + "description": "Sync request" + } + ] + } + } + }, + "description": "Sync request", + "required": true + }, + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github_com_stacklok_toolhive_pkg_skills.SyncResult" + } + } + }, + "description": "OK" + }, + "400": { + "content": { + "application/json": { + "schema": { + "type": "string" + } + } + }, + "description": "Bad Request" + }, + "500": { + "content": { + "application/json": { + "schema": { + "type": "string" + } + } + }, + "description": "Internal Server Error" + } + }, + "summary": "Sync a project's skills to its lock file", + "tags": [ + "skills" + ] + } + }, + "/api/v1beta/skills/upgrade": { + "post": { + "description": "Re-resolve each lock file entry's source and install newer content, if any", + "requestBody": { + "content": { + "application/json": { + "schema": { + "oneOf": [ + { + "type": "object" + }, + { + "$ref": "#/components/schemas/pkg_api_v1.upgradeSkillsRequest", + "summary": "request", + "description": "Upgrade request" + } + ] + } + } + }, + "description": "Upgrade request", + "required": true + }, + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github_com_stacklok_toolhive_pkg_skills.UpgradeResult" + } + } + }, + "description": "OK" + }, + "400": { + "content": { + "application/json": { + "schema": { + "type": "string" + } + } + }, + "description": "Bad Request" + }, + "404": { + "content": { + "application/json": { + "schema": { + "type": "string" + } + } + }, + "description": "Not Found (requested skill name not in lock file)" + }, + "500": { + "content": { + "application/json": { + "schema": { + "type": "string" + } + } + }, + "description": "Internal Server Error" + } + }, + "summary": "Upgrade a project's locked skills", + "tags": [ + "skills" + ] + } + }, "/api/v1beta/skills/validate": { "post": { "description": "Validate a skill definition", diff --git a/docs/server/swagger.json b/docs/server/swagger.json index 07e9d5089f..8882566ba1 100644 --- a/docs/server/swagger.json +++ b/docs/server/swagger.json @@ -1805,6 +1805,117 @@ }, "type": "object" }, + "github_com_stacklok_toolhive_pkg_skills.SyncFailure": { + "properties": { + "error": { + "description": "Error is a human-readable description of the failure.", + "type": "string" + }, + "name": { + "description": "Name is the skill name that failed.", + "type": "string" + } + }, + "type": "object" + }, + "github_com_stacklok_toolhive_pkg_skills.SyncResult": { + "properties": { + "failed": { + "description": "Failed lists skills that could not be synced, with the reason for each.", + "items": { + "$ref": "#/components/schemas/github_com_stacklok_toolhive_pkg_skills.SyncFailure" + }, + "type": "array", + "uniqueItems": false + }, + "installed": { + "description": "Installed lists skills that were installed or reinstalled to match the lock file.", + "items": { + "type": "string" + }, + "type": "array", + "uniqueItems": false + }, + "pruned": { + "description": "Pruned lists unmanaged skills that were uninstalled because Prune was set.", + "items": { + "type": "string" + }, + "type": "array", + "uniqueItems": false + }, + "unmanaged": { + "description": "Unmanaged lists project-scoped skills present on disk but absent from the lock file.", + "items": { + "type": "string" + }, + "type": "array", + "uniqueItems": false + }, + "up_to_date": { + "description": "UpToDate lists skills that already matched the lock file's pinned digest.", + "items": { + "type": "string" + }, + "type": "array", + "uniqueItems": false + } + }, + "type": "object" + }, + "github_com_stacklok_toolhive_pkg_skills.UpgradeOutcome": { + "properties": { + "error": { + "description": "Error is a human-readable description of the failure, set only when Status is UpgradeStatusFailed.", + "type": "string" + }, + "name": { + "description": "Name is the skill name.", + "type": "string" + }, + "new_digest": { + "description": "NewDigest is the digest the source currently resolves to. Equal to\nOldDigest when Status is UpgradeStatusUpToDate.", + "type": "string" + }, + "old_digest": { + "description": "OldDigest is the digest pinned in the lock file before this operation.", + "type": "string" + }, + "status": { + "$ref": "#/components/schemas/github_com_stacklok_toolhive_pkg_skills.UpgradeStatus" + } + }, + "type": "object" + }, + "github_com_stacklok_toolhive_pkg_skills.UpgradeResult": { + "properties": { + "outcomes": { + "description": "Outcomes contains one entry per skill considered for upgrade.", + "items": { + "$ref": "#/components/schemas/github_com_stacklok_toolhive_pkg_skills.UpgradeOutcome" + }, + "type": "array", + "uniqueItems": false + } + }, + "type": "object" + }, + "github_com_stacklok_toolhive_pkg_skills.UpgradeStatus": { + "description": "Status is the outcome of the upgrade attempt.", + "enum": [ + "upgraded", + "up-to-date", + "not-upgradable", + "failed" + ], + "type": "string", + "x-enum-varnames": [ + "UpgradeStatusUpgraded", + "UpgradeStatusUpToDate", + "UpgradeStatusNotUpgradable", + "UpgradeStatusFailed" + ] + }, "github_com_stacklok_toolhive_pkg_skills.ValidationResult": { "properties": { "errors": { @@ -3398,6 +3509,28 @@ }, "type": "object" }, + "pkg_api_v1.syncSkillsRequest": { + "description": "Request to sync a project's installed skills to match its lock file", + "properties": { + "clients": { + "description": "Clients lists target client identifiers, or omit for every detected client", + "items": { + "type": "string" + }, + "type": "array", + "uniqueItems": false + }, + "project_root": { + "description": "ProjectRoot is the project root path whose lock file should be synced", + "type": "string" + }, + "prune": { + "description": "Prune removes project-scoped skills that are installed but not present in the lock file", + "type": "boolean" + } + }, + "type": "object" + }, "pkg_api_v1.toolOverride": { "description": "Tool override", "properties": { @@ -3598,6 +3731,36 @@ }, "type": "object" }, + "pkg_api_v1.upgradeSkillsRequest": { + "description": "Request to check for and install newer content for locked skills", + "properties": { + "clients": { + "description": "Clients lists target client identifiers, or omit for every detected client", + "items": { + "type": "string" + }, + "type": "array", + "uniqueItems": false + }, + "dry_run": { + "description": "DryRun reports what would change without installing anything", + "type": "boolean" + }, + "names": { + "description": "Names restricts the upgrade to specific skill names, or omit for every locked skill", + "items": { + "type": "string" + }, + "type": "array", + "uniqueItems": false + }, + "project_root": { + "description": "ProjectRoot is the project root path whose lock file should be upgraded", + "type": "string" + } + }, + "type": "object" + }, "pkg_api_v1.validateSkillRequest": { "description": "Request to validate a skill definition", "properties": { @@ -6202,6 +6365,138 @@ ] } }, + "/api/v1beta/skills/sync": { + "post": { + "description": "Install the exact pinned digest for every lock file entry, restoring drift", + "requestBody": { + "content": { + "application/json": { + "schema": { + "oneOf": [ + { + "type": "object" + }, + { + "$ref": "#/components/schemas/pkg_api_v1.syncSkillsRequest", + "summary": "request", + "description": "Sync request" + } + ] + } + } + }, + "description": "Sync request", + "required": true + }, + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github_com_stacklok_toolhive_pkg_skills.SyncResult" + } + } + }, + "description": "OK" + }, + "400": { + "content": { + "application/json": { + "schema": { + "type": "string" + } + } + }, + "description": "Bad Request" + }, + "500": { + "content": { + "application/json": { + "schema": { + "type": "string" + } + } + }, + "description": "Internal Server Error" + } + }, + "summary": "Sync a project's skills to its lock file", + "tags": [ + "skills" + ] + } + }, + "/api/v1beta/skills/upgrade": { + "post": { + "description": "Re-resolve each lock file entry's source and install newer content, if any", + "requestBody": { + "content": { + "application/json": { + "schema": { + "oneOf": [ + { + "type": "object" + }, + { + "$ref": "#/components/schemas/pkg_api_v1.upgradeSkillsRequest", + "summary": "request", + "description": "Upgrade request" + } + ] + } + } + }, + "description": "Upgrade request", + "required": true + }, + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/github_com_stacklok_toolhive_pkg_skills.UpgradeResult" + } + } + }, + "description": "OK" + }, + "400": { + "content": { + "application/json": { + "schema": { + "type": "string" + } + } + }, + "description": "Bad Request" + }, + "404": { + "content": { + "application/json": { + "schema": { + "type": "string" + } + } + }, + "description": "Not Found (requested skill name not in lock file)" + }, + "500": { + "content": { + "application/json": { + "schema": { + "type": "string" + } + } + }, + "description": "Internal Server Error" + } + }, + "summary": "Upgrade a project's locked skills", + "tags": [ + "skills" + ] + } + }, "/api/v1beta/skills/validate": { "post": { "description": "Validate a skill definition", diff --git a/docs/server/swagger.yaml b/docs/server/swagger.yaml index c12579cae9..34b982e354 100644 --- a/docs/server/swagger.yaml +++ b/docs/server/swagger.yaml @@ -1786,6 +1786,96 @@ components: description: Version is the semantic version of the skill. type: string type: object + github_com_stacklok_toolhive_pkg_skills.SyncFailure: + properties: + error: + description: Error is a human-readable description of the failure. + type: string + name: + description: Name is the skill name that failed. + type: string + type: object + github_com_stacklok_toolhive_pkg_skills.SyncResult: + properties: + failed: + description: Failed lists skills that could not be synced, with the reason + for each. + items: + $ref: '#/components/schemas/github_com_stacklok_toolhive_pkg_skills.SyncFailure' + type: array + uniqueItems: false + installed: + description: Installed lists skills that were installed or reinstalled to + match the lock file. + items: + type: string + type: array + uniqueItems: false + pruned: + description: Pruned lists unmanaged skills that were uninstalled because + Prune was set. + items: + type: string + type: array + uniqueItems: false + unmanaged: + description: Unmanaged lists project-scoped skills present on disk but absent + from the lock file. + items: + type: string + type: array + uniqueItems: false + up_to_date: + description: UpToDate lists skills that already matched the lock file's + pinned digest. + items: + type: string + type: array + uniqueItems: false + type: object + github_com_stacklok_toolhive_pkg_skills.UpgradeOutcome: + properties: + error: + description: Error is a human-readable description of the failure, set only + when Status is UpgradeStatusFailed. + type: string + name: + description: Name is the skill name. + type: string + new_digest: + description: |- + NewDigest is the digest the source currently resolves to. Equal to + OldDigest when Status is UpgradeStatusUpToDate. + type: string + old_digest: + description: OldDigest is the digest pinned in the lock file before this + operation. + type: string + status: + $ref: '#/components/schemas/github_com_stacklok_toolhive_pkg_skills.UpgradeStatus' + type: object + github_com_stacklok_toolhive_pkg_skills.UpgradeResult: + properties: + outcomes: + description: Outcomes contains one entry per skill considered for upgrade. + items: + $ref: '#/components/schemas/github_com_stacklok_toolhive_pkg_skills.UpgradeOutcome' + type: array + uniqueItems: false + type: object + github_com_stacklok_toolhive_pkg_skills.UpgradeStatus: + description: Status is the outcome of the upgrade attempt. + enum: + - upgraded + - up-to-date + - not-upgradable + - failed + type: string + x-enum-varnames: + - UpgradeStatusUpgraded + - UpgradeStatusUpToDate + - UpgradeStatusNotUpgradable + - UpgradeStatusFailed github_com_stacklok_toolhive_pkg_skills.ValidationResult: properties: errors: @@ -3083,6 +3173,26 @@ components: type: array uniqueItems: false type: object + pkg_api_v1.syncSkillsRequest: + description: Request to sync a project's installed skills to match its lock + file + properties: + clients: + description: Clients lists target client identifiers, or omit for every + detected client + items: + type: string + type: array + uniqueItems: false + project_root: + description: ProjectRoot is the project root path whose lock file should + be synced + type: string + prune: + description: Prune removes project-scoped skills that are installed but + not present in the lock file + type: boolean + type: object pkg_api_v1.toolOverride: description: Tool override properties: @@ -3243,6 +3353,31 @@ components: type: array uniqueItems: false type: object + pkg_api_v1.upgradeSkillsRequest: + description: Request to check for and install newer content for locked skills + properties: + clients: + description: Clients lists target client identifiers, or omit for every + detected client + items: + type: string + type: array + uniqueItems: false + dry_run: + description: DryRun reports what would change without installing anything + type: boolean + names: + description: Names restricts the upgrade to specific skill names, or omit + for every locked skill + items: + type: string + type: array + uniqueItems: false + project_root: + description: ProjectRoot is the project root path whose lock file should + be upgraded + type: string + type: object pkg_api_v1.validateSkillRequest: description: Request to validate a skill definition properties: @@ -5136,6 +5271,86 @@ paths: summary: Push a skill tags: - skills + /api/v1beta/skills/sync: + post: + description: Install the exact pinned digest for every lock file entry, restoring + drift + requestBody: + content: + application/json: + schema: + oneOf: + - type: object + - $ref: '#/components/schemas/pkg_api_v1.syncSkillsRequest' + description: Sync request + summary: request + description: Sync request + required: true + responses: + "200": + content: + application/json: + schema: + $ref: '#/components/schemas/github_com_stacklok_toolhive_pkg_skills.SyncResult' + description: OK + "400": + content: + application/json: + schema: + type: string + description: Bad Request + "500": + content: + application/json: + schema: + type: string + description: Internal Server Error + summary: Sync a project's skills to its lock file + tags: + - skills + /api/v1beta/skills/upgrade: + post: + description: Re-resolve each lock file entry's source and install newer content, + if any + requestBody: + content: + application/json: + schema: + oneOf: + - type: object + - $ref: '#/components/schemas/pkg_api_v1.upgradeSkillsRequest' + description: Upgrade request + summary: request + description: Upgrade request + required: true + responses: + "200": + content: + application/json: + schema: + $ref: '#/components/schemas/github_com_stacklok_toolhive_pkg_skills.UpgradeResult' + description: OK + "400": + content: + application/json: + schema: + type: string + description: Bad Request + "404": + content: + application/json: + schema: + type: string + description: Not Found (requested skill name not in lock file) + "500": + content: + application/json: + schema: + type: string + description: Internal Server Error + summary: Upgrade a project's locked skills + tags: + - skills /api/v1beta/skills/validate: post: description: Validate a skill definition diff --git a/pkg/api/v1/skills.go b/pkg/api/v1/skills.go index b0f58c9001..a4b82b468e 100644 --- a/pkg/api/v1/skills.go +++ b/pkg/api/v1/skills.go @@ -37,6 +37,8 @@ func SkillsRouter(skillService skills.SkillService) http.Handler { r.Get("/builds", apierrors.ErrorHandler(routes.listBuilds)) r.Delete("/builds/{tag}", apierrors.ErrorHandler(routes.deleteBuild)) r.Get("/content", apierrors.ErrorHandler(routes.getSkillContent)) + r.Post("/sync", apierrors.ErrorHandler(routes.syncSkills)) + r.Post("/upgrade", apierrors.ErrorHandler(routes.upgradeSkills)) return r } @@ -365,3 +367,75 @@ func (s *SkillsRoutes) getSkillContent(w http.ResponseWriter, r *http.Request) e w.Header().Set("Content-Type", "application/json") return json.NewEncoder(w).Encode(content) } + +// syncSkills installs the exact name/digest pinned in a project's skills lock +// file for every entry, restoring skills that are missing or drifted. +// +// @Summary Sync a project's skills to its lock file +// @Description Install the exact pinned digest for every lock file entry, restoring drift +// @Tags skills +// @Accept json +// @Produce json +// @Param request body syncSkillsRequest true "Sync request" +// @Success 200 {object} skills.SyncResult +// @Failure 400 {string} string "Bad Request" +// @Failure 500 {string} string "Internal Server Error" +// @Router /api/v1beta/skills/sync [post] +func (s *SkillsRoutes) syncSkills(w http.ResponseWriter, r *http.Request) error { + var req syncSkillsRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + return httperr.WithCode( + fmt.Errorf("invalid request body: %w", err), + http.StatusBadRequest, + ) + } + + result, err := s.skillService.Sync(r.Context(), skills.SyncOptions{ + ProjectRoot: req.ProjectRoot, + Clients: req.Clients, + Prune: req.Prune, + }) + if err != nil { + return err + } + + w.Header().Set("Content-Type", "application/json") + return json.NewEncoder(w).Encode(result) +} + +// upgradeSkills re-resolves each lock file entry's original source and +// installs any newer content it finds. +// +// @Summary Upgrade a project's locked skills +// @Description Re-resolve each lock file entry's source and install newer content, if any +// @Tags skills +// @Accept json +// @Produce json +// @Param request body upgradeSkillsRequest true "Upgrade request" +// @Success 200 {object} skills.UpgradeResult +// @Failure 400 {string} string "Bad Request" +// @Failure 404 {string} string "Not Found (requested skill name not in lock file)" +// @Failure 500 {string} string "Internal Server Error" +// @Router /api/v1beta/skills/upgrade [post] +func (s *SkillsRoutes) upgradeSkills(w http.ResponseWriter, r *http.Request) error { + var req upgradeSkillsRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + return httperr.WithCode( + fmt.Errorf("invalid request body: %w", err), + http.StatusBadRequest, + ) + } + + result, err := s.skillService.Upgrade(r.Context(), skills.UpgradeOptions{ + ProjectRoot: req.ProjectRoot, + Names: req.Names, + DryRun: req.DryRun, + Clients: req.Clients, + }) + if err != nil { + return err + } + + w.Header().Set("Content-Type", "application/json") + return json.NewEncoder(w).Encode(result) +} diff --git a/pkg/api/v1/skills_test.go b/pkg/api/v1/skills_test.go index 5ad7599923..faed829b0f 100644 --- a/pkg/api/v1/skills_test.go +++ b/pkg/api/v1/skills_test.go @@ -575,6 +575,115 @@ func TestSkillsRouter(t *testing.T) { expectedStatus: http.StatusInternalServerError, expectedBody: "Internal Server Error", }, + // syncSkills + { + name: "sync skills success", + method: "POST", + path: "/sync", + body: `{"project_root":"{{project_root}}"}`, + setupMock: func(svc *skillsmocks.MockSkillService, projectRoot string) { + svc.EXPECT().Sync(gomock.Any(), skills.SyncOptions{ProjectRoot: projectRoot}). + Return(&skills.SyncResult{Installed: []string{"my-skill"}}, nil) + }, + expectedStatus: http.StatusOK, + expectedBody: `"installed":["my-skill"]`, + }, + { + name: "sync skills with prune and clients", + method: "POST", + path: "/sync", + body: `{"project_root":"{{project_root}}","clients":["claude-code"],"prune":true}`, + setupMock: func(svc *skillsmocks.MockSkillService, projectRoot string) { + svc.EXPECT().Sync(gomock.Any(), skills.SyncOptions{ + ProjectRoot: projectRoot, + Clients: []string{"claude-code"}, + Prune: true, + }).Return(&skills.SyncResult{Pruned: []string{"extra-skill"}}, nil) + }, + expectedStatus: http.StatusOK, + expectedBody: `"pruned":["extra-skill"]`, + }, + { + name: "sync skills malformed json", + method: "POST", + path: "/sync", + body: `{invalid`, + setupMock: func(_ *skillsmocks.MockSkillService, _ string) {}, + expectedStatus: http.StatusBadRequest, + expectedBody: "invalid request body", + }, + { + name: "sync skills service error", + method: "POST", + path: "/sync", + body: `{"project_root":"/not/a/repo"}`, + setupMock: func(svc *skillsmocks.MockSkillService, _ string) { + svc.EXPECT().Sync(gomock.Any(), skills.SyncOptions{ProjectRoot: "/not/a/repo"}). + Return(nil, httperr.WithCode(fmt.Errorf("project_root must be a git repository"), http.StatusBadRequest)) + }, + expectedStatus: http.StatusBadRequest, + expectedBody: "must be a git repository", + }, + // upgradeSkills + { + name: "upgrade skills success", + method: "POST", + path: "/upgrade", + body: `{"project_root":"{{project_root}}"}`, + setupMock: func(svc *skillsmocks.MockSkillService, projectRoot string) { + svc.EXPECT().Upgrade(gomock.Any(), skills.UpgradeOptions{ProjectRoot: projectRoot}). + Return(&skills.UpgradeResult{ + Outcomes: []skills.UpgradeOutcome{ + {Name: "my-skill", Status: skills.UpgradeStatusUpgraded, OldDigest: "sha256:old", NewDigest: "sha256:new"}, + }, + }, nil) + }, + expectedStatus: http.StatusOK, + expectedBody: `"status":"upgraded"`, + }, + { + name: "upgrade skills dry run with names and clients", + method: "POST", + path: "/upgrade", + body: `{"project_root":"{{project_root}}","names":["my-skill"],"dry_run":true,"clients":["claude-code"]}`, + setupMock: func(svc *skillsmocks.MockSkillService, projectRoot string) { + svc.EXPECT().Upgrade(gomock.Any(), skills.UpgradeOptions{ + ProjectRoot: projectRoot, + Names: []string{"my-skill"}, + DryRun: true, + Clients: []string{"claude-code"}, + }).Return(&skills.UpgradeResult{ + Outcomes: []skills.UpgradeOutcome{ + {Name: "my-skill", Status: skills.UpgradeStatusUpToDate, OldDigest: "sha256:old", NewDigest: "sha256:old"}, + }, + }, nil) + }, + expectedStatus: http.StatusOK, + expectedBody: `"status":"up-to-date"`, + }, + { + name: "upgrade skills malformed json", + method: "POST", + path: "/upgrade", + body: `{invalid`, + setupMock: func(_ *skillsmocks.MockSkillService, _ string) {}, + expectedStatus: http.StatusBadRequest, + expectedBody: "invalid request body", + }, + { + name: "upgrade skills unknown name", + method: "POST", + path: "/upgrade", + body: `{"project_root":"{{project_root}}","names":["missing-skill"]}`, + setupMock: func(svc *skillsmocks.MockSkillService, projectRoot string) { + svc.EXPECT().Upgrade(gomock.Any(), skills.UpgradeOptions{ + ProjectRoot: projectRoot, + Names: []string{"missing-skill"}, + }).Return(nil, httperr.WithCode(fmt.Errorf(`skill "missing-skill" is not present in the lock file`), http.StatusNotFound)) + }, + expectedStatus: http.StatusNotFound, + expectedBody: "not present in the lock file", + }, } for _, tt := range tests { diff --git a/pkg/api/v1/skills_types.go b/pkg/api/v1/skills_types.go index 833a9b0aa5..e28a07d47d 100644 --- a/pkg/api/v1/skills_types.go +++ b/pkg/api/v1/skills_types.go @@ -76,3 +76,29 @@ type buildListResponse struct { // List of locally-built OCI skill artifacts Builds []skills.LocalBuild `json:"builds"` } + +// syncSkillsRequest represents the request to sync a project's skills lock file. +// +// @Description Request to sync a project's installed skills to match its lock file +type syncSkillsRequest struct { + // ProjectRoot is the project root path whose lock file should be synced + ProjectRoot string `json:"project_root"` + // Clients lists target client identifiers, or omit for every detected client + Clients []string `json:"clients,omitempty"` + // Prune removes project-scoped skills that are installed but not present in the lock file + Prune bool `json:"prune,omitempty"` +} + +// upgradeSkillsRequest represents the request to upgrade a project's locked skills. +// +// @Description Request to check for and install newer content for locked skills +type upgradeSkillsRequest struct { + // ProjectRoot is the project root path whose lock file should be upgraded + ProjectRoot string `json:"project_root"` + // Names restricts the upgrade to specific skill names, or omit for every locked skill + Names []string `json:"names,omitempty"` + // DryRun reports what would change without installing anything + DryRun bool `json:"dry_run,omitempty"` + // Clients lists target client identifiers, or omit for every detected client + Clients []string `json:"clients,omitempty"` +} diff --git a/pkg/skills/client/client.go b/pkg/skills/client/client.go index 4c5c69a606..7cfd640098 100644 --- a/pkg/skills/client/client.go +++ b/pkg/skills/client/client.go @@ -267,6 +267,39 @@ func (c *Client) GetContent(ctx context.Context, opts skills.ContentOptions) (*s return &content, nil } +// Sync installs the exact name/digest pinned in the project's lock file for +// every entry, restoring skills that are missing or have drifted. +func (c *Client) Sync(ctx context.Context, opts skills.SyncOptions) (*skills.SyncResult, error) { + body := syncRequest{ + ProjectRoot: opts.ProjectRoot, + Clients: opts.Clients, + Prune: opts.Prune, + } + + var result skills.SyncResult + if err := c.doJSONRequest(ctx, http.MethodPost, "/sync", nil, body, &result); err != nil { + return nil, err + } + return &result, nil +} + +// Upgrade re-resolves each lock file entry's original source and installs +// any newer content it finds. +func (c *Client) Upgrade(ctx context.Context, opts skills.UpgradeOptions) (*skills.UpgradeResult, error) { + body := upgradeRequest{ + ProjectRoot: opts.ProjectRoot, + Names: opts.Names, + DryRun: opts.DryRun, + Clients: opts.Clients, + } + + var result skills.UpgradeResult + if err := c.doJSONRequest(ctx, http.MethodPost, "/upgrade", nil, body, &result); err != nil { + return nil, err + } + return &result, nil +} + // --- internal helpers --- func (c *Client) buildURL(path string, query url.Values) string { diff --git a/pkg/skills/client/dto.go b/pkg/skills/client/dto.go index 104f7e2fad..172997cebe 100644 --- a/pkg/skills/client/dto.go +++ b/pkg/skills/client/dto.go @@ -41,3 +41,16 @@ type installResponse struct { type listBuildsResponse struct { Builds []skills.LocalBuild `json:"builds"` } + +type syncRequest struct { + ProjectRoot string `json:"project_root"` + Clients []string `json:"clients,omitempty"` + Prune bool `json:"prune,omitempty"` +} + +type upgradeRequest struct { + ProjectRoot string `json:"project_root"` + Names []string `json:"names,omitempty"` + DryRun bool `json:"dry_run,omitempty"` + Clients []string `json:"clients,omitempty"` +} diff --git a/pkg/skills/lockfile/lockfile.go b/pkg/skills/lockfile/lockfile.go new file mode 100644 index 0000000000..408f34cc40 --- /dev/null +++ b/pkg/skills/lockfile/lockfile.go @@ -0,0 +1,176 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +// Package lockfile manages the project-level skills lock file +// (toolhive.lock.yaml). The lock file pins the exact name, version, and +// digest of every project-scoped skill install so a team can restore +// ("thv skill sync") or refresh ("thv skill upgrade") the pinned state. +package lockfile + +import ( + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "sort" + + "gopkg.in/yaml.v3" + + "github.com/stacklok/toolhive/pkg/fileutils" +) + +// FileName is the name of the project-level skills lock file, written to the +// project root alongside .git. +const FileName = "toolhive.lock.yaml" + +// CurrentVersion is the schema version written to new lock files. +const CurrentVersion = 1 + +// Entry represents a single pinned skill installation in the lock file. +type Entry struct { + // Name is the skill's unique name. + Name string `yaml:"name"` + // Version is the skill's declared version, if any (from SKILL.md frontmatter). + Version string `yaml:"version,omitempty"` + // Source is exactly what the user (or the registry resolver) originally + // requested — a plain registry name, an OCI reference, or a git:// + // reference. Upgrade re-resolves this value to check for newer content. + Source string `yaml:"source"` + // ResolvedReference is the concrete OCI reference or git:// URL that + // Source resolved to at install time. + ResolvedReference string `yaml:"resolvedReference,omitempty"` + // Digest pins the exact content installed: an OCI "sha256:..." digest or + // a git commit hash. + Digest string `yaml:"digest"` +} + +// Lockfile is the parsed contents of a project's toolhive.lock.yaml. +type Lockfile struct { + // Version is the lock file schema version. + Version int `yaml:"version"` + // Skills is the set of pinned skill installations, sorted by name. + Skills []Entry `yaml:"skills,omitempty"` +} + +// Path returns the absolute path to the lock file for the given project root. +func Path(projectRoot string) string { + return filepath.Join(projectRoot, FileName) +} + +// Load reads and parses the lock file for projectRoot. A missing lock file +// is not an error — it returns an empty Lockfile ready to be populated. +func Load(projectRoot string) (*Lockfile, error) { + data, err := os.ReadFile(Path(projectRoot)) // #nosec G304 -- projectRoot is validated by callers before reaching this package + if errors.Is(err, fs.ErrNotExist) { + return &Lockfile{Version: CurrentVersion}, nil + } + if err != nil { + return nil, fmt.Errorf("reading lock file: %w", err) + } + + var lf Lockfile + if err := yaml.Unmarshal(data, &lf); err != nil { + return nil, fmt.Errorf("parsing lock file: %w", err) + } + if lf.Version == 0 { + lf.Version = CurrentVersion + } + sortEntries(lf.Skills) + return &lf, nil +} + +// Get returns the entry for name, if present. +func (l *Lockfile) Get(name string) (Entry, bool) { + for _, e := range l.Skills { + if e.Name == name { + return e, true + } + } + return Entry{}, false +} + +// Upsert inserts or replaces the entry with a matching name, keeping the +// slice sorted by name for stable diffs. +func (l *Lockfile) Upsert(entry Entry) { + for i := range l.Skills { + if l.Skills[i].Name == entry.Name { + l.Skills[i] = entry + return + } + } + l.Skills = append(l.Skills, entry) + sortEntries(l.Skills) +} + +// Remove deletes the entry with the given name, if present. Reports whether +// an entry was removed. +func (l *Lockfile) Remove(name string) bool { + for i, e := range l.Skills { + if e.Name == name { + l.Skills = append(l.Skills[:i], l.Skills[i+1:]...) + return true + } + } + return false +} + +// Save writes the lock file to projectRoot, creating it if necessary. +// Callers that need read-modify-write atomicity across processes should use +// [UpsertEntry] or [RemoveEntry] instead of calling Load+Save directly. +func (l *Lockfile) Save(projectRoot string) error { + if l.Version == 0 { + l.Version = CurrentVersion + } + sortEntries(l.Skills) + + data, err := yaml.Marshal(l) + if err != nil { + return fmt.Errorf("marshaling lock file: %w", err) + } + + path := Path(projectRoot) + tmp := path + ".tmp" + if err := os.WriteFile(tmp, data, 0o644); err != nil { // #nosec G306 -- lock file is committed to git, not sensitive + return fmt.Errorf("writing lock file: %w", err) + } + if err := os.Rename(tmp, path); err != nil { + _ = os.Remove(tmp) + return fmt.Errorf("saving lock file: %w", err) + } + return nil +} + +// UpsertEntry loads the lock file, upserts entry, and saves it back, all +// under a single file lock so concurrent installs cannot race on +// read-modify-write. +func UpsertEntry(projectRoot string, entry Entry) error { + return fileutils.WithFileLock(Path(projectRoot), func() error { + lf, err := Load(projectRoot) + if err != nil { + return err + } + lf.Upsert(entry) + return lf.Save(projectRoot) + }) +} + +// RemoveEntry loads the lock file, removes the named entry if present, and +// saves it back, all under a single file lock. Removing an entry that does +// not exist is a no-op, not an error. +func RemoveEntry(projectRoot string, name string) error { + return fileutils.WithFileLock(Path(projectRoot), func() error { + lf, err := Load(projectRoot) + if err != nil { + return err + } + if !lf.Remove(name) { + return nil + } + return lf.Save(projectRoot) + }) +} + +func sortEntries(entries []Entry) { + sort.Slice(entries, func(i, j int) bool { return entries[i].Name < entries[j].Name }) +} diff --git a/pkg/skills/lockfile/lockfile_test.go b/pkg/skills/lockfile/lockfile_test.go new file mode 100644 index 0000000000..0adc06f037 --- /dev/null +++ b/pkg/skills/lockfile/lockfile_test.go @@ -0,0 +1,127 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package lockfile + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func resolvedTempDir(t *testing.T) string { + t.Helper() + dir, err := filepath.EvalSymlinks(t.TempDir()) + require.NoError(t, err) + return dir +} + +func TestLoadMissingFileReturnsEmptyLockfile(t *testing.T) { + t.Parallel() + dir := resolvedTempDir(t) + + lf, err := Load(dir) + require.NoError(t, err) + assert.Equal(t, CurrentVersion, lf.Version) + assert.Empty(t, lf.Skills) +} + +func TestLoadRejectsMalformedYAML(t *testing.T) { + t.Parallel() + dir := resolvedTempDir(t) + require.NoError(t, os.WriteFile(Path(dir), []byte("not: [valid: yaml"), 0o644)) + + _, err := Load(dir) + require.Error(t, err) + assert.Contains(t, err.Error(), "parsing lock file") +} + +func TestSaveAndLoadRoundTrip(t *testing.T) { + t.Parallel() + dir := resolvedTempDir(t) + + lf := &Lockfile{Version: CurrentVersion} + entry := Entry{ + Name: "code-review", + Version: "1.0.0", + Source: "code-review", + ResolvedReference: "ghcr.io/org/code-review:1.0.0", + Digest: "sha256:abc123", + } + lf.Upsert(entry) + require.NoError(t, lf.Save(dir)) + + loaded, err := Load(dir) + require.NoError(t, err) + require.Len(t, loaded.Skills, 1) + assert.Equal(t, entry, loaded.Skills[0]) +} + +func TestLockfileGetUpsertRemove(t *testing.T) { + t.Parallel() + + lf := &Lockfile{Version: CurrentVersion} + + _, ok := lf.Get("missing") + assert.False(t, ok) + + a := Entry{Name: "b-skill", Digest: "sha256:1"} + c := Entry{Name: "a-skill", Digest: "sha256:2"} + lf.Upsert(a) + lf.Upsert(c) + + // Entries are kept sorted by name. + require.Len(t, lf.Skills, 2) + assert.Equal(t, "a-skill", lf.Skills[0].Name) + assert.Equal(t, "b-skill", lf.Skills[1].Name) + + got, ok := lf.Get("b-skill") + require.True(t, ok) + assert.Equal(t, "sha256:1", got.Digest) + + // Upsert replaces an existing entry rather than duplicating it. + updated := Entry{Name: "b-skill", Digest: "sha256:new"} + lf.Upsert(updated) + require.Len(t, lf.Skills, 2) + got, ok = lf.Get("b-skill") + require.True(t, ok) + assert.Equal(t, "sha256:new", got.Digest) + + removed := lf.Remove("b-skill") + assert.True(t, removed) + require.Len(t, lf.Skills, 1) + + removedAgain := lf.Remove("b-skill") + assert.False(t, removedAgain) +} + +func TestUpsertEntryAndRemoveEntry(t *testing.T) { + t.Parallel() + dir := resolvedTempDir(t) + + entry := Entry{Name: "my-skill", Digest: "sha256:abc"} + require.NoError(t, UpsertEntry(dir, entry)) + + lf, err := Load(dir) + require.NoError(t, err) + require.Len(t, lf.Skills, 1) + assert.Equal(t, "my-skill", lf.Skills[0].Name) + + // Upserting a second entry preserves the first. + require.NoError(t, UpsertEntry(dir, Entry{Name: "other-skill", Digest: "sha256:def"})) + lf, err = Load(dir) + require.NoError(t, err) + assert.Len(t, lf.Skills, 2) + + require.NoError(t, RemoveEntry(dir, "my-skill")) + lf, err = Load(dir) + require.NoError(t, err) + require.Len(t, lf.Skills, 1) + assert.Equal(t, "other-skill", lf.Skills[0].Name) + + // Removing a name that isn't present is a no-op, not an error. + require.NoError(t, RemoveEntry(dir, "does-not-exist")) +} diff --git a/pkg/skills/mocks/mock_service.go b/pkg/skills/mocks/mock_service.go index e176f22f44..cf3100035d 100644 --- a/pkg/skills/mocks/mock_service.go +++ b/pkg/skills/mocks/mock_service.go @@ -159,6 +159,21 @@ func (mr *MockSkillServiceMockRecorder) Push(ctx, opts any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Push", reflect.TypeOf((*MockSkillService)(nil).Push), ctx, opts) } +// Sync mocks base method. +func (m *MockSkillService) Sync(ctx context.Context, opts skills.SyncOptions) (*skills.SyncResult, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Sync", ctx, opts) + ret0, _ := ret[0].(*skills.SyncResult) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Sync indicates an expected call of Sync. +func (mr *MockSkillServiceMockRecorder) Sync(ctx, opts any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Sync", reflect.TypeOf((*MockSkillService)(nil).Sync), ctx, opts) +} + // Uninstall mocks base method. func (m *MockSkillService) Uninstall(ctx context.Context, opts skills.UninstallOptions) error { m.ctrl.T.Helper() @@ -173,6 +188,21 @@ func (mr *MockSkillServiceMockRecorder) Uninstall(ctx, opts any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Uninstall", reflect.TypeOf((*MockSkillService)(nil).Uninstall), ctx, opts) } +// Upgrade mocks base method. +func (m *MockSkillService) Upgrade(ctx context.Context, opts skills.UpgradeOptions) (*skills.UpgradeResult, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Upgrade", ctx, opts) + ret0, _ := ret[0].(*skills.UpgradeResult) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Upgrade indicates an expected call of Upgrade. +func (mr *MockSkillServiceMockRecorder) Upgrade(ctx, opts any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Upgrade", reflect.TypeOf((*MockSkillService)(nil).Upgrade), ctx, opts) +} + // Validate mocks base method. func (m *MockSkillService) Validate(ctx context.Context, path string) (*skills.ValidationResult, error) { m.ctrl.T.Helper() diff --git a/pkg/skills/options.go b/pkg/skills/options.go index a822c8918f..5d993b07a8 100644 --- a/pkg/skills/options.go +++ b/pkg/skills/options.go @@ -133,6 +133,90 @@ type PushOptions struct { Reference string `json:"reference"` } +// SyncOptions configures the behavior of the Sync operation. +type SyncOptions struct { + // ProjectRoot is the project root path whose lock file should be synced. + ProjectRoot string `json:"project_root"` + // Clients lists target clients (e.g., "claude-code"). Empty means every + // skill-supporting client detected on this host. + Clients []string `json:"clients,omitempty"` + // Prune removes project-scoped skills that are installed but not present + // in the lock file. When false, such skills are only reported. + Prune bool `json:"prune,omitempty"` +} + +// SyncFailure describes a single skill that failed to sync. +type SyncFailure struct { + // Name is the skill name that failed. + Name string `json:"name"` + // Error is a human-readable description of the failure. + Error string `json:"error"` +} + +// SyncResult contains the outcome of a Sync operation. +type SyncResult struct { + // Installed lists skills that were installed or reinstalled to match the lock file. + Installed []string `json:"installed,omitempty"` + // UpToDate lists skills that already matched the lock file's pinned digest. + UpToDate []string `json:"up_to_date,omitempty"` + // Unmanaged lists project-scoped skills present on disk but absent from the lock file. + Unmanaged []string `json:"unmanaged,omitempty"` + // Pruned lists unmanaged skills that were uninstalled because Prune was set. + Pruned []string `json:"pruned,omitempty"` + // Failed lists skills that could not be synced, with the reason for each. + Failed []SyncFailure `json:"failed,omitempty"` +} + +// UpgradeOptions configures the behavior of the Upgrade operation. +type UpgradeOptions struct { + // ProjectRoot is the project root path whose lock file should be upgraded. + ProjectRoot string `json:"project_root"` + // Names restricts the upgrade to specific skill names. Empty means every + // entry in the lock file. + Names []string `json:"names,omitempty"` + // DryRun reports what would change without installing anything. + DryRun bool `json:"dry_run,omitempty"` + // Clients lists target clients (e.g., "claude-code"). Empty means every + // skill-supporting client detected on this host. + Clients []string `json:"clients,omitempty"` +} + +// UpgradeStatus represents the outcome of upgrading a single skill. +type UpgradeStatus string + +const ( + // UpgradeStatusUpgraded indicates the skill was installed at a new digest. + UpgradeStatusUpgraded UpgradeStatus = "upgraded" + // UpgradeStatusUpToDate indicates the resolved source still points at the pinned digest. + UpgradeStatusUpToDate UpgradeStatus = "up-to-date" + // UpgradeStatusNotUpgradable indicates the entry is pinned to an immutable + // reference (an OCI digest or a full git commit hash) and cannot be upgraded. + UpgradeStatusNotUpgradable UpgradeStatus = "not-upgradable" + // UpgradeStatusFailed indicates the upgrade attempt failed. + UpgradeStatusFailed UpgradeStatus = "failed" +) + +// UpgradeOutcome describes the result of attempting to upgrade one skill. +type UpgradeOutcome struct { + // Name is the skill name. + Name string `json:"name"` + // Status is the outcome of the upgrade attempt. + Status UpgradeStatus `json:"status"` + // OldDigest is the digest pinned in the lock file before this operation. + OldDigest string `json:"old_digest,omitempty"` + // NewDigest is the digest the source currently resolves to. Equal to + // OldDigest when Status is UpgradeStatusUpToDate. + NewDigest string `json:"new_digest,omitempty"` + // Error is a human-readable description of the failure, set only when Status is UpgradeStatusFailed. + Error string `json:"error,omitempty"` +} + +// UpgradeResult contains the outcome of an Upgrade operation. +type UpgradeResult struct { + // Outcomes contains one entry per skill considered for upgrade. + Outcomes []UpgradeOutcome `json:"outcomes"` +} + // LocalBuild represents a locally-built OCI skill artifact in the local store. type LocalBuild struct { // Tag is the OCI tag or name used to reference the artifact. diff --git a/pkg/skills/service.go b/pkg/skills/service.go index 53dcce75c4..34826df0db 100644 --- a/pkg/skills/service.go +++ b/pkg/skills/service.go @@ -30,4 +30,13 @@ type SkillService interface { // GetContent retrieves the SKILL.md body and file listing from an OCI artifact // without installing it. Works for both remote registry references and local build tags. GetContent(ctx context.Context, opts ContentOptions) (*SkillContent, error) + // Sync installs the exact name/digest pinned in the project's lock file + // for every entry, restoring drifted or missing skills. Skills installed + // at project scope but absent from the lock file are reported as + // unmanaged, or removed when opts.Prune is set. + Sync(ctx context.Context, opts SyncOptions) (*SyncResult, error) + // Upgrade re-resolves each lock file entry's original source and, when the + // resolved digest differs from the pinned one, installs the new content + // and updates the lock file entry. + Upgrade(ctx context.Context, opts UpgradeOptions) (*UpgradeResult, error) } diff --git a/pkg/skills/skillsvc/install.go b/pkg/skills/skillsvc/install.go index 028f78530f..b5155ce5d1 100644 --- a/pkg/skills/skillsvc/install.go +++ b/pkg/skills/skillsvc/install.go @@ -14,6 +14,7 @@ import ( "github.com/stacklok/toolhive/pkg/groups" "github.com/stacklok/toolhive/pkg/skills" "github.com/stacklok/toolhive/pkg/skills/gitresolver" + "github.com/stacklok/toolhive/pkg/skills/lockfile" ) // Install installs a skill. When the Name field contains an OCI reference @@ -21,7 +22,49 @@ import ( // the registry and extracted. When LayerData is provided, the skill is extracted // to disk and a full installation record is created. Without LayerData, a // pending record is created. +// +// For project-scoped installs, a successful result also upserts an entry in +// the project's toolhive.lock.yaml, pinning the resolved reference and digest +// so the install can be restored later with "thv skill sync". opts.Name is +// recorded as the entry's Source exactly as given here (before any internal +// resolution), so "thv skill upgrade" can re-resolve it later. func (s *service) Install(ctx context.Context, opts skills.InstallOptions) (*skills.InstallResult, error) { + source := opts.Name + result, err := s.installInternal(ctx, opts) + if err != nil { + return nil, err + } + recordLockEntry(source, result.Skill) + return result, nil +} + +// recordLockEntry upserts a project-scope lock file entry after a successful +// install. It is a no-op for user-scoped installs. Lock-file errors are +// logged but never fail the install — the skill's files and DB record are +// already correct at this point, so surfacing an error here would be +// misleading (and a subsequent sync/upgrade can repair the lock file). +func recordLockEntry(source string, sk skills.InstalledSkill) { + if sk.Scope != skills.ScopeProject || sk.ProjectRoot == "" { + return + } + entry := lockfile.Entry{ + Name: sk.Metadata.Name, + Version: sk.Metadata.Version, + Source: source, + ResolvedReference: sk.Reference, + Digest: sk.Digest, + } + if err := lockfile.UpsertEntry(sk.ProjectRoot, entry); err != nil { + slog.Warn("failed to update skills lock file", + "skill", sk.Metadata.Name, "project_root", sk.ProjectRoot, "error", err) + } +} + +// installInternal performs the actual install dispatch without touching the +// lock file. It is used directly by Install, and by Sync/Upgrade which +// manage lock file entries themselves (they pass a Name that has already +// been resolved or pinned, which must not be recorded as the entry's Source). +func (s *service) installInternal(ctx context.Context, opts skills.InstallOptions) (*skills.InstallResult, error) { scope, projectRoot, err := normalizeProjectRoot(opts.Scope, opts.ProjectRoot) if err != nil { return nil, err diff --git a/pkg/skills/skillsvc/lock_test.go b/pkg/skills/skillsvc/lock_test.go new file mode 100644 index 0000000000..1701a05a7b --- /dev/null +++ b/pkg/skills/skillsvc/lock_test.go @@ -0,0 +1,163 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package skillsvc + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "github.com/stacklok/toolhive/pkg/skills" + "github.com/stacklok/toolhive/pkg/skills/lockfile" + skillsmocks "github.com/stacklok/toolhive/pkg/skills/mocks" + "github.com/stacklok/toolhive/pkg/storage" + storemocks "github.com/stacklok/toolhive/pkg/storage/mocks" +) + +func TestInstallRecordsProjectScopeLockEntry(t *testing.T) { + t.Parallel() + + layerData := makeLayerData(t) + projectRoot := makeProjectRoot(t) + + ctrl := gomock.NewController(t) + store := storemocks.NewMockSkillStore(ctrl) + pr := skillsmocks.NewMockPathResolver(ctrl) + + targetDir := filepath.Join(projectRoot, ".claude", "skills", "my-skill") + pr.EXPECT().ListSkillSupportingClients().Return([]string{"claude-code"}) + pr.EXPECT().GetSkillPath("claude-code", "my-skill", skills.ScopeProject, projectRoot).Return(targetDir, nil) + store.EXPECT().Get(gomock.Any(), "my-skill", skills.ScopeProject, projectRoot). + Return(skills.InstalledSkill{}, storage.ErrNotFound) + store.EXPECT().Create(gomock.Any(), gomock.Any()).Return(nil) + + svc := New(store, WithPathResolver(pr)) + result, err := svc.Install(t.Context(), skills.InstallOptions{ + Name: "my-skill", + LayerData: layerData, + Digest: "sha256:abc", + Reference: "ghcr.io/org/my-skill:v1", + Version: "1.0.0", + Scope: skills.ScopeProject, + ProjectRoot: projectRoot, + }) + require.NoError(t, err) + assert.Equal(t, "my-skill", result.Skill.Metadata.Name) + + lf, err := lockfile.Load(projectRoot) + require.NoError(t, err) + require.Len(t, lf.Skills, 1) + entry := lf.Skills[0] + assert.Equal(t, "my-skill", entry.Name) + assert.Equal(t, "1.0.0", entry.Version) + // Source is exactly what the caller passed as Name — what "thv skill + // upgrade" later re-resolves — not the OCI reference it happened to install. + assert.Equal(t, "my-skill", entry.Source) + assert.Equal(t, "ghcr.io/org/my-skill:v1", entry.ResolvedReference) + assert.Equal(t, "sha256:abc", entry.Digest) +} + +func TestInstallUserScopeDoesNotWriteLockFile(t *testing.T) { + t.Parallel() + + layerData := makeLayerData(t) + baseDir := tempDir(t) + + ctrl := gomock.NewController(t) + store := storemocks.NewMockSkillStore(ctrl) + pr := skillsmocks.NewMockPathResolver(ctrl) + + targetDir := filepath.Join(baseDir, "my-skill") + pr.EXPECT().ListSkillSupportingClients().Return([]string{"claude-code"}) + pr.EXPECT().GetSkillPath("claude-code", "my-skill", skills.ScopeUser, "").Return(targetDir, nil) + store.EXPECT().Get(gomock.Any(), "my-skill", skills.ScopeUser, "").Return(skills.InstalledSkill{}, storage.ErrNotFound) + store.EXPECT().Create(gomock.Any(), gomock.Any()).Return(nil) + + svc := New(store, WithPathResolver(pr)) + _, err := svc.Install(t.Context(), skills.InstallOptions{ + Name: "my-skill", + LayerData: layerData, + Digest: "sha256:abc", + }) + require.NoError(t, err) + + _, statErr := os.Stat(lockfile.Path(baseDir)) + assert.ErrorIs(t, statErr, os.ErrNotExist) +} + +func TestUninstallRemovesProjectScopeLockEntry(t *testing.T) { + t.Parallel() + + projectRoot := makeProjectRoot(t) + require.NoError(t, lockfile.UpsertEntry(projectRoot, lockfile.Entry{ + Name: "my-skill", + Source: "my-skill", + Digest: "sha256:abc", + })) + require.NoError(t, lockfile.UpsertEntry(projectRoot, lockfile.Entry{ + Name: "other-skill", + Source: "other-skill", + Digest: "sha256:def", + })) + + ctrl := gomock.NewController(t) + store := storemocks.NewMockSkillStore(ctrl) + + existing := skills.InstalledSkill{ + Metadata: skills.SkillMetadata{Name: "my-skill"}, + Scope: skills.ScopeProject, + ProjectRoot: projectRoot, + } + store.EXPECT().Get(gomock.Any(), "my-skill", skills.ScopeProject, projectRoot).Return(existing, nil) + store.EXPECT().Delete(gomock.Any(), "my-skill", skills.ScopeProject, projectRoot).Return(nil) + + svc := New(store) + err := svc.Uninstall(t.Context(), skills.UninstallOptions{ + Name: "my-skill", + Scope: skills.ScopeProject, + ProjectRoot: projectRoot, + }) + require.NoError(t, err) + + lf, err := lockfile.Load(projectRoot) + require.NoError(t, err) + require.Len(t, lf.Skills, 1) + assert.Equal(t, "other-skill", lf.Skills[0].Name) +} + +func TestInstallInternalDoesNotTouchLockFile(t *testing.T) { + t.Parallel() + + layerData := makeLayerData(t) + projectRoot := makeProjectRoot(t) + + ctrl := gomock.NewController(t) + store := storemocks.NewMockSkillStore(ctrl) + pr := skillsmocks.NewMockPathResolver(ctrl) + + targetDir := filepath.Join(projectRoot, ".claude", "skills", "my-skill") + pr.EXPECT().ListSkillSupportingClients().Return([]string{"claude-code"}) + pr.EXPECT().GetSkillPath("claude-code", "my-skill", skills.ScopeProject, projectRoot).Return(targetDir, nil) + store.EXPECT().Get(gomock.Any(), "my-skill", skills.ScopeProject, projectRoot). + Return(skills.InstalledSkill{}, storage.ErrNotFound) + store.EXPECT().Create(gomock.Any(), gomock.Any()).Return(nil) + + svc := New(store, WithPathResolver(pr)).(*service) + _, err := svc.installInternal(context.Background(), skills.InstallOptions{ + Name: "my-skill", + LayerData: layerData, + Digest: "sha256:abc", + Scope: skills.ScopeProject, + ProjectRoot: projectRoot, + }) + require.NoError(t, err) + + _, statErr := os.Stat(lockfile.Path(projectRoot)) + assert.ErrorIs(t, statErr, os.ErrNotExist) +} diff --git a/pkg/skills/skillsvc/pin.go b/pkg/skills/skillsvc/pin.go new file mode 100644 index 0000000000..257de9ee20 --- /dev/null +++ b/pkg/skills/skillsvc/pin.go @@ -0,0 +1,72 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package skillsvc + +import ( + "fmt" + "regexp" + "strings" + + nameref "github.com/google/go-containerregistry/pkg/name" + + "github.com/stacklok/toolhive/pkg/skills/gitresolver" + "github.com/stacklok/toolhive/pkg/skills/lockfile" +) + +// fullCommitHashRe matches a full 40-character hex git commit hash. +var fullCommitHashRe = regexp.MustCompile(`^[0-9a-fA-F]{40}$`) + +// buildPinnedReference returns an install Name that installs exactly the +// digest recorded in entry, regardless of what its resolved reference's tag +// or branch currently points to. Used by Sync to restore drifted skills. +func buildPinnedReference(entry lockfile.Entry) (string, error) { + if gitresolver.IsGitReference(entry.ResolvedReference) { + return pinGitReference(entry.ResolvedReference, entry.Digest) + } + return pinOCIReference(entry.ResolvedReference, entry.Digest) +} + +// pinOCIReference rewrites ref to reference digest directly, dropping any tag. +func pinOCIReference(ref, digest string) (string, error) { + parsed, err := nameref.ParseReference(ref) + if err != nil { + return "", fmt.Errorf("parsing pinned OCI reference %q: %w", ref, err) + } + return parsed.Context().Digest(digest).String(), nil +} + +// pinGitReference rewrites a git:// reference to pin its ref (branch/tag) to +// the exact commit hash, preserving the host/repo and any skill subfolder. +func pinGitReference(resolvedRef, commitHash string) (string, error) { + parsed, err := gitresolver.ParseGitReference(resolvedRef) + if err != nil { + return "", fmt.Errorf("parsing pinned git reference %q: %w", resolvedRef, err) + } + hostPath := strings.TrimPrefix(strings.TrimPrefix(parsed.URL, "https://"), "http://") + pinned := "git://" + hostPath + "@" + commitHash + if parsed.Path != "" { + pinned += "#" + parsed.Path + } + return pinned, nil +} + +// isImmutableSource reports whether source already pins an exact, unambiguous +// artifact — an OCI digest reference or a git reference at a full commit +// hash — such that re-resolving it can never surface newer content. +func isImmutableSource(source string) bool { + if gitresolver.IsGitReference(source) { + gitRef, err := gitresolver.ParseGitReference(source) + if err != nil { + return false + } + return fullCommitHashRe.MatchString(gitRef.Ref) + } + + ref, isOCI, err := parseOCIReference(source) + if err != nil || !isOCI { + return false + } + _, isDigest := ref.(nameref.Digest) + return isDigest +} diff --git a/pkg/skills/skillsvc/sync.go b/pkg/skills/skillsvc/sync.go new file mode 100644 index 0000000000..59fed038fb --- /dev/null +++ b/pkg/skills/skillsvc/sync.go @@ -0,0 +1,109 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package skillsvc + +import ( + "context" + "errors" + "fmt" + "net/http" + + "github.com/stacklok/toolhive-core/httperr" + "github.com/stacklok/toolhive/pkg/skills" + "github.com/stacklok/toolhive/pkg/skills/lockfile" + "github.com/stacklok/toolhive/pkg/storage" +) + +// Sync installs the exact name/digest pinned in the project's lock file for +// every entry, restoring skills that are missing or have drifted from their +// pinned digest. Project-scoped skills that are installed but absent from +// the lock file are reported as unmanaged, or removed when opts.Prune is set. +func (s *service) Sync(ctx context.Context, opts skills.SyncOptions) (*skills.SyncResult, error) { + projectRoot, err := skills.ValidateProjectRoot(opts.ProjectRoot) + if err != nil { + return nil, httperr.WithCode(err, http.StatusBadRequest) + } + + lf, err := lockfile.Load(projectRoot) + if err != nil { + return nil, fmt.Errorf("loading lock file: %w", err) + } + + result := &skills.SyncResult{} + locked := make(map[string]struct{}, len(lf.Skills)) + for _, entry := range lf.Skills { + locked[entry.Name] = struct{}{} + s.syncEntry(ctx, projectRoot, entry, opts.Clients, result) + } + + if err := s.syncUnmanaged(ctx, projectRoot, locked, opts.Prune, result); err != nil { + return nil, err + } + + return result, nil +} + +// syncEntry restores a single lock entry, appending its outcome to result. +func (s *service) syncEntry( + ctx context.Context, projectRoot string, entry lockfile.Entry, clients []string, result *skills.SyncResult, +) { + existing, storeErr := s.store.Get(ctx, entry.Name, skills.ScopeProject, projectRoot) + if storeErr == nil && existing.Digest == entry.Digest { + result.UpToDate = append(result.UpToDate, entry.Name) + return + } + + pinnedRef, err := buildPinnedReference(entry) + if err != nil { + result.Failed = append(result.Failed, skills.SyncFailure{Name: entry.Name, Error: err.Error()}) + return + } + + // installInternal (not Install) is used deliberately: pinnedRef is a + // digest-pinned reference derived from the entry, not the user-facing + // source, so it must not overwrite the lock entry's Source field. Since + // we install exactly the pinned digest, the entry itself never needs to + // change as a result of a sync. + if _, err := s.installInternal(ctx, skills.InstallOptions{ + Name: pinnedRef, + Scope: skills.ScopeProject, + ProjectRoot: projectRoot, + Clients: clients, + Force: true, + }); err != nil { + result.Failed = append(result.Failed, skills.SyncFailure{Name: entry.Name, Error: err.Error()}) + return + } + result.Installed = append(result.Installed, entry.Name) +} + +// syncUnmanaged finds project-scoped skills installed outside of the lock +// file and either reports or prunes them. +func (s *service) syncUnmanaged( + ctx context.Context, projectRoot string, locked map[string]struct{}, prune bool, result *skills.SyncResult, +) error { + installed, err := s.store.List(ctx, storage.ListFilter{Scope: skills.ScopeProject, ProjectRoot: projectRoot}) + if err != nil { + return fmt.Errorf("listing installed skills: %w", err) + } + + for _, sk := range installed { + name := sk.Metadata.Name + if _, ok := locked[name]; ok { + continue + } + if !prune { + result.Unmanaged = append(result.Unmanaged, name) + continue + } + if err := s.Uninstall(ctx, skills.UninstallOptions{ + Name: name, Scope: skills.ScopeProject, ProjectRoot: projectRoot, + }); err != nil && !errors.Is(err, storage.ErrNotFound) { + result.Failed = append(result.Failed, skills.SyncFailure{Name: name, Error: err.Error()}) + continue + } + result.Pruned = append(result.Pruned, name) + } + return nil +} diff --git a/pkg/skills/skillsvc/sync_test.go b/pkg/skills/skillsvc/sync_test.go new file mode 100644 index 0000000000..b5ca5e27d2 --- /dev/null +++ b/pkg/skills/skillsvc/sync_test.go @@ -0,0 +1,130 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package skillsvc + +import ( + "net/http" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "github.com/stacklok/toolhive-core/httperr" + ociskills "github.com/stacklok/toolhive-core/oci/skills" + ocimocks "github.com/stacklok/toolhive-core/oci/skills/mocks" + "github.com/stacklok/toolhive/pkg/skills" + "github.com/stacklok/toolhive/pkg/skills/lockfile" + skillsmocks "github.com/stacklok/toolhive/pkg/skills/mocks" + "github.com/stacklok/toolhive/pkg/storage" + storemocks "github.com/stacklok/toolhive/pkg/storage/mocks" +) + +func TestSyncInstallsDriftedReportsUpToDateAndUnmanaged(t *testing.T) { + t.Parallel() + projectRoot := makeProjectRoot(t) + + ociStore, err := ociskills.NewStore(tempDir(t)) + require.NoError(t, err) + indexDigest := buildTestArtifact(t, ociStore, "my-skill", "1.0.0") + + pinnedRef, err := pinOCIReference("ghcr.io/org/my-skill:v1", indexDigest.String()) + require.NoError(t, err) + + require.NoError(t, lockfile.UpsertEntry(projectRoot, lockfile.Entry{ + Name: "my-skill", + Version: "1.0.0", + Source: "my-skill", + ResolvedReference: "ghcr.io/org/my-skill:v1", + Digest: indexDigest.String(), + })) + require.NoError(t, lockfile.UpsertEntry(projectRoot, lockfile.Entry{ + Name: "already-current", + Source: "already-current", + ResolvedReference: "ghcr.io/org/already-current:v1", + Digest: "sha256:matches", + })) + + ctrl := gomock.NewController(t) + reg := ocimocks.NewMockRegistryClient(ctrl) + reg.EXPECT().Pull(gomock.Any(), ociStore, pinnedRef).Return(indexDigest, nil) + + store := storemocks.NewMockSkillStore(ctrl) + // Called once by Sync's own drift check, and once more inside the normal + // install flow that Sync delegates to for the actual (re)install. + store.EXPECT().Get(gomock.Any(), "my-skill", skills.ScopeProject, projectRoot). + Return(skills.InstalledSkill{}, storage.ErrNotFound).Times(2) + store.EXPECT().Get(gomock.Any(), "already-current", skills.ScopeProject, projectRoot). + Return(skills.InstalledSkill{ + Metadata: skills.SkillMetadata{Name: "already-current"}, + Digest: "sha256:matches", + }, nil) + store.EXPECT().Create(gomock.Any(), gomock.Any()).Return(nil) + store.EXPECT().List(gomock.Any(), storage.ListFilter{Scope: skills.ScopeProject, ProjectRoot: projectRoot}). + Return([]skills.InstalledSkill{ + {Metadata: skills.SkillMetadata{Name: "my-skill"}}, + {Metadata: skills.SkillMetadata{Name: "already-current"}}, + {Metadata: skills.SkillMetadata{Name: "extra-skill"}}, + }, nil) + + pr := skillsmocks.NewMockPathResolver(ctrl) + targetDir := filepath.Join(tempDir(t), "installed", "my-skill") + pr.EXPECT().GetSkillPath("claude-code", "my-skill", skills.ScopeProject, projectRoot).Return(targetDir, nil) + + svc := New(store, WithPathResolver(pr), WithRegistryClient(reg), WithOCIStore(ociStore)) + result, err := svc.Sync(t.Context(), skills.SyncOptions{ + ProjectRoot: projectRoot, + Clients: []string{"claude-code"}, + }) + require.NoError(t, err) + assert.Equal(t, []string{"my-skill"}, result.Installed) + assert.Equal(t, []string{"already-current"}, result.UpToDate) + assert.Equal(t, []string{"extra-skill"}, result.Unmanaged) + assert.Empty(t, result.Pruned) + assert.Empty(t, result.Failed) + + // A sync that only restores pinned digests must never rewrite the lock file. + lf, err := lockfile.Load(projectRoot) + require.NoError(t, err) + entry, ok := lf.Get("my-skill") + require.True(t, ok) + assert.Equal(t, "my-skill", entry.Source) +} + +func TestSyncPrunesUnmanagedSkills(t *testing.T) { + t.Parallel() + projectRoot := makeProjectRoot(t) + + ctrl := gomock.NewController(t) + store := storemocks.NewMockSkillStore(ctrl) + store.EXPECT().List(gomock.Any(), storage.ListFilter{Scope: skills.ScopeProject, ProjectRoot: projectRoot}). + Return([]skills.InstalledSkill{ + {Metadata: skills.SkillMetadata{Name: "extra-skill"}, Scope: skills.ScopeProject, ProjectRoot: projectRoot}, + }, nil) + store.EXPECT().Get(gomock.Any(), "extra-skill", skills.ScopeProject, projectRoot). + Return(skills.InstalledSkill{ + Metadata: skills.SkillMetadata{Name: "extra-skill"}, + Scope: skills.ScopeProject, + ProjectRoot: projectRoot, + }, nil) + store.EXPECT().Delete(gomock.Any(), "extra-skill", skills.ScopeProject, projectRoot).Return(nil) + + svc := New(store) + result, err := svc.Sync(t.Context(), skills.SyncOptions{ProjectRoot: projectRoot, Prune: true}) + require.NoError(t, err) + assert.Equal(t, []string{"extra-skill"}, result.Pruned) + assert.Empty(t, result.Unmanaged) +} + +func TestSyncRejectsInvalidProjectRoot(t *testing.T) { + t.Parallel() + ctrl := gomock.NewController(t) + store := storemocks.NewMockSkillStore(ctrl) + + svc := New(store) + _, err := svc.Sync(t.Context(), skills.SyncOptions{ProjectRoot: "relative/path"}) + require.Error(t, err) + assert.Equal(t, http.StatusBadRequest, httperr.Code(err)) +} diff --git a/pkg/skills/skillsvc/uninstall.go b/pkg/skills/skillsvc/uninstall.go index 4c74d8ec98..a6a4193b8f 100644 --- a/pkg/skills/skillsvc/uninstall.go +++ b/pkg/skills/skillsvc/uninstall.go @@ -7,6 +7,7 @@ import ( "context" "errors" "fmt" + "log/slog" "net/http" "os" "path/filepath" @@ -14,6 +15,7 @@ import ( "github.com/stacklok/toolhive-core/httperr" "github.com/stacklok/toolhive/pkg/groups" "github.com/stacklok/toolhive/pkg/skills" + "github.com/stacklok/toolhive/pkg/skills/lockfile" ) // Uninstall removes an installed skill and cleans up files for all clients. @@ -70,6 +72,8 @@ func (s *service) Uninstall(ctx context.Context, opts skills.UninstallOptions) e return err } + removeLockEntry(scope, opts.ProjectRoot, opts.Name) + // Remove the skill from all groups — best-effort, same pattern as file cleanup. if s.groupManager != nil { if groupErr := groups.RemoveSkillFromAllGroups(ctx, s.groupManager, opts.Name); groupErr != nil { @@ -79,3 +83,16 @@ func (s *service) Uninstall(ctx context.Context, opts skills.UninstallOptions) e return errors.Join(cleanupErrs...) } + +// removeLockEntry removes a project's lock file entry, if any — best-effort, +// same pattern as file cleanup. A no-op for user scope or for skills that +// predate the lock file. +func removeLockEntry(scope skills.Scope, projectRoot, name string) { + if scope != skills.ScopeProject || projectRoot == "" { + return + } + if err := lockfile.RemoveEntry(projectRoot, name); err != nil { + slog.Warn("failed to update skills lock file", + "skill", name, "project_root", projectRoot, "error", err) + } +} diff --git a/pkg/skills/skillsvc/upgrade.go b/pkg/skills/skillsvc/upgrade.go new file mode 100644 index 0000000000..6e23b926ec --- /dev/null +++ b/pkg/skills/skillsvc/upgrade.go @@ -0,0 +1,203 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package skillsvc + +import ( + "context" + "errors" + "fmt" + "net/http" + + nameref "github.com/google/go-containerregistry/pkg/name" + + "github.com/stacklok/toolhive-core/httperr" + "github.com/stacklok/toolhive/pkg/skills" + "github.com/stacklok/toolhive/pkg/skills/gitresolver" + "github.com/stacklok/toolhive/pkg/skills/lockfile" +) + +// Upgrade re-resolves each lock file entry's original source and, when the +// resolved digest differs from the pinned one, installs the new content and +// updates the lock file entry. Entries pinned to an immutable reference (an +// OCI digest or a full git commit hash) are reported as not upgradable. +func (s *service) Upgrade(ctx context.Context, opts skills.UpgradeOptions) (*skills.UpgradeResult, error) { + projectRoot, err := skills.ValidateProjectRoot(opts.ProjectRoot) + if err != nil { + return nil, httperr.WithCode(err, http.StatusBadRequest) + } + + lf, err := lockfile.Load(projectRoot) + if err != nil { + return nil, fmt.Errorf("loading lock file: %w", err) + } + + targets, err := selectUpgradeTargets(lf.Skills, opts.Names) + if err != nil { + return nil, err + } + + result := &skills.UpgradeResult{} + for _, entry := range targets { + result.Outcomes = append(result.Outcomes, s.upgradeEntry(ctx, projectRoot, entry, opts)) + } + return result, nil +} + +// selectUpgradeTargets returns the subset of entries named in names, in the +// order requested, or all entries when names is empty. Requesting a name +// absent from the lock file is a 404, not a silent skip. +func selectUpgradeTargets(entries []lockfile.Entry, names []string) ([]lockfile.Entry, error) { + if len(names) == 0 { + return entries, nil + } + byName := make(map[string]lockfile.Entry, len(entries)) + for _, e := range entries { + byName[e.Name] = e + } + targets := make([]lockfile.Entry, 0, len(names)) + for _, name := range names { + entry, ok := byName[name] + if !ok { + return nil, httperr.WithCode( + fmt.Errorf("skill %q is not present in the lock file", name), http.StatusNotFound) + } + targets = append(targets, entry) + } + return targets, nil +} + +// upgradeEntry attempts to upgrade a single lock entry. +func (s *service) upgradeEntry( + ctx context.Context, projectRoot string, entry lockfile.Entry, opts skills.UpgradeOptions, +) skills.UpgradeOutcome { + if isImmutableSource(entry.Source) { + return skills.UpgradeOutcome{Name: entry.Name, Status: skills.UpgradeStatusNotUpgradable, OldDigest: entry.Digest} + } + + if opts.DryRun { + return s.upgradeEntryDryRun(ctx, entry) + } + + // installInternal (not Install) is used deliberately: re-installing from + // entry.Source must not change the lock entry's Source field, and this + // function decides for itself whether and how to rewrite the entry below. + result, err := s.installInternal(ctx, skills.InstallOptions{ + Name: entry.Source, + Scope: skills.ScopeProject, + ProjectRoot: projectRoot, + Clients: opts.Clients, + Force: true, + }) + if err != nil { + return skills.UpgradeOutcome{ + Name: entry.Name, Status: skills.UpgradeStatusFailed, OldDigest: entry.Digest, Error: err.Error(), + } + } + + if result.Skill.Digest == entry.Digest { + return skills.UpgradeOutcome{ + Name: entry.Name, Status: skills.UpgradeStatusUpToDate, OldDigest: entry.Digest, NewDigest: result.Skill.Digest, + } + } + + newEntry := lockfile.Entry{ + Name: entry.Name, + Version: result.Skill.Metadata.Version, + Source: entry.Source, + ResolvedReference: result.Skill.Reference, + Digest: result.Skill.Digest, + } + if lockErr := lockfile.UpsertEntry(projectRoot, newEntry); lockErr != nil { + return skills.UpgradeOutcome{ + Name: entry.Name, Status: skills.UpgradeStatusFailed, OldDigest: entry.Digest, NewDigest: result.Skill.Digest, + Error: fmt.Sprintf("skill upgraded but failed to update lock file: %v", lockErr), + } + } + return skills.UpgradeOutcome{ + Name: entry.Name, Status: skills.UpgradeStatusUpgraded, OldDigest: entry.Digest, NewDigest: result.Skill.Digest, + } +} + +// upgradeEntryDryRun reports what an upgrade would do without installing +// anything or modifying the lock file. +func (s *service) upgradeEntryDryRun(ctx context.Context, entry lockfile.Entry) skills.UpgradeOutcome { + newDigest, err := s.resolveLatestDigest(ctx, entry.Source) + if err != nil { + return skills.UpgradeOutcome{ + Name: entry.Name, Status: skills.UpgradeStatusFailed, OldDigest: entry.Digest, Error: err.Error(), + } + } + if newDigest == entry.Digest { + return skills.UpgradeOutcome{ + Name: entry.Name, Status: skills.UpgradeStatusUpToDate, OldDigest: entry.Digest, NewDigest: newDigest, + } + } + return skills.UpgradeOutcome{ + Name: entry.Name, Status: skills.UpgradeStatusUpgraded, OldDigest: entry.Digest, NewDigest: newDigest, + } +} + +// resolveLatestDigest resolves source to the digest or commit hash it +// currently points to, without installing it or writing any files or DB +// records. Used for --dry-run upgrade checks. +// +// There is no lightweight "peek" API for either backend, so this still pulls +// the OCI artifact into the local cache, or clones the git repository — but +// it performs none of the extraction, filesystem, or database writes that a +// real install does. +func (s *service) resolveLatestDigest(ctx context.Context, source string) (string, error) { + if gitresolver.IsGitReference(source) { + return s.resolveGitDigest(ctx, source) + } + + ref, isOCI, err := parseOCIReference(source) + if err != nil { + return "", httperr.WithCode(fmt.Errorf("invalid OCI reference %q: %w", source, err), http.StatusBadRequest) + } + if isOCI { + return s.resolveOCIDigest(ctx, ref) + } + + // Plain registry name — resolve through the catalog to find the current package. + resolved, err := s.resolveFromRegistry(source) + if err != nil { + return "", err + } + if resolved == nil { + return "", httperr.WithCode(fmt.Errorf("skill %q not found in registry", source), http.StatusNotFound) + } + if resolved.OCIRef != nil { + return s.resolveOCIDigest(ctx, resolved.OCIRef) + } + return s.resolveGitDigest(ctx, resolved.GitURL) +} + +func (s *service) resolveGitDigest(ctx context.Context, gitURL string) (string, error) { + if s.gitResolver == nil { + return "", httperr.WithCode(errors.New("git resolver is not configured"), http.StatusInternalServerError) + } + gitRef, err := gitresolver.ParseGitReference(gitURL) + if err != nil { + return "", httperr.WithCode(fmt.Errorf("invalid git reference: %w", err), http.StatusBadRequest) + } + resolved, err := s.gitResolver.Resolve(ctx, gitRef) + if err != nil { + return "", httperr.WithCode(fmt.Errorf("resolving git skill: %w", err), http.StatusBadGateway) + } + return resolved.CommitHash, nil +} + +func (s *service) resolveOCIDigest(ctx context.Context, ref nameref.Reference) (string, error) { + if s.registry == nil || s.ociStore == nil { + return "", httperr.WithCode(errors.New("OCI registry is not configured"), http.StatusInternalServerError) + } + ociRef := qualifiedOCIRef(ref) + pullCtx, cancel := context.WithTimeout(ctx, ociPullTimeout) + defer cancel() + d, err := s.registry.Pull(pullCtx, s.ociStore, ociRef) + if err != nil { + return "", httperr.WithCode(fmt.Errorf("pulling OCI artifact %q: %w", ociRef, err), classifyPullError(err)) + } + return d.String(), nil +} diff --git a/pkg/skills/skillsvc/upgrade_test.go b/pkg/skills/skillsvc/upgrade_test.go new file mode 100644 index 0000000000..bdd8aedaaa --- /dev/null +++ b/pkg/skills/skillsvc/upgrade_test.go @@ -0,0 +1,164 @@ +// SPDX-FileCopyrightText: Copyright 2025 Stacklok, Inc. +// SPDX-License-Identifier: Apache-2.0 + +package skillsvc + +import ( + "net/http" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + "github.com/stacklok/toolhive-core/httperr" + ociskills "github.com/stacklok/toolhive-core/oci/skills" + ocimocks "github.com/stacklok/toolhive-core/oci/skills/mocks" + "github.com/stacklok/toolhive/pkg/skills" + "github.com/stacklok/toolhive/pkg/skills/lockfile" + skillsmocks "github.com/stacklok/toolhive/pkg/skills/mocks" + storemocks "github.com/stacklok/toolhive/pkg/storage/mocks" +) + +func TestUpgradeInstallsNewDigestAndRewritesLockEntry(t *testing.T) { + t.Parallel() + projectRoot := makeProjectRoot(t) + + ociStore, err := ociskills.NewStore(tempDir(t)) + require.NoError(t, err) + newDigest := buildTestArtifact(t, ociStore, "my-skill", "2.0.0") + + require.NoError(t, lockfile.UpsertEntry(projectRoot, lockfile.Entry{ + Name: "my-skill", + Version: "1.0.0", + Source: "ghcr.io/org/my-skill:v1", + ResolvedReference: "ghcr.io/org/my-skill:v1", + Digest: "sha256:old", + })) + + ctrl := gomock.NewController(t) + reg := ocimocks.NewMockRegistryClient(ctrl) + reg.EXPECT().Pull(gomock.Any(), ociStore, "ghcr.io/org/my-skill:v1").Return(newDigest, nil) + + store := storemocks.NewMockSkillStore(ctrl) + existing := skills.InstalledSkill{ + Metadata: skills.SkillMetadata{Name: "my-skill", Version: "1.0.0"}, + Scope: skills.ScopeProject, + ProjectRoot: projectRoot, + Digest: "sha256:old", + Clients: []string{"claude-code"}, + } + store.EXPECT().Get(gomock.Any(), "my-skill", skills.ScopeProject, projectRoot).Return(existing, nil) + store.EXPECT().Update(gomock.Any(), gomock.Any()).Return(nil) + + pr := skillsmocks.NewMockPathResolver(ctrl) + targetDir := filepath.Join(tempDir(t), "installed", "my-skill") + pr.EXPECT().GetSkillPath("claude-code", "my-skill", skills.ScopeProject, projectRoot).Return(targetDir, nil) + + svc := New(store, WithPathResolver(pr), WithRegistryClient(reg), WithOCIStore(ociStore)) + result, err := svc.Upgrade(t.Context(), skills.UpgradeOptions{ + ProjectRoot: projectRoot, + Clients: []string{"claude-code"}, + }) + require.NoError(t, err) + require.Len(t, result.Outcomes, 1) + outcome := result.Outcomes[0] + assert.Equal(t, skills.UpgradeStatusUpgraded, outcome.Status) + assert.Equal(t, "sha256:old", outcome.OldDigest) + assert.Equal(t, newDigest.String(), outcome.NewDigest) + + lf, err := lockfile.Load(projectRoot) + require.NoError(t, err) + require.Len(t, lf.Skills, 1) + assert.Equal(t, newDigest.String(), lf.Skills[0].Digest) + assert.Equal(t, "2.0.0", lf.Skills[0].Version) + // Source is what "thv skill upgrade" re-resolves next time — it must + // never be replaced by the concrete reference an upgrade happened to use. + assert.Equal(t, "ghcr.io/org/my-skill:v1", lf.Skills[0].Source) +} + +func TestUpgradeDryRunReportsWithoutInstallingOrRewritingLock(t *testing.T) { + t.Parallel() + projectRoot := makeProjectRoot(t) + + ociStore, err := ociskills.NewStore(tempDir(t)) + require.NoError(t, err) + newDigest := buildTestArtifact(t, ociStore, "my-skill", "2.0.0") + + require.NoError(t, lockfile.UpsertEntry(projectRoot, lockfile.Entry{ + Name: "my-skill", + Source: "ghcr.io/org/my-skill:v1", + Digest: "sha256:old", + })) + + ctrl := gomock.NewController(t) + reg := ocimocks.NewMockRegistryClient(ctrl) + reg.EXPECT().Pull(gomock.Any(), ociStore, "ghcr.io/org/my-skill:v1").Return(newDigest, nil) + + // No SkillStore or PathResolver expectations: a dry run must never touch + // installed state. + store := storemocks.NewMockSkillStore(ctrl) + + svc := New(store, WithRegistryClient(reg), WithOCIStore(ociStore)) + result, err := svc.Upgrade(t.Context(), skills.UpgradeOptions{ProjectRoot: projectRoot, DryRun: true}) + require.NoError(t, err) + require.Len(t, result.Outcomes, 1) + assert.Equal(t, skills.UpgradeStatusUpgraded, result.Outcomes[0].Status) + assert.Equal(t, newDigest.String(), result.Outcomes[0].NewDigest) + + lf, err := lockfile.Load(projectRoot) + require.NoError(t, err) + require.Len(t, lf.Skills, 1) + assert.Equal(t, "sha256:old", lf.Skills[0].Digest) +} + +func TestUpgradeImmutableSourceReportsNotUpgradable(t *testing.T) { + t.Parallel() + + fakeCommit := strings.Repeat("a", 40) + fakeDigest := "sha256:" + strings.Repeat("b", 64) + + tests := []struct { + name string + source string + }{ + {name: "OCI digest reference", source: "ghcr.io/org/my-skill@" + fakeDigest}, + {name: "git full commit hash", source: "git://github.com/org/my-skill@" + fakeCommit}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + projectRoot := makeProjectRoot(t) + require.NoError(t, lockfile.UpsertEntry(projectRoot, lockfile.Entry{ + Name: "my-skill", + Source: tt.source, + Digest: "sha256:pinned", + })) + + store := storemocks.NewMockSkillStore(gomock.NewController(t)) + svc := New(store) + result, err := svc.Upgrade(t.Context(), skills.UpgradeOptions{ProjectRoot: projectRoot}) + require.NoError(t, err) + require.Len(t, result.Outcomes, 1) + assert.Equal(t, skills.UpgradeStatusNotUpgradable, result.Outcomes[0].Status) + assert.Equal(t, "sha256:pinned", result.Outcomes[0].OldDigest) + }) + } +} + +func TestUpgradeUnknownNameReturns404(t *testing.T) { + t.Parallel() + projectRoot := makeProjectRoot(t) + + store := storemocks.NewMockSkillStore(gomock.NewController(t)) + svc := New(store) + _, err := svc.Upgrade(t.Context(), skills.UpgradeOptions{ + ProjectRoot: projectRoot, + Names: []string{"missing-skill"}, + }) + require.Error(t, err) + assert.Equal(t, http.StatusNotFound, httperr.Code(err)) +}