diff --git a/internal/session/comments.go b/internal/session/comments.go index 028415cf..08fe2eb3 100644 --- a/internal/session/comments.go +++ b/internal/session/comments.go @@ -15,7 +15,7 @@ import ( // earlier one, and a subsequent review_item_failed drops it. Comments that // were persisted without a path inherit the record's file path. func LoadComments(repoDir, sessionID string) ([]model.LlmComment, error) { - path, err := SessionFilePath(repoDir, sessionID) + path, err := findSessionFile(repoDir, sessionID) if err != nil { return nil, err } diff --git a/internal/session/list.go b/internal/session/list.go index 0fd424b5..4c133bc4 100644 --- a/internal/session/list.go +++ b/internal/session/list.go @@ -96,40 +96,60 @@ type summaryRecord struct { // SessionsDir returns the on-disk directory that holds JSONL session files // for a given repository. It does not create the directory. func SessionsDir(repoDir string) (string, error) { - home, err := os.UserHomeDir() - if err != nil { - return "", fmt.Errorf("resolve home dir: %w", err) - } - return filepath.Join(home, ".opencodereview", sessionSubDir, encodeRepoPath(repoDir)), nil + current, _, err := sessionDirectories(repoDir) + return current, err } // ListSessions enumerates all persisted sessions for the given repository // directory, sorted by StartTime descending (most recent first). Missing // directories return an empty slice with no error. func ListSessions(repoDir string) ([]Summary, error) { - dir, err := SessionsDir(repoDir) + currentDir, legacyDir, err := sessionDirectories(repoDir) if err != nil { return nil, err } - entries, err := os.ReadDir(dir) - if err != nil { - if os.IsNotExist(err) { - return nil, nil + summaries := make([]Summary, 0) + seenSessionIDs := make(map[string]struct{}) + readDir := func(dir string, legacy bool) error { + entries, readErr := os.ReadDir(dir) + if readErr != nil { + if os.IsNotExist(readErr) { + return nil + } + return fmt.Errorf("read sessions dir %q: %w", dir, readErr) } - return nil, fmt.Errorf("read sessions dir %q: %w", dir, err) - } - summaries := make([]Summary, 0, len(entries)) - for _, entry := range entries { - name := entry.Name() - if entry.IsDir() || !strings.HasSuffix(name, ".jsonl") { - continue + for _, entry := range entries { + name := entry.Name() + if entry.IsDir() || !strings.HasSuffix(name, ".jsonl") { + continue + } + path := filepath.Join(dir, name) + if legacy { + matches, matchErr := sessionFileBelongsToRepo(path, repoDir) + if matchErr != nil || !matches { + continue + } + } + sessionID := strings.TrimSuffix(name, ".jsonl") + if _, seen := seenSessionIDs[sessionID]; seen { + continue + } + summary, loadErr := loadSummaryFromFile(path, sessionID, repoDir) + if loadErr != nil { + continue + } + summaries = append(summaries, *summary) + seenSessionIDs[sessionID] = struct{}{} } - sessionID := strings.TrimSuffix(name, ".jsonl") - summary, err := loadSummaryFromFile(filepath.Join(dir, name), sessionID, repoDir) - if err != nil { - continue + return nil + } + if err := readDir(currentDir, false); err != nil { + return nil, err + } + if legacyDir != currentDir { + if err := readDir(legacyDir, true); err != nil { + return nil, err } - summaries = append(summaries, *summary) } sort.Slice(summaries, func(i, j int) bool { return summaries[i].StartTime.After(summaries[j].StartTime) @@ -140,7 +160,7 @@ func ListSessions(repoDir string) ([]Summary, error) { // LoadSummary loads a single session's Summary. Errors when the session // file is missing or unreadable. func LoadSummary(repoDir, sessionID string) (*Summary, error) { - path, err := SessionFilePath(repoDir, sessionID) + path, err := findSessionFile(repoDir, sessionID) if err != nil { return nil, err } @@ -149,7 +169,7 @@ func LoadSummary(repoDir, sessionID string) (*Summary, error) { // LoadDetail returns the summary plus per-file item records for one session. func LoadDetail(repoDir, sessionID string) (*Summary, []ItemDetail, error) { - path, err := SessionFilePath(repoDir, sessionID) + path, err := findSessionFile(repoDir, sessionID) if err != nil { return nil, nil, err } diff --git a/internal/session/list_test.go b/internal/session/list_test.go index 6cfedeae..470bd585 100644 --- a/internal/session/list_test.go +++ b/internal/session/list_test.go @@ -4,6 +4,8 @@ package session import ( + "encoding/json" + "os" "path/filepath" "testing" "time" @@ -24,6 +26,203 @@ func TestListSessions_EmptyRepoReturnsNil(t *testing.T) { } } +func TestListSessions_FiltersCollidingLegacySessionsByRepository(t *testing.T) { + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + base := t.TempDir() + repoA := filepath.Join(base, "team", "service-api") + repoB := filepath.Join(base, "team-service", "api") + if err := os.MkdirAll(repoA, 0755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(repoB, 0755); err != nil { + t.Fatal(err) + } + + legacyDir := filepath.Join(tmpHome, ".opencodereview", "test-sessions", encodeRepoPath(repoA)) + if err := os.MkdirAll(legacyDir, 0700); err != nil { + t.Fatal(err) + } + writeJSONL := func(sessionID, cwd string) { + t.Helper() + path := filepath.Join(legacyDir, sessionID+".jsonl") + data, err := json.Marshal(map[string]string{ + "type": "session_start", + "sessionId": sessionID, + "timestamp": "2026-08-01T00:00:00Z", + "cwd": cwd, + }) + if err != nil { + t.Fatal(err) + } + data = append(data, '\n') + if err := os.WriteFile(path, []byte(data), 0600); err != nil { + t.Fatal(err) + } + } + writeJSONL("repo-a-session", repoA) + writeJSONL("repo-b-session", repoB) + commentRecord, err := json.Marshal(map[string]any{ + "type": "review_item_done", + "filePath": "legacy.go", + "fingerprint": "fp-legacy", + "comments": []model.LlmComment{{Content: "legacy comment"}}, + }) + if err != nil { + t.Fatal(err) + } + commentFile, err := os.OpenFile(filepath.Join(legacyDir, "repo-a-session.jsonl"), os.O_APPEND|os.O_WRONLY, 0600) + if err != nil { + t.Fatal(err) + } + if _, err := commentFile.Write(append(commentRecord, '\n')); err != nil { + commentFile.Close() + t.Fatal(err) + } + if err := commentFile.Close(); err != nil { + t.Fatal(err) + } + + gotA, err := ListSessions(repoA) + if err != nil { + t.Fatalf("ListSessions(repoA): %v", err) + } + if len(gotA) != 1 || gotA[0].SessionID != "repo-a-session" { + t.Fatalf("repo A sessions = %+v, want only repo-a-session", gotA) + } + + gotB, err := ListSessions(repoB) + if err != nil { + t.Fatalf("ListSessions(repoB): %v", err) + } + if len(gotB) != 1 || gotB[0].SessionID != "repo-b-session" { + t.Fatalf("repo B sessions = %+v, want only repo-b-session", gotB) + } + + if _, err := LoadSummary(repoA, "repo-b-session"); !os.IsNotExist(err) { + t.Fatalf("LoadSummary(repoA, repo-b-session) error = %v, want not found", err) + } + + detail, items, err := LoadDetail(repoA, "repo-a-session") + if err != nil { + t.Fatalf("LoadDetail(repoA, repo-a-session): %v", err) + } + if detail.RepoDir != repoA || len(items) != 1 || items[0].FilePath != "legacy.go" { + t.Fatalf("legacy detail = (%+v, %+v), want repo A legacy item", detail, items) + } + if _, _, err := LoadDetail(repoB, "repo-a-session"); !os.IsNotExist(err) { + t.Fatalf("LoadDetail(repoB, repo-a-session) error = %v, want not found", err) + } + + state, err := LoadResumeState(repoA, "repo-a-session") + if err != nil { + t.Fatalf("LoadResumeState(repoA, repo-a-session): %v", err) + } + if state.RepoDir != repoA { + t.Fatalf("legacy resume RepoDir = %q, want %q", state.RepoDir, repoA) + } + if _, err := LoadResumeState(repoB, "repo-a-session"); !os.IsNotExist(err) { + t.Fatalf("LoadResumeState(repoB, repo-a-session) error = %v, want not found", err) + } + + comments, err := LoadComments(repoA, "repo-a-session") + if err != nil { + t.Fatalf("LoadComments(repoA, repo-a-session): %v", err) + } + if len(comments) != 1 || comments[0].Content != "legacy comment" || comments[0].Path != "legacy.go" { + t.Fatalf("legacy comments = %+v, want one inherited-path comment", comments) + } + if _, err := LoadComments(repoB, "repo-a-session"); !os.IsNotExist(err) { + t.Fatalf("LoadComments(repoB, repo-a-session) error = %v, want not found", err) + } +} + +func TestListSessions_DeduplicatesCurrentAndLegacySessionID(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + repoDir := t.TempDir() + currentDir, legacyDir, err := sessionDirectories(repoDir) + if err != nil { + t.Fatal(err) + } + for _, dir := range []string{currentDir, legacyDir} { + if err := os.MkdirAll(dir, 0700); err != nil { + t.Fatal(err) + } + } + + const sessionID = "duplicate-session" + writeStart := func(dir, model string) { + t.Helper() + data, marshalErr := json.Marshal(map[string]string{ + "type": "session_start", + "sessionId": sessionID, + "timestamp": "2026-08-01T00:00:00Z", + "cwd": repoDir, + "model": model, + }) + if marshalErr != nil { + t.Fatal(marshalErr) + } + if err := os.WriteFile(filepath.Join(dir, sessionID+".jsonl"), append(data, '\n'), 0600); err != nil { + t.Fatal(err) + } + } + writeStart(currentDir, "current-model") + writeStart(legacyDir, "legacy-model") + + summaries, err := ListSessions(repoDir) + if err != nil { + t.Fatal(err) + } + if len(summaries) != 1 { + t.Fatalf("sessions = %+v, want one deduplicated session", summaries) + } + if summaries[0].SessionID != sessionID || summaries[0].Model != "current-model" { + t.Fatalf("session = %+v, want current-format version", summaries[0]) + } +} + +func TestNewSessionsUseDistinctDirectoriesForCollidingPaths(t *testing.T) { + tmpHome := t.TempDir() + t.Setenv("HOME", tmpHome) + base := t.TempDir() + repoA := filepath.Join(base, "team", "service-api") + repoB := filepath.Join(base, "team-service", "api") + if err := os.MkdirAll(repoA, 0755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(repoB, 0755); err != nil { + t.Fatal(err) + } + + first := New(repoA, "main", "mock", SessionOptions{ReviewMode: ReviewModeWorkspace}) + if err := first.Finalize(); err != nil { + t.Fatal(err) + } + second := New(repoB, "main", "mock", SessionOptions{ReviewMode: ReviewModeWorkspace}) + if err := second.Finalize(); err != nil { + t.Fatal(err) + } + + dirA, err := SessionsDir(repoA) + if err != nil { + t.Fatal(err) + } + dirB, err := SessionsDir(repoB) + if err != nil { + t.Fatal(err) + } + if dirA == dirB { + t.Fatalf("session directories collided: %q", dirA) + } + if _, err := os.Stat(filepath.Join(dirA, first.SessionID+".jsonl")); err != nil { + t.Fatalf("repo A session missing from its directory: %v", err) + } + if _, err := os.Stat(filepath.Join(dirB, second.SessionID+".jsonl")); err != nil { + t.Fatalf("repo B session missing from its directory: %v", err) + } +} + func TestListSessions_SortsAndAggregates(t *testing.T) { tmpHome := t.TempDir() t.Setenv("HOME", tmpHome) diff --git a/internal/session/path.go b/internal/session/path.go new file mode 100644 index 00000000..bd24de06 --- /dev/null +++ b/internal/session/path.go @@ -0,0 +1,171 @@ +// SPDX-License-Identifier: Apache-2.0 +// Copyright 2026 alibaba/open-code-review Contributors + +package session + +import ( + "bufio" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "io" + "os" + "path/filepath" + "runtime" + "strings" +) + +const sessionPathVersion = "v2" + +// RepoSessionKey returns the collision-resistant directory name used for a +// repository's persisted sessions. The readable prefix is only for humans; +// the digest is derived from the canonical path and is the identity boundary. +func RepoSessionKey(repoDir string) string { + canonical := canonicalRepoPath(repoDir) + readable := encodeRepoPath(canonical) + if len(readable) > 48 { + readable = strings.TrimRight(readable[:48], "-") + } + if readable == "" { + readable = "empty" + } + digest := sha256.Sum256([]byte(canonical)) + return fmt.Sprintf("%s-%s-%x", sessionPathVersion, readable, digest[:12]) +} + +// IsRepoSessionKey reports whether key has the shape produced by +// RepoSessionKey. Checking the complete shape avoids mistaking legacy encoded +// paths that merely begin with "v2-" for current-format directories. +func IsRepoSessionKey(key string) bool { + if !strings.HasPrefix(key, sessionPathVersion+"-") { + return false + } + separator := strings.LastIndexByte(key, '-') + if separator <= len(sessionPathVersion) || len(key)-separator-1 != 24 { + return false + } + _, err := hex.DecodeString(key[separator+1:]) + return err == nil +} + +// canonicalRepoPath normalizes aliases that refer to the same repository +// before they are hashed. EvalSymlinks is best-effort because callers may ask +// for a path that has not been created yet (notably in tests). +func canonicalRepoPath(repoDir string) string { + if repoDir == "" { + return "empty" + } + + p, err := filepath.Abs(repoDir) + if err != nil { + p = filepath.Clean(repoDir) + } + if resolved, err := filepath.EvalSymlinks(p); err == nil { + p = resolved + } + p = filepath.Clean(p) + if runtime.GOOS == "windows" { + p = strings.ToLower(p) + } + return p +} + +// encodeRepoPath preserves the pre-v2 directory encoding for compatibility +// with sessions written by older OCR releases. It must not be used for new +// session directories. +func encodeRepoPath(p string) string { + if p == "" { + return "empty" + } + + vol := filepath.VolumeName(p) + p = p[len(vol):] + p = strings.TrimLeft(p, "/\\") + p = strings.ReplaceAll(p, "/", "-") + p = strings.ReplaceAll(p, "\\", "-") + vol = strings.ReplaceAll(vol, ":", "_") + result := vol + p + if result == "" { + return "empty" + } + return result +} + +func sessionDirectories(repoDir string) (current, legacy string, err error) { + home, err := os.UserHomeDir() + if err != nil { + return "", "", fmt.Errorf("resolve home dir: %w", err) + } + root := filepath.Join(home, ".opencodereview", sessionSubDir) + return filepath.Join(root, RepoSessionKey(repoDir)), filepath.Join(root, encodeRepoPath(repoDir)), nil +} + +func sessionFileCandidates(repoDir, sessionID string) (current, legacy string, err error) { + if sessionID == "" { + return "", "", fmt.Errorf("session id is required") + } + currentDir, legacyDir, err := sessionDirectories(repoDir) + if err != nil { + return "", "", err + } + return filepath.Join(currentDir, sessionID+".jsonl"), filepath.Join(legacyDir, sessionID+".jsonl"), nil +} + +// findSessionFile returns the current-format file when present. If only a +// legacy file exists, it is accepted only when its session_start cwd belongs +// to repoDir; this prevents old colliding directories from leaking sessions. +func findSessionFile(repoDir, sessionID string) (string, error) { + current, legacy, err := sessionFileCandidates(repoDir, sessionID) + if err != nil { + return "", err + } + if _, err := os.Stat(current); err == nil { + return current, nil + } else if !os.IsNotExist(err) { + return "", err + } + if _, err := os.Stat(legacy); err == nil { + matches, matchErr := sessionFileBelongsToRepo(legacy, repoDir) + if matchErr != nil { + return "", matchErr + } + if matches { + return legacy, nil + } + return "", os.ErrNotExist + } else if !os.IsNotExist(err) { + return "", err + } + return current, nil +} + +func sessionFileBelongsToRepo(path, repoDir string) (bool, error) { + f, err := os.Open(path) + if err != nil { + return false, err + } + defer f.Close() + + reader := bufio.NewReader(f) + for { + line, readErr := reader.ReadBytes('\n') + var rec struct { + Type string `json:"type"` + Cwd string `json:"cwd"` + } + if json.Unmarshal(line, &rec) == nil && rec.Type == "session_start" { + return rec.Cwd != "" && sameRepoPath(rec.Cwd, repoDir), nil + } + if readErr == io.EOF { + return false, nil + } + if readErr != nil { + return false, readErr + } + } +} + +func sameRepoPath(a, b string) bool { + return canonicalRepoPath(a) == canonicalRepoPath(b) +} diff --git a/internal/session/persist.go b/internal/session/persist.go index 9bf502c4..d2eea040 100644 --- a/internal/session/persist.go +++ b/internal/session/persist.go @@ -11,7 +11,6 @@ import ( "io" "os" "path/filepath" - "strings" "sync" "time" @@ -21,7 +20,7 @@ import ( var sessionSubDir = "sessions" // jsonlWriter streams session records to a JSONL file under -// $HOME/.opencodereview/sessions//.jsonl. +// $HOME/.opencodereview/sessions//.jsonl. // It is safe for concurrent use by multiple goroutines. type jsonlWriter struct { mu sync.Mutex @@ -73,40 +72,13 @@ func generateUUID() string { b[0:4], b[4:6], b[6:8], b[8:10], b[10:]) } -func encodeRepoPath(p string) string { - // Handle empty or invalid input - if p == "" { - return "empty" - } - - vol := filepath.VolumeName(p) - p = p[len(vol):] - - // Trim leading path separators - p = strings.TrimLeft(p, "/\\") - - // Replace separators with - - p = strings.ReplaceAll(p, "/", "-") - p = strings.ReplaceAll(p, "\\", "-") - - // Replace colons (from Windows drive letters) - vol = strings.ReplaceAll(vol, ":", "_") - - // Handle edge case where path was only separators or volume name - result := vol + p - if result == "" { - return "empty" - } - return result -} - func (jw *jsonlWriter) open() error { home, err := os.UserHomeDir() if err != nil { return fmt.Errorf("resolve home dir: %w", err) } - sessionDir := filepath.Join(home, ".opencodereview", sessionSubDir, encodeRepoPath(jw.repoDir)) + sessionDir := filepath.Join(home, ".opencodereview", sessionSubDir, RepoSessionKey(jw.repoDir)) if err := os.MkdirAll(sessionDir, 0700); err != nil { return fmt.Errorf("create session dir: %w", err) } diff --git a/internal/session/persist_test.go b/internal/session/persist_test.go index 604518d9..3dfb7f7e 100644 --- a/internal/session/persist_test.go +++ b/internal/session/persist_test.go @@ -110,6 +110,30 @@ func TestEncodeRepoPath(t *testing.T) { } } +func TestRepoSessionKey_DistinguishesCollidingLegacyPaths(t *testing.T) { + base := t.TempDir() + pathA := filepath.Join(base, "team", "service-api") + pathB := filepath.Join(base, "team-service", "api") + + if encodeRepoPath(pathA) != encodeRepoPath(pathB) { + t.Fatalf("test paths no longer collide under the legacy encoder: %q vs %q", encodeRepoPath(pathA), encodeRepoPath(pathB)) + } + if RepoSessionKey(pathA) == RepoSessionKey(pathB) { + t.Fatalf("RepoSessionKey(%q) and RepoSessionKey(%q) collided", pathA, pathB) + } + if !strings.HasPrefix(RepoSessionKey(pathA), "v2-") { + t.Fatalf("RepoSessionKey(%q) = %q, want versioned key", pathA, RepoSessionKey(pathA)) + } + if !IsRepoSessionKey(RepoSessionKey(pathA)) { + t.Fatalf("IsRepoSessionKey rejected generated key %q", RepoSessionKey(pathA)) + } + for _, legacy := range []string{"repo", "v2-repo", "v2-repo-not-a-digest"} { + if IsRepoSessionKey(legacy) { + t.Errorf("IsRepoSessionKey(%q) = true for legacy-shaped key", legacy) + } + } +} + func readJSONLRecords(t *testing.T, path string) []map[string]any { t.Helper() f, err := os.Open(path) @@ -136,7 +160,7 @@ func sessionJSONLPath(t *testing.T, repoDir, sessionID string) string { if err != nil { t.Fatalf("home dir: %v", err) } - return filepath.Join(home, ".opencodereview", "test-sessions", encodeRepoPath(repoDir), sessionID+".jsonl") + return filepath.Join(home, ".opencodereview", "test-sessions", RepoSessionKey(repoDir), sessionID+".jsonl") } func TestSetErrorIncrementsCounter(t *testing.T) { @@ -221,7 +245,7 @@ func TestSessionFilePermissions(t *testing.T) { jw.WriteSessionStart(time.Now()) defer jw.flushAndClose() - sessionDir := filepath.Join(tmpHome, ".opencodereview", "test-sessions", encodeRepoPath(repoDir)) + sessionDir := filepath.Join(tmpHome, ".opencodereview", "test-sessions", RepoSessionKey(repoDir)) sessionFile := filepath.Join(sessionDir, sessionID+".jsonl") dirInfo, err := os.Stat(sessionDir) diff --git a/internal/session/resume.go b/internal/session/resume.go index c9d70a9e..7293491a 100644 --- a/internal/session/resume.go +++ b/internal/session/resume.go @@ -79,14 +79,8 @@ type resumeRecord struct { // SessionFilePath returns the JSONL path for a persisted session. func SessionFilePath(repoDir, sessionID string) (string, error) { - if sessionID == "" { - return "", fmt.Errorf("session id is required") - } - home, err := os.UserHomeDir() - if err != nil { - return "", fmt.Errorf("resolve home dir: %w", err) - } - return filepath.Join(home, ".opencodereview", sessionSubDir, encodeRepoPath(repoDir), sessionID+".jsonl"), nil + current, _, err := sessionFileCandidates(repoDir, sessionID) + return current, err } // LoadResumeState replays a previous session JSONL into a fingerprint index. A @@ -109,7 +103,7 @@ func LoadReviewResumeState(repoDir, sessionID string) (*ResumeState, error) { } func loadResumeState(repoDir, sessionID string, skipUnparseable bool) (*ResumeState, error) { - path, err := SessionFilePath(repoDir, sessionID) + path, err := findSessionFile(repoDir, sessionID) if err != nil { return nil, err } diff --git a/internal/session/resume_test.go b/internal/session/resume_test.go index ab28f2f4..1e5183a1 100644 --- a/internal/session/resume_test.go +++ b/internal/session/resume_test.go @@ -31,7 +31,7 @@ func TestSessionFilePath_ValidID(t *testing.T) { t.Fatalf("unexpected error: %v", err) } - expectedSuffix := filepath.Join("test-sessions", encodeRepoPath("/some/repo"), "abc-123.jsonl") + expectedSuffix := filepath.Join("test-sessions", RepoSessionKey("/some/repo"), "abc-123.jsonl") if !strings.Contains(path, expectedSuffix) { t.Errorf("path %q does not contain expected suffix %q", path, expectedSuffix) } diff --git a/internal/viewer/store.go b/internal/viewer/store.go index d85abd1d..60dea8e4 100644 --- a/internal/viewer/store.go +++ b/internal/viewer/store.go @@ -37,7 +37,10 @@ type RepoInfo struct { LastModified time.Time } -// DiscoverRepos walks the sessions root and returns one entry per subdirectory. +// DiscoverRepos walks the sessions root and returns one entry per repository. +// Legacy session directories may contain records for multiple repositories +// because their path encoding was not collision-resistant; those records are +// split by the cwd recorded in each session_start event. func DiscoverRepos(root string) ([]RepoInfo, error) { entries, err := os.ReadDir(root) if err != nil { @@ -47,30 +50,46 @@ func DiscoverRepos(root string) ([]RepoInfo, error) { return nil, fmt.Errorf("read sessions dir: %w", err) } - var repos []RepoInfo + repoGroups := make(map[string]*RepoInfo) for _, e := range entries { if !e.IsDir() { continue } repoDir := filepath.Join(root, e.Name()) - info := RepoInfo{EncodedPath: e.Name()} - subEntries, err := os.ReadDir(repoDir) if err != nil { continue } for _, se := range subEntries { - if strings.HasSuffix(se.Name(), ".jsonl") { - info.SessionCount++ - if fi, err := se.Info(); err == nil { - if fi.ModTime().After(info.LastModified) { - info.LastModified = fi.ModTime() - } + if se.IsDir() || !strings.HasSuffix(se.Name(), ".jsonl") { + continue + } + key := e.Name() + if !isVersionedRepoKey(key) { + cwd, readErr := readSessionCWD(filepath.Join(repoDir, se.Name())) + if readErr != nil { + continue } + if cwd != "" { + key = session.RepoSessionKey(cwd) + } + } + info := repoGroups[key] + if info == nil { + info = &RepoInfo{EncodedPath: key} + repoGroups[key] = info + } + info.SessionCount++ + if fi, statErr := se.Info(); statErr == nil && fi.ModTime().After(info.LastModified) { + info.LastModified = fi.ModTime() } } + } + + var repos []RepoInfo + for _, info := range repoGroups { if info.SessionCount > 0 { - repos = append(repos, info) + repos = append(repos, *info) } } @@ -112,6 +131,12 @@ func ListSessions(root, encodedRepo string) ([]SessionSummary, error) { repoDir := filepath.Join(root, encodedRepo) entries, err := os.ReadDir(repoDir) if err != nil { + if !os.IsNotExist(err) { + return nil, fmt.Errorf("read repo dir: %w", err) + } + if isVersionedRepoKey(encodedRepo) { + return listLegacySessions(root, encodedRepo) + } return nil, fmt.Errorf("read repo dir: %w", err) } @@ -129,6 +154,88 @@ func ListSessions(root, encodedRepo string) ([]SessionSummary, error) { summaries = append(summaries, s) } + sort.Slice(summaries, func(i, j int) bool { + return summaries[i].Timestamp.After(summaries[j].Timestamp) + }) + if isVersionedRepoKey(encodedRepo) { + legacy, legacyErr := listLegacySessions(root, encodedRepo) + if legacyErr != nil { + return nil, legacyErr + } + summaries = append(summaries, legacy...) + sort.Slice(summaries, func(i, j int) bool { + return summaries[i].Timestamp.After(summaries[j].Timestamp) + }) + } + return summaries, nil +} + +func isVersionedRepoKey(encodedRepo string) bool { + return session.IsRepoSessionKey(encodedRepo) +} + +// readSessionCWD reads only through the session_start record. Repository +// discovery and legacy lookup need the owning repository, not the potentially +// large request and response records that follow it. +func readSessionCWD(path string) (string, error) { + f, err := os.Open(path) + if err != nil { + return "", err + } + defer f.Close() + + reader := bufio.NewReader(f) + for { + line, readErr := reader.ReadBytes('\n') + var rec struct { + Type string `json:"type"` + CWD string `json:"cwd"` + } + if json.Unmarshal(line, &rec) == nil && rec.Type == "session_start" { + return rec.CWD, nil + } + if readErr == io.EOF { + return "", nil + } + if readErr != nil { + return "", readErr + } + } +} + +func listLegacySessions(root, encodedRepo string) ([]SessionSummary, error) { + entries, err := os.ReadDir(root) + if err != nil { + return nil, fmt.Errorf("read sessions root: %w", err) + } + var summaries []SessionSummary + for _, dirEntry := range entries { + if !dirEntry.IsDir() { + continue + } + // Current-format directories are already read directly. Scanning them + // here would duplicate their sessions in the legacy compatibility pass. + if isVersionedRepoKey(dirEntry.Name()) { + continue + } + dir := filepath.Join(root, dirEntry.Name()) + files, readErr := os.ReadDir(dir) + if readErr != nil { + continue + } + for _, fileEntry := range files { + if fileEntry.IsDir() || !strings.HasSuffix(fileEntry.Name(), ".jsonl") { + continue + } + path := filepath.Join(dir, fileEntry.Name()) + summary, peekErr := peekSession(path) + if peekErr != nil || summary.CWD == "" || session.RepoSessionKey(summary.CWD) != encodedRepo { + continue + } + summary.SessionID = strings.TrimSuffix(fileEntry.Name(), ".jsonl") + summaries = append(summaries, summary) + } + } sort.Slice(summaries, func(i, j int) bool { return summaries[i].Timestamp.After(summaries[j].Timestamp) }) @@ -312,7 +419,17 @@ func LoadSession(root, encodedRepo, sessionID string) (*ViewSession, error) { path := filepath.Join(root, encodedRepo, sessionID+".jsonl") f, err := os.Open(path) if err != nil { - return nil, fmt.Errorf("open session file: %w", err) + if !os.IsNotExist(err) { + return nil, fmt.Errorf("open session file: %w", err) + } + path, err = findLegacySession(root, encodedRepo, sessionID) + if err != nil { + return nil, fmt.Errorf("open session file: %w", err) + } + f, err = os.Open(path) + if err != nil { + return nil, fmt.Errorf("open session file: %w", err) + } } defer f.Close() @@ -568,6 +685,31 @@ func LoadSession(root, encodedRepo, sessionID string) (*ViewSession, error) { return vs, readErr } +func findLegacySession(root, encodedRepo, sessionID string) (string, error) { + entries, err := os.ReadDir(root) + if err != nil { + return "", err + } + name := sessionID + ".jsonl" + for _, dirEntry := range entries { + if !dirEntry.IsDir() { + continue + } + if isVersionedRepoKey(dirEntry.Name()) { + continue + } + path := filepath.Join(root, dirEntry.Name(), name) + if _, statErr := os.Stat(path); statErr != nil { + continue + } + cwd, readErr := readSessionCWD(path) + if readErr == nil && cwd != "" && session.RepoSessionKey(cwd) == encodedRepo { + return path, nil + } + } + return "", os.ErrNotExist +} + func applySessionEnd(summary *SessionSummary, rec map[string]any) { summary.Aborted = false if dur, ok := rec["duration_seconds"].(float64); ok { diff --git a/internal/viewer/store_test.go b/internal/viewer/store_test.go index ddc3ea6f..ed752c9c 100644 --- a/internal/viewer/store_test.go +++ b/internal/viewer/store_test.go @@ -4,10 +4,14 @@ package viewer import ( + "encoding/json" + "errors" "os" "path/filepath" "testing" "time" + + "github.com/alibaba/open-code-review/internal/session" ) func writeJSONL(t *testing.T, path string, lines ...string) { @@ -21,6 +25,19 @@ func writeJSONL(t *testing.T, path string, lines ...string) { } } +func sessionStartLine(t *testing.T, timestamp, cwd string) string { + t.Helper() + b, err := json.Marshal(map[string]string{ + "type": "session_start", + "timestamp": timestamp, + "cwd": cwd, + }) + if err != nil { + t.Fatal(err) + } + return string(b) +} + func TestDiscoverRepos_Empty(t *testing.T) { root := t.TempDir() repos, err := DiscoverRepos(root) @@ -96,6 +113,88 @@ func TestDiscoverRepos_FindsRepos(t *testing.T) { } } +func TestDiscoverRepos_SplitsCollidingLegacyRepositories(t *testing.T) { + root := t.TempDir() + legacy := filepath.Join(root, "team-service-api") + if err := os.MkdirAll(legacy, 0755); err != nil { + t.Fatal(err) + } + repoA := filepath.Join(t.TempDir(), "team", "service-api") + repoB := filepath.Join(t.TempDir(), "team-service", "api") + writeJSONL(t, filepath.Join(legacy, "a.jsonl"), + sessionStartLine(t, "2026-08-01T00:00:00Z", repoA)) + writeJSONL(t, filepath.Join(legacy, "b.jsonl"), + sessionStartLine(t, "2026-08-02T00:00:00Z", repoB)) + + repos, err := DiscoverRepos(root) + if err != nil { + t.Fatal(err) + } + if len(repos) != 2 { + t.Fatalf("expected two logical repositories, got %+v", repos) + } + keys := map[string]int{} + for _, repo := range repos { + keys[repo.EncodedPath] = repo.SessionCount + } + if keys[session.RepoSessionKey(repoA)] != 1 || keys[session.RepoSessionKey(repoB)] != 1 { + t.Fatalf("legacy repositories were not split by cwd: %+v", keys) + } + + sessions, err := ListSessions(root, session.RepoSessionKey(repoA)) + if err != nil { + t.Fatal(err) + } + if len(sessions) != 1 || sessions[0].SessionID != "a" { + t.Fatalf("repo A sessions = %+v, want only a", sessions) + } + if _, err := LoadSession(root, session.RepoSessionKey(repoB), "a"); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("LoadSession(repo B, a) error = %v, want not found", err) + } +} + +func TestViewerMergesCurrentAndLegacySessionsForRepository(t *testing.T) { + root := t.TempDir() + repoDir := filepath.Join(t.TempDir(), "repo") + currentKey := session.RepoSessionKey(repoDir) + currentDir := filepath.Join(root, currentKey) + legacyDir := filepath.Join(root, "legacy-repo") + if err := os.MkdirAll(currentDir, 0755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(legacyDir, 0755); err != nil { + t.Fatal(err) + } + writeJSONL(t, filepath.Join(currentDir, "current.jsonl"), + sessionStartLine(t, "2026-08-02T00:00:00Z", repoDir)) + writeJSONL(t, filepath.Join(legacyDir, "legacy.jsonl"), + sessionStartLine(t, "2026-08-01T00:00:00Z", repoDir)) + + repos, err := DiscoverRepos(root) + if err != nil { + t.Fatal(err) + } + if len(repos) != 1 || repos[0].EncodedPath != currentKey || repos[0].SessionCount != 2 { + t.Fatalf("repositories = %+v, want one merged repository with two sessions", repos) + } + + sessions, err := ListSessions(root, currentKey) + if err != nil { + t.Fatal(err) + } + if len(sessions) != 2 || sessions[0].SessionID != "current" || sessions[1].SessionID != "legacy" { + t.Fatalf("sessions = %+v, want current and legacy in timestamp order", sessions) + } + + loaded, err := LoadSession(root, currentKey, "legacy") + if err != nil { + t.Fatalf("LoadSession legacy fallback: %v", err) + } + if loaded.Summary.SessionID != "legacy" || loaded.Summary.CWD != repoDir { + t.Fatalf("loaded legacy session = %+v", loaded.Summary) + } +} + func TestDiscoverRepos_SkipsDirsWithNoJSONL(t *testing.T) { root := t.TempDir() emptyRepo := filepath.Join(root, "empty-repo")