Skip to content

Commit 841507f

Browse files
Alan-TheGentlemanAlan-TheGentleman
authored
fix(mcp): resolve active session from store for mem_save instead of manual-save fallback (#386) (#449)
1 parent 6bfe33d commit 841507f

4 files changed

Lines changed: 297 additions & 4 deletions

File tree

‎internal/mcp/mcp.go‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,7 +1100,7 @@ func handleSave(s *store.Store, cfg MCPConfig, activity *SessionActivity) server
11001100
typ = "manual"
11011101
}
11021102
if sessionID == "" {
1103-
sessionID = defaultSessionID(project)
1103+
sessionID = resolveFallbackSessionID(s, project)
11041104
}
11051105
suggestedTopicKey := suggestTopicKey(typ, title, content)
11061106

@@ -1350,7 +1350,7 @@ func handleSavePrompt(s *store.Store, cfg MCPConfig, activity *SessionActivity)
13501350
project, _ := store.NormalizeProject(detRes.Project)
13511351

13521352
if sessionID == "" {
1353-
sessionID = defaultSessionID(project)
1353+
sessionID = resolveFallbackSessionID(s, project)
13541354
}
13551355

13561356
// Ensure the implicit MCP session exists with the current working directory.
@@ -1647,7 +1647,7 @@ func handleSessionSummary(s *store.Store, cfg MCPConfig, activity *SessionActivi
16471647
project, _ := store.NormalizeProject(detRes.Project)
16481648

16491649
if sessionID == "" {
1650-
sessionID = defaultSessionID(project)
1650+
sessionID = resolveFallbackSessionID(s, project)
16511651
}
16521652

16531653
// Ensure the implicit MCP session exists with the current working directory.
@@ -1767,7 +1767,7 @@ func handleCapturePassive(s *store.Store, cfg MCPConfig, activity *SessionActivi
17671767
}
17681768

17691769
if sessionID == "" {
1770-
sessionID = defaultSessionID(project)
1770+
sessionID = resolveFallbackSessionID(s, project)
17711771
_ = ensureImplicitSessionWithCWD(s, sessionID, project)
17721772
}
17731773

@@ -2695,6 +2695,27 @@ func defaultSessionID(project string) string {
26952695
return "manual-save-" + project
26962696
}
26972697

2698+
// resolveFallbackSessionID resolves the session a write should attach to when
2699+
// the caller did not provide an explicit session_id.
2700+
//
2701+
// It first consults the persisted sessions table for the most recent active
2702+
// (un-ended) session of the project (issue #386). The SessionStart hook
2703+
// registers a UUID session via the HTTP server, a SEPARATE process from this
2704+
// MCP (stdio) server; the two share only the SQLite store, so the active
2705+
// session must be resolved from disk rather than from any in-process map.
2706+
//
2707+
// When no active session exists for the project (or the store query fails for
2708+
// any reason), it falls back to the manual-save-{project} session, preserving
2709+
// the prior behavior for projects with no live session.
2710+
func resolveFallbackSessionID(s *store.Store, project string) string {
2711+
if s != nil {
2712+
if id, ok, err := s.MostRecentActiveSession(project); err == nil && ok {
2713+
return id
2714+
}
2715+
}
2716+
return defaultSessionID(project)
2717+
}
2718+
26982719
func intArg(req mcp.CallToolRequest, key string, defaultVal int) int {
26992720
v, ok := req.GetArguments()[key].(float64)
27002721
if !ok {

‎internal/mcp/mcp_test.go‎

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,126 @@ func TestHandleSaveRecordsActivityForExplicitSessionID(t *testing.T) {
324324
}
325325
}
326326

327+
// TestHandleSaveResolvesActiveSessionFromStore reproduces issue #386: the
328+
// SessionStart hook registers a UUID session via POST /sessions (a separate
329+
// process from the MCP server, sharing only the SQLite store). A later
330+
// mem_save with no explicit session_id must attach to that UUID session,
331+
// resolved from the persisted sessions table — NOT fall back to
332+
// manual-save-{project}. The two processes never share in-memory state, so
333+
// store-based resolution is the only thing that survives the process split.
334+
func TestHandleSaveResolvesActiveSessionFromStore(t *testing.T) {
335+
s := newMCPTestStore(t)
336+
337+
// Simulate the SessionStart hook registering a UUID session (POST /sessions
338+
// ultimately calls store.CreateSession).
339+
const uuidSession = "0c8e7f2a-1b34-4d9e-9a77-aaaabbbbcccc"
340+
if err := s.CreateSession(uuidSession, "engram", "/work/engram"); err != nil {
341+
t.Fatalf("create UUID session: %v", err)
342+
}
343+
344+
// mem_save with NO session_id — exactly what the proactive protocol does.
345+
h := handleSave(s, MCPConfig{}, NewSessionActivity(10*time.Minute))
346+
res, err := h(context.Background(), mcppkg.CallToolRequest{Params: mcppkg.CallToolParams{Arguments: map[string]any{
347+
"title": "Active session resolution",
348+
"content": "**What**: saved without session_id\n**Why**: repro for #386",
349+
"type": "bugfix",
350+
"project": "engram",
351+
}}})
352+
if err != nil {
353+
t.Fatalf("handler error: %v", err)
354+
}
355+
if res.IsError {
356+
t.Fatalf("unexpected save error: %s", callResultText(t, res))
357+
}
358+
359+
obs, err := s.RecentObservations("engram", "project", 5)
360+
if err != nil {
361+
t.Fatalf("recent observations: %v", err)
362+
}
363+
if len(obs) == 0 {
364+
t.Fatalf("expected at least one observation, got none")
365+
}
366+
if obs[0].SessionID != uuidSession {
367+
t.Fatalf("expected observation to attach to active UUID session %q, got %q (regression #386: fell back to manual-save)", uuidSession, obs[0].SessionID)
368+
}
369+
}
370+
371+
// TestHandleSaveFallsBackToManualSaveWhenNoActiveSession is the regression
372+
// guard for the preserved behavior: when there is no un-ended session for the
373+
// project, mem_save with no session_id must still use manual-save-{project}.
374+
func TestHandleSaveFallsBackToManualSaveWhenNoActiveSession(t *testing.T) {
375+
s := newMCPTestStore(t)
376+
377+
h := handleSave(s, MCPConfig{}, NewSessionActivity(10*time.Minute))
378+
res, err := h(context.Background(), mcppkg.CallToolRequest{Params: mcppkg.CallToolParams{Arguments: map[string]any{
379+
"title": "No active session",
380+
"content": "**What**: saved with no active session\n**Why**: fallback regression guard",
381+
"type": "bugfix",
382+
"project": "engram",
383+
}}})
384+
if err != nil {
385+
t.Fatalf("handler error: %v", err)
386+
}
387+
if res.IsError {
388+
t.Fatalf("unexpected save error: %s", callResultText(t, res))
389+
}
390+
391+
obs, err := s.RecentObservations("engram", "project", 5)
392+
if err != nil {
393+
t.Fatalf("recent observations: %v", err)
394+
}
395+
if len(obs) == 0 {
396+
t.Fatalf("expected at least one observation, got none")
397+
}
398+
if want := defaultSessionID("engram"); obs[0].SessionID != want {
399+
t.Fatalf("expected fallback to %q with no active session, got %q", want, obs[0].SessionID)
400+
}
401+
}
402+
403+
// TestHandleSaveResolvesMostRecentActiveSession covers the multi-session edge
404+
// case: two un-ended sessions exist; mem_save must attach to the most recent.
405+
func TestHandleSaveResolvesMostRecentActiveSession(t *testing.T) {
406+
s := newMCPTestStore(t)
407+
408+
if err := s.CreateSession("uuid-older", "engram", "/work/engram"); err != nil {
409+
t.Fatalf("create older session: %v", err)
410+
}
411+
if _, err := s.DB().Exec(`UPDATE sessions SET started_at = ? WHERE id = ?`, "2025-01-01 00:00:00", "uuid-older"); err != nil {
412+
t.Fatalf("backdate older session: %v", err)
413+
}
414+
if err := s.CreateSession("uuid-newer", "engram", "/work/engram"); err != nil {
415+
t.Fatalf("create newer session: %v", err)
416+
}
417+
if _, err := s.DB().Exec(`UPDATE sessions SET started_at = ? WHERE id = ?`, "2025-06-01 00:00:00", "uuid-newer"); err != nil {
418+
t.Fatalf("set newer session started_at: %v", err)
419+
}
420+
421+
h := handleSave(s, MCPConfig{}, NewSessionActivity(10*time.Minute))
422+
res, err := h(context.Background(), mcppkg.CallToolRequest{Params: mcppkg.CallToolParams{Arguments: map[string]any{
423+
"title": "Most recent active session",
424+
"content": "**What**: saved with two active sessions\n**Why**: multi-session edge case",
425+
"type": "bugfix",
426+
"project": "engram",
427+
}}})
428+
if err != nil {
429+
t.Fatalf("handler error: %v", err)
430+
}
431+
if res.IsError {
432+
t.Fatalf("unexpected save error: %s", callResultText(t, res))
433+
}
434+
435+
obs, err := s.RecentObservations("engram", "project", 5)
436+
if err != nil {
437+
t.Fatalf("recent observations: %v", err)
438+
}
439+
if len(obs) == 0 {
440+
t.Fatalf("expected at least one observation, got none")
441+
}
442+
if obs[0].SessionID != "uuid-newer" {
443+
t.Fatalf("expected most recent active session uuid-newer, got %q", obs[0].SessionID)
444+
}
445+
}
446+
327447
func TestHandleSaveWithNilActivityStillSucceeds(t *testing.T) {
328448
s := newMCPTestStore(t)
329449
h := handleSave(s, MCPConfig{}, nil)

‎internal/store/store.go‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2015,6 +2015,50 @@ func (s *Store) GetSession(id string) (*Session, error) {
20152015
return &sess, nil
20162016
}
20172017

2018+
// MostRecentActiveSession resolves the active (un-ended) session for a project
2019+
// from the persisted sessions table. It returns the session ID and ok=true when
2020+
// such a session exists, or ok=false when none does.
2021+
//
2022+
// This is the cross-process resolution that fixes issue #386: the SessionStart
2023+
// hook registers a UUID session via the HTTP server (POST /sessions) in one
2024+
// process, while mem_save runs in the separate MCP (stdio) process. The two
2025+
// share only the SQLite store, so the active session must be read from disk —
2026+
// never from in-memory state.
2027+
//
2028+
// Selection rules:
2029+
// - Scope to the (normalized) project.
2030+
// - Require ended_at IS NULL — ended sessions are never returned, so stale
2031+
// sessions naturally fall out without any explicit clearing step.
2032+
// - Exclude the manual-save fallback sessions (id LIKE 'manual-save%'); those
2033+
// are created by the fallback path itself and must not be resolved as "the
2034+
// active session", which would make resolution circular.
2035+
// - When multiple un-ended sessions exist, pick the MOST RECENT by
2036+
// started_at DESC, with id DESC as a deterministic tie-breaker.
2037+
func (s *Store) MostRecentActiveSession(project string) (string, bool, error) {
2038+
project, _ = NormalizeProject(project)
2039+
if project == "" {
2040+
return "", false, nil
2041+
}
2042+
2043+
var id string
2044+
err := s.db.QueryRow(`
2045+
SELECT id
2046+
FROM sessions
2047+
WHERE LOWER(project) = ?
2048+
AND ended_at IS NULL
2049+
AND id NOT LIKE 'manual-save%'
2050+
ORDER BY datetime(started_at) DESC, id DESC
2051+
LIMIT 1
2052+
`, project).Scan(&id)
2053+
if errors.Is(err, sql.ErrNoRows) {
2054+
return "", false, nil
2055+
}
2056+
if err != nil {
2057+
return "", false, err
2058+
}
2059+
return id, true, nil
2060+
}
2061+
20182062
func (s *Store) RecentSessions(project string, limit int) ([]SessionSummary, error) {
20192063
// Normalize project filter for case-insensitive matching
20202064
project, _ = NormalizeProject(project)

‎internal/store/store_test.go‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8039,3 +8039,111 @@ func TestDeleteProjectOrphansMemoryRelations(t *testing.T) {
80398039
t.Errorf("expected relation judgment_status = orphaned after hard delete, got %q", judgmentStatus)
80408040
}
80418041
}
8042+
8043+
func TestMostRecentActiveSessionReturnsUnEndedSession(t *testing.T) {
8044+
s := newTestStore(t)
8045+
8046+
// A hook-registered UUID session, never ended.
8047+
if err := s.CreateSession("uuid-active-1", "engram", "/work/engram"); err != nil {
8048+
t.Fatalf("create session: %v", err)
8049+
}
8050+
8051+
id, ok, err := s.MostRecentActiveSession("engram")
8052+
if err != nil {
8053+
t.Fatalf("MostRecentActiveSession: %v", err)
8054+
}
8055+
if !ok || id != "uuid-active-1" {
8056+
t.Fatalf("expected active session uuid-active-1, got id=%q ok=%v", id, ok)
8057+
}
8058+
}
8059+
8060+
func TestMostRecentActiveSessionSkipsEndedSessions(t *testing.T) {
8061+
s := newTestStore(t)
8062+
8063+
if err := s.CreateSession("uuid-ended-1", "engram", "/work/engram"); err != nil {
8064+
t.Fatalf("create session: %v", err)
8065+
}
8066+
if err := s.EndSession("uuid-ended-1", "done"); err != nil {
8067+
t.Fatalf("end session: %v", err)
8068+
}
8069+
8070+
_, ok, err := s.MostRecentActiveSession("engram")
8071+
if err != nil {
8072+
t.Fatalf("MostRecentActiveSession: %v", err)
8073+
}
8074+
if ok {
8075+
t.Fatalf("expected no active session when the only session is ended, got ok=%v", ok)
8076+
}
8077+
}
8078+
8079+
func TestMostRecentActiveSessionNoSessionsReturnsFalse(t *testing.T) {
8080+
s := newTestStore(t)
8081+
8082+
_, ok, err := s.MostRecentActiveSession("engram")
8083+
if err != nil {
8084+
t.Fatalf("MostRecentActiveSession: %v", err)
8085+
}
8086+
if ok {
8087+
t.Fatalf("expected ok=false for a project with no sessions, got ok=%v", ok)
8088+
}
8089+
}
8090+
8091+
func TestMostRecentActiveSessionPicksMostRecentWhenMultipleActive(t *testing.T) {
8092+
s := newTestStore(t)
8093+
8094+
// Two un-ended UUID sessions for the same project; the newer started_at wins.
8095+
if err := s.CreateSession("uuid-old", "engram", "/work/engram"); err != nil {
8096+
t.Fatalf("create old session: %v", err)
8097+
}
8098+
if _, err := s.db.Exec(`UPDATE sessions SET started_at = ? WHERE id = ?`, "2025-01-01 00:00:00", "uuid-old"); err != nil {
8099+
t.Fatalf("backdate old session: %v", err)
8100+
}
8101+
if err := s.CreateSession("uuid-new", "engram", "/work/engram"); err != nil {
8102+
t.Fatalf("create new session: %v", err)
8103+
}
8104+
if _, err := s.db.Exec(`UPDATE sessions SET started_at = ? WHERE id = ?`, "2025-06-01 00:00:00", "uuid-new"); err != nil {
8105+
t.Fatalf("set new session started_at: %v", err)
8106+
}
8107+
8108+
id, ok, err := s.MostRecentActiveSession("engram")
8109+
if err != nil {
8110+
t.Fatalf("MostRecentActiveSession: %v", err)
8111+
}
8112+
if !ok || id != "uuid-new" {
8113+
t.Fatalf("expected most recent active session uuid-new, got id=%q ok=%v", id, ok)
8114+
}
8115+
}
8116+
8117+
func TestMostRecentActiveSessionIgnoresManualSaveSessions(t *testing.T) {
8118+
s := newTestStore(t)
8119+
8120+
// The manual-save fallback session is also un-ended, but it must NOT be
8121+
// resolved as "the active session" — otherwise resolution becomes circular.
8122+
if err := s.CreateSession("manual-save-engram", "engram", "/work/engram"); err != nil {
8123+
t.Fatalf("create manual-save session: %v", err)
8124+
}
8125+
8126+
_, ok, err := s.MostRecentActiveSession("engram")
8127+
if err != nil {
8128+
t.Fatalf("MostRecentActiveSession: %v", err)
8129+
}
8130+
if ok {
8131+
t.Fatalf("expected manual-save session to be ignored, got ok=%v", ok)
8132+
}
8133+
}
8134+
8135+
func TestMostRecentActiveSessionScopedByProject(t *testing.T) {
8136+
s := newTestStore(t)
8137+
8138+
if err := s.CreateSession("uuid-other-proj", "other", "/work/other"); err != nil {
8139+
t.Fatalf("create session: %v", err)
8140+
}
8141+
8142+
_, ok, err := s.MostRecentActiveSession("engram")
8143+
if err != nil {
8144+
t.Fatalf("MostRecentActiveSession: %v", err)
8145+
}
8146+
if ok {
8147+
t.Fatalf("expected no active session for engram when only 'other' has one, got ok=%v", ok)
8148+
}
8149+
}

0 commit comments

Comments
 (0)