From 19c35317ad895968a12ae037b7f111473ff77545 Mon Sep 17 00:00:00 2001 From: afjcjsbx Date: Tue, 21 Apr 2026 19:19:39 +0200 Subject: [PATCH] feat(web): hide ephemeral media refs from persisted session history --- web/backend/api/session.go | 52 +++++++++++----------- web/backend/api/session_test.go | 77 +++++++++++++++++++++++++++------ 2 files changed, 90 insertions(+), 39 deletions(-) diff --git a/web/backend/api/session.go b/web/backend/api/session.go index 8382fe6fe..0483b57cc 100644 --- a/web/backend/api/session.go +++ b/web/backend/api/session.go @@ -5,7 +5,6 @@ import ( "encoding/json" "errors" "net/http" - "net/url" "os" "path/filepath" "sort" @@ -407,10 +406,12 @@ func (h *Handler) findLegacyPicoSession(dir, sessionID string) (picoLegacySessio } func buildSessionListItem(sessionID string, sess sessionFile, toolFeedbackMaxArgsLength int) sessionListItem { + transcript := visibleSessionMessages(sess.Messages, toolFeedbackMaxArgsLength) + preview := "" - for _, msg := range sess.Messages { + for _, msg := range transcript { if msg.Role == "user" { - preview = sessionMessagePreview(msg) + preview = sessionChatMessagePreview(msg) } if preview != "" { break @@ -423,13 +424,11 @@ func buildSessionListItem(sessionID string, sess sessionFile, toolFeedbackMaxArg } title := preview - validMessageCount := len(visibleSessionMessages(sess.Messages, toolFeedbackMaxArgsLength)) - return sessionListItem{ ID: sessionID, Title: title, Preview: preview, - MessageCount: validMessageCount, + MessageCount: len(transcript), Created: sess.Created.Format(time.RFC3339), Updated: sess.Updated.Format(time.RFC3339), } @@ -450,11 +449,11 @@ func truncateRunes(s string, maxLen int) string { return string(runes[:maxLen]) + "..." } -func sessionMessageVisible(msg providers.Message) bool { +func sessionChatMessageVisible(msg sessionChatMessage) bool { return strings.TrimSpace(msg.Content) != "" || len(msg.Media) > 0 || len(msg.Attachments) > 0 } -func sessionMessagePreview(msg providers.Message) string { +func sessionChatMessagePreview(msg sessionChatMessage) string { if content := strings.TrimSpace(msg.Content); content != "" { return content } @@ -484,13 +483,14 @@ func visibleSessionMessages(messages []providers.Message, toolFeedbackMaxArgsLen continue case "user": - if sessionMessageVisible(msg) { - transcript = append(transcript, sessionChatMessage{ - Role: "user", - Content: msg.Content, - Media: append([]string(nil), msg.Media...), - Attachments: attachments, - }) + chatMsg := sessionChatMessage{ + Role: "user", + Content: msg.Content, + Media: append([]string(nil), msg.Media...), + Attachments: attachments, + } + if sessionChatMessageVisible(chatMsg) { + transcript = append(transcript, chatMsg) } case "assistant": @@ -513,10 +513,6 @@ func visibleSessionMessages(messages []providers.Message, toolFeedbackMaxArgsLen // Pico web chat can persist both visible `message` tool output and a // later plain assistant reply in the same turn. Hide only the fixed // internal summary that marks handled tool delivery. - if !sessionMessageVisible(msg) { - continue - } - content := msg.Content if assistantMessageInternalOnly(msg) { if len(attachments) == 0 { @@ -525,12 +521,17 @@ func visibleSessionMessages(messages []providers.Message, toolFeedbackMaxArgsLen content = "" } - transcript = append(transcript, sessionChatMessage{ + chatMsg := sessionChatMessage{ Role: "assistant", Content: content, Media: append([]string(nil), msg.Media...), Attachments: attachments, - }) + } + if !sessionChatMessageVisible(chatMsg) { + continue + } + + transcript = append(transcript, chatMsg) } } @@ -587,11 +588,10 @@ func sessionAttachmentURL(attachment providers.Attachment) (string, bool) { return "", false } if strings.HasPrefix(ref, "media://") { - refID := strings.TrimSpace(strings.TrimPrefix(ref, "media://")) - if refID == "" { - return "", false - } - return "/pico/media/" + url.PathEscape(refID), true + // Persisted session history must only expose durable attachment locations. + // media:// refs depend on the live in-memory MediaStore and may stop + // resolving after a restart or cleanup, so omit them from reopened history. + return "", false } return ref, true } diff --git a/web/backend/api/session_test.go b/web/backend/api/session_test.go index 76df11bd0..d2efb3879 100644 --- a/web/backend/api/session_test.go +++ b/web/backend/api/session_test.go @@ -218,7 +218,7 @@ func TestHandleGetSession_JSONLStorage(t *testing.T) { } } -func TestHandleGetSession_ExposesHandledToolAttachments(t *testing.T) { +func TestHandleGetSession_HidesHandledToolAttachmentsBackedByMediaRefs(t *testing.T) { configPath, cleanup := setupOAuthTestEnv(t) defer cleanup() @@ -260,16 +260,63 @@ func TestHandleGetSession_ExposesHandledToolAttachments(t *testing.T) { } var resp struct { - Messages []struct { - Role string `json:"role"` - Content string `json:"content"` - Attachments []struct { - Type string `json:"type"` - URL string `json:"url"` - Filename string `json:"filename"` - ContentType string `json:"content_type"` - } `json:"attachments"` - } `json:"messages"` + Messages []sessionChatMessage `json:"messages"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("Unmarshal() error = %v", err) + } + + if len(resp.Messages) != 1 { + t.Fatalf("len(resp.Messages) = %d, want 1", len(resp.Messages)) + } + if resp.Messages[0].Role != "user" || resp.Messages[0].Content != "send me the report" { + t.Fatalf("message = %#v, want only user request", resp.Messages[0]) + } +} + +func TestHandleGetSession_ExposesHandledToolAttachmentsWithDurableURL(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) + } + + sessionKey := legacyPicoSessionPrefix + "attachment-history-durable" + for _, msg := range []providers.Message{ + {Role: "user", Content: "send me the report"}, + { + Role: "assistant", + Content: handledToolResponseSummaryText, + Attachments: []providers.Attachment{{ + Type: "file", + URL: "https://example.com/report.txt", + Filename: "report.txt", + ContentType: "text/plain", + }}, + }, + } { + if err := store.AddFullMessage(nil, sessionKey, msg); err != nil { + t.Fatalf("AddFullMessage() error = %v", err) + } + } + + h := NewHandler(configPath) + mux := http.NewServeMux() + h.RegisterRoutes(mux) + + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/api/sessions/attachment-history-durable", 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 resp struct { + Messages []sessionChatMessage `json:"messages"` } if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { t.Fatalf("Unmarshal() error = %v", err) @@ -289,8 +336,12 @@ func TestHandleGetSession_ExposesHandledToolAttachments(t *testing.T) { if len(assistant.Attachments) != 1 { t.Fatalf("len(assistant.Attachments) = %d, want 1", len(assistant.Attachments)) } - if assistant.Attachments[0].URL != "/pico/media/attachment-1" { - t.Fatalf("attachment url = %q, want %q", assistant.Attachments[0].URL, "/pico/media/attachment-1") + if assistant.Attachments[0].URL != "https://example.com/report.txt" { + t.Fatalf( + "attachment url = %q, want %q", + assistant.Attachments[0].URL, + "https://example.com/report.txt", + ) } if assistant.Attachments[0].Filename != "report.txt" { t.Fatalf("attachment filename = %q, want %q", assistant.Attachments[0].Filename, "report.txt")