From 957909f070c00fe2b290610998ddb157614b24fa Mon Sep 17 00:00:00 2001 From: Cameron Sjo Date: Sun, 5 Apr 2026 16:05:25 -0500 Subject: [PATCH 01/12] feat: add config.SharedExpertDir for user-level shared expert paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Returns ~/.agent-pool/experts/{name}/ with filename validation. Foundation for v0.7 shared experts — pure addition, no existing behavior changed. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/config/config.go | 19 ++++++++++++ internal/config/config_test.go | 53 ++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/internal/config/config.go b/internal/config/config.go index e4b287a..aabae84 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -79,6 +79,25 @@ type ExpertSection struct { AllowedTools []string `toml:"allowed_tools"` } +// SharedExpertDir returns the user-level directory for a shared expert. +// The path is ~/.agent-pool/experts/{name}/. The name must be a simple +// filename (no path separators, not "." or ".."). +func SharedExpertDir(name string) (string, error) { + if name == "" { + return "", fmt.Errorf("shared expert name is empty") + } + if name != filepath.Base(name) || name == "." || name == ".." { + return "", fmt.Errorf("invalid shared expert name %q: must be a simple filename", name) + } + + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("resolving home directory: %w", err) + } + + return filepath.Join(home, ".agent-pool", "experts", name), nil +} + // DiscoverPoolDir finds the pool directory by checking: // 1. The given path (if non-empty) // 2. Current directory for pool.toml diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 84f6037..ff23771 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -14,6 +14,13 @@ // [x] Happy: valid duration "10m" (TestDefaultsSection_ParseSessionTimeout) // [x] Happy: valid duration "30s" (TestDefaultsSection_ParseSessionTimeout) // [x] Unhappy: invalid duration string (TestDefaultsSection_ParseSessionTimeout) +// +// SharedExpertDir (Classification: PATH RESOLVER) +// [x] Happy: returns ~/.agent-pool/experts/{name}/ +// [x] Unhappy: empty name +// [x] Unhappy: path traversal (../) +// [x] Unhappy: dot name (.) +// [x] Unhappy: path separator in name package config_test @@ -341,6 +348,52 @@ func assertSliceEqual(t *testing.T, field string, expected []string, actual []st } } +func TestSharedExpertDir(t *testing.T) { + home, err := os.UserHomeDir() + if err != nil { + t.Skipf("cannot determine home directory: %v", err) + } + + t.Run("happy path", func(t *testing.T) { + dir, err := config.SharedExpertDir("security-standards") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + expected := filepath.Join(home, ".agent-pool", "experts", "security-standards") + if dir != expected { + t.Errorf("got %q, want %q", dir, expected) + } + }) + + t.Run("empty name", func(t *testing.T) { + _, err := config.SharedExpertDir("") + if err == nil { + t.Fatal("expected error for empty name") + } + }) + + t.Run("path traversal", func(t *testing.T) { + _, err := config.SharedExpertDir("../evil") + if err == nil { + t.Fatal("expected error for path traversal") + } + }) + + t.Run("dot name", func(t *testing.T) { + _, err := config.SharedExpertDir(".") + if err == nil { + t.Fatal("expected error for dot name") + } + }) + + t.Run("path separator", func(t *testing.T) { + _, err := config.SharedExpertDir("foo/bar") + if err == nil { + t.Fatal("expected error for path separator") + } + }) +} + func TestDefaultsSection_ParseSessionTimeout(t *testing.T) { tests := []struct { name string From 1fa7409f80164d2963801b2f85bdf3d64213aedd Mon Sep 17 00:00:00 2001 From: Cameron Sjo Date: Sun, 5 Apr 2026 16:06:18 -0500 Subject: [PATCH 02/12] feat: add mail.ResolveShared* for shared expert path resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three new functions for shared expert directories: - ResolveSharedExpertDir → ~/.agent-pool/experts/{name}/ - ResolveSharedInbox → {poolDir}/shared-state/{name}/inbox/ - ResolveSharedLogDir → {poolDir}/shared-state/{name}/logs/ Pure additions — existing ResolveExpertDir/ResolveInbox unchanged. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/mail/router.go | 20 ++++++++++++ internal/mail/router_test.go | 60 ++++++++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/internal/mail/router.go b/internal/mail/router.go index b2b28b2..e5ac34b 100644 --- a/internal/mail/router.go +++ b/internal/mail/router.go @@ -6,6 +6,8 @@ import ( "log/slog" "os" "path/filepath" + + "github.com/cameronsjo/agent-pool/internal/config" ) // builtinRoles are roles with top-level directories (not under experts/). @@ -46,6 +48,24 @@ func ResolveInbox(poolDir, recipient string) string { return filepath.Join(poolDir, "experts", recipient, "inbox") } +// ResolveSharedExpertDir returns the user-level state directory for a shared +// expert: ~/.agent-pool/experts/{name}/. Delegates validation to config.SharedExpertDir. +func ResolveSharedExpertDir(name string) (string, error) { + return config.SharedExpertDir(name) +} + +// ResolveSharedInbox returns the pool-scoped inbox for a shared expert: +// {poolDir}/shared-state/{name}/inbox/. +func ResolveSharedInbox(poolDir, name string) string { + return filepath.Join(poolDir, "shared-state", name, "inbox") +} + +// ResolveSharedLogDir returns the pool-scoped log directory for a shared expert: +// {poolDir}/shared-state/{name}/logs/. +func ResolveSharedLogDir(poolDir, name string) string { + return filepath.Join(poolDir, "shared-state", name, "logs") +} + // Route parses a message from the postoffice, copies it to the recipient's // inbox, and deletes the original. Delivery is at-least-once: the copy // completes before the delete. diff --git a/internal/mail/router_test.go b/internal/mail/router_test.go index 5ece98f..09417d4 100644 --- a/internal/mail/router_test.go +++ b/internal/mail/router_test.go @@ -351,6 +351,54 @@ func TestIsBuiltinRole(t *testing.T) { } } +func TestResolveSharedExpertDir(t *testing.T) { + home, err := os.UserHomeDir() + if err != nil { + t.Skipf("cannot determine home directory: %v", err) + } + + t.Run("happy path", func(t *testing.T) { + dir, err := mail.ResolveSharedExpertDir("security-standards") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + expected := filepath.Join(home, ".agent-pool", "experts", "security-standards") + if dir != expected { + t.Errorf("got %q, want %q", dir, expected) + } + }) + + t.Run("rejects empty name", func(t *testing.T) { + _, err := mail.ResolveSharedExpertDir("") + if err == nil { + t.Fatal("expected error for empty name") + } + }) + + t.Run("rejects path traversal", func(t *testing.T) { + _, err := mail.ResolveSharedExpertDir("../evil") + if err == nil { + t.Fatal("expected error for path traversal") + } + }) +} + +func TestResolveSharedInbox(t *testing.T) { + got := mail.ResolveSharedInbox("/pool", "security-standards") + want := "/pool/shared-state/security-standards/inbox" + if got != want { + t.Errorf("got %q, want %q", got, want) + } +} + +func TestResolveSharedLogDir(t *testing.T) { + got := mail.ResolveSharedLogDir("/pool", "security-standards") + want := "/pool/shared-state/security-standards/logs" + if got != want { + t.Errorf("got %q, want %q", got, want) + } +} + // Test Plan for router.go // // IsBuiltinRole (Classification: PURE LOGIC) @@ -363,6 +411,18 @@ func TestIsBuiltinRole(t *testing.T) { // [x] Happy: experts resolve correctly (TestResolveInbox_Experts) // [x] Boundary: empty recipient (TestResolveInbox_EmptyRecipient) // +// +// ResolveSharedExpertDir (Classification: PATH RESOLVER) +// [x] Happy: returns ~/.agent-pool/experts/{name}/ (TestResolveSharedExpertDir) +// [x] Unhappy: rejects empty name (TestResolveSharedExpertDir) +// [x] Unhappy: rejects path traversal (TestResolveSharedExpertDir) +// +// ResolveSharedInbox (Classification: PURE LOGIC) +// [x] Happy: returns {poolDir}/shared-state/{name}/inbox (TestResolveSharedInbox) +// +// ResolveSharedLogDir (Classification: PURE LOGIC) +// [x] Happy: returns {poolDir}/shared-state/{name}/logs (TestResolveSharedLogDir) +// // Route (Classification: I/O BOUNDARY) // [x] Happy: end-to-end routing (TestRoute_EndToEnd) // [x] Unhappy: unknown recipient (TestRoute_UnknownRecipient) From e9d9d336dba0ff8c081456d43af488d80b1b627a Mon Sep 17 00:00:00 2001 From: Cameron Sjo Date: Sun, 5 Apr 2026 16:07:15 -0500 Subject: [PATCH 03/12] feat: add PoolConfig.Validate for shared expert name validation Validates shared.include entries don't conflict with builtin roles or pool-scoped experts, and are filename-safe. Called at end of LoadPool. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/config/config.go | 32 +++++++++++++++++ internal/config/config_test.go | 63 ++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+) diff --git a/internal/config/config.go b/internal/config/config.go index aabae84..11d4047 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -79,6 +79,34 @@ type ExpertSection struct { AllowedTools []string `toml:"allowed_tools"` } +// builtinRoleNames are the built-in role names that shared experts must not +// conflict with. Kept in sync with mail.builtinRoles. +var builtinRoleNames = map[string]bool{ + "architect": true, + "researcher": true, + "concierge": true, +} + +// Validate checks the pool config for internal consistency. +// Returns an error if shared.include entries conflict with builtin roles or +// pool-scoped experts, or contain invalid characters. +func (c *PoolConfig) Validate() error { + for _, name := range c.Shared.Include { + if name == "" || name != filepath.Base(name) || name == "." || name == ".." { + return fmt.Errorf("invalid shared expert name %q: must be a simple filename", name) + } + if builtinRoleNames[name] { + return fmt.Errorf("shared expert %q conflicts with built-in role", name) + } + if c.Experts != nil { + if _, exists := c.Experts[name]; exists { + return fmt.Errorf("shared expert %q conflicts with pool-scoped expert", name) + } + } + } + return nil +} + // SharedExpertDir returns the user-level directory for a shared expert. // The path is ~/.agent-pool/experts/{name}/. The name must be a simple // filename (no path separators, not "." or ".."). @@ -194,5 +222,9 @@ func LoadPool(poolDir string) (*PoolConfig, error) { cfg.Curation.IntervalHours = 168 } + if err := cfg.Validate(); err != nil { + return nil, fmt.Errorf("validating %s: %w", configPath, err) + } + return &cfg, nil } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index ff23771..0de887b 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -21,6 +21,13 @@ // [x] Unhappy: path traversal (../) // [x] Unhappy: dot name (.) // [x] Unhappy: path separator in name +// +// PoolConfig.Validate (Classification: VALIDATION) +// [x] Happy: valid config with shared.include (TestValidate_HappyPath) +// [x] Happy: empty shared.include passes (TestValidate_EmptySharedInclude) +// [x] Unhappy: shared conflicts with builtin role (TestValidate_SharedConflictsWithBuiltin) +// [x] Unhappy: shared conflicts with pool-scoped expert (TestValidate_SharedConflictsWithPoolExpert) +// [x] Unhappy: shared name has path separator (TestValidate_SharedPathTraversal) package config_test @@ -394,6 +401,62 @@ func TestSharedExpertDir(t *testing.T) { }) } +func TestValidate_HappyPath(t *testing.T) { + cfg := &config.PoolConfig{ + Shared: config.SharedSection{Include: []string{"security-standards", "corporate-policies"}}, + Experts: map[string]config.ExpertSection{ + "auth": {Model: "sonnet"}, + }, + } + if err := cfg.Validate(); err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestValidate_EmptySharedInclude(t *testing.T) { + cfg := &config.PoolConfig{} + if err := cfg.Validate(); err != nil { + t.Fatalf("unexpected error for empty shared.include: %v", err) + } +} + +func TestValidate_SharedConflictsWithBuiltin(t *testing.T) { + cfg := &config.PoolConfig{ + Shared: config.SharedSection{Include: []string{"architect"}}, + } + err := cfg.Validate() + if err == nil { + t.Fatal("expected error for shared name conflicting with builtin role") + } + if !strings.Contains(err.Error(), "architect") { + t.Errorf("error should mention 'architect', got: %v", err) + } +} + +func TestValidate_SharedConflictsWithPoolExpert(t *testing.T) { + cfg := &config.PoolConfig{ + Shared: config.SharedSection{Include: []string{"auth"}}, + Experts: map[string]config.ExpertSection{"auth": {Model: "sonnet"}}, + } + err := cfg.Validate() + if err == nil { + t.Fatal("expected error for shared name conflicting with pool-scoped expert") + } + if !strings.Contains(err.Error(), "auth") { + t.Errorf("error should mention 'auth', got: %v", err) + } +} + +func TestValidate_SharedPathTraversal(t *testing.T) { + cfg := &config.PoolConfig{ + Shared: config.SharedSection{Include: []string{"../evil"}}, + } + err := cfg.Validate() + if err == nil { + t.Fatal("expected error for path traversal in shared name") + } +} + func TestDefaultsSection_ParseSessionTimeout(t *testing.T) { tests := []struct { name string From 592d5b17b8928bd185bedad7a0e0d8335efba4b6 Mon Sep 17 00:00:00 2001 From: Cameron Sjo Date: Sun, 5 Apr 2026 16:09:08 -0500 Subject: [PATCH 04/12] feat: create shared-state dirs in ensureDirs, add isSharedExpert helper ensureDirs now creates {poolDir}/shared-state/{name}/{inbox,logs} for each shared.include entry. isSharedExpert checks membership in the list. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/daemon/daemon.go | 19 +++++++++++++++++ internal/daemon/daemon_test.go | 39 ++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 55b70d9..e75ab31 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -1142,6 +1142,15 @@ func (d *Daemon) ensureDirs() error { ) } + // Shared experts get pool-scoped shared-state directories for inbox and logs. + // Identity and user-level state live at ~/.agent-pool/experts/{name}/ (not created here). + for _, name := range d.cfg.Shared.Include { + dirs = append(dirs, + mail.ResolveSharedInbox(d.poolDir, name), + mail.ResolveSharedLogDir(d.poolDir, name), + ) + } + for _, dir := range dirs { if err := os.MkdirAll(dir, 0o755); err != nil { return fmt.Errorf("creating %s: %w", dir, err) @@ -1151,6 +1160,16 @@ func (d *Daemon) ensureDirs() error { return nil } +// isSharedExpert reports whether the named expert is in the pool's shared.include list. +func (d *Daemon) isSharedExpert(name string) bool { + for _, shared := range d.cfg.Shared.Include { + if shared == name { + return true + } + } + return false +} + // absEqual compares two paths after resolving to absolute form. func absEqual(a, b string) bool { absA, _ := filepath.Abs(a) diff --git a/internal/daemon/daemon_test.go b/internal/daemon/daemon_test.go index 9d96aec..16371cc 100644 --- a/internal/daemon/daemon_test.go +++ b/internal/daemon/daemon_test.go @@ -251,6 +251,45 @@ Test routing. } } +func TestDaemon_EnsureDirsCreatesSharedStateDirs(t *testing.T) { + poolDir := t.TempDir() + + cfg := writePoolConfig(t, poolDir, `[pool] +name = "shared-test" +project_dir = "PROJECT_DIR" + +[shared] +include = ["security-standards", "corporate-policies"] + +[experts.auth] +model = "sonnet" +`) + + cancel, errCh := startTestDaemon(t, cfg, poolDir, &fakeSpawner{}) + defer func() { + cancel() + <-errCh + }() + + // Verify shared-state directories were created + for _, name := range []string{"security-standards", "corporate-policies"} { + for _, sub := range []string{"inbox", "logs"} { + dir := filepath.Join(poolDir, "shared-state", name, sub) + if _, err := os.Stat(dir); os.IsNotExist(err) { + t.Errorf("expected shared-state directory %s to exist", dir) + } + } + } + + // Verify pool-scoped expert dirs still created + for _, sub := range []string{"inbox", "logs"} { + dir := filepath.Join(poolDir, "experts", "auth", sub) + if _, err := os.Stat(dir); os.IsNotExist(err) { + t.Errorf("expected pool-scoped expert directory %s to exist", dir) + } + } +} + // writeMessage writes a YAML-frontmatter mail file to the given directory. func writeMessage(t *testing.T, dir, id, from, to string) string { t.Helper() From 73982c764f0990b50414b1ed18d1f77a1f0965ba Mon Sep 17 00:00:00 2001 From: Cameron Sjo Date: Sun, 5 Apr 2026 16:11:04 -0500 Subject: [PATCH 05/12] feat: add OverlayDir to SpawnConfig, layered state in AssemblePrompt SpawnConfig gains OverlayDir for pool-scoped project state overlay. AssemblePrompt reads overlay state.md and emits "## Project State" section between user-level state and errors. Empty OverlayDir is a no-op, preserving backward compatibility for pool-scoped experts. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/expert/expert.go | 16 ++++ internal/expert/expert_test.go | 148 +++++++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+) diff --git a/internal/expert/expert.go b/internal/expert/expert.go index 681b11d..44e5952 100644 --- a/internal/expert/expert.go +++ b/internal/expert/expert.go @@ -28,6 +28,7 @@ type SpawnConfig struct { AllowedTools []string ProjectDir string // working directory for claude ExpertDir string // contains identity.md, state.md, errors.md + OverlayDir string // pool-scoped overlay for shared experts (optional) PoolDir string // pool root directory (set as AGENT_POOL_DIR env var) TaskMessage *mail.Message // the task to execute MCPConfigPath string // path to MCP config JSON; if set, --mcp-config is added @@ -72,6 +73,21 @@ func AssemblePrompt(cfg *SpawnConfig) (string, error) { b.WriteString("\n\n") } + // Project-specific overlay state (shared experts only). + // When OverlayDir is set, read state.md from the overlay directory + // and emit it under a separate heading. + if cfg.OverlayDir != "" { + overlayState, overlayErr := readOptionalFile(cfg.OverlayDir, "state.md") + if overlayErr != nil { + return "", fmt.Errorf("reading overlay state: %w", overlayErr) + } + if overlayState != "" { + b.WriteString("## Project State\n\n") + b.WriteString(overlayState) + b.WriteString("\n\n") + } + } + if errors != "" { b.WriteString("## Known Errors & Pitfalls\n\n") b.WriteString(errors) diff --git a/internal/expert/expert_test.go b/internal/expert/expert_test.go index d2237d1..26aa638 100644 --- a/internal/expert/expert_test.go +++ b/internal/expert/expert_test.go @@ -660,6 +660,154 @@ func TestAssemblePrompt_NilConfig(t *testing.T) { } } +func TestAssemblePrompt_WithOverlay(t *testing.T) { + expertDir := t.TempDir() + overlayDir := t.TempDir() + + os.WriteFile(filepath.Join(expertDir, "identity.md"), []byte("I am the security expert."), 0o644) + os.WriteFile(filepath.Join(expertDir, "state.md"), []byte("User-level state: OWASP top 10 checklist."), 0o644) + os.WriteFile(filepath.Join(overlayDir, "state.md"), []byte("Project-level state: API gateway audit pending."), 0o644) + os.WriteFile(filepath.Join(expertDir, "errors.md"), []byte("- False positive on CORS check"), 0o644) + + cfg := &expert.SpawnConfig{ + Name: "security-standards", + ExpertDir: expertDir, + OverlayDir: overlayDir, + TaskMessage: &mail.Message{ + ID: "task-sec-001", + From: "architect", + Type: mail.TypeTask, + Body: "Review the auth module.", + }, + } + + prompt, err := expert.AssemblePrompt(cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Both state sections present under separate headings + for _, want := range []string{ + "## Expert Identity", + "I am the security expert.", + "## Current State", + "User-level state: OWASP top 10 checklist.", + "## Project State", + "Project-level state: API gateway audit pending.", + "## Known Errors & Pitfalls", + "False positive on CORS check", + "## Task", + "Review the auth module.", + } { + if !strings.Contains(prompt, want) { + t.Errorf("prompt missing %q", want) + } + } + + // Project State appears after Current State + stateIdx := strings.Index(prompt, "## Current State") + projectIdx := strings.Index(prompt, "## Project State") + if projectIdx <= stateIdx { + t.Error("## Project State should appear after ## Current State") + } +} + +func TestAssemblePrompt_OverlayOnly(t *testing.T) { + expertDir := t.TempDir() + overlayDir := t.TempDir() + + // No user-level state.md, only overlay + os.WriteFile(filepath.Join(expertDir, "identity.md"), []byte("I am the expert."), 0o644) + os.WriteFile(filepath.Join(overlayDir, "state.md"), []byte("Project-specific knowledge."), 0o644) + + cfg := &expert.SpawnConfig{ + Name: "shared-expert", + ExpertDir: expertDir, + OverlayDir: overlayDir, + TaskMessage: &mail.Message{ + ID: "task-overlay", + From: "architect", + Type: mail.TypeTask, + Body: "Do the thing.", + }, + } + + prompt, err := expert.AssemblePrompt(cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if strings.Contains(prompt, "## Current State") { + t.Error("should not have Current State section when user-level state.md is missing") + } + if !strings.Contains(prompt, "## Project State") { + t.Error("should have Project State section from overlay") + } +} + +func TestAssemblePrompt_OverlayDirSetButNoFile(t *testing.T) { + expertDir := t.TempDir() + overlayDir := t.TempDir() // exists but no state.md + + os.WriteFile(filepath.Join(expertDir, "identity.md"), []byte("Expert identity."), 0o644) + os.WriteFile(filepath.Join(expertDir, "state.md"), []byte("User state."), 0o644) + + cfg := &expert.SpawnConfig{ + Name: "shared-expert", + ExpertDir: expertDir, + OverlayDir: overlayDir, + TaskMessage: &mail.Message{ + ID: "task-no-overlay", + From: "architect", + Type: mail.TypeTask, + Body: "Do the thing.", + }, + } + + prompt, err := expert.AssemblePrompt(cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !strings.Contains(prompt, "## Current State") { + t.Error("should have Current State from user-level state") + } + if strings.Contains(prompt, "## Project State") { + t.Error("should not have Project State when overlay state.md is missing") + } +} + +func TestAssemblePrompt_EmptyOverlayDir(t *testing.T) { + expertDir := t.TempDir() + + os.WriteFile(filepath.Join(expertDir, "identity.md"), []byte("Expert."), 0o644) + os.WriteFile(filepath.Join(expertDir, "state.md"), []byte("User state."), 0o644) + + cfg := &expert.SpawnConfig{ + Name: "pool-scoped", + ExpertDir: expertDir, + OverlayDir: "", // empty = no overlay (pool-scoped expert) + TaskMessage: &mail.Message{ + ID: "task-no-overlay", + From: "architect", + Type: mail.TypeTask, + Body: "Do the thing.", + }, + } + + prompt, err := expert.AssemblePrompt(cfg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if !strings.Contains(prompt, "## Current State") { + t.Error("should have Current State") + } + if strings.Contains(prompt, "## Project State") { + t.Error("should not have Project State when OverlayDir is empty") + } +} + func TestAppendIndex_SanitizesAllCells(t *testing.T) { dir := t.TempDir() entry := &expert.LogEntry{ From e88b1d9cfaa1926843c2b5250458afd98d1c56ef Mon Sep 17 00:00:00 2001 From: Cameron Sjo Date: Sun, 5 Apr 2026 16:16:37 -0500 Subject: [PATCH 06/12] feat: wire shared expert spawn, routing, and inbox watching in daemon Major changes: - processInboxMessage detects shared experts, sets ExpertDir to user-level and OverlayDir to pool-scoped shared-state dir - Logs written to pool-scoped shared-state/{name}/ for shared experts - Route gains sharedNames param to route to shared-state inboxes - drainInbox, drainAllInboxes, handleCancel resolve shared inbox paths - resolveExpertName matches shared inbox paths - Run() watches shared expert inboxes alongside pool-scoped ones Merges Phase 2.2 and 2.3 since Route + spawn wiring are tightly coupled. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/daemon/daemon.go | 84 +++++++++++++++++++++++++++++---- internal/daemon/daemon_test.go | 86 ++++++++++++++++++++++++++++++++++ internal/mail/router.go | 13 ++++- internal/mail/router_test.go | 14 +++--- 4 files changed, 180 insertions(+), 17 deletions(-) diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index e75ab31..80788e8 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -161,6 +161,14 @@ func (d *Daemon) Run(ctx context.Context) error { } } + // Watch shared expert inboxes (pool-scoped under shared-state/) + for _, name := range d.cfg.Shared.Include { + inboxDir := mail.ResolveSharedInbox(d.poolDir, name) + if err := watcher.Add(inboxDir); err != nil { + return fmt.Errorf("watching shared inbox for %s: %w", name, err) + } + } + // Start socket server for CLI→daemon communication sockPath := d.resolveSockPath() sock, err := newSocketServer(sockPath, d, cancel) @@ -256,7 +264,7 @@ func (d *Daemon) handlePostoffice(ctx context.Context, path string) { } // Non-cancel: route to recipient inbox. - routed, err := mail.Route(d.logger, d.poolDir, path) + routed, err := mail.Route(d.logger, d.poolDir, path, d.sharedNamesMap()) if err != nil { d.logger.Error("Failed to route message", "path", path, @@ -366,7 +374,13 @@ func (d *Daemon) handleCancel(msg *mail.Message, cancelPath string) { d.mu.Unlock() // Remove the inbox file for this task if it exists. - inboxPath := filepath.Join(mail.ResolveInbox(d.poolDir, task.Expert), targetID+".md") + var cancelInboxDir string + if d.isSharedExpert(task.Expert) { + cancelInboxDir = mail.ResolveSharedInbox(d.poolDir, task.Expert) + } else { + cancelInboxDir = mail.ResolveInbox(d.poolDir, task.Expert) + } + inboxPath := filepath.Join(cancelInboxDir, targetID+".md") if err := os.Remove(inboxPath); err != nil && !os.IsNotExist(err) { d.logger.Warn("Failed to remove inbox file for cancelled task", "task_id", targetID, @@ -557,7 +571,27 @@ func (d *Daemon) processInboxMessage(ctx context.Context, expertName string, pat Data: ExpertSpawningData{Expert: expertName, TaskID: msg.ID, Model: model}, }) projectDir := d.resolveProjectDir() - expertDir := d.resolveExpertDir(expertName) + + // Resolve directories: shared experts use user-level identity + pool overlay + var expertDir, overlayDir, logDir string + if d.isSharedExpert(expertName) { + userDir, resolveErr := mail.ResolveSharedExpertDir(expertName) + if resolveErr != nil { + d.logger.Error("Failed to resolve shared expert directory", + "expert", expertName, + "task_id", msg.ID, + "error", resolveErr, + ) + d.markTaskFailed(msg.ID, -1) + return false + } + expertDir = userDir + overlayDir = filepath.Join(d.poolDir, "shared-state", expertName) + logDir = overlayDir // WriteLog/AppendIndex create logs/ subdir inside this + } else { + expertDir = d.resolveExpertDir(expertName) + logDir = expertDir + } var mcpConfigPath string var mcpErr error @@ -593,6 +627,7 @@ func (d *Daemon) processInboxMessage(ctx context.Context, expertName string, pat AllowedTools: allTools, ProjectDir: projectDir, ExpertDir: expertDir, + OverlayDir: overlayDir, PoolDir: d.poolDir, TaskMessage: msg, MCPConfigPath: mcpConfigPath, @@ -625,8 +660,9 @@ func (d *Daemon) processInboxMessage(ctx context.Context, expertName string, pat return false } - // Always write logs — the archive is append-only by design - if err := expert.WriteLog(expertDir, result.TaskID, result.Output); err != nil { + // Always write logs — the archive is append-only by design. + // For shared experts, logs go to pool-scoped shared-state dir. + if err := expert.WriteLog(logDir, result.TaskID, result.Output); err != nil { d.logger.Error("Failed to write task log", "expert", expertName, "task_id", result.TaskID, @@ -635,7 +671,7 @@ func (d *Daemon) processInboxMessage(ctx context.Context, expertName string, pat } if len(result.Stderr) > 0 { - if err := expert.WriteStderr(expertDir, result.TaskID, result.Stderr); err != nil { + if err := expert.WriteStderr(logDir, result.TaskID, result.Stderr); err != nil { d.logger.Error("Failed to write stderr log", "expert", expertName, "task_id", result.TaskID, @@ -644,7 +680,7 @@ func (d *Daemon) processInboxMessage(ctx context.Context, expertName string, pat } } - if err := expert.AppendIndex(expertDir, &expert.LogEntry{ + if err := expert.AppendIndex(logDir, &expert.LogEntry{ TaskID: result.TaskID, Timestamp: msg.Timestamp, From: msg.From, @@ -825,13 +861,24 @@ func (d *Daemon) drainAllInboxes(ctx context.Context) { d.wg.Add(1) go func(n string) { defer d.wg.Done(); d.handleInbox(ctx, n, "") }(name) } + + // Drain shared expert inboxes + for _, name := range d.cfg.Shared.Include { + d.wg.Add(1) + go func(n string) { defer d.wg.Done(); d.handleInbox(ctx, n, "") }(name) + } } // drainInbox iteratively processes all .md files in an expert's inbox (oldest // first). It reads the file list once and processes each in order. Files that // fail to parse are skipped (logged and left in inbox for manual inspection). func (d *Daemon) drainInbox(ctx context.Context, expertName string) { - inboxDir := mail.ResolveInbox(d.poolDir, expertName) + var inboxDir string + if d.isSharedExpert(expertName) { + inboxDir = mail.ResolveSharedInbox(d.poolDir, expertName) + } else { + inboxDir = mail.ResolveInbox(d.poolDir, expertName) + } // Track files we've already attempted so we don't loop forever on // non-zero exit files (which are preserved in inbox). @@ -1117,6 +1164,15 @@ func (d *Daemon) resolveExpertName(inboxDir string) string { return name } } + + // Check shared expert inboxes + for _, name := range d.cfg.Shared.Include { + expected := mail.ResolveSharedInbox(d.poolDir, name) + if absEqual(inboxDir, expected) { + return name + } + } + return "" } @@ -1160,6 +1216,18 @@ func (d *Daemon) ensureDirs() error { return nil } +// sharedNamesMap returns the shared.include list as a set for O(1) lookup. +func (d *Daemon) sharedNamesMap() map[string]bool { + if len(d.cfg.Shared.Include) == 0 { + return nil + } + m := make(map[string]bool, len(d.cfg.Shared.Include)) + for _, name := range d.cfg.Shared.Include { + m[name] = true + } + return m +} + // isSharedExpert reports whether the named expert is in the pool's shared.include list. func (d *Daemon) isSharedExpert(name string) bool { for _, shared := range d.cfg.Shared.Include { diff --git a/internal/daemon/daemon_test.go b/internal/daemon/daemon_test.go index 16371cc..158cde0 100644 --- a/internal/daemon/daemon_test.go +++ b/internal/daemon/daemon_test.go @@ -290,6 +290,92 @@ model = "sonnet" } } +func TestDaemon_SharedExpertSpawnConfig(t *testing.T) { + // Set HOME to a temp dir so SharedExpertDir resolves to a controlled location + fakeHome := t.TempDir() + origHome := os.Getenv("HOME") + os.Setenv("HOME", fakeHome) + t.Cleanup(func() { os.Setenv("HOME", origHome) }) + + // Create the user-level shared expert directory + sharedExpertDir := filepath.Join(fakeHome, ".agent-pool", "experts", "security-standards") + os.MkdirAll(sharedExpertDir, 0o755) + os.WriteFile(filepath.Join(sharedExpertDir, "identity.md"), []byte("I am the security expert."), 0o644) + + poolDir := t.TempDir() + cfg := writePoolConfig(t, poolDir, `[pool] +name = "shared-spawn-test" +project_dir = "PROJECT_DIR" + +[shared] +include = ["security-standards"] + +[experts.auth] +model = "sonnet" +`) + + fake := &fakeSpawner{} + cancel, errCh := startTestDaemon(t, cfg, poolDir, fake) + + // Send a task to the shared expert + postoffice := filepath.Join(poolDir, "postoffice") + writeMessage(t, postoffice, "task-shared-001", "architect", "security-standards") + + // Wait for the log file to appear in the pool-scoped shared-state dir + logPath := filepath.Join(poolDir, "shared-state", "security-standards", "logs", "task-shared-001.json") + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + if _, err := os.Stat(logPath); err == nil { + break + } + time.Sleep(100 * time.Millisecond) + } + + cancel() + select { + case err := <-errCh: + if err != nil { + t.Errorf("daemon returned error: %v", err) + } + case <-time.After(3 * time.Second): + t.Error("daemon did not shut down in time") + } + + // Verify spawn was called + calls := fake.getCalls() + if len(calls) == 0 { + t.Fatal("expected at least one spawn call for shared expert") + } + + // Find the shared expert spawn call + var sharedCall *expert.SpawnConfig + for _, c := range calls { + if c.Name == "security-standards" { + sharedCall = c + break + } + } + if sharedCall == nil { + t.Fatal("no spawn call for security-standards") + } + + // ExpertDir should point to user-level dir + if sharedCall.ExpertDir != sharedExpertDir { + t.Errorf("ExpertDir = %q, want %q", sharedCall.ExpertDir, sharedExpertDir) + } + + // OverlayDir should point to pool-scoped shared-state + wantOverlay := filepath.Join(poolDir, "shared-state", "security-standards") + if sharedCall.OverlayDir != wantOverlay { + t.Errorf("OverlayDir = %q, want %q", sharedCall.OverlayDir, wantOverlay) + } + + // Logs should be in pool-scoped shared-state dir, not user-level + if _, err := os.Stat(logPath); os.IsNotExist(err) { + t.Error("logs should be written to shared-state/{name}/logs/") + } +} + // writeMessage writes a YAML-frontmatter mail file to the given directory. func writeMessage(t *testing.T, dir, id, from, to string) string { t.Helper() diff --git a/internal/mail/router.go b/internal/mail/router.go index e5ac34b..4cecdcf 100644 --- a/internal/mail/router.go +++ b/internal/mail/router.go @@ -69,13 +69,22 @@ func ResolveSharedLogDir(poolDir, name string) string { // Route parses a message from the postoffice, copies it to the recipient's // inbox, and deletes the original. Delivery is at-least-once: the copy // completes before the delete. -func Route(logger *slog.Logger, poolDir, filePath string) (*Message, error) { +// +// sharedNames lists experts that are shared (user-level). Messages to shared +// experts are routed to the pool-scoped shared-state inbox. Pass nil when +// no shared experts are configured. +func Route(logger *slog.Logger, poolDir, filePath string, sharedNames map[string]bool) (*Message, error) { msg, err := ParseFile(filePath) if err != nil { return nil, fmt.Errorf("parsing message for routing: %w", err) } - inboxDir := ResolveInbox(poolDir, msg.To) + var inboxDir string + if sharedNames[msg.To] { + inboxDir = ResolveSharedInbox(poolDir, msg.To) + } else { + inboxDir = ResolveInbox(poolDir, msg.To) + } if _, err := os.Stat(inboxDir); err != nil { return nil, fmt.Errorf("inbox not available for recipient %q: %w", msg.To, err) diff --git a/internal/mail/router_test.go b/internal/mail/router_test.go index 09417d4..79dc343 100644 --- a/internal/mail/router_test.go +++ b/internal/mail/router_test.go @@ -65,7 +65,7 @@ Do the thing. } logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelWarn})) - msg, err := mail.Route(logger, poolDir, srcPath) + msg, err := mail.Route(logger, poolDir, srcPath, nil) if err != nil { t.Fatalf("Route failed: %v", err) } @@ -105,7 +105,7 @@ This should fail. os.WriteFile(srcPath, []byte(msgContent), 0o644) logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelWarn})) - _, err := mail.Route(logger, poolDir, srcPath) + _, err := mail.Route(logger, poolDir, srcPath, nil) if err == nil { t.Fatal("expected error for unknown recipient") } @@ -146,7 +146,7 @@ Idempotent delivery test. } logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelWarn})) - _, err := mail.Route(logger, poolDir, srcPath) + _, err := mail.Route(logger, poolDir, srcPath, nil) if err != nil { t.Fatalf("Route failed on idempotent delivery: %v", err) } @@ -195,7 +195,7 @@ This should fail and preserve the original. } logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelWarn})) - _, err := mail.Route(logger, poolDir, srcPath) + _, err := mail.Route(logger, poolDir, srcPath, nil) if err == nil { t.Fatal("expected error when inbox directory does not exist") } @@ -245,7 +245,7 @@ Unreadable source test. }) logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelWarn})) - _, err := mail.Route(logger, poolDir, srcPath) + _, err := mail.Route(logger, poolDir, srcPath, nil) if err == nil { t.Fatal("expected error when source file is unreadable") } @@ -290,7 +290,7 @@ Read-only inbox test. }) logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelWarn})) - _, err := mail.Route(logger, poolDir, srcPath) + _, err := mail.Route(logger, poolDir, srcPath, nil) if err == nil { t.Fatal("expected error when inbox directory is read-only") } @@ -315,7 +315,7 @@ func TestRoute_ParseError(t *testing.T) { } logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelWarn})) - _, err := mail.Route(logger, poolDir, srcPath) + _, err := mail.Route(logger, poolDir, srcPath, nil) if err == nil { t.Fatal("expected error for invalid message content") } From d716f6bf1bfdaae24f7195ffa977ec59729cfa55 Mon Sep 17 00:00:00 2001 From: Cameron Sjo Date: Sun, 5 Apr 2026 16:18:45 -0500 Subject: [PATCH 07/12] feat: add IsShared to ServerConfig, --shared CLI flag, WriteTempConfigShared ServerConfig gains IsShared and SharedOverlayDir for shared expert MCP. WriteTempConfigShared passes --shared true in MCP args. cmdMCP parses the flag. Daemon uses WriteTempConfigShared for shared experts. Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/agent-pool/main.go | 8 +++++++- internal/daemon/daemon.go | 2 ++ internal/mcp/config.go | 8 ++++++++ internal/mcp/config_test.go | 32 ++++++++++++++++++++++++++++++++ internal/mcp/server.go | 12 +++++++----- 5 files changed, 56 insertions(+), 6 deletions(-) diff --git a/cmd/agent-pool/main.go b/cmd/agent-pool/main.go index d823422..0fc57c6 100644 --- a/cmd/agent-pool/main.go +++ b/cmd/agent-pool/main.go @@ -431,11 +431,12 @@ func connectAndSend(sockPath, method string) (*socketResponse, error) { // agent-pool mcp --pool --expert Expert MCP server // agent-pool mcp --pool --role Built-in role MCP server func cmdMCP() { - flags := parseFlags(2, "pool", "expert", "role") + flags := parseFlags(2, "pool", "expert", "role", "shared") poolDir := flags["pool"] expertName := flags["expert"] role := flags["role"] + isShared := flags["shared"] == "true" // --role and --expert are mutually exclusive if role != "" && expertName != "" { @@ -456,9 +457,14 @@ func cmdMCP() { PoolDir: poolDir, ExpertName: expertName, Role: role, + IsShared: isShared, Logger: newStderrLogger(), } + if isShared { + cfg.SharedOverlayDir = filepath.Join(poolDir, "shared-state", expertName) + } + // Load pool config to get approval mode for architect. // Fail-closed: if config can't be loaded, the architect must not start // with an empty approval mode (which would bypass human approval). diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 80788e8..9f1572a 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -597,6 +597,8 @@ func (d *Daemon) processInboxMessage(ctx context.Context, expertName string, pat var mcpErr error if mail.IsBuiltinRole(expertName) { mcpConfigPath, mcpErr = agentmcp.WriteTempConfigForRole(d.poolDir, expertName) + } else if d.isSharedExpert(expertName) { + mcpConfigPath, mcpErr = agentmcp.WriteTempConfigShared(d.poolDir, expertName) } else { mcpConfigPath, mcpErr = agentmcp.WriteTempConfig(d.poolDir, expertName) } diff --git a/internal/mcp/config.go b/internal/mcp/config.go index 892bf73..00ef8ed 100644 --- a/internal/mcp/config.go +++ b/internal/mcp/config.go @@ -43,6 +43,14 @@ func WriteTempConfigForRole(poolDir, role string) (string, error) { return writeTempConfigWithArgs([]string{"mcp", "--pool", poolDir, "--role", role}) } +// WriteTempConfigShared writes an MCP config JSON file for a shared expert session. +// Includes --shared true so the MCP server knows the expert is shared. +func WriteTempConfigShared(poolDir, expertName string) (string, error) { + return writeTempConfigWithArgs([]string{ + "mcp", "--pool", poolDir, "--expert", expertName, "--shared", "true", + }) +} + // writeTempConfigWithArgs creates a temp MCP config JSON file pointing claude // at the current agent-pool binary with the given args. func writeTempConfigWithArgs(args []string) (string, error) { diff --git a/internal/mcp/config_test.go b/internal/mcp/config_test.go index 70aa443..1b4efd2 100644 --- a/internal/mcp/config_test.go +++ b/internal/mcp/config_test.go @@ -5,6 +5,9 @@ // - JSON matches expected MCP config schema // - File path is returned and file exists // - Caller can remove the file +// +// WriteTempConfigShared: +// - Creates valid JSON with --shared true in args package mcp_test @@ -79,3 +82,32 @@ func TestWriteTempConfig_FileCleanup(t *testing.T) { t.Error("temp file still exists after removal") } } + +func TestWriteTempConfigShared_IncludesSharedFlag(t *testing.T) { + path, err := agentmcp.WriteTempConfigShared("/tmp/test-pool", "security-standards") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + defer os.Remove(path) + + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("reading temp file: %v", err) + } + + var cfg agentmcp.MCPConfig + if err := json.Unmarshal(data, &cfg); err != nil { + t.Fatalf("invalid JSON: %v", err) + } + + entry := cfg.MCPServers["agent-pool"] + expectedArgs := []string{"mcp", "--pool", "/tmp/test-pool", "--expert", "security-standards", "--shared", "true"} + if len(entry.Args) != len(expectedArgs) { + t.Fatalf("Args = %v, want %v", entry.Args, expectedArgs) + } + for i, want := range expectedArgs { + if entry.Args[i] != want { + t.Errorf("Args[%d] = %q, want %q", i, entry.Args[i], want) + } + } +} diff --git a/internal/mcp/server.go b/internal/mcp/server.go index d0b2a21..3759b28 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -16,11 +16,13 @@ import ( // ServerConfig holds the parameters for the MCP server instance. type ServerConfig struct { - PoolDir string - ExpertName string // for experts: "auth"; for architect: "architect" - Role string // "expert" (default) or "architect" - ApprovalMode string // architect only: "none", "decomposition", "all" - Logger *slog.Logger + PoolDir string + ExpertName string // for experts: "auth"; for architect: "architect" + Role string // "expert" (default) or "architect" + ApprovalMode string // architect only: "none", "decomposition", "all" + IsShared bool // true for shared experts (user-level identity + pool overlay) + SharedOverlayDir string // pool-scoped overlay dir for shared experts + Logger *slog.Logger } // Validate checks that required fields are set. From 7ea28f044860fa9d3c57934be36eebddb4de4cd6 Mon Sep 17 00:00:00 2001 From: Cameron Sjo Date: Sun, 5 Apr 2026 16:22:32 -0500 Subject: [PATCH 08/12] feat: scope-aware update_state and read_state for shared experts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit update_state gains optional scope param (default "project"): - Shared expert + scope=project → writes to SharedOverlayDir/state.md - Shared expert + scope=user → writes to user-level expertDir/state.md - Pool-scoped expert → scope ignored, writes to expertDir as before read_state for shared experts returns project_state field from overlay. RegisterExpertTools resolves expertDir via ResolveSharedExpertDir when IsShared is true. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/mcp/testhelp_test.go | 39 +++++++++ internal/mcp/tools.go | 73 +++++++++++++--- internal/mcp/tools_test.go | 151 ++++++++++++++++++++++++++++++++++ 3 files changed, 253 insertions(+), 10 deletions(-) diff --git a/internal/mcp/testhelp_test.go b/internal/mcp/testhelp_test.go index 01a2e9a..d2a6ee6 100644 --- a/internal/mcp/testhelp_test.go +++ b/internal/mcp/testhelp_test.go @@ -201,6 +201,45 @@ func listToolNames(t *testing.T, srv *server.MCPServer) map[string]bool { return names } +// buildSharedMCPTestServer creates an MCP server configured for a shared expert. +// The expert's state dir is expertDir (user-level), with overlayDir for project state. +func buildSharedMCPTestServer(t *testing.T, poolDir, expertName, overlayDir string) *server.MCPServer { + t.Helper() + cfg := &agentmcp.ServerConfig{ + PoolDir: poolDir, + ExpertName: expertName, + Role: "expert", + IsShared: true, + SharedOverlayDir: overlayDir, + Logger: slog.New(slog.NewJSONHandler(os.Stderr, nil)), + } + srv := server.NewMCPServer("agent-pool-test", "0.5.0-test") + agentmcp.RegisterExpertTools(srv, cfg) + + initMsg := mustJSON(t, map[string]any{ + "jsonrpc": "2.0", + "id": 1, + "method": "initialize", + "params": map[string]any{ + "protocolVersion": "2025-03-26", + "capabilities": map[string]any{}, + "clientInfo": map[string]any{"name": "test", "version": "0.1"}, + }, + }) + resp := srv.HandleMessage(context.Background(), initMsg) + respBytes, _ := json.Marshal(resp) + var initResp struct { + Error *struct{ Code int; Message string } `json:"error"` + } + if err := json.Unmarshal(respBytes, &initResp); err != nil { + t.Fatalf("unmarshaling init response: %v", err) + } + if initResp.Error != nil { + t.Fatalf("MCP init failed: %d %s", initResp.Error.Code, initResp.Error.Message) + } + return srv +} + // mustJSON marshals v to JSON or fails the test. func mustJSON(t *testing.T, v any) json.RawMessage { t.Helper() diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index bfe4cad..fe83ada 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "os" "path/filepath" "strings" "time" @@ -15,27 +16,42 @@ import ( "github.com/cameronsjo/agent-pool/internal/mail" ) +// resolveExpertDirForMCP returns the state directory for the expert. +// Shared experts resolve to the user-level dir; pool-scoped experts use the pool dir. +func resolveExpertDirForMCP(cfg *ServerConfig) string { + if cfg.IsShared { + dir, err := mail.ResolveSharedExpertDir(cfg.ExpertName) + if err != nil { + // Fallback to pool-scoped path if user-level resolution fails + return mail.ResolveExpertDir(cfg.PoolDir, cfg.ExpertName) + } + return dir + } + return mail.ResolveExpertDir(cfg.PoolDir, cfg.ExpertName) +} + // RegisterExpertTools adds all expert-scope tools to the MCP server. func RegisterExpertTools(srv *server.MCPServer, cfg *ServerConfig) { if cfg == nil { return } - expertDir := mail.ResolveExpertDir(cfg.PoolDir, cfg.ExpertName) + expertDir := resolveExpertDirForMCP(cfg) srv.AddTool( mcp.NewTool("read_state", - mcp.WithDescription("Read current expert state files (identity.md, state.md, errors.md)"), + mcp.WithDescription("Read current expert state files (identity.md, state.md, errors.md). For shared experts, also returns project_state."), mcp.WithToolAnnotation(mcp.ToolAnnotation{ReadOnlyHint: boolPtr(true)}), ), - handleReadState(expertDir), + handleReadState(expertDir, cfg), ) srv.AddTool( mcp.NewTool("update_state", - mcp.WithDescription("Update the expert's working memory (state.md). Content must be non-empty and under 50KB."), + mcp.WithDescription("Update the expert's working memory (state.md). Content must be non-empty and under 50KB. For shared experts, use scope to target user-level or project-level state."), mcp.WithString("content", mcp.Required(), mcp.Description("New state.md content")), + mcp.WithString("scope", mcp.Description("For shared experts: 'project' (default) or 'user'. Ignored for pool-scoped experts.")), ), - handleUpdateState(expertDir), + handleUpdateState(expertDir, cfg), ) srv.AddTool( @@ -75,7 +91,7 @@ func RegisterExpertTools(srv *server.MCPServer, cfg *ServerConfig) { ) } -func handleReadState(expertDir string) server.ToolHandlerFunc { +func handleReadState(expertDir string, cfg *ServerConfig) server.ToolHandlerFunc { return func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { identity, state, errors, err := expert.ReadState(expertDir) if err != nil { @@ -88,6 +104,14 @@ func handleReadState(expertDir string) server.ToolHandlerFunc { "errors": errors, } + // For shared experts, also read project-level overlay state + if cfg.IsShared && cfg.SharedOverlayDir != "" { + overlayState := readOverlayState(cfg.SharedOverlayDir) + if overlayState != "" { + result["project_state"] = overlayState + } + } + data, err := json.MarshalIndent(result, "", " ") if err != nil { return mcp.NewToolResultError(fmt.Sprintf("marshaling result: %v", err)), nil @@ -97,18 +121,47 @@ func handleReadState(expertDir string) server.ToolHandlerFunc { } } -func handleUpdateState(expertDir string) server.ToolHandlerFunc { +// readOverlayState reads state.md from the overlay directory. Returns empty +// string if the file doesn't exist. +func readOverlayState(overlayDir string) string { + data, err := os.ReadFile(filepath.Join(overlayDir, "state.md")) + if err != nil { + return "" + } + return strings.TrimSpace(string(data)) +} + +func handleUpdateState(expertDir string, cfg *ServerConfig) server.ToolHandlerFunc { return func(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { content := request.GetString("content", "") if content == "" { return mcp.NewToolResultError("content parameter is required"), nil } - if err := expert.WriteState(expertDir, content); err != nil { - return mcp.NewToolResultError(fmt.Sprintf("writing state: %v", err)), nil + if !cfg.IsShared { + // Pool-scoped expert: ignore scope, write to expertDir + if err := expert.WriteState(expertDir, content); err != nil { + return mcp.NewToolResultError(fmt.Sprintf("writing state: %v", err)), nil + } + return mcp.NewToolResultText("state.md updated"), nil } - return mcp.NewToolResultText("state.md updated"), nil + // Shared expert: route by scope + scope := request.GetString("scope", "project") + switch scope { + case "user": + if err := expert.WriteState(expertDir, content); err != nil { + return mcp.NewToolResultError(fmt.Sprintf("writing state: %v", err)), nil + } + return mcp.NewToolResultText("user-level state.md updated"), nil + case "project": + if err := expert.WriteState(cfg.SharedOverlayDir, content); err != nil { + return mcp.NewToolResultError(fmt.Sprintf("writing state: %v", err)), nil + } + return mcp.NewToolResultText("project-level state.md updated"), nil + default: + return mcp.NewToolResultError(fmt.Sprintf("invalid scope %q: use 'user' or 'project'", scope)), nil + } } } diff --git a/internal/mcp/tools_test.go b/internal/mcp/tools_test.go index 0803154..2563e1f 100644 --- a/internal/mcp/tools_test.go +++ b/internal/mcp/tools_test.go @@ -1,3 +1,14 @@ +// Test plan for tools.go (v0.7 shared expert scope): +// +//update_state (shared expert): +// - scope=project → writes to SharedOverlayDir/state.md +// - scope=user → writes to expertDir/state.md (user-level) +// - default scope → project +// - pool-scoped expert → scope ignored, writes to expertDir +// +//read_state (shared expert): +// - returns project_state field alongside state +// // Test plan for tools.go: // // Each handler is tested by constructing a JSON-RPC tools/call message, @@ -305,3 +316,143 @@ func TestSearchIndex_NoMatches(t *testing.T) { t.Errorf("result = %q, want 'no matching tasks found'", text) } } + +// --- Shared expert scoped state tests --- + +// setupSharedExpertPool creates pool dirs and a user-level expert dir (simulated +// by a temp dir since we can't override HOME in unit tests). +func setupSharedExpertPool(t *testing.T) (poolDir, userExpertDir, overlayDir string) { + t.Helper() + poolDir = makePoolDirs(t, "postoffice", "shared-state/security-standards") + userExpertDir = t.TempDir() // simulates ~/.agent-pool/experts/security-standards/ + overlayDir = filepath.Join(poolDir, "shared-state", "security-standards") + return +} + +func TestUpdateState_SharedExpert_ProjectScope(t *testing.T) { + poolDir, _, overlayDir := setupSharedExpertPool(t) + srv := buildSharedMCPTestServer(t, poolDir, "security-standards", overlayDir) + + result := callTool(t, srv, "update_state", map[string]any{ + "content": "project-specific knowledge", + "scope": "project", + }) + + text := resultText(t, result) + if !strings.Contains(text, "project") { + t.Errorf("result = %q, want mention of project", text) + } + + // Verify file written to overlay dir + data, err := os.ReadFile(filepath.Join(overlayDir, "state.md")) + if err != nil { + t.Fatalf("reading overlay state.md: %v", err) + } + if !strings.Contains(string(data), "project-specific knowledge") { + t.Errorf("overlay state.md = %q, want 'project-specific knowledge'", string(data)) + } +} + +func TestUpdateState_SharedExpert_UserScope(t *testing.T) { + // Set HOME to a temp dir so ResolveSharedExpertDir resolves to a controlled path + fakeHome := t.TempDir() + origHome := os.Getenv("HOME") + os.Setenv("HOME", fakeHome) + t.Cleanup(func() { os.Setenv("HOME", origHome) }) + + // Create the user-level shared expert directory + userExpertDir := filepath.Join(fakeHome, ".agent-pool", "experts", "security-standards") + os.MkdirAll(userExpertDir, 0o755) + + poolDir, _, overlayDir := setupSharedExpertPool(t) + srv := buildSharedMCPTestServer(t, poolDir, "security-standards", overlayDir) + + result := callTool(t, srv, "update_state", map[string]any{ + "content": "user-level knowledge", + "scope": "user", + }) + + text := resultText(t, result) + if !strings.Contains(text, "user") { + t.Errorf("result = %q, want mention of user", text) + } + + // Verify file written to user-level dir + data, err := os.ReadFile(filepath.Join(userExpertDir, "state.md")) + if err != nil { + t.Fatalf("reading user-level state.md: %v", err) + } + if !strings.Contains(string(data), "user-level knowledge") { + t.Errorf("user-level state.md = %q, want 'user-level knowledge'", string(data)) + } +} + +func TestUpdateState_SharedExpert_DefaultScope(t *testing.T) { + poolDir, _, overlayDir := setupSharedExpertPool(t) + srv := buildSharedMCPTestServer(t, poolDir, "security-standards", overlayDir) + + // No scope param = defaults to "project" + result := callTool(t, srv, "update_state", map[string]any{ + "content": "default-scope content", + }) + + text := resultText(t, result) + if !strings.Contains(text, "project") { + t.Errorf("result = %q, want mention of project (default scope)", text) + } + + data, err := os.ReadFile(filepath.Join(overlayDir, "state.md")) + if err != nil { + t.Fatalf("reading overlay state.md: %v", err) + } + if !strings.Contains(string(data), "default-scope content") { + t.Errorf("overlay state.md = %q, want 'default-scope content'", string(data)) + } +} + +func TestUpdateState_PoolScoped_ScopeIgnored(t *testing.T) { + poolDir, expertDir := setupExpertPool(t, "auth") + + srv := buildMCPTestServer(t, poolDir, "auth", "") + result := callTool(t, srv, "update_state", map[string]any{ + "content": "pool-scoped state", + "scope": "user", // should be ignored for pool-scoped experts + }) + + text := resultText(t, result) + if !strings.Contains(text, "updated") { + t.Errorf("result = %q, want 'updated'", text) + } + + // Verify written to expertDir (pool-scoped) + data, err := os.ReadFile(filepath.Join(expertDir, "state.md")) + if err != nil { + t.Fatalf("reading state.md: %v", err) + } + if !strings.Contains(string(data), "pool-scoped state") { + t.Errorf("state.md = %q, want 'pool-scoped state'", string(data)) + } +} + +func TestReadState_SharedExpert_IncludesProjectState(t *testing.T) { + poolDir, _, overlayDir := setupSharedExpertPool(t) + + // The expertDir for the MCP server is resolved by RegisterExpertTools. + // For shared experts, it calls ResolveSharedExpertDir which needs HOME. + // In unit tests, we write state files to whatever dir the server uses. + // Since we can't control HOME here, we'll use the overlay dir test. + + // Write overlay state + os.WriteFile(filepath.Join(overlayDir, "state.md"), []byte("Project overlay state"), 0o644) + + srv := buildSharedMCPTestServer(t, poolDir, "security-standards", overlayDir) + result := callTool(t, srv, "read_state", map[string]any{}) + + text := resultText(t, result) + if !strings.Contains(text, "project_state") { + t.Errorf("result should contain 'project_state' key, got: %s", text) + } + if !strings.Contains(text, "Project overlay state") { + t.Errorf("result should contain overlay state content, got: %s", text) + } +} From 84d05c14a928ab0d5e77cc2b7dedeb08dffb18a9 Mon Sep 17 00:00:00 2001 From: Cameron Sjo Date: Sun, 5 Apr 2026 16:24:20 -0500 Subject: [PATCH 09/12] feat: update flush hook and concierge tools for shared expert paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FlushConfig gains IsShared — resolves user-level state dir when set. cmdFlush parses --shared flag. readExpertResult falls back to shared-state dir when pool-scoped log lookup fails. Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/agent-pool/main.go | 4 +++- internal/hooks/flush.go | 12 +++++++++++- internal/mcp/concierge_tools.go | 10 +++++++++- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/cmd/agent-pool/main.go b/cmd/agent-pool/main.go index 0fc57c6..e94c2f1 100644 --- a/cmd/agent-pool/main.go +++ b/cmd/agent-pool/main.go @@ -484,11 +484,12 @@ func cmdMCP() { } func cmdFlush() { - flags := parseFlags(2, "pool", "expert", "task") + flags := parseFlags(2, "pool", "expert", "task", "shared") poolDir := flags["pool"] expertName := flags["expert"] taskID := flags["task"] + isShared := flags["shared"] == "true" if poolDir == "" || expertName == "" { fmt.Fprintf(os.Stderr, "usage: agent-pool flush --pool --expert --task \n") @@ -499,6 +500,7 @@ func cmdFlush() { PoolDir: poolDir, ExpertName: expertName, TaskID: taskID, + IsShared: isShared, } if err := hooks.Flush(newStderrLogger(), cfg); err != nil { diff --git a/internal/hooks/flush.go b/internal/hooks/flush.go index b6c5936..6ccd1ac 100644 --- a/internal/hooks/flush.go +++ b/internal/hooks/flush.go @@ -20,6 +20,7 @@ type FlushConfig struct { PoolDir string ExpertName string TaskID string + IsShared bool // true for shared experts (user-level identity + pool overlay) } // Flush is the Stop hook safety net. It verifies that state.md was updated @@ -42,7 +43,16 @@ func Flush(logger *slog.Logger, cfg *FlushConfig) error { return fmt.Errorf("invalid expert name %q: must not contain path separators", cfg.ExpertName) } - expertDir := mail.ResolveExpertDir(cfg.PoolDir, cfg.ExpertName) + var expertDir string + if cfg.IsShared { + dir, resolveErr := mail.ResolveSharedExpertDir(cfg.ExpertName) + if resolveErr != nil { + return fmt.Errorf("resolving shared expert dir: %w", resolveErr) + } + expertDir = dir + } else { + expertDir = mail.ResolveExpertDir(cfg.PoolDir, cfg.ExpertName) + } statePath := filepath.Join(expertDir, "state.md") info, err := os.Stat(statePath) diff --git a/internal/mcp/concierge_tools.go b/internal/mcp/concierge_tools.go index edef68e..2b228b5 100644 --- a/internal/mcp/concierge_tools.go +++ b/internal/mcp/concierge_tools.go @@ -354,12 +354,20 @@ func pollForCompletion(ctx context.Context, cfg *ServerConfig, taskID string) (s } // readExpertResult reads the expert's session log and extracts the full result. +// Checks pool-scoped expert dir first, then shared-state dir for shared experts. func readExpertResult(poolDir, expertName, taskID string) (string, error) { expertDir := mail.ResolveExpertDir(poolDir, expertName) logData, err := expert.ReadLog(expertDir, taskID) if err != nil { - return "", fmt.Errorf("reading expert log: %w", err) + // Try shared-state dir — shared expert logs live there + sharedDir := filepath.Join(poolDir, "shared-state", expertName) + logData, sharedErr := expert.ReadLog(sharedDir, taskID) + if sharedErr != nil { + return "", fmt.Errorf("reading expert log: %w (also tried shared-state: %v)", err, sharedErr) + } + result := expert.ExtractResult(logData) + return result, nil } result := expert.ExtractResult(logData) From 8db97c7dbbf6bae4f45a1578588c9d78b695dc60 Mon Sep 17 00:00:00 2001 From: Cameron Sjo Date: Sun, 5 Apr 2026 16:25:00 -0500 Subject: [PATCH 10/12] feat: warn at startup if shared expert missing identity.md Non-fatal warning during ensureDirs when a shared.include entry has no identity.md at ~/.agent-pool/experts/{name}/. Helps catch misconfigured shared expert references early. Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/daemon/daemon.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index 9f1572a..a7ed8e8 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -1215,6 +1215,21 @@ func (d *Daemon) ensureDirs() error { } } + // Warn if shared expert user-level directory is missing identity.md + for _, name := range d.cfg.Shared.Include { + userDir, resolveErr := mail.ResolveSharedExpertDir(name) + if resolveErr != nil { + continue + } + identityPath := filepath.Join(userDir, "identity.md") + if _, err := os.Stat(identityPath); os.IsNotExist(err) { + d.logger.Warn("Shared expert missing identity.md at user level", + "expert", name, + "expected", identityPath, + ) + } + } + return nil } From 1548090c0766720897fbb2830bf2ef14952df917 Mon Sep 17 00:00:00 2001 From: Cameron Sjo Date: Mon, 6 Apr 2026 17:26:54 -0500 Subject: [PATCH 11/12] docs: add v0.7 shared experts implementation plan Approved plan covering 4 phases: shared expert resolution, layered prompt assembly, scoped state writes, and startup validation. Co-Authored-By: Claude Opus 4.6 (1M context) --- docs/plans/2026-04-05-shared-experts.md | 240 ++++++++++++++++++++++++ 1 file changed, 240 insertions(+) create mode 100644 docs/plans/2026-04-05-shared-experts.md diff --git a/docs/plans/2026-04-05-shared-experts.md b/docs/plans/2026-04-05-shared-experts.md new file mode 100644 index 0000000..2145a73 --- /dev/null +++ b/docs/plans/2026-04-05-shared-experts.md @@ -0,0 +1,240 @@ +# v0.7 Shared Experts Implementation Plan + +## Context + +Experts are currently pool-scoped — identity, state, logs all live under `{poolDir}/experts/{name}/`. This means a `security-standards` expert bootstrapped in one pool can't be reused in another without duplicating everything. v0.7 lifts experts to user scope (`~/.agent-pool/experts/`), with per-pool project overlays for project-specific state. Pools opt in via `shared.include` in pool.toml. + +**Design decisions (resolved per v0.7 prompt):** +- Inbox: pool-scoped at `{poolDir}/shared-state/{name}/inbox/` +- Logs: pool-scoped at `{poolDir}/shared-state/{name}/logs/` +- Errors: user-level only at `~/.agent-pool/experts/{name}/errors.md` +- Multi-pool: deferred — separate `agent-pool start` processes per pool + +## Phase 1: Shared Expert Resolution + +### 1.1 — `config.SharedExpertDir` helper + +**File:** `internal/config/config.go` + +```go +func SharedExpertDir(name string) (string, error) +``` + +Returns `~/.agent-pool/experts/{name}/`. Validates name (no path separators, not empty, not `.`/`..`). + +**Tests** (`internal/config/config_test.go`): happy path, path traversal rejection, empty name. + +### 1.2 — Shared resolution functions in mail package + +**File:** `internal/mail/router.go` + +Add three new functions (existing `ResolveExpertDir`/`ResolveInbox` unchanged): + +```go +func ResolveSharedExpertDir(name string) (string, error) // → ~/.agent-pool/experts/{name}/ +func ResolveSharedInbox(poolDir, name string) string // → {poolDir}/shared-state/{name}/inbox/ +func ResolveSharedLogDir(poolDir, name string) string // → {poolDir}/shared-state/{name}/logs/ +``` + +`ResolveSharedExpertDir` delegates to `config.SharedExpertDir`. + +**Tests** (`internal/mail/router_test.go`): verify paths for each function. + +### 1.3 — Config validation + +**File:** `internal/config/config.go` + +Add `Validate()` method on `PoolConfig`, called at end of `LoadPool`: +- `shared.include` names must not be builtin roles +- `shared.include` names must not overlap with `[experts.*]` keys +- Names must be filename-safe (no path separators) + +**Tests** (`internal/config/config_test.go`): conflict with builtin, conflict with pool-scoped, path traversal, happy path. + +### 1.4 — `ensureDirs` + `isSharedExpert` helper + +**File:** `internal/daemon/daemon.go` + +```go +func (d *Daemon) isSharedExpert(name string) bool +``` + +In `ensureDirs()`, iterate `d.cfg.Shared.Include` and create: +- `{poolDir}/shared-state/{name}/inbox/` +- `{poolDir}/shared-state/{name}/logs/` + +**Test** (`internal/daemon/daemon_test.go`): verify dirs created for shared.include entries. + +--- + +## Phase 2: Layered Prompt Assembly + +### 2.1 — `SpawnConfig.OverlayDir` + `AssemblePrompt` update + +**File:** `internal/expert/expert.go` + +Add `OverlayDir string` to `SpawnConfig`. + +Update `AssemblePrompt`: after the "## Current State" section (user-level), if `OverlayDir` is set, read `{OverlayDir}/state.md` and emit "## Project State" section. Missing overlay file is silently skipped. + +**Tests** (`internal/expert/expert_test.go`): +- Both user-level and overlay state present → two sections +- Overlay only (no user state.md) → just project state section +- OverlayDir set but no state.md file → graceful skip +- OverlayDir empty string → no overlay section (backward compat) + +### 2.2 — Daemon shared expert spawn wiring + +**File:** `internal/daemon/daemon.go` + +Update `processInboxMessage` (~line 559-595): + +```go +var expertDir, overlayDir, logDir string +if d.isSharedExpert(expertName) { + userDir, err := mail.ResolveSharedExpertDir(expertName) + // ... error handling + expertDir = userDir + overlayDir = filepath.Join(d.poolDir, "shared-state", expertName) + logDir = mail.ResolveSharedLogDir(d.poolDir, expertName) +} else { + expertDir = d.resolveExpertDir(expertName) + logDir = expertDir +} +``` + +Set `OverlayDir` on `SpawnConfig`. Write logs to `logDir` instead of `expertDir` for shared experts. + +Update MCP config creation to use `WriteTempConfigShared` for shared experts. + +**Tests** (`internal/daemon/daemon_test.go`): +- `TestSharedExpert_SpawnConfig` — verify ExpertDir is user-level, OverlayDir is pool-scoped +- `TestSharedExpert_LogsInPoolScope` — logs written to shared-state/{name}/logs/ + +### 2.3 — Watcher + inbox routing for shared experts + +**File:** `internal/daemon/daemon.go` + +Three changes in `Run()` (~line 156-162): +1. Watch shared expert inboxes alongside pool-scoped ones +2. Update `resolveExpertName` to match shared inbox paths +3. Update `drainAllInboxes` to drain shared expert inboxes +4. Update `drainInbox` to resolve shared inbox paths + +Also update `handleCancel` (line 369) to resolve shared inbox paths. + +Also update `Route` in `internal/mail/router.go` — add `sharedNames map[string]bool` parameter so it routes to `ResolveSharedInbox` for shared experts. The daemon passes `d.sharedNamesMap()`. + +**Call sites for `mail.Route`:** Only `daemon.handlePostoffice` (daemon.go:259). + +**Call sites for `mail.ResolveInbox`:** +- daemon.go:145 (architect inbox watch) — unchanged +- daemon.go:158 (expert inbox watch) — unchanged, shared handled separately +- daemon.go:369 (handleCancel) — needs shared check +- daemon.go:834 (drainInbox) — needs shared check +- daemon.go:1109-1115 (resolveExpertName) — needs shared check + +**Tests** (`internal/daemon/daemon_test.go`): +- `TestSharedExpert_InboxDrainedOnStartup` — pre-existing message in shared inbox processed +- `TestSharedExpert_RouteToSharedInbox` — message addressed to shared expert lands in shared-state inbox +- `TestSharedAndPoolScoped_Coexist` — both types work in same pool + +--- + +## Phase 3: Scoped State Writes + +### 3.1 — `ServerConfig.IsShared` + `--shared` CLI flag + +**File:** `internal/mcp/server.go` — add `IsShared bool` and `SharedOverlayDir string` to `ServerConfig`. + +**File:** `internal/mcp/config.go` — add `WriteTempConfigShared(poolDir, expertName string)` that passes `--shared true` in MCP args. + +**File:** `cmd/agent-pool/main.go` — update `cmdMCP` to parse `--shared` flag, set `ServerConfig.IsShared` and compute `SharedOverlayDir`. + +### 3.2 — Scope-aware `update_state` and `read_state` + +**File:** `internal/mcp/tools.go` + +Update `RegisterExpertTools` to resolve expertDir via shared-aware helper and pass `cfg` to handlers. + +`update_state` gains `scope` parameter (optional, default `"project"`): +- Pool-scoped expert: `scope` ignored, writes to `expertDir/state.md` as today +- Shared expert + `scope=project`: writes to `{SharedOverlayDir}/state.md` +- Shared expert + `scope=user`: writes to `{expertDir}/state.md` (user-level) + +`read_state` for shared experts: returns `project_state` field alongside existing `state` field. + +`append_error`: unchanged — `expertDir` is already user-level for shared experts. + +**Tests** (`internal/mcp/tools_test.go`): +- Shared expert: project scope write, user scope write, default scope +- Pool-scoped: scope parameter ignored +- read_state with project_state for shared expert + +### 3.3 — Update flush hook + concierge tools + +**File:** `internal/hooks/flush.go` (line 45) — needs to resolve shared expert dir correctly. Add `IsShared bool` to `FlushConfig`, use `mail.ResolveSharedExpertDir` when true. + +**File:** `cmd/agent-pool/main.go` — update `cmdFlush` to parse `--shared`. + +**File:** `internal/mcp/concierge_tools.go` (line 358) — `readExpertResult` reads logs from expertDir. For shared experts, logs are in `{poolDir}/shared-state/{name}/logs/`. Need to check shared.include list. Since concierge has pool config access, pass shared names through. + +--- + +## Phase 4: Startup Validation + +At the end of `ensureDirs`, warn (don't error) if a shared expert's user-level directory is missing `identity.md`: + +```go +for _, name := range d.cfg.Shared.Include { + userDir, _ := mail.ResolveSharedExpertDir(name) + if _, err := os.Stat(filepath.Join(userDir, "identity.md")); os.IsNotExist(err) { + d.logger.Warn("Shared expert missing identity.md", "expert", name, "path", userDir) + } +} +``` + +--- + +## Commit Sequence + +| # | Scope | Files | Risk | +|---|-------|-------|------| +| 1 | `config.SharedExpertDir` | config.go, config_test.go | None — pure addition | +| 2 | `mail.ResolveShared*` functions | router.go, router_test.go | None — pure addition | +| 3 | Config validation | config.go, config_test.go | Low — existing valid configs unchanged | +| 4 | `ensureDirs` + `isSharedExpert` | daemon.go, daemon_test.go | Low — new dirs only | +| 5 | `SpawnConfig.OverlayDir` + `AssemblePrompt` | expert.go, expert_test.go | Low — empty OverlayDir is no-op | +| 6 | Daemon shared expert spawn | daemon.go, daemon_test.go | Medium — spawn path diverges | +| 7 | Watcher + Route + inbox routing | daemon.go, router.go, *_test.go | Medium — Route signature changes | +| 8 | `ServerConfig.IsShared` + `--shared` + `WriteTempConfigShared` | server.go, config.go, main.go | Low — additive | +| 9 | Scope-aware MCP tools | tools.go, tools_test.go | Medium — handler logic branches | +| 10 | Flush hook + concierge tools | flush.go, concierge_tools.go, main.go | Low — path resolution | +| 11 | Startup validation | daemon.go | None — warn only | + +--- + +## Verification + +1. `make test` — full suite passes after each commit +2. **Manual integration test:** + - Create `~/.agent-pool/experts/test-shared/identity.md` with content + - Create pool with `shared.include = ["test-shared"]` and a pool-scoped expert + - Send task to shared expert → verify prompt includes identity from user-level + - Call `update_state` with scope=project → verify file at `shared-state/test-shared/state.md` + - Call `update_state` with scope=user → verify file at `~/.agent-pool/experts/test-shared/state.md` + - Send task to pool-scoped expert → verify unchanged behavior +3. `make test-cover` — new code has tests + +## Critical Files + +- `internal/config/config.go` — SharedExpertDir, Validate +- `internal/mail/router.go` — ResolveShared*, Route signature +- `internal/expert/expert.go` — SpawnConfig.OverlayDir, AssemblePrompt +- `internal/daemon/daemon.go` — processInboxMessage, ensureDirs, resolveExpertName, drainInbox, handleCancel, Run +- `internal/mcp/tools.go` — RegisterExpertTools, handleUpdateState, handleReadState +- `internal/mcp/config.go` — WriteTempConfigShared +- `internal/mcp/server.go` — ServerConfig.IsShared +- `internal/hooks/flush.go` — FlushConfig.IsShared +- `internal/mcp/concierge_tools.go` — readExpertResult +- `cmd/agent-pool/main.go` — cmdMCP, cmdFlush From 24a6deab1ca5b95e2034e3569f88178ce7e0465f Mon Sep 17 00:00:00 2001 From: Cameron Sjo Date: Mon, 6 Apr 2026 17:45:13 -0500 Subject: [PATCH 12/12] fix: address review findings (round 1) - Extract BuiltinRoleNames to config package as single source of truth, mail.IsBuiltinRole now delegates to config.BuiltinRoleNames - Guard empty SharedOverlayDir before project-scope write (Major finding) - Return error from resolveExpertDirForMCP instead of silent fallback - Cache sharedSet in Daemon struct, isSharedExpert is now O(1) - Use t.Setenv instead of manual HOME manipulation in tests - Check os.MkdirAll/os.WriteFile errors in test setup Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/config/config.go | 8 ++++---- internal/daemon/daemon.go | 28 +++++++++++++--------------- internal/daemon/daemon_test.go | 12 +++++++----- internal/mail/router.go | 13 +++---------- internal/mcp/tools.go | 21 +++++++++++++++------ internal/mcp/tools_test.go | 8 ++++---- 6 files changed, 46 insertions(+), 44 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 11d4047..d4f70e4 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -79,9 +79,9 @@ type ExpertSection struct { AllowedTools []string `toml:"allowed_tools"` } -// builtinRoleNames are the built-in role names that shared experts must not -// conflict with. Kept in sync with mail.builtinRoles. -var builtinRoleNames = map[string]bool{ +// BuiltinRoleNames is the canonical set of built-in role names. The mail +// package uses this via IsBuiltinRole to avoid duplicating the list. +var BuiltinRoleNames = map[string]bool{ "architect": true, "researcher": true, "concierge": true, @@ -95,7 +95,7 @@ func (c *PoolConfig) Validate() error { if name == "" || name != filepath.Base(name) || name == "." || name == ".." { return fmt.Errorf("invalid shared expert name %q: must be a simple filename", name) } - if builtinRoleNames[name] { + if BuiltinRoleNames[name] { return fmt.Errorf("shared expert %q conflicts with built-in role", name) } if c.Experts != nil { diff --git a/internal/daemon/daemon.go b/internal/daemon/daemon.go index a7ed8e8..6bf5fd5 100644 --- a/internal/daemon/daemon.go +++ b/internal/daemon/daemon.go @@ -56,6 +56,7 @@ type Daemon struct { startedAt time.Time sockPathOver string // overrides default socket path (for tests with long TempDir paths) events *eventBus + sharedSet map[string]bool // cached shared.include lookup set, built once in New } // Option configures a Daemon. @@ -112,6 +113,15 @@ func New(cfg *config.PoolConfig, poolDir string, logger *slog.Logger, opts ...Op drainTimeout: 30 * time.Second, events: newEventBus(), } + + // Build cached shared expert lookup set + if len(cfg.Shared.Include) > 0 { + d.sharedSet = make(map[string]bool, len(cfg.Shared.Include)) + for _, name := range cfg.Shared.Include { + d.sharedSet[name] = true + } + } + for _, opt := range opts { opt(d) } @@ -1233,26 +1243,14 @@ func (d *Daemon) ensureDirs() error { return nil } -// sharedNamesMap returns the shared.include list as a set for O(1) lookup. +// sharedNamesMap returns the cached shared expert lookup set. May be nil. func (d *Daemon) sharedNamesMap() map[string]bool { - if len(d.cfg.Shared.Include) == 0 { - return nil - } - m := make(map[string]bool, len(d.cfg.Shared.Include)) - for _, name := range d.cfg.Shared.Include { - m[name] = true - } - return m + return d.sharedSet } // isSharedExpert reports whether the named expert is in the pool's shared.include list. func (d *Daemon) isSharedExpert(name string) bool { - for _, shared := range d.cfg.Shared.Include { - if shared == name { - return true - } - } - return false + return d.sharedSet[name] } // absEqual compares two paths after resolving to absolute form. diff --git a/internal/daemon/daemon_test.go b/internal/daemon/daemon_test.go index 158cde0..4c696d7 100644 --- a/internal/daemon/daemon_test.go +++ b/internal/daemon/daemon_test.go @@ -293,14 +293,16 @@ model = "sonnet" func TestDaemon_SharedExpertSpawnConfig(t *testing.T) { // Set HOME to a temp dir so SharedExpertDir resolves to a controlled location fakeHome := t.TempDir() - origHome := os.Getenv("HOME") - os.Setenv("HOME", fakeHome) - t.Cleanup(func() { os.Setenv("HOME", origHome) }) + t.Setenv("HOME", fakeHome) // Create the user-level shared expert directory sharedExpertDir := filepath.Join(fakeHome, ".agent-pool", "experts", "security-standards") - os.MkdirAll(sharedExpertDir, 0o755) - os.WriteFile(filepath.Join(sharedExpertDir, "identity.md"), []byte("I am the security expert."), 0o644) + if err := os.MkdirAll(sharedExpertDir, 0o755); err != nil { + t.Fatalf("creating shared expert dir: %v", err) + } + if err := os.WriteFile(filepath.Join(sharedExpertDir, "identity.md"), []byte("I am the security expert."), 0o644); err != nil { + t.Fatalf("writing identity.md: %v", err) + } poolDir := t.TempDir() cfg := writePoolConfig(t, poolDir, `[pool] diff --git a/internal/mail/router.go b/internal/mail/router.go index 4cecdcf..f2a9e7c 100644 --- a/internal/mail/router.go +++ b/internal/mail/router.go @@ -10,23 +10,16 @@ import ( "github.com/cameronsjo/agent-pool/internal/config" ) -// builtinRoles are roles with top-level directories (not under experts/). -var builtinRoles = map[string]bool{ - "architect": true, - "researcher": true, - "concierge": true, -} - // IsBuiltinRole reports whether the given name is a built-in role (architect, // researcher, concierge) rather than a pool-scoped expert. func IsBuiltinRole(name string) bool { - return builtinRoles[name] + return config.BuiltinRoleNames[name] } // ResolveExpertDir returns the state directory for an expert or built-in role. // Built-in roles use {poolDir}/{role}/, experts use {poolDir}/experts/{name}/. func ResolveExpertDir(poolDir, name string) string { - if builtinRoles[name] { + if config.BuiltinRoleNames[name] { return filepath.Join(poolDir, name) } return filepath.Join(poolDir, "experts", name) @@ -42,7 +35,7 @@ func ResolveExpertDir(poolDir, name string) string { // // {poolDir}/experts/{name}/inbox/ func ResolveInbox(poolDir, recipient string) string { - if builtinRoles[recipient] { + if config.BuiltinRoleNames[recipient] { return filepath.Join(poolDir, recipient, "inbox") } return filepath.Join(poolDir, "experts", recipient, "inbox") diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index fe83ada..60cb913 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -18,16 +18,15 @@ import ( // resolveExpertDirForMCP returns the state directory for the expert. // Shared experts resolve to the user-level dir; pool-scoped experts use the pool dir. -func resolveExpertDirForMCP(cfg *ServerConfig) string { +func resolveExpertDirForMCP(cfg *ServerConfig) (string, error) { if cfg.IsShared { dir, err := mail.ResolveSharedExpertDir(cfg.ExpertName) if err != nil { - // Fallback to pool-scoped path if user-level resolution fails - return mail.ResolveExpertDir(cfg.PoolDir, cfg.ExpertName) + return "", fmt.Errorf("resolving shared expert dir for %q: %w", cfg.ExpertName, err) } - return dir + return dir, nil } - return mail.ResolveExpertDir(cfg.PoolDir, cfg.ExpertName) + return mail.ResolveExpertDir(cfg.PoolDir, cfg.ExpertName), nil } // RegisterExpertTools adds all expert-scope tools to the MCP server. @@ -35,7 +34,14 @@ func RegisterExpertTools(srv *server.MCPServer, cfg *ServerConfig) { if cfg == nil { return } - expertDir := resolveExpertDirForMCP(cfg) + expertDir, err := resolveExpertDirForMCP(cfg) + if err != nil { + cfg.Logger.Error("Failed to resolve expert directory, tools will not function correctly", + "expert", cfg.ExpertName, + "error", err, + ) + return + } srv.AddTool( mcp.NewTool("read_state", @@ -155,6 +161,9 @@ func handleUpdateState(expertDir string, cfg *ServerConfig) server.ToolHandlerFu } return mcp.NewToolResultText("user-level state.md updated"), nil case "project": + if cfg.SharedOverlayDir == "" { + return mcp.NewToolResultError("shared expert has no project overlay directory configured"), nil + } if err := expert.WriteState(cfg.SharedOverlayDir, content); err != nil { return mcp.NewToolResultError(fmt.Sprintf("writing state: %v", err)), nil } diff --git a/internal/mcp/tools_test.go b/internal/mcp/tools_test.go index 2563e1f..1ce8b71 100644 --- a/internal/mcp/tools_test.go +++ b/internal/mcp/tools_test.go @@ -356,13 +356,13 @@ func TestUpdateState_SharedExpert_ProjectScope(t *testing.T) { func TestUpdateState_SharedExpert_UserScope(t *testing.T) { // Set HOME to a temp dir so ResolveSharedExpertDir resolves to a controlled path fakeHome := t.TempDir() - origHome := os.Getenv("HOME") - os.Setenv("HOME", fakeHome) - t.Cleanup(func() { os.Setenv("HOME", origHome) }) + t.Setenv("HOME", fakeHome) // Create the user-level shared expert directory userExpertDir := filepath.Join(fakeHome, ".agent-pool", "experts", "security-standards") - os.MkdirAll(userExpertDir, 0o755) + if err := os.MkdirAll(userExpertDir, 0o755); err != nil { + t.Fatalf("creating shared expert dir: %v", err) + } poolDir, _, overlayDir := setupSharedExpertPool(t) srv := buildSharedMCPTestServer(t, poolDir, "security-standards", overlayDir)