From 56d15e51c17edc1516bd3fe96b6c28534ed23335 Mon Sep 17 00:00:00 2001 From: keyolk Date: Tue, 15 Sep 2026 15:41:48 +0900 Subject: [PATCH] fix: surface nested scratchpad files and name published artifacts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A session's scratchpad listing read only the top level, so every file an agent organized into a subdirectory was invisible. A session whose own summary pointed at "scratchpad/kp/pr/pr.md" showed one unrelated file in the preview — the listing looked complete while omitting the work. The walk is now recursive, bounded three ways so it stays on the UI thread: * A git checkout in the scratchpad is listed as one row, not walked. One cloned karpenter tree otherwise filled every listing slot and pushed the session's own pr/issue.md out entirely. * The file cap trims by mtime, not walk order — a walk is alphabetical, so trimming mid-walk keeps whatever sorts first rather than whatever the session last touched. Bodies are read after trimming, so the byte budget is spent on files that survived. * A visit budget bounds the walk itself, which the file cap cannot: the cost is in traversing entries, paid before any file is selected. One measured scratchpad holds 70k entries and took ~1.2s to walk fully — long enough to freeze the preview. It now costs ~136ms. When a bound bites, the listing says so rather than silently ending short, so a missing file is not mistaken for proof it never existed. Artifacts had the opposite problem: the URL was tracked but the row read "artifact:6c6ced7d", naming nothing. The Artifact tool's result line carries the source path, so its basename becomes the ref Title, which the renderer already preferred. Multiple publishes on one line are paired by URL — mislabeling an artifact is worse than leaving it unnamed. --- .../session/artifact_publish_scope_test.go | 34 +++ internal/session/outputs.go | 16 ++ internal/session/refs.go | 40 +++- internal/session/scratchpad.go | 211 +++++++++++++++--- internal/session/scratchpad_test.go | 114 +++++++++- internal/tui/app.go | 8 + internal/tui/conversation_meta_entry.go | 6 + 7 files changed, 393 insertions(+), 36 deletions(-) diff --git a/internal/session/artifact_publish_scope_test.go b/internal/session/artifact_publish_scope_test.go index defd803..161e76f 100644 --- a/internal/session/artifact_publish_scope_test.go +++ b/internal/session/artifact_publish_scope_test.go @@ -61,3 +61,37 @@ func TestBuildSessionFlow_ArtifactRefSkipsTextMention(t *testing.T) { t.Errorf("facet ArtifactRef count = %d, want 0 for text-only mention", facets.Counts[ArtifactRef]) } } + +// An artifact's UUID label identifies nothing — "artifact:6c6ced7d" tells a +// reader only that a page exists. The Artifact tool's result line carries the +// source path, and its basename is the name a person recognizes, so the ref +// carries it as Title. +func TestExtractSessionRefsFromFile_ArtifactTitleFromPublishPath(t *testing.T) { + dir := t.TempDir() + fp := filepath.Join(dir, "s.jsonl") + line := `{"type":"user","timestamp":"2026-07-01T10:00:12.000Z","message":{"role":"user","content":[{"type":"tool_result","tool_use_id":"t1","content":"Published /tmp/claude-502/enc/sid/scratchpad/mobile-audit-verdict.html at https://claude.ai/code/artifact/11111111-2222-3333-4444-555555555555\n\nLive subscription: arming"}]}}` + if err := os.WriteFile(fp, []byte(line+"\n"), 0o644); err != nil { + t.Fatal(err) + } + refs := ExtractSessionRefsFromFile(fp) + if len(refs) != 1 { + t.Fatalf("want 1 ref, got %d: %+v", len(refs), refs) + } + if refs[0].Title != "mobile-audit-verdict.html" { + t.Errorf("Title = %q, want %q", refs[0].Title, "mobile-audit-verdict.html") + } +} + +// One line can announce several publishes. Pairing a name to the wrong URL is +// worse than leaving it unnamed, so the title must come from the entry whose +// URL matches. +func TestArtifactTitleFromPublish_PairsNameToItsOwnURL(t *testing.T) { + text := "Published /a/first.html at https://claude.ai/code/artifact/11111111-1111-1111-1111-111111111111\n" + + "Published /b/second.html at https://claude.ai/code/artifact/22222222-2222-2222-2222-222222222222" + if got := artifactTitleFromPublish(text, "https://claude.ai/code/artifact/22222222-2222-2222-2222-222222222222"); got != "second.html" { + t.Errorf("got %q, want second.html", got) + } + if got := artifactTitleFromPublish(text, "https://claude.ai/code/artifact/33333333-3333-3333-3333-333333333333"); got != "" { + t.Errorf("unmatched url should yield no title, got %q", got) + } +} diff --git a/internal/session/outputs.go b/internal/session/outputs.go index b5d0fa0..37f9968 100644 --- a/internal/session/outputs.go +++ b/internal/session/outputs.go @@ -245,7 +245,23 @@ func collectScratchpadOutputs(sess Session) []SessionOutput { files := LoadScratchpadFiles(sess.ProjectPath, sess.ID) outs := make([]SessionOutput, 0, len(files)) for _, f := range files { + // The truncation note is a property of the listing, not an output. + if f.Name == ScratchpadTruncatedMarker { + continue + } mt := time.Unix(f.ModTime, 0) + if f.IsRepo { + outs = append(outs, SessionOutput{ + Kind: OutputScratchpad, + Title: f.Name, + Detail: "git repository", + Path: f.Path, + First: mt, + Last: mt, + Count: 1, + }) + continue + } outs = append(outs, SessionOutput{ Kind: OutputScratchpad, Title: f.Name, diff --git a/internal/session/refs.go b/internal/session/refs.go index 29dfc20..d8e5a15 100644 --- a/internal/session/refs.go +++ b/internal/session/refs.go @@ -10,6 +10,7 @@ import ( "net/url" "os" "os/exec" + "path" "regexp" "sort" "strings" @@ -149,8 +150,11 @@ func ExtractSessionRefsFromFile(filePath string) []SessionRef { // tool_result says "Published at ". A URL merely quoted // in text (often a link to another session's artifact) is not a ref // this session produced, so skip it. - if ref.Kind == RefArtifact && !bytes.Contains(line, []byte("Published")) { - continue + if ref.Kind == RefArtifact { + if !bytes.Contains(line, []byte("Published")) { + continue + } + ref.Title = artifactTitleFromPublish(string(line), u) } if !tsParsed { ts = lineTimestamp(line) @@ -236,8 +240,11 @@ func ExtractSessionRefs(entries []Entry) []SessionRef { } // Artifacts: only from publish-marker text (Artifact // tool_result "Published … at "), not text mentions. - if ref.Kind == RefArtifact && !strings.Contains(text, "Published") { - continue + if ref.Kind == RefArtifact { + if !strings.Contains(text, "Published") { + continue + } + ref.Title = artifactTitleFromPublish(text, u) } seen[ref.Label] = true ref.FirstSeen = ts @@ -308,6 +315,31 @@ func classifyRef(u string) (SessionRef, bool) { return SessionRef{}, false } +// artifactPublishRegex captures the source file path out of the Artifact tool's +// result line, "Published at ". The path is the only human-readable +// name the transcript carries for a published page: the UUID label identifies +// nothing, so without this an artifact row reads "artifact:6c6ced7d" and the +// reader has to open the URL to find out what it is. +var artifactPublishRegex = regexp.MustCompile(`Published\s+(\S+)\s+at\s+(\S+)`) + +// artifactTitleFromPublish returns the base filename of the page published at +// url, given the text of the tool result that announced it. Returns "" when the +// text announces a different artifact — a single line can carry several +// publishes, and pairing the wrong name to a URL is worse than no name. +func artifactTitleFromPublish(text, url string) string { + for _, m := range artifactPublishRegex.FindAllStringSubmatch(text, -1) { + if cleanRefURL(m[2]) != url { + continue + } + name := path.Base(m[1]) + if name == "." || name == "/" { + return "" + } + return name + } + return "" +} + // artifactURLRegex captures the UUID at the end of a claude.ai artifact URL. var artifactURLRegex = regexp.MustCompile(`claude\.ai/code/artifact/([0-9a-fA-F-]{8,})`) diff --git a/internal/session/scratchpad.go b/internal/session/scratchpad.go index 7a83e08..be63ded 100644 --- a/internal/session/scratchpad.go +++ b/internal/session/scratchpad.go @@ -2,6 +2,7 @@ package session import ( "io" + "io/fs" "os" "path/filepath" "sort" @@ -15,13 +16,16 @@ import ( // at scratchpadMaxBody bytes, with Truncated set when the file is larger); // binary files carry a placeholder instead. type ScratchpadFile struct { - Name string // base filename + Name string // path relative to the scratchpad root ("pr/issue.md") Path string // absolute path Size int64 ModTime int64 // unix seconds; avoids importing time in callers IsText bool Truncated bool // true when Body is a prefix of a larger text file Body string + // IsRepo marks a checked-out git working tree that was listed as one row + // instead of walked. Path points at the directory. + IsRepo bool } // scratchpadMaxBody caps how much of a scratchpad file we read into memory for @@ -29,6 +33,35 @@ type ScratchpadFile struct { // Truncated so the caller can render a "(truncated)" marker. const scratchpadMaxBody = 256 * 1024 +// scratchpadMaxFiles caps how many files one scratchpad contributes. Sessions +// that unpack an archive or generate per-cell manifests hit five figures; the +// listing is a digest of what the session produced, not a file manager. When +// the cap bites, the most recently modified files win — those are the ones the +// session was actually working on. +const scratchpadMaxFiles = 300 + +// scratchpadMaxTotalBody caps the summed body bytes across all files. Without +// it a recursive walk over a few hundred 256KB files would pull ~75MB into +// memory on every preview. +const scratchpadMaxTotalBody = 4 * 1024 * 1024 + +// scratchpadMaxVisits bounds the walk itself, which the file cap cannot: the +// cost is in traversing directory entries, and that is paid before any file is +// selected. One measured scratchpad holds 70k entries across 18k directories +// and takes ~1.2s to walk fully — long enough to freeze the preview, since this +// runs on the UI thread. At 10k entries the walk costs ~136ms. Hitting the +// budget sets Truncated on the synthetic listing rather than failing quietly. +const scratchpadMaxVisits = 10000 + +// scratchpadSkipDirs are never descended into: build/cache output and +// dependency trees are not session products, and listing them buries the files +// that are. +var scratchpadSkipDirs = map[string]bool{ + "node_modules": true, "vendor": true, "target": true, "dist": true, "build": true, + "__pycache__": true, ".mypy_cache": true, ".pytest_cache": true, ".ruff_cache": true, + ".venv": true, "venv": true, ".tox": true, ".terraform": true, +} + // scratchpadBaseOverride lets tests redirect ScratchpadBase to a temp dir // without touching /tmp. Empty in production. var scratchpadBaseOverride string @@ -54,60 +87,176 @@ func SetScratchpadBaseOverride(dir string) func() { return func() { scratchpadBaseOverride = prev } } -// LoadScratchpadFiles reads and parses every file in the given session's -// scratchpad directory, sorted by name. Returns nil when the directory is -// absent or empty. projectPath is the session's ProjectPath (the unencoded -// absolute path); sessionID is the session UUID. +// LoadScratchpadFiles walks the given session's scratchpad directory +// recursively and returns every file, sorted by path. Returns nil when the +// directory is absent or empty. projectPath is the session's ProjectPath (the +// unencoded absolute path); sessionID is the session UUID. +// +// The walk is recursive because agents organize their scratchpad into +// subdirectories (pr/issue.md, kp/pr/pr.md). A top-level-only listing silently +// dropped those — the session's own summary would point at "scratchpad/kp/pr/ +// pr.md" while the preview showed nothing. +// +// Recursion means the walk can also wander into a repository the session +// cloned into its scratchpad, so it is bounded three ways: .git and dependency +// /build directories are never descended into, the file count is capped at +// scratchpadMaxFiles, and summed body bytes at scratchpadMaxTotalBody. Once a +// cap is hit, remaining files are still listed (name/size/mtime) with empty +// bodies rather than dropped, so the listing stays honest about what exists. func LoadScratchpadFiles(projectPath, sessionID string) []ScratchpadFile { if projectPath == "" || sessionID == "" { return nil } - dir := filepath.Join(ScratchpadBase(), EncodeProjectPath(projectPath), sessionID, "scratchpad") - entries, err := os.ReadDir(dir) - if err != nil { + root := filepath.Join(ScratchpadBase(), EncodeProjectPath(projectPath), sessionID, "scratchpad") + if _, err := os.Stat(root); err != nil { return nil } var files []ScratchpadFile - for _, e := range entries { - if e.IsDir() { - continue + visits := 0 + truncatedWalk := false + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + visits++ + if visits > scratchpadMaxVisits { + truncatedWalk = true + return fs.SkipAll } - info, err := e.Info() if err != nil { - continue + // An unreadable subdirectory must not abort the whole walk — the + // rest of the scratchpad is still worth listing. + if d != nil && d.IsDir() { + return fs.SkipDir + } + return nil + } + if d.IsDir() { + if path == root { + return nil + } + name := d.Name() + // .git carries thousands of objects and is never a session product. + if name == ".git" || scratchpadSkipDirs[strings.ToLower(name)] { + return fs.SkipDir + } + // A repository the session cloned into its scratchpad is upstream + // code, not session output. Walking it buried the files the session + // actually wrote: one karpenter checkout filled all 300 slots and + // pushed the session's own pr/issue.md out of the listing entirely. + // It is listed as a single row so the clone is still visible. + if isGitWorkTree(path) { + info, ierr := d.Info() + if ierr == nil { + rel, rerr := filepath.Rel(root, path) + if rerr != nil { + rel = name + } + files = append(files, ScratchpadFile{ + Name: filepath.ToSlash(rel) + "/", + Path: path, + ModTime: info.ModTime().Unix(), + IsRepo: true, + Body: "(git repository)", + }) + } + return fs.SkipDir + } + return nil + } + if !d.Type().IsRegular() { + return nil // symlinks, sockets, devices + } + info, ierr := d.Info() + if ierr != nil { + return nil } - full := filepath.Join(dir, e.Name()) - sf := ScratchpadFile{ - Name: e.Name(), - Path: full, + rel, rerr := filepath.Rel(root, path) + if rerr != nil { + rel = d.Name() + } + files = append(files, ScratchpadFile{ + Name: filepath.ToSlash(rel), + Path: path, Size: info.Size(), ModTime: info.ModTime().Unix(), + }) + return nil + }) + if err != nil && len(files) == 0 { + return nil + } + if len(files) == 0 { + return nil + } + + // Trim by recency, not by walk order: a walk is alphabetical, so capping + // mid-walk would keep whatever sorts first rather than whatever the session + // last touched. + trimmed := truncatedWalk + if len(files) > scratchpadMaxFiles { + trimmed = true + sort.SliceStable(files, func(i, j int) bool { + return files[i].ModTime > files[j].ModTime + }) + files = files[:scratchpadMaxFiles] + } + + // Bodies are read only after trimming, so the budget is spent on files that + // survived rather than on whatever the walk happened to reach first. + var bodyBytes int64 + for i := range files { + if files[i].IsRepo { + continue } - data, truncated, err := readCapped(full, info.Size()) - if err == nil { - sf.IsText = isLikelyText(data) - sf.Truncated = truncated - if sf.IsText { - sf.Body = string(data) - } else { - sf.Body = "(binary file)" - } + if bodyBytes >= scratchpadMaxTotalBody { + // Past the budget the file still belongs in the listing; only its + // content is withheld. + files[i].IsText = true + files[i].Truncated = true + continue + } + data, truncated, rerr := readCapped(files[i].Path, files[i].Size) + if rerr != nil { + files[i].Body = "(unreadable)" + continue + } + files[i].IsText = isLikelyText(data) + files[i].Truncated = truncated + if files[i].IsText { + files[i].Body = string(data) } else { - sf.Body = "(unreadable)" + files[i].Body = "(binary file)" } - files = append(files, sf) + bodyBytes += int64(len(data)) } - if len(files) == 0 { - return nil - } sort.Slice(files, func(i, j int) bool { return files[i].Name < files[j].Name }) + if trimmed { + // A silently short listing reads as "this is everything". Say so + // instead, so the absence of a file is not mistaken for proof it does + // not exist. + files = append(files, ScratchpadFile{ + Name: ScratchpadTruncatedMarker, + Path: root, + IsText: true, + Body: "(listing truncated — open the directory to see the rest)", + }) + } return files } +// ScratchpadTruncatedMarker names the synthetic row appended when the listing +// could not cover the whole directory. Renderers show it as a note, not a file. +const ScratchpadTruncatedMarker = "… (truncated)" + +// isGitWorkTree reports whether dir is the root of a git checkout. Both a +// normal clone (.git directory) and a worktree/submodule (.git file) count. +func isGitWorkTree(dir string) bool { + _, err := os.Stat(filepath.Join(dir, ".git")) + return err == nil +} + // readCapped reads up to scratchpadMaxBody bytes from path, returning the // content, whether the file exceeded the cap, and any read error. Avoids // loading huge files fully into memory. diff --git a/internal/session/scratchpad_test.go b/internal/session/scratchpad_test.go index 93941d2..50feadb 100644 --- a/internal/session/scratchpad_test.go +++ b/internal/session/scratchpad_test.go @@ -17,7 +17,11 @@ func writeScratchpadFixture(t *testing.T, projectPath, sessionID string, files m t.Fatalf("mkdir: %v", err) } for name, content := range files { - if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil { + full := filepath.Join(dir, name) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatalf("mkdir %s: %v", name, err) + } + if err := os.WriteFile(full, []byte(content), 0o644); err != nil { t.Fatalf("write %s: %v", name, err) } } @@ -140,3 +144,111 @@ func TestLoadScratchpadFiles_TruncatesLargeText(t *testing.T) { t.Errorf("Size should reflect full file: got %d want %d", files[0].Size, len(big)) } } + +// scratchpadNames returns the listed names, so a test can assert on the listing +// without caring about bodies. +func scratchpadNames(files []ScratchpadFile) []string { + out := make([]string, 0, len(files)) + for _, f := range files { + out = append(out, f.Name) + } + return out +} + +// Agents organize their scratchpad into subdirectories ("kp/pr/pr.md"), and a +// top-level-only listing dropped every one of them: a session whose own summary +// pointed at scratchpad/kp/pr/pr.md showed nothing in the preview. +func TestLoadScratchpadFiles_Recurses(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + defer SetScratchpadBaseOverride(t.TempDir())() + proj, sid := writeScratchpadFixture(t, "/p", "sid", map[string]string{ + "reply.txt": "top level", + "kp/pr/issue.md": "issue body", + "kp/pr/pr.md": "pr body", + }) + + got := scratchpadNames(LoadScratchpadFiles(proj, sid)) + want := []string{"kp/pr/issue.md", "kp/pr/pr.md", "reply.txt"} + if len(got) != len(want) { + t.Fatalf("got %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("index %d: got %q, want %q", i, got[i], want[i]) + } + } +} + +// A repository cloned into the scratchpad is upstream code, not session output. +// Walking it buried the session's own files: one karpenter checkout filled every +// listing slot and pushed kp/pr/issue.md out entirely. +func TestLoadScratchpadFiles_ListsClonedRepoAsOneRow(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + defer SetScratchpadBaseOverride(t.TempDir())() + files := map[string]string{"kp/pr/pr.md": "pr body"} + for i := 0; i < 20; i++ { + files[filepath.Join("kp/upstream/pkg", "f"+itoa(i)+".go")] = "package upstream" + } + files["kp/upstream/.git/HEAD"] = "ref: refs/heads/main" + proj, sid := writeScratchpadFixture(t, "/p", "sid", files) + + got := LoadScratchpadFiles(proj, sid) + names := scratchpadNames(got) + want := []string{"kp/pr/pr.md", "kp/upstream/"} + if len(names) != len(want) { + t.Fatalf("got %v, want %v", names, want) + } + for i := range want { + if names[i] != want[i] { + t.Errorf("index %d: got %q, want %q", i, names[i], want[i]) + } + } + for _, f := range got { + if f.Name == "kp/upstream/" && !f.IsRepo { + t.Error("cloned repo row should set IsRepo") + } + } +} + +// Build/dependency trees are not session products; descending into them buries +// what is. +func TestLoadScratchpadFiles_SkipsDependencyDirs(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + defer SetScratchpadBaseOverride(t.TempDir())() + proj, sid := writeScratchpadFixture(t, "/p", "sid", map[string]string{ + "notes.md": "keep", + "node_modules/left-pad.js": "drop", + "__pycache__/x.pyc": "drop", + }) + + got := scratchpadNames(LoadScratchpadFiles(proj, sid)) + if len(got) != 1 || got[0] != "notes.md" { + t.Fatalf("got %v, want [notes.md]", got) + } +} + +// When the listing cannot cover everything, saying so beats a silently short +// list that reads as "this is everything". +func TestLoadScratchpadFiles_MarksTruncation(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + defer SetScratchpadBaseOverride(t.TempDir())() + files := make(map[string]string, scratchpadMaxFiles+10) + for i := 0; i < scratchpadMaxFiles+10; i++ { + files["f"+itoa(i)+".txt"] = "x" + } + proj, sid := writeScratchpadFixture(t, "/p", "sid", files) + + got := LoadScratchpadFiles(proj, sid) + if len(got) != scratchpadMaxFiles+1 { + t.Fatalf("got %d rows, want %d files + 1 truncation note", len(got), scratchpadMaxFiles) + } + found := false + for _, f := range got { + if f.Name == ScratchpadTruncatedMarker { + found = true + } + } + if !found { + t.Errorf("truncated listing must carry %q; got %v", ScratchpadTruncatedMarker, scratchpadNames(got)) + } +} diff --git a/internal/tui/app.go b/internal/tui/app.go index 01da58e..9e0720c 100644 --- a/internal/tui/app.go +++ b/internal/tui/app.go @@ -7610,6 +7610,14 @@ func (a *App) buildScratchpadContent(sess session.Session) string { func (a *App) renderScratchpadFile(f session.ScratchpadFile, width int) string { var sb strings.Builder + // A repo row and the truncation note have no size/mtime worth showing — + // they describe the listing, not a file the session wrote. + if f.IsRepo || f.Name == session.ScratchpadTruncatedMarker { + sb.WriteString(dimStyle.Render("── ") + lipgloss.NewStyle().Foreground(colorAccent).Render(f.Name) + "\n") + sb.WriteString(dimStyle.Render(" "+f.Body) + "\n\n") + return sb.String() + } + size := humanSize(f.Size) mtime := time.Unix(f.ModTime, 0).Format("2006-01-02 15:04") header := dimStyle.Render("── ") + lipgloss.NewStyle().Foreground(colorAccent).Render(f.Name) + diff --git a/internal/tui/conversation_meta_entry.go b/internal/tui/conversation_meta_entry.go index 982fb20..97058ae 100644 --- a/internal/tui/conversation_meta_entry.go +++ b/internal/tui/conversation_meta_entry.go @@ -1233,7 +1233,13 @@ func (a *App) metaScratchpadEntries() []metaEntry { // scratchpadFileRow renders one scratchpad file as a selectable row: name, size, // and mtime. Binary files are marked. func scratchpadFileRow(f session.ScratchpadFile) string { + if f.Name == session.ScratchpadTruncatedMarker { + return dimStyle.Render(f.Name + " " + f.Body) + } row := lipgloss.NewStyle().Foreground(colorAccent).Bold(true).Render(f.Name) + if f.IsRepo { + return row + dimStyle.Render(" (git repository)") + } row += dimStyle.Render(fmt.Sprintf(" %s %s", humanSize(f.Size), scratchpadMtime(f.ModTime))) if !f.IsText { row += dimStyle.Render(" (binary)")