fix(session): invalidate stale agent session IDs on agent type switch (#129)
* fix(session): invalidate stale agent session IDs on agent type switch When a project's agent type is changed in config (e.g. opencode → pi), persisted sessions still contain agent_session_id values from the old agent. The new agent fails to resume these foreign session IDs, causing errors and empty responses. Add an `agent_type` field to Session that tracks which agent created the session ID. On engine startup, InvalidateForAgent() clears session IDs whose agent_type doesn't match the current agent. All code paths that set AgentSessionID now also record the agent type. Closes #128 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: add unit test for SessionManager.InvalidateForAgent Cover the three key cases: mismatched agent type (invalidated), matching agent type (preserved), and empty agent type (migration-safe). Also verifies sessions with no agent session ID are untouched. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
452e5fab6e
commit
cd4a85e858
4 files changed
+109
-24
No files matched your search
+16
-12
@@ -251,6 +251,10 @@ func NewEngine(name string, ag Agent, platforms []Platform, sessionStorePath str
|
||||
eventIdleTimeout: defaultEventIdleTimeout,
|
||||
}
|
||||
|
||||
if ag != nil {
|
||||
e.sessions.InvalidateForAgent(ag.Name())
|
||||
}
|
||||
|
||||
if cp, ok := ag.(CommandProvider); ok {
|
||||
e.commands.SetAgentDirs(cp.CommandDirs())
|
||||
}
|
||||
@@ -1427,7 +1431,7 @@ func (e *Engine) getOrCreateInteractiveStateWith(sessionKey string, p Platform,
|
||||
}
|
||||
|
||||
if newID := agentSession.CurrentSessionID(); newID != "" {
|
||||
session.CompareAndSetAgentSessionID(newID)
|
||||
session.CompareAndSetAgentSessionID(newID, agent.Name())
|
||||
}
|
||||
|
||||
state = &interactiveState{
|
||||
@@ -1632,7 +1636,7 @@ func (e *Engine) processInteractiveEvents(state *interactiveState, session *Sess
|
||||
}
|
||||
}
|
||||
if event.SessionID != "" {
|
||||
if session.CompareAndSetAgentSessionID(event.SessionID) {
|
||||
if session.CompareAndSetAgentSessionID(event.SessionID, e.agent.Name()) {
|
||||
pendingName := session.GetName()
|
||||
if pendingName != "" && pendingName != "session" && pendingName != "default" {
|
||||
e.sessions.SetSessionName(event.SessionID, pendingName)
|
||||
@@ -1721,7 +1725,7 @@ func (e *Engine) processInteractiveEvents(state *interactiveState, session *Sess
|
||||
|
||||
case EventResult:
|
||||
if event.SessionID != "" {
|
||||
session.SetAgentSessionID(event.SessionID)
|
||||
session.SetAgentSessionID(event.SessionID, e.agent.Name())
|
||||
}
|
||||
|
||||
fullResponse := event.Content
|
||||
@@ -2292,7 +2296,7 @@ func (e *Engine) cmdSwitch(p Platform, msg *Message, args []string) {
|
||||
slog.Info("cmdSwitch: cleanup done", "session_key", msg.SessionKey)
|
||||
|
||||
session := sessions.GetOrCreateActive(msg.SessionKey)
|
||||
session.SetAgentInfo(matched.ID, matched.Summary)
|
||||
session.SetAgentInfo(matched.ID, agent.Name(), matched.Summary)
|
||||
session.ClearHistory()
|
||||
sessions.Save()
|
||||
|
||||
@@ -3538,7 +3542,7 @@ func (e *Engine) cmdModel(p Platform, msg *Message, args []string) {
|
||||
e.cleanupInteractiveState(msg.SessionKey)
|
||||
|
||||
s := e.sessions.GetOrCreateActive(msg.SessionKey)
|
||||
s.SetAgentSessionID("")
|
||||
s.SetAgentSessionID("", "")
|
||||
s.ClearHistory()
|
||||
e.sessions.Save()
|
||||
|
||||
@@ -3619,7 +3623,7 @@ func (e *Engine) cmdReasoning(p Platform, msg *Message, args []string) {
|
||||
e.cleanupInteractiveState(msg.SessionKey)
|
||||
|
||||
s := e.sessions.GetOrCreateActive(msg.SessionKey)
|
||||
s.SetAgentSessionID("")
|
||||
s.SetAgentSessionID("", "")
|
||||
s.ClearHistory()
|
||||
e.sessions.Save()
|
||||
|
||||
@@ -4591,7 +4595,7 @@ func (e *Engine) executeCardAction(cmd, args, sessionKey string) {
|
||||
switcher.SetModel(target)
|
||||
e.cleanupInteractiveState(sessionKey)
|
||||
s := e.sessions.GetOrCreateActive(sessionKey)
|
||||
s.SetAgentSessionID("")
|
||||
s.SetAgentSessionID("", "")
|
||||
s.ClearHistory()
|
||||
e.sessions.Save()
|
||||
|
||||
@@ -4613,7 +4617,7 @@ func (e *Engine) executeCardAction(cmd, args, sessionKey string) {
|
||||
switcher.SetReasoningEffort(target)
|
||||
e.cleanupInteractiveState(sessionKey)
|
||||
s := e.sessions.GetOrCreateActive(sessionKey)
|
||||
s.SetAgentSessionID("")
|
||||
s.SetAgentSessionID("", "")
|
||||
s.ClearHistory()
|
||||
e.sessions.Save()
|
||||
return
|
||||
@@ -4709,7 +4713,7 @@ func (e *Engine) executeCardAction(cmd, args, sessionKey string) {
|
||||
interactiveKey := e.interactiveKeyForSessionKey(sessionKey)
|
||||
e.cleanupInteractiveState(interactiveKey)
|
||||
session := sessions.GetOrCreateActive(sessionKey)
|
||||
session.SetAgentInfo(matched.ID, matched.Summary)
|
||||
session.SetAgentInfo(matched.ID, agent.Name(), matched.Summary)
|
||||
session.ClearHistory()
|
||||
sessions.Save()
|
||||
|
||||
@@ -7094,7 +7098,7 @@ func (e *Engine) HandleRelay(ctx context.Context, fromProject, chatID, message s
|
||||
}
|
||||
defer agentSession.Close()
|
||||
|
||||
if session.CompareAndSetAgentSessionID(agentSession.CurrentSessionID()) {
|
||||
if session.CompareAndSetAgentSessionID(agentSession.CurrentSessionID(), e.agent.Name()) {
|
||||
e.sessions.Save()
|
||||
}
|
||||
|
||||
@@ -7113,13 +7117,13 @@ func (e *Engine) HandleRelay(ctx context.Context, fromProject, chatID, message s
|
||||
textParts = append(textParts, event.Content)
|
||||
}
|
||||
if event.SessionID != "" {
|
||||
if session.CompareAndSetAgentSessionID(event.SessionID) {
|
||||
if session.CompareAndSetAgentSessionID(event.SessionID, e.agent.Name()) {
|
||||
e.sessions.Save()
|
||||
}
|
||||
}
|
||||
case EventResult:
|
||||
if event.SessionID != "" {
|
||||
session.SetAgentSessionID(event.SessionID)
|
||||
session.SetAgentSessionID(event.SessionID, e.agent.Name())
|
||||
e.sessions.Save()
|
||||
}
|
||||
resp := event.Content
|
||||
|
||||
+5
-5
@@ -908,7 +908,7 @@ func TestCmdCurrent_UsesLegacyTextOnPlatformWithoutCardSupport(t *testing.T) {
|
||||
msg := &Message{SessionKey: "test:user1", ReplyCtx: "ctx"}
|
||||
session := e.sessions.GetOrCreateActive(msg.SessionKey)
|
||||
session.Name = "Focus"
|
||||
session.SetAgentSessionID("session-123")
|
||||
session.SetAgentSessionID("session-123", "test")
|
||||
session.History = append(session.History, HistoryEntry{Role: "user", Content: "hello", Timestamp: time.Now()})
|
||||
|
||||
e.cmdCurrent(p, msg)
|
||||
@@ -1243,7 +1243,7 @@ func TestDeleteMode_SubmitBlocksActiveSession(t *testing.T) {
|
||||
}}}
|
||||
e := NewEngine("test", agent, []Platform{p}, "", LangEnglish)
|
||||
msg := &Message{SessionKey: "feishu:user1", ReplyCtx: "ctx"}
|
||||
e.sessions.GetOrCreateActive(msg.SessionKey).SetAgentSessionID("session-1")
|
||||
e.sessions.GetOrCreateActive(msg.SessionKey).SetAgentSessionID("session-1", "test")
|
||||
|
||||
e.cmdDelete(p, msg, nil)
|
||||
_ = e.handleCardNav("act:/delete-mode toggle session-1", msg.SessionKey)
|
||||
@@ -1267,7 +1267,7 @@ func TestDeleteMode_ActiveSessionMarkedWithArrowAndNotSelectable(t *testing.T) {
|
||||
}}}
|
||||
e := NewEngine("test", agent, []Platform{p}, "", LangEnglish)
|
||||
msg := &Message{SessionKey: "feishu:user1", ReplyCtx: "ctx"}
|
||||
e.sessions.GetOrCreateActive(msg.SessionKey).SetAgentSessionID("session-1")
|
||||
e.sessions.GetOrCreateActive(msg.SessionKey).SetAgentSessionID("session-1", "test")
|
||||
|
||||
e.cmdDelete(p, msg, nil)
|
||||
if len(p.repliedCards) != 1 {
|
||||
@@ -1442,7 +1442,7 @@ func TestCmdReasoning_SwitchesEffortAndResetsSession(t *testing.T) {
|
||||
msg := &Message{SessionKey: "test:user1", ReplyCtx: "ctx"}
|
||||
|
||||
s := e.sessions.GetOrCreateActive(msg.SessionKey)
|
||||
s.SetAgentSessionID("existing-session")
|
||||
s.SetAgentSessionID("existing-session", "test")
|
||||
s.AddHistory("user", "hello")
|
||||
|
||||
e.cmdReasoning(p, msg, []string{"3"})
|
||||
@@ -1769,7 +1769,7 @@ func TestRenderListCard_MakesEveryVisibleSessionClickable(t *testing.T) {
|
||||
}
|
||||
|
||||
e := NewEngine("test", &stubListAgent{sessions: sessions}, []Platform{&stubPlatformEngine{n: "test"}}, "", LangEnglish)
|
||||
e.sessions.GetOrCreateActive("test:user1").SetAgentSessionID(sessions[5].ID)
|
||||
e.sessions.GetOrCreateActive("test:user1").SetAgentSessionID(sessions[5].ID, "test")
|
||||
|
||||
card, err := e.renderListCard("test:user1", 1)
|
||||
if err != nil {
|
||||
|
||||
+39
-5
@@ -15,6 +15,7 @@ type Session struct {
|
||||
ID string `json:"id"`
|
||||
Name string `json:"name"`
|
||||
AgentSessionID string `json:"agent_session_id"`
|
||||
AgentType string `json:"agent_type,omitempty"`
|
||||
History []HistoryEntry `json:"history"`
|
||||
CreatedAt time.Time `json:"created_at"`
|
||||
UpdatedAt time.Time `json:"updated_at"`
|
||||
@@ -50,11 +51,12 @@ func (s *Session) AddHistory(role, content string) {
|
||||
})
|
||||
}
|
||||
|
||||
// SetAgentInfo atomically sets the agent session ID and name.
|
||||
func (s *Session) SetAgentInfo(agentSessionID, name string) {
|
||||
// SetAgentInfo atomically sets the agent session ID, agent type, and name.
|
||||
func (s *Session) SetAgentInfo(agentSessionID, agentType, name string) {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
s.AgentSessionID = agentSessionID
|
||||
s.AgentType = agentType
|
||||
s.Name = name
|
||||
}
|
||||
|
||||
@@ -72,22 +74,24 @@ func (s *Session) GetName() string {
|
||||
return s.Name
|
||||
}
|
||||
|
||||
// SetAgentSessionID atomically sets the agent session ID.
|
||||
func (s *Session) SetAgentSessionID(id string) {
|
||||
// SetAgentSessionID atomically sets the agent session ID and agent type.
|
||||
func (s *Session) SetAgentSessionID(id, agentType string) {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
s.AgentSessionID = id
|
||||
s.AgentType = agentType
|
||||
}
|
||||
|
||||
// CompareAndSetAgentSessionID sets the agent session ID only if it is currently empty.
|
||||
// Returns true if the value was set, false if it was already non-empty.
|
||||
func (s *Session) CompareAndSetAgentSessionID(id string) bool {
|
||||
func (s *Session) CompareAndSetAgentSessionID(id, agentType string) bool {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
if s.AgentSessionID != "" {
|
||||
return false
|
||||
}
|
||||
s.AgentSessionID = id
|
||||
s.AgentType = agentType
|
||||
return true
|
||||
}
|
||||
|
||||
@@ -305,6 +309,7 @@ func (sm *SessionManager) saveLocked() {
|
||||
ID: s.ID,
|
||||
Name: s.Name,
|
||||
AgentSessionID: s.AgentSessionID,
|
||||
AgentType: s.AgentType,
|
||||
History: append([]HistoryEntry(nil), s.History...),
|
||||
CreatedAt: s.CreatedAt,
|
||||
UpdatedAt: s.UpdatedAt,
|
||||
@@ -367,3 +372,32 @@ func (sm *SessionManager) load() {
|
||||
|
||||
slog.Info("session: loaded from disk", "path", sm.storePath, "sessions", len(sm.sessions))
|
||||
}
|
||||
|
||||
// InvalidateForAgent clears AgentSessionID on all sessions whose AgentType
|
||||
// does not match the current agent. This handles the case where the user
|
||||
// switches agent types (e.g. opencode → pi) and stale session IDs from the
|
||||
// old agent would cause errors.
|
||||
func (sm *SessionManager) InvalidateForAgent(agentType string) {
|
||||
sm.mu.Lock()
|
||||
defer sm.mu.Unlock()
|
||||
|
||||
invalidated := 0
|
||||
for _, s := range sm.sessions {
|
||||
s.mu.Lock()
|
||||
if s.AgentSessionID != "" && s.AgentType != "" && s.AgentType != agentType {
|
||||
slog.Info("session: invalidating stale agent session",
|
||||
"session", s.ID,
|
||||
"old_agent", s.AgentType,
|
||||
"new_agent", agentType,
|
||||
"old_agent_session_id", s.AgentSessionID,
|
||||
)
|
||||
s.AgentSessionID = ""
|
||||
s.AgentType = agentType
|
||||
invalidated++
|
||||
}
|
||||
s.mu.Unlock()
|
||||
}
|
||||
if invalidated > 0 {
|
||||
sm.saveLocked()
|
||||
}
|
||||
}
|
||||
+49
-2
@@ -198,7 +198,7 @@ func TestSession_GetAgentSessionID(t *testing.T) {
|
||||
if got := s.GetAgentSessionID(); got != "" {
|
||||
t.Errorf("initial GetAgentSessionID = %q, want empty", got)
|
||||
}
|
||||
s.SetAgentSessionID("sess-1")
|
||||
s.SetAgentSessionID("sess-1", "test")
|
||||
if got := s.GetAgentSessionID(); got != "sess-1" {
|
||||
t.Errorf("GetAgentSessionID = %q, want %q", got, "sess-1")
|
||||
}
|
||||
@@ -211,6 +211,53 @@ func TestSession_GetName(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestSessionManager_InvalidateForAgent(t *testing.T) {
|
||||
sm := NewSessionManager("")
|
||||
|
||||
// Create sessions with different agent types
|
||||
s1 := sm.NewSession("user1", "sess1")
|
||||
s1.SetAgentSessionID("old-id-1", "opencode")
|
||||
|
||||
s2 := sm.NewSession("user2", "sess2")
|
||||
s2.SetAgentSessionID("old-id-2", "claudecode")
|
||||
|
||||
s3 := sm.NewSession("user3", "sess3")
|
||||
s3.SetAgentSessionID("old-id-3", "") // pre-migration, no agent type
|
||||
|
||||
s4 := sm.NewSession("user4", "sess4") // no agent session ID at all
|
||||
|
||||
sm.InvalidateForAgent("claudecode")
|
||||
|
||||
// s1: opencode → should be invalidated
|
||||
if got := s1.GetAgentSessionID(); got != "" {
|
||||
t.Errorf("s1 (opencode) AgentSessionID = %q, want empty (should be invalidated)", got)
|
||||
}
|
||||
if s1.AgentType != "claudecode" {
|
||||
t.Errorf("s1 AgentType = %q, want %q after invalidation", s1.AgentType, "claudecode")
|
||||
}
|
||||
|
||||
// s2: claudecode → should be untouched
|
||||
if got := s2.GetAgentSessionID(); got != "old-id-2" {
|
||||
t.Errorf("s2 (claudecode) AgentSessionID = %q, want %q (should be preserved)", got, "old-id-2")
|
||||
}
|
||||
if s2.AgentType != "claudecode" {
|
||||
t.Errorf("s2 AgentType = %q, want %q", s2.AgentType, "claudecode")
|
||||
}
|
||||
|
||||
// s3: empty agent type → should be untouched (backward compat)
|
||||
if got := s3.GetAgentSessionID(); got != "old-id-3" {
|
||||
t.Errorf("s3 (empty type) AgentSessionID = %q, want %q (migration-safe)", got, "old-id-3")
|
||||
}
|
||||
if s3.AgentType != "" {
|
||||
t.Errorf("s3 AgentType = %q, want empty (pre-migration should be untouched)", s3.AgentType)
|
||||
}
|
||||
|
||||
// s4: no agent session ID → should be untouched
|
||||
if got := s4.GetAgentSessionID(); got != "" {
|
||||
t.Errorf("s4 (no session ID) AgentSessionID = %q, want empty", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSession_ConcurrentGetSet(t *testing.T) {
|
||||
s := &Session{}
|
||||
var wg sync.WaitGroup
|
||||
@@ -218,7 +265,7 @@ func TestSession_ConcurrentGetSet(t *testing.T) {
|
||||
wg.Add(2)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
s.SetAgentSessionID("id")
|
||||
s.SetAgentSessionID("id", "test")
|
||||
}()
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
|
||||
Reference in new issue
Block a user