From ffffb6a0cb98ea264c4842b54da67d4551b7cb53 Mon Sep 17 00:00:00 2001 From: Ethan1918 <75773519+Ethan1918@users.noreply.github.com> Date: Sat, 4 Jul 2026 14:04:37 +0800 Subject: [PATCH] refactor(agent): route /clear through ContextManager.Clear for all agents Address review feedback: the routed-agent /clear path bypassed the public ContextManager.Clear contract via an unexported hook. Restore the single Clear call in the command path and resolve the session's owning agent inside the built-in implementations instead: - legacy: Clear resolves the owning agent via agentForSession instead of assuming the default agent - seahorse: drop ClearContextStore; Clear wipes the engine state and the owning agent's session store - command path: persist session scope metadata before Clear so ownership resolves even when /clear is the session's first message - add coverage that a custom ContextManager receives Clear for routed agents Co-Authored-By: Claude Fable 5 --- pkg/agent/agent_command.go | 45 ++++----------- pkg/agent/context_legacy.go | 4 +- pkg/agent/context_manager_test.go | 96 ++++++++++++++++++++++++++++++- pkg/agent/context_seahorse.go | 24 +++++--- 4 files changed, 123 insertions(+), 46 deletions(-) diff --git a/pkg/agent/agent_command.go b/pkg/agent/agent_command.go index 237297503..9c7584a3f 100644 --- a/pkg/agent/agent_command.go +++ b/pkg/agent/agent_command.go @@ -333,7 +333,16 @@ func (al *AgentLoop) buildCommandsRuntime( if opts == nil { return fmt.Errorf("process options not available") } - return al.clearAgentSessionContext(ctx, agent, opts.SessionKey) + // /clear can arrive before any turn has persisted session scope + // metadata (runAgentLoop records it per turn), so record it here to + // let the ContextManager resolve which agent owns the session. + ensureSessionMetadata( + agent.Sessions, + opts.Dispatch.SessionKey, + opts.Dispatch.SessionScope, + opts.Dispatch.SessionAliases, + ) + return al.contextManager.Clear(ctx, opts.SessionKey) } rt.AskSideQuestion = func(ctx context.Context, question string) (string, error) { @@ -363,40 +372,6 @@ func (al *AgentLoop) buildCommandsRuntime( return rt } -type contextStoreClearer interface { - ClearContextStore(ctx context.Context, sessionKey string) error -} - -func (al *AgentLoop) clearAgentSessionContext( - ctx context.Context, - agent *AgentInstance, - sessionKey string, -) error { - if agent == nil || agent.Sessions == nil { - return fmt.Errorf("sessions not initialized") - } - if al != nil && al.registry != nil && agent == al.registry.GetDefaultAgent() { - if al.contextManager != nil { - return al.contextManager.Clear(ctx, sessionKey) - } - return clearAgentSessionStore(agent, sessionKey) - } - if al != nil && al.contextManager != nil { - if clearer, ok := al.contextManager.(contextStoreClearer); ok { - if err := clearer.ClearContextStore(ctx, sessionKey); err != nil { - return err - } - } - } - return clearAgentSessionStore(agent, sessionKey) -} - -func clearAgentSessionStore(agent *AgentInstance, sessionKey string) error { - agent.Sessions.SetHistory(sessionKey, []providers.Message{}) - agent.Sessions.SetSummary(sessionKey, "") - return agent.Sessions.Save(sessionKey) -} - func summarizeMCPToolParameters(schema any) []commands.MCPToolParameterInfo { schemaMap := normalizeMCPSchema(schema) properties, ok := schemaMap["properties"].(map[string]any) diff --git a/pkg/agent/context_legacy.go b/pkg/agent/context_legacy.go index 908c3d859..eeaba3de9 100644 --- a/pkg/agent/context_legacy.go +++ b/pkg/agent/context_legacy.go @@ -63,7 +63,9 @@ func (m *legacyContextManager) Ingest(_ context.Context, _ *IngestRequest) error } func (m *legacyContextManager) Clear(_ context.Context, sessionKey string) error { - agent := m.al.registry.GetDefaultAgent() + // Routed (non-default) agents keep history in their own session store, + // so resolve the owning agent instead of assuming the default one. + agent := m.al.agentForSession(sessionKey) if agent == nil || agent.Sessions == nil { return fmt.Errorf("sessions not initialized") } diff --git a/pkg/agent/context_manager_test.go b/pkg/agent/context_manager_test.go index 46e521be4..bdccf4f69 100644 --- a/pkg/agent/context_manager_test.go +++ b/pkg/agent/context_manager_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "os" + "path/filepath" "strings" "sync" "sync/atomic" @@ -658,6 +659,91 @@ func TestIngestCalledDuringTurn(t *testing.T) { } } +func TestClearCommandRoutedAgentCallsContextManagerClear(t *testing.T) { + cleanup := resetCMRegistry() + defer cleanup() + + mock := &trackingContextManager{} + factory := func(cfg json.RawMessage, al *AgentLoop) (ContextManager, error) { + return mock, nil + } + if err := RegisterContextManager("clear_track_cm", factory); err != nil { + t.Fatalf("register failed: %v", err) + } + + workspace := t.TempDir() + cfg := &config.Config{ + Agents: config.AgentsConfig{ + Defaults: config.AgentDefaults{ + Workspace: filepath.Join(workspace, "default"), + ModelName: "test-model", + MaxTokens: 4096, + MaxToolIterations: 10, + ContextManager: "clear_track_cm", + }, + List: []config.AgentConfig{ + { + ID: "main", + Default: true, + Workspace: filepath.Join(workspace, "main"), + }, + { + ID: "support", + Workspace: filepath.Join(workspace, "support"), + }, + }, + Dispatch: &config.DispatchConfig{ + Rules: []config.DispatchRule{ + { + Name: "support-dingtalk", + Agent: "support", + When: config.DispatchSelector{ + Channel: "dingtalk", + }, + }, + }, + }, + }, + Session: config.SessionConfig{ + Dimensions: []string{"chat"}, + }, + } + + al := NewAgentLoop(cfg, bus.NewMessageBus(), &simpleMockProvider{response: "done"}) + if al.contextManager != mock { + t.Fatalf("expected mock context manager, got %T", al.contextManager) + } + + msg := testInboundMessage(bus.InboundMessage{ + Context: bus.InboundContext{ + Channel: "dingtalk", + ChatID: "chat1", + ChatType: "direct", + SenderID: "user1", + }, + Content: "/clear", + }) + route, _, err := al.resolveMessageRoute(msg) + if err != nil { + t.Fatalf("resolveMessageRoute() error = %v", err) + } + sessionKey := al.allocateRouteSession(route, msg).SessionKey + + if _, err := al.processMessage(context.Background(), msg); err != nil { + t.Fatalf("processMessage() error = %v", err) + } + + if got := mock.clearCalls.Load(); got != 1 { + t.Fatalf("Clear calls = %d, want 1", got) + } + mock.mu.Lock() + gotKey := mock.lastClearKey + mock.mu.Unlock() + if gotKey != sessionKey { + t.Fatalf("Clear session key = %q, want %q", gotKey, sessionKey) + } +} + // --------------------------------------------------------------------------- // forceCompression edge cases (via legacy Compact) // --------------------------------------------------------------------------- @@ -712,10 +798,12 @@ type trackingContextManager struct { assembleCalls atomic.Int64 compactCalls atomic.Int64 ingestCalls atomic.Int64 + clearCalls atomic.Int64 mu sync.Mutex lastAssemble *AssembleRequest lastCompact *CompactRequest lastIngest *IngestRequest + lastClearKey string } func (m *trackingContextManager) Assemble(_ context.Context, req *AssembleRequest) (*AssembleResponse, error) { @@ -742,7 +830,13 @@ func (m *trackingContextManager) Ingest(_ context.Context, req *IngestRequest) e return nil } -func (m *trackingContextManager) Clear(_ context.Context, _ string) error { return nil } +func (m *trackingContextManager) Clear(_ context.Context, sessionKey string) error { + m.clearCalls.Add(1) + m.mu.Lock() + m.lastClearKey = sessionKey + m.mu.Unlock() + return nil +} // resetCMRegistry clears the global factory registry and returns a cleanup // function that restores the original state after the test. diff --git a/pkg/agent/context_seahorse.go b/pkg/agent/context_seahorse.go index c2ad3ea8d..4e4c55fd9 100644 --- a/pkg/agent/context_seahorse.go +++ b/pkg/agent/context_seahorse.go @@ -20,6 +20,7 @@ import ( type seahorseContextManager struct { engine *seahorse.Engine sessions session.SessionStore // for startup bootstrap + al *AgentLoop // for resolving the agent that owns a session } // newSeahorseContextManager creates a seahorse-backed ContextManager. @@ -47,6 +48,7 @@ func newSeahorseContextManager(_ json.RawMessage, al *AgentLoop) (ContextManager mgr := &seahorseContextManager{ engine: engine, sessions: agent.Sessions, + al: al, } // Register seahorse tools with the agent's tool registry @@ -157,21 +159,25 @@ func (m *seahorseContextManager) Ingest(ctx context.Context, req *IngestRequest) // Clear removes all stored context for a session (seahorse DB + JSONL). func (m *seahorseContextManager) Clear(ctx context.Context, sessionKey string) error { - if err := m.ClearContextStore(ctx, sessionKey); err != nil { + if err := m.engine.ClearSession(ctx, sessionKey); err != nil { return err } - if m.sessions != nil { - m.sessions.SetHistory(sessionKey, []providers.Message{}) - m.sessions.SetSummary(sessionKey, "") - return m.sessions.Save(sessionKey) + // The session may belong to a routed (non-default) agent whose JSONL + // store differs from the bootstrap store, so clear the owner's store. + sessions := m.sessions + if m.al != nil { + if agent := m.al.agentForSession(sessionKey); agent != nil && agent.Sessions != nil { + sessions = agent.Sessions + } + } + if sessions != nil { + sessions.SetHistory(sessionKey, []providers.Message{}) + sessions.SetSummary(sessionKey, "") + return sessions.Save(sessionKey) } return nil } -func (m *seahorseContextManager) ClearContextStore(ctx context.Context, sessionKey string) error { - return m.engine.ClearSession(ctx, sessionKey) -} - // bootstrapSession reconciles JSONL session history into seahorse SQLite. func (m *seahorseContextManager) bootstrapSession(ctx context.Context, sessionKey string) { if m.sessions == nil {