From dd8abfc3c9a7d571ae9ec94a7daedf6849826853 Mon Sep 17 00:00:00 2001 From: nmccready Date: Tue, 7 Jul 2026 08:25:17 -0400 Subject: [PATCH] =?UTF-8?q?feat(source):=20linked=20(editable)=20sources?= =?UTF-8?q?=20=E2=80=94=20SPEC-007=20(closes=20#69)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an npm-link / go-replace mode to the SPEC-003 source system: a linked source symlinks a managed .agents/ artifact at a live local git checkout instead of a SHA-pinned tarball snapshot, so edits flow both ways and `git pull` in the checkout reaps updates with no re-fetch. - schema: Override.Link + LockEntry.Link/ManagedClone; link⊕pin_sha and absolute-path rejection validated at manifest load - file: scheme parser (relative ok, absolute rejected) resolved against .agents/ - `source add --link= ` (link a checkout), `--link ` (managed clone under .agents/.sources/, scanned once via SPEC-005), and `--link=` (derive owner/repo from the checkout's git remote) - pull skips the fetcher and verifies/self-heals the symlink; dangling target warns instead of silently re-fetching; update git-pulls managed clones - index follows symlinks so linked skills enumerate in AGENTS.md - `source list` marks linked entries; `detach` freezes a link into a vendored snapshot (keeps upstream identity, drops the link override) - all persisted paths + on-disk symlinks are relative (portability invariant) - scan/copyTree skip nested .git; docs + SPEC-007 status → Implemented - tests: Go (parse, three add forms, dangling, detach, index) + 7 bats e2e Co-Authored-By: Claude Opus 4.8 --- README.md | 31 +- internal/agent/agent.go | 6 +- internal/agent/globalsync.go | 22 +- internal/agent/link_index_test.go | 66 ++++ internal/agent/source/bundle.go | 110 +++++- internal/agent/source/bundle_test.go | 2 +- internal/agent/source/link.go | 497 +++++++++++++++++++++++++++ internal/agent/source/link_test.go | 367 ++++++++++++++++++++ internal/agent/source/manifest.go | 65 +++- internal/agent/source/pull.go | 50 ++- internal/agent/source/scan.go | 11 +- internal/agent/sourcecmd.go | 55 ++- main.go | 23 +- specs/SPEC-007-linked-sources.md | 4 +- test/sync-agents.bats | 97 ++++++ 15 files changed, 1376 insertions(+), 30 deletions(-) create mode 100644 internal/agent/link_index_test.go create mode 100644 internal/agent/source/link.go create mode 100644 internal/agent/source/link_test.go diff --git a/README.md b/README.md index 8d7b891..66e87c8 100644 --- a/README.md +++ b/README.md @@ -109,10 +109,11 @@ AGENTS.md is also symlinked to CLAUDE.md so that Claude reads the index natively | `pull [--dry-run\|--offline\|--force\|--only NAME\|--global]` | Fetch every `sources.yaml` entry, verify integrity, install into the matching buckets | | `update [NAME]` | Re-resolve refs and re-pull entries whose upstream moved; SHA-pinned entries are skipped | | `source add ` | Append an entry to `sources.yaml` and pull it | +| `source add --link[=] []` | Declare a **linked (editable)** source — symlink a live local checkout instead of a fetched snapshot (SPEC-007) | | `source remove [--keep]` | Remove the manifest entry and delete the artifact (`--keep` converts it to manual) | -| `source list [--json]` | Show each entry's local state: `ok` / `outdated` / `modified` / `missing` | +| `source list [--json]` | Show each entry's local state: `ok` / `outdated` / `modified` / `missing` / `linked` | | `source bundle` | Rebuild `sources.yaml` from installed artifacts' origin metadata | -| `source detach ` | Un-manage an artifact: flip its origin to manual and drop the manifest entry | +| `source detach ` | Un-manage an artifact: flip its origin to manual and drop the manifest entry (for a **linked** source, freeze the live copy into a vendored snapshot) | | `quarantine` | List remotely-fetched artifacts awaiting review, with their scan findings | | `approve \|--all [--force]` | Promote a quarantined artifact into `.agents/` (`--force` accepts critical findings, recorded in the lock) | | `reject \|--all` | Delete a quarantined artifact without installing it | @@ -240,6 +241,32 @@ sync-agents reject sketchy-rule # delete without installing Critical findings block `approve`; overriding with `--force` is recorded in `sources.lock` as `approved_with_findings` so the decision is auditable. `--trust` on `pull`/`update` skips the gate for one invocation (findings still print), and `quarantine = off` in `.agents/config` disables it for teams that review via pinned SHAs in PRs instead. +## Linked (editable) sources + +When you're **actively developing** a skill that lives in another repo (or the upstream repo itself), a SHA-pinned snapshot forces a slow `edit → commit → push → update` loop. A **linked** source is the `npm link` / `go mod replace` for `.agents/` — it symlinks the artifact at a live local checkout, so edits flow both ways and `git pull` in the checkout reaps updates with no re-fetch (SPEC-007). + +```bash +# Link a checkout you already have (path relative to cwd or absolute): +sync-agents source add --link=../foo-skill skill:me/foo-skill + +# Derive the entry from the checkout's github remote + layout: +sync-agents source add --link=../foo-skill + +# Managed clone: sync-agents clones the repo under .agents/.sources/ and links out of it +# (the initial clone is scanned once through the quarantine scanner; --trust to skip): +sync-agents source add --link skill:me/foo-skill +``` + +The link is recorded declaratively as a `link:` override in `sources.yaml` and echoed in `sources.lock`, so the intent is committed and reviewable: + +```yaml +overrides: + - match: skill:me/foo-skill + link: file:../foo-skill # relative to .agents/; mutually exclusive with pin_sha +``` + +All persisted paths are **relative** (`file:` scheme) and the on-disk symlink is created relative too — an absolute path would break the instant another machine cloned the repo, so absolute `file:` paths are rejected at parse time. `pull` verifies (and self-heals) the symlink and never re-fetches a snapshot over it; a missing checkout surfaces as a warning, not a silent revert. `update` advances a **managed clone** with `git pull --ff-only` (a checkout you own is yours to drive). `source list` shows linked entries as `[linked] … link → file:../foo-skill`, and `source detach` **freezes** the live copy into an ordinary vendored snapshot (materialize the files, drop the `link` override, keep the upstream identity). Because a linked source is a local working tree you own, it is trusted by default — only the first fetch of a managed clone is scanned. + ## ADRs (Architecture Decision Records) ADRs live in `.agents/adrs/` with **status encoded by subdirectory**: `proposed/`, `accepted/`, `denied/`. `add adr ` scaffolds into `proposed/`; `sync-agents adr accept|deny|propose ` moves a record between statuses (nested grouping subdirs are preserved) and regenerates the index. diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 13e1bce..f51b3f0 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -1442,7 +1442,9 @@ func (a *App) generateAgentsMD() { skillsDir := filepath.Join(agentsDir, "skills") if entries, err := os.ReadDir(skillsDir); err == nil { for _, entry := range entries { - if !entry.IsDir() { + // Follow symlinks so a linked skill (SPEC-007) — a symlink + // to a checkout dir — is indexed like a vendored one. + if !entryIsDir(filepath.Join(skillsDir, entry.Name()), entry) { continue } name := entry.Name() @@ -1454,7 +1456,7 @@ func (a *App) generateAgentsMD() { } // Legacy flat skills for _, entry := range entries { - if entry.IsDir() { + if entryIsDir(filepath.Join(skillsDir, entry.Name()), entry) { continue } name := entry.Name() diff --git a/internal/agent/globalsync.go b/internal/agent/globalsync.go index 26b5e68..214152c 100644 --- a/internal/agent/globalsync.go +++ b/internal/agent/globalsync.go @@ -520,7 +520,10 @@ func discoverBucketDir(dir string, b Bucket, prefix string) ([]Artifact, error) func artifactFromEntry(dir string, e os.DirEntry, b Bucket, prefix string) (Artifact, bool) { abs := filepath.Join(dir, e.Name()) if b.DirPerArtifact { - if !e.IsDir() { + // A linked skill (SPEC-007) is a symlink to a checkout dir, so + // e.IsDir() is false — stat-through the entry so linked skills + // enumerate exactly like vendored ones. + if !entryIsDir(abs, e) { return Artifact{}, false } if _, err := os.Stat(filepath.Join(abs, "SKILL.md")); err != nil { @@ -528,8 +531,23 @@ func artifactFromEntry(dir string, e os.DirEntry, b Bucket, prefix string) (Arti } return Artifact{Type: b.Artifact, Name: prefix + e.Name(), SourcePath: abs}, true } - if e.IsDir() || !strings.HasSuffix(e.Name(), ".md") { + if entryIsDir(abs, e) || !strings.HasSuffix(e.Name(), ".md") { return Artifact{}, false } return Artifact{Type: b.Artifact, Name: prefix + strings.TrimSuffix(e.Name(), ".md"), SourcePath: abs}, true } + +// entryIsDir reports whether a directory entry is (or points at, via a +// symlink) a directory. os.DirEntry.IsDir() reports the symlink itself, +// not its target, so a linked skill dir would otherwise read as a +// non-directory and vanish from discovery (SPEC-007 §index). +func entryIsDir(abs string, e os.DirEntry) bool { + if e.IsDir() { + return true + } + if e.Type()&os.ModeSymlink == 0 { + return false + } + fi, err := os.Stat(abs) // follows the symlink + return err == nil && fi.IsDir() +} diff --git a/internal/agent/link_index_test.go b/internal/agent/link_index_test.go new file mode 100644 index 0000000..c95cf15 --- /dev/null +++ b/internal/agent/link_index_test.go @@ -0,0 +1,66 @@ +package agent + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +// link_index_test.go covers the SPEC-007 §index guarantee: a linked +// skill — a symlink to a live checkout dir — must enumerate in AGENTS.md +// and DiscoverArtifacts exactly like a vendored one. os.DirEntry.IsDir() +// reports the symlink, not its target, so the discovery paths have to +// stat through the link. + +func TestLinkedSkillIsIndexed(t *testing.T) { + root := t.TempDir() + agentsDir := filepath.Join(root, ".agents") + if err := os.MkdirAll(filepath.Join(agentsDir, "skills"), 0o755); err != nil { + t.Fatal(err) + } + + // Real checkout living outside the .agents tree. + checkout := filepath.Join(root, "checkout", "linked-skill") + if err := os.MkdirAll(checkout, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(checkout, "SKILL.md"), + []byte("---\nname: linked-skill\ndescription: A linked skill. Use when testing SPEC-007.\n---\n"), 0o644); err != nil { + t.Fatal(err) + } + + // Symlink the skill into the bucket (relative, as SPEC-007 wires it). + dest := filepath.Join(agentsDir, "skills", "linked-skill") + rel, err := filepath.Rel(filepath.Dir(dest), checkout) + if err != nil { + t.Fatal(err) + } + if err := os.Symlink(rel, dest); err != nil { + t.Fatal(err) + } + + // DiscoverArtifacts follows the symlink. + arts, err := DiscoverArtifacts(agentsDir) + if err != nil { + t.Fatalf("DiscoverArtifacts: %v", err) + } + found := false + for _, a := range arts { + if a.Name == "linked-skill" && a.Type == ArtifactSkill { + found = true + } + } + if !found { + t.Fatalf("linked skill not discovered: %+v", arts) + } + + // AGENTS.md enumerates it. + a := &App{ProjectRoot: root, Stdout: &bytes.Buffer{}, Stderr: &bytes.Buffer{}} + a.generateAgentsMD() + idx := readFile(t, filepath.Join(root, "AGENTS.md")) + if !strings.Contains(idx, "linked-skill") { + t.Fatalf("linked skill missing from AGENTS.md:\n%s", idx) + } +} diff --git a/internal/agent/source/bundle.go b/internal/agent/source/bundle.go index 68eef9c..cb6556d 100644 --- a/internal/agent/source/bundle.go +++ b/internal/agent/source/bundle.go @@ -4,7 +4,9 @@ import ( "context" "fmt" "os" + "path/filepath" "strings" + "time" ) // bundle.go implements the manifest-maintenance half of SPEC-003: @@ -119,20 +121,32 @@ func (p *Puller) removeOrDetachArtifact(dest string, dirArtifact bool, o Origin, return nil } -// Detach flips an artifact to source:"manual" and removes its -// manifest + lock entries. The artifact stays on disk — this is the -// documented escape hatch for "I want to keep my local edits". -func (p *Puller) Detach(name string) error { +// Detach severs a pulled artifact from live management while keeping it +// on disk. For a normal source it flips origin to source:"manual" and +// drops the manifest + lock entries. For a linked source (SPEC-007) it +// instead FREEZES the dev copy into a vendored snapshot — see +// detachLink. The returned frozen bool is true in the latter case, so +// callers can report the right outcome. +func (p *Puller) Detach(name string) (frozen bool, err error) { m, exists, err := LoadManifest(p.AgentsDir) if err != nil { - return err + return false, err } if !exists { - return fmt.Errorf("no %s found", ManifestPath(p.AgentsDir)) + return false, fmt.Errorf("no %s found", ManifestPath(p.AgentsDir)) } raw, e, err := findEntry(m, name) if err != nil { - return err + return false, err + } + + // SPEC-007: detaching a linked source is "freeze my dev copy" — + // materialize the symlink target into a real vendored artifact, + // drop the link override, and record a normal snapshot lock entry. + // The upstream identity in `sources` stays, so pull governs it as a + // snapshot from here on. + if linkRel := linkFor(e, m); linkRel != "" { + return true, p.detachLink(raw, e, linkRel, m) } if e.Type == EntryTree { @@ -140,7 +154,7 @@ func (p *Puller) Detach(name string) error { if a.origin.Owner == e.Owner && a.origin.Repo == e.Repo { a.origin.Source = SourceManual if err := WriteOriginFor(a.dest, a.dirArtifact, a.origin); err != nil { - return err + return false, err } } } @@ -150,12 +164,68 @@ func (p *Puller) Detach(name string) error { if o, oerr := ReadOriginFor(dest, dirArtifact); oerr == nil { o.Source = SourceManual if err := WriteOriginFor(dest, dirArtifact, o); err != nil { - return err + return false, err } } } m.Sources = removeString(m.Sources, raw) + if err := SaveManifest(p.AgentsDir, m); err != nil { + return false, err + } + lock, err := LoadLock(p.AgentsDir) + if err != nil { + return false, err + } + lock.Remove(raw) + return false, SaveLock(p.AgentsDir, lock) +} + +// detachLink freezes a linked source (SPEC-007): it replaces the +// symlink with a real copy of the current target contents, drops the +// link override (keeping the upstream `sources` entry), writes normal +// origin metadata, and records a snapshot lock entry hashed over the +// materialized files. After this the entry is an ordinary vendored +// artifact that pull governs by SHA. +func (p *Puller) detachLink(raw string, e Entry, linkRel string, m Manifest) error { + name, _ := applyOverrides(e, m) + dest, dirArtifact := p.artifactDest(e, name) + + checkoutRoot := filepath.Join(p.AgentsDir, filepath.FromSlash(linkRel)) + target := linkArtifactTarget(checkoutRoot, e) + if _, err := os.Stat(target); err != nil { + return fmt.Errorf("cannot detach %s: link target %s is missing (%w) — restore the checkout or `source remove` it", name, target, err) + } + + // Provenance to freeze into the snapshot: the informational HEAD the + // lock recorded at link/update time (empty if never resolved). + sha := "" + if lock, err := LoadLock(p.AgentsDir); err == nil { + if le := lock.Find(raw); le != nil { + sha = le.ResolvedSHA + } + } + + // Materialize: real copy replaces the symlink. installArtifact copies + // the target into a sibling temp first, then swaps — the checkout is + // never mutated. + if err := installArtifact(target, dest, dirArtifact); err != nil { + return err + } + contentHash, err := HashTree(dest) + if err != nil { + return err + } + fetchedAt := p.now().Format(time.RFC3339) + if err := WriteOriginFor(dest, dirArtifact, Origin{ + Owner: e.Owner, Repo: e.Repo, Path: e.Path, Ref: e.Ref, + SHA: sha, ContentHash: contentHash, FetchedAt: fetchedAt, Source: SourceManifest, + }); err != nil { + return err + } + + // Drop the link override; keep the sources identity entry. + m.Overrides = dropLinkOverride(m.Overrides, e) if err := SaveManifest(p.AgentsDir, m); err != nil { return err } @@ -163,10 +233,30 @@ func (p *Puller) Detach(name string) error { if err != nil { return err } - lock.Remove(raw) + lock.Set(LockEntry{ + Entry: raw, ResolvedSHA: sha, ContentHash: contentHash, FetchedAt: fetchedAt, + }) return SaveLock(p.AgentsDir, lock) } +// dropLinkOverride clears the link field from an entry's override, +// removing the override entirely when nothing else remains on it. +func dropLinkOverride(overrides []Override, e Entry) []Override { + out := overrides[:0] + for _, o := range overrides { + if o.Matches(e.Raw) && o.Link != "" { + o.Link = "" + // An override that was link-only is now empty — drop it so + // the manifest stays clean. + if o.Rename == "" && o.PinSHA == "" && len(o.ExcludePaths) == 0 { + continue + } + } + out = append(out, o) + } + return out +} + // BundleReport summarizes what Bundle changed. type BundleReport struct { Added []string // entries appended to sources.yaml diff --git a/internal/agent/source/bundle_test.go b/internal/agent/source/bundle_test.go index b98e682..f39f375 100644 --- a/internal/agent/source/bundle_test.go +++ b/internal/agent/source/bundle_test.go @@ -107,7 +107,7 @@ func TestDetach_FlipsToManualAndDropsEntry(t *testing.T) { } dest := filepath.Join(p.AgentsDir, "rules", "security.md") - if err := p.Detach("security"); err != nil { + if _, err := p.Detach("security"); err != nil { t.Fatalf("detach: %v", err) } if _, err := os.Stat(dest); err != nil { diff --git a/internal/agent/source/link.go b/internal/agent/source/link.go new file mode 100644 index 0000000..97a6b66 --- /dev/null +++ b/internal/agent/source/link.go @@ -0,0 +1,497 @@ +package source + +import ( + "context" + "fmt" + "os" + "os/exec" + "path" + "path/filepath" + "regexp" + "strings" + "time" +) + +// link.go implements SPEC-007: linked (editable) sources — the +// npm-link / go-replace analogue for .agents artifacts. Instead of a +// SHA-pinned tarball snapshot, a linked source symlinks the artifact in +// .agents/ at a live local git checkout, so edits flow both ways and +// `git pull` in the checkout reaps upstream updates with no re-fetch. +// +// Everything here keeps persisted paths RELATIVE (the file: scheme, +// resolved against the .agents/ directory) and creates the on-disk +// symlink relative too — an absolute path in the git-tracked +// sources.lock breaks the instant another machine clones the repo. + +// linkScheme is npm's file: convention. The literal after it is a path +// relative to the .agents/ directory (where sources.yaml lives). +const linkScheme = "file:" + +// managedSourcesDir is the dot-dir under .agents/ that holds clones +// sync-agents owns for bare `--link `. Dot-prefixed so sync and +// the AGENTS.md index skip it, same convention as .quarantine. +const managedSourcesDir = ".sources" + +// ParseLinkPath validates and normalizes a file: link value from a +// manifest override or the lock. It returns the cleaned RELATIVE path +// (scheme stripped). Absolute paths are a parse error — they do not +// round-trip through git — and the error names the fix. +func ParseLinkPath(raw string) (string, error) { + if !strings.HasPrefix(raw, linkScheme) { + return "", fmt.Errorf("link %q must use the file: scheme (e.g. file:../foo-skill)", raw) + } + rel := strings.TrimPrefix(raw, linkScheme) + if rel == "" { + return "", fmt.Errorf("link %q has an empty path", raw) + } + // Reject absolute before and after cleaning. A leading slash, or a + // Windows volume path, cannot be shared across machines. + if filepath.IsAbs(rel) || strings.HasPrefix(rel, "/") { + return "", fmt.Errorf("link path %q is absolute — use a relative file:../ path so the lock stays portable across machines", rel) + } + clean := path.Clean(rel) + if path.IsAbs(clean) { + return "", fmt.Errorf("link path %q resolves to an absolute path — use a relative file:../ path", rel) + } + return clean, nil +} + +// Now implements the injectable clock for link lock records. +func (p *Puller) workDir() (string, error) { + if p.WorkDir != "" { + return p.WorkDir, nil + } + return os.Getwd() +} + +// cloneURL returns the git URL to clone a managed source from. Tests +// override p.CloneURL to point at a local repo; production defaults to +// GitHub over HTTPS (gh/https auth is the fetcher's concern elsewhere). +func (p *Puller) cloneURL(owner, repo string) string { + if p.CloneURL != nil { + return p.CloneURL(owner, repo) + } + return fmt.Sprintf("https://github.com/%s/%s.git", owner, repo) +} + +// linkArtifactTarget returns the absolute path the symlink should point +// at: the in-repo artifact inside the checkout root. Mirrors +// locateArtifact's defaults — a path-less skill is the checkout root +// itself (SKILL.md at repo root). +func linkArtifactTarget(checkoutRoot string, e Entry) string { + if e.Path != "" { + return filepath.Join(checkoutRoot, filepath.FromSlash(e.Path)) + } + return checkoutRoot +} + +// createRelSymlink wires dest → targetAbs as a RELATIVE symlink +// (computed from dest's parent), creating parent dirs first and +// replacing any existing symlink atomically-ish (remove + create). +func createRelSymlink(dest, targetAbs string) error { + if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil { + return err + } + rel, err := filepath.Rel(filepath.Dir(dest), targetAbs) + if err != nil { + return err + } + // Remove a pre-existing symlink so os.Symlink doesn't EEXIST. A + // real dir/file is the caller's responsibility to reject first. + if fi, err := os.Lstat(dest); err == nil && fi.Mode()&os.ModeSymlink != 0 { + if err := os.Remove(dest); err != nil { + return err + } + } + return os.Symlink(rel, dest) +} + +// AddLink implements `source add --link` in its three forms (SPEC-007 +// §Behavior). Exactly one of these shapes is expected: +// +// linkPath!="" entry!="" → link an existing checkout you own +// linkPath=="" entry!="" → managed clone under .agents/.sources/ +// linkPath!="" entry=="" → derive owner/repo from the checkout's remote +// +// It writes the upstream identity to sources.yaml, a link override, the +// relative symlink, and the lock — then returns a one-entry report for +// uniform rendering. +func (p *Puller) AddLink(ctx context.Context, linkPath, entry string, opts PullOpts) (PullReport, error) { + managed := linkPath == "" && entry != "" + + // Form 3: derive the entry from the checkout's git remote + layout. + if linkPath != "" && entry == "" { + derived, err := p.deriveEntryFromCheckout(linkPath) + if err != nil { + return PullReport{}, err + } + entry = derived + } + if entry == "" { + return PullReport{}, fmt.Errorf("source add --link needs an entry (or a --link= to a checkout with a github remote)") + } + + e, err := ParseEntry(entry) + if err != nil { + return PullReport{}, err + } + if e.Type == EntryTree { + return PullReport{}, fmt.Errorf("tree: entries cannot be linked — link a specific skill/rule/workflow instead") + } + + m, _, err := LoadManifest(p.AgentsDir) + if err != nil { + return PullReport{}, err + } + + // Resolve the checkout root and the relative file: path stored in + // the override/lock, then the symlink target and destination. + var linkRel string // relative to .agents/, stored as file: + var checkoutRoot string // absolute + if managed { + linkRel = path.Join(managedSourcesDir, e.Owner+"-"+e.Repo) + checkoutRoot = filepath.Join(p.AgentsDir, filepath.FromSlash(linkRel)) + } else { + abs, err := p.absCheckout(linkPath) + if err != nil { + return PullReport{}, err + } + checkoutRoot = abs + rel, err := filepath.Rel(p.AgentsDir, abs) + if err != nil { + return PullReport{}, err + } + relSlash := filepath.ToSlash(rel) + if filepath.IsAbs(relSlash) || strings.HasPrefix(relSlash, "/") { + return PullReport{}, fmt.Errorf("checkout %q cannot be expressed relative to %s — link a checkout on the same filesystem", linkPath, p.AgentsDir) + } + linkRel = relSlash + } + + name, _ := applyOverrides(e, m) + dest, dirArtifact := p.artifactDest(e, name) + res := EntryResult{Entry: e.Raw, Name: name} + + // Refuse to clobber a real (non-symlink) artifact. + if fi, err := os.Lstat(dest); err == nil { + if fi.Mode()&os.ModeSymlink == 0 && !opts.Force { + return PullReport{}, fmt.Errorf("%s already exists as a real %s — remove it or re-run with --force to replace it with a link", + dest, artifactKind(dirArtifact)) + } + } + + // Managed clone: fetch remote code, then scan once (SPEC-005). + if managed { + if err := p.cloneManaged(ctx, e, checkoutRoot); err != nil { + return PullReport{}, err + } + target := linkArtifactTarget(checkoutRoot, e) + if _, err := os.Stat(target); err != nil { + os.RemoveAll(checkoutRoot) + return PullReport{}, fmt.Errorf("cloned %s/%s but the artifact path %q is not present in it", e.Owner, e.Repo, refArtifactPath(e)) + } + findings := ScanTree(target) + if p.Quarantine && !opts.Trust && len(findings) > 0 { + p.printFindings(name, findings) + os.RemoveAll(checkoutRoot) + return PullReport{}, fmt.Errorf("managed clone %s/%s has %d scan finding(s) — refusing to link; review the source and re-run with --trust to link anyway", e.Owner, e.Repo, len(findings)) + } + p.printFindings(name, findings) + } else { + // User checkout: trusted by default, no scan. Just confirm the + // artifact is actually there. + target := linkArtifactTarget(checkoutRoot, e) + if _, err := os.Stat(target); err != nil { + return PullReport{}, fmt.Errorf("checkout %s has no artifact at %q (expected the %s there)", checkoutRoot, refArtifactPath(e), e.Type) + } + } + + if opts.DryRun { + res.Kind = ResultWouldAdd + return PullReport{Results: []EntryResult{res}}, nil + } + + // Wire the symlink. + if err := createRelSymlink(dest, linkArtifactTarget(checkoutRoot, e)); err != nil { + return PullReport{}, err + } + + // Persist: sources entry (identity), link override, lock record. + if !containsString(m.Sources, e.Raw) { + m.Sources = append(m.Sources, e.Raw) + } + m.Overrides = upsertLinkOverride(m.Overrides, e, linkScheme+linkRel) + if err := SaveManifest(p.AgentsDir, m); err != nil { + return PullReport{}, err + } + + sha, _ := gitHEAD(checkoutRoot) // informational; empty if not a git repo + lock, err := LoadLock(p.AgentsDir) + if err != nil { + return PullReport{}, err + } + lock.Set(LockEntry{ + Entry: e.Raw, + ResolvedSHA: sha, + ContentHash: "", + Link: linkScheme + linkRel, + ManagedClone: managed, + FetchedAt: p.now().Format(time.RFC3339), + }) + if err := SaveLock(p.AgentsDir, lock); err != nil { + return PullReport{}, err + } + + res.Kind = ResultAdded + res.SHA = sha + res.Detail = "link → " + linkScheme + linkRel + if managed { + res.Detail += " (managed clone)" + } + return PullReport{Results: []EntryResult{res}}, nil +} + +// absCheckout resolves a user-supplied --link path (relative to the +// working directory, or absolute) to an absolute, symlink-cleaned path. +func (p *Puller) absCheckout(linkPath string) (string, error) { + abs := linkPath + if !filepath.IsAbs(abs) { + wd, err := p.workDir() + if err != nil { + return "", err + } + abs = filepath.Join(wd, linkPath) + } + abs = filepath.Clean(abs) + if _, err := os.Stat(abs); err != nil { + return "", fmt.Errorf("checkout path %q not found: %w", linkPath, err) + } + return abs, nil +} + +// cloneManaged clones the upstream repo into checkoutRoot (a managed +// dir under .agents/.sources/). A pre-existing clone is reused. +func (p *Puller) cloneManaged(ctx context.Context, e Entry, checkoutRoot string) error { + if fi, err := os.Stat(filepath.Join(checkoutRoot, ".git")); err == nil && fi.IsDir() { + return nil // already cloned + } + if err := os.MkdirAll(filepath.Dir(checkoutRoot), 0o755); err != nil { + return err + } + url := p.cloneURL(e.Owner, e.Repo) + args := []string{"clone", url, checkoutRoot} + if e.Ref != "" { + args = []string{"clone", "--branch", e.Ref, url, checkoutRoot} + } + cmd := exec.CommandContext(ctx, "git", args...) + out, err := cmd.CombinedOutput() + if err != nil { + os.RemoveAll(checkoutRoot) + return fmt.Errorf("git clone %s failed: %s", url, strings.TrimSpace(string(out))) + } + return nil +} + +// pullLinked handles a linked entry during pull/update: it skips the +// HTTP fetcher entirely. On pull it verifies (and self-heals) the +// symlink; on update it advances a managed clone with git pull --ff-only +// and refreshes the informational resolved_sha. It never re-fetches a +// remote snapshot — a broken dev checkout surfaces as a warning, not a +// silent revert. +func (p *Puller) pullLinked(e Entry, name, linkRel string, le *LockEntry, lock *Lock, opts PullOpts) (EntryResult, bool) { + res := EntryResult{Entry: e.Raw, Name: name} + dest, dirArtifact := p.artifactDest(e, name) + managed := le != nil && le.ManagedClone + checkoutRoot := filepath.Join(p.AgentsDir, filepath.FromSlash(linkRel)) + target := linkArtifactTarget(checkoutRoot, e) + + // Update mode: advance a managed clone. A user checkout is the + // operator's to drive — we only refresh the recorded SHA below. + if opts.UpdateMode && managed && !opts.DryRun { + if out, err := gitPullFFOnly(checkoutRoot); err != nil { + res.Kind = ResultFailed + res.Detail = fmt.Sprintf("git pull --ff-only in managed clone failed: %s", strings.TrimSpace(out)) + return res, false + } + } + + // Verify / establish the symlink. + fi, lerr := os.Lstat(dest) + created := false + switch { + case os.IsNotExist(lerr): + if opts.DryRun { + res.Kind = ResultWouldAdd + return res, false + } + if _, err := os.Stat(target); err != nil { + res.Kind = ResultFailed + res.Detail = fmt.Sprintf("linked checkout %s is missing the artifact at %q — clone/restore it, then re-run", checkoutRoot, refArtifactPath(e)) + return res, false + } + if err := createRelSymlink(dest, target); err != nil { + res.Kind = ResultFailed + res.Detail = err.Error() + return res, false + } + created = true + res.Kind = ResultAdded + case lerr != nil: + res.Kind = ResultFailed + res.Detail = lerr.Error() + return res, false + case fi.Mode()&os.ModeSymlink == 0: + res.Kind = ResultFailed + res.Detail = fmt.Sprintf("%s exists but is not a symlink — a linked source expects a symlink here; remove the real %s or `source detach` it", + dest, artifactKind(dirArtifact)) + return res, false + default: + // Symlink present: confirm it resolves; a dangling target is a + // warning, never a silent re-fetch. + if _, serr := os.Stat(dest); serr != nil { + fmt.Fprintf(p.errOut(), "[warn] linked source %s: target %s is missing (checkout moved or deleted) — leaving the dangling link, not re-fetching\n", name, target) + res.Kind = ResultCurrent + res.Detail = "dangling link — target missing" + } else { + res.Kind = ResultCurrent + } + } + + // Refresh the informational SHA (best-effort) and decide whether the + // lock needs rewriting. Plain pull is a no-op unless it had to + // establish the link or record it for the first time; update rewrites + // when HEAD advanced. + sha := "" + if s, err := gitHEAD(checkoutRoot); err == nil { + sha = s + } else if le != nil { + sha = le.ResolvedSHA + } + res.SHA = sha + + needLock := le == nil || created + if opts.UpdateMode && le != nil && sha != "" && sha != le.ResolvedSHA { + needLock = true + if res.Kind == ResultCurrent { + res.Kind = ResultUpdated + res.Detail = fmt.Sprintf("checkout advanced to %s", shortSHA(sha)) + } + } + if needLock && !opts.DryRun { + lock.Set(LockEntry{ + Entry: e.Raw, + ResolvedSHA: sha, + ContentHash: "", + Link: linkScheme + linkRel, + ManagedClone: managed, + FetchedAt: p.now().Format(time.RFC3339), + }) + } + return res, needLock +} + +// deriveEntryFromCheckout reads a checkout's git remote and layout to +// synthesize a source entry (SPEC-007 form 3). v1 infers skill: from a +// SKILL.md at the repo root; anything else asks for an explicit entry. +func (p *Puller) deriveEntryFromCheckout(linkPath string) (string, error) { + abs, err := p.absCheckout(linkPath) + if err != nil { + return "", err + } + url, err := gitRemoteURL(abs) + if err != nil { + return "", fmt.Errorf("cannot derive owner/repo from %s: %w (pass an explicit entry instead)", linkPath, err) + } + owner, repo, err := parseGitHubRemote(url) + if err != nil { + return "", fmt.Errorf("%s: %w — pass an explicit entry (e.g. skill:%s)", linkPath, err, filepath.Base(abs)) + } + if _, err := os.Stat(filepath.Join(abs, "SKILL.md")); err == nil { + return fmt.Sprintf("skill:%s/%s", owner, repo), nil + } + return "", fmt.Errorf("cannot infer artifact type for %s/%s (no SKILL.md at the checkout root) — pass an explicit entry like rule:%s/%s@main/rules/name.md", owner, repo, owner, repo) +} + +// upsertLinkOverride adds or replaces the link override for an entry. +// The match glob is anchored to the entry string with a trailing * so a +// versioned/pathful entry still matches (mirrors the spec example). +func upsertLinkOverride(overrides []Override, e Entry, link string) []Override { + match := e.Raw + for i := range overrides { + if overrides[i].Match == match || overrides[i].Match == match+"*" { + overrides[i].Link = link + overrides[i].PinSHA = "" // link and pin_sha are exclusive + return overrides + } + } + return append(overrides, Override{Match: match, Link: link}) +} + +func containsString(list []string, s string) bool { + for _, v := range list { + if v == s { + return true + } + } + return false +} + +// artifactKind labels a destination for error messages. +func artifactKind(dirArtifact bool) string { + if dirArtifact { + return "directory" + } + return "file" +} + +// refArtifactPath describes the in-repo artifact path for messages. +func refArtifactPath(e Entry) string { + if e.Path != "" { + return e.Path + } + if e.Type == EntrySkill { + return "SKILL.md (repo root)" + } + return "(repo root)" +} + +// ------------------------------------------------------------------------- +// git helpers (SPEC-007 uses git only for managed clones + provenance) +// ------------------------------------------------------------------------- + +func gitHEAD(dir string) (string, error) { + out, err := exec.Command("git", "-C", dir, "rev-parse", "HEAD").Output() + if err != nil { + return "", err + } + return strings.TrimSpace(string(out)), nil +} + +func gitRemoteURL(dir string) (string, error) { + out, err := exec.Command("git", "-C", dir, "config", "--get", "remote.origin.url").Output() + if err != nil { + return "", fmt.Errorf("no origin remote") + } + url := strings.TrimSpace(string(out)) + if url == "" { + return "", fmt.Errorf("origin remote is empty") + } + return url, nil +} + +func gitPullFFOnly(dir string) (string, error) { + out, err := exec.Command("git", "-C", dir, "pull", "--ff-only").CombinedOutput() + return string(out), err +} + +var githubRemoteRe = regexp.MustCompile(`github\.com[:/]+([^/]+)/([^/]+?)(?:\.git)?/?$`) + +// parseGitHubRemote extracts owner/repo from a github.com remote URL in +// any of git's common forms (https, ssh scp-style, ssh://). +func parseGitHubRemote(url string) (owner, repo string, err error) { + m := githubRemoteRe.FindStringSubmatch(strings.TrimSpace(url)) + if m == nil { + return "", "", fmt.Errorf("remote %q is not a github.com URL", url) + } + return m[1], m[2], nil +} diff --git a/internal/agent/source/link_test.go b/internal/agent/source/link_test.go new file mode 100644 index 0000000..add392d --- /dev/null +++ b/internal/agent/source/link_test.go @@ -0,0 +1,367 @@ +package source + +import ( + "bytes" + "context" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" +) + +// link_test.go covers SPEC-007 linked sources: file: parse rules, the +// three `source add --link` forms, pull no-op/dangling behavior, the +// index-follows-symlink guarantee, detach-freezes-to-snapshot, and the +// portability invariant (no absolute paths persisted). + +func TestParseLinkPath(t *testing.T) { + cases := []struct { + in string + want string + wantErr bool + }{ + {"file:../foo-skill", "../foo-skill", false}, + {"file:./vendor/foo-skill", "vendor/foo-skill", false}, + {"file:foo/bar", "foo/bar", false}, + {"file:../../a/b", "../../a/b", false}, + {"file:/Users/tars/foo", "", true}, // absolute rejected + {"file:/abs", "", true}, + {"file:", "", true}, // empty + {"../no-scheme", "", true}, // missing file: scheme + {"skill:me/foo", "", true}, // wrong scheme + } + for _, c := range cases { + got, err := ParseLinkPath(c.in) + if c.wantErr { + if err == nil { + t.Errorf("ParseLinkPath(%q): expected error, got %q", c.in, got) + } + continue + } + if err != nil { + t.Errorf("ParseLinkPath(%q): unexpected error: %v", c.in, err) + continue + } + if got != c.want { + t.Errorf("ParseLinkPath(%q) = %q, want %q", c.in, got, c.want) + } + } +} + +func TestLoadManifest_LinkPinExclusive(t *testing.T) { + dir := filepath.Join(t.TempDir(), ".agents") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + yaml := "version: 1\n" + + "sources:\n - skill:me/foo\n" + + "overrides:\n - match: skill:me/foo*\n link: file:../foo\n pin_sha: " + shaA + "\n" + if err := os.WriteFile(ManifestPath(dir), []byte(yaml), 0o644); err != nil { + t.Fatal(err) + } + if _, _, err := LoadManifest(dir); err == nil || !strings.Contains(err.Error(), "cannot also be SHA-pinned") { + t.Fatalf("expected mutual-exclusion error, got %v", err) + } +} + +func TestLoadManifest_LinkAbsoluteRejected(t *testing.T) { + dir := filepath.Join(t.TempDir(), ".agents") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + yaml := "version: 1\nsources:\n - skill:me/foo\noverrides:\n - match: skill:me/foo*\n link: file:/abs/foo\n" + if err := os.WriteFile(ManifestPath(dir), []byte(yaml), 0o644); err != nil { + t.Fatal(err) + } + if _, _, err := LoadManifest(dir); err == nil || !strings.Contains(err.Error(), "absolute") { + t.Fatalf("expected absolute-path error, got %v", err) + } +} + +// makeSkillCheckout creates a git repo at / with a +// SKILL.md at its root and (optionally) an origin remote, returning the +// checkout path. git is required; the test skips if it is absent. +func makeSkillCheckout(t *testing.T, parent, name, remote string) string { + t.Helper() + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not available") + } + dir := filepath.Join(parent, name) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte("---\nname: "+name+"\n---\n# "+name+"\n"), 0o644); err != nil { + t.Fatal(err) + } + runGit(t, dir, "init", "-q") + runGit(t, dir, "config", "user.email", "t@example.com") + runGit(t, dir, "config", "user.name", "t") + runGit(t, dir, "add", ".") + runGit(t, dir, "commit", "-q", "-m", "init") + if remote != "" { + runGit(t, dir, "remote", "add", "origin", remote) + } + return dir +} + +func runGit(t *testing.T, dir string, args ...string) { + t.Helper() + cmd := exec.Command("git", append([]string{"-C", dir}, args...)...) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git %s: %v\n%s", strings.Join(args, " "), err, out) + } +} + +func newLinkPuller(t *testing.T) *Puller { + t.Helper() + agentsDir := filepath.Join(t.TempDir(), "proj", ".agents") + if err := os.MkdirAll(agentsDir, 0o755); err != nil { + t.Fatal(err) + } + return &Puller{ + AgentsDir: agentsDir, + Buckets: testBuckets, + Quarantine: true, + Out: &bytes.Buffer{}, + Err: &bytes.Buffer{}, + Now: func() time.Time { return time.Date(2026, 7, 6, 0, 0, 0, 0, time.UTC) }, + } +} + +func TestAddLink_UserCheckout(t *testing.T) { + p := newLinkPuller(t) + // checkout sits as a sibling of the project root (../foo-skill from + // the project dir, ../../foo-skill from .agents). + projRoot := filepath.Dir(p.AgentsDir) + checkout := makeSkillCheckout(t, filepath.Dir(projRoot), "foo-skill", "") + + rep, err := p.AddLink(context.Background(), checkout, "skill:me/foo-skill", PullOpts{}) + if err != nil { + t.Fatalf("AddLink: %v", err) + } + if len(rep.Results) != 1 || rep.Results[0].Kind != ResultAdded { + t.Fatalf("expected one added result, got %+v", rep.Results) + } + + dest := filepath.Join(p.AgentsDir, "skills", "foo-skill") + fi, err := os.Lstat(dest) + if err != nil { + t.Fatalf("lstat dest: %v", err) + } + if fi.Mode()&os.ModeSymlink == 0 { + t.Fatal("dest is not a symlink") + } + linkVal, _ := os.Readlink(dest) + if filepath.IsAbs(linkVal) { + t.Fatalf("symlink is absolute (%q) — must be relative for portability", linkVal) + } + // Resolves to the real SKILL.md. + if _, err := os.Stat(filepath.Join(dest, "SKILL.md")); err != nil { + t.Fatalf("symlink does not resolve to SKILL.md: %v", err) + } + + // Manifest: source entry + link override. + m, _, err := LoadManifest(p.AgentsDir) + if err != nil { + t.Fatalf("LoadManifest: %v", err) + } + if !containsString(m.Sources, "skill:me/foo-skill") { + t.Errorf("sources missing entry: %+v", m.Sources) + } + if link := linkFor(Entry{Raw: "skill:me/foo-skill"}, m); link == "" { + t.Errorf("no link override recorded: %+v", m.Overrides) + } + + // Lock: link recorded, managed_clone false, no content hash. + lock, _ := LoadLock(p.AgentsDir) + le := lock.Find("skill:me/foo-skill") + if le == nil || le.Link == "" || le.ManagedClone || le.ContentHash != "" { + t.Fatalf("lock entry wrong: %+v", le) + } + + // Portability: nothing persisted contains the absolute checkout path. + assertNoAbsPaths(t, p.AgentsDir, checkout) + + // Re-pull is a no-op and does not clobber the link. + saveManifestForPull(t, p) // ensure manifest is what pull reads + rep2, err := p.Pull(context.Background(), PullOpts{}) + if err != nil { + t.Fatalf("re-pull: %v", err) + } + if k := rep2.Results[0].Kind; k != ResultCurrent { + t.Errorf("re-pull expected current, got %s", k) + } + fi2, _ := os.Lstat(dest) + if fi2.Mode()&os.ModeSymlink == 0 { + t.Fatal("re-pull clobbered the symlink") + } +} + +// saveManifestForPull is a no-op placeholder documenting that AddLink +// already persisted the manifest; Pull reads it directly. +func saveManifestForPull(t *testing.T, _ *Puller) { t.Helper() } + +func TestAddLink_DeriveFromRemote(t *testing.T) { + p := newLinkPuller(t) + projRoot := filepath.Dir(p.AgentsDir) + checkout := makeSkillCheckout(t, filepath.Dir(projRoot), "widget", "https://github.com/acme/widget.git") + + rep, err := p.AddLink(context.Background(), checkout, "", PullOpts{}) + if err != nil { + t.Fatalf("AddLink derive: %v", err) + } + if rep.Results[0].Entry != "skill:acme/widget" { + t.Fatalf("derived entry = %q, want skill:acme/widget", rep.Results[0].Entry) + } + if _, err := os.Stat(filepath.Join(p.AgentsDir, "skills", "widget", "SKILL.md")); err != nil { + t.Fatalf("linked skill not resolvable: %v", err) + } +} + +func TestAddLink_ManagedClone(t *testing.T) { + p := newLinkPuller(t) + // A local "upstream" git repo stands in for github.com/acme/widget. + upstream := makeSkillCheckout(t, t.TempDir(), "upstream", "") + p.CloneURL = func(owner, repo string) string { return "file://" + upstream } + + rep, err := p.AddLink(context.Background(), "", "skill:acme/widget", PullOpts{}) + if err != nil { + t.Fatalf("AddLink managed: %v", err) + } + if rep.Results[0].Kind != ResultAdded { + t.Fatalf("managed link result: %+v", rep.Results[0]) + } + clone := filepath.Join(p.AgentsDir, managedSourcesDir, "acme-widget") + if fi, err := os.Stat(filepath.Join(clone, ".git")); err != nil || !fi.IsDir() { + t.Fatalf("managed clone missing .git: %v", err) + } + lock, _ := LoadLock(p.AgentsDir) + le := lock.Find("skill:acme/widget") + if le == nil || !le.ManagedClone { + t.Fatalf("expected managed_clone true, got %+v", le) + } + if _, err := os.Stat(filepath.Join(p.AgentsDir, "skills", "widget", "SKILL.md")); err != nil { + t.Fatalf("managed-clone skill not resolvable: %v", err) + } + assertNoAbsPaths(t, p.AgentsDir, clone) +} + +func TestPullLinked_DanglingWarns(t *testing.T) { + p := newLinkPuller(t) + projRoot := filepath.Dir(p.AgentsDir) + checkout := makeSkillCheckout(t, filepath.Dir(projRoot), "foo-skill", "") + if _, err := p.AddLink(context.Background(), checkout, "skill:me/foo-skill", PullOpts{}); err != nil { + t.Fatal(err) + } + // Move the checkout away → the symlink dangles. + if err := os.RemoveAll(checkout); err != nil { + t.Fatal(err) + } + errBuf := &bytes.Buffer{} + p.Err = errBuf + rep, err := p.Pull(context.Background(), PullOpts{}) + if err != nil { + t.Fatalf("pull over dangling link should not error: %v", err) + } + if rep.Results[0].Kind == ResultFailed { + t.Fatalf("dangling link should not fail: %+v", rep.Results[0]) + } + if !strings.Contains(errBuf.String(), "dangling") && !strings.Contains(errBuf.String(), "missing") { + t.Errorf("expected a dangling-target warning, got %q", errBuf.String()) + } + // The symlink is left in place (not re-fetched into a real dir). + fi, _ := os.Lstat(filepath.Join(p.AgentsDir, "skills", "foo-skill")) + if fi == nil || fi.Mode()&os.ModeSymlink == 0 { + t.Fatal("dangling link was replaced instead of preserved") + } +} + +func TestDetachLink_FreezesToSnapshot(t *testing.T) { + p := newLinkPuller(t) + projRoot := filepath.Dir(p.AgentsDir) + checkout := makeSkillCheckout(t, filepath.Dir(projRoot), "foo-skill", "") + if _, err := p.AddLink(context.Background(), checkout, "skill:me/foo-skill", PullOpts{}); err != nil { + t.Fatal(err) + } + + frozen, err := p.Detach("foo-skill") + if err != nil { + t.Fatalf("Detach: %v", err) + } + if !frozen { + t.Fatal("Detach of a link should report frozen=true") + } + + dest := filepath.Join(p.AgentsDir, "skills", "foo-skill") + fi, err := os.Lstat(dest) + if err != nil { + t.Fatalf("lstat after detach: %v", err) + } + if fi.Mode()&os.ModeSymlink != 0 { + t.Fatal("detach left a symlink — expected a real vendored copy") + } + if !fi.IsDir() { + t.Fatal("detached artifact is not a directory") + } + if _, err := os.Stat(filepath.Join(dest, "SKILL.md")); err != nil { + t.Fatalf("materialized copy missing SKILL.md: %v", err) + } + // No .git leaked into the vendored copy. + if _, err := os.Stat(filepath.Join(dest, ".git")); err == nil { + t.Fatal(".git leaked into the detached vendored copy") + } + + // Origin written with a content hash, source: manifest. + o, err := ReadOriginFor(dest, true) + if err != nil { + t.Fatalf("origin after detach: %v", err) + } + if o.ContentHash == "" { + t.Error("detached origin has empty content_hash") + } + + // Manifest: link override gone, sources identity kept. + m, _, _ := LoadManifest(p.AgentsDir) + if linkFor(Entry{Raw: "skill:me/foo-skill"}, m) != "" { + t.Error("link override survived detach") + } + if !containsString(m.Sources, "skill:me/foo-skill") { + t.Error("detach dropped the sources identity entry") + } + + // Lock: snapshot entry with content hash, no link. + lock, _ := LoadLock(p.AgentsDir) + le := lock.Find("skill:me/foo-skill") + if le == nil || le.Link != "" || le.ContentHash == "" { + t.Fatalf("post-detach lock not a snapshot: %+v", le) + } +} + +// assertNoAbsPaths fails if sources.yaml, sources.lock, or the on-disk +// symlinks contain the given absolute path — the SPEC-007 portability +// invariant. +func assertNoAbsPaths(t *testing.T, agentsDir, absPath string) { + t.Helper() + for _, f := range []string{ManifestPath(agentsDir), LockPath(agentsDir)} { + data, err := os.ReadFile(f) + if err != nil { + continue + } + if strings.Contains(string(data), absPath) { + t.Errorf("%s contains absolute path %q", filepath.Base(f), absPath) + } + } + filepath.WalkDir(filepath.Join(agentsDir, "skills"), func(path string, d os.DirEntry, err error) error { + if err != nil { + return nil + } + if d.Type()&os.ModeSymlink != 0 { + if v, _ := os.Readlink(path); filepath.IsAbs(v) { + t.Errorf("symlink %s is absolute: %q", path, v) + } + } + return nil + }) +} diff --git a/internal/agent/source/manifest.go b/internal/agent/source/manifest.go index 108b465..ce4c0f8 100644 --- a/internal/agent/source/manifest.go +++ b/internal/agent/source/manifest.go @@ -36,14 +36,20 @@ type Manifest struct { } // Override is a per-entry tweak matched by glob against the entry -// string. v1 applies rename and pin_sha; exclude_paths is parsed and -// preserved but not yet applied (TODO: filter excluded paths during +// string. v1 applies rename, pin_sha, and link; exclude_paths is parsed +// and preserved but not yet applied (TODO: filter excluded paths during // staging — accepted-but-inert so manifests written for a future // version still parse today). +// +// Link redirects resolution to a live local git checkout via a +// relative file: path instead of an immutable SHA-pinned snapshot +// (SPEC-007). It is mutually exclusive with PinSHA — a link cannot also +// be SHA-pinned — and the exclusion is enforced at load time. type Override struct { Match string `yaml:"match"` Rename string `yaml:"rename,omitempty"` PinSHA string `yaml:"pin_sha,omitempty"` + Link string `yaml:"link,omitempty"` ExcludePaths []string `yaml:"exclude_paths,omitempty"` } @@ -78,8 +84,20 @@ type LockEntry struct { // ApprovedWithFindings marks an artifact promoted out of // quarantine with `approve --force` despite critical scanner // findings — an auditable record of the human override. - ApprovedWithFindings bool `yaml:"approved_with_findings,omitempty"` - FetchedAt string `yaml:"fetched_at"` + ApprovedWithFindings bool `yaml:"approved_with_findings,omitempty"` + // Link echoes the override's relative file: path for a linked + // source (SPEC-007). Present ⇒ this entry is a symlink into a live + // checkout, not a fetched snapshot. ResolvedSHA for a linked entry + // is informational (the checkout's HEAD at link/update time; not + // used for drift or integrity), and ContentHash is empty (a live + // checkout has no stable content hash). + Link string `yaml:"link,omitempty"` + // ManagedClone is true when sync-agents owns the checkout under + // .agents/.sources/ (bare `--link `); false for a user's + // own checkout linked by path. `update` only `git pull`s managed + // clones. + ManagedClone bool `yaml:"managed_clone,omitempty"` + FetchedAt string `yaml:"fetched_at"` } // Find returns the lock entry for the given manifest entry string, @@ -140,6 +158,21 @@ func LoadManifest(agentsDir string) (Manifest, bool, error) { if m.Version != 1 { return Manifest{}, true, fmt.Errorf("%s: unsupported manifest version %d (this build understands 1)", ManifestPath(agentsDir), m.Version) } + // SPEC-007: validate link overrides at load so a malformed manifest + // fails once, up front, rather than deep inside a pull. A link may + // not also be SHA-pinned, and its path must be a relative file: + // reference (absolute paths do not round-trip through git). + for _, o := range m.Overrides { + if o.Link == "" { + continue + } + if o.PinSHA != "" { + return Manifest{}, true, fmt.Errorf("%s: override %q sets both link and pin_sha — a linked source cannot also be SHA-pinned", ManifestPath(agentsDir), o.Match) + } + if _, err := ParseLinkPath(o.Link); err != nil { + return Manifest{}, true, fmt.Errorf("%s: override %q: %w", ManifestPath(agentsDir), o.Match, err) + } + } return m, true, nil } @@ -202,3 +235,27 @@ func applyOverrides(e Entry, m Manifest) (name, pinSHA string) { } return name, pinSHA } + +// linkFor returns the effective link override (relative file: path, +// with the file: scheme stripped) for an entry, or "" when the entry +// is not linked. Last matching override wins, mirroring +// applyOverrides' last-writer semantics. +func linkFor(e Entry, m Manifest) string { + link := "" + for _, o := range m.Overrides { + if !o.Matches(e.Raw) { + continue + } + if o.Link != "" { + // Already validated at load; a parse error here would have + // aborted LoadManifest, so the error branch is unreachable + // in normal flow — fall back to the raw value defensively. + if rel, err := ParseLinkPath(o.Link); err == nil { + link = rel + } else { + link = strings.TrimPrefix(o.Link, linkScheme) + } + } + } + return link +} diff --git a/internal/agent/source/pull.go b/internal/agent/source/pull.go index 2d5d7ca..44d3fbf 100644 --- a/internal/agent/source/pull.go +++ b/internal/agent/source/pull.go @@ -60,6 +60,16 @@ type Puller struct { // Now is injectable for deterministic fetched_at timestamps in // tests. Nil means time.Now. Now func() time.Time + + // CloneURL maps an entry's owner/repo to the git URL a managed + // linked source (SPEC-007) clones from. Nil defaults to + // https://github.com//.git; tests point it at a local + // repo to stay hermetic. + CloneURL func(owner, repo string) string + + // WorkDir is the directory a user-supplied --link path is resolved + // against. Empty means os.Getwd() (normal shell semantics). + WorkDir string } func (p *Puller) now() time.Time { @@ -240,6 +250,14 @@ func (p *Puller) pullOne(ctx context.Context, raw string, m Manifest, lock *Lock name, pinSHA := applyOverrides(e, m) res := EntryResult{Entry: raw, Name: name} + // SPEC-007: a linked entry resolves to a live checkout, not a + // tarball — skip the fetcher entirely and verify/advance the link. + // The manifest override is authoritative; a lingering lock link + // with no override means the user unlinked, so fall through. + if linkRel := linkFor(e, m); linkRel != "" { + return p.pullLinked(e, name, linkRel, lock.Find(raw), lock, opts) + } + // Update semantics: a SHA-pinned entry can never advance, so skip // it before any network traffic (AC-3's "no fetch" guarantee). if opts.UpdateMode && (e.PinnedSHA() || pinSHA != "") { @@ -919,6 +937,11 @@ func copyTree(srcDir, dstDir string) error { if rel == "." { return nil } + // Never vendor a nested VCS dir (a linked/managed checkout being + // frozen by `detach` carries .git — SPEC-007). + if d.IsDir() && d.Name() == ".git" { + return filepath.SkipDir + } target := filepath.Join(dstDir, rel) if d.IsDir() { return os.MkdirAll(target, 0o755) @@ -950,8 +973,12 @@ type ListItem struct { Entry string `json:"entry"` Name string `json:"name"` ResolvedSHA string `json:"resolved_sha,omitempty"` - // Status ∈ ok | outdated | modified | missing (AC-5). + // Status ∈ ok | outdated | modified | missing | linked (AC-5, + // SPEC-007 adds linked). Status string `json:"status"` + // Link is the relative file: path for a linked entry (SPEC-007), + // empty for a normal snapshot entry. + Link string `json:"link,omitempty"` } // List reports each manifest entry's local state without touching the @@ -982,12 +1009,33 @@ func (p *Puller) List() ([]ListItem, error) { if le := lock.Find(raw); le != nil { item.ResolvedSHA = le.ResolvedSHA } + if linkRel := linkFor(e, m); linkRel != "" { + item.Link = linkScheme + linkRel + item.Status = p.linkStatus(e, name) + items = append(items, item) + continue + } item.Status = p.entryStatus(e, name, item.ResolvedSHA) items = append(items, item) } return items, nil } +// linkStatus reports a linked entry's local state: "linked" when the +// symlink is present and resolves, "missing" when it is absent or +// dangling (SPEC-007). No network, same as the rest of list. +func (p *Puller) linkStatus(e Entry, name string) string { + dest, _ := p.artifactDest(e, name) + fi, err := os.Lstat(dest) + if err != nil || fi.Mode()&os.ModeSymlink == 0 { + return "missing" + } + if _, err := os.Stat(dest); err != nil { + return "missing" // dangling target + } + return "linked" +} + func (p *Puller) entryStatus(e Entry, name, lockSHA string) string { if e.Type == EntryTree { return p.treeStatus(e, lockSHA) diff --git a/internal/agent/source/scan.go b/internal/agent/source/scan.go index cbd8fbd..cfb3902 100644 --- a/internal/agent/source/scan.go +++ b/internal/agent/source/scan.go @@ -162,7 +162,16 @@ func ScanTree(root string) []Finding { return findings } filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { - if err != nil || d.IsDir() { + if err != nil { + return nil + } + if d.IsDir() { + // A linked/managed checkout (SPEC-007) carries a .git dir; + // its binary objects are not artifact content and would + // drown the scan in false "binary file" warnings. + if d.Name() == ".git" { + return filepath.SkipDir + } return nil } rel, rerr := filepath.Rel(root, path) diff --git a/internal/agent/sourcecmd.go b/internal/agent/sourcecmd.go index ce062e5..987cf2f 100644 --- a/internal/agent/sourcecmd.go +++ b/internal/agent/sourcecmd.go @@ -43,9 +43,20 @@ type SourceCmdOpts struct { // scan still runs and prints). SPEC-005 Part B. Trust bool + // Link marks `source add --link` (SPEC-007): declare a linked + // (editable) source instead of a fetched snapshot. LinkPath is the + // checkout path when the flag was given as `--link=`; an + // empty LinkPath with Link set means a managed clone. + Link bool + LinkPath string + // Fetcher overrides the GitHub fetcher — tests inject an // httptest-backed one here. Nil means production GitHub. Fetcher source.Fetcher + + // CloneURL overrides the managed-clone git URL (SPEC-007) — tests + // point it at a local repo. Nil means github.com over HTTPS. + CloneURL func(owner, repo string) string } // sourceAgentsDir resolves which .agents/ tree the command targets. @@ -77,6 +88,7 @@ func (a *App) sourcePuller(opts SourceCmdOpts) *source.Puller { Quarantine: ReadConfigQuarantine(agentsDir), Out: a.Stdout, Err: a.Stderr, + CloneURL: opts.CloneURL, } } @@ -162,8 +174,12 @@ func (a *App) CmdUpdate(name string, opts SourceCmdOpts) error { return nil } -// CmdSourceAdd implements `sync-agents source add `. +// CmdSourceAdd implements `sync-agents source add ` and, with +// --link, `source add --link[=]` (SPEC-007 linked sources). func (a *App) CmdSourceAdd(entry string, opts SourceCmdOpts) error { + if opts.Link { + return a.cmdSourceAddLink(entry, opts) + } if entry == "" { a.Error("Usage: sync-agents source add :/[@ref][/path]") return fmt.Errorf("missing entry") @@ -185,6 +201,28 @@ func (a *App) CmdSourceAdd(entry string, opts SourceCmdOpts) error { return nil } +// cmdSourceAddLink handles the --link variants of `source add`. +func (a *App) cmdSourceAddLink(entry string, opts SourceCmdOpts) error { + if opts.LinkPath == "" && entry == "" { + a.Error("Usage: sync-agents source add --link= | --link | --link=") + return fmt.Errorf("missing entry or path") + } + p := a.sourcePuller(opts) + rep, err := p.AddLink(context.Background(), opts.LinkPath, entry, source.PullOpts{ + Force: a.Force, + DryRun: a.DryRun, + Trust: opts.Trust, + }) + a.renderPullReport(rep) + a.finishSourceWrite(rep.Changed(), opts) + if err != nil { + a.Error(err.Error()) + return err + } + a.Info("Linked source wired") + return nil +} + // CmdSourceRemove implements `sync-agents source remove `. func (a *App) CmdSourceRemove(name string, opts SourceCmdOpts) error { if name == "" { @@ -226,6 +264,11 @@ func (a *App) CmdSourceList(opts SourceCmdOpts) error { return nil } for _, it := range items { + if it.Link != "" { + // Linked entries (SPEC-007) render their target, not a SHA. + fmt.Fprintf(a.Stdout, "[%s] %s link → %s\n", it.Status, it.Entry, it.Link) + continue + } sha := it.ResolvedSHA if sha == "" { sha = "(never pulled)" @@ -267,11 +310,17 @@ func (a *App) CmdSourceDetach(name string, opts SourceCmdOpts) error { return fmt.Errorf("missing name") } p := a.sourcePuller(opts) - if err := p.Detach(name); err != nil { + frozen, err := p.Detach(name) + if err != nil { a.Error(err.Error()) return err } - a.Info(fmt.Sprintf("Detached %s — artifact kept, now source: \"manual\"", name)) + a.finishSourceWrite(frozen, opts) + if frozen { + a.Info(fmt.Sprintf("Detached %s — link frozen into a vendored snapshot (still manifest-governed)", name)) + } else { + a.Info(fmt.Sprintf("Detached %s — artifact kept, now source: \"manual\"", name)) + } return nil } diff --git a/main.go b/main.go index c9e2055..d89a1d7 100644 --- a/main.go +++ b/main.go @@ -324,19 +324,38 @@ func main() { sourceCmd.PersistentFlags().BoolVar(&srcGlobal, "global", false, "Operate on the global ~/.agents/ tree") var addOffline bool + var addTrust bool + var addLink string sourceAddCmd := &cobra.Command{ Use: "add ", Short: "Declare a source entry and pull it", - Args: cobra.MaximumNArgs(1), + Long: "Declare a source entry and pull it.\n\n" + + "With --link, declare a linked (editable) source instead of a fetched\n" + + "snapshot (SPEC-007) — the artifact symlinks a live local checkout:\n" + + " source add --link= link a checkout you own\n" + + " source add --link managed clone under .agents/.sources/\n" + + " source add --link= derive the entry from the checkout's git remote", + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { entry := "" if len(args) > 0 { entry = args[0] } - return app.CmdSourceAdd(entry, agent.SourceCmdOpts{Global: srcGlobal, Offline: addOffline}) + return app.CmdSourceAdd(entry, agent.SourceCmdOpts{ + Global: srcGlobal, + Offline: addOffline, + Trust: addTrust, + Link: cmd.Flags().Changed("link"), + LinkPath: addLink, + }) }, } sourceAddCmd.Flags().BoolVar(&addOffline, "offline", false, "Serve from the local cache only; fail on cache misses") + sourceAddCmd.Flags().BoolVar(&addTrust, "trust", false, "Skip the quarantine gate on a managed-clone link (the scan still prints)") + sourceAddCmd.Flags().StringVar(&addLink, "link", "", "Declare a linked (editable) source; optional = to a local checkout") + // NoOptDefVal lets `--link` stand alone (managed clone) or take a + // value (`--link=`), disambiguating the three SPEC-007 forms. + sourceAddCmd.Flags().Lookup("link").NoOptDefVal = "" sourceCmd.AddCommand(sourceAddCmd) var removeKeep bool diff --git a/specs/SPEC-007-linked-sources.md b/specs/SPEC-007-linked-sources.md index 0ffb397..7db140e 100644 --- a/specs/SPEC-007-linked-sources.md +++ b/specs/SPEC-007-linked-sources.md @@ -1,10 +1,10 @@ --- id: SPEC-007 title: "Linked (editable) sources: npm-link / go-replace for .agents artifacts" -status: Draft +status: Implemented owner: nmccready created: 2026-07-06 -updated: 2026-07-06 +updated: 2026-07-07 related: SPEC-003, SPEC-005 --- diff --git a/test/sync-agents.bats b/test/sync-agents.bats index 2a93c7f..94ba962 100644 --- a/test/sync-agents.bats +++ b/test/sync-agents.bats @@ -1574,3 +1574,100 @@ CONF [ "$status" -ne 0 ] [[ "$output" == *"source add"* ]] } + +# -------------------------------------------------------------------------- +# SPEC-007: linked (editable) sources +# -------------------------------------------------------------------------- + +# Create a local skill checkout at $TEST_DIR/ with a SKILL.md and +# an optional git origin remote. +_make_checkout() { + local name="$1" remote="$2" + mkdir -p "$TEST_DIR/$name" + printf -- '---\nname: %s\ndescription: linked demo. Use when testing.\n---\n# %s\n' "$name" "$name" > "$TEST_DIR/$name/SKILL.md" + git -C "$TEST_DIR/$name" init --quiet + git -C "$TEST_DIR/$name" config user.email t@example.com + git -C "$TEST_DIR/$name" config user.name t + git -C "$TEST_DIR/$name" add . >/dev/null + git -C "$TEST_DIR/$name" commit --quiet -m init + [ -n "$remote" ] && git -C "$TEST_DIR/$name" remote add origin "$remote" + return 0 +} + +@test "source add --link= wires a relative symlink and indexes the skill" { + "$SCRIPT" -d "$TEST_DIR" init + _make_checkout foo-skill "" + run "$SCRIPT" -d "$TEST_DIR" source add --link="$TEST_DIR/foo-skill" skill:me/foo-skill + [ "$status" -eq 0 ] + [ -L "$TEST_DIR/.agents/skills/foo-skill" ] + # symlink target must be relative (portability invariant) + local target + target="$(readlink "$TEST_DIR/.agents/skills/foo-skill")" + [[ "$target" != /* ]] + # resolves to the real SKILL.md + [ -f "$TEST_DIR/.agents/skills/foo-skill/SKILL.md" ] + # indexed in AGENTS.md + grep -q "foo-skill" "$TEST_DIR/AGENTS.md" +} + +@test "linked source: lock and manifest contain no absolute paths" { + "$SCRIPT" -d "$TEST_DIR" init + _make_checkout foo-skill "" + "$SCRIPT" -d "$TEST_DIR" source add --link="$TEST_DIR/foo-skill" skill:me/foo-skill + run grep -F "$TEST_DIR" "$TEST_DIR/.agents/sources.lock" + [ "$status" -ne 0 ] + run grep -F "$TEST_DIR" "$TEST_DIR/.agents/sources.yaml" + [ "$status" -ne 0 ] +} + +@test "source list marks a linked entry" { + "$SCRIPT" -d "$TEST_DIR" init + _make_checkout foo-skill "" + "$SCRIPT" -d "$TEST_DIR" source add --link="$TEST_DIR/foo-skill" skill:me/foo-skill + run "$SCRIPT" -d "$TEST_DIR" source list + [ "$status" -eq 0 ] + [[ "$output" == *"linked"* ]] + [[ "$output" == *"link →"* ]] +} + +@test "re-pull over a link is a no-op and does not clobber the symlink" { + "$SCRIPT" -d "$TEST_DIR" init + _make_checkout foo-skill "" + "$SCRIPT" -d "$TEST_DIR" source add --link="$TEST_DIR/foo-skill" skill:me/foo-skill + run "$SCRIPT" -d "$TEST_DIR" pull + [ "$status" -eq 0 ] + [ -L "$TEST_DIR/.agents/skills/foo-skill" ] +} + +@test "source add --link= derives the entry from the git remote" { + "$SCRIPT" -d "$TEST_DIR" init + _make_checkout widget "https://github.com/acme/widget.git" + run "$SCRIPT" -d "$TEST_DIR" source add --link="$TEST_DIR/widget" + [ "$status" -eq 0 ] + [ -f "$TEST_DIR/.agents/skills/widget/SKILL.md" ] + grep -q "skill:acme/widget" "$TEST_DIR/.agents/sources.yaml" +} + +@test "detach freezes a link into a real vendored copy" { + "$SCRIPT" -d "$TEST_DIR" init + _make_checkout foo-skill "" + "$SCRIPT" -d "$TEST_DIR" source add --link="$TEST_DIR/foo-skill" skill:me/foo-skill + run "$SCRIPT" -d "$TEST_DIR" source detach foo-skill + [ "$status" -eq 0 ] + [ ! -L "$TEST_DIR/.agents/skills/foo-skill" ] + [ -f "$TEST_DIR/.agents/skills/foo-skill/SKILL.md" ] + [ -f "$TEST_DIR/.agents/skills/foo-skill/_origin.json" ] + # link override dropped, identity entry kept + run grep -c "link:" "$TEST_DIR/.agents/sources.yaml" + [ "$output" -eq 0 ] + grep -q "skill:me/foo-skill" "$TEST_DIR/.agents/sources.yaml" +} + +@test "source add refuses an absolute file: link in the manifest" { + "$SCRIPT" -d "$TEST_DIR" init + mkdir -p "$TEST_DIR/.agents" + printf 'version: 1\nsources:\n - skill:me/foo\noverrides:\n - match: skill:me/foo*\n link: file:/abs/foo\n' > "$TEST_DIR/.agents/sources.yaml" + run "$SCRIPT" -d "$TEST_DIR" source list + [ "$status" -ne 0 ] + [[ "$output" == *"absolute"* ]] +}