From e3fe10a1b99a444a4a57c38271e55ceec8828fb3 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 26 Jun 2026 10:57:33 -0400 Subject: [PATCH 1/7] feat(cli): encrypted secret store + manifest entrypoint/env resolution for agent nodes Adds the foundation for making 'af install'/'af run' usable for real agent nodes (which start via 'python -m pkg.app' and have no top-level main.py): - internal/packages/secrets.go: encrypted at-rest secret store. KeyfileProvider keeps a random 32-byte key at ~/.agentfield/keyring/master.key (0600); SecretStore encrypts global.enc + .enc via AES-256-GCM, with node scope overriding global so shared keys (API tokens) are entered once. - internal/packages/env_resolver.go: resolves declared env vars in order process-env -> node store -> global store -> manifest default -> prompt (hidden for type:secret), persisting prompted secrets encrypted. Injected only into the child process; never written to disk in plaintext. - installer.go: manifest gains entrypoint{start,healthcheck}, dependencies.nodes, and per-var scope. Validation accepts entrypoint.start instead of requiring main.py; package copy excludes .git/venv/.env/__pycache__. - runner.go: launches via manifest entrypoint, exports AGENTFIELD_SERVER (the var the SDK actually reads) alongside legacy AGENTFIELD_SERVER_URL, honors the manifest healthcheck path, and resolves env via the secret store. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../internal/packages/env_resolver.go | 165 +++++++++++ .../internal/packages/env_resolver_test.go | 181 ++++++++++++ .../internal/packages/github_coverage_test.go | 3 +- control-plane/internal/packages/installer.go | 108 +++++++- .../packages/installer_additional_test.go | 8 +- .../packages/installer_coverage_test.go | 15 +- .../internal/packages/installer_test.go | 33 ++- control-plane/internal/packages/runner.go | 138 +++++----- .../internal/packages/runner_coverage_test.go | 36 +-- .../internal/packages/runner_test.go | 30 +- control-plane/internal/packages/secrets.go | 259 ++++++++++++++++++ .../internal/packages/secrets_test.go | 193 +++++++++++++ 12 files changed, 1034 insertions(+), 135 deletions(-) create mode 100644 control-plane/internal/packages/env_resolver.go create mode 100644 control-plane/internal/packages/env_resolver_test.go create mode 100644 control-plane/internal/packages/secrets.go create mode 100644 control-plane/internal/packages/secrets_test.go diff --git a/control-plane/internal/packages/env_resolver.go b/control-plane/internal/packages/env_resolver.go new file mode 100644 index 000000000..31631c8f4 --- /dev/null +++ b/control-plane/internal/packages/env_resolver.go @@ -0,0 +1,165 @@ +package packages + +import ( + "bufio" + "fmt" + "os" + "regexp" + "strings" + + "golang.org/x/term" +) + +// Prompter asks the user to supply a value for a missing required variable. +// It is an interface so tests can supply values without a TTY. +type Prompter interface { + // Interactive reports whether prompting is possible (a real terminal). + Interactive() bool + // Prompt requests a value for the given variable. Secret types should be + // read without echo. + Prompt(v UserEnvironmentVar) (string, error) +} + +// EnvResolver resolves an agent node's environment variables, prompting for and +// persisting missing required secrets via the SecretStore. +type EnvResolver struct { + Store *SecretStore + NodeName string + Prompter Prompter +} + +// Resolve produces the KEY=VALUE map to inject into the node process. +// +// Resolution order per variable: +// 1. process environment (already exported) — used as-is, never persisted +// 2. node-scoped secret store, then global-scoped store +// 3. manifest default +// 4. (required only) prompt the user, validate, and persist encrypted +// +// Required variables that remain unset after prompting (or in a non-interactive +// session) are returned as a list of missing names alongside an error. +func (r *EnvResolver) Resolve(env UserEnvironmentConfig) (map[string]string, error) { + resolved := map[string]string{} + var missing []string + + resolveOne := func(v UserEnvironmentVar, required bool) error { + // 1. Process environment wins and is never written to disk. + if val, ok := os.LookupEnv(v.Name); ok && val != "" { + resolved[v.Name] = val + return nil + } + // 2. Secret store (node overrides global). + if val, ok, err := r.Store.Get(r.NodeName, v.Name); err != nil { + return err + } else if ok { + resolved[v.Name] = val + return nil + } + // 3. Manifest default. + if v.Default != "" { + resolved[v.Name] = v.Default + return nil + } + if !required { + return nil // optional and unset: leave it out + } + // 4. Prompt (required only). + if r.Prompter == nil || !r.Prompter.Interactive() { + missing = append(missing, v.Name) + return nil + } + val, err := r.promptAndValidate(v) + if err != nil { + return err + } + if val == "" { + missing = append(missing, v.Name) + return nil + } + if err := r.Store.Set(v.SecretScope(r.NodeName), v.Name, val); err != nil { + return fmt.Errorf("failed to save %s: %w", v.Name, err) + } + resolved[v.Name] = val + return nil + } + + for _, v := range env.Required { + if err := resolveOne(v, true); err != nil { + return nil, err + } + } + for _, v := range env.Optional { + if err := resolveOne(v, false); err != nil { + return nil, err + } + } + + if len(missing) > 0 { + return nil, fmt.Errorf("missing required environment variables: %s", strings.Join(missing, ", ")) + } + return resolved, nil +} + +// promptAndValidate prompts until the value satisfies the variable's validation +// regex (if any), or returns the raw value when no pattern is set. +func (r *EnvResolver) promptAndValidate(v UserEnvironmentVar) (string, error) { + var re *regexp.Regexp + if v.Validation != "" { + compiled, err := regexp.Compile(v.Validation) + if err != nil { + return "", fmt.Errorf("invalid validation regex for %s: %w", v.Name, err) + } + re = compiled + } + + for attempt := 0; attempt < 3; attempt++ { + val, err := r.Prompter.Prompt(v) + if err != nil { + return "", err + } + val = strings.TrimSpace(val) + if val == "" { + return "", nil // user skipped + } + if re != nil && !re.MatchString(val) { + fmt.Printf(" value does not match required format (%s), try again\n", v.Validation) + continue + } + return val, nil + } + return "", fmt.Errorf("%s: too many invalid attempts", v.Name) +} + +// TTYPrompter reads values from the terminal, hiding secret input. +type TTYPrompter struct{} + +// Interactive reports whether stdin is a terminal. +func (TTYPrompter) Interactive() bool { + return term.IsTerminal(int(os.Stdin.Fd())) +} + +// Prompt asks for a value on stdin, reading without echo for secret types. +func (TTYPrompter) Prompt(v UserEnvironmentVar) (string, error) { + label := v.Name + if v.Description != "" { + label = fmt.Sprintf("%s (%s)", v.Name, v.Description) + } + + if v.Type == "secret" { + fmt.Printf(" Enter %s [hidden]: ", label) + data, err := term.ReadPassword(int(os.Stdin.Fd())) + fmt.Println() + if err != nil { + return "", fmt.Errorf("failed to read secret: %w", err) + } + return string(data), nil + } + + fmt.Printf(" Enter %s: ", label) + reader := bufio.NewReader(os.Stdin) + line, err := reader.ReadString('\n') + if err != nil && line == "" { + return "", fmt.Errorf("failed to read input: %w", err) + } + return line, nil +} diff --git a/control-plane/internal/packages/env_resolver_test.go b/control-plane/internal/packages/env_resolver_test.go new file mode 100644 index 000000000..0062b6047 --- /dev/null +++ b/control-plane/internal/packages/env_resolver_test.go @@ -0,0 +1,181 @@ +package packages + +import ( + "testing" +) + +// fakePrompter returns canned answers and records what it was asked. +type fakePrompter struct { + interactive bool + answers map[string]string + asked []string +} + +func (f *fakePrompter) Interactive() bool { return f.interactive } +func (f *fakePrompter) Prompt(v UserEnvironmentVar) (string, error) { + f.asked = append(f.asked, v.Name) + return f.answers[v.Name], nil +} + +func newResolver(t *testing.T, node string, p Prompter) *EnvResolver { + t.Helper() + store, err := NewSecretStore(t.TempDir()) + if err != nil { + t.Fatalf("store: %v", err) + } + return &EnvResolver{Store: store, NodeName: node, Prompter: p} +} + +// Contract: a value already in the process environment is used and NOT persisted. +func TestResolve_ProcessEnvWinsAndIsNotPersisted(t *testing.T) { + t.Setenv("OPENROUTER_API_KEY", "from-env") + p := &fakePrompter{interactive: true} + r := newResolver(t, "pr-af", p) + + got, err := r.Resolve(UserEnvironmentConfig{ + Required: []UserEnvironmentVar{{Name: "OPENROUTER_API_KEY", Type: "secret"}}, + }) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if got["OPENROUTER_API_KEY"] != "from-env" { + t.Fatalf("got %q, want from-env", got["OPENROUTER_API_KEY"]) + } + if len(p.asked) != 0 { + t.Fatalf("should not prompt when env is set, asked=%v", p.asked) + } + if _, ok, _ := r.Store.Get("pr-af", "OPENROUTER_API_KEY"); ok { + t.Fatalf("process-env value must not be persisted to the store") + } +} + +// Contract: a missing required secret is prompted once, persisted encrypted to +// the global scope, and reused on the next resolve without prompting again. +func TestResolve_PromptsThenPersistsAndReuses(t *testing.T) { + p := &fakePrompter{interactive: true, answers: map[string]string{"OPENROUTER_API_KEY": "sk-prompted"}} + r := newResolver(t, "pr-af", p) + cfg := UserEnvironmentConfig{Required: []UserEnvironmentVar{{Name: "OPENROUTER_API_KEY", Type: "secret"}}} + + got, err := r.Resolve(cfg) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if got["OPENROUTER_API_KEY"] != "sk-prompted" { + t.Fatalf("got %q, want sk-prompted", got["OPENROUTER_API_KEY"]) + } + if v, ok, _ := r.Store.Get("pr-af", "OPENROUTER_API_KEY"); !ok || v != "sk-prompted" { + t.Fatalf("secret not persisted to global scope") + } + + // Second resolve with a non-interactive prompter must not be asked again. + r2 := &EnvResolver{Store: r.Store, NodeName: "pr-af", Prompter: &fakePrompter{interactive: false}} + got2, err := r2.Resolve(cfg) + if err != nil { + t.Fatalf("second Resolve: %v", err) + } + if got2["OPENROUTER_API_KEY"] != "sk-prompted" { + t.Fatalf("stored secret not reused: %q", got2["OPENROUTER_API_KEY"]) + } +} + +// Contract: a shared (global-scope) secret entered for one node is reused by +// another node without re-prompting. +func TestResolve_GlobalSecretSharedAcrossNodes(t *testing.T) { + store, _ := NewSecretStore(t.TempDir()) + cfg := UserEnvironmentConfig{Required: []UserEnvironmentVar{{Name: "OPENROUTER_API_KEY", Type: "secret"}}} + + r1 := &EnvResolver{Store: store, NodeName: "pr-af", + Prompter: &fakePrompter{interactive: true, answers: map[string]string{"OPENROUTER_API_KEY": "shared"}}} + if _, err := r1.Resolve(cfg); err != nil { + t.Fatalf("r1: %v", err) + } + + // Different node, non-interactive — must resolve from the shared global value. + r2 := &EnvResolver{Store: store, NodeName: "sec-af", Prompter: &fakePrompter{interactive: false}} + got, err := r2.Resolve(cfg) + if err != nil { + t.Fatalf("r2: %v", err) + } + if got["OPENROUTER_API_KEY"] != "shared" { + t.Fatalf("sec-af did not reuse shared key: %q", got["OPENROUTER_API_KEY"]) + } +} + +// Contract: a node-scoped variable persists to the node scope, not global. +func TestResolve_NodeScopedPersistsToNode(t *testing.T) { + p := &fakePrompter{interactive: true, answers: map[string]string{"NODE_TOKEN": "nv"}} + r := newResolver(t, "pr-af", p) + if _, err := r.Resolve(UserEnvironmentConfig{ + Required: []UserEnvironmentVar{{Name: "NODE_TOKEN", Type: "secret", Scope: "node"}}, + }); err != nil { + t.Fatalf("Resolve: %v", err) + } + if keys, _ := r.Store.List("pr-af"); len(keys) != 1 || keys[0] != "NODE_TOKEN" { + t.Fatalf("NODE_TOKEN not in node scope: %v", keys) + } + if keys, _ := r.Store.List("global"); len(keys) != 0 { + t.Fatalf("node-scoped secret leaked into global: %v", keys) + } +} + +// Contract: in a non-interactive session, a missing required var is an error +// naming the missing variable. +func TestResolve_NonInteractiveMissingErrors(t *testing.T) { + r := newResolver(t, "pr-af", &fakePrompter{interactive: false}) + _, err := r.Resolve(UserEnvironmentConfig{ + Required: []UserEnvironmentVar{{Name: "MUST_HAVE", Type: "secret"}}, + }) + if err == nil { + t.Fatalf("expected error for missing required var") + } +} + +// Contract: optional vars fall back to their default and are never prompted. +func TestResolve_OptionalUsesDefaultNoPrompt(t *testing.T) { + p := &fakePrompter{interactive: true} + r := newResolver(t, "pr-af", p) + got, err := r.Resolve(UserEnvironmentConfig{ + Optional: []UserEnvironmentVar{{Name: "REGION", Default: "us-east-1"}}, + }) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if got["REGION"] != "us-east-1" { + t.Fatalf("REGION = %q, want default us-east-1", got["REGION"]) + } + if len(p.asked) != 0 { + t.Fatalf("optional vars must not prompt, asked=%v", p.asked) + } +} + +// Contract: a value failing the validation regex is rejected; a later valid +// answer is accepted. +func TestResolve_ValidationRegexRejectsThenAccepts(t *testing.T) { + // retryPrompter returns a bad value first, then a good one. + rp := &retryPrompter{values: []string{"bad value", "GOOD123"}} + r := newResolver(t, "pr-af", rp) + got, err := r.Resolve(UserEnvironmentConfig{ + Required: []UserEnvironmentVar{{Name: "CODE", Validation: "^[A-Z0-9]+$"}}, + }) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if got["CODE"] != "GOOD123" { + t.Fatalf("CODE = %q, want GOOD123", got["CODE"]) + } +} + +type retryPrompter struct { + values []string + i int +} + +func (r *retryPrompter) Interactive() bool { return true } +func (r *retryPrompter) Prompt(v UserEnvironmentVar) (string, error) { + if r.i >= len(r.values) { + return "", nil + } + val := r.values[r.i] + r.i++ + return val, nil +} diff --git a/control-plane/internal/packages/github_coverage_test.go b/control-plane/internal/packages/github_coverage_test.go index 969e81e38..286d37511 100644 --- a/control-plane/internal/packages/github_coverage_test.go +++ b/control-plane/internal/packages/github_coverage_test.go @@ -1,4 +1,3 @@ - package packages import ( @@ -94,7 +93,7 @@ func TestGitHubInstallerErrorCasesFinal(t *testing.T) { w.Write([]byte("short body")) })) defer server.Close() - + gi := &GitHubInstaller{} info := &GitHubPackageInfo{ArchiveURL: server.URL} diff --git a/control-plane/internal/packages/installer.go b/control-plane/internal/packages/installer.go index 939e5af36..47010737e 100644 --- a/control-plane/internal/packages/installer.go +++ b/control-plane/internal/packages/installer.go @@ -6,6 +6,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "sync" "time" @@ -21,6 +22,16 @@ type UserEnvironmentVar struct { Default string `yaml:"default"` Optional bool `yaml:"optional"` Validation string `yaml:"validation"` // regex pattern + Scope string `yaml:"scope"` // "global" (shared across nodes, default) or "node" +} + +// SecretScope returns the secret store scope for this variable given the node +// name. Variables default to global so shared keys (API tokens) are entered once. +func (v UserEnvironmentVar) SecretScope(nodeName string) string { + if v.Scope == "node" { + return nodeName + } + return globalScope } // UserEnvironmentConfig represents user-configurable environment variables @@ -37,6 +48,7 @@ type PackageMetadata struct { Author string `yaml:"author"` Type string `yaml:"type"` Main string `yaml:"main"` + Entrypoint EntrypointConfig `yaml:"entrypoint"` AgentNode AgentNodeConfig `yaml:"agent_node"` Dependencies DependencyConfig `yaml:"dependencies"` Capabilities CapabilityConfig `yaml:"capabilities"` @@ -44,6 +56,16 @@ type PackageMetadata struct { Metadata map[string]interface{} `yaml:"metadata"` } +// EntrypointConfig describes how to start the agent node process. +type EntrypointConfig struct { + // Start is the shell-free command used to launch the node, e.g. + // "python -m pr_af.app". The first token is resolved against the package + // venv when it is "python"/"python3". Empty falls back to "python main.py". + Start string `yaml:"start"` + // Healthcheck is the HTTP path polled to confirm readiness (default "/health"). + Healthcheck string `yaml:"healthcheck"` +} + // AgentNodeConfig represents agent node specific configuration type AgentNodeConfig struct { NodeID string `yaml:"node_id"` @@ -54,6 +76,10 @@ type AgentNodeConfig struct { type DependencyConfig struct { Python []string `yaml:"python"` System []string `yaml:"system"` + // Nodes lists other agent nodes this node depends on. Each entry is an + // installable source: an "af://registry/[@version]" ref or a git URL. + // Installing this node installs its node dependencies recursively. + Nodes []string `yaml:"nodes"` } // CapabilityConfig represents agent node capabilities @@ -396,18 +422,48 @@ func (pi *PackageInstaller) validatePackage(sourcePath string) error { return fmt.Errorf("agentfield-package.yaml not found in %s", sourcePath) } - // Check if main.py exists + // A node must declare how to start: either a manifest entrypoint.start + // (e.g. "python -m pr_af.app") or a top-level main.py. Real nodes use a + // module entrypoint and have no main.py, so we no longer require it. + metadata, err := ParsePackageMetadata(sourcePath) + if err != nil { + return err + } + if metadata.Entrypoint.Start != "" { + return nil + } mainPyPath := filepath.Join(sourcePath, "main.py") if _, err := os.Stat(mainPyPath); os.IsNotExist(err) { - return fmt.Errorf("main.py not found in %s", sourcePath) + return fmt.Errorf("package must declare entrypoint.start in agentfield-package.yaml or contain a main.py") } return nil } -// parsePackageMetadata parses the agentfield-package.yaml file -func (pi *PackageInstaller) parsePackageMetadata(sourcePath string) (*PackageMetadata, error) { - packageYamlPath := filepath.Join(sourcePath, "agentfield-package.yaml") +// StartCommand returns the tokens used to launch the node. It prefers the +// manifest entrypoint.start and falls back to "python
" (default main.py). +func (m *PackageMetadata) StartCommand() []string { + if strings.TrimSpace(m.Entrypoint.Start) != "" { + return strings.Fields(m.Entrypoint.Start) + } + main := m.Main + if main == "" { + main = "main.py" + } + return []string{"python", main} +} + +// HealthcheckPath returns the readiness path, defaulting to "/health". +func (m *PackageMetadata) HealthcheckPath() string { + if p := strings.TrimSpace(m.Entrypoint.Healthcheck); p != "" { + return p + } + return "/health" +} + +// ParsePackageMetadata parses agentfield-package.yaml from a package directory. +func ParsePackageMetadata(dir string) (*PackageMetadata, error) { + packageYamlPath := filepath.Join(dir, "agentfield-package.yaml") data, err := os.ReadFile(packageYamlPath) if err != nil { @@ -433,6 +489,11 @@ func (pi *PackageInstaller) parsePackageMetadata(sourcePath string) (*PackageMet return &metadata, nil } +// parsePackageMetadata parses the agentfield-package.yaml file. +func (pi *PackageInstaller) parsePackageMetadata(sourcePath string) (*PackageMetadata, error) { + return ParsePackageMetadata(sourcePath) +} + // isPackageInstalled checks if a package is already installed func (pi *PackageInstaller) isPackageInstalled(packageName string) bool { registryPath := filepath.Join(pi.AgentFieldHome, "installed.yaml") @@ -474,6 +535,15 @@ func (pi *PackageInstaller) copyPackage(sourcePath, destPath string) error { return err } + // Skip VCS, build artifacts, local venvs and plaintext secrets so they + // never get copied into ~/.agentfield/packages. + if shouldSkipCopy(relPath, info) { + if info.IsDir() { + return filepath.SkipDir + } + return nil + } + destFilePath := filepath.Join(destPath, relPath) if info.IsDir() { @@ -485,6 +555,34 @@ func (pi *PackageInstaller) copyPackage(sourcePath, destPath string) error { }) } +// copyExcludedNames are directory/file names skipped during package copy. +var copyExcludedNames = map[string]bool{ + ".git": true, + "venv": true, + ".venv": true, + "__pycache__": true, + ".env": true, + "node_modules": true, + ".mypy_cache": true, + ".pytest_cache": true, +} + +// shouldSkipCopy reports whether a walked path should be excluded from the copy. +func shouldSkipCopy(relPath string, info os.FileInfo) bool { + if relPath == "." { + return false + } + base := filepath.Base(relPath) + if copyExcludedNames[base] { + return true + } + // Skip stray .env.* local overrides but keep .env.example. + if strings.HasPrefix(base, ".env.") && base != ".env.example" { + return true + } + return false +} + // copyFile copies a single file from src to dst func (pi *PackageInstaller) copyFile(src, dst string) error { sourceFile, err := os.Open(src) diff --git a/control-plane/internal/packages/installer_additional_test.go b/control-plane/internal/packages/installer_additional_test.go index c69d834a5..b94628097 100644 --- a/control-plane/internal/packages/installer_additional_test.go +++ b/control-plane/internal/packages/installer_additional_test.go @@ -117,10 +117,10 @@ func TestInstallDependenciesAdditionalCoverage(t *testing.T) { wantErr: "failed to install requirements.txt dependencies", }, { - name: "dependency install failure", - pythonDeps: []string{"baddep"}, - failDep: "baddep", - wantErr: "failed to install dependency baddep", + name: "dependency install failure", + pythonDeps: []string{"baddep"}, + failDep: "baddep", + wantErr: "failed to install dependency baddep", wantPipCalls: []string{ "install --upgrade pip", "install baddep", diff --git a/control-plane/internal/packages/installer_coverage_test.go b/control-plane/internal/packages/installer_coverage_test.go index e7e7c2f06..f2d5e67ff 100644 --- a/control-plane/internal/packages/installer_coverage_test.go +++ b/control-plane/internal/packages/installer_coverage_test.go @@ -1,4 +1,3 @@ - package packages import ( @@ -20,7 +19,7 @@ func TestInstallerCoverage(t *testing.T) { } }) - t.Run("copyFile-open-fails", func(t *testing.T){ + t.Run("copyFile-open-fails", func(t *testing.T) { pi := &PackageInstaller{} err := pi.copyFile("/nonexistent", "/tmp/dest") if err == nil { @@ -28,7 +27,7 @@ func TestInstallerCoverage(t *testing.T) { } }) - t.Run("copyFile-create-fails", func(t *testing.T){ + t.Run("copyFile-create-fails", func(t *testing.T) { pi := &PackageInstaller{} src := filepath.Join(t.TempDir(), "src") if err := os.WriteFile(src, []byte{}, 0644); err != nil { @@ -46,7 +45,7 @@ func TestInstallerCoverage(t *testing.T) { _ = os.Chmod(destDir, 0755) }) - t.Run("installDependencies-python-fails", func(t *testing.T){ + t.Run("installDependencies-python-fails", func(t *testing.T) { pi := &PackageInstaller{} pkgPath := t.TempDir() metadata := &PackageMetadata{ @@ -63,8 +62,8 @@ func TestInstallerCoverage(t *testing.T) { t.Fatalf("expected python to fail, got %v", err) } }) - - t.Run("installDependencies-pip-fails", func(t *testing.T){ + + t.Run("installDependencies-pip-fails", func(t *testing.T) { pi := &PackageInstaller{} pkgPath := t.TempDir() metadata := &PackageMetadata{ @@ -86,8 +85,8 @@ func TestInstallerCoverage(t *testing.T) { t.Fatalf("expected pip to fail, got %v", err) } }) - - t.Run("InstallPackage-install-deps-fails", func(t *testing.T){ + + t.Run("InstallPackage-install-deps-fails", func(t *testing.T) { pi := &PackageInstaller{AgentFieldHome: t.TempDir()} sourcePath := t.TempDir() writeTestPackage(t, sourcePath, ` diff --git a/control-plane/internal/packages/installer_test.go b/control-plane/internal/packages/installer_test.go index 807bbc33b..1c3faa8cb 100644 --- a/control-plane/internal/packages/installer_test.go +++ b/control-plane/internal/packages/installer_test.go @@ -78,6 +78,35 @@ user_environment: default: us `) + "\n" + t.Run("validate package with entrypoint and no main.py", func(t *testing.T) { + // Real nodes start via a module entrypoint and have no top-level main.py. + pkg := filepath.Join(base, "entrypoint-only") + _ = os.MkdirAll(pkg, 0755) + yaml := strings.TrimSpace(` +name: pr-af +version: 0.1.0 +entrypoint: + start: python -m pr_af.app + healthcheck: /health +`) + "\n" + if err := os.WriteFile(filepath.Join(pkg, "agentfield-package.yaml"), []byte(yaml), 0644); err != nil { + t.Fatalf("write manifest: %v", err) + } + if err := installer.validatePackage(pkg); err != nil { + t.Fatalf("validatePackage with entrypoint should pass: %v", err) + } + meta, err := installer.parsePackageMetadata(pkg) + if err != nil { + t.Fatalf("parsePackageMetadata: %v", err) + } + if got := meta.StartCommand(); len(got) != 3 || got[0] != "python" || got[2] != "pr_af.app" { + t.Fatalf("StartCommand = %v, want [python -m pr_af.app]", got) + } + if meta.HealthcheckPath() != "/health" { + t.Fatalf("HealthcheckPath = %q", meta.HealthcheckPath()) + } + }) + t.Run("validate package", func(t *testing.T) { pkg := filepath.Join(base, "valid") writeTestPackage(t, pkg, validYAML) @@ -112,12 +141,12 @@ user_environment: wantErr: "agentfield-package.yaml not found", }, { - name: "missing main", + name: "missing main and entrypoint", setup: func(dir string) { _ = os.MkdirAll(dir, 0755) _ = os.WriteFile(filepath.Join(dir, "agentfield-package.yaml"), []byte(validYAML), 0644) }, - wantErr: "main.py not found", + wantErr: "entrypoint.start", }, { name: "missing name", diff --git a/control-plane/internal/packages/runner.go b/control-plane/internal/packages/runner.go index b93abda18..5c204a00f 100644 --- a/control-plane/internal/packages/runner.go +++ b/control-plane/internal/packages/runner.go @@ -62,7 +62,11 @@ func (ar *AgentNodeRunner) RunAgentNode(agentNodeName string) error { } // 5. Wait for agent node to be ready - if err := ar.waitForAgentNode(port, 10*time.Second); err != nil { + healthPath := "/health" + if metadata, err := ParsePackageMetadata(agentNode.Path); err == nil { + healthPath = metadata.HealthcheckPath() + } + if err := ar.waitForAgentNode(port, healthPath, 10*time.Second); err != nil { if killErr := cmd.Process.Kill(); killErr != nil && !errors.Is(killErr, os.ErrProcessDone) { fmt.Printf("⚠️ Failed to kill agent node process: %v\n", killErr) } @@ -110,37 +114,51 @@ func (ar *AgentNodeRunner) isPortAvailable(port int) bool { // startAgentNodeProcess starts the agent node process func (ar *AgentNodeRunner) startAgentNodeProcess(agentNode InstalledPackage, port int) (*exec.Cmd, error) { - // Prepare environment variables + // Read the package manifest for the entrypoint and declared environment. + // Fall back to defaults (python main.py, /health, no declared env) if a + // manifest is missing so legacy installs still start. + metadata, err := ParsePackageMetadata(agentNode.Path) + if err != nil { + fmt.Printf("⚠️ No usable manifest (%v); falling back to python main.py\n", err) + metadata = &PackageMetadata{} + } + + // Prepare environment variables. Export both AGENTFIELD_SERVER (what the + // SDK reads) and the legacy AGENTFIELD_SERVER_URL for back-compat. + serverURL := resolveServerURL() env := os.Environ() env = append(env, fmt.Sprintf("PORT=%d", port)) - env = append(env, fmt.Sprintf("AGENTFIELD_SERVER_URL=%s", resolveServerURL())) + env = append(env, fmt.Sprintf("AGENTFIELD_SERVER=%s", serverURL)) + env = append(env, fmt.Sprintf("AGENTFIELD_SERVER_URL=%s", serverURL)) - // Load environment variables from package .env file - if envVars, err := ar.loadPackageEnvFile(agentNode.Path); err == nil { - for key, value := range envVars { - env = append(env, fmt.Sprintf("%s=%s", key, value)) - } - fmt.Printf("🔧 Loaded %d environment variables from .env file\n", len(envVars)) + // Resolve declared variables from the encrypted secret store, prompting for + // missing required ones and persisting them. Secrets are only ever injected + // into this child process — never written to disk in plaintext. + resolvedEnv, err := ar.resolveEnvironment(agentNode.Name, metadata) + if err != nil { + return nil, err + } + for key, value := range resolvedEnv { + env = append(env, fmt.Sprintf("%s=%s", key, value)) } // Prepare command - use virtual environment if available - var pythonPath string - venvPath := filepath.Join(agentNode.Path, "venv") + startArgs := metadata.StartCommand() + program := startArgs[0] + args := startArgs[1:] - // Check if virtual environment exists - if _, err := os.Stat(filepath.Join(venvPath, "bin", "python")); err == nil { - pythonPath = filepath.Join(venvPath, "bin", "python") - fmt.Printf("🐍 Using virtual environment: %s\n", venvPath) - } else if _, err := os.Stat(filepath.Join(venvPath, "Scripts", "python.exe")); err == nil { - pythonPath = filepath.Join(venvPath, "Scripts", "python.exe") // Windows - fmt.Printf("🐍 Using virtual environment: %s\n", venvPath) - } else { - // Fallback to system python - pythonPath = "python" - fmt.Printf("⚠️ Virtual environment not found, using system Python\n") + venvPath := filepath.Join(agentNode.Path, "venv") + if program == "python" || program == "python3" { + if p := venvPython(venvPath); p != "" { + program = p + fmt.Printf("🐍 Using virtual environment: %s\n", venvPath) + } else { + program = "python" + fmt.Printf("⚠️ Virtual environment not found, using system Python\n") + } } - cmd := exec.Command(pythonPath, "main.py") + cmd := exec.Command(program, args...) cmd.Dir = agentNode.Path cmd.Env = env @@ -161,13 +179,47 @@ func (ar *AgentNodeRunner) startAgentNodeProcess(agentNode InstalledPackage, por return cmd, nil } +// venvPython returns the venv python interpreter path, or "" if no venv exists. +func venvPython(venvPath string) string { + if p := filepath.Join(venvPath, "bin", "python"); fileExists(p) { + return p + } + if p := filepath.Join(venvPath, "Scripts", "python.exe"); fileExists(p) { // Windows + return p + } + return "" +} + +func fileExists(path string) bool { + _, err := os.Stat(path) + return err == nil +} + +// resolveEnvironment resolves the node's declared variables via the encrypted +// secret store, prompting for missing required ones. +func (ar *AgentNodeRunner) resolveEnvironment(nodeName string, metadata *PackageMetadata) (map[string]string, error) { + env := metadata.UserEnvironment + if len(env.Required) == 0 && len(env.Optional) == 0 { + return map[string]string{}, nil + } + store, err := NewSecretStore(ar.AgentFieldHome) + if err != nil { + return nil, fmt.Errorf("failed to open secret store: %w", err) + } + resolver := &EnvResolver{Store: store, NodeName: nodeName, Prompter: TTYPrompter{}} + return resolver.Resolve(env) +} + // waitForAgentNode waits for the agent node to become ready -func (ar *AgentNodeRunner) waitForAgentNode(port int, timeout time.Duration) error { +func (ar *AgentNodeRunner) waitForAgentNode(port int, healthPath string, timeout time.Duration) error { + if healthPath == "" { + healthPath = "/health" + } client := &http.Client{Timeout: 1 * time.Second} deadline := time.Now().Add(timeout) for time.Now().Before(deadline) { - resp, err := client.Get(fmt.Sprintf("http://localhost:%d/health", port)) + resp, err := client.Get(fmt.Sprintf("http://localhost:%d%s", port, healthPath)) if err == nil && resp.StatusCode == 200 { resp.Body.Close() return nil @@ -290,39 +342,3 @@ func (ar *AgentNodeRunner) loadRegistry() (*InstallationRegistry, error) { return registry, nil } - -// loadPackageEnvFile loads environment variables from package .env file -func (ar *AgentNodeRunner) loadPackageEnvFile(packagePath string) (map[string]string, error) { - envPath := filepath.Join(packagePath, ".env") - - data, err := os.ReadFile(envPath) - if err != nil { - return nil, err - } - - envVars := make(map[string]string) - lines := strings.Split(string(data), "\n") - - for _, line := range lines { - line = strings.TrimSpace(line) - if line == "" || strings.HasPrefix(line, "#") { - continue - } - - parts := strings.SplitN(line, "=", 2) - if len(parts) == 2 { - key := strings.TrimSpace(parts[0]) - value := strings.TrimSpace(parts[1]) - - // Remove quotes if present - if (strings.HasPrefix(value, "\"") && strings.HasSuffix(value, "\"")) || - (strings.HasPrefix(value, "'") && strings.HasSuffix(value, "'")) { - value = value[1 : len(value)-1] - } - - envVars[key] = value - } - } - - return envVars, nil -} diff --git a/control-plane/internal/packages/runner_coverage_test.go b/control-plane/internal/packages/runner_coverage_test.go index 69b9ef2a6..ca3a1ea98 100644 --- a/control-plane/internal/packages/runner_coverage_test.go +++ b/control-plane/internal/packages/runner_coverage_test.go @@ -1,4 +1,3 @@ - package packages import ( @@ -65,11 +64,13 @@ func TestRunnerErrorCases(t *testing.T) { t.Run("RunAgentNode-start-process-fails", func(t *testing.T) { home := t.TempDir() ar := &AgentNodeRunner{AgentFieldHome: home} - + pkgPath := filepath.Join(home, "packages", "demo") - if err := os.MkdirAll(pkgPath, 0755); err != nil {t.Fatal(err)} + if err := os.MkdirAll(pkgPath, 0755); err != nil { + t.Fatal(err) + } registry := &InstallationRegistry{Installed: map[string]InstalledPackage{ - "demo": { Name: "demo", Path: pkgPath }, + "demo": {Name: "demo", Path: pkgPath}, }} if err := (&PackageUninstaller{AgentFieldHome: home}).saveRegistry(registry); err != nil { t.Fatal(err) @@ -85,23 +86,23 @@ func TestRunnerErrorCases(t *testing.T) { t.Fatalf("expected start process error, got %v", err) } }) - + t.Run("displayCapabilities-fails", func(t *testing.T) { ar := &AgentNodeRunner{} - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request){ + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { http.Error(w, "error", http.StatusInternalServerError) })) defer server.Close() port := server.Listener.Addr().(*net.TCPAddr).Port - + err := ar.displayCapabilities(InstalledPackage{}, port) if err == nil { t.Fatalf("expected display capabilities to fail") } }) - t.Run("updateRuntimeInfo-read-only-registry", func(t *testing.T){ + t.Run("updateRuntimeInfo-read-only-registry", func(t *testing.T) { home := t.TempDir() ar := &AgentNodeRunner{AgentFieldHome: home} regPath := filepath.Join(home, "installed.yaml") @@ -113,31 +114,16 @@ func TestRunnerErrorCases(t *testing.T) { } err := ar.updateRuntimeInfo("demo", 1234, 5678) - if err == nil || !(strings.Contains(err.Error(), "permission denied") || strings.Contains(err.Error(), "read-only file system")){ + if err == nil || !(strings.Contains(err.Error(), "permission denied") || strings.Contains(err.Error(), "read-only file system")) { t.Fatalf("expected permission error, got %v", err) } _ = os.Chmod(regPath, 0644) }) - - t.Run("loadPackageEnvFile-unquoted", func(t *testing.T){ - ar := &AgentNodeRunner{} - dir := t.TempDir() - if err := os.WriteFile(filepath.Join(dir, ".env"), []byte("FOO=bar"), 0644); err != nil { - t.Fatal(err) - } - vars, err := ar.loadPackageEnvFile(dir) - if err != nil { - t.Fatal(err) - } - if vars["FOO"] != "bar" { - t.Fatalf("expected bar, got %s", vars["FOO"]) - } - }) t.Run("waitForAgentNode-timeout", func(t *testing.T) { ar := &AgentNodeRunner{} // just use a port that is not listening - err := ar.waitForAgentNode(1, 10 * time.Millisecond) + err := ar.waitForAgentNode(1, "/health", 10*time.Millisecond) if err == nil { t.Fatal("expected timeout error") } diff --git a/control-plane/internal/packages/runner_test.go b/control-plane/internal/packages/runner_test.go index b65691d3b..7a95bfa2e 100644 --- a/control-plane/internal/packages/runner_test.go +++ b/control-plane/internal/packages/runner_test.go @@ -50,32 +50,6 @@ func TestAgentNodeRunnerPortEnvAndRegistry(t *testing.T) { } _ = listener.Close() - pkgPath := filepath.Join(t.TempDir(), "pkg") - if err := os.MkdirAll(pkgPath, 0755); err != nil { - t.Fatalf("mkdir: %v", err) - } - if err := os.WriteFile(filepath.Join(pkgPath, ".env"), []byte(strings.Join([]string{ - "PLAIN=value", - `QUOTED="double"`, - "SINGLE='single'", - "# comment", - "", - }, "\n")), 0644); err != nil { - t.Fatalf("write env: %v", err) - } - - envVars, err := runner.loadPackageEnvFile(pkgPath) - if err != nil { - t.Fatalf("loadPackageEnvFile: %v", err) - } - if envVars["PLAIN"] != "value" || envVars["QUOTED"] != "double" || envVars["SINGLE"] != "single" { - t.Fatalf("unexpected env vars: %#v", envVars) - } - - if _, err := runner.loadPackageEnvFile(filepath.Join(t.TempDir(), "missing")); err == nil { - t.Fatalf("expected missing env error") - } - registry := &InstallationRegistry{ Installed: map[string]InstalledPackage{ "demo": {Name: "demo"}, @@ -135,7 +109,7 @@ func TestAgentNodeRunnerWaitDisplayAndStartProcess(t *testing.T) { }) waitForHTTPServer(t, fmt.Sprintf("127.0.0.1:%d", port)) - if err := runner.waitForAgentNode(port, 2*time.Second); err != nil { + if err := runner.waitForAgentNode(port, "/health", 2*time.Second); err != nil { t.Fatalf("waitForAgentNode: %v", err) } if err := runner.displayCapabilities(InstalledPackage{Name: "demo"}, port); err != nil { @@ -148,7 +122,7 @@ func TestAgentNodeRunnerWaitDisplayAndStartProcess(t *testing.T) { } unusedPort := unusedPortListener.Addr().(*net.TCPAddr).Port _ = unusedPortListener.Close() - if err := runner.waitForAgentNode(unusedPort, 600*time.Millisecond); err == nil || !strings.Contains(err.Error(), "did not become ready") { + if err := runner.waitForAgentNode(unusedPort, "/health", 600*time.Millisecond); err == nil || !strings.Contains(err.Error(), "did not become ready") { t.Fatalf("expected timeout error, got %v", err) } diff --git a/control-plane/internal/packages/secrets.go b/control-plane/internal/packages/secrets.go new file mode 100644 index 000000000..94a835a4d --- /dev/null +++ b/control-plane/internal/packages/secrets.go @@ -0,0 +1,259 @@ +package packages + +import ( + "crypto/rand" + "encoding/hex" + "encoding/json" + "fmt" + "os" + "path/filepath" + "sort" + + "github.com/Agent-Field/agentfield/control-plane/internal/encryption" +) + +// Secrets layout under the AgentField home directory: +// +// ~/.agentfield/ +// keyring/master.key # 0600, random 32 bytes — the local decrypt key +// secrets/global.enc # encrypted JSON map, shared across all nodes +// secrets/.enc # encrypted JSON map, scoped to a single node +// +// Values are encrypted at rest with AES-256-GCM (via encryption.EncryptionService) +// and are only ever decrypted into a child process' environment at start time — +// never written back to disk in plaintext. + +const ( + globalScope = "global" + masterKeyName = "master.key" + secretFilePerm = 0o600 + keyDirPerm = 0o700 +) + +// KeyProvider supplies the passphrase used to derive the at-rest encryption key. +// It is an interface so we can add OS-keychain / passphrase providers later +// without touching the SecretStore. +type KeyProvider interface { + // Passphrase returns the secret material used to derive the encryption key. + Passphrase() (string, error) +} + +// KeyfileProvider keeps a random 32-byte key in a 0600 file under keyring/. +// It is generated on first use. This protects secrets from casual reading, +// version control, and process listings, while requiring no prompt. +type KeyfileProvider struct { + Path string // full path to the key file +} + +// NewKeyfileProvider returns a KeyfileProvider rooted at the given AgentField home. +func NewKeyfileProvider(agentFieldHome string) *KeyfileProvider { + return &KeyfileProvider{Path: filepath.Join(agentFieldHome, "keyring", masterKeyName)} +} + +// Passphrase returns the hex-encoded key material, creating it on first use. +func (kp *KeyfileProvider) Passphrase() (string, error) { + if data, err := os.ReadFile(kp.Path); err == nil { + key := string(data) + if key == "" { + return "", fmt.Errorf("master key file %s is empty", kp.Path) + } + return key, nil + } else if !os.IsNotExist(err) { + return "", fmt.Errorf("failed to read master key: %w", err) + } + + // Generate a new random key on first use. + if err := os.MkdirAll(filepath.Dir(kp.Path), keyDirPerm); err != nil { + return "", fmt.Errorf("failed to create keyring directory: %w", err) + } + raw := make([]byte, 32) + if _, err := rand.Read(raw); err != nil { + return "", fmt.Errorf("failed to generate master key: %w", err) + } + key := hex.EncodeToString(raw) + if err := os.WriteFile(kp.Path, []byte(key), secretFilePerm); err != nil { + return "", fmt.Errorf("failed to write master key: %w", err) + } + return key, nil +} + +// SecretStore reads and writes encrypted secret maps scoped globally or per-node. +type SecretStore struct { + AgentFieldHome string + provider KeyProvider + enc *encryption.EncryptionService +} + +// NewSecretStore builds a SecretStore using a keyfile provider by default. +func NewSecretStore(agentFieldHome string) (*SecretStore, error) { + return NewSecretStoreWithProvider(agentFieldHome, NewKeyfileProvider(agentFieldHome)) +} + +// NewSecretStoreWithProvider builds a SecretStore with a custom key provider. +func NewSecretStoreWithProvider(agentFieldHome string, provider KeyProvider) (*SecretStore, error) { + passphrase, err := provider.Passphrase() + if err != nil { + return nil, err + } + return &SecretStore{ + AgentFieldHome: agentFieldHome, + provider: provider, + enc: encryption.NewEncryptionService(passphrase), + }, nil +} + +// scopeFile returns the encrypted file path for a scope ("global" or a node name). +func (s *SecretStore) scopeFile(scope string) string { + if scope == "" { + scope = globalScope + } + return filepath.Join(s.AgentFieldHome, "secrets", scope+".enc") +} + +// load decrypts a scope's secret map. A missing file yields an empty map. +func (s *SecretStore) load(scope string) (map[string]string, error) { + path := s.scopeFile(scope) + data, err := os.ReadFile(path) + if os.IsNotExist(err) { + return map[string]string{}, nil + } + if err != nil { + return nil, fmt.Errorf("failed to read secrets %s: %w", scope, err) + } + plaintext, err := s.enc.DecryptBytes(data) + if err != nil { + return nil, fmt.Errorf("failed to decrypt secrets %s (wrong key?): %w", scope, err) + } + out := map[string]string{} + if err := json.Unmarshal(plaintext, &out); err != nil { + return nil, fmt.Errorf("failed to parse secrets %s: %w", scope, err) + } + return out, nil +} + +// save encrypts and writes a scope's secret map with 0600 permissions. +func (s *SecretStore) save(scope string, values map[string]string) error { + path := s.scopeFile(scope) + if err := os.MkdirAll(filepath.Dir(path), keyDirPerm); err != nil { + return fmt.Errorf("failed to create secrets directory: %w", err) + } + plaintext, err := json.Marshal(values) + if err != nil { + return fmt.Errorf("failed to marshal secrets %s: %w", scope, err) + } + ciphertext, err := s.enc.EncryptBytes(plaintext) + if err != nil { + return fmt.Errorf("failed to encrypt secrets %s: %w", scope, err) + } + if err := os.WriteFile(path, ciphertext, secretFilePerm); err != nil { + return fmt.Errorf("failed to write secrets %s: %w", scope, err) + } + return nil +} + +// Set stores a secret in the given scope (use "global" or a node name). +func (s *SecretStore) Set(scope, key, value string) error { + values, err := s.load(scope) + if err != nil { + return err + } + values[key] = value + return s.save(scope, values) +} + +// Delete removes a secret from the given scope. Missing keys are a no-op. +func (s *SecretStore) Delete(scope, key string) error { + values, err := s.load(scope) + if err != nil { + return err + } + if _, ok := values[key]; !ok { + return nil + } + delete(values, key) + return s.save(scope, values) +} + +// Get resolves a secret for a node, preferring the node scope over global. +// The second return value reports whether the key was found. +func (s *SecretStore) Get(node, key string) (string, bool, error) { + if node != "" && node != globalScope { + nodeVals, err := s.load(node) + if err != nil { + return "", false, err + } + if v, ok := nodeVals[key]; ok { + return v, true, nil + } + } + globalVals, err := s.load(globalScope) + if err != nil { + return "", false, err + } + v, ok := globalVals[key] + return v, ok, nil +} + +// SecretRef identifies a stored secret and the scope it lives in. +type SecretRef struct { + Key string + Scope string +} + +// List returns the keys present in a scope (sorted). Values are never returned +// here so callers cannot accidentally print them. +func (s *SecretStore) List(scope string) ([]string, error) { + values, err := s.load(scope) + if err != nil { + return nil, err + } + keys := make([]string, 0, len(values)) + for k := range values { + keys = append(keys, k) + } + sort.Strings(keys) + return keys, nil +} + +// ListAll returns every stored secret reference across global and all node +// scopes, sorted by scope then key. Values are never returned. +func (s *SecretStore) ListAll() ([]SecretRef, error) { + dir := filepath.Join(s.AgentFieldHome, "secrets") + entries, err := os.ReadDir(dir) + if os.IsNotExist(err) { + return nil, nil + } + if err != nil { + return nil, fmt.Errorf("failed to read secrets directory: %w", err) + } + var refs []SecretRef + for _, e := range entries { + name := e.Name() + if e.IsDir() || filepath.Ext(name) != ".enc" { + continue + } + scope := name[:len(name)-len(".enc")] + keys, err := s.List(scope) + if err != nil { + return nil, err + } + for _, k := range keys { + refs = append(refs, SecretRef{Key: k, Scope: scope}) + } + } + sort.Slice(refs, func(i, j int) bool { + if refs[i].Scope != refs[j].Scope { + return refs[i].Scope < refs[j].Scope + } + return refs[i].Key < refs[j].Key + }) + return refs, nil +} + +// MaskSecret returns a display-safe rendering of a secret value. +func MaskSecret(value string) string { + if len(value) <= 4 { + return "****" + } + return value[:2] + "****" + value[len(value)-2:] +} diff --git a/control-plane/internal/packages/secrets_test.go b/control-plane/internal/packages/secrets_test.go new file mode 100644 index 000000000..156edcb16 --- /dev/null +++ b/control-plane/internal/packages/secrets_test.go @@ -0,0 +1,193 @@ +package packages + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func newTestStore(t *testing.T) (*SecretStore, string) { + t.Helper() + home := t.TempDir() + store, err := NewSecretStore(home) + if err != nil { + t.Fatalf("NewSecretStore: %v", err) + } + return store, home +} + +// Contract: a secret set in a scope round-trips back out unchanged. +func TestSecretStore_SetGetRoundtrip(t *testing.T) { + store, _ := newTestStore(t) + + if err := store.Set(globalScope, "OPENROUTER_API_KEY", "sk-abc123"); err != nil { + t.Fatalf("Set: %v", err) + } + got, ok, err := store.Get("", "OPENROUTER_API_KEY") + if err != nil { + t.Fatalf("Get: %v", err) + } + if !ok || got != "sk-abc123" { + t.Fatalf("got (%q, %v), want (sk-abc123, true)", got, ok) + } +} + +// Contract: a node-scoped secret overrides the global one for that node, +// while other nodes still see the global value. +func TestSecretStore_NodeOverridesGlobal(t *testing.T) { + store, _ := newTestStore(t) + + if err := store.Set(globalScope, "MODEL", "global-model"); err != nil { + t.Fatalf("Set global: %v", err) + } + if err := store.Set("pr-af", "MODEL", "node-model"); err != nil { + t.Fatalf("Set node: %v", err) + } + + got, _, _ := store.Get("pr-af", "MODEL") + if got != "node-model" { + t.Fatalf("pr-af MODEL = %q, want node-model", got) + } + got, _, _ = store.Get("sec-af", "MODEL") + if got != "global-model" { + t.Fatalf("sec-af MODEL = %q, want global-model (fallback)", got) + } +} + +// Contract: deleting a secret removes it; missing keys are a no-op. +func TestSecretStore_Delete(t *testing.T) { + store, _ := newTestStore(t) + _ = store.Set(globalScope, "GH_TOKEN", "ghp_xxx") + + if err := store.Delete(globalScope, "GH_TOKEN"); err != nil { + t.Fatalf("Delete: %v", err) + } + if _, ok, _ := store.Get("", "GH_TOKEN"); ok { + t.Fatalf("GH_TOKEN still present after delete") + } + if err := store.Delete(globalScope, "GH_TOKEN"); err != nil { + t.Fatalf("Delete of missing key should be no-op, got %v", err) + } +} + +// Contract: secret files and the master key are written with 0600 perms, +// and the keyring directory with 0700. +func TestSecretStore_FilePermissions(t *testing.T) { + store, home := newTestStore(t) + _ = store.Set(globalScope, "K", "v") + + checks := map[string]os.FileMode{ + filepath.Join(home, "keyring", masterKeyName): 0o600, + filepath.Join(home, "secrets", "global.enc"): 0o600, + } + for path, want := range checks { + info, err := os.Stat(path) + if err != nil { + t.Fatalf("stat %s: %v", path, err) + } + if info.Mode().Perm() != want { + t.Errorf("%s perm = %o, want %o", path, info.Mode().Perm(), want) + } + } +} + +// Contract: the value never appears in plaintext anywhere on disk. +func TestSecretStore_NoPlaintextOnDisk(t *testing.T) { + store, home := newTestStore(t) + const secret = "super-secret-value-9f3a" + _ = store.Set(globalScope, "API_KEY", secret) + + err := filepath.Walk(home, func(path string, info os.FileInfo, err error) error { + if err != nil || info.IsDir() { + return err + } + data, _ := os.ReadFile(path) + if strings.Contains(string(data), secret) { + t.Errorf("plaintext secret found in %s", path) + } + return nil + }) + if err != nil { + t.Fatalf("walk: %v", err) + } +} + +// Contract: a store built from the same home (same keyfile) can decrypt +// secrets written by an earlier instance. +func TestSecretStore_PersistsAcrossInstances(t *testing.T) { + home := t.TempDir() + first, err := NewSecretStore(home) + if err != nil { + t.Fatalf("first store: %v", err) + } + _ = first.Set(globalScope, "TOKEN", "abc") + + second, err := NewSecretStore(home) + if err != nil { + t.Fatalf("second store: %v", err) + } + if got, ok, _ := second.Get("", "TOKEN"); !ok || got != "abc" { + t.Fatalf("second store Get = (%q,%v), want (abc,true)", got, ok) + } +} + +// Contract: List returns keys only (never values), sorted; ListAll spans scopes. +func TestSecretStore_ListMasksValues(t *testing.T) { + store, _ := newTestStore(t) + _ = store.Set(globalScope, "B_KEY", "v1") + _ = store.Set(globalScope, "A_KEY", "v2") + _ = store.Set("pr-af", "N_KEY", "v3") + + keys, err := store.List(globalScope) + if err != nil { + t.Fatalf("List: %v", err) + } + if len(keys) != 2 || keys[0] != "A_KEY" || keys[1] != "B_KEY" { + t.Fatalf("List = %v, want sorted [A_KEY B_KEY]", keys) + } + + all, err := store.ListAll() + if err != nil { + t.Fatalf("ListAll: %v", err) + } + if len(all) != 3 { + t.Fatalf("ListAll len = %d, want 3", len(all)) + } +} + +func TestMaskSecret(t *testing.T) { + cases := map[string]string{ + "": "****", + "abcd": "****", + "sk-abcdef1234": "sk****34", + } + for in, want := range cases { + if got := MaskSecret(in); got != want { + t.Errorf("MaskSecret(%q) = %q, want %q", in, got, want) + } + } +} + +// Contract: a wrong key cannot decrypt another store's secrets. +func TestSecretStore_WrongKeyFails(t *testing.T) { + home := t.TempDir() + store, err := NewSecretStore(home) + if err != nil { + t.Fatalf("store: %v", err) + } + _ = store.Set(globalScope, "K", "v") + + // Replace the keyfile with a different key, then try to read. + if err := os.WriteFile(filepath.Join(home, "keyring", masterKeyName), + []byte("00deadbeef00deadbeef00deadbeef00deadbeef00deadbeef00deadbeef0000"), 0o600); err != nil { + t.Fatalf("rewrite key: %v", err) + } + tampered, err := NewSecretStore(home) + if err != nil { + t.Fatalf("tampered store: %v", err) + } + if _, _, err := tampered.Get("", "K"); err == nil { + t.Fatalf("expected decryption to fail with wrong key") + } +} From ceb4f9557f6e87cbecf49b23264e398a069f9ef0 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 26 Jun 2026 11:02:44 -0400 Subject: [PATCH 2/7] feat(cli): af secrets command + node-to-node dependency install/start - af secrets set/ls/rm manages the encrypted store (hidden input for set, masked listing, global + --node scopes). - install resolves dependencies.nodes recursively (af://registry/ -> github.com/Agent-Field/, or git URLs), skipping already-installed nodes to break cycles. - af run brings up a node's installed node-dependencies first, in dependency order, with cycle protection. Co-Authored-By: Claude Opus 4.8 (1M context) --- control-plane/internal/cli/root.go | 1 + control-plane/internal/cli/secrets.go | 152 ++++++++++++++++++ .../internal/core/services/package_service.go | 83 ++++++++++ control-plane/internal/packages/installer.go | 20 +++ .../internal/packages/noderef_test.go | 19 +++ control-plane/internal/packages/runner.go | 45 +++++- control-plane/internal/packages/secrets.go | 3 + 7 files changed, 322 insertions(+), 1 deletion(-) create mode 100644 control-plane/internal/cli/secrets.go create mode 100644 control-plane/internal/packages/noderef_test.go diff --git a/control-plane/internal/cli/root.go b/control-plane/internal/cli/root.go index f98cc2530..b89299369 100644 --- a/control-plane/internal/cli/root.go +++ b/control-plane/internal/cli/root.go @@ -112,6 +112,7 @@ AI Agent? Run "af agent help" for structured JSON output optimized for programma // Add remaining old commands (not yet migrated) RootCmd.AddCommand(NewUninstallCommand()) + RootCmd.AddCommand(NewSecretsCommand()) RootCmd.AddCommand(NewListCommand()) RootCmd.AddCommand(NewStopCommand()) RootCmd.AddCommand(NewLogsCommand()) diff --git a/control-plane/internal/cli/secrets.go b/control-plane/internal/cli/secrets.go new file mode 100644 index 000000000..ccdd093b8 --- /dev/null +++ b/control-plane/internal/cli/secrets.go @@ -0,0 +1,152 @@ +package cli + +import ( + "bufio" + "fmt" + "os" + "strings" + + "github.com/Agent-Field/agentfield/control-plane/internal/packages" + "github.com/spf13/cobra" + "golang.org/x/term" +) + +// NewSecretsCommand returns the `af secrets` command tree for managing the +// encrypted secret store used by agent nodes. +func NewSecretsCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "secrets", + Short: "Manage encrypted secrets for agent nodes", + Long: `Manage the encrypted secret store under ~/.agentfield. + +Secrets are stored encrypted at rest (AES-256-GCM) and are only ever decrypted +into an agent node's process environment at start time. Global secrets are +shared across all nodes; node-scoped secrets override the global value for a +single node.`, + } + + cmd.AddCommand(newSecretsSetCommand()) + cmd.AddCommand(newSecretsListCommand()) + cmd.AddCommand(newSecretsRemoveCommand()) + return cmd +} + +func openSecretStore() (*packages.SecretStore, error) { + return packages.NewSecretStore(getAgentFieldHomeDir()) +} + +func newSecretsSetCommand() *cobra.Command { + var node string + cmd := &cobra.Command{ + Use: "set KEY [VALUE]", + Short: "Store a secret (prompts hidden if VALUE is omitted)", + Args: cobra.RangeArgs(1, 2), + RunE: func(cmd *cobra.Command, args []string) error { + key := args[0] + var value string + if len(args) == 2 { + value = args[1] + } else { + v, err := readHiddenValue(fmt.Sprintf("Enter value for %s", key)) + if err != nil { + return err + } + value = v + } + if strings.TrimSpace(value) == "" { + return fmt.Errorf("value must not be empty") + } + + store, err := openSecretStore() + if err != nil { + return err + } + scope := packages.GlobalScope + if node != "" { + scope = node + } + if err := store.Set(scope, key, value); err != nil { + return err + } + PrintSuccess(fmt.Sprintf("Stored %s in %s scope", key, scope)) + return nil + }, + } + cmd.Flags().StringVar(&node, "node", "", "store as a node-scoped secret instead of global") + return cmd +} + +func newSecretsListCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "ls", + Aliases: []string{"list"}, + Short: "List stored secrets (values masked)", + Args: cobra.NoArgs, + RunE: func(cmd *cobra.Command, args []string) error { + store, err := openSecretStore() + if err != nil { + return err + } + refs, err := store.ListAll() + if err != nil { + return err + } + if len(refs) == 0 { + PrintInfo("No secrets stored yet. Add one with: af secrets set KEY") + return nil + } + fmt.Printf("%-30s %s\n", "KEY", "SCOPE") + for _, ref := range refs { + fmt.Printf("%-30s %s\n", ref.Key, ref.Scope) + } + return nil + }, + } + return cmd +} + +func newSecretsRemoveCommand() *cobra.Command { + var node string + cmd := &cobra.Command{ + Use: "rm KEY", + Aliases: []string{"remove", "delete"}, + Short: "Remove a stored secret", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + store, err := openSecretStore() + if err != nil { + return err + } + scope := packages.GlobalScope + if node != "" { + scope = node + } + if err := store.Delete(scope, args[0]); err != nil { + return err + } + PrintSuccess(fmt.Sprintf("Removed %s from %s scope", args[0], scope)) + return nil + }, + } + cmd.Flags().StringVar(&node, "node", "", "remove from a node scope instead of global") + return cmd +} + +// readHiddenValue reads a single line without echo when stdin is a terminal, +// falling back to plain line input otherwise. +func readHiddenValue(prompt string) (string, error) { + fmt.Printf("%s: ", prompt) + if term.IsTerminal(int(os.Stdin.Fd())) { + data, err := term.ReadPassword(int(os.Stdin.Fd())) + fmt.Println() + if err != nil { + return "", fmt.Errorf("failed to read value: %w", err) + } + return strings.TrimSpace(string(data)), nil + } + line, err := bufio.NewReader(os.Stdin).ReadString('\n') + if err != nil && line == "" { + return "", fmt.Errorf("failed to read value: %w", err) + } + return strings.TrimSpace(line), nil +} diff --git a/control-plane/internal/core/services/package_service.go b/control-plane/internal/core/services/package_service.go index f4de9b403..817152049 100644 --- a/control-plane/internal/core/services/package_service.go +++ b/control-plane/internal/core/services/package_service.go @@ -7,6 +7,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "sync" "time" @@ -39,6 +40,19 @@ func NewPackageService( // InstallPackage installs a package from the given source func (ps *DefaultPackageService) InstallPackage(source string, options domain.InstallOptions) error { + // Snapshot installed packages so we can discover what this install adds and + // recursively pull in any node-to-node dependencies it declares. + before := ps.installedNames() + + if err := ps.installOne(source, options); err != nil { + return err + } + + return ps.installNodeDependencies(before, options) +} + +// installOne installs a single package from a git URL or local path. +func (ps *DefaultPackageService) installOne(source string, options domain.InstallOptions) error { // Check if it's a Git URL (GitHub, GitLab, Bitbucket, etc.) if packages.IsGitURL(source) { installer := &packages.GitInstaller{ @@ -52,6 +66,75 @@ func (ps *DefaultPackageService) InstallPackage(source string, options domain.In return ps.installLocalPackage(source, options.Force, options.Verbose) } +// installedNames returns the set of currently-installed package names. +func (ps *DefaultPackageService) installedNames() map[string]bool { + names := map[string]bool{} + registry, err := ps.loadRegistryDirect() + if err != nil { + return names + } + for name := range registry.Installed { + names[name] = true + } + return names +} + +// installNodeDependencies installs the node-to-node dependencies declared by any +// packages added since `before`, recursively. Already-installed nodes are +// skipped, which also breaks dependency cycles. +func (ps *DefaultPackageService) installNodeDependencies(before map[string]bool, options domain.InstallOptions) error { + registry, err := ps.loadRegistryDirect() + if err != nil { + return nil // base install already succeeded; don't fail on dep discovery + } + + for name, pkg := range registry.Installed { + if before[name] { + continue // not newly installed in this pass + } + metadata, err := packages.ParsePackageMetadata(pkg.Path) + if err != nil { + continue + } + for _, dep := range metadata.Dependencies.Nodes { + depSource, depName := resolveNodeRef(dep) + if depName != "" && ps.isPackageInstalled(depName) { + continue // already present — also handles cycles + } + fmt.Printf("\n%s Installing node dependency: %s\n", ps.blue("→"), dep) + snapshot := ps.installedNames() + if err := ps.installOne(depSource, options); err != nil { + fmt.Printf("%s Failed to install node dependency %s: %v\n", ps.statusError(), dep, err) + continue + } + // Recurse for the dependency's own node deps. + if err := ps.installNodeDependencies(snapshot, options); err != nil { + return err + } + } + } + return nil +} + +// resolveNodeRef maps a node dependency reference to an installable source and, +// when known, the resulting package name. Supported forms: +// +// af://registry/[@version] -> https://github.com/Agent-Field/ +// https://github.com/org/repo -> used as-is +// / -> used as-is +func resolveNodeRef(ref string) (source string, name string) { + const afPrefix = "af://registry/" + if strings.HasPrefix(ref, afPrefix) { + spec := strings.TrimPrefix(ref, afPrefix) + if at := strings.Index(spec, "@"); at >= 0 { + spec = spec[:at] // drop version constraint (not yet enforced) + } + spec = strings.TrimSuffix(spec, "/") + return "https://github.com/Agent-Field/" + spec, spec + } + return ref, "" +} + // installLocalPackage installs a package from a local source path func (ps *DefaultPackageService) installLocalPackage(sourcePath string, force bool, verbose bool) error { // Get package name first for better messaging diff --git a/control-plane/internal/packages/installer.go b/control-plane/internal/packages/installer.go index 47010737e..c406d7d85 100644 --- a/control-plane/internal/packages/installer.go +++ b/control-plane/internal/packages/installer.go @@ -453,6 +453,26 @@ func (m *PackageMetadata) StartCommand() []string { return []string{"python", main} } +// NodeDepName extracts the installed package name from a node dependency +// reference such as "af://registry/@v" or a git URL. Returns "" when the +// name cannot be derived from the reference alone. +func NodeDepName(ref string) string { + const afPrefix = "af://registry/" + if strings.HasPrefix(ref, afPrefix) { + spec := strings.TrimPrefix(ref, afPrefix) + if at := strings.Index(spec, "@"); at >= 0 { + spec = spec[:at] + } + return strings.Trim(spec, "/") + } + // Git URL: derive the repo name (last path segment, sans .git). + trimmed := strings.TrimSuffix(strings.TrimSuffix(ref, "/"), ".git") + if idx := strings.LastIndexAny(trimmed, "/:"); idx >= 0 { + return trimmed[idx+1:] + } + return "" +} + // HealthcheckPath returns the readiness path, defaulting to "/health". func (m *PackageMetadata) HealthcheckPath() string { if p := strings.TrimSpace(m.Entrypoint.Healthcheck); p != "" { diff --git a/control-plane/internal/packages/noderef_test.go b/control-plane/internal/packages/noderef_test.go new file mode 100644 index 000000000..2c208565c --- /dev/null +++ b/control-plane/internal/packages/noderef_test.go @@ -0,0 +1,19 @@ +package packages + +import "testing" + +func TestNodeDepName(t *testing.T) { + cases := map[string]string{ + "af://registry/swe-planner": "swe-planner", + "af://registry/swe-planner@^1.0": "swe-planner", + "af://registry/pr-af/": "pr-af", + "https://github.com/Agent-Field/pr-af": "pr-af", + "https://github.com/Agent-Field/pr-af.git": "pr-af", + "git@github.com:Agent-Field/sec-af.git": "sec-af", + } + for ref, want := range cases { + if got := NodeDepName(ref); got != want { + t.Errorf("NodeDepName(%q) = %q, want %q", ref, got, want) + } + } +} diff --git a/control-plane/internal/packages/runner.go b/control-plane/internal/packages/runner.go index 5c204a00f..a2f86663c 100644 --- a/control-plane/internal/packages/runner.go +++ b/control-plane/internal/packages/runner.go @@ -22,9 +22,17 @@ type AgentNodeRunner struct { Detach bool } -// RunAgentNode starts an installed agent node +// RunAgentNode starts an installed agent node, bringing up its declared node +// dependencies first. func (ar *AgentNodeRunner) RunAgentNode(agentNodeName string) error { + return ar.runAgentNode(agentNodeName, map[string]bool{}) +} + +// runAgentNode starts a node; inProgress tracks nodes already being started in +// this dependency chain to break cycles. +func (ar *AgentNodeRunner) runAgentNode(agentNodeName string, inProgress map[string]bool) error { fmt.Printf("🚀 Launching agent node: %s\n", agentNodeName) + inProgress[agentNodeName] = true // 1. Check if agent node is installed registry, err := ar.loadRegistry() @@ -42,6 +50,9 @@ func (ar *AgentNodeRunner) RunAgentNode(agentNodeName string) error { return fmt.Errorf("agent node %s is already running on port %d", agentNodeName, *agentNode.Runtime.Port) } + // 2b. Start declared node dependencies first (best-effort, in dep order). + ar.startNodeDependencies(agentNode, inProgress) + // 3. Allocate port fmt.Printf("🔍 Searching for available port...\n") port := ar.Port @@ -179,6 +190,38 @@ func (ar *AgentNodeRunner) startAgentNodeProcess(agentNode InstalledPackage, por return cmd, nil } +// startNodeDependencies starts any installed, not-yet-running node dependencies +// of the given node before the node itself. `inProgress` guards against cycles. +func (ar *AgentNodeRunner) startNodeDependencies(node InstalledPackage, inProgress map[string]bool) { + metadata, err := ParsePackageMetadata(node.Path) + if err != nil { + return + } + for _, ref := range metadata.Dependencies.Nodes { + depName := NodeDepName(ref) + if depName == "" || inProgress[depName] { + continue + } + registry, err := ar.loadRegistry() + if err != nil { + return + } + dep, exists := registry.Installed[depName] + if !exists { + fmt.Printf("⚠️ Node dependency %s is declared but not installed (run: af install %s)\n", depName, ref) + continue + } + if dep.Status == "running" { + continue + } + fmt.Printf("🔗 Starting node dependency: %s\n", depName) + depRunner := &AgentNodeRunner{AgentFieldHome: ar.AgentFieldHome} + if err := depRunner.runAgentNode(depName, inProgress); err != nil { + fmt.Printf("⚠️ Failed to start node dependency %s: %v\n", depName, err) + } + } +} + // venvPython returns the venv python interpreter path, or "" if no venv exists. func venvPython(venvPath string) string { if p := filepath.Join(venvPath, "bin", "python"); fileExists(p) { diff --git a/control-plane/internal/packages/secrets.go b/control-plane/internal/packages/secrets.go index 94a835a4d..064b397b6 100644 --- a/control-plane/internal/packages/secrets.go +++ b/control-plane/internal/packages/secrets.go @@ -30,6 +30,9 @@ const ( keyDirPerm = 0o700 ) +// GlobalScope is the exported name of the shared (cross-node) secret scope. +const GlobalScope = globalScope + // KeyProvider supplies the passphrase used to derive the at-rest encryption key. // It is an interface so we can add OS-keychain / passphrase providers later // without touching the SecretStore. From af729ebc8037daa25fa6cf9bbecec6a433cbd8ea Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 26 Jun 2026 11:05:10 -0400 Subject: [PATCH 3/7] docs: agent-node install/run/secrets guide - docs/installing-agent-nodes.md: full guide to af install/run, the agentfield-package.yaml manifest (entrypoint, node deps, user_environment), the encrypted runtime-only secrets model, and af secrets. - cli-toolkit.md reference: document af install, af run, af secrets (+ embedded skill_data copy synced). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../agentfield/references/cli-toolkit.md | 16 ++ .../agentfield/references/scaffold-recipe.md | 4 - docs/installing-agent-nodes.md | 141 ++++++++++++++++++ skills/agentfield/references/cli-toolkit.md | 16 ++ 4 files changed, 173 insertions(+), 4 deletions(-) create mode 100644 docs/installing-agent-nodes.md diff --git a/control-plane/internal/skillkit/skill_data/agentfield/references/cli-toolkit.md b/control-plane/internal/skillkit/skill_data/agentfield/references/cli-toolkit.md index 27c2510f9..88309cde6 100644 --- a/control-plane/internal/skillkit/skill_data/agentfield/references/cli-toolkit.md +++ b/control-plane/internal/skillkit/skill_data/agentfield/references/cli-toolkit.md @@ -92,6 +92,22 @@ Use `--default-model ` to bake the model from `af doctor` into the scaffo --- +## Installing and running agent nodes + +See [docs/installing-agent-nodes.md](../../../docs/installing-agent-nodes.md) for the full guide. + +### `af install ` + +Install an agent node from a local directory, a git/GitHub URL, or a registry name. The node is described by its `agentfield-package.yaml` manifest; its Python deps are installed into a per-node venv. If the manifest declares `dependencies.nodes` (e.g. `af://registry/swe-planner`), those nodes are installed recursively. + +### `af run ` + +Start an installed node in the background. Brings up the node's declared node dependencies first. Resolves the node's required environment from the encrypted secret store, **prompting once** for anything missing (hidden input for `type: secret`) and remembering it encrypted. Secrets are injected only into the node process — never written to disk in plaintext. Exports `AGENTFIELD_SERVER` + `PORT` to the node. + +### `af secrets set [VALUE]` / `af secrets ls` / `af secrets rm ` + +Manage the encrypted secret store under `~/.agentfield/secrets/` (AES-256-GCM, key in `~/.agentfield/keyring/master.key`, mode 0600). `set` with no value prompts hidden; `--node ` scopes a secret to one node (default is `global`, shared across all nodes). `ls` shows keys + scope only — values are never printed. + ## Running and observing ### `af list` diff --git a/control-plane/internal/skillkit/skill_data/agentfield/references/scaffold-recipe.md b/control-plane/internal/skillkit/skill_data/agentfield/references/scaffold-recipe.md index 693a9aead..590b608b6 100644 --- a/control-plane/internal/skillkit/skill_data/agentfield/references/scaffold-recipe.md +++ b/control-plane/internal/skillkit/skill_data/agentfield/references/scaffold-recipe.md @@ -245,10 +245,6 @@ services: AGENT_CALLBACK_URL: http://:8001 AGENT_NODE_ID: ${AGENT_NODE_ID:-} OPENROUTER_API_KEY: ${OPENROUTER_API_KEY:-} - AGENTFIELD_OPENROUTER_SITE_URL: ${AGENTFIELD_OPENROUTER_SITE_URL:-https://agentfield.ai} - AGENTFIELD_OPENROUTER_APP_NAME: ${AGENTFIELD_OPENROUTER_APP_NAME:-AgentField AI} - OR_SITE_URL: ${OR_SITE_URL:-https://agentfield.ai} - OR_APP_NAME: ${OR_APP_NAME:-AgentField AI} OPENAI_API_KEY: ${OPENAI_API_KEY:-} ANTHROPIC_API_KEY: ${ANTHROPIC_API_KEY:-} GOOGLE_API_KEY: ${GOOGLE_API_KEY:-} diff --git a/docs/installing-agent-nodes.md b/docs/installing-agent-nodes.md new file mode 100644 index 000000000..a611a49cb --- /dev/null +++ b/docs/installing-agent-nodes.md @@ -0,0 +1,141 @@ +# Installing & Running Agent Nodes + +`af install` and `af run` turn a published agent-node repository into a locally +running agent that connects to your control plane. Nodes are described by an +`agentfield-package.yaml` manifest, their dependencies (Python packages **and** +other agent nodes) are resolved automatically, and the secrets they need are +stored encrypted and injected only at runtime. + +```bash +# Install a node straight from GitHub (pulls in its node dependencies too) +af install https://github.com/Agent-Field/pr-af + +# Start it — you'll be prompted once for any missing required secrets +af run pr-af +``` + +Everything lives under `~/.agentfield/` (override with `AGENTFIELD_HOME`): + +``` +~/.agentfield/ +├── installed.yaml # registry of installed nodes + runtime state +├── packages// # the installed node + its Python venv +├── logs/.log # process logs (af logs ) +├── keyring/master.key # 0600 — local key that decrypts your secrets +└── secrets/ + ├── global.enc # secrets shared across all nodes (encrypted) + └── .enc # secrets scoped to one node (encrypted) +``` + +## The manifest: `agentfield-package.yaml` + +Every installable node has this file at its repo root. Only `name`, `version`, +and a way to start (either `entrypoint.start` or a top-level `main.py`) are +required; everything else is optional. + +```yaml +name: pr-af +version: 0.1.0 +description: Opens draft PRs from a task description +author: Agent-Field + +# How to launch the node. The first token is run inside the node's venv when it +# is "python"/"python3". Omit this only if the repo has a top-level main.py. +entrypoint: + start: python -m pr_af.app + healthcheck: /health # polled after launch to confirm readiness + +agent_node: + node_id: pr-af + default_port: 8004 + +dependencies: + python: # extra pip installs (requirements.txt is also honored) + - httpx>=0.27 + nodes: # other agent nodes this one calls — installed recursively + - af://registry/swe-planner + +# Variables the node needs. Required ones are prompted for on first run and +# remembered (encrypted). type: secret hides the input and stores it encrypted. +user_environment: + required: + - name: OPENROUTER_API_KEY + description: LLM provider key + type: secret + scope: global # global (default) = shared across nodes; node = this node only + optional: + - name: PR_AF_MODEL + description: Override the default model + default: openrouter/moonshotai/kimi-k2 +``` + +### Node dependencies + +`dependencies.nodes` lets one node declare that it needs others. Each entry is +an installable reference: + +| Reference | Resolves to | +| ------------------------------------------- | --------------------------------------------- | +| `af://registry/` | `https://github.com/Agent-Field/` | +| `af://registry/@` | same (version constraint is recorded, not yet enforced) | +| `https://github.com//` | used as-is | + +`af install ` installs the node **and** any declared node dependencies it +doesn't already have, recursively. Already-installed nodes are skipped, which +also breaks dependency cycles. `af run ` starts a node's installed +dependencies first (in dependency order) before the node itself. + +## Secrets: encrypted, shared, runtime-only + +Secrets are never written to disk in plaintext and never baked into the package. +They are encrypted with AES-256-GCM under a random 32-byte key kept in +`~/.agentfield/keyring/master.key` (mode `0600`). At start time they are decrypted +straight into the child process' environment — nowhere else. + +When `af run` needs a required variable, it resolves it in this order: + +1. **Process environment** — if `OPENROUTER_API_KEY` is already exported, it's + used as-is and **not** persisted. +2. **Node-scoped store** (`secrets/.enc`), then **global store** + (`secrets/global.enc`). +3. **Manifest `default`**. +4. **Prompt** (required variables only, when attached to a terminal). The value + is validated against the manifest `validation` regex, then saved encrypted to + the variable's scope. + +Because most provider keys are `scope: global`, you enter `OPENROUTER_API_KEY` +once and every node reuses it. A `scope: node` variable is stored per-node and +overrides the global value for that node. + +### Managing secrets directly + +```bash +af secrets set OPENROUTER_API_KEY # prompts, hidden input, stored global +af secrets set GH_TOKEN ghp_xxx # value inline +af secrets set PR_AF_MODEL ... --node pr-af # node-scoped override +af secrets ls # keys + scope only (values never shown) +af secrets rm GH_TOKEN # remove from global +af secrets rm PR_AF_MODEL --node pr-af # remove a node-scoped secret +``` + +In a non-interactive session (CI, no TTY), missing required secrets are reported +as an error listing the variable names instead of hanging on a prompt — set them +ahead of time with `af secrets set` or by exporting them. + +## Control-plane connection + +`af run` exports `AGENTFIELD_SERVER` (the variable the SDK reads) and the legacy +`AGENTFIELD_SERVER_URL`, plus the assigned `PORT`, into the node process. The +server URL is resolved from your local configuration. + +## Lifecycle reference + +| Command | Does | +| --------------------------- | -------------------------------------------------------------- | +| `af install ` | Install from a local path, git URL, or registry name + node deps | +| `af run ` | Start a node (and its node deps) in the background | +| `af list` | Show installed nodes and runtime state | +| `af logs ` | Tail a node's process log | +| `af stop ` | Stop a running node | +| `af uninstall ` | Stop and remove a node | +| `af secrets set/ls/rm` | Manage the encrypted secret store | diff --git a/skills/agentfield/references/cli-toolkit.md b/skills/agentfield/references/cli-toolkit.md index 27c2510f9..88309cde6 100644 --- a/skills/agentfield/references/cli-toolkit.md +++ b/skills/agentfield/references/cli-toolkit.md @@ -92,6 +92,22 @@ Use `--default-model ` to bake the model from `af doctor` into the scaffo --- +## Installing and running agent nodes + +See [docs/installing-agent-nodes.md](../../../docs/installing-agent-nodes.md) for the full guide. + +### `af install ` + +Install an agent node from a local directory, a git/GitHub URL, or a registry name. The node is described by its `agentfield-package.yaml` manifest; its Python deps are installed into a per-node venv. If the manifest declares `dependencies.nodes` (e.g. `af://registry/swe-planner`), those nodes are installed recursively. + +### `af run ` + +Start an installed node in the background. Brings up the node's declared node dependencies first. Resolves the node's required environment from the encrypted secret store, **prompting once** for anything missing (hidden input for `type: secret`) and remembering it encrypted. Secrets are injected only into the node process — never written to disk in plaintext. Exports `AGENTFIELD_SERVER` + `PORT` to the node. + +### `af secrets set [VALUE]` / `af secrets ls` / `af secrets rm ` + +Manage the encrypted secret store under `~/.agentfield/secrets/` (AES-256-GCM, key in `~/.agentfield/keyring/master.key`, mode 0600). `set` with no value prompts hidden; `--node ` scopes a secret to one node (default is `global`, shared across all nodes). `ls` shows keys + scope only — values are never printed. + ## Running and observing ### `af list` From af19bf69f2ad7afe76cecf6449cf0332a1eaf1c8 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 26 Jun 2026 12:04:29 -0400 Subject: [PATCH 4/7] fix(cli): apply install/run fixes to the active service layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Local end-to-end verification revealed the CLI's install/run path goes through internal/core/services (DefaultPackageService/DefaultAgentService), which duplicated — and so bypassed — the fixes previously made in internal/packages. 'af install' on an entrypoint-only node still failed with 'main.py not found', and 'af run' still exported only AGENTFIELD_SERVER_URL and loaded plaintext .env. - package_service: validate/parse/copy now delegate to the shared packages.ValidatePackage / ParsePackageMetadata / ShouldSkipCopy (entrypoint accepted, junk excluded). Install guidance points at 'af secrets set'. - agent_service: buildProcessConfig launches via the manifest entrypoint, exports AGENTFIELD_SERVER, resolves env via the encrypted secret store (prompting for missing required), honors the manifest healthcheck path, and drops the plaintext .env loader. RunAgent starts node deps first with a threaded cycle guard. - packages: export ValidatePackage + ShouldSkipCopy as the single source of truth. - tests updated to the new contract (entrypoint validation, store-based env injection instead of .env). Verified end-to-end: install entrypoint-only node -> missing-secret errors cleanly -> af secrets set -> af run injects AGENTFIELD_SERVER + the stored secret + manifest default into the process (confirmed via the node's env dump). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../cli/coverage_gap_additional_test.go | 35 ++-- control-plane/internal/cli/doctor.go | 36 ++-- .../internal/cli/framework/command_test.go | 8 +- .../internal/cli/framework/output_test.go | 28 +-- control-plane/internal/cli/skill.go | 14 +- .../internal/cli/vc_additional_test.go | 40 ++--- .../internal/core/services/agent_service.go | 161 ++++++++++++------ .../core/services/agent_service_test.go | 3 +- .../core/services/coverage_additional_test.go | 42 ++--- .../core/services/coverage_targeted_test.go | 6 +- .../internal/core/services/package_service.go | 50 ++---- .../core/services/package_service_test.go | 4 +- control-plane/internal/packages/installer.go | 20 ++- 13 files changed, 237 insertions(+), 210 deletions(-) diff --git a/control-plane/internal/cli/coverage_gap_additional_test.go b/control-plane/internal/cli/coverage_gap_additional_test.go index cd6169ea8..19ff3eeb0 100644 --- a/control-plane/internal/cli/coverage_gap_additional_test.go +++ b/control-plane/internal/cli/coverage_gap_additional_test.go @@ -219,10 +219,10 @@ func TestCLIExitHelper(t *testing.T) { _ = verifyVC(filepath.Join(t.TempDir(), "missing.json"), VerifyOptions{OutputFormat: "json"}) case "output-json-invalid": _ = outputJSON(VCVerificationResult{ - Valid: false, - Type: "credential", + Valid: false, + Type: "credential", FormatValid: false, - Message: "bad", + Message: "bad", }) case "output-pretty-invalid": _ = outputPretty(VCVerificationResult{ @@ -607,9 +607,9 @@ func TestEnhancedWorkflowVerificationBranches(t *testing.T) { t.Run("validate vc structure covers required fields", func(t *testing.T) { verifier := NewEnhancedVCVerifier(nil, false) cases := []struct { - name string - doc types.VCDocument - want string + name string + doc types.VCDocument + want string }{ {name: "missing context", doc: types.VCDocument{}, want: "missing @context"}, {name: "missing type", doc: types.VCDocument{Context: []string{"ctx"}}, want: "missing type"}, @@ -669,17 +669,17 @@ func signedWorkflowVCForTest(t *testing.T, issuer string, componentIDs []string) require.NoError(t, err) return types.WorkflowVC{ - WorkflowID: "wf-1", - ComponentVCs: componentIDs, - Status: "completed", - VCDocument: raw, - }, DIDResolutionInfo{ - DID: issuer, - Method: "key", - PublicKeyJWK: map[string]interface{}{ - "x": base64.RawURLEncoding.EncodeToString(publicKey), - }, - } + WorkflowID: "wf-1", + ComponentVCs: componentIDs, + Status: "completed", + VCDocument: raw, + }, DIDResolutionInfo{ + DID: issuer, + Method: "key", + PublicKeyJWK: map[string]interface{}{ + "x": base64.RawURLEncoding.EncodeToString(publicKey), + }, + } } func TestPrepareConfigFixtureShape(t *testing.T) { @@ -721,7 +721,6 @@ func withStreamingStdin(t *testing.T, chunks []string, fn func()) { <-done } - func TestProxyErrorOutputIncludesHint(t *testing.T) { output, err := runCLITestHelper(t, "proxy-http-error-with-default-error") require.Error(t, err) diff --git a/control-plane/internal/cli/doctor.go b/control-plane/internal/cli/doctor.go index ef87cbbc4..3e6038368 100644 --- a/control-plane/internal/cli/doctor.go +++ b/control-plane/internal/cli/doctor.go @@ -19,15 +19,15 @@ import ( // Coding agents (and skills like agentfield) call this once to learn // what's actually available in the environment instead of probing manually. type DoctorReport struct { - OS string `json:"os"` - Arch string `json:"arch"` - Python ToolStatus `json:"python"` - Node ToolStatus `json:"node"` - Docker ToolStatus `json:"docker"` + OS string `json:"os"` + Arch string `json:"arch"` + Python ToolStatus `json:"python"` + Node ToolStatus `json:"node"` + Docker ToolStatus `json:"docker"` HarnessProviders map[string]ToolStatus `json:"harness_providers"` - ProviderKeys map[string]ProviderKey `json:"provider_keys"` - ControlPlane ControlPlaneStatus `json:"control_plane"` - Recommendation Recommendation `json:"recommendation"` + ProviderKeys map[string]ProviderKey `json:"provider_keys"` + ControlPlane ControlPlaneStatus `json:"control_plane"` + Recommendation Recommendation `json:"recommendation"` } // ToolStatus describes whether a CLI is available and, if so, where. @@ -47,21 +47,21 @@ type ProviderKey struct { // ControlPlaneStatus reports whether a local control plane is reachable // and whether the Docker image is locally available. type ControlPlaneStatus struct { - URL string `json:"url"` - Reachable bool `json:"reachable"` - HealthStatus string `json:"health_status,omitempty"` - DockerImageName string `json:"docker_image_name"` - DockerImageLocal bool `json:"docker_image_local"` + URL string `json:"url"` + Reachable bool `json:"reachable"` + HealthStatus string `json:"health_status,omitempty"` + DockerImageName string `json:"docker_image_name"` + DockerImageLocal bool `json:"docker_image_local"` } // Recommendation tells the caller (a skill or a coding agent) what to default to, // based on what's actually present in the environment. type Recommendation struct { - Provider string `json:"provider"` // "openrouter" / "openai" / "anthropic" / "google" / "none" - AIModel string `json:"ai_model"` // suggested LiteLLM-style model string - HarnessUsable bool `json:"harness_usable"` // true only if at least one provider CLI is on PATH - HarnessProviders []string `json:"harness_providers"` // available provider CLI names - Notes []string `json:"notes"` // human-readable suggestions + Provider string `json:"provider"` // "openrouter" / "openai" / "anthropic" / "google" / "none" + AIModel string `json:"ai_model"` // suggested LiteLLM-style model string + HarnessUsable bool `json:"harness_usable"` // true only if at least one provider CLI is on PATH + HarnessProviders []string `json:"harness_providers"` // available provider CLI names + Notes []string `json:"notes"` // human-readable suggestions } // providerEnvVars maps provider name -> env var. Order matters for the recommendation. diff --git a/control-plane/internal/cli/framework/command_test.go b/control-plane/internal/cli/framework/command_test.go index a60598d0b..a0b897004 100644 --- a/control-plane/internal/cli/framework/command_test.go +++ b/control-plane/internal/cli/framework/command_test.go @@ -59,12 +59,12 @@ func TestCommandRegistryRegisterAndGetCommands(t *testing.T) { func TestCommandRegistryBuildCobraCommands(t *testing.T) { tests := []struct { - name string - commands []Command - wantUses []string + name string + commands []Command + wantUses []string }{ { - name: "empty registry", + name: "empty registry", wantUses: nil, }, { diff --git a/control-plane/internal/cli/framework/output_test.go b/control-plane/internal/cli/framework/output_test.go index 623213e23..340d240b2 100644 --- a/control-plane/internal/cli/framework/output_test.go +++ b/control-plane/internal/cli/framework/output_test.go @@ -60,33 +60,33 @@ func TestOutputFormatterPrintMethods(t *testing.T) { wantOut string }{ { - name: "success", - print: func(o *OutputFormatter) { o.PrintSuccess("done") }, + name: "success", + print: func(o *OutputFormatter) { o.PrintSuccess("done") }, wantOut: "✅ done\n", }, { - name: "error", - print: func(o *OutputFormatter) { o.PrintError("failed") }, + name: "error", + print: func(o *OutputFormatter) { o.PrintError("failed") }, wantOut: "❌ failed\n", }, { - name: "info", - print: func(o *OutputFormatter) { o.PrintInfo("details") }, + name: "info", + print: func(o *OutputFormatter) { o.PrintInfo("details") }, wantOut: "ℹ️ details\n", }, { - name: "warning", - print: func(o *OutputFormatter) { o.PrintWarning("careful") }, + name: "warning", + print: func(o *OutputFormatter) { o.PrintWarning("careful") }, wantOut: "⚠️ careful\n", }, { - name: "header", - print: func(o *OutputFormatter) { o.PrintHeader("title") }, + name: "header", + print: func(o *OutputFormatter) { o.PrintHeader("title") }, wantOut: "\n🧠 title\n", }, { - name: "progress", - print: func(o *OutputFormatter) { o.PrintProgress("working") }, + name: "progress", + print: func(o *OutputFormatter) { o.PrintProgress("working") }, wantOut: "⏳ working\n", }, { @@ -98,8 +98,8 @@ func TestOutputFormatterPrintMethods(t *testing.T) { wantOut: "🔍 trace\n", }, { - name: "verbose disabled", - print: func(o *OutputFormatter) { o.PrintVerbose("hidden") }, + name: "verbose disabled", + print: func(o *OutputFormatter) { o.PrintVerbose("hidden") }, wantOut: "", }, } diff --git a/control-plane/internal/cli/skill.go b/control-plane/internal/cli/skill.go index b6a37f58b..6c9ecd420 100644 --- a/control-plane/internal/cli/skill.go +++ b/control-plane/internal/cli/skill.go @@ -60,13 +60,13 @@ Examples: func newSkillInstallCommand() *cobra.Command { var ( - skillName string - version string - targets []string - allDetected bool - allTargets bool - force bool - dryRun bool + skillName string + version string + targets []string + allDetected bool + allTargets bool + force bool + dryRun bool nonInteractive bool ) diff --git a/control-plane/internal/cli/vc_additional_test.go b/control-plane/internal/cli/vc_additional_test.go index 4c1f098c8..d0a5017e1 100644 --- a/control-plane/internal/cli/vc_additional_test.go +++ b/control-plane/internal/cli/vc_additional_test.go @@ -327,26 +327,26 @@ func signedExecutionVC(t *testing.T, issuer string) (types.ExecutionVC, []byte, require.NoError(t, err) return types.ExecutionVC{ - VCID: "vc-1", - ExecutionID: "exec-1", - WorkflowID: "wf-1", - SessionID: "session-1", - IssuerDID: issuer, - TargetDID: "did:key:target", - CallerDID: "did:key:caller", - VCDocument: raw, - Signature: doc.Proof.ProofValue, - InputHash: "input-hash", - OutputHash: "output-hash", - Status: "completed", - CreatedAt: mustParseRFC3339(t, doc.IssuanceDate), - }, raw, DIDResolutionInfo{ - DID: issuer, - Method: "key", - PublicKeyJWK: map[string]interface{}{ - "x": base64.RawURLEncoding.EncodeToString(publicKey), - }, - } + VCID: "vc-1", + ExecutionID: "exec-1", + WorkflowID: "wf-1", + SessionID: "session-1", + IssuerDID: issuer, + TargetDID: "did:key:target", + CallerDID: "did:key:caller", + VCDocument: raw, + Signature: doc.Proof.ProofValue, + InputHash: "input-hash", + OutputHash: "output-hash", + Status: "completed", + CreatedAt: mustParseRFC3339(t, doc.IssuanceDate), + }, raw, DIDResolutionInfo{ + DID: issuer, + Method: "key", + PublicKeyJWK: map[string]interface{}{ + "x": base64.RawURLEncoding.EncodeToString(publicKey), + }, + } } func mustParseRFC3339(t *testing.T, value string) time.Time { diff --git a/control-plane/internal/core/services/agent_service.go b/control-plane/internal/core/services/agent_service.go index 6757b7b6a..ac8187664 100644 --- a/control-plane/internal/core/services/agent_service.go +++ b/control-plane/internal/core/services/agent_service.go @@ -49,7 +49,14 @@ func NewAgentService( // RunAgent starts an installed agent func (as *DefaultAgentService) RunAgent(name string, options domain.RunOptions) (*domain.RunningAgent, error) { + return as.runAgentGuarded(name, options, map[string]bool{}) +} + +// runAgentGuarded starts a node; inProgress tracks nodes already being started +// in this dependency chain to break cycles. +func (as *DefaultAgentService) runAgentGuarded(name string, options domain.RunOptions, inProgress map[string]bool) (*domain.RunningAgent, error) { fmt.Printf("🚀 Launching agent node: %s\n", name) + inProgress[name] = true // 1. Check if agent node is installed registry, err := as.loadRegistryDirect() @@ -93,16 +100,26 @@ func (as *DefaultAgentService) RunAgent(name string, options domain.RunOptions) fmt.Printf("✅ Assigned port: %d\n", port) + // 3b. Start declared node dependencies first (best-effort, dependency order). + as.startNodeDependencies(agentNode, inProgress, options) + // 4. Start agent node process fmt.Printf("📡 Starting agent node process...\n") - processConfig := as.buildProcessConfig(agentNode, port) + processConfig, err := as.buildProcessConfig(agentNode, port) + if err != nil { + return nil, err + } pid, err := as.processManager.Start(processConfig) if err != nil { return nil, fmt.Errorf("failed to start agent node: %w", err) } // 5. Wait for agent node to be ready - if err := as.waitForAgentNode(port, 10*time.Second); err != nil { + healthPath := "/health" + if metadata, err := packages.ParsePackageMetadata(agentNode.Path); err == nil { + healthPath = metadata.HealthcheckPath() + } + if err := as.waitForAgentNode(port, healthPath, 10*time.Second); err != nil { // Kill the process if it failed to start properly if stopErr := as.processManager.Stop(pid); stopErr != nil { return nil, fmt.Errorf("agent node failed to start: %w (additionally failed to stop process: %v)", err, stopErr) @@ -444,19 +461,68 @@ func (as *DefaultAgentService) convertToRunningAgent(pkg packages.InstalledPacka return agent } -// buildProcessConfig creates a process configuration for starting an agent -func (as *DefaultAgentService) buildProcessConfig(agentNode packages.InstalledPackage, port int) interfaces.ProcessConfig { - // Prepare environment variables +// startNodeDependencies starts a node's installed, not-yet-running node +// dependencies before the node itself. inProgress guards against cycles. +func (as *DefaultAgentService) startNodeDependencies(node packages.InstalledPackage, inProgress map[string]bool, options domain.RunOptions) { + metadata, err := packages.ParsePackageMetadata(node.Path) + if err != nil { + return + } + for _, ref := range metadata.Dependencies.Nodes { + depName := packages.NodeDepName(ref) + if depName == "" || inProgress[depName] { + continue + } + registry, err := as.loadRegistryDirect() + if err != nil { + return + } + dep, _, exists := as.findAgentInRegistry(registry, depName) + if !exists { + fmt.Printf("⚠️ Node dependency %s is declared but not installed (run: af install %s)\n", depName, ref) + continue + } + if running, _ := as.reconcileProcessState(&dep, depName); running { + continue + } + fmt.Printf("🔗 Starting node dependency: %s\n", depName) + // Dependencies get an auto-assigned port, not the parent's --port. + depOptions := options + depOptions.Port = 0 + if _, err := as.runAgentGuarded(depName, depOptions, inProgress); err != nil { + fmt.Printf("⚠️ Failed to start node dependency %s: %v\n", depName, err) + } + } +} + +// buildProcessConfig creates a process configuration for starting an agent. +// It reads the manifest entrypoint and resolves declared environment variables +// from the encrypted secret store (prompting for missing required ones). +func (as *DefaultAgentService) buildProcessConfig(agentNode packages.InstalledPackage, port int) (interfaces.ProcessConfig, error) { + // Read the manifest for the entrypoint, healthcheck and declared env. Fall + // back to defaults (python main.py) if no manifest is present. + metadata, err := packages.ParsePackageMetadata(agentNode.Path) + if err != nil { + fmt.Printf("⚠️ No usable manifest (%v); falling back to python main.py\n", err) + metadata = &packages.PackageMetadata{} + } + + // Prepare environment variables. Export both AGENTFIELD_SERVER (the var the + // SDK reads) and the legacy AGENTFIELD_SERVER_URL. + serverURL := resolveServerURL() env := os.Environ() env = append(env, fmt.Sprintf("PORT=%d", port)) - env = append(env, fmt.Sprintf("AGENTFIELD_SERVER_URL=%s", resolveServerURL())) + env = append(env, fmt.Sprintf("AGENTFIELD_SERVER=%s", serverURL)) + env = append(env, fmt.Sprintf("AGENTFIELD_SERVER_URL=%s", serverURL)) - // Load environment variables from package .env file - if envVars, err := as.loadPackageEnvFile(agentNode.Path); err == nil { - for key, value := range envVars { - env = append(env, fmt.Sprintf("%s=%s", key, value)) - } - fmt.Printf("🔧 Loaded %d environment variables from .env file\n", len(envVars)) + // Resolve declared variables from the encrypted secret store. Secrets are + // injected only into this child process — never written to disk in plaintext. + resolvedEnv, err := as.resolveNodeEnvironment(agentNode.Name, metadata) + if err != nil { + return interfaces.ProcessConfig{}, err + } + for key, value := range resolvedEnv { + env = append(env, fmt.Sprintf("%s=%s", key, value)) } // Determine Python path - use virtual environment if available @@ -516,22 +582,49 @@ func (as *DefaultAgentService) buildProcessConfig(agentNode packages.InstalledPa fmt.Printf("⚠️ Virtual environment not found at %s, using system Python: %s\n", venvPath, pythonPath) } + // Launch via the manifest entrypoint (e.g. "python -m pr_af.app"). When the + // program token is python/python3, substitute the resolved interpreter. + startArgs := metadata.StartCommand() + command := startArgs[0] + args := startArgs[1:] + if command == "python" || command == "python3" { + command = pythonPath + } + return interfaces.ProcessConfig{ - Command: pythonPath, - Args: []string{"main.py"}, + Command: command, + Args: args, Env: env, WorkDir: agentNode.Path, LogFile: agentNode.Runtime.LogFile, + }, nil +} + +// resolveNodeEnvironment resolves a node's declared variables via the encrypted +// secret store, prompting for missing required ones. +func (as *DefaultAgentService) resolveNodeEnvironment(nodeName string, metadata *packages.PackageMetadata) (map[string]string, error) { + env := metadata.UserEnvironment + if len(env.Required) == 0 && len(env.Optional) == 0 { + return map[string]string{}, nil + } + store, err := packages.NewSecretStore(as.agentfieldHome) + if err != nil { + return nil, fmt.Errorf("failed to open secret store: %w", err) } + resolver := &packages.EnvResolver{Store: store, NodeName: nodeName, Prompter: packages.TTYPrompter{}} + return resolver.Resolve(env) } // waitForAgentNode waits for the agent node to become ready -func (as *DefaultAgentService) waitForAgentNode(port int, timeout time.Duration) error { +func (as *DefaultAgentService) waitForAgentNode(port int, healthPath string, timeout time.Duration) error { + if healthPath == "" { + healthPath = "/health" + } client := &http.Client{Timeout: 1 * time.Second} deadline := time.Now().Add(timeout) for time.Now().Before(deadline) { - resp, err := client.Get(fmt.Sprintf("http://localhost:%d/health", port)) + resp, err := client.Get(fmt.Sprintf("http://localhost:%d%s", port, healthPath)) if err == nil && resp.StatusCode == 200 { resp.Body.Close() return nil @@ -662,42 +755,6 @@ func (as *DefaultAgentService) findAgentInRegistry(registry *packages.Installati return packages.InstalledPackage{}, "", false } -// loadPackageEnvFile loads environment variables from package .env file -func (as *DefaultAgentService) loadPackageEnvFile(packagePath string) (map[string]string, error) { - envPath := filepath.Join(packagePath, ".env") - - data, err := os.ReadFile(envPath) - if err != nil { - return nil, err - } - - envVars := make(map[string]string) - lines := strings.Split(string(data), "\n") - - for _, line := range lines { - line = strings.TrimSpace(line) - if line == "" || strings.HasPrefix(line, "#") { - continue - } - - parts := strings.SplitN(line, "=", 2) - if len(parts) == 2 { - key := strings.TrimSpace(parts[0]) - value := strings.TrimSpace(parts[1]) - - // Remove quotes if present - if (strings.HasPrefix(value, "\"") && strings.HasSuffix(value, "\"")) || - (strings.HasPrefix(value, "'") && strings.HasSuffix(value, "'")) { - value = value[1 : len(value)-1] - } - - envVars[key] = value - } - } - - return envVars, nil -} - // findPythonExecutable tries to find a suitable Python executable func (as *DefaultAgentService) findPythonExecutable() string { // Try common Python executable names in order of preference diff --git a/control-plane/internal/core/services/agent_service_test.go b/control-plane/internal/core/services/agent_service_test.go index d8438e2f3..5d19f7667 100644 --- a/control-plane/internal/core/services/agent_service_test.go +++ b/control-plane/internal/core/services/agent_service_test.go @@ -869,7 +869,8 @@ func TestBuildProcessConfig(t *testing.T) { tmpDir, ).(*DefaultAgentService) - config := service.buildProcessConfig(agentNode, 8001) + config, err := service.buildProcessConfig(agentNode, 8001) + require.NoError(t, err) // Check for any Python command (python, python3, or full path to python3) assert.True(t, config.Command == "python" || config.Command == "python3" || strings.Contains(config.Command, "python3"), diff --git a/control-plane/internal/core/services/coverage_additional_test.go b/control-plane/internal/core/services/coverage_additional_test.go index 0bd291d59..599b2fc58 100644 --- a/control-plane/internal/core/services/coverage_additional_test.go +++ b/control-plane/internal/core/services/coverage_additional_test.go @@ -61,45 +61,23 @@ func TestAgentServiceFindAgentInRegistry(t *testing.T) { }) } -func TestAgentServiceLoadPackageEnvFile(t *testing.T) { - dir := t.TempDir() - envPath := filepath.Join(dir, ".env") - require.NoError(t, os.WriteFile(envPath, []byte(strings.Join([]string{ - "# comment", - "FOO=bar", - `QUOTED="value with spaces"`, - "SINGLE='quoted'", - "INVALID_LINE", - "", - }, "\n")), 0o644)) - - service := &DefaultAgentService{} - envVars, err := service.loadPackageEnvFile(dir) - require.NoError(t, err) - assert.Equal(t, map[string]string{ - "FOO": "bar", - "QUOTED": "value with spaces", - "SINGLE": "quoted", - }, envVars) -} - func TestAgentServiceBuildProcessConfig(t *testing.T) { dir := t.TempDir() venvBin := filepath.Join(dir, "venv", "bin") require.NoError(t, os.MkdirAll(venvBin, 0o755)) pythonPath := filepath.Join(venvBin, "python") require.NoError(t, os.WriteFile(pythonPath, []byte("#!/bin/sh\nexit 0\n"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(dir, ".env"), []byte("TOKEN=secret\n"), 0o644)) logFile := filepath.Join(dir, "agent.log") service := &DefaultAgentService{} - cfg := service.buildProcessConfig(packages.InstalledPackage{ + cfg, err := service.buildProcessConfig(packages.InstalledPackage{ Name: "agent", Path: dir, Runtime: packages.RuntimeInfo{ LogFile: logFile, }, }, 8123) + require.NoError(t, err) assert.Equal(t, pythonPath, cfg.Command) assert.Equal(t, []string{"main.py"}, cfg.Args) @@ -107,7 +85,7 @@ func TestAgentServiceBuildProcessConfig(t *testing.T) { assert.Equal(t, logFile, cfg.LogFile) assert.Contains(t, cfg.Env, "PORT=8123") assert.Contains(t, cfg.Env, "AGENTFIELD_SERVER_URL=http://localhost:8080") - assert.Contains(t, cfg.Env, "TOKEN=secret") + assert.Contains(t, cfg.Env, "AGENTFIELD_SERVER=http://localhost:8080") assert.Contains(t, cfg.Env, "VIRTUAL_ENV="+filepath.Join(dir, "venv")) assertEnvWithPrefix(t, cfg.Env, "PATH=", venvBin) assert.Contains(t, cfg.Env, "PYTHONHOME=") @@ -126,13 +104,13 @@ func TestAgentServiceWaitForAgentNode(t *testing.T) { defer server.Close() service := &DefaultAgentService{} - require.NoError(t, service.waitForAgentNode(port, 2*time.Second)) + require.NoError(t, service.waitForAgentNode(port, "/health", 2*time.Second)) }) t.Run("timeout", func(t *testing.T) { port := findFreePortInRange(t) service := &DefaultAgentService{} - err := service.waitForAgentNode(port, 750*time.Millisecond) + err := service.waitForAgentNode(port, "/health", 750*time.Millisecond) require.Error(t, err) assert.Contains(t, err.Error(), "did not become ready") }) @@ -207,7 +185,7 @@ func TestPackageServiceHelpers(t *testing.T) { require.NoError(t, os.WriteFile(filepath.Join(dir, "agentfield-package.yaml"), []byte("name: sample\nversion: 1.0.0\nmain: main.py\n"), 0o644)) err := service.validatePackage(dir) require.Error(t, err) - assert.Contains(t, err.Error(), "main.py not found") + assert.Contains(t, err.Error(), "main.py") require.NoError(t, os.WriteFile(filepath.Join(dir, "main.py"), []byte("print('ok')\n"), 0o644)) require.NoError(t, service.validatePackage(dir)) @@ -433,7 +411,13 @@ func TestAgentServiceRunStopAndStatusWithLiveProcess(t *testing.T) { venvBin := filepath.Join(agentPath, "venv", "bin") require.NoError(t, os.MkdirAll(venvBin, 0o755)) require.NoError(t, os.WriteFile(filepath.Join(venvBin, "python"), []byte("#!/bin/sh\nexit 0\n"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(agentPath, ".env"), []byte("TOKEN=run-secret\n"), 0o644)) + // Declare TOKEN in the manifest and seed it into the encrypted secret + // store; af run injects it into the process env at start time. + require.NoError(t, os.WriteFile(filepath.Join(agentPath, "agentfield-package.yaml"), + []byte("name: run-agent\nversion: 1.0.0\nuser_environment:\n optional:\n - name: TOKEN\n type: secret\n"), 0o644)) + store, err := packages.NewSecretStore(home) + require.NoError(t, err) + require.NoError(t, store.Set("run-agent", "TOKEN", "run-secret")) registry := &packages.InstallationRegistry{ Installed: map[string]packages.InstalledPackage{ diff --git a/control-plane/internal/core/services/coverage_targeted_test.go b/control-plane/internal/core/services/coverage_targeted_test.go index 2505bd239..684df6d22 100644 --- a/control-plane/internal/core/services/coverage_targeted_test.go +++ b/control-plane/internal/core/services/coverage_targeted_test.go @@ -499,13 +499,14 @@ func TestAgentServiceBuildProcessConfigAdditionalCoverage(t *testing.T) { require.NoError(t, os.WriteFile(pythonPath, []byte("stub"), 0o755)) service := &DefaultAgentService{} - cfg := service.buildProcessConfig(packages.InstalledPackage{ + cfg, err := service.buildProcessConfig(packages.InstalledPackage{ Name: "windows-agent", Path: dir, Runtime: packages.RuntimeInfo{ LogFile: filepath.Join(dir, "agent.log"), }, }, 8141) + require.NoError(t, err) assert.Equal(t, pythonPath, cfg.Command) assert.Contains(t, cfg.Env, "VIRTUAL_ENV="+filepath.Join(dir, "venv")) @@ -525,10 +526,11 @@ func TestAgentServiceBuildProcessConfigAdditionalCoverage(t *testing.T) { service := &DefaultAgentService{} assert.Equal(t, fakePython, service.findPythonExecutable()) - cfg := service.buildProcessConfig(packages.InstalledPackage{ + cfg, err := service.buildProcessConfig(packages.InstalledPackage{ Name: "fallback-agent", Path: dir, }, 8142) + require.NoError(t, err) assert.Equal(t, fakePython, cfg.Command) }) diff --git a/control-plane/internal/core/services/package_service.go b/control-plane/internal/core/services/package_service.go index 817152049..874c8852a 100644 --- a/control-plane/internal/core/services/package_service.go +++ b/control-plane/internal/core/services/package_service.go @@ -460,47 +460,12 @@ func (s *Spinner) Error(message string) { // validatePackage checks if the package has required files func (ps *DefaultPackageService) validatePackage(sourcePath string) error { - // Check if agentfield-package.yaml exists - packageYamlPath := filepath.Join(sourcePath, "agentfield-package.yaml") - if _, err := os.Stat(packageYamlPath); os.IsNotExist(err) { - return fmt.Errorf("agentfield-package.yaml not found in %s", sourcePath) - } - - // Check if main.py exists - mainPyPath := filepath.Join(sourcePath, "main.py") - if _, err := os.Stat(mainPyPath); os.IsNotExist(err) { - return fmt.Errorf("main.py not found in %s", sourcePath) - } - - return nil + return packages.ValidatePackage(sourcePath) } // parsePackageMetadata parses the agentfield-package.yaml file func (ps *DefaultPackageService) parsePackageMetadata(sourcePath string) (*packages.PackageMetadata, error) { - packageYamlPath := filepath.Join(sourcePath, "agentfield-package.yaml") - - data, err := os.ReadFile(packageYamlPath) - if err != nil { - return nil, fmt.Errorf("failed to read agentfield-package.yaml: %w", err) - } - - var metadata packages.PackageMetadata - if err := yaml.Unmarshal(data, &metadata); err != nil { - return nil, fmt.Errorf("failed to parse agentfield-package.yaml: %w", err) - } - - // Validate required fields - if metadata.Name == "" { - return nil, fmt.Errorf("package name is required in agentfield-package.yaml") - } - if metadata.Version == "" { - return nil, fmt.Errorf("package version is required in agentfield-package.yaml") - } - if metadata.Main == "" { - metadata.Main = "main.py" // Default - } - - return &metadata, nil + return packages.ParsePackageMetadata(sourcePath) } // isPackageInstalled checks if a package is already installed @@ -544,6 +509,14 @@ func (ps *DefaultPackageService) copyPackage(sourcePath, destPath string) error return err } + // Skip VCS, build artifacts, local venvs and plaintext secrets. + if packages.ShouldSkipCopy(relPath, info) { + if info.IsDir() { + return filepath.SkipDir + } + return nil + } + destFilePath := filepath.Join(destPath, relPath) if info.IsDir() { @@ -706,8 +679,9 @@ func (ps *DefaultPackageService) checkEnvironmentVariables(metadata *packages.Pa if len(missingRequired) > 0 { fmt.Printf("\n%s %s\n", ps.yellow("⚠"), ps.bold("Missing required environment variables:")) for _, envVar := range missingRequired { - fmt.Printf(" %s\n", ps.cyan(fmt.Sprintf("af config %s --set %s=your-value-here", metadata.Name, envVar.Name))) + fmt.Printf(" %s\n", ps.cyan(fmt.Sprintf("af secrets set %s", envVar.Name))) } + fmt.Printf(" %s\n", ps.gray("(or you'll be prompted on first 'af run')")) } // Show optional environment variables if any diff --git a/control-plane/internal/core/services/package_service_test.go b/control-plane/internal/core/services/package_service_test.go index e61eb691f..610bcbc13 100644 --- a/control-plane/internal/core/services/package_service_test.go +++ b/control-plane/internal/core/services/package_service_test.go @@ -443,7 +443,7 @@ func TestValidatePackage_Success(t *testing.T) { require.NoError(t, os.MkdirAll(packagePath, 0755)) packageYamlPath := filepath.Join(packagePath, "agentfield-package.yaml") - require.NoError(t, os.WriteFile(packageYamlPath, []byte("name: test"), 0644)) + require.NoError(t, os.WriteFile(packageYamlPath, []byte("name: test\nversion: 1.0.0\n"), 0644)) mainPyPath := filepath.Join(packagePath, "main.py") require.NoError(t, os.WriteFile(mainPyPath, []byte("# test"), 0644)) @@ -480,7 +480,7 @@ func TestValidatePackage_MissingMainPy(t *testing.T) { require.NoError(t, os.MkdirAll(packagePath, 0755)) packageYamlPath := filepath.Join(packagePath, "agentfield-package.yaml") - require.NoError(t, os.WriteFile(packageYamlPath, []byte("name: test"), 0644)) + require.NoError(t, os.WriteFile(packageYamlPath, []byte("name: test\nversion: 1.0.0\n"), 0644)) registryStorage := newMockPackageRegistryStorage() fileSystem := newMockFileSystemAdapter() diff --git a/control-plane/internal/packages/installer.go b/control-plane/internal/packages/installer.go index c406d7d85..a9de9d3cd 100644 --- a/control-plane/internal/packages/installer.go +++ b/control-plane/internal/packages/installer.go @@ -414,17 +414,21 @@ func (pu *PackageUninstaller) saveRegistry(registry *InstallationRegistry) error return nil } -// validatePackage checks if the package has required files +// validatePackage checks if the package has required files. func (pi *PackageInstaller) validatePackage(sourcePath string) error { - // Check if agentfield-package.yaml exists + return ValidatePackage(sourcePath) +} + +// ValidatePackage checks that a directory is an installable agent node: it must +// have an agentfield-package.yaml and declare how to start — either a manifest +// entrypoint.start (e.g. "python -m pr_af.app") or a top-level main.py. Real +// nodes use a module entrypoint and have no main.py, so main.py is not required. +func ValidatePackage(sourcePath string) error { packageYamlPath := filepath.Join(sourcePath, "agentfield-package.yaml") if _, err := os.Stat(packageYamlPath); os.IsNotExist(err) { return fmt.Errorf("agentfield-package.yaml not found in %s", sourcePath) } - // A node must declare how to start: either a manifest entrypoint.start - // (e.g. "python -m pr_af.app") or a top-level main.py. Real nodes use a - // module entrypoint and have no main.py, so we no longer require it. metadata, err := ParsePackageMetadata(sourcePath) if err != nil { return err @@ -587,6 +591,12 @@ var copyExcludedNames = map[string]bool{ ".pytest_cache": true, } +// ShouldSkipCopy reports whether a walked path should be excluded when copying +// a package into ~/.agentfield/packages (VCS, venvs, caches, plaintext secrets). +func ShouldSkipCopy(relPath string, info os.FileInfo) bool { + return shouldSkipCopy(relPath, info) +} + // shouldSkipCopy reports whether a walked path should be excluded from the copy. func shouldSkipCopy(relPath string, info os.FileInfo) bool { if relPath == "." { From 1d3298accc41578434179b68eb8d9125104645b1 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 26 Jun 2026 13:21:55 -0400 Subject: [PATCH 5/7] fix(cli): start node dependencies before allocating the parent's port Local multi-agent verification showed a port collision: dependencies were started after the parent allocated its port, so the parent's port (not yet bound) was handed out again to a dependency, which then failed to bind. Move dependency startup ahead of port allocation so each dependency fully binds its own port first. Verified end-to-end against a live local control plane: 'af run greeter-node' auto-starts its dependency echo-node (distinct ports 8002/8003), both register, both reasoners execute through the control plane, and an already-running dependency is left untouched (same PID). Co-Authored-By: Claude Opus 4.8 (1M context) --- control-plane/internal/core/services/agent_service.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/control-plane/internal/core/services/agent_service.go b/control-plane/internal/core/services/agent_service.go index ac8187664..7dfeb5829 100644 --- a/control-plane/internal/core/services/agent_service.go +++ b/control-plane/internal/core/services/agent_service.go @@ -88,6 +88,10 @@ func (as *DefaultAgentService) runAgentGuarded(name string, options domain.RunOp return nil, fmt.Errorf("agent node %s is already running on port %d", name, *agentNode.Runtime.Port) } + // 2b. Start declared node dependencies first, before allocating this node's + // port — each dependency fully binds its own port, avoiding collisions. + as.startNodeDependencies(agentNode, inProgress, options) + // 3. Allocate port fmt.Printf("🔍 Searching for available port...\n") port := options.Port @@ -100,9 +104,6 @@ func (as *DefaultAgentService) runAgentGuarded(name string, options domain.RunOp fmt.Printf("✅ Assigned port: %d\n", port) - // 3b. Start declared node dependencies first (best-effort, dependency order). - as.startNodeDependencies(agentNode, inProgress, options) - // 4. Start agent node process fmt.Printf("📡 Starting agent node process...\n") processConfig, err := as.buildProcessConfig(agentNode, port) From 538313cb89291a8900d75ca32fb5552d1f2c7fdc Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 26 Jun 2026 14:02:28 -0400 Subject: [PATCH 6/7] test(cli): cover node-dependency resolution and ordered-start helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Unit tests for resolveNodeRef, installedNames, installNodeDependencies (skip-already-installed), and startNodeDependencies (not-installed warning + already-running skip) in both the service and packages layers — covering the new patch lines and pinning the behaviors verified end-to-end. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../internal/core/services/node_deps_test.go | 126 ++++++++++++++++++ .../internal/packages/node_deps_test.go | 33 +++++ 2 files changed, 159 insertions(+) create mode 100644 control-plane/internal/core/services/node_deps_test.go create mode 100644 control-plane/internal/packages/node_deps_test.go diff --git a/control-plane/internal/core/services/node_deps_test.go b/control-plane/internal/core/services/node_deps_test.go new file mode 100644 index 000000000..a7b72558a --- /dev/null +++ b/control-plane/internal/core/services/node_deps_test.go @@ -0,0 +1,126 @@ +package services + +import ( + "os" + "path/filepath" + "testing" + + "github.com/Agent-Field/agentfield/control-plane/internal/core/domain" + "github.com/Agent-Field/agentfield/control-plane/internal/packages" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" +) + +// Contract: an af://registry ref resolves to the Agent-Field GitHub repo and +// yields the package name; any other ref is used as-is with an empty name. +func TestResolveNodeRef(t *testing.T) { + cases := []struct { + ref string + wantSource string + wantName string + }{ + {"af://registry/swe-planner", "https://github.com/Agent-Field/swe-planner", "swe-planner"}, + {"af://registry/pr-af@^1.0", "https://github.com/Agent-Field/pr-af", "pr-af"}, + {"https://github.com/acme/widget", "https://github.com/acme/widget", ""}, + } + for _, tc := range cases { + src, name := resolveNodeRef(tc.ref) + assert.Equal(t, tc.wantSource, src, "source for %s", tc.ref) + assert.Equal(t, tc.wantName, name, "name for %s", tc.ref) + } +} + +func writeRegistry(t *testing.T, home string, reg *packages.InstallationRegistry) { + t.Helper() + data, err := yaml.Marshal(reg) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(home, "installed.yaml"), data, 0o644)) +} + +func writeManifest(t *testing.T, dir, body string) { + t.Helper() + require.NoError(t, os.MkdirAll(dir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "agentfield-package.yaml"), []byte(body), 0o644)) +} + +// Contract: installedNames reflects the registry contents. +func TestInstalledNames(t *testing.T) { + home := t.TempDir() + ps := &DefaultPackageService{agentfieldHome: home} + assert.Empty(t, ps.installedNames()) + + writeRegistry(t, home, &packages.InstallationRegistry{ + Installed: map[string]packages.InstalledPackage{"a": {Name: "a"}, "b": {Name: "b"}}, + }) + names := ps.installedNames() + assert.True(t, names["a"] && names["b"]) +} + +// Contract: a node dependency that is already installed is not re-installed +// (which also breaks cycles) — installNodeDependencies returns without error. +func TestInstallNodeDependencies_SkipsAlreadyInstalled(t *testing.T) { + home := t.TempDir() + ps := &DefaultPackageService{agentfieldHome: home} + + callerDir := filepath.Join(home, "packages", "caller") + writeManifest(t, callerDir, "name: caller\nversion: 1.0.0\ndependencies:\n nodes:\n - af://registry/echo-node\n") + echoDir := filepath.Join(home, "packages", "echo-node") + writeManifest(t, echoDir, "name: echo-node\nversion: 1.0.0\n") + + writeRegistry(t, home, &packages.InstallationRegistry{ + Installed: map[string]packages.InstalledPackage{ + "caller": {Name: "caller", Path: callerDir}, + "echo-node": {Name: "echo-node", Path: echoDir}, + }, + }) + + // `before` is empty, so both packages count as newly installed; the declared + // dep (echo-node) is already installed and must be skipped — no network call. + err := ps.installNodeDependencies(map[string]bool{}, domain.InstallOptions{}) + require.NoError(t, err) +} + +// Contract: starting a node whose declared dependency is not installed logs a +// warning and continues, without attempting to run anything. +func TestStartNodeDependencies_NotInstalledWarns(t *testing.T) { + home := t.TempDir() + as := &DefaultAgentService{agentfieldHome: home} + + nodeDir := filepath.Join(home, "packages", "parent") + writeManifest(t, nodeDir, "name: parent\nversion: 1.0.0\ndependencies:\n nodes:\n - af://registry/absent-node\n") + writeRegistry(t, home, &packages.InstallationRegistry{Installed: map[string]packages.InstalledPackage{}}) + + node := packages.InstalledPackage{Name: "parent", Path: nodeDir} + // Must not panic despite a nil process/port manager — the dep is absent so + // the run path is never reached. + as.startNodeDependencies(node, map[string]bool{"parent": true}, domain.RunOptions{}) +} + +// Contract: an already-running dependency is left alone (not restarted). +func TestStartNodeDependencies_SkipsRunningDep(t *testing.T) { + home := t.TempDir() + as := &DefaultAgentService{agentfieldHome: home} + + parentDir := filepath.Join(home, "packages", "parent") + writeManifest(t, parentDir, "name: parent\nversion: 1.0.0\ndependencies:\n nodes:\n - af://registry/dep-node\n") + depDir := filepath.Join(home, "packages", "dep-node") + writeManifest(t, depDir, "name: dep-node\nversion: 1.0.0\n") + + pid := os.Getpid() // a live process, so reconcile considers it running + port := 8123 + writeRegistry(t, home, &packages.InstallationRegistry{ + Installed: map[string]packages.InstalledPackage{ + "dep-node": { + Name: "dep-node", + Path: depDir, + Status: "running", + Runtime: packages.RuntimeInfo{PID: &pid, Port: &port}, + }, + }, + }) + + node := packages.InstalledPackage{Name: "parent", Path: parentDir} + // dep-node is running → skipped before the run path; nil managers never used. + as.startNodeDependencies(node, map[string]bool{"parent": true}, domain.RunOptions{}) +} diff --git a/control-plane/internal/packages/node_deps_test.go b/control-plane/internal/packages/node_deps_test.go new file mode 100644 index 000000000..ebf1da2b6 --- /dev/null +++ b/control-plane/internal/packages/node_deps_test.go @@ -0,0 +1,33 @@ +package packages + +import ( + "os" + "path/filepath" + "testing" + + "gopkg.in/yaml.v3" +) + +// Contract: starting a node whose declared dependency is not installed warns +// and continues without panicking or starting anything. +func TestRunner_StartNodeDependencies_NotInstalled(t *testing.T) { + home := t.TempDir() + nodeDir := filepath.Join(home, "packages", "parent") + if err := os.MkdirAll(nodeDir, 0o755); err != nil { + t.Fatal(err) + } + manifest := "name: parent\nversion: 1.0.0\ndependencies:\n nodes:\n - af://registry/absent\n" + if err := os.WriteFile(filepath.Join(nodeDir, "agentfield-package.yaml"), []byte(manifest), 0o644); err != nil { + t.Fatal(err) + } + reg := &InstallationRegistry{Installed: map[string]InstalledPackage{}} + data, _ := yaml.Marshal(reg) + if err := os.WriteFile(filepath.Join(home, "installed.yaml"), data, 0o644); err != nil { + t.Fatal(err) + } + + ar := &AgentNodeRunner{AgentFieldHome: home} + node := InstalledPackage{Name: "parent", Path: nodeDir} + // Absent dependency → warns and returns; must not panic or recurse into run. + ar.startNodeDependencies(node, map[string]bool{"parent": true}) +} From c63f66059a63b69ef80f34450b4d1882adf3714e Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 26 Jun 2026 15:14:26 -0400 Subject: [PATCH 7/7] fix(cli): git/github install validation + pyproject dependency install MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit End-to-end install testing against the published node repos surfaced two gaps: 1. The git and GitHub install paths (git.go/github.go findPackageRoot) were a third and fourth copy of the 'main.py required' check, so 'af install ' failed for entrypoint-only nodes (no top-level main.py) such as SWE-AF and cloudsecurity-af. Both now delegate to the shared ValidatePackage (accepts a manifest entrypoint.start). 2. Dependency install only ran for requirements.txt projects, so pyproject-only nodes (pr-af, sec-af, cloudsecurity-af) installed with no venv and no deps. Dependency install is now a single shared InstallPythonDependencies that also runs 'pip install .' for pyproject.toml/setup.py projects. Verified: all five published node repos now install from their GitHub URLs; a pyproject node (sec-af) builds its venv and 'pip install .' succeeds, with sec_af + agentfield importable from the node's venv. (Nodes that declare requires-python >=3.11 need a matching interpreter on PATH — pip reports this clearly.) Tests updated for the new validation contract; new unit tests cover the pyproject branch and entrypoint-accepting findPackageRoot. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../internal/core/services/package_service.go | 56 +---------- .../internal/packages/coverage_boost_test.go | 2 +- control-plane/internal/packages/git.go | 8 +- .../internal/packages/git_additional_test.go | 2 +- control-plane/internal/packages/git_test.go | 4 +- control-plane/internal/packages/github.go | 8 +- .../internal/packages/github_test.go | 4 +- .../internal/packages/install_deps_test.go | 92 +++++++++++++++++++ control-plane/internal/packages/installer.go | 66 ++++++++----- docs/installing-agent-nodes.md | 13 +++ 10 files changed, 161 insertions(+), 94 deletions(-) create mode 100644 control-plane/internal/packages/install_deps_test.go diff --git a/control-plane/internal/core/services/package_service.go b/control-plane/internal/core/services/package_service.go index 874c8852a..1c744df93 100644 --- a/control-plane/internal/core/services/package_service.go +++ b/control-plane/internal/core/services/package_service.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "os" - "os/exec" "path/filepath" "strings" "sync" @@ -548,60 +547,7 @@ func (ps *DefaultPackageService) copyFile(src, dst string) error { // installDependencies installs package dependencies func (ps *DefaultPackageService) installDependencies(packagePath string, metadata *packages.PackageMetadata) error { - // Install Python dependencies in a virtual environment - if len(metadata.Dependencies.Python) > 0 || ps.hasRequirementsFile(packagePath) { - // Create virtual environment - venvPath := filepath.Join(packagePath, "venv") - - cmd := exec.Command("python3", "-m", "venv", venvPath) - if _, err := cmd.CombinedOutput(); err != nil { - // Try with python if python3 fails - cmd = exec.Command("python", "-m", "venv", venvPath) - if output, err := cmd.CombinedOutput(); err != nil { - return fmt.Errorf("failed to create virtual environment: %w\nOutput: %s", err, output) - } - } - - // Determine pip path - var pipPath string - if _, err := os.Stat(filepath.Join(venvPath, "bin", "pip")); err == nil { - pipPath = filepath.Join(venvPath, "bin", "pip") - } else { - pipPath = filepath.Join(venvPath, "Scripts", "pip.exe") // Windows - } - - // Upgrade pip first (ignore failures) - cmd = exec.Command(pipPath, "install", "--upgrade", "pip") - _, _ = cmd.CombinedOutput() - - // Install from requirements.txt if it exists - requirementsPath := filepath.Join(packagePath, "requirements.txt") - if _, err := os.Stat(requirementsPath); err == nil { - cmd = exec.Command(pipPath, "install", "-r", requirementsPath) - cmd.Dir = packagePath - if output, err := cmd.CombinedOutput(); err != nil { - return fmt.Errorf("failed to install requirements.txt dependencies: %w\nOutput: %s", err, output) - } - } - - // Install dependencies from agentfield-package.yaml - if len(metadata.Dependencies.Python) > 0 { - for _, dep := range metadata.Dependencies.Python { - cmd = exec.Command(pipPath, "install", dep) - cmd.Dir = packagePath - if output, err := cmd.CombinedOutput(); err != nil { - return fmt.Errorf("failed to install dependency %s: %w\nOutput: %s", dep, err, output) - } - } - } - } - - // Install system dependencies (if any) - for _, dep := range metadata.Dependencies.System { - fmt.Printf("System dependency required: %s (please install manually)\n", dep) - } - - return nil + return packages.InstallPythonDependencies(packagePath, metadata.Dependencies.Python, metadata.Dependencies.System) } // hasRequirementsFile checks if requirements.txt exists diff --git a/control-plane/internal/packages/coverage_boost_test.go b/control-plane/internal/packages/coverage_boost_test.go index 21ab8cd60..dfe9acf4a 100644 --- a/control-plane/internal/packages/coverage_boost_test.go +++ b/control-plane/internal/packages/coverage_boost_test.go @@ -298,7 +298,7 @@ func TestGitHubInstallerAdditionalErrorCoverage(t *testing.T) { } err = gi.InstallFromGitHub("acme/repo@main", true) - if err == nil || !strings.Contains(err.Error(), "failed to parse package metadata") { + if err == nil || !strings.Contains(err.Error(), "version is required") { t.Fatalf("InstallFromGitHub error = %v", err) } }) diff --git a/control-plane/internal/packages/git.go b/control-plane/internal/packages/git.go index 3a4314471..53156c329 100644 --- a/control-plane/internal/packages/git.go +++ b/control-plane/internal/packages/git.go @@ -288,10 +288,10 @@ func (gi *GitInstaller) findPackageRoot(cloneDir string) (string, error) { return "", fmt.Errorf("agentfield-package.yaml not found in the repository") } - // Also check for main.py - mainPyPath := filepath.Join(packageRoot, "main.py") - if _, err := os.Stat(mainPyPath); os.IsNotExist(err) { - return "", fmt.Errorf("main.py not found in package root") + // The node must declare how to start: a manifest entrypoint.start or a + // top-level main.py. Real nodes use a module entrypoint and have no main.py. + if err := ValidatePackage(packageRoot); err != nil { + return "", err } return packageRoot, nil diff --git a/control-plane/internal/packages/git_additional_test.go b/control-plane/internal/packages/git_additional_test.go index ce2d8965d..2966d917e 100644 --- a/control-plane/internal/packages/git_additional_test.go +++ b/control-plane/internal/packages/git_additional_test.go @@ -167,7 +167,7 @@ func TestGitInstallerInstallFromGitAdditionalCoverage(t *testing.T) { gi := &GitInstaller{AgentFieldHome: home} err := gi.InstallFromGit("https://gitlab.com/acme/repo", true) - if err == nil || !strings.Contains(err.Error(), "failed to parse package metadata") { + if err == nil || !strings.Contains(err.Error(), "version is required") { t.Fatalf("InstallFromGit error = %v", err) } }) diff --git a/control-plane/internal/packages/git_test.go b/control-plane/internal/packages/git_test.go index 74aa5e7f1..dbee2bc5f 100644 --- a/control-plane/internal/packages/git_test.go +++ b/control-plane/internal/packages/git_test.go @@ -77,8 +77,8 @@ func TestGitInstallerFindPackageRootAndRegistry(t *testing.T) { if err := os.WriteFile(filepath.Join(missingMain, "agentfield-package.yaml"), []byte("name: bad\nversion: 1.0.0\n"), 0644); err != nil { t.Fatalf("write yaml: %v", err) } - if _, err := gi.findPackageRoot(missingMain); err == nil || !strings.Contains(err.Error(), "main.py not found") { - t.Fatalf("expected main.py error, got %v", err) + if _, err := gi.findPackageRoot(missingMain); err == nil || !strings.Contains(err.Error(), "main.py") { + t.Fatalf("expected entrypoint/main.py error, got %v", err) } emptyRepo := t.TempDir() diff --git a/control-plane/internal/packages/github.go b/control-plane/internal/packages/github.go index 8a234d51c..e9879beef 100644 --- a/control-plane/internal/packages/github.go +++ b/control-plane/internal/packages/github.go @@ -341,10 +341,10 @@ func (gi *GitHubInstaller) findPackageRoot(extractDir string) (string, error) { return "", fmt.Errorf("agentfield-package.yaml not found in the repository") } - // Also check for main.py - mainPyPath := filepath.Join(packageRoot, "main.py") - if _, err := os.Stat(mainPyPath); os.IsNotExist(err) { - return "", fmt.Errorf("main.py not found in package root") + // The node must declare how to start: a manifest entrypoint.start or a + // top-level main.py. Real nodes use a module entrypoint and have no main.py. + if err := ValidatePackage(packageRoot); err != nil { + return "", err } return packageRoot, nil diff --git a/control-plane/internal/packages/github_test.go b/control-plane/internal/packages/github_test.go index 508035368..786485af9 100644 --- a/control-plane/internal/packages/github_test.go +++ b/control-plane/internal/packages/github_test.go @@ -181,8 +181,8 @@ func TestGitHubInstallerExtractZipRejectsTraversal(t *testing.T) { if err := gi.extractZip(missingMainZip, dest); err != nil { t.Fatalf("extractZip: %v", err) } - if _, err := gi.findPackageRoot(dest); err == nil || !strings.Contains(err.Error(), "main.py not found") { - t.Fatalf("expected main.py error, got %v", err) + if _, err := gi.findPackageRoot(dest); err == nil || !strings.Contains(err.Error(), "main.py") { + t.Fatalf("expected entrypoint/main.py error, got %v", err) } badZipServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/control-plane/internal/packages/install_deps_test.go b/control-plane/internal/packages/install_deps_test.go new file mode 100644 index 000000000..2f80bcc60 --- /dev/null +++ b/control-plane/internal/packages/install_deps_test.go @@ -0,0 +1,92 @@ +package packages + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// fakePythonOnPath installs a stub `python3` whose `-m venv ` creates a +// venv layout with a no-op `pip`, so dependency installation can be exercised +// offline without invoking real Python/pip. +func fakePythonOnPath(t *testing.T) { + t.Helper() + binDir := t.TempDir() + py := "#!/bin/sh\n" + + "if [ \"$1\" = \"-m\" ] && [ \"$2\" = \"venv\" ]; then\n" + + " mkdir -p \"$3/bin\"\n" + + " printf '#!/bin/sh\\nexit 0\\n' > \"$3/bin/pip\"\n" + + " chmod +x \"$3/bin/pip\"\n" + + "fi\n" + + "exit 0\n" + if err := os.WriteFile(filepath.Join(binDir, "python3"), []byte(py), 0o755); err != nil { + t.Fatal(err) + } + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) +} + +// Contract: a pyproject.toml package gets a venv and is installed via +// `pip install .`, even without a requirements.txt. +func TestInstallPythonDependencies_Pyproject(t *testing.T) { + fakePythonOnPath(t) + pkg := t.TempDir() + if err := os.WriteFile(filepath.Join(pkg, "pyproject.toml"), + []byte("[project]\nname = \"demo\"\nversion = \"0.1.0\"\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := InstallPythonDependencies(pkg, nil, nil); err != nil { + t.Fatalf("InstallPythonDependencies: %v", err) + } + if _, err := os.Stat(filepath.Join(pkg, "venv", "bin", "pip")); err != nil { + t.Fatalf("expected a venv to be created for a pyproject project: %v", err) + } +} + +// Contract: requirements.txt + manifest-declared deps also trigger a venv. +func TestInstallPythonDependencies_RequirementsAndManifestDeps(t *testing.T) { + fakePythonOnPath(t) + pkg := t.TempDir() + if err := os.WriteFile(filepath.Join(pkg, "requirements.txt"), []byte("httpx\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := InstallPythonDependencies(pkg, []string{"pydantic>=2"}, []string{"libfoo"}); err != nil { + t.Fatalf("InstallPythonDependencies: %v", err) + } + if _, err := os.Stat(filepath.Join(pkg, "venv")); err != nil { + t.Fatalf("expected venv: %v", err) + } +} + +// Contract: a package with no Python sources needs no venv and is a no-op. +func TestInstallPythonDependencies_NothingToDo(t *testing.T) { + pkg := t.TempDir() + if err := InstallPythonDependencies(pkg, nil, nil); err != nil { + t.Fatalf("expected no-op, got %v", err) + } + if _, err := os.Stat(filepath.Join(pkg, "venv")); !os.IsNotExist(err) { + t.Fatalf("no venv should be created when there are no deps") + } +} + +// Contract: the git installer's findPackageRoot accepts a manifest that declares +// an entrypoint.start and has no main.py (the shape real nodes use). +func TestGitInstaller_FindPackageRoot_AcceptsEntrypoint(t *testing.T) { + root := t.TempDir() + pkg := filepath.Join(root, "repo") + if err := os.MkdirAll(pkg, 0o755); err != nil { + t.Fatal(err) + } + manifest := "name: node\nversion: 1.0.0\nentrypoint:\n start: python -m node.app\n" + if err := os.WriteFile(filepath.Join(pkg, "agentfield-package.yaml"), []byte(manifest), 0o644); err != nil { + t.Fatal(err) + } + gi := &GitInstaller{AgentFieldHome: t.TempDir()} + got, err := gi.findPackageRoot(root) + if err != nil { + t.Fatalf("findPackageRoot should accept an entrypoint-only manifest: %v", err) + } + if !strings.HasSuffix(got, "repo") { + t.Fatalf("unexpected package root: %s", got) + } +} diff --git a/control-plane/internal/packages/installer.go b/control-plane/internal/packages/installer.go index a9de9d3cd..1e4bea3f3 100644 --- a/control-plane/internal/packages/installer.go +++ b/control-plane/internal/packages/installer.go @@ -633,62 +633,78 @@ func (pi *PackageInstaller) copyFile(src, dst string) error { // installDependencies installs package dependencies func (pi *PackageInstaller) installDependencies(packagePath string, metadata *PackageMetadata) error { - // Install Python dependencies in a virtual environment - if len(metadata.Dependencies.Python) > 0 || pi.hasRequirementsFile(packagePath) { - // Create virtual environment + return InstallPythonDependencies(packagePath, metadata.Dependencies.Python, metadata.Dependencies.System) +} + +// InstallPythonDependencies sets up a per-package virtual environment and +// installs the node's Python dependencies. A venv is created when the package +// has a requirements.txt, a pyproject.toml, or manifest-declared Python deps. +// Install sources, in order: requirements.txt, `pip install .` for a +// pyproject.toml/setup.py project, then any manifest-declared packages. +func InstallPythonDependencies(packagePath string, pyDeps, systemDeps []string) error { + hasReq := fileExistsAt(packagePath, "requirements.txt") + hasProject := fileExistsAt(packagePath, "pyproject.toml") || fileExistsAt(packagePath, "setup.py") + + if hasReq || hasProject || len(pyDeps) > 0 { venvPath := filepath.Join(packagePath, "venv") cmd := exec.Command("python3", "-m", "venv", venvPath) if _, err := cmd.CombinedOutput(); err != nil { - // Try with python if python3 fails cmd = exec.Command("python", "-m", "venv", venvPath) if output, err := cmd.CombinedOutput(); err != nil { return fmt.Errorf("failed to create virtual environment: %w\nOutput: %s", err, output) } } - // Determine pip path - var pipPath string - if _, err := os.Stat(filepath.Join(venvPath, "bin", "pip")); err == nil { - pipPath = filepath.Join(venvPath, "bin", "pip") - } else { + pipPath := filepath.Join(venvPath, "bin", "pip") + if _, err := os.Stat(pipPath); err != nil { pipPath = filepath.Join(venvPath, "Scripts", "pip.exe") // Windows } // Upgrade pip first (ignore failures) - cmd = exec.Command(pipPath, "install", "--upgrade", "pip") - _, _ = cmd.CombinedOutput() + _, _ = exec.Command(pipPath, "install", "--upgrade", "pip").CombinedOutput() - // Install from requirements.txt if it exists - requirementsPath := filepath.Join(packagePath, "requirements.txt") - if _, err := os.Stat(requirementsPath); err == nil { - cmd = exec.Command(pipPath, "install", "-r", requirementsPath) + // requirements.txt + if hasReq { + cmd = exec.Command(pipPath, "install", "-r", "requirements.txt") cmd.Dir = packagePath if output, err := cmd.CombinedOutput(); err != nil { return fmt.Errorf("failed to install requirements.txt dependencies: %w\nOutput: %s", err, output) } } - // Install dependencies from agentfield-package.yaml - if len(metadata.Dependencies.Python) > 0 { - for _, dep := range metadata.Dependencies.Python { - cmd = exec.Command(pipPath, "install", dep) - cmd.Dir = packagePath - if output, err := cmd.CombinedOutput(); err != nil { - return fmt.Errorf("failed to install dependency %s: %w\nOutput: %s", dep, err, output) - } + // pyproject.toml / setup.py project (installs the project and its deps) + if hasProject { + cmd = exec.Command(pipPath, "install", ".") + cmd.Dir = packagePath + if output, err := cmd.CombinedOutput(); err != nil { + return fmt.Errorf("failed to install project (pip install .): %w\nOutput: %s", err, output) + } + } + + // Manifest-declared Python packages + for _, dep := range pyDeps { + cmd = exec.Command(pipPath, "install", dep) + cmd.Dir = packagePath + if output, err := cmd.CombinedOutput(); err != nil { + return fmt.Errorf("failed to install dependency %s: %w\nOutput: %s", dep, err, output) } } } - // Install system dependencies (if any) - for _, dep := range metadata.Dependencies.System { + for _, dep := range systemDeps { fmt.Printf("System dependency required: %s (please install manually)\n", dep) } return nil } +// fileExistsAt reports whether name exists directly under dir. +func fileExistsAt(dir, name string) bool { + _, err := os.Stat(filepath.Join(dir, name)) + return err == nil +} + // hasRequirementsFile checks if requirements.txt exists func (pi *PackageInstaller) hasRequirementsFile(packagePath string) bool { requirementsPath := filepath.Join(packagePath, "requirements.txt") diff --git a/docs/installing-agent-nodes.md b/docs/installing-agent-nodes.md index a611a49cb..24001917b 100644 --- a/docs/installing-agent-nodes.md +++ b/docs/installing-agent-nodes.md @@ -69,6 +69,19 @@ user_environment: default: openrouter/moonshotai/kimi-k2 ``` +### Python dependencies + +On install, a node's Python dependencies are installed into a per-node virtual +environment under `~/.agentfield/packages//venv`. Sources are honored in +order: `requirements.txt`, then `pip install .` for a `pyproject.toml`/`setup.py` +project, then any packages listed under `dependencies.python` in the manifest. +`af run` uses this venv automatically. + +The venv is built with the `python3`/`python` on your `PATH`. If a node declares +`requires-python` (e.g. `>=3.11`) that your interpreter doesn't satisfy, `pip` +reports it and install fails — point `af` at a compatible interpreter (e.g. via +`pyenv`/`PATH`) and reinstall. + ### Node dependencies `dependencies.nodes` lets one node declare that it needs others. Each entry is