diff --git a/internal/agent/loop_history.go b/internal/agent/loop_history.go index 37ec117c..91f20e48 100644 --- a/internal/agent/loop_history.go +++ b/internal/agent/loop_history.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "log/slog" + "path/filepath" "strings" "github.com/google/uuid" @@ -307,14 +308,14 @@ func (l *Loop) buildMessages(ctx context.Context, history []providers.Message, s // but base-only files (like auto-injected delegation info) are preserved. func (l *Loop) resolveContextFiles(ctx context.Context, userID string) []bootstrap.ContextFile { if l.contextFileLoader == nil || userID == "" { - return l.contextFiles + return dropBuiltinUserFileIfPredefined(l.agentType, l.contextFiles) } userFiles := l.contextFileLoader(ctx, l.agentUUID, userID, l.agentType) if len(userFiles) == 0 { - return l.contextFiles + return dropBuiltinUserFileIfPredefined(l.agentType, l.contextFiles) } if len(l.contextFiles) == 0 { - return userFiles + return dropBuiltinUserFileIfPredefined(l.agentType, userFiles) } // Merge: start with per-user files, then append base-only files @@ -329,7 +330,36 @@ func (l *Loop) resolveContextFiles(ctx context.Context, userID string) []bootstr merged = append(merged, base) } } - return merged + return dropBuiltinUserFileIfPredefined(l.agentType, merged) +} + +// dropBuiltinUserFileIfPredefined removes the built-in USER.md entry from the +// merged context files when the agent is predefined AND has an operator-authored +// USER_PREDEFINED.md. The operator owns the entire user-context portion of the +// system prompt in that case, so the built-in USER.md template must never be +// injected alongside it (per-turn name/timezone/pronoun nag). +func dropBuiltinUserFileIfPredefined(agentType string, files []bootstrap.ContextFile) []bootstrap.ContextFile { + if agentType != store.AgentTypePredefined { + return files + } + hasUserPredefined := false + for _, f := range files { + if filepath.Base(f.Path) == bootstrap.UserPredefinedFile { + hasUserPredefined = true + break + } + } + if !hasUserPredefined { + return files + } + filtered := make([]bootstrap.ContextFile, 0, len(files)) + for _, f := range files { + if filepath.Base(f.Path) == bootstrap.UserFile { + continue + } + filtered = append(filtered, f) + } + return filtered } // mergeContextFallback adds fallback (in-memory) files into contextFiles, diff --git a/internal/agent/loop_history_userpredefined_test.go b/internal/agent/loop_history_userpredefined_test.go new file mode 100644 index 00000000..37788954 --- /dev/null +++ b/internal/agent/loop_history_userpredefined_test.go @@ -0,0 +1,54 @@ +package agent + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/nextlevelbuilder/goclaw/internal/bootstrap" + "github.com/nextlevelbuilder/goclaw/internal/store" +) + +// TestDropBuiltinUserFileIfPredefined_WithUserPredefined verifies that USER.md +// is dropped from the merged context files for predefined agents when +// USER_PREDEFINED.md is present — the operator owns the user-context prompt. +func TestDropBuiltinUserFileIfPredefined_WithUserPredefined(t *testing.T) { + files := []bootstrap.ContextFile{ + {Path: bootstrap.UserFile, Content: "built-in user template"}, + {Path: bootstrap.UserPredefinedFile, Content: "operator authored"}, + {Path: bootstrap.SoulFile, Content: "soul"}, + } + + got := dropBuiltinUserFileIfPredefined(store.AgentTypePredefined, files) + + for _, f := range got { + assert.NotEqual(t, bootstrap.UserFile, f.Path, "USER.md must be dropped when USER_PREDEFINED.md is present") + } + assert.Len(t, got, 2) +} + +// TestDropBuiltinUserFileIfPredefined_WithoutUserPredefined verifies USER.md is +// kept for predefined agents that do NOT have USER_PREDEFINED.md. +func TestDropBuiltinUserFileIfPredefined_WithoutUserPredefined(t *testing.T) { + files := []bootstrap.ContextFile{ + {Path: bootstrap.UserFile, Content: "built-in user template"}, + {Path: bootstrap.SoulFile, Content: "soul"}, + } + + got := dropBuiltinUserFileIfPredefined(store.AgentTypePredefined, files) + + assert.Equal(t, files, got) +} + +// TestDropBuiltinUserFileIfPredefined_OpenAgentUntouched verifies open agents +// are never affected, even if USER_PREDEFINED.md were somehow present. +func TestDropBuiltinUserFileIfPredefined_OpenAgentUntouched(t *testing.T) { + files := []bootstrap.ContextFile{ + {Path: bootstrap.UserFile, Content: "built-in user template"}, + {Path: bootstrap.UserPredefinedFile, Content: "operator authored"}, + } + + got := dropBuiltinUserFileIfPredefined(store.AgentTypeOpen, files) + + assert.Equal(t, files, got) +} diff --git a/internal/bootstrap/seed_store.go b/internal/bootstrap/seed_store.go index a166811f..2a631145 100644 --- a/internal/bootstrap/seed_store.go +++ b/internal/bootstrap/seed_store.go @@ -196,10 +196,35 @@ func SeedUserFiles(ctx context.Context, agentStore store.AgentStore, agentID uui return nil, nil } + // For predefined agents: load agent-level files once to use as seed fallback. + // USER.md at agent-level may contain a pre-configured owner profile (e.g. set by + // the wizard or management dashboard). Use it as the per-user seed instead of the + // blank embedded template so the agent starts with the correct owner context. + var agentLevelFiles map[string]string + if agentType == store.AgentTypePredefined { + agentFiles, err := agentStore.GetAgentContextFiles(ctx, agentID) + if err == nil && len(agentFiles) > 0 { + agentLevelFiles = make(map[string]string, len(agentFiles)) + for _, f := range agentFiles { + if f.Content != "" { + agentLevelFiles[f.FileName] = f.Content + } + } + } + } + + // When a predefined agent has an operator-authored USER_PREDEFINED.md, the + // operator owns the entire user-context portion of the system prompt. Skip + // seeding the built-in USER.md and BOOTSTRAP.md templates so they never + // impose themselves (or their per-turn name/timezone/pronoun nag) on top + // of the operator's own content. + _, hasUserPredefined := agentLevelFiles[UserPredefinedFile] + skipBuiltinUserFiles := agentType == store.AgentTypePredefined && hasUserPredefined + // Channel-provided contact info: skip bootstrap, pre-fill USER.md directly. // Currently only Pancake channel (Facebook Messenger) provides enough user info // (display_name from Facebook profile) to skip the interactive onboarding flow. - if shouldSkipBootstrap(channelMeta) { + if !skipBuiltinUserFiles && shouldSkipBootstrap(channelMeta) { userContent := buildPrefilledUser(channelMeta) if err := retryOnBusy(func() error { return agentStore.SetUserContextFile(ctx, agentID, userID, UserFile, userContent) @@ -241,29 +266,16 @@ func SeedUserFiles(ctx context.Context, agentStore store.AgentStore, agentID uui } } - // For predefined agents: load agent-level files once to use as seed fallback. - // USER.md at agent-level may contain a pre-configured owner profile (e.g. set by - // the wizard or management dashboard). Use it as the per-user seed instead of the - // blank embedded template so the agent starts with the correct owner context. - var agentLevelFiles map[string]string - if agentType == store.AgentTypePredefined { - agentFiles, err := agentStore.GetAgentContextFiles(ctx, agentID) - if err == nil && len(agentFiles) > 0 { - agentLevelFiles = make(map[string]string, len(agentFiles)) - for _, f := range agentFiles { - if f.Content != "" { - agentLevelFiles[f.FileName] = f.Content - } - } - } - } - var seeded []string for _, name := range files { if hasFile[name] { continue // already has personalized content, don't overwrite } + if skipBuiltinUserFiles && (name == UserFile || name == BootstrapFile) { + continue // operator-authored USER_PREDEFINED.md owns the user-context prompt + } + // For predefined agents seeding USER.md: prefer agent-level content as seed. // This propagates wizard/dashboard-configured owner profile to the first user. if agentType == store.AgentTypePredefined && name == UserFile { diff --git a/internal/bootstrap/seed_store_test.go b/internal/bootstrap/seed_store_test.go index 5fcb8a30..e83f0105 100644 --- a/internal/bootstrap/seed_store_test.go +++ b/internal/bootstrap/seed_store_test.go @@ -235,6 +235,57 @@ func TestSeedUserFiles_PredefinedAgent_DoesNotOverwriteExistingPerUserContent(t } } +// TestSeedUserFiles_PredefinedAgent_WithUserPredefined_SkipsBuiltinUserAndBootstrap +// verifies that when a predefined agent has an operator-authored USER_PREDEFINED.md, +// SeedUserFiles skips seeding both the built-in USER.md and BOOTSTRAP.md templates — +// the operator owns the user-context portion of the system prompt. +func TestSeedUserFiles_PredefinedAgent_WithUserPredefined_SkipsBuiltinUserAndBootstrap(t *testing.T) { + as := newSeedStub() + agentID := uuid.New() + as.agentFiles[UserPredefinedFile] = "# USER_PREDEFINED.md\nOperator-authored user context" + + seeded, err := SeedUserFiles(context.Background(), as, agentID, "user-erin", store.AgentTypePredefined, false, nil) + if err != nil { + t.Fatalf("SeedUserFiles returned error: %v", err) + } + + for _, f := range seeded { + if f == UserFile || f == BootstrapFile { + t.Errorf("USER.md/BOOTSTRAP.md must not be seeded when USER_PREDEFINED.md exists, got seeded=%v", seeded) + } + } + if _, wrote := as.seededUserFiles[UserFile]; wrote { + t.Error("USER.md should not be written when USER_PREDEFINED.md exists") + } + if _, wrote := as.seededUserFiles[BootstrapFile]; wrote { + t.Error("BOOTSTRAP.md should not be written when USER_PREDEFINED.md exists") + } +} + +// TestSeedUserFiles_PredefinedAgent_WithUserPredefined_SkipsChannelPrefill verifies +// that the channel-provided contact info prefill path (shouldSkipBootstrap) also +// respects USER_PREDEFINED.md and does not write a prefilled USER.md. +func TestSeedUserFiles_PredefinedAgent_WithUserPredefined_SkipsChannelPrefill(t *testing.T) { + as := newSeedStub() + agentID := uuid.New() + as.agentFiles[UserPredefinedFile] = "# USER_PREDEFINED.md\nOperator-authored user context" + meta := &ChannelMeta{ChannelType: "pancake", DisplayName: "Alice", DefaultTimezone: "Asia/Ho_Chi_Minh"} + + seeded, err := SeedUserFiles(context.Background(), as, agentID, "user-fay", store.AgentTypePredefined, false, meta) + if err != nil { + t.Fatalf("SeedUserFiles returned error: %v", err) + } + + for _, f := range seeded { + if f == UserFile { + t.Errorf("USER.md must not be seeded via channel prefill when USER_PREDEFINED.md exists, got seeded=%v", seeded) + } + } + if _, wrote := as.seededUserFiles[UserFile]; wrote { + t.Error("USER.md should not be written via channel prefill when USER_PREDEFINED.md exists") + } +} + // TestSeedUserFiles_OpenAgent_UsesEmbeddedTemplate verifies that open agents // are completely unaffected — they still receive embedded templates per-user. func TestSeedUserFiles_OpenAgent_UsesEmbeddedTemplate(t *testing.T) {