diff --git a/README.md b/README.md index 475de91cd..5b82a7d6d 100644 --- a/README.md +++ b/README.md @@ -411,12 +411,12 @@ Two examples already run at this load. The [deep-research engine](https://agentf Each of these is a real, installable agent node. With a control plane running, drop any of them into your setup with a single command — `af install ` clones the repo, isolates its dependencies, and registers the node. You're prompted once for shared secrets like `OPENROUTER_API_KEY` (stored encrypted and reused across every node), then the node's reasoners are callable over REST or with `af call`: ```bash -af install https://github.com/Agent-Field/SWE-AF # autonomous engineering team → node: swe-planner +af install https://github.com/Agent-Field/SWE-AF # autonomous engineering team → node: swe-planner af install https://github.com/Agent-Field/sec-af # security auditor → node: sec-af af install https://github.com/Agent-Field/cloudsecurity-af # cloud / IaC security scanner → node: cloudsecurity af install https://github.com/Agent-Field/pr-af # agentic code review → node: pr-af -af run swe-planner # start a node (prompts once for required secrets) +af run swe-planner # start a node (prompts once for required secrets) af call swe-planner.build --in '{"goal": "Add JWT auth", "repo_url": "https://github.com/user/my-repo"}' ``` diff --git a/control-plane/internal/cli/catalog.go b/control-plane/internal/cli/catalog.go index 36cc6c5f8..a4a97dfca 100644 --- a/control-plane/internal/cli/catalog.go +++ b/control-plane/internal/cli/catalog.go @@ -16,7 +16,8 @@ import ( // registry search lands. It is seeded from the desktop app's curated list // (desktop/src/shared/catalog.ts) — keep the two in sync when adding nodes. // `name` MUST equal the node's agentfield-package.yaml `name:` (the registry -// key after install), which is often not the repo name (SWE-AF → swe-planner). +// key after install), which is often not the repo name (SWE-AF//go → +// swe-planner). type nodeCatalogEntry struct { Name string `json:"name"` Description string `json:"description"` @@ -31,14 +32,7 @@ type nodeCatalogEntry struct { var nodeCatalog = []nodeCatalogEntry{ { Name: "swe-planner", - Description: "Autonomous software-engineering fleet: plan, code, test, and ship production-grade PRs", - Source: "https://github.com/Agent-Field/SWE-AF", - Docs: "https://github.com/Agent-Field/SWE-AF", - Language: "python", - }, - { - Name: "swe-planner-go", - Description: "Go port of the SWE fleet: same planning/execution reasoners, one static binary", + Description: "Autonomous software-engineering fleet: plan, code, test, and ship production-grade PRs — one static binary", Source: "https://github.com/Agent-Field/SWE-AF//go", Docs: "https://github.com/Agent-Field/SWE-AF", Language: "go", diff --git a/control-plane/internal/cli/catalog_test.go b/control-plane/internal/cli/catalog_test.go index 3baf112d3..9a4357ac4 100644 --- a/control-plane/internal/cli/catalog_test.go +++ b/control-plane/internal/cli/catalog_test.go @@ -3,6 +3,7 @@ package cli import ( "bytes" "encoding/json" + "strings" "testing" "github.com/stretchr/testify/require" @@ -31,6 +32,25 @@ func TestRunCatalogPrettyEndsWithInstallHint(t *testing.T) { require.Contains(t, out, "swe-planner") } +// The SWE fleet ships as exactly one catalog row, named for the product rather +// than the implementation and installed from the `//go` source selector. A +// second entry — a re-added root/Python row, or the old implementation-suffixed +// name creeping back — must fail here rather than reappear in `af catalog`. +func TestCatalogHasSingleGoSWEEntry(t *testing.T) { + var sweEntries []nodeCatalogEntry + for _, e := range nodeCatalog { + if strings.Contains(e.Source, "Agent-Field/SWE-AF") { + sweEntries = append(sweEntries, e) + } + } + + require.Len(t, sweEntries, 1, "exactly one catalog entry may install from Agent-Field/SWE-AF") + require.Equal(t, "swe-planner", sweEntries[0].Name, + "the SWE entry is named for the product, not the implementation") + require.True(t, strings.HasSuffix(sweEntries[0].Source, "//go"), + "SWE entry source must select the go subdirectory, got %q", sweEntries[0].Source) +} + func TestRunCatalogRejectsUnknownFormat(t *testing.T) { var stdout bytes.Buffer err := runCatalog(&stdout, "csv") diff --git a/control-plane/internal/packages/git.go b/control-plane/internal/packages/git.go index 915715158..cffe4576d 100644 --- a/control-plane/internal/packages/git.go +++ b/control-plane/internal/packages/git.go @@ -38,8 +38,20 @@ type GitInstaller struct { // subdirectory becomes the package root that is copied and installed. It // composes with an @ref pin on the URL, which is parsed independently. Subdir string + + // redirects counts how many superseded_by hops led here, bounding a cycle + // (A superseded by B, B superseded by A) instead of cloning forever. + redirects int + // installedName records the package name this installer actually installed. + // A superseded_by redirect needs it to hand the old package's node-scoped + // secrets to the successor, whose name it cannot know in advance. + installedName string } +// maxSupersedeRedirects bounds a superseded_by chain. Three is generous for the +// real case (one hop) and still fails fast on a manifest cycle. +const maxSupersedeRedirects = 3 + // newSpinner creates a new spinner with the given message func (gi *GitInstaller) newSpinner(message string) *Spinner { return &Spinner{ @@ -202,7 +214,15 @@ func (gi *GitInstaller) InstallFromGit(gitURL string, force bool) error { return fmt.Errorf("failed to parse package metadata: %w", err) } - // 4. Use existing installer for the rest + // 4. A superseded package installs its successor instead. This runs before + // the force check and before anything is copied, so a redirect never + // half-installs the package it is redirecting away from. + if target := strings.TrimSpace(metadata.SupersededBy); target != "" { + return gi.followSupersededBy(metadata.Name, target, force) + } + gi.installedName = metadata.Name + + // 5. Use existing installer for the rest installer := &PackageInstaller{ AgentFieldHome: gi.AgentFieldHome, Verbose: gi.Verbose, @@ -216,10 +236,21 @@ func (gi *GitInstaller) InstallFromGit(gitURL string, force bool) error { // Install using existing flow destPath := filepath.Join(gi.AgentFieldHome, "packages", metadata.Name) + // Reinstalling clears the destination before the replacement is copied, + // and long before its dependencies finish building — a missing toolchain + // is enough to fail there. Without this the user would be left with + // neither the package they had nor a working new one. Set the existing + // directory aside instead, and put it back if anything below fails. + backup, err := stashExistingPackage(destPath) + if err != nil { + return err + } + spinner = gi.newSpinner("Setting up environment") spinner.Start() if err := installer.copyPackage(packagePath, destPath); err != nil { spinner.Error("Failed to copy package") + backup.restore() return fmt.Errorf("failed to copy package: %w", err) } spinner.Success("Environment configured") @@ -228,14 +259,17 @@ func (gi *GitInstaller) InstallFromGit(gitURL string, force bool) error { spinner.Start() if err := installer.installDependencies(destPath, metadata); err != nil { spinner.Error("Failed to install dependencies") + backup.restore() return fmt.Errorf("failed to install dependencies: %w", err) } spinner.Success("Dependencies installed") // Update registry with Git source information if err := gi.updateRegistryWithGit(metadata, info, packagePath, destPath); err != nil { + backup.restore() return fmt.Errorf("failed to update registry: %w", err) } + backup.discard() fmt.Println() fmt.Println(installSummaryPanel(metadata.Name, metadata.Version, info.URL, info.Ref, destPath)) @@ -391,6 +425,181 @@ func (gi *GitInstaller) parsePackageMetadata(packagePath string) (*PackageMetada return installer.parsePackageMetadata(packagePath) } +// followSupersededBy installs the successor a manifest points at, then retires +// the superseded package when it was already installed. Order matters: the +// successor is installed FIRST, so a failure leaves the user's existing node +// exactly as it was rather than with nothing. +func (gi *GitInstaller) followSupersededBy(fromName, target string, force bool) error { + if gi.redirects >= maxSupersedeRedirects { + return fmt.Errorf( + "superseded_by chain longer than %d hops (at %q → %q) — the manifests most likely point at each other", + maxSupersedeRedirects, fromName, target) + } + + installer := &PackageInstaller{AgentFieldHome: gi.AgentFieldHome} + replacing := installer.isPackageInstalled(fromName) + + fmt.Println() + fmt.Printf("⚠️ %s has been superseded by %s\n", fromName, target) + if replacing { + fmt.Printf("⚠️ %s is currently installed and WILL BE REPLACED. The successor is installed\n", fromName) + fmt.Println(" first and node-scoped secrets are carried over; if that fails, what you") + fmt.Println(" have now is left as it is.") + } + fmt.Println(ui.Muted(" installing the successor instead")) + fmt.Println() + + successor := &GitInstaller{ + AgentFieldHome: gi.AgentFieldHome, + Verbose: gi.Verbose, + redirects: gi.redirects + 1, + } + // A successor may carry the same name as the package it retires — that is + // a node renaming itself in place, and it is the shape a rename takes when + // the old and new names are meant to converge. The warning above already + // said this replaces the current install, so carry that consent into the + // successor rather than failing the redirect with "already installed". + if err := successor.InstallFromGit(target, force || replacing); err != nil { + return fmt.Errorf("installing %s, the successor of %s: %w", target, fromName, err) + } + gi.installedName = successor.installedName + + if !replacing || successor.installedName == fromName { + return nil + } + gi.retireSuperseded(fromName, successor.installedName) + return nil +} + +// retireSuperseded removes the old package once its successor is in place. It +// never fails the install: the successor is already working, so a stubborn +// leftover is a cleanup chore, not a reason to report failure. +func (gi *GitInstaller) retireSuperseded(oldName, newName string) { + if store, err := NewSecretStore(gi.AgentFieldHome); err == nil { + migrateNodeScopedSecrets(store, oldName, newName) + } + uninstaller := &PackageUninstaller{AgentFieldHome: gi.AgentFieldHome} + if err := uninstaller.UninstallPackage(oldName); err != nil { + fmt.Printf("⚠️ Could not remove the superseded %s: %v\n", oldName, err) + fmt.Printf(" %s is installed and usable; remove the old one with: af uninstall %s\n", newName, oldName) + return + } + fmt.Printf("✓ Replaced %s with %s\n", oldName, newName) +} + +// packageBackup holds an installed package directory that a reinstall is about +// to overwrite, so it can be put back if the reinstall fails partway. The zero +// value is a valid no-op, which is what a first-time install gets. +type packageBackup struct { + original string + saved string +} + +// stashExistingPackage moves an installed package directory aside so a failed +// reinstall can restore it. A missing directory is not an error — there is +// simply nothing to protect. +func stashExistingPackage(destPath string) (*packageBackup, error) { + if _, err := os.Stat(destPath); err != nil { + if os.IsNotExist(err) { + return &packageBackup{}, nil + } + return nil, fmt.Errorf("failed to inspect %s: %w", destPath, err) + } + // Dot-prefixed and alongside the original: same filesystem, so the move is + // a rename rather than a copy, and it cannot be mistaken for a package. + dir, name := filepath.Split(strings.TrimRight(destPath, string(os.PathSeparator))) + saved := filepath.Join(dir, "."+name+".previous") + // A leftover from an interrupted run would make the rename fail. + if err := os.RemoveAll(saved); err != nil { + return nil, fmt.Errorf("failed to clear a stale backup at %s: %w", saved, err) + } + if err := os.Rename(destPath, saved); err != nil { + return nil, fmt.Errorf("failed to set the existing package aside: %w", err) + } + return &packageBackup{original: destPath, saved: saved}, nil +} + +// restore puts the stashed package back, undoing a failed reinstall. It never +// returns an error: the install is already failing, and the caller's report of +// why is more useful than a cleanup problem layered on top. A backup that +// cannot be moved back is still on disk, so say where. +func (b *packageBackup) restore() { + if b == nil || b.saved == "" { + return + } + if err := os.RemoveAll(b.original); err != nil { + fmt.Printf("⚠️ Could not clear the failed install at %s: %v\n", b.original, err) + fmt.Printf(" your previous version is still on disk at %s\n", b.saved) + return + } + if err := os.Rename(b.saved, b.original); err != nil { + fmt.Printf("⚠️ Could not restore your previous version: %v\n", err) + fmt.Printf(" it is still on disk at %s\n", b.saved) + return + } + fmt.Printf(" restored the previously installed version at %s\n", b.original) + b.saved = "" +} + +// discard drops the stashed copy once the reinstall has succeeded. +func (b *packageBackup) discard() { + if b == nil || b.saved == "" { + return + } + os.RemoveAll(b.saved) + b.saved = "" +} + +// migrateNodeScopedSecrets hands node-scoped secrets to the successor before +// the old package is uninstalled, which deletes that scope outright. Without +// this every `af secrets set KEY --node ` value is silently lost in the +// swap. Global secrets are shared and untouched. A value already set on the +// successor wins: the user set that one deliberately, and later. +func migrateNodeScopedSecrets(store *SecretStore, oldName, newName string) { + // Read the scope directly rather than via Get, which falls back to the + // global scope and would copy shared secrets into the node scope. + oldValues, err := store.load(oldName) + if err != nil || len(oldValues) == 0 { + return + } + newValues, err := store.load(newName) + if err != nil { + newValues = map[string]string{} + } + moved := 0 + for key, value := range oldValues { + if _, exists := newValues[key]; exists { + continue + } + if err := store.Set(newName, key, value); err != nil { + fmt.Printf("⚠️ Could not move secret %s to %s: %v\n", key, newName, err) + continue + } + moved++ + } + if moved > 0 { + fmt.Printf(" moved %d node-scoped secret(s) from %s to %s\n", moved, oldName, newName) + } +} + +// appendSubdirSelector rewrites "https://host/owner/repo[@ref]" into +// "https://host/owner/repo//subdir[@ref]" so a recorded source round-trips +// through ParseGitURL. It is needed whenever the subdirectory arrived by the +// --path flag (or the install API, which splits the selector off before +// calling): without it the registry records the REPO ROOT, and the next update +// installs whatever lives there instead of the package that is installed. +func appendSubdirSelector(url, subdir string) string { + subdir = strings.Trim(strings.TrimSpace(subdir), "/") + if subdir == "" { + return url + } + base, ref := url, "" + if at := strings.LastIndex(url, "@"); at > strings.LastIndex(url, "/") { + base, ref = url[:at], url[at:] + } + return base + "//" + subdir + ref +} + // updateRegistryWithGit updates the installation registry with Git source info func (gi *GitInstaller) updateRegistryWithGit(metadata *PackageMetadata, info *GitPackageInfo, sourcePath, destPath string) error { registryPath := filepath.Join(gi.AgentFieldHome, "installed.yaml") @@ -422,6 +631,13 @@ func (gi *GitInstaller) updateRegistryWithGit(metadata *PackageMetadata, info *G // any @ref and //subdir the user gave. (Appending the ref again used to // produce doubled "…@main@main" entries.) sourcePathStr := info.URL + // …except when the subdirectory came from --path (or the install API, which + // splits `//subdir` off the URL before calling). Then info.URL is the bare + // repo and the selector has to be put back, or this records a source that + // resolves to the repo root and the next update installs a different package. + if strings.TrimSpace(info.Subdir) == "" && strings.TrimSpace(gi.Subdir) != "" { + sourcePathStr = appendSubdirSelector(sourcePathStr, gi.Subdir) + } // Add/update package entry with Git information registry.Installed[metadata.Name] = InstalledPackage{ diff --git a/control-plane/internal/packages/git_supersede_test.go b/control-plane/internal/packages/git_supersede_test.go new file mode 100644 index 000000000..07e56ccc8 --- /dev/null +++ b/control-plane/internal/packages/git_supersede_test.go @@ -0,0 +1,432 @@ +package packages + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// These cover the shape a real repo uses to retire a node: the root manifest +// declares itself superseded by a package in a subdirectory of the same repo, +// so `af install ` lands on the successor. + +const supersededRoot = "name: dual-node\nversion: 1.0.0\n" + + "superseded_by: https://gitlab.com/acme/dual//go\n" + +func seedInstalled(t *testing.T, home, name string) string { + t.Helper() + pkgDir := filepath.Join(home, "packages", name) + if err := os.MkdirAll(pkgDir, 0o755); err != nil { + t.Fatal(err) + } + pu := &PackageUninstaller{AgentFieldHome: home} + registry, err := pu.loadRegistry() + if err != nil { + t.Fatal(err) + } + registry.Installed[name] = InstalledPackage{Name: name, Path: pkgDir, Status: "stopped"} + if err := pu.saveRegistry(registry); err != nil { + t.Fatal(err) + } + return pkgDir +} + +// Contract: installing a superseded package installs its successor instead, +// and the superseded name never reaches the registry. +func TestInstallFromGit_SupersededRedirectsToSuccessor(t *testing.T) { + home := t.TempDir() + repo := filepath.Join(t.TempDir(), "repo") + writeTestPackage(t, repo, supersededRoot) + writeSubdirManifest(t, filepath.Join(repo, "go"), "dual-node-go") + setupFakeGit(t, "copy", repo, false) + + if err := (&GitInstaller{AgentFieldHome: home}). + InstallFromGit("https://gitlab.com/acme/dual", false); err != nil { + t.Fatalf("InstallFromGit: %v", err) + } + + registry := readRegistryFile(t, filepath.Join(home, "installed.yaml")) + if _, ok := registry.Installed["dual-node-go"]; !ok { + t.Fatalf("successor missing from registry, got %v", registry.Installed) + } + if _, ok := registry.Installed["dual-node"]; ok { + t.Fatal("the superseded package must not be installed") + } + if _, err := os.Stat(filepath.Join(home, "packages", "dual-node-go", "agentfield-package.yaml")); err != nil { + t.Fatalf("successor not on disk: %v", err) + } +} + +// Contract: when the superseded package is already installed it is replaced — +// the successor lands first, then the old package is stopped and removed. +func TestInstallFromGit_SupersededReplacesExistingInstall(t *testing.T) { + home := t.TempDir() + repo := filepath.Join(t.TempDir(), "repo") + writeTestPackage(t, repo, supersededRoot) + writeSubdirManifest(t, filepath.Join(repo, "go"), "dual-node-go") + setupFakeGit(t, "copy", repo, false) + + oldDir := seedInstalled(t, home, "dual-node") + + if err := (&GitInstaller{AgentFieldHome: home}). + InstallFromGit("https://gitlab.com/acme/dual", false); err != nil { + t.Fatalf("InstallFromGit: %v", err) + } + + registry := readRegistryFile(t, filepath.Join(home, "installed.yaml")) + if _, ok := registry.Installed["dual-node-go"]; !ok { + t.Fatalf("successor missing, got %v", registry.Installed) + } + if _, ok := registry.Installed["dual-node"]; ok { + t.Fatal("superseded package should have been retired from the registry") + } + if _, err := os.Stat(oldDir); !os.IsNotExist(err) { + t.Fatalf("superseded package dir should be gone, stat err = %v", err) + } +} + +// Contract: node-scoped secrets follow the user across the swap, because +// uninstalling the old package deletes that scope outright. A value already +// set on the successor wins, and global secrets are untouched. +func TestInstallFromGit_SupersededMigratesNodeScopedSecrets(t *testing.T) { + home := t.TempDir() + repo := filepath.Join(t.TempDir(), "repo") + writeTestPackage(t, repo, supersededRoot) + writeSubdirManifest(t, filepath.Join(repo, "go"), "dual-node-go") + setupFakeGit(t, "copy", repo, false) + + seedInstalled(t, home, "dual-node") + store, err := NewSecretStore(home) + if err != nil { + t.Fatal(err) + } + if err := store.Set("dual-node", "CARRIED", "from-old"); err != nil { + t.Fatal(err) + } + if err := store.Set("dual-node", "KEPT", "old-value"); err != nil { + t.Fatal(err) + } + if err := store.Set("dual-node-go", "KEPT", "new-value"); err != nil { + t.Fatal(err) + } + if err := store.Set("global", "SHARED", "shared-value"); err != nil { + t.Fatal(err) + } + + if err := (&GitInstaller{AgentFieldHome: home}). + InstallFromGit("https://gitlab.com/acme/dual", false); err != nil { + t.Fatalf("InstallFromGit: %v", err) + } + + after, err := NewSecretStore(home) + if err != nil { + t.Fatal(err) + } + values, err := after.load("dual-node-go") + if err != nil { + t.Fatal(err) + } + if values["CARRIED"] != "from-old" { + t.Fatalf("secret did not follow the swap: %v", values) + } + if values["KEPT"] != "new-value" { + t.Fatalf("successor's own value must win, got %q", values["KEPT"]) + } + globals, err := after.load("global") + if err != nil { + t.Fatal(err) + } + if globals["SHARED"] != "shared-value" { + t.Fatal("global secrets must survive the swap") + } +} + +// Contract: with nothing to replace, the redirect is a plain install — no +// error, and no attempt to retire a package that was never there. +func TestInstallFromGit_SupersededWithoutPriorInstall(t *testing.T) { + home := t.TempDir() + repo := filepath.Join(t.TempDir(), "repo") + writeTestPackage(t, repo, supersededRoot) + writeSubdirManifest(t, filepath.Join(repo, "go"), "dual-node-go") + setupFakeGit(t, "copy", repo, false) + + if err := (&GitInstaller{AgentFieldHome: home}). + InstallFromGit("https://gitlab.com/acme/dual", false); err != nil { + t.Fatalf("InstallFromGit: %v", err) + } + registry := readRegistryFile(t, filepath.Join(home, "installed.yaml")) + if len(registry.Installed) != 1 { + t.Fatalf("expected exactly the successor installed, got %v", registry.Installed) + } +} + +// Contract: two manifests pointing at each other fail loudly instead of +// redirecting forever. +func TestInstallFromGit_SupersededCycleIsBounded(t *testing.T) { + home := t.TempDir() + repo := filepath.Join(t.TempDir(), "repo") + writeTestPackage(t, repo, supersededRoot) + // The successor points straight back at the root: A → B → A → … + if err := os.MkdirAll(filepath.Join(repo, "go"), 0o755); err != nil { + t.Fatal(err) + } + manifest := "name: dual-node-go\nversion: 1.0.0\n" + + "entrypoint:\n start: python -m dual-node-go\n" + + "superseded_by: https://gitlab.com/acme/dual\n" + if err := os.WriteFile( + filepath.Join(repo, "go", "agentfield-package.yaml"), []byte(manifest), 0o644, + ); err != nil { + t.Fatal(err) + } + setupFakeGit(t, "copy", repo, false) + + err := (&GitInstaller{AgentFieldHome: home}). + InstallFromGit("https://gitlab.com/acme/dual", false) + if err == nil || !strings.Contains(err.Error(), "superseded_by chain longer than") { + t.Fatalf("expected a bounded-chain error, got %v", err) + } + // Nothing was installed, so the registry was never even created. + if _, statErr := os.Stat(filepath.Join(home, "installed.yaml")); !os.IsNotExist(statErr) { + registry := readRegistryFile(t, filepath.Join(home, "installed.yaml")) + if len(registry.Installed) != 0 { + t.Fatalf("a cycle must install nothing, got %v", registry.Installed) + } + } +} + +// writeMarkedSubdirPackage writes a subdirectory package that shares the root +// manifest's name — the shape a node takes when it renames itself in place — +// carrying a marker file so a test can tell whose files ended up installed. +func writeMarkedSubdirPackage(t *testing.T, dir, name, marker string) { + t.Helper() + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + manifest := "name: " + name + "\nversion: 2.0.0\nentrypoint:\n start: bin/" + name + "\n" + if err := os.WriteFile(filepath.Join(dir, "agentfield-package.yaml"), []byte(manifest), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, marker), []byte("successor\n"), 0o644); err != nil { + t.Fatal(err) + } +} + +// Contract: a successor that carries the SAME name as the package it retires +// replaces it in place. The redirect already warned the user, so it must not +// fail with "already installed (use --force)" — the case a node hits when it +// renames itself to the name its predecessor held. +func TestInstallFromGit_SupersededSameNameReplacesInPlace(t *testing.T) { + home := t.TempDir() + repo := filepath.Join(t.TempDir(), "repo") + writeTestPackage(t, repo, supersededRoot) + writeMarkedSubdirPackage(t, filepath.Join(repo, "go"), "dual-node", "successor.txt") + setupFakeGit(t, "copy", repo, false) + + oldDir := seedInstalled(t, home, "dual-node") + if err := os.WriteFile(filepath.Join(oldDir, "predecessor.txt"), []byte("old\n"), 0o644); err != nil { + t.Fatal(err) + } + + if err := (&GitInstaller{AgentFieldHome: home}). + InstallFromGit("https://gitlab.com/acme/dual", false); err != nil { + t.Fatalf("same-name supersede must not need --force: %v", err) + } + + registry := readRegistryFile(t, filepath.Join(home, "installed.yaml")) + pkg, ok := registry.Installed["dual-node"] + if !ok { + t.Fatalf("the shared name must still be installed, got %v", registry.Installed) + } + if pkg.Version != "2.0.0" { + t.Fatalf("registry still describes the predecessor: version %q", pkg.Version) + } + if _, err := os.Stat(filepath.Join(oldDir, "successor.txt")); err != nil { + t.Fatalf("successor's files are not installed: %v", err) + } + if _, err := os.Stat(filepath.Join(oldDir, "predecessor.txt")); !os.IsNotExist(err) { + t.Fatalf("predecessor's files must not survive the replace, stat err = %v", err) + } +} + +// Contract: node-scoped secrets survive a same-name replace. They never move — +// the scope name is unchanged — so the risk is the retire path deleting them. +func TestInstallFromGit_SupersededSameNameKeepsNodeScopedSecrets(t *testing.T) { + home := t.TempDir() + repo := filepath.Join(t.TempDir(), "repo") + writeTestPackage(t, repo, supersededRoot) + writeMarkedSubdirPackage(t, filepath.Join(repo, "go"), "dual-node", "successor.txt") + setupFakeGit(t, "copy", repo, false) + + seedInstalled(t, home, "dual-node") + store, err := NewSecretStore(home) + if err != nil { + t.Fatal(err) + } + if err := store.Set("dual-node", "KEPT", "node-value"); err != nil { + t.Fatal(err) + } + + if err := (&GitInstaller{AgentFieldHome: home}). + InstallFromGit("https://gitlab.com/acme/dual", false); err != nil { + t.Fatalf("InstallFromGit: %v", err) + } + + after, err := NewSecretStore(home) + if err != nil { + t.Fatal(err) + } + values, err := after.load("dual-node") + if err != nil { + t.Fatal(err) + } + if values["KEPT"] != "node-value" { + t.Fatalf("node-scoped secret lost in an in-place replace: %v", values) + } +} + +// Contract: an install that fails after the destination has been cleared puts +// the previously installed package back. Without this a replace that dies in +// the dependency step — a missing toolchain is enough — leaves the user with +// neither their old node nor a working new one. +func TestInstallFromGit_FailedReinstallRestoresPreviousPackage(t *testing.T) { + home := t.TempDir() + repo := filepath.Join(t.TempDir(), "repo") + // language: typescript with no package.json fails in installDependencies, + // which runs after the destination has already been cleared and copied. + writeTestPackage(t, repo, "name: solo-node\nversion: 2.0.0\nlanguage: typescript\n") + setupFakeGit(t, "copy", repo, false) + + oldDir := seedInstalled(t, home, "solo-node") + if err := os.WriteFile(filepath.Join(oldDir, "predecessor.txt"), []byte("old\n"), 0o644); err != nil { + t.Fatal(err) + } + + err := (&GitInstaller{AgentFieldHome: home}). + InstallFromGit("https://gitlab.com/acme/solo", true) + if err == nil { + t.Fatal("expected the install to fail in the dependency step") + } + + if _, statErr := os.Stat(filepath.Join(oldDir, "predecessor.txt")); statErr != nil { + t.Fatalf("previously installed package was not restored: %v", statErr) + } + entries, readErr := os.ReadDir(filepath.Join(home, "packages")) + if readErr != nil { + t.Fatal(readErr) + } + for _, e := range entries { + if strings.HasPrefix(e.Name(), ".") { + t.Fatalf("a backup was left behind in packages/: %s", e.Name()) + } + } +} + +// Contract: stashing is a no-op when there is nothing installed yet, and a +// discarded stash leaves no residue — the first-install path must not be +// burdened with cleanup that does not apply to it. +func TestStashExistingPackage(t *testing.T) { + home := t.TempDir() + missing := filepath.Join(home, "packages", "never-installed") + backup, err := stashExistingPackage(missing) + if err != nil { + t.Fatalf("stashing a missing package must succeed: %v", err) + } + backup.restore() // must not recreate anything + if _, err := os.Stat(missing); !os.IsNotExist(err) { + t.Fatalf("restoring a no-op stash created something, stat err = %v", err) + } + + present := filepath.Join(home, "packages", "installed") + if err := os.MkdirAll(present, 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(present, "keep.txt"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + + backup, err = stashExistingPackage(present) + if err != nil { + t.Fatal(err) + } + if _, err := os.Stat(present); !os.IsNotExist(err) { + t.Fatalf("stashing must move the directory aside, stat err = %v", err) + } + backup.restore() + if _, err := os.Stat(filepath.Join(present, "keep.txt")); err != nil { + t.Fatalf("restore did not put the package back: %v", err) + } + + backup, err = stashExistingPackage(present) + if err != nil { + t.Fatal(err) + } + backup.discard() + entries, err := os.ReadDir(filepath.Join(home, "packages")) + if err != nil { + t.Fatal(err) + } + if len(entries) != 0 { + t.Fatalf("discard left residue in packages/: %v", entries) + } +} + +// Contract: a source recorded for a --path install round-trips through +// ParseGitURL back to the same repo AND subdir, so the next update resolves +// the package that is actually installed rather than the repo root. +func TestAppendSubdirSelectorRoundTrips(t *testing.T) { + cases := []struct { + url, subdir, want, wantRef string + }{ + {"https://github.com/acme/repo", "go", "https://github.com/acme/repo//go", ""}, + {"https://github.com/acme/repo@main", "go", "https://github.com/acme/repo//go@main", "main"}, + {"https://github.com/acme/repo", "nested/dir", "https://github.com/acme/repo//nested/dir", ""}, + {"https://github.com/acme/repo", "", "https://github.com/acme/repo", ""}, + } + for _, c := range cases { + got := appendSubdirSelector(c.url, c.subdir) + if got != c.want { + t.Errorf("appendSubdirSelector(%q, %q) = %q, want %q", c.url, c.subdir, got, c.want) + continue + } + info, err := ParseGitURL(got) + if err != nil { + t.Errorf("ParseGitURL(%q): %v", got, err) + continue + } + wantSubdir := strings.Trim(c.subdir, "/") + if info.Subdir != wantSubdir || info.Ref != c.wantRef { + t.Errorf("round-trip of %q = subdir %q ref %q, want %q %q", + got, info.Subdir, info.Ref, wantSubdir, c.wantRef) + } + } +} + +// Contract: the registry records the subdirectory even when it arrived by the +// --path flag rather than the URL selector. Without this the stored source +// resolves to the repo root and the next update installs a different package. +func TestInstallFromGit_PathFlagRecordsSubdirInSource(t *testing.T) { + home := t.TempDir() + repo := filepath.Join(t.TempDir(), "repo") + writeTestPackage(t, repo, "name: dual-node\nversion: 1.0.0\n") + writeSubdirManifest(t, filepath.Join(repo, "go"), "dual-node-go") + setupFakeGit(t, "copy", repo, false) + + gi := &GitInstaller{AgentFieldHome: home, Subdir: "go"} + if err := gi.InstallFromGit("https://gitlab.com/acme/dual", false); err != nil { + t.Fatalf("InstallFromGit: %v", err) + } + + registry := readRegistryFile(t, filepath.Join(home, "installed.yaml")) + pkg, ok := registry.Installed["dual-node-go"] + if !ok { + t.Fatalf("expected dual-node-go installed, got %v", registry.Installed) + } + if pkg.SourcePath != "https://gitlab.com/acme/dual//go" { + t.Fatalf("source path = %q, want the //go selector recorded", pkg.SourcePath) + } + info, err := ParseGitURL(pkg.SourcePath) + if err != nil || info.Subdir != "go" { + t.Fatalf("recorded source must resolve back to the subdir: %v / %+v", err, info) + } +} diff --git a/control-plane/internal/packages/installer.go b/control-plane/internal/packages/installer.go index 5dde0013a..f5d296f76 100644 --- a/control-plane/internal/packages/installer.go +++ b/control-plane/internal/packages/installer.go @@ -115,7 +115,17 @@ type PackageMetadata struct { // at parse time by detection (a go.mod at the package root => "go", otherwise // "python"), so existing Python manifests keep working with no new field. This // is an *additive* optional key: it does NOT bump config_version. - Language string `yaml:"language"` + Language string `yaml:"language"` + // SupersededBy retires this package in favour of another one, named by an + // installable source (any string `af install` accepts, including a `//subdir` + // selector and an @ref). Installing a superseded package installs the + // successor instead, and replaces the old one when it is already present. + // + // This is how a node author renames or replaces their own node without the + // control plane knowing anything about them: the redirect lives in the + // package's manifest, not in a table here. Absent means "not superseded", + // so this is an *additive* optional key: it does NOT bump config_version. + SupersededBy string `yaml:"superseded_by"` Main string `yaml:"main"` Entrypoint EntrypointConfig `yaml:"entrypoint"` AgentNode AgentNodeConfig `yaml:"agent_node"` diff --git a/desktop/src/main/agentfield.test.ts b/desktop/src/main/agentfield.test.ts index a1248360e..dace3b2b2 100644 --- a/desktop/src/main/agentfield.test.ts +++ b/desktop/src/main/agentfield.test.ts @@ -562,6 +562,17 @@ describe('install catalog', () => { expect(catalogEntry(CATALOG[0].name)).toEqual(CATALOG[0]) expect(catalogEntry('definitely-not-real')).toBeUndefined() }) + + // The SWE fleet is offered as a single install, named for the product and + // sourced from the `//go` subdirectory. A second SWE row — or the old + // implementation-suffixed name creeping back in — must fail here rather than + // quietly reappear in the Install view. + it('offers the SWE fleet as one product-named entry sourced from //go', () => { + const sweEntries = CATALOG.filter((e) => e.source.includes('Agent-Field/SWE-AF')) + expect(sweEntries).toHaveLength(1) + expect(sweEntries[0].name).toBe('swe-planner') + expect(sweEntries[0].source.endsWith('//go')).toBe(true) + }) }) describe('installCommand', () => { diff --git a/desktop/src/shared/catalog.ts b/desktop/src/shared/catalog.ts index 8badbe75c..72cf7862d 100644 --- a/desktop/src/shared/catalog.ts +++ b/desktop/src/shared/catalog.ts @@ -14,17 +14,10 @@ import type { CatalogEntry } from './types' // ports living beside their Python originals are installed). When adding an // entry, `name` MUST equal the manifest's `name:` (the registry key after // install — how the app detects installed state), which is often NOT the -// repo name (SWE-AF → swe-planner). +// repo name (SWE-AF//go → swe-planner). export const CATALOG: CatalogEntry[] = [ { name: 'swe-planner', - description: - 'Software factory — turn any issue into a production-ready pull request, end to end', - source: 'https://github.com/Agent-Field/SWE-AF', - language: 'python' - }, - { - name: 'swe-planner-go', description: 'Software factory — turn any issue into a production-ready pull request, end to end', source: 'https://github.com/Agent-Field/SWE-AF//go', diff --git a/docs/installing-agent-nodes.md b/docs/installing-agent-nodes.md index f15d4ed18..68750356f 100644 --- a/docs/installing-agent-nodes.md +++ b/docs/installing-agent-nodes.md @@ -223,8 +223,9 @@ dependencies first (in dependency order) before the node itself. By default `af install ` looks for the `agentfield-package.yaml` at the root of the source (a git repo or a local directory). When a single repository ships -more than one installable node — for example a Python node at the root and a Go -port under `go/` — use `--path` to select the subdirectory to install: +more than one installable node — for example SWE-AF, whose advertised install is +the Go node under `go/`, alongside a Python node that also lives at the repo +root — use `--path` to select the subdirectory to install: ```bash # Install the node whose manifest lives at go/agentfield-package.yaml @@ -239,12 +240,48 @@ af install ./SWE-AF --path go The subdirectory must contain its own `agentfield-package.yaml`; that subtree becomes the package root that is copied to `~/.agentfield/packages/` and -installed (a Go node builds relative to it). Because registry entries are keyed by -the manifest `name`, the root node and a `--path` node from the same repo coexist -as separate installs. `--path` is a path **relative to the source root**: absolute -paths and paths that escape the root with `..` are rejected, and a missing manifest -is reported with the full expected path. A bare `af install ` (no `--path`) is -unchanged — the root manifest is always what you get by default. +installed (a Go node builds relative to it). Registry entries are keyed by the +manifest `name`, so a root node and a `--path` node from the same repo coexist as +separate installs when their names differ — and replace one another when they do +not. `--path` is a path **relative to the source root**: absolute paths and paths +that escape the root with `..` are rejected, and a missing manifest is reported +with the full expected path. A bare `af install ` (no `--path`) is unchanged +— the root manifest is always what you get by default, unless it redirects. + +## Retiring or renaming a node: `superseded_by` + +A manifest can declare that it has been replaced by another package. Installing +it then installs that other package instead: + +```yaml +name: my-node +superseded_by: https://github.com/me/my-repo//v2 +``` + +The value is any source `af install` accepts — including a `//subdir` selector +and an `@ref`. This lets you move, rename, or reimplement your node without +anyone having to learn a new install command, and without AgentField holding a +list of who redirects where: the redirect lives in your manifest. + +What happens on install: + +- The redirect is resolved **before** anything is copied, so a redirected + install never leaves the superseded package half-installed. +- If the superseded package is currently installed, the user is warned that it + will be replaced, and the successor is installed **first** — a failure leaves + what they had exactly as it was. +- Node-scoped secrets follow: when the successor takes a different name they are + copied across before the old package is uninstalled (which would delete that + scope); values already set on the successor win. When the successor takes the + *same* name — a node renaming itself in place — they are already in the right + scope and stay put. +- Retiring the old package never fails the install. If it cannot be removed you + are told how to remove it by hand. +- Chains are bounded at three hops, so two manifests pointing at each other fail + with a clear error instead of looping. + +The redirect applies to git installs. A local-path install ignores it, which is +how you install a superseded package deliberately. ## Previewing requirements before installing: `af show-requirements`