From 553afcccfbf5d950defd4c3a02c30be2273e1516 Mon Sep 17 00:00:00 2001 From: Alix-007 <267018309+Alix-007@users.noreply.github.com> Date: Thu, 26 Mar 2026 04:51:14 +0800 Subject: [PATCH] fix(web): include non-pico sessions in history API --- web/backend/api/session.go | 82 ++++++++++++---- web/backend/api/session_test.go | 167 ++++++++++++++++++++++++++++++++ 2 files changed, 232 insertions(+), 17 deletions(-) diff --git a/web/backend/api/session.go b/web/backend/api/session.go index 42d451a05..27674c912 100644 --- a/web/backend/api/session.go +++ b/web/backend/api/session.go @@ -56,7 +56,7 @@ type sessionMetaFile struct { // // agent:main:pico:direct:pico: // -// The sanitized filename replaces ':' with '_', so on disk it becomes: +// The sanitized filename replaces ':', '/' and '\' with '_', so on disk it becomes: // // agent_main_pico_direct_pico_.json const ( @@ -83,11 +83,44 @@ func extractPicoSessionIDFromSanitizedKey(key string) (string, bool) { } func sanitizeSessionKey(key string) string { - return strings.ReplaceAll(key, ":", "_") + key = strings.ReplaceAll(key, ":", "_") + key = strings.ReplaceAll(key, "/", "_") + return strings.ReplaceAll(key, "\\", "_") } -func (h *Handler) readLegacySession(dir, sessionID string) (sessionFile, error) { - path := filepath.Join(dir, sanitizeSessionKey(picoSessionPrefix+sessionID)+".json") +// sessionIDFromKey preserves the short Pico session IDs already used by the +// web UI, but returns the full key for every other channel so list/detail/delete +// can round-trip the same identifier. +func sessionIDFromKey(key string) string { + if id, ok := extractPicoSessionID(key); ok { + return id + } + return key +} + +// resolveSessionKey keeps existing Pico detail/delete compatibility for legacy +// short IDs, while allowing non-Pico sessions to pass their full session key. +func resolveSessionKey(sessionID string) string { + if strings.HasPrefix(sessionID, "agent:") { + return sessionID + } + return picoSessionPrefix + sessionID +} + +func (h *Handler) resolveSessionKeyFromSanitizedBase(dir, base string) (string, bool) { + metaPath := filepath.Join(dir, base+".meta.json") + meta, err := h.readSessionMeta(metaPath, "") + if err == nil && strings.TrimSpace(meta.Key) != "" { + return meta.Key, true + } + if id, ok := extractPicoSessionIDFromSanitizedKey(base); ok { + return picoSessionPrefix + id, true + } + return "", false +} + +func (h *Handler) readLegacySessionByKey(dir, sessionKey string) (sessionFile, error) { + path := filepath.Join(dir, sanitizeSessionKey(sessionKey)+".json") data, err := os.ReadFile(path) if err != nil { return sessionFile{}, err @@ -154,8 +187,7 @@ func (h *Handler) readSessionMessages(path string, skip int) ([]providers.Messag return msgs, nil } -func (h *Handler) readJSONLSession(dir, sessionID string) (sessionFile, error) { - sessionKey := picoSessionPrefix + sessionID +func (h *Handler) readJSONLSessionByKey(dir, sessionKey string) (sessionFile, error) { base := filepath.Join(dir, sanitizeSessionKey(sessionKey)) jsonlPath := base + ".jsonl" metaPath := base + ".meta.json" @@ -192,6 +224,14 @@ func (h *Handler) readJSONLSession(dir, sessionID string) (sessionFile, error) { }, nil } +func (h *Handler) readLegacySession(dir, sessionID string) (sessionFile, error) { + return h.readLegacySessionByKey(dir, resolveSessionKey(sessionID)) +} + +func (h *Handler) readJSONLSession(dir, sessionID string) (sessionFile, error) { + return h.readJSONLSessionByKey(dir, resolveSessionKey(sessionID)) +} + func buildSessionListItem(sessionID string, sess sessionFile) sessionListItem { preview := "" for _, msg := range sess.Messages { @@ -310,24 +350,30 @@ func (h *Handler) handleListSessions(w http.ResponseWriter, r *http.Request) { switch { case strings.HasSuffix(name, ".jsonl"): - sessionID, ok = extractPicoSessionIDFromSanitizedKey(strings.TrimSuffix(name, ".jsonl")) - if !ok { + base := strings.TrimSuffix(name, ".jsonl") + sessionKey, found := h.resolveSessionKeyFromSanitizedBase(dir, base) + if !found { continue } - sess, loadErr = h.readJSONLSession(dir, sessionID) + sess, loadErr = h.readJSONLSessionByKey(dir, sessionKey) if loadErr == nil && isEmptySession(sess) { continue } + sessionID = sessionIDFromKey(sess.Key) + if sessionID == "" { + sessionID = sessionIDFromKey(sessionKey) + } + ok = sessionID != "" + if !ok { + continue + } case strings.HasSuffix(name, ".meta.json"): continue case filepath.Ext(name) == ".json": base := strings.TrimSuffix(name, ".json") if _, statErr := os.Stat(filepath.Join(dir, base+".jsonl")); statErr == nil { - if jsonlSessionID, found := extractPicoSessionIDFromSanitizedKey(base); found { - if jsonlSess, jsonlErr := h.readJSONLSession( - dir, - jsonlSessionID, - ); jsonlErr == nil && + if sessionKey, found := h.resolveSessionKeyFromSanitizedBase(dir, base); found { + if jsonlSess, jsonlErr := h.readJSONLSessionByKey(dir, sessionKey); jsonlErr == nil && !isEmptySession(jsonlSess) { continue } @@ -343,7 +389,8 @@ func (h *Handler) handleListSessions(w http.ResponseWriter, r *http.Request) { if isEmptySession(sess) { continue } - sessionID, ok = extractPicoSessionID(sess.Key) + sessionID = sessionIDFromKey(sess.Key) + ok = sessionID != "" if !ok { continue } @@ -422,7 +469,7 @@ func (h *Handler) handleGetSession(w http.ResponseWriter, r *http.Request) { } if err != nil { if errors.Is(err, os.ErrNotExist) { - sess, err = h.readLegacySession(dir, sessionID) + sess, err = h.readLegacySessionByKey(dir, resolveSessionKey(sessionID)) if err == nil && isEmptySession(sess) { err = os.ErrNotExist } @@ -480,7 +527,8 @@ func (h *Handler) handleDeleteSession(w http.ResponseWriter, r *http.Request) { return } - base := filepath.Join(dir, sanitizeSessionKey(picoSessionPrefix+sessionID)) + sessionKey := resolveSessionKey(sessionID) + base := filepath.Join(dir, sanitizeSessionKey(sessionKey)) jsonlPath := base + ".jsonl" metaPath := base + ".meta.json" legacyPath := base + ".json" diff --git a/web/backend/api/session_test.go b/web/backend/api/session_test.go index 21ef5b5b8..78096b724 100644 --- a/web/backend/api/session_test.go +++ b/web/backend/api/session_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "net/url" "os" "path/filepath" "testing" @@ -320,3 +321,169 @@ func TestHandleSessions_FiltersEmptyJSONLFiles(t *testing.T) { t.Fatalf("detail status = %d, want %d, body=%s", detailRec.Code, http.StatusNotFound, detailRec.Body.String()) } } + +func TestHandleSessions_NonPicoJSONLStorage(t *testing.T) { + configPath, cleanup := setupOAuthTestEnv(t) + defer cleanup() + + dir := sessionsTestDir(t, configPath) + store, err := memory.NewJSONLStore(dir) + if err != nil { + t.Fatalf("NewJSONLStore() error = %v", err) + } + + nonPicoKey := "agent:main:slack:channel:C123456/1723456789.123456" + if err := store.AddFullMessage(nil, nonPicoKey, providers.Message{ + Role: "user", + Content: "slack user message", + }); err != nil { + t.Fatalf("AddFullMessage(user) error = %v", err) + } + if err := store.AddFullMessage(nil, nonPicoKey, providers.Message{ + Role: "assistant", + Content: "slack assistant message", + }); err != nil { + t.Fatalf("AddFullMessage(assistant) error = %v", err) + } + if err := store.SetSummary(nil, nonPicoKey, "Slack session summary"); err != nil { + t.Fatalf("SetSummary() error = %v", err) + } + + h := NewHandler(configPath) + mux := http.NewServeMux() + h.RegisterRoutes(mux) + + listRec := httptest.NewRecorder() + listReq := httptest.NewRequest(http.MethodGet, "/api/sessions", nil) + mux.ServeHTTP(listRec, listReq) + + if listRec.Code != http.StatusOK { + t.Fatalf("list status = %d, want %d, body=%s", listRec.Code, http.StatusOK, listRec.Body.String()) + } + + var items []sessionListItem + if err := json.Unmarshal(listRec.Body.Bytes(), &items); err != nil { + t.Fatalf("Unmarshal(list) error = %v", err) + } + if len(items) != 1 { + t.Fatalf("len(items) = %d, want 1", len(items)) + } + if items[0].ID != nonPicoKey { + t.Fatalf("items[0].ID = %q, want %q", items[0].ID, nonPicoKey) + } + if items[0].MessageCount != 2 { + t.Fatalf("items[0].MessageCount = %d, want 2", items[0].MessageCount) + } + + escapedKey := url.PathEscape(nonPicoKey) + + detailRec := httptest.NewRecorder() + detailReq := httptest.NewRequest(http.MethodGet, "/api/sessions/"+escapedKey, nil) + mux.ServeHTTP(detailRec, detailReq) + + if detailRec.Code != http.StatusOK { + t.Fatalf("detail status = %d, want %d, body=%s", detailRec.Code, http.StatusOK, detailRec.Body.String()) + } + + var detail struct { + ID string `json:"id"` + Messages []struct { + Role string `json:"role"` + Content string `json:"content"` + } `json:"messages"` + } + if err := json.Unmarshal(detailRec.Body.Bytes(), &detail); err != nil { + t.Fatalf("Unmarshal(detail) error = %v", err) + } + if detail.ID != nonPicoKey { + t.Fatalf("detail.ID = %q, want %q", detail.ID, nonPicoKey) + } + if len(detail.Messages) != 2 { + t.Fatalf("len(detail.Messages) = %d, want 2", len(detail.Messages)) + } + + deleteRec := httptest.NewRecorder() + deleteReq := httptest.NewRequest(http.MethodDelete, "/api/sessions/"+escapedKey, nil) + mux.ServeHTTP(deleteRec, deleteReq) + + if deleteRec.Code != http.StatusNoContent { + t.Fatalf("delete status = %d, want %d, body=%s", deleteRec.Code, http.StatusNoContent, deleteRec.Body.String()) + } + + base := filepath.Join(dir, sanitizeSessionKey(nonPicoKey)) + for _, path := range []string{base + ".jsonl", base + ".meta.json"} { + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Fatalf("expected %s to be removed, stat err = %v", path, err) + } + } +} + +func TestHandleListSessions_NonPicoLegacyJSONStorage(t *testing.T) { + configPath, cleanup := setupOAuthTestEnv(t) + defer cleanup() + + dir := sessionsTestDir(t, configPath) + nonPicoKey := "agent:main:discord:channel:guild-42/thread-7" + legacyPath := filepath.Join(dir, sanitizeSessionKey(nonPicoKey)+".json") + + legacySession := sessionFile{ + Key: nonPicoKey, + Messages: []providers.Message{ + {Role: "user", Content: "legacy discord user"}, + {Role: "assistant", Content: "legacy discord assistant"}, + }, + Summary: "Legacy discord session", + } + data, err := json.Marshal(legacySession) + if err != nil { + t.Fatalf("Marshal() error = %v", err) + } + if err := os.WriteFile(legacyPath, data, 0o644); err != nil { + t.Fatalf("WriteFile() error = %v", err) + } + + h := NewHandler(configPath) + mux := http.NewServeMux() + h.RegisterRoutes(mux) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/api/sessions", nil) + mux.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d, body=%s", rec.Code, http.StatusOK, rec.Body.String()) + } + + var items []sessionListItem + if err := json.Unmarshal(rec.Body.Bytes(), &items); err != nil { + t.Fatalf("Unmarshal() error = %v", err) + } + if len(items) != 1 { + t.Fatalf("len(items) = %d, want 1", len(items)) + } + if items[0].ID != nonPicoKey { + t.Fatalf("items[0].ID = %q, want %q", items[0].ID, nonPicoKey) + } + + escapedKey := url.PathEscape(nonPicoKey) + + detailRec := httptest.NewRecorder() + detailReq := httptest.NewRequest(http.MethodGet, "/api/sessions/"+escapedKey, nil) + mux.ServeHTTP(detailRec, detailReq) + + if detailRec.Code != http.StatusOK { + t.Fatalf("detail status = %d, want %d, body=%s", detailRec.Code, http.StatusOK, detailRec.Body.String()) + } + + deleteRec := httptest.NewRecorder() + deleteReq := httptest.NewRequest(http.MethodDelete, "/api/sessions/"+escapedKey, nil) + mux.ServeHTTP(deleteRec, deleteReq) + + if deleteRec.Code != http.StatusNoContent { + t.Fatalf("delete status = %d, want %d, body=%s", deleteRec.Code, http.StatusNoContent, deleteRec.Body.String()) + } + + if _, err := os.Stat(legacyPath); !os.IsNotExist(err) { + t.Fatalf("expected %s to be removed, stat err = %v", legacyPath, err) + } +}