feat(session): add 'ocr session rm' to delete a saved session (#1490)

Sessions accumulate in ~/.opencodereview/sessions/ with no way to remove one
short of deleting files by hand. The Viewer stays read-only by design, so
deletion belongs in the CLI, where the caller already owns the files.

The session id is enough on its own: the command looks for it under every
repository, so it works from anywhere. That a session is stored under a
directory derived from its repository is an implementation detail, and making
the caller stand in the right repository to delete by id pushes that detail
onto them. --repo narrows the search rather than enabling it.

An id saved for more than one repository lists the candidates and deletes
nothing. encodeRepoPath replaces separators with "-", so two repository paths
can share one sessions directory and one id can name two different files;
choosing between them would be a coin flip on a file that cannot be recovered.

The command prints the session's repository, branch, start time, file count
and comment count in the same format `ocr session list` uses, then asks to
confirm. A non-interactive stdin answers no, so `--yes` has to be passed
deliberately from a pipeline or CI.

Reports an unreadable session rather than folding it into "corrupt" -- a
session whose metadata cannot be parsed is still deletable, since that is the
record most likely to need removing. Documented in all five locales of the CLI
reference.

Co-authored-by: Qiyuanqiii <267806965+Qiyuanqiii@users.noreply.github.com>
Co-authored-by: wu21-web <wu2196674@icloud.com>
This commit is contained in:
BASIL K AJI
2026-09-23 20:14:06 +08:00
committed by GitHub
co-authored by Qiyuanqiii wu21-web
parent 5f8e5ab328
commit dc2beb56fa
11 changed files with 1530 additions and 25 deletions
@@ -10,11 +10,6 @@ import (
"testing"
)
// TestCLIReferenceDocumentsSessionCompare pins that every locale of the CLI
// reference documents `ocr session compare`. The four files are hand-synced
// (see PR #920), so the usual failure is a new command landing in `en` only.
// ponytail: substring checks, not a markdown parse - the whole point is to
// catch a missing file, and a parser would not catch it any better.
func TestCLIReferenceDocumentsSessionCompare(t *testing.T) {
for _, locale := range []string{"en", "zh", "ja", "ru"} {
t.Run(locale, func(t *testing.T) {
@@ -37,14 +32,6 @@ func TestCLIReferenceDocumentsSessionCompare(t *testing.T) {
}
}
// TestCLIReferenceDocumentsSessionExport is the sibling guard for
// `ocr session export`. It pins the command surface — the summary row, the
// reference section, the default no-id form and --output — across every
// locale, including `ko`, which the compare pin above predates.
//
// The note that the exported page embeds reviewed source is currently only in
// the English page, so it is not pinned here; translating it is left to the
// locale maintainers.
func TestCLIReferenceDocumentsSessionExport(t *testing.T) {
for _, locale := range []string{"en", "zh", "ja", "ru", "ko"} {
t.Run(locale, func(t *testing.T) {
@@ -67,10 +54,6 @@ func TestCLIReferenceDocumentsSessionExport(t *testing.T) {
}
}
// TestCLIReferenceUsesSubtaskUnit pins that every locale of the CLI reference
// describes --concurrency/--timeout/--max-tools/--max-tokens/--no-filter in
// terms of a subtask, not a file or file group. The five files are
// hand-synced; the usual failure is one locale keeping the old unit.
func TestCLIReferenceUsesSubtaskUnit(t *testing.T) {
type localePin struct {
locale string
@@ -135,3 +118,28 @@ func firstLineContaining(body, marker string) (string, bool) {
}
return "", false
}
func TestCLIReferenceDocumentsSessionRm(t *testing.T) {
for _, locale := range []string{"en", "zh", "ja", "ru", "ko"} {
t.Run(locale, func(t *testing.T) {
path := filepath.Join("..", "..", "pages", "src", "content", "docs", locale, "cli-reference.md")
body, err := os.ReadFile(path)
if err != nil {
t.Fatalf("read %s: %v", path, err)
}
for _, want := range []string{
"`ocr session rm <id>`", // command-summary table row
"### `ocr session rm`", // reference section
"`ocr session delete <id>`", // alias
"ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b --yes", // the flag in use, with an id shaped like a real one
"`--yes`", // the flag itself, however the locale punctuates the row
"`-y`", // and its shorthand
"stdin", // the non-interactive rule
} {
if !strings.Contains(string(body), want) {
t.Errorf("%s: missing %q", path, want)
}
}
})
}
}
+183 -8
View File
@@ -4,10 +4,12 @@
package main
import (
"bufio"
"encoding/json"
"errors"
"fmt"
"io"
"io/fs"
"os"
"path/filepath"
"strings"
@@ -83,9 +85,6 @@ var sessionCompareCmd = &cobra.Command{
Short: "Compare the findings of two sessions",
Long: "Group the findings of two review sessions into new, persisting, resolved and not-reviewed.\nFindings are matched on path, category and the offending snippet, so a finding that only moved down the file still counts as persisting.\nUse --json for machine-readable output.",
Args: exactArgs(2),
// Both positionals are session ids, so the first one already typed must not
// stop completion of the second - hence the args reset rather than plain
// completeSessionIDs, which stops after one argument.
ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
if len(args) > 1 {
return nil, cobra.ShellCompDirectiveNoFileComp
@@ -122,10 +121,56 @@ var sessionExportCmd = &cobra.Command{
},
}
var sessionRmRepoDir string
var sessionRmYes bool
var sessionRmCmd = &cobra.Command{
Use: "rm [flags] <session-id>",
Aliases: []string{"delete", "remove"},
Short: "Delete one saved review session",
Long: "Delete a persisted review session from ~/.opencodereview/sessions/.\n\n" +
"The id is enough on its own, so this works from anywhere; --repo only narrows\n" +
"the search. An id saved for more than one repository is listed, not guessed at.\n\n" +
"The file cannot be recovered, so this confirms first unless --yes is given.",
Example: " ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b\n" +
" ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b --yes\n" +
" ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b --repo ~/work/my-project",
Args: exactArgs(1),
ValidArgsFunction: completeSessionIDsAnywhere,
RunE: func(cmd *cobra.Command, args []string) error {
return runSessionRm(cmd, args[0])
},
}
// completeSessionIDs offers the persisted session ids for the current repo
// (or --repo, when already typed) as shell completions, newest first, with a
// short summary as the completion description. It completes one positional;
// a command with two session ids (compare) resets args per positional.
func completeSessionIDsAnywhere(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
if len(args) != 0 {
return nil, cobra.ShellCompDirectiveNoFileComp
}
if repo, _ := cmd.Flags().GetString("repo"); strings.TrimSpace(repo) != "" {
return completeSessionIDs(cmd, args, toComplete)
}
locations, err := session.ListAllSessionIDs()
if err != nil {
return nil, cobra.ShellCompDirectiveNoFileComp
}
completions := make([]string, 0, len(locations))
for _, loc := range locations {
if !strings.HasPrefix(loc.SessionID, toComplete) {
continue
}
repo := loc.RepoDir
if repo == "" {
repo = "(repository not recorded)"
}
completions = append(completions, loc.SessionID+"\t"+repo)
}
return completions, cobra.ShellCompDirectiveNoFileComp
}
func completeSessionIDs(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
if len(args) != 0 {
return nil, cobra.ShellCompDirectiveNoFileComp
@@ -171,11 +216,15 @@ func init() {
sessionExportCmd.Flags().StringVar(&sessionExportRepoDir, "repo", "", "root directory of the git repository (default: current dir)")
addOutputPathFlag(sessionExportCmd, &sessionExportOutput)
sessionRmCmd.Flags().StringVar(&sessionRmRepoDir, "repo", "", "only look for the session under this repository (default: every repository)")
sessionRmCmd.Flags().BoolVarP(&sessionRmYes, "yes", "y", false, "delete without asking for confirmation")
sessionCmd.AddCommand(sessionListCmd)
sessionCmd.AddCommand(sessionShowCmd)
sessionCmd.AddCommand(sessionCommentsCmd)
sessionCmd.AddCommand(sessionCompareCmd)
sessionCmd.AddCommand(sessionExportCmd)
sessionCmd.AddCommand(sessionRmCmd)
}
// runSessionExport writes one session's standalone HTML page to --output, or to
@@ -196,7 +245,6 @@ func runSessionExport(sessionID string) (retErr error) {
if len(summaries) == 0 {
return fmt.Errorf("no sessions found for %s; run a review first, or pass a session id", resolvedRepo)
}
// ListSessions sorts StartTime descending, so [0] is the newest.
sessionID = summaries[0].SessionID
}
@@ -317,15 +365,11 @@ func runSessionCompare(beforeID, afterID string) error {
if err != nil {
return fmt.Errorf("load session %q: %w", afterID, err)
}
// Comparing findings across repositories is meaningless, so it is an error
// rather than a warning. A different review mode or range still compares
// usefully (a full scan against a diff run, say), so that only warns.
if beforeSummary.RepoDir != afterSummary.RepoDir {
return fmt.Errorf("sessions belong to different repositories: %s was recorded in %s, %s in %s",
beforeID, beforeSummary.RepoDir, afterID, afterSummary.RepoDir)
}
if beforeSummary.ReviewMode != afterSummary.ReviewMode {
// stderr, never stdout: --json output is piped into other tools.
fmt.Fprintf(os.Stderr, "[ocr] WARNING review modes differ (%s vs %s); the two runs may not have looked at the same files\n",
displayMode(beforeSummary.ReviewMode), displayMode(afterSummary.ReviewMode))
}
@@ -440,6 +484,137 @@ func parseFilterSet(s string) map[string]bool {
return set
}
// runSessionRm deletes one persisted session, confirming first unless --yes.
func runSessionRmAnywhere(cmd *cobra.Command, sessionID string) error {
found, err := session.FindSessionsByID(sessionID)
if err != nil {
return err
}
switch len(found) {
case 0:
return fmt.Errorf("no session %q is saved for any repository; run 'ocr session list' to see what is saved", sessionID)
case 1:
default:
var b strings.Builder
fmt.Fprintf(&b, "session %q is saved for more than one repository:\n", sessionID)
for _, loc := range found {
repo := loc.RepoDir
if repo == "" {
repo = "(repository not recorded)"
}
fmt.Fprintf(&b, " %s\n", repo)
}
b.WriteString("\nRe-run with --repo to say which one to delete from.")
return errors.New(b.String())
}
loc := found[0]
summary, summaryErr := session.LoadSummaryAt(loc.Path, sessionID, loc.RepoDir)
if summaryErr != nil {
return fmt.Errorf("cannot read session %q: %w", sessionID, summaryErr)
}
readable := summary != nil && !summary.StartTime.IsZero()
if !sessionRmYes {
if readable {
fmt.Fprintf(cmd.OutOrStdout(), "Delete session %s?\n repo: %s\n branch: %s\n started: %s\n files: %s\n comments: %d\n",
sessionID, summary.RepoDir, summary.GitBranch, describeStart(*summary), describeFiles(*summary), summary.TotalComments)
} else {
fmt.Fprintf(cmd.OutOrStdout(), "Delete session %s? (its metadata could not be read)\n", sessionID)
}
ok, err := confirmDeletion(cmd)
if err != nil {
return err
}
if !ok {
fmt.Fprintln(cmd.OutOrStdout(), "Cancelled.")
return nil
}
}
if err := session.DeleteSessionAt(loc); err != nil {
if errors.Is(err, session.ErrSessionNotFound) {
return fmt.Errorf("no session %q is saved for any repository; run 'ocr session list' to see what is saved", sessionID)
}
return err
}
fmt.Fprintf(cmd.OutOrStdout(), "Deleted session %s\n", sessionID)
return nil
}
func runSessionRm(cmd *cobra.Command, sessionID string) error {
if err := session.ValidateSessionID(sessionID); err != nil {
return err
}
if strings.TrimSpace(sessionRmRepoDir) == "" {
return runSessionRmAnywhere(cmd, sessionID)
}
repoDir, err := resolveWorkingDirForSession(sessionRmRepoDir)
if err != nil {
return err
}
summary, summaryErr := session.LoadSummary(repoDir, sessionID)
switch {
case summaryErr != nil && errors.Is(summaryErr, fs.ErrNotExist):
return fmt.Errorf("no session %q for this repository; run 'ocr session list' to see what is saved", sessionID)
case summaryErr != nil:
return fmt.Errorf("cannot read session %q: %w", sessionID, summaryErr)
}
if err := session.CheckSessionRepo(repoDir, sessionID); err != nil {
if errors.Is(err, session.ErrSessionUnverifiable) {
return fmt.Errorf("%w\n\nRun 'ocr session rm %s' without --repo: the id locates the file on\nits own, so nothing has to be verified against a repository.", err, sessionID)
}
if errors.Is(err, session.ErrSessionOtherRepo) {
return fmt.Errorf("%w\n\nTwo repository paths can share one session directory, so this session is\nvisible here but belongs elsewhere. Delete it from the repository that\nrecorded it.", err)
}
return err
}
readable := summary != nil && !summary.StartTime.IsZero()
if !sessionRmYes {
if readable {
fmt.Fprintf(cmd.OutOrStdout(), "Delete session %s?\n repo: %s\n branch: %s\n started: %s\n files: %s\n comments: %d\n",
sessionID, summary.RepoDir, summary.GitBranch, describeStart(*summary), describeFiles(*summary), summary.TotalComments)
} else {
fmt.Fprintf(cmd.OutOrStdout(), "Delete session %s? (its metadata could not be read)\n", sessionID)
}
ok, err := confirmDeletion(cmd)
if err != nil {
return err
}
if !ok {
fmt.Fprintln(cmd.OutOrStdout(), "Cancelled.")
return nil
}
}
if err := session.DeleteSession(repoDir, sessionID); err != nil {
if errors.Is(err, session.ErrSessionNotFound) {
return fmt.Errorf("no session %q for this repository; run 'ocr session list' to see what is saved", sessionID)
}
return err
}
fmt.Fprintf(cmd.OutOrStdout(), "Deleted session %s\n", sessionID)
return nil
}
// confirmDeletion reads a yes/no answer; a non-interactive stdin answers no.
func confirmDeletion(cmd *cobra.Command) (bool, error) {
fmt.Fprint(cmd.OutOrStdout(), "Type 'y' to confirm: ")
reader := bufio.NewReader(cmd.InOrStdin())
line, err := reader.ReadString('\n')
if err != nil && !errors.Is(err, io.EOF) {
return false, fmt.Errorf("read confirmation: %w", err)
}
answer := strings.ToLower(strings.TrimSpace(line))
return answer == "y" || answer == "yes", nil
}
// resolveWorkingDirForSession accepts an explicit --repo flag value and falls
// back to the current working directory. Unlike resolveRepoDir it does not
// require the target to be a git repository, so users can inspect sessions
+536
View File
@@ -0,0 +1,536 @@
// SPDX-License-Identifier: Apache-2.0
// Copyright 2026 alibaba/open-code-review Contributors
package main
import (
"bytes"
"errors"
"os"
"path/filepath"
"strings"
"testing"
"github.com/spf13/cobra"
"github.com/alibaba/open-code-review/internal/session"
)
func seedRmSession(t *testing.T, _ /*home*/, repoDir, sessionID string) string {
t.Helper()
dir, err := session.SessionsDir(repoDir)
if err != nil {
t.Fatalf("SessionsDir: %v", err)
}
if err := os.MkdirAll(dir, 0o700); err != nil {
t.Fatalf("MkdirAll: %v", err)
}
path := filepath.Join(dir, sessionID+".jsonl")
if err := os.WriteFile(path, []byte("{}\n"), 0o600); err != nil {
t.Fatalf("WriteFile: %v", err)
}
return path
}
func TestSessionRm_DeclinedConfirmationKeepsTheSession(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
repoDir := t.TempDir()
path := seedRmSessionRecorded(t, repoDir, "20250601-100000-aaaaaa", "main")
sessionRmRepoDir = repoDir
sessionRmYes = false
t.Cleanup(func() { sessionRmRepoDir, sessionRmYes = "", false })
out := &bytes.Buffer{}
sessionRmCmd.SetOut(out)
sessionRmCmd.SetIn(strings.NewReader("n\n"))
if err := runSessionRm(sessionRmCmd, "20250601-100000-aaaaaa"); err != nil {
t.Fatalf("runSessionRm: %v", err)
}
if _, err := os.Stat(path); err != nil {
t.Errorf("session was deleted despite a declined confirmation: %v", err)
}
if !strings.Contains(out.String(), "Cancelled") {
t.Errorf("expected the command to say it cancelled, got %q", out.String())
}
}
func TestSessionRm_NonInteractiveStdinDoesNotDelete(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
repoDir := t.TempDir()
path := seedRmSessionRecorded(t, repoDir, "20250601-100000-aaaaaa", "main")
sessionRmRepoDir = repoDir
sessionRmYes = false
t.Cleanup(func() { sessionRmRepoDir, sessionRmYes = "", false })
sessionRmCmd.SetOut(&bytes.Buffer{})
sessionRmCmd.SetIn(strings.NewReader(""))
if err := runSessionRm(sessionRmCmd, "20250601-100000-aaaaaa"); err != nil {
t.Fatalf("runSessionRm: %v", err)
}
if _, err := os.Stat(path); err != nil {
t.Errorf("session was deleted on an empty stdin: %v", err)
}
}
func TestSessionRm_YesFlagDeletesWithoutPrompting(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
repoDir := t.TempDir()
path := seedRmSessionRecorded(t, repoDir, "20250601-100000-aaaaaa", "main")
sessionRmRepoDir = repoDir
sessionRmYes = true
t.Cleanup(func() { sessionRmRepoDir, sessionRmYes = "", false })
out := &bytes.Buffer{}
sessionRmCmd.SetOut(out)
sessionRmCmd.SetIn(strings.NewReader(""))
if err := runSessionRm(sessionRmCmd, "20250601-100000-aaaaaa"); err != nil {
t.Fatalf("runSessionRm: %v", err)
}
if _, err := os.Stat(path); !os.IsNotExist(err) {
t.Errorf("session still present after --yes: %v", err)
}
if !strings.Contains(out.String(), "Deleted session") {
t.Errorf("expected a confirmation line, got %q", out.String())
}
}
func TestSessionRm_UnknownIDExplainsHowToList(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
repoDir := t.TempDir()
sessionRmRepoDir = repoDir
sessionRmYes = true
t.Cleanup(func() { sessionRmRepoDir, sessionRmYes = "", false })
sessionRmCmd.SetOut(&bytes.Buffer{})
sessionRmCmd.SetIn(strings.NewReader(""))
err := runSessionRm(sessionRmCmd, "20250601-999999-zzzzzz")
if err == nil {
t.Fatal("expected an error for an unknown session id")
}
if !strings.Contains(err.Error(), "ocr session list") {
t.Errorf("error should point at 'ocr session list', got %q", err.Error())
}
}
func TestSessionRm_RejectsTraversalIDsBeforeTouchingTheFilesystem(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
repoDir := t.TempDir()
sessionRmRepoDir = repoDir
sessionRmYes = true
t.Cleanup(func() { sessionRmRepoDir, sessionRmYes = "", false })
out := &bytes.Buffer{}
sessionRmCmd.SetOut(out)
sessionRmCmd.SetIn(strings.NewReader(""))
for _, id := range []string{"../escape", "../../escape", "a/b", "..", "."} {
t.Run(id, func(t *testing.T) {
err := runSessionRm(sessionRmCmd, id)
if err == nil {
t.Fatalf("runSessionRm(%q) returned no error", id)
}
if strings.Contains(err.Error(), "ocr session list") {
t.Errorf("runSessionRm(%q) reported a missing session; an invalid id must be refused as invalid", id)
}
})
}
}
func TestSessionRm_CorruptSessionIsDeletableAndDescribedHonestly(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
repoDir := t.TempDir()
dir, err := session.SessionsDir(repoDir)
if err != nil {
t.Fatalf("SessionsDir: %v", err)
}
if err := os.MkdirAll(dir, 0o700); err != nil {
t.Fatalf("MkdirAll: %v", err)
}
path := filepath.Join(dir, "20250601-100000-corrupt.jsonl")
if err := os.WriteFile(path, []byte("this is not json\n{\"also\": not json\n"), 0o600); err != nil {
t.Fatalf("WriteFile: %v", err)
}
sessionRmRepoDir = ""
sessionRmYes = false
t.Cleanup(func() { sessionRmRepoDir, sessionRmYes = "", false })
out := &bytes.Buffer{}
sessionRmCmd.SetOut(out)
sessionRmCmd.SetIn(strings.NewReader("y\n"))
if err := runSessionRm(sessionRmCmd, "20250601-100000-corrupt"); err != nil {
t.Fatalf("runSessionRm: %v", err)
}
if !strings.Contains(out.String(), "could not be read") {
t.Errorf("a corrupt session should be described as unreadable, got %q", out.String())
}
if strings.Contains(out.String(), "started: 0001-01-01") {
t.Errorf("zero-value metadata was printed as if real: %q", out.String())
}
if _, err := os.Stat(path); !os.IsNotExist(err) {
t.Errorf("corrupt session was not deleted: %v", err)
}
}
func TestSessionRm_OtherRepoRefusedBeforePrompting(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
base := t.TempDir()
repoA := filepath.Join(base, "a-b", "c")
repoB := filepath.Join(base, "a", "b-c")
for _, d := range []string{repoA, repoB} {
if err := os.MkdirAll(d, 0o755); err != nil {
t.Fatalf("MkdirAll: %v", err)
}
}
dirA, _ := session.SessionsDir(repoA)
dirB, _ := session.SessionsDir(repoB)
if dirA != dirB {
t.Skip("paths no longer collide; the encoding may have been fixed")
}
if err := os.MkdirAll(dirB, 0o700); err != nil {
t.Fatalf("MkdirAll: %v", err)
}
path := filepath.Join(dirB, "20250601-100000-bbbbbb.jsonl")
rec := `{"type":"session_start","cwd":"` + strings.ReplaceAll(repoB, `\`, `\\`) + `","sessionId":"20250601-100000-bbbbbb","timestamp":"2025-06-01T10:00:00Z"}` + "\n"
if err := os.WriteFile(path, []byte(rec), 0o600); err != nil {
t.Fatalf("WriteFile: %v", err)
}
sessionRmRepoDir = repoA
sessionRmYes = false
t.Cleanup(func() { sessionRmRepoDir, sessionRmYes = "", false })
out := &bytes.Buffer{}
sessionRmCmd.SetOut(out)
sessionRmCmd.SetIn(strings.NewReader("y\n"))
err := runSessionRm(sessionRmCmd, "20250601-100000-bbbbbb")
if err == nil {
t.Fatal("deleting another repository's session must fail")
}
if !strings.Contains(err.Error(), "different repository") {
t.Errorf("error should name the cause, got %q", err.Error())
}
if strings.Contains(out.String(), "Delete session") {
t.Errorf("the user was prompted before the refusal: %q", out.String())
}
if _, statErr := os.Stat(path); statErr != nil {
t.Errorf("the other repository's session was deleted: %v", statErr)
}
}
func makeUnreadable(t *testing.T, path string) {
t.Helper()
if err := os.Chmod(path, 0o000); err != nil {
t.Fatalf("Chmod: %v", err)
}
t.Cleanup(func() { _ = os.Chmod(path, 0o600) })
if _, err := os.ReadFile(path); err == nil {
t.Skipf("this platform does not enforce the file mode: %s is still readable after chmod 000", path)
}
}
func TestSessionRm_UnreadableSessionIsReportedNotTreatedAsCorrupt(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
repoDir := t.TempDir()
path := seedRmSession(t, home, repoDir, "20250601-100000-locked")
makeUnreadable(t, path)
sessionRmRepoDir = repoDir
sessionRmYes = true
t.Cleanup(func() { sessionRmRepoDir, sessionRmYes = "", false })
sessionRmCmd.SetOut(&bytes.Buffer{})
sessionRmCmd.SetIn(strings.NewReader(""))
err := runSessionRm(sessionRmCmd, "20250601-100000-locked")
if err == nil {
t.Fatal("an unreadable session must be reported, not deleted")
}
if !strings.Contains(err.Error(), "cannot read session") {
t.Errorf("error should say the file could not be read, got %q", err.Error())
}
if _, statErr := os.Stat(path); statErr != nil {
t.Errorf("an unreadable session was deleted anyway: %v", statErr)
}
}
func TestSessionRm_PromptMatchesTheListFormatting(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
repoDir := t.TempDir()
dir, err := session.SessionsDir(repoDir)
if err != nil {
t.Fatalf("SessionsDir: %v", err)
}
if err := os.MkdirAll(dir, 0o700); err != nil {
t.Fatalf("MkdirAll: %v", err)
}
path := filepath.Join(dir, "20250601-100000-fmt.jsonl")
rec := `{"type":"session_start","cwd":"` + strings.ReplaceAll(repoDir, `\`, `\\`) +
`","sessionId":"20250601-100000-fmt","timestamp":"2025-06-01T10:00:00Z","gitBranch":"feature-x"}` + "\n"
if err := os.WriteFile(path, []byte(rec), 0o600); err != nil {
t.Fatalf("WriteFile: %v", err)
}
sessionRmRepoDir = repoDir
sessionRmYes = false
t.Cleanup(func() { sessionRmRepoDir, sessionRmYes = "", false })
out := &bytes.Buffer{}
sessionRmCmd.SetOut(out)
sessionRmCmd.SetIn(strings.NewReader("n\n"))
if err := runSessionRm(sessionRmCmd, "20250601-100000-fmt"); err != nil {
t.Fatalf("runSessionRm: %v", err)
}
got := out.String()
if strings.Contains(got, "T10:00:00Z") {
t.Errorf("prompt used RFC3339; 'ocr session list' shows a local timestamp: %q", got)
}
for _, want := range []string{"repo:", "branch:", "started:", "files:", "comments:"} {
if !strings.Contains(got, want) {
t.Errorf("prompt is missing %q: %q", want, got)
}
}
}
func seedRmSessionRecorded(t *testing.T, repoDir, sessionID, branch string) string {
t.Helper()
path := seedRmSession(t, "", repoDir, sessionID)
rec := `{"type":"session_start","cwd":"` + strings.ReplaceAll(repoDir, `\`, `\\`) +
`","sessionId":"` + sessionID + `","timestamp":"2025-06-01T10:00:00Z","gitBranch":"` + branch + `"}` + "\n"
if err := os.WriteFile(path, []byte(rec), 0o600); err != nil {
t.Fatalf("WriteFile: %v", err)
}
return path
}
func runSessionRmForTest(t *testing.T, repoFlag, sessionID, stdin string, yes bool) (string, error) {
t.Helper()
sessionRmRepoDir = repoFlag
sessionRmYes = yes
t.Cleanup(func() { sessionRmRepoDir, sessionRmYes = "", false })
var out bytes.Buffer
sessionRmCmd.SetOut(&out)
sessionRmCmd.SetIn(strings.NewReader(stdin))
err := runSessionRm(sessionRmCmd, sessionID)
return out.String(), err
}
func TestSessionRm_FindsTheSessionWithoutBeingToldTheRepository(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
repoDir := t.TempDir()
path := seedRmSessionRecorded(t, repoDir, "20250601-100000-alone", "main")
out, err := runSessionRmForTest(t, "", "20250601-100000-alone", "y\n", false)
if err != nil {
t.Fatalf("runSessionRm: %v", err)
}
if !strings.Contains(out, repoDir) {
t.Errorf("the prompt must name the repository the session was found in, got %q", out)
}
if _, statErr := os.Stat(path); !os.IsNotExist(statErr) {
t.Errorf("session was not deleted: %v", statErr)
}
}
func TestSessionRm_ListsCandidatesRatherThanGuessing(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
repoA := t.TempDir()
repoB := t.TempDir()
pathA := seedRmSessionRecorded(t, repoA, "20250601-100000-shared", "main")
pathB := seedRmSessionRecorded(t, repoB, "20250601-100000-shared", "feature")
_, err := runSessionRmForTest(t, "", "20250601-100000-shared", "y\n", true)
if err == nil {
t.Fatal("an id saved for two repositories must not be deleted on a guess")
}
for _, want := range []string{repoA, repoB, "--repo"} {
if !strings.Contains(err.Error(), want) {
t.Errorf("error should mention %q so the caller can disambiguate, got %q", want, err.Error())
}
}
for _, path := range []string{pathA, pathB} {
if _, statErr := os.Stat(path); statErr != nil {
t.Errorf("an ambiguous id deleted %s anyway: %v", path, statErr)
}
}
}
func TestSessionRm_RepoFlagNarrowsAnAmbiguousID(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
repoA := t.TempDir()
repoB := t.TempDir()
pathA := seedRmSessionRecorded(t, repoA, "20250601-100000-shared", "main")
pathB := seedRmSessionRecorded(t, repoB, "20250601-100000-shared", "feature")
if _, err := runSessionRmForTest(t, repoB, "20250601-100000-shared", "", true); err != nil {
t.Fatalf("runSessionRm --repo: %v", err)
}
if _, statErr := os.Stat(pathB); !os.IsNotExist(statErr) {
t.Errorf("the named repository's session was not deleted: %v", statErr)
}
if _, statErr := os.Stat(pathA); statErr != nil {
t.Errorf("the other repository's session was deleted too: %v", statErr)
}
}
func TestSessionRm_UnknownIDAnywhereSaysSoWithoutPrompting(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
seedRmSessionRecorded(t, t.TempDir(), "20250601-100000-present", "main")
out, err := runSessionRmForTest(t, "", "20250601-100000-missing", "y\n", false)
if err == nil {
t.Fatal("an id that is saved nowhere must be an error")
}
if !strings.Contains(err.Error(), "ocr session list") {
t.Errorf("error should point at the listing command, got %q", err.Error())
}
if strings.Contains(out, "Delete session") {
t.Errorf("the caller must not be asked to confirm deleting something that is not there, got %q", out)
}
}
func TestSessionRm_TraversalIDIsRefusedBeforeSearchingEveryRepository(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
for _, id := range []string{"../escape", "sub/../../escape", `..\escape`, ".", ".."} {
if _, err := session.FindSessionsByID(id); err == nil {
t.Errorf("FindSessionsByID(%q) must be refused", id)
}
if _, err := runSessionRmForTest(t, "", id, "", true); err == nil {
t.Errorf("ocr session rm %q must be refused", id)
}
}
}
func TestSessionRm_RepoFlagRefusesAnUnverifiableSession(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
repoDir := t.TempDir()
path := seedRmSession(t, home, repoDir, "20250601-100000-nocwd")
out, err := runSessionRmForTest(t, repoDir, "20250601-100000-nocwd", "y\n", true)
if err == nil {
t.Fatal("a session that records no repository must not be deleted on the strength of --repo")
}
if !errors.Is(err, session.ErrSessionUnverifiable) {
t.Errorf("error should be ErrSessionUnverifiable, got %v", err)
}
if !strings.Contains(err.Error(), "without --repo") {
t.Errorf("error should say how to delete it instead, got %q", err.Error())
}
if strings.Contains(out, "Delete session") {
t.Errorf("the caller must not be prompted before the refusal, got %q", out)
}
if _, statErr := os.Stat(path); statErr != nil {
t.Errorf("the session was deleted anyway: %v", statErr)
}
}
func TestSessionRm_UnverifiableSessionIsStillDeletableByIDAlone(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
repoDir := t.TempDir()
path := seedRmSession(t, home, repoDir, "20250601-100000-nocwd")
if _, err := runSessionRmForTest(t, "", "20250601-100000-nocwd", "", true); err != nil {
t.Fatalf("runSessionRm without --repo: %v", err)
}
if _, statErr := os.Stat(path); !os.IsNotExist(statErr) {
t.Errorf("the session was not deleted: %v", statErr)
}
}
func TestSessionRm_CompletionOffersSessionsFromEveryRepository(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
here := t.TempDir()
elsewhere := t.TempDir()
seedRmSessionRecorded(t, here, "20250601-100000-here", "main")
seedRmSessionRecorded(t, elsewhere, "20250601-100000-away", "main")
sessionRmRepoDir = ""
t.Cleanup(func() { sessionRmRepoDir = "" })
if err := sessionRmCmd.Flags().Set("repo", ""); err != nil {
t.Fatalf("clear --repo: %v", err)
}
got, _ := completeSessionIDsAnywhere(sessionRmCmd, nil, "20250601")
joined := strings.Join(got, "\n")
for _, want := range []string{"20250601-100000-here", "20250601-100000-away"} {
if !strings.Contains(joined, want) {
t.Errorf("completion is missing %q; got %q", want, joined)
}
}
if !strings.Contains(joined, elsewhere) {
t.Errorf("completion should name the repository each id belongs to; got %q", joined)
}
}
func TestSessionRm_CompletionNarrowsWithRepoFlag(t *testing.T) {
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
here := t.TempDir()
elsewhere := t.TempDir()
seedRmSessionRecorded(t, here, "20250601-100000-here", "main")
seedRmSessionRecorded(t, elsewhere, "20250601-100000-away", "main")
if err := sessionRmCmd.Flags().Set("repo", here); err != nil {
t.Fatalf("set --repo: %v", err)
}
t.Cleanup(func() { _ = sessionRmCmd.Flags().Set("repo", ""); sessionRmRepoDir = "" })
joined := strings.Join(firstOf(completeSessionIDsAnywhere(sessionRmCmd, nil, "20250601")), "\n")
if !strings.Contains(joined, "20250601-100000-here") {
t.Errorf("completion lost this repository's session; got %q", joined)
}
if strings.Contains(joined, "20250601-100000-away") {
t.Errorf("--repo should scope the completion; got %q", joined)
}
}
func firstOf(completions []string, _ cobra.ShellCompDirective) []string { return completions }
+275
View File
@@ -0,0 +1,275 @@
// SPDX-License-Identifier: Apache-2.0
// Copyright 2026 alibaba/open-code-review Contributors
package session
import (
"errors"
"fmt"
"os"
"path/filepath"
"sort"
"strings"
)
// ErrSessionNotFound reports that no persisted session matched the id.
var ErrSessionNotFound = errors.New("session not found")
// ErrSessionOtherRepo reports that a session records a different repository.
var ErrSessionOtherRepo = errors.New("session belongs to a different repository")
// ErrSessionOutsideRoot reports that a Location falls outside the sessions directory.
var ErrSessionOutsideRoot = errors.New("session path is outside the sessions directory")
// ErrSessionUnverifiable reports that a session records no repository to check --repo against.
var ErrSessionUnverifiable = errors.New("session records no repository")
// ValidateSessionID rejects ids that are not a single path element.
func ValidateSessionID(sessionID string) error {
if strings.TrimSpace(sessionID) == "" {
return fmt.Errorf("session id is required")
}
if sessionID != filepath.Base(sessionID) ||
strings.ContainsAny(sessionID, `/\`) ||
sessionID == "." || sessionID == ".." {
return fmt.Errorf("invalid session id %q: must not contain a path", sessionID)
}
return nil
}
// CheckSessionRepo reports whether a session may be operated on from repoDir.
func CheckSessionRepo(repoDir, sessionID string) error {
if err := ValidateSessionID(sessionID); err != nil {
return err
}
path, err := sessionPath(repoDir, sessionID)
if err != nil {
return err
}
if _, err := os.Stat(path); err != nil {
if os.IsNotExist(err) {
return fmt.Errorf("%w: %s", ErrSessionNotFound, sessionID)
}
return fmt.Errorf("stat session %q: %w", sessionID, err)
}
recorded, ok := recordedRepoDir(path)
if !ok {
return fmt.Errorf(
"%w: %s records no repository, so it cannot be verified as this one; delete it by id alone, without --repo",
ErrSessionUnverifiable, sessionID,
)
}
if !sameRepoPath(recorded, repoDir) {
return fmt.Errorf("%w: %s was recorded for %s", ErrSessionOtherRepo, sessionID, recorded)
}
return nil
}
// sessionPath resolves a validated session id to its file inside the sessions directory.
func sessionPath(repoDir, sessionID string) (string, error) {
dir, err := SessionsDir(repoDir)
if err != nil {
return "", err
}
path := filepath.Join(dir, sessionID+".jsonl")
if filepath.Dir(path) != filepath.Clean(dir) {
return "", fmt.Errorf("invalid session id %q: resolves outside the sessions directory", sessionID)
}
return path, nil
}
// DeleteSession removes one persisted session for a repository.
func DeleteSession(repoDir, sessionID string) error {
if err := CheckSessionRepo(repoDir, sessionID); err != nil {
return err
}
path, err := sessionPath(repoDir, sessionID)
if err != nil {
return err
}
if err := os.Remove(path); err != nil {
return fmt.Errorf("delete session %q: %w", sessionID, err)
}
return nil
}
// recordedRepoDir returns the working directory a session recorded, and whether it had one.
func recordedRepoDir(path string) (string, bool) {
var cwd string
if err := walkSessionFile(path, func(rec summaryRecord) {
if rec.Cwd != "" {
cwd = rec.Cwd
}
}); err != nil {
return "", false
}
if cwd == "" {
return "", false
}
return cwd, true
}
// sameRepoPath reports whether two repository paths name the same directory.
func sameRepoPath(a, b string) bool {
ca, cb := filepath.Clean(a), filepath.Clean(b)
if ca == cb {
return true
}
fa, errA := os.Stat(ca)
fb, errB := os.Stat(cb)
if errA != nil || errB != nil {
return false
}
return os.SameFile(fa, fb)
}
// Location is one on-disk session file, with the repository it recorded.
type Location struct {
SessionID string
Path string
RepoDir string
EncodedDir string
}
// FindSessionsByID returns every session file matching an id, in any repository.
func FindSessionsByID(sessionID string) ([]Location, error) {
if err := ValidateSessionID(sessionID); err != nil {
return nil, err
}
home, err := os.UserHomeDir()
if err != nil {
return nil, fmt.Errorf("resolve home dir: %w", err)
}
root := filepath.Join(home, ".opencodereview", sessionSubDir)
entries, err := os.ReadDir(root)
if err != nil {
if os.IsNotExist(err) {
return nil, nil
}
return nil, fmt.Errorf("read sessions dir %q: %w", root, err)
}
var found []Location
for _, entry := range entries {
if !entry.IsDir() {
continue
}
path := filepath.Join(root, entry.Name(), sessionID+".jsonl")
info, statErr := os.Stat(path)
if statErr != nil {
if os.IsNotExist(statErr) {
continue
}
return nil, fmt.Errorf("stat session %q: %w", path, statErr)
}
if info.IsDir() {
continue
}
repoDir, _ := recordedRepoDir(path)
found = append(found, Location{
SessionID: sessionID,
Path: path,
RepoDir: repoDir,
EncodedDir: entry.Name(),
})
}
sort.Slice(found, func(i, j int) bool {
if found[i].RepoDir != found[j].RepoDir {
return found[i].RepoDir < found[j].RepoDir
}
return found[i].EncodedDir < found[j].EncodedDir
})
return found, nil
}
// sessionsRoot returns the directory that holds every repository's sessions.
func sessionsRoot() (string, error) {
home, err := os.UserHomeDir()
if err != nil {
return "", fmt.Errorf("resolve home dir: %w", err)
}
return filepath.Join(home, ".opencodereview", sessionSubDir), nil
}
// DeleteSessionAt removes one session file found by FindSessionsByID.
func DeleteSessionAt(loc Location) error {
path, err := resolveLocationPath(loc)
if err != nil {
return err
}
if err := os.Remove(path); err != nil {
if os.IsNotExist(err) {
return fmt.Errorf("%w: %s", ErrSessionNotFound, loc.SessionID)
}
return fmt.Errorf("delete session %q: %w", loc.SessionID, err)
}
return nil
}
// resolveLocationPath rebuilds a Location's path from its parts and checks the two agree.
func resolveLocationPath(loc Location) (string, error) {
if err := ValidateSessionID(loc.SessionID); err != nil {
return "", err
}
if loc.EncodedDir == "" ||
loc.EncodedDir != filepath.Base(loc.EncodedDir) ||
strings.ContainsAny(loc.EncodedDir, `/\`) ||
loc.EncodedDir == "." || loc.EncodedDir == ".." {
return "", fmt.Errorf("%w: %q is not a single sessions directory", ErrSessionOutsideRoot, loc.EncodedDir)
}
root, err := sessionsRoot()
if err != nil {
return "", err
}
want := filepath.Join(root, loc.EncodedDir, loc.SessionID+".jsonl")
if filepath.Clean(loc.Path) != want {
return "", fmt.Errorf("%w: %s is not %s", ErrSessionOutsideRoot, loc.Path, want)
}
return want, nil
}
// ListAllSessionIDs returns every saved session id.
func ListAllSessionIDs() ([]Location, error) {
root, err := sessionsRoot()
if err != nil {
return nil, err
}
entries, err := os.ReadDir(root)
if err != nil {
if os.IsNotExist(err) {
return nil, nil
}
return nil, fmt.Errorf("read sessions dir %q: %w", root, err)
}
var found []Location
for _, entry := range entries {
if !entry.IsDir() {
continue
}
files, readErr := os.ReadDir(filepath.Join(root, entry.Name()))
if readErr != nil {
continue
}
for _, f := range files {
name := f.Name()
if f.IsDir() || !strings.HasSuffix(name, ".jsonl") {
continue
}
id := strings.TrimSuffix(name, ".jsonl")
if ValidateSessionID(id) != nil {
continue
}
path := filepath.Join(root, entry.Name(), name)
repoDir, _ := recordedRepoDir(path)
found = append(found, Location{
SessionID: id,
Path: path,
RepoDir: repoDir,
EncodedDir: entry.Name(),
})
}
}
sort.Slice(found, func(i, j int) bool { return found[i].SessionID < found[j].SessionID })
return found, nil
}
+361
View File
@@ -0,0 +1,361 @@
// SPDX-License-Identifier: Apache-2.0
// Copyright 2026 alibaba/open-code-review Contributors
package session
import (
"errors"
"os"
"path/filepath"
"strconv"
"strings"
"testing"
)
func seedSession(t *testing.T, repoDir, sessionID string) string {
t.Helper()
dir, err := SessionsDir(repoDir)
if err != nil {
t.Fatalf("SessionsDir: %v", err)
}
if err := os.MkdirAll(dir, 0o700); err != nil {
t.Fatalf("MkdirAll: %v", err)
}
path := filepath.Join(dir, sessionID+".jsonl")
if err := os.WriteFile(path, []byte("{}\n"), 0o600); err != nil {
t.Fatalf("WriteFile: %v", err)
}
return path
}
func seedAttributedSession(t *testing.T, repoDir, sessionID string) string {
t.Helper()
path := seedSession(t, repoDir, sessionID)
rec := `{"type":"session_start","cwd":"` + strings.ReplaceAll(repoDir, `\`, `\\`) +
`","sessionId":"` + sessionID + `","timestamp":"2025-06-01T10:00:00Z"}` + "\n"
if err := os.WriteFile(path, []byte(rec), 0o600); err != nil {
t.Fatalf("WriteFile: %v", err)
}
return path
}
func TestDeleteSession_RemovesOnlyTheNamedSession(t *testing.T) {
setTestHome(t, t.TempDir())
repoDir := t.TempDir()
target := seedAttributedSession(t, repoDir, "20250601-100000-aaaaaa")
keep := seedAttributedSession(t, repoDir, "20250601-110000-bbbbbb")
if err := DeleteSession(repoDir, "20250601-100000-aaaaaa"); err != nil {
t.Fatalf("DeleteSession: %v", err)
}
if _, err := os.Stat(target); !os.IsNotExist(err) {
t.Errorf("target session still present: %v", err)
}
if _, err := os.Stat(keep); err != nil {
t.Errorf("unrelated session was removed: %v", err)
}
}
func TestDeleteSession_MissingSessionIsReportedNotSilentlyIgnored(t *testing.T) {
setTestHome(t, t.TempDir())
repoDir := t.TempDir()
seedSession(t, repoDir, "20250601-100000-aaaaaa")
err := DeleteSession(repoDir, "20250601-999999-zzzzzz")
if !errors.Is(err, ErrSessionNotFound) {
t.Fatalf("want ErrSessionNotFound, got %v", err)
}
}
func TestDeleteSession_RefusesIdsThatEscapeTheSessionsDirectory(t *testing.T) {
setTestHome(t, t.TempDir())
repoDir := t.TempDir()
dir, err := SessionsDir(repoDir)
if err != nil {
t.Fatalf("SessionsDir: %v", err)
}
if err := os.MkdirAll(dir, 0o700); err != nil {
t.Fatalf("MkdirAll: %v", err)
}
outside := filepath.Join(filepath.Dir(dir), "important.jsonl")
if err := os.WriteFile(outside, []byte("keep me\n"), 0o600); err != nil {
t.Fatalf("WriteFile: %v", err)
}
for _, id := range []string{
"../important",
"../../important",
"..",
".",
"sub/../../important",
`..\important`,
"a/b",
} {
t.Run(id, func(t *testing.T) {
err := DeleteSession(repoDir, id)
if err == nil {
t.Fatalf("DeleteSession(%q) returned no error", id)
}
if errors.Is(err, ErrSessionNotFound) {
t.Errorf("DeleteSession(%q) treated a traversal as a missing session; it must be refused outright", id)
}
})
}
if _, err := os.Stat(outside); err != nil {
t.Fatalf("a file outside the sessions directory was removed: %v", err)
}
}
func TestDeleteSession_RefusesAnEmptyID(t *testing.T) {
setTestHome(t, t.TempDir())
for _, id := range []string{"", " "} {
if err := DeleteSession(t.TempDir(), id); err == nil {
t.Errorf("DeleteSession(%q) returned no error", id)
}
}
}
func TestDeleteSession_RefusesASessionRecordedForAnotherRepo(t *testing.T) {
home := t.TempDir()
setTestHome(t, home)
base := t.TempDir()
repoA := filepath.Join(base, "a-b", "c")
repoB := filepath.Join(base, "a", "b-c")
for _, d := range []string{repoA, repoB} {
if err := os.MkdirAll(d, 0o755); err != nil {
t.Fatalf("MkdirAll: %v", err)
}
}
dirA, err := SessionsDir(repoA)
if err != nil {
t.Fatalf("SessionsDir(A): %v", err)
}
dirB, err := SessionsDir(repoB)
if err != nil {
t.Fatalf("SessionsDir(B): %v", err)
}
if dirA != dirB {
t.Skipf("paths no longer collide (%s vs %s); the encoding may have been fixed", dirA, dirB)
}
if err := os.MkdirAll(dirB, 0o700); err != nil {
t.Fatalf("MkdirAll: %v", err)
}
path := filepath.Join(dirB, "20250601-100000-bbbbbb.jsonl")
rec := `{"type":"session_start","cwd":` + strconv.Quote(repoB) + `,"sessionId":"20250601-100000-bbbbbb"}` + "\n"
if err := os.WriteFile(path, []byte(rec), 0o600); err != nil {
t.Fatalf("WriteFile: %v", err)
}
err = DeleteSession(repoA, "20250601-100000-bbbbbb")
if !errors.Is(err, ErrSessionOtherRepo) {
t.Fatalf("want ErrSessionOtherRepo, got %v", err)
}
if _, statErr := os.Stat(path); statErr != nil {
t.Errorf("another repository's session was deleted: %v", statErr)
}
if err := DeleteSession(repoB, "20250601-100000-bbbbbb"); err != nil {
t.Fatalf("DeleteSession from the owning repo: %v", err)
}
}
func TestDeleteSession_UnattributableSessionIsRefusedAgainstARepository(t *testing.T) {
setTestHome(t, t.TempDir())
repoDir := t.TempDir()
path := seedSession(t, repoDir, "20250601-100000-nocwd")
err := DeleteSession(repoDir, "20250601-100000-nocwd")
if !errors.Is(err, ErrSessionUnverifiable) {
t.Fatalf("DeleteSession error = %v, want ErrSessionUnverifiable", err)
}
if _, statErr := os.Stat(path); statErr != nil {
t.Errorf("session was deleted anyway: %v", statErr)
}
}
func TestDeleteSessionAt_UnattributableSessionIsStillDeletable(t *testing.T) {
setTestHome(t, t.TempDir())
repoDir := t.TempDir()
path := seedSession(t, repoDir, "20250601-100000-nocwd")
found, err := FindSessionsByID("20250601-100000-nocwd")
if err != nil {
t.Fatalf("FindSessionsByID: %v", err)
}
if len(found) != 1 {
t.Fatalf("found %d sessions, want 1", len(found))
}
if err := DeleteSessionAt(found[0]); err != nil {
t.Fatalf("DeleteSessionAt: %v", err)
}
if _, statErr := os.Stat(path); !os.IsNotExist(statErr) {
t.Errorf("session still present: %v", statErr)
}
_ = repoDir
}
func TestSameRepoPath_OneDirectorySpelledTwoWays(t *testing.T) {
base := t.TempDir()
repo := filepath.Join(base, "a-b")
if err := os.MkdirAll(repo, 0o755); err != nil {
t.Fatalf("MkdirAll: %v", err)
}
t.Run("trailing separator and dot segments", func(t *testing.T) {
if !sameRepoPath(repo, repo+string(os.PathSeparator)) {
t.Error("a trailing separator must not read as a different repository")
}
if !sameRepoPath(repo, filepath.Join(repo, ".")) {
t.Error("a dot segment must not read as a different repository")
}
})
t.Run("reached through a symlink", func(t *testing.T) {
link := filepath.Join(base, "link-to-a-b")
if err := os.Symlink(repo, link); err != nil {
t.Skipf("symlinks unavailable: %v", err)
}
if !sameRepoPath(repo, link) {
t.Error("a symlink to the same directory must compare equal")
}
})
t.Run("case variation", func(t *testing.T) {
upper := filepath.Join(base, "A-B")
if _, err := os.Stat(upper); err != nil {
t.Skip("case-sensitive filesystem: A-B and a-b are different directories here")
}
if !sameRepoPath(repo, upper) {
t.Error("on a case-insensitive filesystem the two spellings are one directory")
}
})
t.Run("genuinely different directories stay distinct", func(t *testing.T) {
other := filepath.Join(base, "c-d")
if err := os.MkdirAll(other, 0o755); err != nil {
t.Fatalf("MkdirAll: %v", err)
}
if sameRepoPath(repo, other) {
t.Error("two different directories must not compare equal")
}
})
t.Run("a vanished directory falls back to exact spelling", func(t *testing.T) {
gone := filepath.Join(base, "removed")
if !sameRepoPath(gone, gone) {
t.Error("an identical path must match even when it no longer exists")
}
if sameRepoPath(gone, filepath.Join(base, "removed-other")) {
t.Error("without a filesystem to ask, differing paths must not match")
}
})
}
func TestDeleteSessionAt_RefusesPathOutsideSessionsRoot(t *testing.T) {
setTestHome(t, t.TempDir())
outside := filepath.Join(t.TempDir(), "victim.jsonl")
if err := os.WriteFile(outside, []byte("not a session\n"), 0o600); err != nil {
t.Fatalf("WriteFile: %v", err)
}
err := DeleteSessionAt(Location{SessionID: "victim", Path: outside, EncodedDir: "anything"})
if !errors.Is(err, ErrSessionOutsideRoot) {
t.Fatalf("DeleteSessionAt error = %v, want ErrSessionOutsideRoot", err)
}
if _, statErr := os.Stat(outside); statErr != nil {
t.Errorf("a file outside the sessions root was deleted: %v", statErr)
}
}
func TestDeleteSessionAt_RefusesATraversingEncodedDir(t *testing.T) {
home := t.TempDir()
setTestHome(t, home)
outside := filepath.Join(home, "important.jsonl")
if err := os.WriteFile(outside, []byte("keep me\n"), 0o600); err != nil {
t.Fatalf("WriteFile: %v", err)
}
for _, encoded := range []string{"..", ".", "../..", "a/b", `a\b`, ""} {
loc := Location{
SessionID: "important",
EncodedDir: encoded,
Path: filepath.Join(home, ".opencodereview", sessionSubDir, encoded, "important.jsonl"),
}
if err := DeleteSessionAt(loc); !errors.Is(err, ErrSessionOutsideRoot) {
t.Errorf("EncodedDir %q: error = %v, want ErrSessionOutsideRoot", encoded, err)
}
}
if _, statErr := os.Stat(outside); statErr != nil {
t.Errorf("a file outside the sessions root was deleted: %v", statErr)
}
}
func TestDeleteSessionAt_RefusesAPathThatDisagreesWithItsParts(t *testing.T) {
setTestHome(t, t.TempDir())
repoDir := t.TempDir()
path := seedSession(t, repoDir, "20250601-100000-aaaaaa")
found, err := FindSessionsByID("20250601-100000-aaaaaa")
if err != nil || len(found) != 1 {
t.Fatalf("FindSessionsByID: %v, %d results", err, len(found))
}
forged := found[0]
forged.EncodedDir = forged.EncodedDir + "-other"
if err := DeleteSessionAt(forged); !errors.Is(err, ErrSessionOutsideRoot) {
t.Errorf("error = %v, want ErrSessionOutsideRoot", err)
}
if _, statErr := os.Stat(path); statErr != nil {
t.Errorf("the session was deleted on a Location that does not describe it: %v", statErr)
}
}
func TestDeleteSessionAt_DeletesALocationFromTheSearch(t *testing.T) {
setTestHome(t, t.TempDir())
repoDir := t.TempDir()
path := seedSession(t, repoDir, "20250601-100000-aaaaaa")
found, err := FindSessionsByID("20250601-100000-aaaaaa")
if err != nil || len(found) != 1 {
t.Fatalf("FindSessionsByID: %v, %d results", err, len(found))
}
if err := DeleteSessionAt(found[0]); err != nil {
t.Fatalf("DeleteSessionAt: %v", err)
}
if _, statErr := os.Stat(path); !os.IsNotExist(statErr) {
t.Errorf("session still present: %v", statErr)
}
}
func TestFindSessionsByID_ReportsAStatFailureRatherThanSkippingIt(t *testing.T) {
if os.Geteuid() == 0 {
t.Skip("root traverses a 0000 directory regardless of its mode")
}
home := t.TempDir()
setTestHome(t, home)
repoDir := t.TempDir()
path := seedSession(t, repoDir, "20250601-100000-aaaaaa")
dir := filepath.Dir(path)
if err := os.Chmod(dir, 0o000); err != nil {
t.Fatalf("chmod: %v", err)
}
t.Cleanup(func() { _ = os.Chmod(dir, 0o755) })
if _, err := os.Stat(path); err == nil || os.IsNotExist(err) {
t.Skip("this environment does not enforce directory modes")
}
found, err := FindSessionsByID("20250601-100000-aaaaaa")
if err == nil {
t.Fatalf("a stat failure must be reported, got %d results and no error", len(found))
}
if os.IsNotExist(err) {
t.Errorf("a permission failure was reported as a missing file: %v", err)
}
}
+10
View File
@@ -340,3 +340,13 @@ func parseRecordTime(s string) time.Time {
}
return time.Time{}
}
// LoadSummaryAt reads one session summary from an exact file path.
//
// LoadSummary derives the path from a repository, which the global id search
// deliberately does not have: it found the file first and learns the repository
// from the record inside it. fallbackRepoDir is only used when the file carries
// no cwd of its own.
func LoadSummaryAt(path, sessionID, fallbackRepoDir string) (*Summary, error) {
return loadSummaryFromFile(path, sessionID, fallbackRepoDir)
}
@@ -82,6 +82,7 @@ ocr review --commit HEAD | gh issue comment 123 --body-file -
| `ocr session comments <id>` | `ocr sessions comments <id>` | Print the review comments recorded in one session. |
| `ocr session compare <before> <after>` | `ocr session diff <before> <after>` | Compare two sessions' findings: new, persisting, resolved, not reviewed. |
| `ocr session export [id]` | — | Export one session as a self-contained HTML file. |
| `ocr session rm <id>` | `ocr session delete <id>`, `ocr session remove <id>` | Delete one saved review session. |
| `ocr viewer` | — | Launch the local web UI for past review sessions (`localhost:5483`). |
| `ocr version` | — | Print version, commit, platform, build date, and GitHub URL. |
@@ -500,6 +501,33 @@ treat the file with the same care as the repository itself before publishing it.
| `--repo <path>` | current dir | Repository whose session should be exported. |
| `--output <path>`, `-o` | stdout | Write the HTML to a file instead of stdout. |
### `ocr session rm`
Deletes one persisted session from `~/.opencodereview/sessions/`.
```bash
ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b
ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b --yes
ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b --repo ~/work/my-project
```
The id is enough on its own, so the command runs from any directory. If the same
id is saved for more than one repository, the candidates are listed and nothing
is deleted; pass `--repo` to pick one.
The session's repository, branch, start time, file count and comment count are
printed, and you are asked to confirm. **A non-interactive stdin answers no**, so
a pipeline or a CI job must pass `--yes` (`-y`) to skip the prompt.
A session whose metadata cannot be parsed is still deletable; one that cannot be
read at all is reported instead. With `--repo`, a session that records a
different repository, or none, is refused: delete it by id alone.
| Flag | Default | Description |
|---|---|---|
| `--repo <path>` | every repository | Only look for the session under this repository. |
| `--yes`, `-y` | `false` | Skip the confirmation prompt. |
## `ocr rules`
Rule introspection. There is exactly one subcommand:
@@ -80,6 +80,7 @@ ocr review --commit HEAD | gh issue comment 123 --body-file -
| `ocr session comments <id>` | `ocr sessions comments <id>` | 1つのセッションに記録されたレビューコメントを表示します。 |
| `ocr session compare <before> <after>` | `ocr session diff <before> <after>` | 2つのセッションの指摘を比較します:新規・継続・解決済み・未レビュー。 |
| `ocr session export [id]` | — | 1つのセッションを自己完結型の HTML ファイルとしてエクスポートします。 |
| `ocr session rm <id>` | `ocr session delete <id>`, `ocr session remove <id>` | 保存済みのレビューセッションを1つ削除します。 |
| `ocr viewer` | — | 過去のレビューセッション用のローカル Web UI を起動します(`localhost:5483`)。 |
| `ocr version` | — | バージョン、commit、プラットフォーム、ビルド日、GitHub URL を出力します。 |
@@ -475,6 +476,34 @@ ocr session export 20250601-100000-abc123 -o review.html
| `--repo <path>` | カレントディレクトリ | エクスポートするセッションが属するリポジトリ。 |
| `--output <path>`、`-o` | 標準出力 | HTML を標準出力ではなくファイルに書き出します。 |
### `ocr session rm`
`~/.opencodereview/sessions/` から保存済みのセッションを1つ削除します。
```bash
ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b
ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b --yes
ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b --repo ~/work/my-project
```
id だけで十分なので、どのディレクトリからでも実行できます。同じ id が複数の
リポジトリに保存されている場合は、候補を一覧表示して何も削除しません。`--repo`
で1つを指定してください。
セッションのリポジトリ、ブランチ、開始時刻、ファイル数、コメント数を表示し、
確認を求めます。**非対話的な stdin は「いいえ」として扱われる**ため、パイプラインや
CI ジョブで確認を省略するには `--yes`(`-y`)を渡してください。
メタデータを解析できないセッションも削除できます。まったく読み取れない場合は、
削除せずにエラーを報告します。`--repo` を指定した場合、別のリポジトリを記録している
セッションや、リポジトリを記録していないセッションは拒否されます。その場合は id
だけで削除してください。
| フラグ | デフォルト | 説明 |
|---|---|---|
| `--repo <path>` | すべてのリポジトリ | このリポジトリの下だけでセッションを探します。 |
| `--yes`、`-y` | `false` | 確認プロンプトを省略します。 |
## `ocr rules`
ルールの自己確認です。サブコマンドは 1 つだけです:
@@ -81,6 +81,7 @@ ocr review --commit HEAD | gh issue comment 123 --body-file -
| `ocr session comments <id>` | `ocr sessions comments <id>` | 세션에 기록된 리뷰 코멘트를 출력합니다. |
| `ocr session compare <before> <after>` | `ocr session diff <before> <after>` | 두 세션의 지적을 비교합니다: 새로 생긴 것, 남아 있는 것, 해결된 것, 리뷰하지 않은 것. |
| `ocr session export [id]` | — | 세션 하나를 단일 HTML 파일로 내보냅니다. |
| `ocr session rm <id>` | `ocr session delete <id>`, `ocr session remove <id>` | 저장된 리뷰 세션 하나를 삭제합니다. |
| `ocr viewer` | — | 지난 리뷰 세션을 볼 수 있는 로컬 웹 UI를 띄웁니다(`localhost:5483`). |
| `ocr version` | — | 버전, 커밋, 플랫폼, 빌드 날짜, GitHub URL을 출력합니다. |
@@ -489,6 +490,33 @@ ocr session export 20250601-100000-abc123 -o review.html
| `--repo <path>` | 현재 디렉터리 | 내보낼 세션이 속한 저장소. |
| `--output <path>`, `-o` | 표준 출력 | HTML을 표준 출력 대신 파일로 씁니다. |
### `ocr session rm` {#ocr-session-rm}
`~/.opencodereview/sessions/`에 저장된 세션 하나를 삭제합니다.
```bash
ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b
ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b --yes
ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b --repo ~/work/my-project
```
id만으로 충분하므로 어느 디렉터리에서든 실행할 수 있습니다. 같은 id가 여러
저장소에 저장되어 있으면 후보를 보여주고 아무것도 삭제하지 않습니다. `--repo`로
하나를 지정하세요.
세션의 저장소, 브랜치, 시작 시각, 파일 수, 댓글 수를 출력하고 확인을 요청합니다.
**비대화형 stdin은 "아니오"로 처리되므로**, 파이프라인이나 CI 작업에서 프롬프트를
건너뛰려면 `--yes`(`-y`)를 전달해야 합니다.
메타데이터를 해석할 수 없는 세션도 삭제할 수 있습니다. 아예 읽을 수 없는 경우에는
삭제하지 않고 오류를 보고합니다. `--repo`를 지정하면 다른 저장소를 기록했거나
저장소를 기록하지 않은 세션은 거부됩니다. 그때는 id만으로 삭제하세요.
| 플래그 | 기본값 | 설명 |
|---|---|---|
| `--repo <path>` | 모든 저장소 | 이 저장소 아래에서만 세션을 찾습니다. |
| `--yes`, `-y` | `false` | 확인 프롬프트를 건너뜁니다. |
## `ocr rules` {#ocr-rules}
규칙을 들여다보는 명령입니다. 하위 명령은 하나뿐입니다:
@@ -81,6 +81,7 @@ ocr review --commit HEAD | gh issue comment 123 --body-file -
| `ocr session comments <id>` | `ocr sessions comments <id>` | Выводит комментарии ревью, записанные в одной сессии. |
| `ocr session compare <before> <after>` | `ocr session diff <before> <after>` | Сравнивает находки двух сессий: новые, сохранившиеся, устранённые, не проверенные. |
| `ocr session export [id]` | — | Экспортирует одну сессию в автономный HTML-файл. |
| `ocr session rm <id>` | `ocr session delete <id>`, `ocr session remove <id>` | Удаляет одну сохранённую сессию ревью. |
| `ocr viewer` | — | Запускает локальный веб-интерфейс для просмотра прошлых сессий ревью (`localhost:5483`). |
| `ocr version` | — | Выводит версию, коммит, платформу, дату сборки и URL GitHub. |
@@ -481,6 +482,35 @@ id сессии. Без `-o` HTML выводится в stdout.
| `--repo <path>` | текущий каталог | Репозиторий, сессия которого экспортируется. |
| `--output <path>`, `-o` | stdout | Записать HTML в файл вместо stdout. |
### `ocr session rm`
Удаляет одну сохранённую сессию из `~/.opencodereview/sessions/`.
```bash
ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b
ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b --yes
ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b --repo ~/work/my-project
```
Идентификатора достаточно самого по себе, поэтому команда работает из любого
каталога. Если один и тот же идентификатор сохранён для нескольких репозиториев,
команда перечисляет кандидатов и ничего не удаляет; укажите нужный через `--repo`.
Команда печатает репозиторий, ветку, время начала, число файлов и число
комментариев сессии и просит подтверждение. **Неинтерактивный stdin означает
«нет»**, поэтому в конвейере или в задании CI нужно передать `--yes` (`-y`),
чтобы пропустить запрос.
Сессию с неразбираемыми метаданными удалить можно; ту, которую вообще не удаётся
прочитать, команда не удаляет, а сообщает об ошибке. С `--repo` сессия, которая
записала другой репозиторий или не записала никакого, отклоняется: удалите её
только по идентификатору.
| Флаг | По умолчанию | Описание |
|---|---|---|
| `--repo <path>` | все репозитории | Искать сессию только в этом репозитории. |
| `--yes`, `-y` | `false` | Пропустить запрос подтверждения. |
## `ocr rules`
Проверка правил. Доступна ровно одна подкоманда:
@@ -80,6 +80,7 @@ ocr review --commit HEAD | gh issue comment 123 --body-file -
| `ocr session comments <id>` | `ocr sessions comments <id>` | 输出单个会话中记录的评审评论。 |
| `ocr session compare <before> <after>` | `ocr session diff <before> <after>` | 对比两个会话的问题:新增、仍存在、已解决、未评审。 |
| `ocr session export [id]` | — | 将单个会话导出为自包含的 HTML 文件。 |
| `ocr session rm <id>` | `ocr session delete <id>`, `ocr session remove <id>` | 删除一个已保存的评审会话。 |
| `ocr viewer` | — | 启动用于历史评审会话的本地 Web UI(`localhost:5483`)。 |
| `ocr version` | — | 打印版本、commit、平台、构建日期与 GitHub URL。 |
@@ -471,6 +472,30 @@ ocr session export 20250601-100000-abc123 -o review.html
| `--repo <path>` | 当前目录 | 要导出会话的仓库。 |
| `--output <path>`、`-o` | 标准输出 | 将 HTML 写入文件而不是标准输出。 |
### `ocr session rm`
从 `~/.opencodereview/sessions/` 中删除一个已保存的会话。
```bash
ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b
ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b --yes
ocr session rm 9f2c1b4a-7e35-4d61-b2f0-6c8a41d9e72b --repo ~/work/my-project
```
只要会话 id 就够了,因此在任何目录下都能运行。如果同一个 id 在多个仓库中都存在,
命令会列出候选项并且不删除任何内容;用 `--repo` 指定其中一个。
命令会打印该会话的仓库、分支、开始时间、文件数和评论数,并请你确认。**非交互式的
stdin 一律视为「否」**,因此流水线或 CI 任务需要传入 `--yes`(`-y`)来跳过确认。
元数据无法解析的会话仍然可以删除;完全读不了的会话则会报错而不是删除。使用
`--repo` 时,记录了其他仓库或没有记录仓库的会话会被拒绝:这种情况请只用 id 删除。
| 标志 | 默认值 | 说明 |
|---|---|---|
| `--repo <path>` | 所有仓库 | 只在该仓库下查找这个会话。 |
| `--yes`、`-y` | `false` | 跳过确认提示。 |
## `ocr rules`
规则自查。只有一个子命令: