diff --git a/cmd/add.go b/cmd/add.go index 1111010..b5dcd7d 100644 --- a/cmd/add.go +++ b/cmd/add.go @@ -29,6 +29,7 @@ var ( addAs string addAll bool addLocal string + addVendor bool ) var addCmd = &cobra.Command{ @@ -102,7 +103,9 @@ func init() { addCmd.Flags().BoolVar(&addAll, "all", false, "install every skill the registry exposes (requires --registry; do not name a skill)") addCmd.Flags().StringVar(&addLocal, "local", "", - "install an immutable copy of a skill from a local folder (no registry; `qvr edit` to make it mutable)") + "install an immutable copy of a skill from a local folder (no registry; `qvr edit` to make it mutable, or add --vendor to commit it into the repo)") + addCmd.Flags().BoolVar(&addVendor, "vendor", false, + "commit the skill as real files in the repo (no symlink into the store) so it travels with a `git clone` — no store, registry, or qvr needed to read it. Composes with --local.") rootCmd.AddCommand(addCmd) } @@ -333,6 +336,7 @@ func addInstallItem(cmd *cobra.Command, cfg *config.Config, installer *skill.Ins Registry: item.registry, SkillPath: item.skillPath, As: addAs, + Vendor: addVendor, RequireSigned: cfg.Security.RequireSigned, TrustedAuthors: trustedAuthorsForRegistry(cfg, item.registry), TrustedAuthorsByRegistry: trustedAuthorsByRegistry(cfg), @@ -702,6 +706,12 @@ func validateAddModes() error { return fmt.Errorf("--frozen cannot be combined with --local") } } + if addVendor && addFrozen { + // --frozen pins to the recorded store hash and forbids drift; vendoring + // rewrites how the skill lands (real in-repo files), so the two are + // contradictory. + return fmt.Errorf("--vendor cannot be combined with --frozen") + } return nil } @@ -763,6 +773,7 @@ func runAddLocal(cmd *cobra.Command, cfg *config.Config, installer *skill.Instal ProjectRoot: projectRoot, LockPath: lockPath, Force: addForce, + Vendor: addVendor, }) if ierr != nil { printer.Error(fmt.Sprintf("add --local %s: %v", resolved, ierr)) @@ -1272,6 +1283,12 @@ func skillDirForEntry(result *skill.InstallResult, lock *model.LockFile) string return result.Worktree } worktreePath := skill.EntryWorktreePath(entry) + if worktreePath == "" { + // No store worktree (vendor/edit/link): the content lives where + // result.Worktree points — already at the skill root, so don't join + // entry.Path. result.Worktree is EffectiveTarget for these modes. + return result.Worktree + } if entry.Path == "" || entry.Path == "." { return worktreePath } diff --git a/cmd/edit.go b/cmd/edit.go index ec5cd7e..5febb71 100644 --- a/cmd/edit.go +++ b/cmd/edit.go @@ -123,6 +123,11 @@ func ejectUnderLock(name, projectRoot, lockPath, projPath string, result **skill if entry.IsLink() { return fmt.Errorf("%s is a link install at %s — edit the source path directly", name, entry.Source) } + // Vendored skills are already real, writable files committed in the repo; + // there's nothing to eject — point the user at the in-repo dir. + if entry.IsVendor() { + return fmt.Errorf("%s is vendored at %s — its files are real and editable in the repo; edit them directly", name, entry.VendorPath) + } // Already ejected: no-op, return current state. if entry.IsEdit() { *idempotent = true diff --git a/internal/model/lockfile.go b/internal/model/lockfile.go index bd93454..699e9fc 100644 --- a/internal/model/lockfile.go +++ b/internal/model/lockfile.go @@ -80,10 +80,11 @@ var ( // Install mode constants. Empty string means "shared" (default add semantics) // for backward compatibility — older v5 locks predate this field. const ( - ModeShared = "" // symlink → ~/.quiver/worktrees/.../ (default for `qvr add`) - ModeEdit = "edit" // canonical real dir at EditPath (set by `qvr edit`) - ModeLink = "link" // absolute path in Source (legacy `qvr link`; read-only compat) - ModeLocal = "local" // immutable copy of a local folder (set by `qvr add --local`) + ModeShared = "" // symlink → ~/.quiver/worktrees/.../ (default for `qvr add`) + ModeEdit = "edit" // canonical real dir at EditPath (set by `qvr edit`) + ModeLink = "link" // absolute path in Source (legacy `qvr link`; read-only compat) + ModeLocal = "local" // immutable copy of a local folder (set by `qvr add --local`) + ModeVendor = "vendor" // real files committed into the repo at VendorPath (set by `qvr add --vendor`) ) // LocalRegistry is the reserved registry name stamped on `qvr add --local` @@ -180,6 +181,15 @@ type LockEntry struct { // target carry relative symlinks to it. Empty when Mode != "edit". EditPath string `json:"editPath,omitempty" toml:"editPath,omitempty"` + // VendorPath is the project-relative path of the canonical vendored + // copy when Mode == "vendor" (e.g. ".claude/skills/auth"). Like + // EditPath it is a real in-repo directory the canonical target IS, + // with sibling targets carrying relative symlinks to it — but the + // bytes are tracked by the outer repo (no nested .git), so a teammate + // who clones the repo gets the skill files with no store, registry, or + // qvr required. Empty when Mode != "vendor". + VendorPath string `json:"vendorPath,omitempty" toml:"vendorPath,omitempty"` + // Targets is the list of agent dirs the skill is symlinked into. Targets []string `json:"targets" toml:"targets"` @@ -289,6 +299,20 @@ func (e *LockEntry) IsEdit() bool { return e.Mode == ModeEdit } +// IsVendor reports whether this entry is vendored: real skill files committed +// into the repo at VendorPath via `qvr add --vendor`. The canonical agent +// target dir at VendorPath is itself a real directory (sibling targets repoint +// at it), and the bytes are version-controlled by the outer repo rather than +// the qvr store — so the skill travels with a `git clone` without needing a +// store worktree or a reachable registry. Like edit installs, the shared store +// worktree is not load-bearing for a vendored skill. +func (e *LockEntry) IsVendor() bool { + if e == nil { + return false + } + return e.Mode == ModeVendor +} + // LockFile is the on-disk record of installed skills. // // The lockfile is qvr's pillar of portability, governance, and reproducibility: diff --git a/internal/registry/path.go b/internal/registry/path.go index adc8144..4c5be4a 100644 --- a/internal/registry/path.go +++ b/internal/registry/path.go @@ -233,6 +233,14 @@ func WorktreePathForEntry(e *model.LockEntry) string { if e.IsLink() { return e.Source } + // Vendored skills live in-repo at VendorPath, not in the store. They claim + // no store worktree, so the transient one materialized at install time + // becomes orphan and is reclaimed by `qvr cache prune` (the content is + // content-addressed and shared, so prune only drops it when no other + // project references it). Returning a path here would wrongly pin it. + if e.IsVendor() { + return "" + } key := e.InstallCommit if key == "" { key = e.Commit diff --git a/internal/skill/eject.go b/internal/skill/eject.go index 54fdec6..1a509ae 100644 --- a/internal/skill/eject.go +++ b/internal/skill/eject.go @@ -28,6 +28,13 @@ var ( // ErrNoTargets is returned by Eject when the lock entry records no // installed targets, leaving no agent dir to eject the copy into. ErrNoTargets = errors.New("entry has no installed targets to eject into") + // ErrCannotVendorLink is returned by VendorIntoRepo for a link install: + // its bytes live at an external path with no store worktree to copy in. + ErrCannotVendorLink = errors.New("cannot vendor a link install") + // ErrCannotEjectVendor is returned by EjectToTarget for a vendored entry: + // its files are already real and writable in the repo, so there is nothing + // to eject. + ErrCannotEjectVendor = errors.New("cannot eject a vendored install — its files are already editable in the repo") ) // EjectRequest drives a `qvr edit` eject. @@ -79,6 +86,12 @@ func EjectToTarget(req EjectRequest) (*EjectResult, error) { if e.IsLink() { return nil, ErrCannotEjectLink } + if e.IsVendor() { + // A vendored skill is already real, writable, in-repo files — there is + // no store worktree to eject and the canonical target is a real dir, so + // the eject rename would refuse to clobber it. Reject explicitly. + return nil, ErrCannotEjectVendor + } if len(e.Targets) == 0 { return nil, ErrNoTargets } @@ -112,6 +125,12 @@ func EjectToTarget(req EjectRequest) (*EjectResult, error) { siblingLinks, err := repointSiblingTargets(e, req, canonicalTarget, canonicalAbs) if err != nil { + // materializeEjectDir already created the canonical real directory; a + // sibling-relink failure must not leave it behind. rollbackLinks (in the + // caller) only removes symlinks, so the real dir would orphan and block + // future installs of this skill. Remove it here to keep eject/vendor + // atomic — the caller still rolls back the target symlinks. + _ = os.RemoveAll(canonicalAbs) return nil, err } @@ -128,47 +147,66 @@ func EjectToTarget(req EjectRequest) (*EjectResult, error) { // materializeEjectDir resolves the shared-worktree source, copies the skill // subtree into a staging sibling dir, restores write bits (the immutable→editable // hinge), inits a fresh git history, then atomically renames the staging dir onto -// the canonical slot — refusing to clobber a real (non-symlink) dir there. The -// staging-then-rename avoids leaving half-copied state if any step fails midway. +// the canonical slot. Thin wrapper over copyTreeToCanonical with the edit-mode +// settings (nested git history seeded). func materializeEjectDir(e *model.LockEntry, req EjectRequest, canonicalAbs string) error { - // Resolve the shared worktree source: where we'll copy the skill files - // from. Falls back to LoadFromPath when the worktree isn't on disk so a - // user who deleted ~/.quiver/worktrees/ before invoking edit gets a clean - // error instead of an empty copy. - sourceDir := EffectiveTarget(e, req.ProjectRoot) + return copyTreeToCanonical(e, req.ProjectRoot, canonicalAbs, ".ejecting", true, req.Author, req.AuthorEmail) +} + +// copyTreeToCanonical resolves the entry's current effective source (the shared +// store worktree or the local copy), copies it into a staging sibling of +// canonicalAbs, restores write bits, optionally seeds a fresh nested git history, +// then atomically renames the staging dir onto the canonical agent-dir slot — +// refusing to clobber a real (non-symlink) dir there. The staging-then-rename +// avoids leaving half-copied state if any step fails midway. +// +// Shared by `qvr edit` (initNestedRepo=true: the eject dir is a standalone fork +// that `qvr publish` later pushes) and `qvr add --vendor` (initNestedRepo=false: +// the bytes are tracked by the OUTER project repo, so a nested .git would be both +// redundant and a committed-repo-inside-a-repo footgun). +func copyTreeToCanonical(e *model.LockEntry, projectRoot, canonicalAbs, stagingSuffix string, initNestedRepo bool, author, authorEmail string) error { + // Resolve the source: where we'll copy the skill files from. Returns "" + // when the worktree isn't on disk so a user who deleted ~/.quiver/worktrees/ + // gets a clean error instead of an empty copy. + sourceDir := EffectiveTarget(e, projectRoot) if sourceDir == "" { - return fmt.Errorf("eject %s: no source worktree to copy from — run `qvr sync` first", e.Name) + return fmt.Errorf("%s: no source worktree to copy from — run `qvr sync` first", e.Name) } if _, err := os.Stat(filepath.Join(sourceDir, "SKILL.md")); err != nil { - return fmt.Errorf("eject %s: source %s does not contain SKILL.md: %w", e.Name, sourceDir, err) + return fmt.Errorf("%s: source %s does not contain SKILL.md: %w", e.Name, sourceDir, err) } // Stage to a sibling tmp dir, then rename onto canonical. Avoids leaving // half-copied state if the copy / git init fails midway. - stagingDir := canonicalAbs + ".ejecting" + stagingDir := canonicalAbs + stagingSuffix _ = os.RemoveAll(stagingDir) if err := copyDir(sourceDir, stagingDir); err != nil { _ = os.RemoveAll(stagingDir) return fmt.Errorf("copy skill tree: %w", err) } - // The shared worktree is frozen read-only and copyDir preserves source - // modes, so the freshly-copied edit tree would be read-only. Restore write - // bits: this copy is the editable working dir, and initEjectRepo is about - // to `git init` and write into it. This is the immutable→editable hinge. + // The source worktree is frozen read-only and copyDir preserves source + // modes, so the freshly-copied tree would be read-only. Restore write bits: + // this copy is the working dir the user (and any `git init` below) writes to. setSubtreeWritable(stagingDir) - if err := initEjectRepo(stagingDir, e, req.Author, req.AuthorEmail); err != nil { - _ = os.RemoveAll(stagingDir) - return fmt.Errorf("init edit repo: %w", err) + if initNestedRepo { + if err := initEjectRepo(stagingDir, e, author, authorEmail); err != nil { + _ = os.RemoveAll(stagingDir) + return fmt.Errorf("init edit repo: %w", err) + } } + return promoteStagingOntoCanonical(e, stagingDir, canonicalAbs) +} - // Remove the existing canonical symlink (or no-op if it isn't there) so - // the rename below lands on a clean slot. CreateSymlink earlier may have - // pointed it at the shared worktree; if a real dir is there already we - // refuse to clobber user content. +// promoteStagingOntoCanonical removes the existing canonical symlink (or no-op +// if absent) so the rename lands on a clean slot, then atomically renames the +// staging dir onto it. CreateSymlink earlier may have pointed the canonical at +// the shared worktree; if a real dir is there already we refuse to clobber user +// content. On any failure the staging dir is removed so no half-state lingers. +func promoteStagingOntoCanonical(e *model.LockEntry, stagingDir, canonicalAbs string) error { if existing, err := os.Lstat(canonicalAbs); err == nil { if existing.Mode()&os.ModeSymlink == 0 { _ = os.RemoveAll(stagingDir) - return fmt.Errorf("eject %s: %s exists and is not a symlink — refuse to overwrite", e.Name, canonicalAbs) + return fmt.Errorf("%s: %s exists and is not a symlink — refuse to overwrite", e.Name, canonicalAbs) } if err := os.Remove(canonicalAbs); err != nil { _ = os.RemoveAll(stagingDir) @@ -181,11 +219,116 @@ func materializeEjectDir(e *model.LockEntry, req EjectRequest, canonicalAbs stri } if err := os.Rename(stagingDir, canonicalAbs); err != nil { _ = os.RemoveAll(stagingDir) - return fmt.Errorf("finalize edit dir: %w", err) + return fmt.Errorf("finalize canonical dir: %w", err) } return nil } +// VendorRequest drives a `qvr add --vendor` post-install vendoring step. +type VendorRequest struct { + Entry *model.LockEntry // the freshly-installed lock entry (mutated in place on success) + ProjectRoot string // absolute project root + // Global, when true, vendors into the user-global agent dir and writes + // VendorPath as an absolute path; project scope (default) writes a + // project-relative VendorPath so the lockfile stays portable across clones. + Global bool +} + +// VendorResult summarises a vendoring for the caller. +type VendorResult struct { + Skill string `json:"skill"` + CanonicalTarget string `json:"canonical_target"` + VendorPath string `json:"vendor_path"` // project-relative (or absolute when Global) + SiblingLinks []string `json:"sibling_links"` // absolute paths of repointed sibling symlinks +} + +// VendorIntoRepo promotes a freshly-installed skill's store worktree into a real +// directory committed into the repo at the alphabetical-first installed target, +// and repoints any other installed targets at it via relative symlinks. The lock +// entry is mutated in place: Mode flips to "vendor", VendorPath records the +// project-relative canonical path, and SubtreeHash is re-sealed against the +// in-repo dir. +// +// Unlike EjectToTarget it seeds NO nested git history — the vendored bytes are +// tracked by the OUTER project repo, which is exactly what lets the skill travel +// with a `git clone` (no store, no registry, no qvr needed to read it). It is the +// `--vendor` counterpart to a normal symlink-into-store install. +// +// Idempotent: a second call when Mode is already "vendor" at the same path is a +// no-op. Refuses link installs (no store worktree to copy from). +func VendorIntoRepo(req VendorRequest) (*VendorResult, error) { + e := req.Entry + if e == nil { + return nil, errors.New("vendor: nil entry") + } + if req.ProjectRoot == "" { + return nil, errors.New("vendor: project root is required") + } + if e.IsLink() { + return nil, ErrCannotVendorLink + } + if len(e.Targets) == 0 { + return nil, ErrNoTargets + } + + canonicalTarget := pickCanonicalTarget(e.Targets) + t, ok := model.LookupTarget(canonicalTarget) + if !ok { + return nil, fmt.Errorf("%w: %s", ErrUnknownTarget, canonicalTarget) + } + + // Reuse the eject path-derivation: --global → absolute canonical/VendorPath, + // project → project-relative. The EjectRequest carries only scope here. + scope := EjectRequest{ProjectRoot: req.ProjectRoot, Global: req.Global} + canonicalAbs, vendorPathForLock, err := ejectCanonicalPaths(t, e, scope) + if err != nil { + return nil, err + } + + // Idempotent no-op when the entry already records this exact vendoring. + if e.IsVendor() && e.VendorPath == vendorPathForLock { + return &VendorResult{Skill: e.Name, CanonicalTarget: canonicalTarget, VendorPath: vendorPathForLock}, nil + } + + if err := copyTreeToCanonical(e, req.ProjectRoot, canonicalAbs, ".vendoring", false, "", ""); err != nil { + return nil, err + } + + siblingLinks, err := repointSiblingTargets(e, scope, canonicalTarget, canonicalAbs) + if err != nil { + // copyTreeToCanonical already materialized the canonical real directory; + // a sibling-relink failure must not leave it behind. The caller's + // rollbackLinks only removes symlinks, so the real dir would orphan and + // block future installs of this skill at that path. Remove it so vendor + // stays atomic (matching EjectToTarget). + _ = os.RemoveAll(canonicalAbs) + return nil, err + } + + finalizeVendoredEntry(e, vendorPathForLock, canonicalAbs) + + return &VendorResult{ + Skill: e.Name, + CanonicalTarget: canonicalTarget, + VendorPath: vendorPathForLock, + SiblingLinks: siblingLinks, + }, nil +} + +// finalizeVendoredEntry mutates the entry on success: flips it to mode:vendor, +// records the VendorPath, and re-seals SubtreeHash against the in-repo dir so +// drift detection (`qvr lock verify`) has a current baseline. The copy is +// byte-identical to the store worktree, so the hash normally matches what the +// install already recorded; re-hashing is defensive and matches eject's pattern. +// HashSubtreeFromDisk excludes .git/, so the seal agrees with a later verify. +func finalizeVendoredEntry(e *model.LockEntry, vendorPathForLock, canonicalAbs string) { + e.Mode = model.ModeVendor + e.VendorPath = vendorPathForLock + if h, err := canonical.HashSubtreeFromDisk(canonicalAbs); err == nil { + e.SubtreeHash = h + } +} + // ejectCanonicalPaths computes the canonical eject dir and the EditPath recorded // in the lock, scoped per issue #82: // diff --git a/internal/skill/installer.go b/internal/skill/installer.go index 75061e4..d3dcad5 100644 --- a/internal/skill/installer.go +++ b/internal/skill/installer.go @@ -113,6 +113,15 @@ type InstallRequest struct { // to the by-name full-index lookup, so this is a pure speedup, never a // correctness change. SkillPath string + // Vendor materializes the skill as real files committed into the repo at + // VendorPath instead of a symlink into the shared store (`qvr add --vendor`). + // After the normal resolve + scan + store materialization, the content is + // promoted into the canonical agent dir as a real directory (sibling targets + // repoint at it via relative symlinks) and the lock entry flips to + // mode:vendor — so the skill travels with a `git clone` without needing the + // store or a reachable registry. Composes with any source (registry or + // --local). + Vendor bool } // InstallResult holds the outcome for a single skill install. @@ -206,11 +215,31 @@ func (in *Installer) Install(req InstallRequest) (*InstallResult, error) { return nil, err } if err := lock.Write(); err != nil { + in.rollbackInstallResult(result, req) return nil, fmt.Errorf("write lock file: %w", err) } return result, nil } +// rollbackInstallResult undoes a completed InstallInto when the single-install +// caller's lock.Write fails: it removes the target symlinks and, for a vendored +// install, the real canonical directory VendorIntoRepo materialized (a plain +// symlink rollback can't, since it's a real dir — a retry would otherwise hit +// "exists and is not a symlink"). Best-effort. +func (in *Installer) rollbackInstallResult(result *InstallResult, req InstallRequest) { + if result == nil { + return + } + for _, t := range result.Targets { + if p, perr := ResolveTargetPath(t, result.Name, req.ProjectRoot, req.Global); perr == nil { + _ = RemoveSymlink(p) + } + } + if req.Vendor && result.Worktree != "" { + _ = os.RemoveAll(result.Worktree) + } +} + // InstallInto runs the full install flow against an already-loaded, in-memory // lock instead of reading and writing the lockfile itself. The caller owns the // lock's lifecycle: it must hold the project file lock (model.WithLock), pass a @@ -280,7 +309,8 @@ func (in *Installer) InstallInto(req InstallRequest, lock *model.LockFile) (*Ins setSubtreeReadOnly(skillDir) } - if err := in.linkInstallTargets(req, plan, skillDir); err != nil { + created, err := in.linkInstallTargets(req, plan, skillDir) + if err != nil { return nil, err } @@ -292,11 +322,25 @@ func (in *Installer) InstallInto(req InstallRequest, lock *model.LockFile) (*Ins } lock.Put(entry) + worktree := finalPath + // --vendor: promote the just-materialized store copy into real files + // committed in the repo, flipping the entry to mode:vendor. The entry is the + // pointer held in the lock map, so mutating it in place is reflected on the + // caller's lock.Write(). The store worktree stays on disk (shared, content- + // addressed) until `qvr cache prune` reclaims it as an orphan. + if req.Vendor { + if _, verr := VendorIntoRepo(VendorRequest{Entry: entry, ProjectRoot: req.ProjectRoot, Global: req.Global}); verr != nil { + rollbackLinks(created) + return nil, fmt.Errorf("vendor %s: %w", localName, verr) + } + worktree = EffectiveTarget(entry, req.ProjectRoot) + } + result := &InstallResult{ Name: localName, Registry: loc.RegistryName, Version: version, - Worktree: finalPath, + Worktree: worktree, Targets: targets, Commit: commit, } @@ -462,7 +506,7 @@ func (in *Installer) gateInstallTrust(req InstallRequest, plan *installPlan, ski // a sanitized agent view for a legacy root-layout worktree with a live .git/ — // issue #154) and creates a symlink for every requested target, rolling back any // links created so far if one fails. -func (in *Installer) linkInstallTargets(req InstallRequest, plan *installPlan, skillDir string) error { +func (in *Installer) linkInstallTargets(req InstallRequest, plan *installPlan, skillDir string) ([]string, error) { loc := plan.loc finalPath := plan.finalPath localName := plan.localName @@ -477,7 +521,7 @@ func (in *Installer) linkInstallTargets(req InstallRequest, plan *installPlan, s if IsRootLayoutPath(loc.Entry.Path) && HasGitDir(finalPath) { view, verr := buildAgentViewAt(finalPath) if verr != nil { - return fmt.Errorf("agent view: %w", verr) + return nil, fmt.Errorf("agent view: %w", verr) } linkTarget = view } @@ -489,15 +533,15 @@ func (in *Installer) linkInstallTargets(req InstallRequest, plan *installPlan, s linkPath, err := ResolveTargetPath(t, localName, req.ProjectRoot, req.Global) if err != nil { rollbackLinks(created) - return err + return nil, err } if err := CreateSymlink(linkPath, linkTarget); err != nil { rollbackLinks(created) - return fmt.Errorf("symlink %s: %w", t, err) + return nil, fmt.Errorf("symlink %s: %w", t, err) } created = append(created, linkPath) } - return nil + return created, nil } // buildLockEntry assembles the in-memory lock entry for the install: it merges @@ -790,21 +834,27 @@ func (in *Installer) RemoveFrom(name string, req InstallRequest, lock *model.Loc } entryGlobal := lock.IsGlobal(quiverHome()) - canonicalEditAbs := "" + // canonicalDirAbs is the in-repo real directory to rm -rf for the modes + // whose canonical target IS a directory rather than a store symlink: + // edit (EditPath) and vendor (VendorPath). + canonicalDirAbs := "" if entry.IsEdit() { if !req.Force { return fmt.Errorf("remove %s: skill is in edit mode — pass --force to delete the eject dir at %s", name, entry.EditPath) } - canonicalEditAbs = entry.EditPath - if canonicalEditAbs != "" && !filepath.IsAbs(canonicalEditAbs) { - canonicalEditAbs = filepath.Join(req.ProjectRoot, canonicalEditAbs) - } + canonicalDirAbs = absInProject(entry.EditPath, req.ProjectRoot) + } + if entry.IsVendor() { + // Vendored files are reproducible (re-run `qvr add --vendor`) and + // tracked by the outer repo, so — unlike an edit dir's hand-authored + // changes — removal needs no --force; `git checkout` restores them. + canonicalDirAbs = absInProject(entry.VendorPath, req.ProjectRoot) } - // Pass 1: drop target symlinks (and the canonical edit dir, when in - // edit mode). Bail without touching the lock if any step fails so the - // user can recover rather than be left with an orphan lock entry. - if err := removeTargetLinks(name, entry, req, entryGlobal, canonicalEditAbs); err != nil { + // Pass 1: drop target symlinks (and the canonical in-repo dir, for edit/ + // vendor). Bail without touching the lock if any step fails so the user can + // recover rather than be left with an orphan lock entry. + if err := removeTargetLinks(name, entry, req, entryGlobal, canonicalDirAbs); err != nil { return err } @@ -821,24 +871,34 @@ func (in *Installer) RemoveFrom(name string, req InstallRequest, lock *model.Loc return nil } -// removeTargetLinks drops every target symlink for the entry, and for a -// mode:edit canonical target rm -rf's the eject dir (siblings are symlinks +// absInProject resolves a lock-recorded path (EditPath/VendorPath) to absolute: +// project-relative paths join projectRoot, absolute paths (e.g. --global) and +// empty pass through unchanged. +func absInProject(p, projectRoot string) string { + if p == "" || filepath.IsAbs(p) { + return p + } + return filepath.Join(projectRoot, p) +} + +// removeTargetLinks drops every target symlink for the entry, and for an +// edit/vendor canonical target rm -rf's the in-repo dir (siblings are symlinks // pointing at canonical and use RemoveSymlink). Bails on the first failure so // the caller can leave the lock untouched for recovery. -func removeTargetLinks(name string, entry *model.LockEntry, req InstallRequest, entryGlobal bool, canonicalEditAbs string) error { +func removeTargetLinks(name string, entry *model.LockEntry, req InstallRequest, entryGlobal bool, canonicalDirAbs string) error { for _, t := range entry.Targets { linkPath, err := ResolveTargetPath(t, name, req.ProjectRoot, entryGlobal) if err != nil { return fmt.Errorf("remove %s: resolve target %s: %w", name, t, err) } - // For mode:edit canonical target: rm -rf the eject dir. Siblings + // For an edit/vendor canonical target: rm -rf the real dir. Siblings // are symlinks pointing at canonical and use RemoveSymlink. - if entry.IsEdit() && canonicalEditAbs != "" { - canonicalAbs, _ := filepath.Abs(canonicalEditAbs) + if (entry.IsEdit() || entry.IsVendor()) && canonicalDirAbs != "" { + canonicalAbs, _ := filepath.Abs(canonicalDirAbs) absLink, _ := filepath.Abs(linkPath) if canonicalAbs == absLink { if err := os.RemoveAll(linkPath); err != nil { - return fmt.Errorf("remove %s: rm eject dir %s: %w", name, linkPath, err) + return fmt.Errorf("remove %s: rm in-repo dir %s: %w", name, linkPath, err) } continue } @@ -859,7 +919,11 @@ func removeTargetLinks(name string, entry *model.LockEntry, req InstallRequest, // entries never had a shared worktree to clean; link installs point at // user-owned dirs we don't touch. func (in *Installer) removeSharedWorktree(name string, entry *model.LockEntry, req InstallRequest, lock *model.LockFile) error { - if entry.IsLink() || entry.IsEdit() { + // Edit/vendor entries' content is the in-repo dir (removed in pass 1); link + // installs point at user-owned dirs. None claim a store worktree to drop — + // the transient one a vendored install left behind is reclaimed by `qvr + // cache prune` once no other project references it. + if entry.IsLink() || entry.IsEdit() || entry.IsVendor() { return nil } worktreePath := EntryWorktreePath(entry) @@ -958,7 +1022,7 @@ func (in *Installer) InstallLocal(localPath string, req InstallRequest) (*Instal return nil, err } - if err := materializeLocalCopy(abs, finalPath); err != nil { + if err := in.materializeLocalCopy(abs, finalPath); err != nil { return nil, err } // Freeze: same immutability contract as a shared install (see immutable.go) @@ -967,21 +1031,12 @@ func (in *Installer) InstallLocal(localPath string, req InstallRequest) (*Instal // `qvr lock verify` and healed by `qvr sync` (re-copy from Source). setSubtreeReadOnly(finalPath) - var created []string - for _, t := range req.Targets { - linkPath, err := ResolveTargetPath(t, name, req.ProjectRoot, req.Global) - if err != nil { - rollbackLinks(created) - return nil, err - } - if err := CreateSymlink(linkPath, finalPath); err != nil { - rollbackLinks(created) - return nil, fmt.Errorf("symlink %s: %w", t, err) - } - created = append(created, linkPath) + created, err := linkLocalTargets(name, finalPath, req) + if err != nil { + return nil, err } - lock.Put(&model.LockEntry{ + entry := &model.LockEntry{ Name: name, Registry: model.LocalRegistry, Source: abs, @@ -992,21 +1047,58 @@ func (in *Installer) InstallLocal(localPath string, req InstallRequest) (*Instal SubtreeHash: hash, Targets: req.Targets, InstalledAt: time.Now().UTC(), - }) + } + lock.Put(entry) + + worktree := finalPath + // --vendor: promote the just-materialized store copy into real files + // committed in the repo, flipping the entry to mode:vendor. Mutates entry + // in place (the same pointer held in the lock map), so the Put above already + // reflects it. On failure the links we created are rolled back. + if req.Vendor { + if _, verr := VendorIntoRepo(VendorRequest{Entry: entry, ProjectRoot: req.ProjectRoot, Global: req.Global}); verr != nil { + rollbackLinks(created) + return nil, fmt.Errorf("vendor %s: %w", name, verr) + } + worktree = EffectiveTarget(entry, req.ProjectRoot) + } + if err := lock.Write(); err != nil { - rollbackLinks(created) + rollbackVendoredInstall(created, req.Vendor, worktree) return nil, fmt.Errorf("write lock file: %w", err) } return &InstallResult{ Name: name, Registry: model.LocalRegistry, Version: "local", - Worktree: finalPath, + Worktree: worktree, Commit: commitKey, Targets: req.Targets, }, nil } +// linkLocalTargets symlinks every requested target at finalPath (the local +// install's immutable store copy), rolling back any links created so far if one +// fails so the filesystem is left consistent. Mirrors linkInstallTargets for the +// registry path, minus the legacy root-layout agent-view handling a copied local +// folder never needs. +func linkLocalTargets(name, finalPath string, req InstallRequest) ([]string, error) { + var created []string + for _, t := range req.Targets { + linkPath, err := ResolveTargetPath(t, name, req.ProjectRoot, req.Global) + if err != nil { + rollbackLinks(created) + return nil, err + } + if err := CreateSymlink(linkPath, finalPath); err != nil { + rollbackLinks(created) + return nil, fmt.Errorf("symlink %s: %w", t, err) + } + created = append(created, linkPath) + } + return created, nil +} + // checkLocalConflict refuses to silently replace an existing entry of the same // name pointing at a different source. Idempotent when the source matches; // --force needed to re-point. Mirrors the remote-install conflict rule. @@ -1024,11 +1116,15 @@ func checkLocalConflict(lock *model.LockFile, name, abs string, force bool) erro return nil } -// materializeLocalCopy copies the local skill folder into the immutable worktree -// at finalPath if it isn't already on disk. It stages in a sibling dir and -// atomically renames so a crashed copy never leaves a half-written worktree -// behind. copyDir skips .git/ and preserves exec bits. -func materializeLocalCopy(abs, finalPath string) error { +// materializeLocalCopy materializes the local skill folder into the immutable +// worktree at finalPath if it isn't already on disk. It stages in a sibling dir +// and atomically renames so a crashed copy never leaves a half-written worktree +// behind. The copy flows through the SAME Materializer seam as a registry install +// (Materializer.MaterializeFromDisk → BlobMaterializer reflink/content-store, +// #205) rather than a bespoke recursive copy, and skips exactly what the canonical +// hasher excludes (.git/, signature wrappers) so the materialized copy hash-agrees +// with the source. +func (in *Installer) materializeLocalCopy(abs, finalPath string) error { if _, statErr := os.Stat(finalPath); statErr == nil { return nil } @@ -1037,9 +1133,10 @@ func materializeLocalCopy(abs, finalPath string) error { if err := os.MkdirAll(filepath.Dir(finalPath), 0o755); err != nil { return fmt.Errorf("create worktrees dir: %w", err) } - if err := copyDir(abs, stagingPath); err != nil { + mat := &Materializer{Blob: in.Blob} + if err := mat.MaterializeFromDisk(abs, stagingPath); err != nil { _ = os.RemoveAll(stagingPath) - return fmt.Errorf("copy local skill: %w", err) + return fmt.Errorf("materialize local skill: %w", err) } if err := os.Rename(stagingPath, finalPath); err != nil { // Lost the race to a concurrent install of the same content: the @@ -1081,6 +1178,17 @@ func rollbackLinks(paths []string) { } } +// rollbackVendoredInstall removes the created target symlinks and, for a +// vendored install, the real canonical directory VendorIntoRepo materialized — +// which rollbackLinks alone can't, since it's a real dir (a retry would +// otherwise hit "exists and is not a symlink"). Best-effort. +func rollbackVendoredInstall(created []string, vendor bool, vendorDir string) { + rollbackLinks(created) + if vendor && vendorDir != "" { + _ = os.RemoveAll(vendorDir) + } +} + func mergeTargets(existing, add []string) []string { set := make(map[string]struct{}) for _, t := range existing { diff --git a/internal/skill/linker.go b/internal/skill/linker.go index 37d16f2..b9d5451 100644 --- a/internal/skill/linker.go +++ b/internal/skill/linker.go @@ -149,6 +149,15 @@ func EffectiveTarget(entry *model.LockEntry, projectRoot string) string { } return entry.EditPath } + // Vendored skills, like edit installs, live as a real in-repo directory at + // VendorPath rather than a store worktree — the canonical agent dir IS the + // content. Resolve project-relative paths against projectRoot. + if entry.IsVendor() && entry.VendorPath != "" { + if projectRoot != "" && !filepath.IsAbs(entry.VendorPath) { + return filepath.Join(projectRoot, entry.VendorPath) + } + return entry.VendorPath + } if entry.IsLink() { return entry.Source } diff --git a/internal/skill/materialize.go b/internal/skill/materialize.go index 1f2d596..d723723 100644 --- a/internal/skill/materialize.go +++ b/internal/skill/materialize.go @@ -13,6 +13,7 @@ import ( "github.com/go-git/go-git/v5/plumbing/filemode" "github.com/go-git/go-git/v5/plumbing/object" + "github.com/astra-sh/qvr/internal/canonical" "github.com/astra-sh/qvr/internal/model" ) @@ -110,6 +111,93 @@ func (m *Materializer) MaterializeSubtree(repoPath, commitish, subpath string, r return hash.String(), nil } +// MaterializeFromDisk copies a skill's content from a local source directory +// into dest, mirroring MaterializeSubtree's on-disk layout and file-mode rules +// so canonical.HashSubtreeFromDisk over dest equals the same hash over srcRoot. +// It walks the filesystem (not git objects) and skips exactly what the canonical +// hasher excludes (.git/, signature wrappers — canonical.IsExcluded), so a +// plain-folder `qvr add --local` materializes through the SAME BlobMaterializer +// seam (reflink/content-store, #205) as a registry install rather than a bespoke +// recursive copy. +// +// Symlinks are recreated as links with the same escaping-target refusal as +// writeFile; regular/executable files preserve their exec bit. Empty dirs are +// not created (git doesn't track them and the disk hasher ignores them). +func (m *Materializer) MaterializeFromDisk(srcRoot, dest string) error { + info, err := os.Stat(srcRoot) + if err != nil { + return fmt.Errorf("stat local source: %w", err) + } + if !info.IsDir() { + return fmt.Errorf("local source is not a directory: %s", srcRoot) + } + return filepath.WalkDir(srcRoot, func(path string, d os.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + rel, relErr := filepath.Rel(srcRoot, path) + if relErr != nil { + return fmt.Errorf("relativize %s: %w", path, relErr) + } + rel = filepath.ToSlash(rel) + if d.IsDir() { + // Don't descend into excluded dirs (.git/ above all) — both saves + // the walk and matches the hasher, which never counts their files. + if rel != "." && canonical.IsExcluded(rel) { + return filepath.SkipDir + } + return nil + } + if canonical.IsExcluded(rel) { + return nil + } + fi, statErr := d.Info() + if statErr != nil { + return fmt.Errorf("stat %s: %w", rel, statErr) + } + return m.writeDiskEntry(path, filepath.Join(dest, filepath.FromSlash(rel)), fi) + }) +} + +// writeDiskEntry materializes one filesystem entry at destAbs, preserving the +// git-relevant mode (symlink / executable / regular) so the disk hash agrees. +// Regular files flow through the BlobMaterializer seam (reflink) when configured. +func (m *Materializer) writeDiskEntry(srcPath, destAbs string, fi os.FileInfo) error { + if err := os.MkdirAll(filepath.Dir(destAbs), 0o755); err != nil { + return fmt.Errorf("create dir for %s: %w", destAbs, err) + } + if fi.Mode()&os.ModeSymlink != 0 { + target, err := os.Readlink(srcPath) + if err != nil { + return fmt.Errorf("read symlink %s: %w", srcPath, err) + } + // Reject any target that escapes the skill dir: absolute, a bare ".." + // (cleans to ".." with no trailing slash), or anything that cleans to a + // "../"-prefixed path. + cleaned := filepath.ToSlash(filepath.Clean(target)) + if filepath.IsAbs(target) || cleaned == ".." || strings.HasPrefix(cleaned, "../") { + return fmt.Errorf("refusing symlink %s with escaping target %q", srcPath, filepath.ToSlash(target)) + } + return os.Symlink(target, destAbs) + } + mode := os.FileMode(0o644) + if fi.Mode().Perm()&0o111 != 0 { + mode = 0o755 + } + read := func() (io.ReadCloser, error) { return os.Open(srcPath) } + if m.Blob != nil { + if err := m.Blob.WriteBlob(destAbs, mode, read); err != nil { + return err + } + } else if err := copyBlobToFile(destAbs, mode, read); err != nil { + return err + } + if mode == 0o755 { + return os.Chmod(destAbs, 0o755) + } + return nil +} + // resolveCommitish turns a SHA or ref label into a commit hash. A full 40-hex // string is taken verbatim (the common, already-resolved case); anything else // is resolved through go-git's revision resolver, which handles branches, tags, diff --git a/internal/skill/reconciler.go b/internal/skill/reconciler.go index a2d4b4e..cdb947a 100644 --- a/internal/skill/reconciler.go +++ b/internal/skill/reconciler.go @@ -106,22 +106,27 @@ func (r *Reconciler) restoreFromLock(lock *model.LockFile, projectRoot string, g continue } if entry.IsEdit() { - // Edit-mode entries live at EditPath inside the project, not in - // the shared cache. The shared worktree is no longer load-bearing, - // so we don't try to (re-)install — just verify the edit dir is - // present and refresh sibling symlinks. If EditPath is missing - // after e.g. a teammate cloned the repo without the dir, - // surface a clear error rather than silently overwriting with - // shared-mode contents. - editDir := EffectiveTarget(entry, projectRoot) - if _, err := os.Stat(editDir); err != nil { - res.Errors = append(res.Errors, fmt.Sprintf("%s: edit dir %s missing — re-run `qvr edit %s` to re-materialize from %s@%s", - entry.Name, editDir, entry.Name, entry.UpstreamSource(), shortHashOrFull(entry.InstallCommit))) - continue - } - if err := r.fixSymlinks(entry, projectRoot, global, opts, res); err != nil { - res.Errors = append(res.Errors, fmt.Sprintf("%s: %v", entry.Name, err)) - } + // Edit-mode entries live at EditPath inside the project, not in the + // shared cache. The shared worktree is no longer load-bearing, so we + // don't (re-)install — just verify the dir is present and refresh + // sibling symlinks. + r.reconcileInRepoEntry(entry, projectRoot, global, opts, res, func(dir string) string { + return fmt.Sprintf("%s: edit dir %s missing — re-run `qvr edit %s` to re-materialize from %s@%s", + entry.Name, dir, entry.Name, entry.UpstreamSource(), shortHashOrFull(entry.InstallCommit)) + }) + continue + } + + if entry.IsVendor() { + // Vendored entries live as real files committed in the repo at + // VendorPath — the source of truth is the project tree, not the + // store. A teammate gets them with the clone; a missing dir means + // the checkout dropped them, so surface a clear error rather than + // silently overwriting. + r.reconcileInRepoEntry(entry, projectRoot, global, opts, res, func(dir string) string { + return fmt.Sprintf("%s: vendored dir %s missing — it is tracked by the repo; restore it from version control or re-run `qvr add --vendor`", + entry.Name, dir) + }) continue } @@ -149,6 +154,22 @@ func (r *Reconciler) restoreFromLock(lock *model.LockFile, projectRoot string, g return nil } +// reconcileInRepoEntry handles entries whose content lives as a real directory +// inside the project (edit + vendor modes): the store worktree is not load- +// bearing, so reconcile only verifies the in-repo dir exists and refreshes +// sibling symlinks — never a re-install. missingMsg builds the error when the +// dir is absent (each mode points the user at its own repair path). +func (r *Reconciler) reconcileInRepoEntry(entry *model.LockEntry, projectRoot string, global bool, opts ReconcileOptions, res *ReconcileResult, missingMsg func(dir string) string) { + dir := EffectiveTarget(entry, projectRoot) + if _, err := os.Stat(dir); err != nil { + res.Errors = append(res.Errors, missingMsg(dir)) + return + } + if err := r.fixSymlinks(entry, projectRoot, global, opts, res); err != nil { + res.Errors = append(res.Errors, fmt.Sprintf("%s: %v", entry.Name, err)) + } +} + // restoreShared restores a shared (registry) entry whose worktree is missing, // re-installing it pinned to the lock's recorded commit (the uv reproducibility // contract). Returns the entry to link (refreshed from the lock after a real @@ -245,22 +266,15 @@ func (r *Reconciler) restoreLocal(entry *model.LockEntry, projectRoot string, gl if worktreePath == "" { return fmt.Errorf("cannot derive worktree path for local copy — re-run `qvr add --local %s`", entry.Source) } - staging := fmt.Sprintf("%s.staging.%d", worktreePath, os.Getpid()) - _ = os.RemoveAll(staging) - if err := os.MkdirAll(filepath.Dir(worktreePath), 0o755); err != nil { - return fmt.Errorf("create worktrees dir: %w", err) - } - if err := copyDir(entry.Source, staging); err != nil { - _ = os.RemoveAll(staging) - return fmt.Errorf("re-copy local skill from %s: %w", entry.Source, err) - } - if err := os.Rename(staging, worktreePath); err != nil { - if _, e := os.Stat(worktreePath); e == nil { - _ = os.RemoveAll(staging) - } else { - _ = os.RemoveAll(staging) - return fmt.Errorf("finalize local worktree: %w", err) - } + if r.Installer == nil { + return fmt.Errorf("cannot re-materialize local copy %s: no installer configured", entry.Name) + } + // Re-materialize through the same seam as the initial install + // (MaterializeFromDisk → reflink), not a bespoke copy — the installer's + // staging + atomic-rename handles the missing-worktree case we just + // confirmed. + if err := r.Installer.materializeLocalCopy(entry.Source, worktreePath); err != nil { + return fmt.Errorf("re-materialize local skill from %s: %w", entry.Source, err) } setSubtreeReadOnly(worktreePath) res.Installed = append(res.Installed, entry.Name) @@ -290,11 +304,12 @@ func (r *Reconciler) fixSymlinks(entry *model.LockEntry, projectRoot string, glo return nil } } - // For edit-mode entries the canonical EditPath is itself a real directory - // that must not be replaced with a self-referential symlink. Identify the - // canonical absolute path so we can skip it in the per-target loop. + // For edit- and vendor-mode entries the canonical EditPath/VendorPath is + // itself a real directory that must not be replaced with a self-referential + // symlink. Identify the canonical absolute path so we can skip it in the + // per-target loop (siblings still get repointed at it). var canonicalAbs string - if entry.IsEdit() { + if entry.IsEdit() || entry.IsVendor() { if abs, err := filepath.Abs(target); err == nil { canonicalAbs = normalize(abs) } diff --git a/internal/skill/vendor_test.go b/internal/skill/vendor_test.go new file mode 100644 index 0000000..b8a0db3 --- /dev/null +++ b/internal/skill/vendor_test.go @@ -0,0 +1,272 @@ +package skill_test + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/astra-sh/qvr/internal/canonical" + "github.com/astra-sh/qvr/internal/model" + "github.com/astra-sh/qvr/internal/registry" + "github.com/astra-sh/qvr/internal/skill" +) + +// assertRealDirWithSkill asserts path is a real directory (not a symlink) +// holding a SKILL.md — the vendoring contract for the canonical target. +func assertRealDirWithSkill(t *testing.T, path string) { + t.Helper() + info, err := os.Lstat(path) + if err != nil { + t.Fatalf("lstat %s: %v", path, err) + } + if info.Mode()&os.ModeSymlink != 0 { + t.Fatalf("%s is a symlink; vendoring must produce a real directory", path) + } + if !info.IsDir() { + t.Fatalf("%s is not a directory", path) + } + if _, err := os.Stat(filepath.Join(path, "SKILL.md")); err != nil { + t.Errorf("vendored dir missing SKILL.md: %v", err) + } +} + +// TestInstallLocal_Vendor vendors a local folder: the canonical target becomes a +// real in-repo directory (not a store symlink), the lock entry is mode:vendor +// with VendorPath set, and the bytes are a snapshot independent of the source. +func TestInstallLocal_Vendor(t *testing.T) { + h := newHarness(t) + src := writeLocalSkill(t, "my-skill", "local dev skill") + + result, err := h.installer.InstallLocal(src, skill.InstallRequest{ + Targets: []string{"claude"}, + ProjectRoot: h.project, + Vendor: true, + }) + if err != nil { + t.Fatalf("InstallLocal --vendor: %v", err) + } + + vendorDir := filepath.Join(h.project, ".claude/skills/my-skill") + assertRealDirWithSkill(t, vendorDir) + if result.Worktree != vendorDir { + t.Errorf("result.Worktree = %s, want vendored dir %s", result.Worktree, vendorDir) + } + + lock, err := model.ReadLockFile(filepath.Join(h.project, model.LockFileName)) + if err != nil { + t.Fatalf("read lock: %v", err) + } + entry, err := lock.Get("my-skill") + if err != nil { + t.Fatalf("lock get: %v", err) + } + if !entry.IsVendor() { + t.Errorf("entry.IsVendor() = false; mode = %q", entry.Mode) + } + if entry.VendorPath != ".claude/skills/my-skill" { + t.Errorf("VendorPath = %q, want .claude/skills/my-skill", entry.VendorPath) + } + if entry.SubtreeHash == "" { + t.Error("vendored entry has empty SubtreeHash") + } + + // Snapshot, not a live link: editing the source must not change the vendored + // copy (its bytes were materialized into the repo). + if err := os.WriteFile(filepath.Join(src, "SKILL.md"), []byte("mutated"), 0o644); err != nil { + t.Fatalf("rewrite source: %v", err) + } + got, _ := os.ReadFile(filepath.Join(vendorDir, "SKILL.md")) + if string(got) == "mutated" { + t.Error("vendored copy changed when source was edited — not a snapshot") + } +} + +// TestInstall_VendorRegistry vendors a registry skill: real in-repo dir with the +// skill at its root, mode:vendor, and provenance preserved from the install. +func TestInstall_VendorRegistry(t *testing.T) { + h := newHarness(t) + remote := seedRemote(t, map[string]string{"code-review": codeReviewSkill}) + h.addRegistry(t, "acme", remote) + + if _, err := h.installer.Install(skill.InstallRequest{ + Skill: "code-review", + Targets: []string{"claude"}, + ProjectRoot: h.project, + Vendor: true, + }); err != nil { + t.Fatalf("Install --vendor: %v", err) + } + + vendorDir := filepath.Join(h.project, ".claude/skills/code-review") + assertRealDirWithSkill(t, vendorDir) + + lock, _ := model.ReadLockFile(filepath.Join(h.project, model.LockFileName)) + entry, err := lock.Get("code-review") + if err != nil { + t.Fatalf("lock get: %v", err) + } + if !entry.IsVendor() { + t.Errorf("entry.IsVendor() = false; mode = %q", entry.Mode) + } + if entry.AuthorIdentity() != "Test " { + t.Errorf("vendored entry lost provenance: commitAuthor = %q", entry.AuthorIdentity()) + } + // The re-sealed hash must match a fresh recomputation over the in-repo dir. + got, err := canonical.HashSubtreeFromDisk(vendorDir) + if err != nil { + t.Fatalf("hash vendored dir: %v", err) + } + if got != entry.SubtreeHash { + t.Errorf("SubtreeHash %q != disk hash %q", entry.SubtreeHash, got) + } +} + +// TestVendor_MultiTargetSiblingSymlinks: the alphabetical-first target is the +// real dir; other targets are RELATIVE symlinks to it (so the layout survives a +// git clone to a different absolute path). +func TestVendor_MultiTargetSiblingSymlinks(t *testing.T) { + h := newHarness(t) + src := writeLocalSkill(t, "my-skill", "local dev skill") + + if _, err := h.installer.InstallLocal(src, skill.InstallRequest{ + Targets: []string{"claude", "cursor"}, + ProjectRoot: h.project, + Vendor: true, + }); err != nil { + t.Fatalf("InstallLocal --vendor multi-target: %v", err) + } + + // claude sorts first → canonical real dir. + assertRealDirWithSkill(t, filepath.Join(h.project, ".claude/skills/my-skill")) + + // cursor (.agents/skills) → relative symlink to the canonical. + sibling := filepath.Join(h.project, ".agents/skills/my-skill") + info, err := os.Lstat(sibling) + if err != nil { + t.Fatalf("lstat sibling: %v", err) + } + if info.Mode()&os.ModeSymlink == 0 { + t.Fatalf("sibling %s should be a symlink", sibling) + } + target, _ := os.Readlink(sibling) + if filepath.IsAbs(target) { + t.Errorf("sibling symlink target %q is absolute; want relative for clone-portability", target) + } + // It must resolve to the canonical real dir. + if _, err := os.Stat(sibling); err != nil { + t.Errorf("sibling symlink does not resolve: %v", err) + } +} + +// TestVendor_TravelsWithoutStore proves the whole point of vendoring: with the +// store wiped (as on a teammate's fresh clone), the skill still reads and +// `qvr sync` reconciles without error. +func TestVendor_TravelsWithoutStore(t *testing.T) { + h := newHarness(t) + src := writeLocalSkill(t, "my-skill", "local dev skill") + + if _, err := h.installer.InstallLocal(src, skill.InstallRequest{ + Targets: []string{"claude"}, + ProjectRoot: h.project, + Vendor: true, + }); err != nil { + t.Fatalf("InstallLocal --vendor: %v", err) + } + + // Nuke the entire store — a teammate cloning the repo has none of it. + if err := os.RemoveAll(registry.WorktreesRoot()); err != nil { + t.Fatalf("wipe store: %v", err) + } + + vendorDir := filepath.Join(h.project, ".claude/skills/my-skill") + assertRealDirWithSkill(t, vendorDir) // still there — it lives in the repo + + lock, _ := model.ReadLockFile(filepath.Join(h.project, model.LockFileName)) + rec := skill.NewReconciler(h.installer) + res, err := rec.Reconcile(lock, h.project, h.home, skill.ReconcileOptions{}) + if err != nil { + t.Fatalf("reconcile: %v", err) + } + if len(res.Errors) != 0 { + t.Errorf("reconcile reported errors with store gone: %v", res.Errors) + } + assertRealDirWithSkill(t, vendorDir) +} + +// TestVendor_Remove tears a vendored skill down without --force: the in-repo dir, +// symlinks, and lock entry all go away (vendored files are reproducible). +func TestVendor_Remove(t *testing.T) { + h := newHarness(t) + src := writeLocalSkill(t, "my-skill", "local dev skill") + + if _, err := h.installer.InstallLocal(src, skill.InstallRequest{ + Targets: []string{"claude", "cursor"}, + ProjectRoot: h.project, + Vendor: true, + }); err != nil { + t.Fatalf("InstallLocal --vendor: %v", err) + } + + vendorDir := filepath.Join(h.project, ".claude/skills/my-skill") + sibling := filepath.Join(h.project, ".agents/skills/my-skill") + + if err := h.installer.Remove("my-skill", skill.InstallRequest{ProjectRoot: h.project}); err != nil { + t.Fatalf("Remove vendored (no --force): %v", err) + } + if _, err := os.Lstat(vendorDir); !os.IsNotExist(err) { + t.Errorf("vendored dir still present after remove: %v", err) + } + if _, err := os.Lstat(sibling); !os.IsNotExist(err) { + t.Errorf("sibling symlink still present after remove: %v", err) + } + lock, _ := model.ReadLockFile(filepath.Join(h.project, model.LockFileName)) + if _, err := lock.Get("my-skill"); err == nil { + t.Error("lock entry still present after remove") + } +} + +// TestEdit_RefusesVendored: a vendored skill is already editable real files, so +// `qvr edit` (EjectToTarget) refuses it rather than producing a confusing +// clobber error. +func TestEdit_RefusesVendored(t *testing.T) { + h := newHarness(t) + src := writeLocalSkill(t, "my-skill", "local dev skill") + + if _, err := h.installer.InstallLocal(src, skill.InstallRequest{ + Targets: []string{"claude"}, + ProjectRoot: h.project, + Vendor: true, + }); err != nil { + t.Fatalf("InstallLocal --vendor: %v", err) + } + + lock, _ := model.ReadLockFile(filepath.Join(h.project, model.LockFileName)) + entry, _ := lock.Get("my-skill") + if _, err := skill.EjectToTarget(skill.EjectRequest{Entry: entry, ProjectRoot: h.project}); err == nil { + t.Fatal("EjectToTarget on a vendored entry should error") + } +} + +// TestMaterializeFromDisk_RefusesEscapingSymlink guards the --local disk +// materialization escape check, including a BARE ".." target (which cleans to +// ".." with no "../" prefix and previously slipped through). +func TestMaterializeFromDisk_RefusesEscapingSymlink(t *testing.T) { + for _, target := range []string{"..", "../outside", "/etc/passwd"} { + t.Run(target, func(t *testing.T) { + src := t.TempDir() + if err := os.WriteFile(filepath.Join(src, "SKILL.md"), + []byte("---\nname: x\ndescription: y\n---\nbody\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, filepath.Join(src, "escape")); err != nil { + t.Fatal(err) + } + dest := filepath.Join(t.TempDir(), "out") + err := (&skill.Materializer{}).MaterializeFromDisk(src, dest) + if err == nil || !strings.Contains(err.Error(), "escaping target") { + t.Errorf("target %q: want escaping-target rejection, got %v", target, err) + } + }) + } +}