fix: differentiate /clear from /new and address PR review feedback

- Split /new and /clear into separate command handlers with distinct behavior:
  /new creates a new session key and sets an override (fresh start)
  /clear resets history and summary in-place, keeping the same session key
- Delete old session files on /new to prevent unbounded disk accumulation
- Return user-friendly error messages instead of raw internal errors
- Add SessionManager.Delete and ClearSession methods
- Update and add tests for the new differentiated behavior

pkg/agent/loop.go

@@ -462,9 +462,66 @@ func (al *AgentLoop) startNewSessionForMessage(msg bus.InboundMessage) (string,
 	overrideKey := buildSessionOverrideKey(msg.Channel, msg.ChatID, agent.ID)
 	al.sessionOverride.Set(overrideKey, newSessionKey)
 
+	// Clean up the old session file from disk to prevent unbounded accumulation.
+	if sessionKey != "" {
+		if err := agent.Sessions.Delete(sessionKey); err != nil {
+			logger.WarnCF("agent", "Failed to delete old session file", map[string]any{
+				"session_key": sessionKey,
+				"error":       err.Error(),
+			})
+		}
+	}
+
 	return formatNewSessionResponse(al.cfg, agent), nil
 }
 
+// clearSessionForMessage clears the current session history and summary in-place,
+// keeping the same session key. Unlike startNewSessionForMessage, this does not
+// create a new session key or set an override.
+func (al *AgentLoop) clearSessionForMessage(msg bus.InboundMessage) (string, error) {
+	if constants.IsInternalChannel(msg.Channel) {
+		return "", fmt.Errorf("clear command is not supported in internal channels")
+	}
+
+	route := al.registry.ResolveRoute(routing.RouteInput{
+		Channel:    msg.Channel,
+		AccountID:  msg.Metadata["account_id"],
+		Peer:       extractPeer(msg),
+		ParentPeer: extractParentPeer(msg),
+		GuildID:    msg.Metadata["guild_id"],
+		TeamID:     msg.Metadata["team_id"],
+	})
+
+	agent, ok := al.registry.GetAgent(route.AgentID)
+	if !ok {
+		agent = al.registry.GetDefaultAgent()
+	}
+	if agent == nil {
+		return "", fmt.Errorf("no agent available to clear session")
+	}
+
+	// Resolve the active session key, honoring any existing override.
+	sessionKey := route.SessionKey
+	if msg.SessionKey != "" && strings.HasPrefix(msg.SessionKey, "agent:") {
+		sessionKey = msg.SessionKey
+	}
+	if override, ok := al.sessionOverride.Get(buildSessionOverrideKey(msg.Channel, msg.ChatID, agent.ID)); ok {
+		sessionKey = override
+	}
+
+	if sessionKey == "" {
+		return "", fmt.Errorf("no active session to clear")
+	}
+
+	agent.Sessions.ClearSession(sessionKey)
+
+	if err := agent.Sessions.Save(sessionKey); err != nil {
+		return "", err
+	}
+
+	return "Conversation history cleared.", nil
+}
+
 func formatNewSessionResponse(cfg *config.Config, agent *AgentInstance) string {
 	provider, model := resolveAgentModelDisplay(cfg, agent)
 	if model == "" {
@@ -1156,13 +1213,25 @@ func (al *AgentLoop) handleCommand(ctx context.Context, msg bus.InboundMessage)
 	args := parts[1:]
 
 	switch cmd {
-	case "/new", "/clear":
+	case "/new":
 		if len(args) > 0 {
-			return fmt.Sprintf("Usage: %s", cmd), true
+			return "Usage: /new", true
 		}
 		response, err := al.startNewSessionForMessage(msg)
 		if err != nil {
-			return fmt.Sprintf("Error: %v", err), true
+			logger.ErrorCF("agent", "Failed to start new session", map[string]any{"error": err.Error()})
+			return "Sorry, I couldn't start a new session. Please try again.", true
+		}
+		return response, true
+
+	case "/clear":
+		if len(args) > 0 {
+			return "Usage: /clear", true
+		}
+		response, err := al.clearSessionForMessage(msg)
+		if err != nil {
+			logger.ErrorCF("agent", "Failed to clear session", map[string]any{"error": err.Error()})
+			return "Sorry, I couldn't clear the conversation. Please try again.", true
 		}
 		return response, true
 

pkg/agent/loop_test.go

@@ -429,9 +429,11 @@ func TestNewSessionCommand_SuccessAndIsolation(t *testing.T) {
 	if len(newHistory) != 2 {
 		t.Fatalf("Expected new session history to have 2 messages, got %d", len(newHistory))
 	}
+
+	// After /new, the old session is deleted to prevent unbounded disk accumulation.
 	oldHistoryAfter := agent.Sessions.GetHistory(oldKey)
-	if len(oldHistoryAfter) != len(oldHistory) {
+	if len(oldHistoryAfter) != 0 {
-		t.Fatalf("Expected old session history to remain unchanged")
+		t.Fatalf("Expected old session to be deleted after /new, got %d messages", len(oldHistoryAfter))
 	}
 }
 
@@ -501,7 +503,7 @@ func TestNewSessionCommand_RejectsArgs(t *testing.T) {
 	}
 }
 
-func TestNewSessionCommand_PreservesOldSessionOnDisk(t *testing.T) {
+func TestNewSessionCommand_DeletesOldSessionFromDisk(t *testing.T) {
 	provider := &simpleMockProvider{response: "OK"}
 	al, tmpDir := newTestAgentLoop(t, provider)
 	defer os.RemoveAll(tmpDir)
@@ -524,7 +526,6 @@ func TestNewSessionCommand_PreservesOldSessionOnDisk(t *testing.T) {
 	filename := strings.ReplaceAll(oldKey, ":", "_") + ".json"
 	sessionsDir := filepath.Join(agent.Workspace, "sessions")
 	filePath := filepath.Join(sessionsDir, filename)
-	_ = os.Remove(filePath)
 
 	_ = helper.executeAndGetResponse(t, ctx, bus.InboundMessage{
 		Channel:  "test",
@@ -533,12 +534,134 @@ func TestNewSessionCommand_PreservesOldSessionOnDisk(t *testing.T) {
 		Content:  "/new",
 	})
 
-	data, err := os.ReadFile(filePath)
+	// After /new, the old session file should be deleted from disk to prevent
-	if err != nil {
+	// unbounded accumulation of session files.
-		t.Fatalf("Expected session file to exist after /new: %v", err)
+	if _, err := os.Stat(filePath); !os.IsNotExist(err) {
+		t.Fatalf("Expected old session file to be deleted after /new, but it still exists")
 	}
-	if !strings.Contains(string(data), "hello") {
+}
-		t.Fatalf("Expected session file to contain prior message")
+
+func TestClearCommand_ClearsHistoryInPlace(t *testing.T) {
+	provider := &simpleMockProvider{response: "OK"}
+	al, tmpDir := newTestAgentLoop(t, provider)
+	defer os.RemoveAll(tmpDir)
+
+	agent := al.registry.GetDefaultAgent()
+	if agent == nil {
+		t.Fatal("No default agent found")
+	}
+
+	helper := testHelper{al: al}
+	ctx := context.Background()
+
+	// Send a message to populate history
+	_ = helper.executeAndGetResponse(t, ctx, bus.InboundMessage{
+		Channel:  "test",
+		SenderID: "user1",
+		ChatID:   "chat1",
+		Content:  "hello",
+	})
+
+	oldKey := strings.ToLower(routing.BuildAgentMainSessionKey(agent.ID))
+	oldHistory := agent.Sessions.GetHistory(oldKey)
+	if len(oldHistory) == 0 {
+		t.Fatalf("Expected session history to be populated before /clear")
+	}
+
+	// Set a summary to verify it gets cleared too
+	agent.Sessions.SetSummary(oldKey, "Test summary")
+
+	// Use /clear
+	response := helper.executeAndGetResponse(t, ctx, bus.InboundMessage{
+		Channel:  "test",
+		SenderID: "user1",
+		ChatID:   "chat1",
+		Content:  "/clear",
+	})
+	if response != "Conversation history cleared." {
+		t.Fatalf("Expected clear confirmation, got %q", response)
+	}
+
+	// Verify: same session key, no override set
+	overrideKey := buildSessionOverrideKey("test", "chat1", agent.ID)
+	if _, ok := al.sessionOverride.Get(overrideKey); ok {
+		t.Fatalf("Expected /clear NOT to set a session override")
+	}
+
+	// Verify: history is empty
+	clearedHistory := agent.Sessions.GetHistory(oldKey)
+	if len(clearedHistory) != 0 {
+		t.Fatalf("Expected session history to be cleared, got %d messages", len(clearedHistory))
+	}
+
+	// Verify: summary is empty
+	if agent.Sessions.GetSummary(oldKey) != "" {
+		t.Fatalf("Expected session summary to be cleared")
+	}
+}
+
+func TestClearCommand_RejectsArgs(t *testing.T) {
+	provider := &simpleMockProvider{response: "OK"}
+	al, tmpDir := newTestAgentLoop(t, provider)
+	defer os.RemoveAll(tmpDir)
+
+	helper := testHelper{al: al}
+	ctx := context.Background()
+	response := helper.executeAndGetResponse(t, ctx, bus.InboundMessage{
+		Channel:  "test",
+		SenderID: "user1",
+		ChatID:   "chat1",
+		Content:  "/clear extra",
+	})
+	if response != "Usage: /clear" {
+		t.Fatalf("Expected usage response, got %q", response)
+	}
+}
+
+func TestClearCommand_SubsequentMessagesUseSameSession(t *testing.T) {
+	provider := &simpleMockProvider{response: "OK"}
+	al, tmpDir := newTestAgentLoop(t, provider)
+	defer os.RemoveAll(tmpDir)
+
+	agent := al.registry.GetDefaultAgent()
+	if agent == nil {
+		t.Fatal("No default agent found")
+	}
+
+	helper := testHelper{al: al}
+	ctx := context.Background()
+
+	// Send a message, then clear, then send another
+	_ = helper.executeAndGetResponse(t, ctx, bus.InboundMessage{
+		Channel:  "test",
+		SenderID: "user1",
+		ChatID:   "chat1",
+		Content:  "first message",
+	})
+
+	sessionKey := strings.ToLower(routing.BuildAgentMainSessionKey(agent.ID))
+
+	_ = helper.executeAndGetResponse(t, ctx, bus.InboundMessage{
+		Channel:  "test",
+		SenderID: "user1",
+		ChatID:   "chat1",
+		Content:  "/clear",
+	})
+
+	_ = helper.executeAndGetResponse(t, ctx, bus.InboundMessage{
+		Channel:  "test",
+		SenderID: "user1",
+		ChatID:   "chat1",
+		Content:  "second message",
+	})
+
+	// History should only contain the post-clear message exchange
+	history := agent.Sessions.GetHistory(sessionKey)
+	if len(history) != 2 {
+		t.Fatalf("Expected 2 messages (post-clear user+assistant), got %d", len(history))
+	}
+	if history[0].Content != "second message" {
+		t.Fatalf("Expected first post-clear message to be 'second message', got %q", history[0].Content)
 	}
 }
 

pkg/session/manager.go

@@ -265,6 +265,43 @@ func (sm *SessionManager) loadSessions() error {
 	return nil
 }
 
+// Delete removes a session from memory and deletes its file from disk.
+func (sm *SessionManager) Delete(key string) error {
+	sm.mu.Lock()
+	delete(sm.sessions, key)
+	sm.mu.Unlock()
+
+	if sm.storage == "" {
+		return nil
+	}
+
+	filename := sanitizeFilename(key)
+	if filename == "." || !filepath.IsLocal(filename) || strings.ContainsAny(filename, `/\`) {
+		return os.ErrInvalid
+	}
+
+	sessionPath := filepath.Join(sm.storage, filename+".json")
+	if err := os.Remove(sessionPath); err != nil && !os.IsNotExist(err) {
+		return err
+	}
+	return nil
+}
+
+// ClearSession resets a session's messages and summary in-place, keeping the same key.
+func (sm *SessionManager) ClearSession(key string) {
+	sm.mu.Lock()
+	defer sm.mu.Unlock()
+
+	session, ok := sm.sessions[key]
+	if !ok {
+		return
+	}
+
+	session.Messages = []providers.Message{}
+	session.Summary = ""
+	session.Updated = time.Now()
+}
+
 // SetHistory updates the messages of a session.
 func (sm *SessionManager) SetHistory(key string, history []providers.Message) {
 	sm.mu.Lock()

pkg/session/manager_test.go

@@ -72,3 +72,87 @@ func TestSave_RejectsPathTraversal(t *testing.T) {
 		}
 	}
 }
+
+func TestDelete_RemovesSessionFromMemoryAndDisk(t *testing.T) {
+	tmpDir := t.TempDir()
+	sm := NewSessionManager(tmpDir)
+
+	key := "agent:main:test"
+	sm.GetOrCreate(key)
+	sm.AddMessage(key, "user", "hello")
+
+	if err := sm.Save(key); err != nil {
+		t.Fatalf("Save failed: %v", err)
+	}
+
+	expectedFile := filepath.Join(tmpDir, "agent_main_test.json")
+	if _, err := os.Stat(expectedFile); os.IsNotExist(err) {
+		t.Fatalf("expected session file to exist before delete")
+	}
+
+	if err := sm.Delete(key); err != nil {
+		t.Fatalf("Delete failed: %v", err)
+	}
+
+	// Verify removed from memory
+	history := sm.GetHistory(key)
+	if len(history) != 0 {
+		t.Errorf("expected empty history after delete, got %d messages", len(history))
+	}
+
+	// Verify removed from disk
+	if _, err := os.Stat(expectedFile); !os.IsNotExist(err) {
+		t.Errorf("expected session file to be deleted from disk")
+	}
+}
+
+func TestDelete_NonexistentKeyNoError(t *testing.T) {
+	tmpDir := t.TempDir()
+	sm := NewSessionManager(tmpDir)
+
+	// Deleting a key that doesn't exist should not error
+	if err := sm.Delete("nonexistent:key"); err != nil {
+		t.Errorf("Delete of nonexistent key should not error, got: %v", err)
+	}
+}
+
+func TestClearSession_ResetsHistoryAndSummary(t *testing.T) {
+	tmpDir := t.TempDir()
+	sm := NewSessionManager(tmpDir)
+
+	key := "agent:main:clear-test"
+	sm.GetOrCreate(key)
+	sm.AddMessage(key, "user", "hello")
+	sm.AddMessage(key, "assistant", "hi there")
+	sm.SetSummary(key, "User said hello")
+
+	// Verify pre-conditions
+	history := sm.GetHistory(key)
+	if len(history) != 2 {
+		t.Fatalf("expected 2 messages before clear, got %d", len(history))
+	}
+	if sm.GetSummary(key) == "" {
+		t.Fatalf("expected non-empty summary before clear")
+	}
+
+	sm.ClearSession(key)
+
+	// Verify history is cleared
+	history = sm.GetHistory(key)
+	if len(history) != 0 {
+		t.Errorf("expected 0 messages after clear, got %d", len(history))
+	}
+
+	// Verify summary is cleared
+	if sm.GetSummary(key) != "" {
+		t.Errorf("expected empty summary after clear, got %q", sm.GetSummary(key))
+	}
+}
+
+func TestClearSession_NonexistentKeyNoPanic(t *testing.T) {
+	tmpDir := t.TempDir()
+	sm := NewSessionManager(tmpDir)
+
+	// Clearing a nonexistent session should not panic
+	sm.ClearSession("nonexistent:key")
+}