From 32bfcffc627d630d2e62cf3d37246e6e7c7ea307 Mon Sep 17 00:00:00 2001 From: Agent Date: Fri, 15 May 2026 16:03:13 +0800 Subject: [PATCH 1/2] fix: skip directory walk for single file uploads in sth send When a single HTML file has no sibling web assets or common web asset subdirectories (assets/, css/, js/, etc.), zip only the file itself instead of walking the entire parent directory. This fixes timeouts and parse errors when the file is in a large directory like /home/user. Closes #55 Co-Authored-By: Claude Opus 4.7 --- internal/cli/cli.go | 85 +++++++++++++++++++++++++++++++++++++--- internal/cli/cli_test.go | 63 +++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+), 6 deletions(-) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 48d3517..870c2f3 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -295,6 +295,84 @@ func newUploadRequestBody(entryFile string, tags []string, category, project str func writeZIPArchive(rootDir string, target io.Writer) error { archive := zip.NewWriter(target) + + var err error + if hasSiblingWebAssets(rootDir) { + err = writeDirEntries(archive, rootDir) + } else { + err = writeSingleFileEntry(archive, rootDir) + } + + if err != nil { + _ = archive.Close() + return err + } + return archive.Close() +} + +func hasSiblingWebAssets(dir string) bool { + entries, err := os.ReadDir(dir) + if err != nil { + return true + } + webCount := 0 + for _, e := range entries { + if e.IsDir() { + if isWebAssetDir(e.Name()) { + return true + } + continue + } + if isWebAsset(e.Name()) { + webCount++ + if webCount > 1 { + return true + } + } + } + return false +} + +func isWebAssetDir(name string) bool { + lower := strings.ToLower(name) + switch lower { + case "assets", "css", "js", "images", "img", "fonts", "media", + "static", "public", "styles", "scripts", "resources": + return true + } + return false +} + +func writeSingleFileEntry(archive *zip.Writer, dir string) error { + entries, err := os.ReadDir(dir) + if err != nil { + return err + } + for _, e := range entries { + if e.IsDir() || !e.Type().IsRegular() || !isWebAsset(e.Name()) { + continue + } + fullPath := filepath.Join(dir, e.Name()) + sourceFile, err := os.Open(fullPath) + if err != nil { + return err + } + w, err := archive.Create(e.Name()) + if err != nil { + _ = sourceFile.Close() + return err + } + _, copyErr := io.Copy(w, sourceFile) + closeErr := sourceFile.Close() + if copyErr != nil { + return copyErr + } + return closeErr + } + return fmt.Errorf("no web asset found in %s", dir) +} + +func writeDirEntries(archive *zip.Writer, rootDir string) error { err := filepath.WalkDir(rootDir, func(filePath string, entry os.DirEntry, walkErr error) error { if walkErr != nil { if errors.Is(walkErr, os.ErrPermission) || strings.Contains(walkErr.Error(), "permission denied") { @@ -350,12 +428,7 @@ func writeZIPArchive(rootDir string, target io.Writer) error { return closeErr }) - if err != nil { - _ = archive.Close() - return err - } - - return archive.Close() + return err } func isHiddenPath(path string) bool { diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 6cdd018..9e7fa27 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -5,6 +5,7 @@ import ( "bytes" "context" "encoding/json" + "fmt" "io" "net/http" "net/http/httptest" @@ -403,3 +404,65 @@ func TestWriteZIPArchiveSkipsPermissionDenied(t *testing.T) { t.Fatalf("expected index.html, got %q", zipReader.File[0].Name) } } + +func TestSingleFileSkipsWalkOnLargeDirectory(t *testing.T) { + t.Parallel() + + rootDir := t.TempDir() + entryFile := filepath.Join(rootDir, "report.html") + if err := os.WriteFile(entryFile, []byte("report"), 0o644); err != nil { + t.Fatal(err) + } + + // Create many non-web sibling files to simulate a large directory (e.g. home dir) + for i := 0; i < 200; i++ { + f := filepath.Join(rootDir, fmt.Sprintf("data-%d.log", i)) + if err := os.WriteFile(f, []byte("log data"), 0o644); err != nil { + t.Fatal(err) + } + } + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + reader, err := r.MultipartReader() + if err != nil { + t.Fatal(err) + } + archiveEntries := map[string]string{} + for { + part, err := reader.NextPart() + if err == io.EOF { + break + } + if err != nil { + t.Fatal(err) + } + if part.FormName() == "archive" { + archiveBytes, _ := io.ReadAll(part) + zr, _ := zip.NewReader(bytes.NewReader(archiveBytes), int64(len(archiveBytes))) + for _, f := range zr.File { + fr, _ := f.Open() + c, _ := io.ReadAll(fr) + fr.Close() + archiveEntries[f.Name] = string(c) + } + } + } + + if len(archiveEntries) != 1 { + t.Fatalf("expected 1 file in archive, got %d: %v", len(archiveEntries), archiveEntries) + } + if archiveEntries["report.html"] == "" { + t.Fatal("archive missing report.html") + } + + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]string{"url": "http://example.test/s/session/"}) + })) + defer srv.Close() + + var stdout bytes.Buffer + var stderr bytes.Buffer + if err := Run([]string{"send", entryFile, "--tag", "test", "--category", "cat", "--project", "proj", "--server", srv.URL}, &stdout, &stderr); err != nil { + t.Fatal(err) + } +} From ac7e13c9153c5ebb79eccc0fd3fdd074f60a2c67 Mon Sep 17 00:00:00 2001 From: Agent Date: Fri, 15 May 2026 16:34:58 +0800 Subject: [PATCH 2/2] fix: use conservative single-file detection to avoid dropping subdirectory assets Address AI code review feedback: - Replace hasSiblingWebAssets + isWebAssetDir with simpler isSingleFileDir: only use fast path when directory has exactly one non-hidden web asset file and zero non-hidden subdirectories. Any subdirectory presence triggers the full WalkDir to prevent silent resource loss. - Pass []os.DirEntry to writeSingleFileEntry to avoid redundant ReadDir. - Add hidden file check in writeSingleFileEntry for consistency with writeDirEntries behavior. - Add regression test TestSubdirectoryWithAssetsStillWalks verifying that non-standard subdirectory names (src/, components/, etc.) are walked. Co-Authored-By: Claude Opus 4.7 --- internal/cli/cli.go | 56 ++++++++++++++++------------------------ internal/cli/cli_test.go | 42 ++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 34 deletions(-) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 870c2f3..5e9692c 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -296,11 +296,14 @@ func newUploadRequestBody(entryFile string, tags []string, category, project str func writeZIPArchive(rootDir string, target io.Writer) error { archive := zip.NewWriter(target) + entries, readErr := os.ReadDir(rootDir) + singleFile := readErr == nil && isSingleFileDir(entries) + var err error - if hasSiblingWebAssets(rootDir) { - err = writeDirEntries(archive, rootDir) + if singleFile { + err = writeSingleFileEntry(archive, rootDir, entries) } else { - err = writeSingleFileEntry(archive, rootDir) + err = writeDirEntries(archive, rootDir) } if err != nil { @@ -310,50 +313,35 @@ func writeZIPArchive(rootDir string, target io.Writer) error { return archive.Close() } -func hasSiblingWebAssets(dir string) bool { - entries, err := os.ReadDir(dir) - if err != nil { - return true - } +// isSingleFileDir returns true only when the directory contains exactly one +// non-hidden web asset file and no non-hidden subdirectories. In all other +// cases we fall back to the full WalkDir to avoid silently dropping resources. +func isSingleFileDir(entries []os.DirEntry) bool { webCount := 0 for _, e := range entries { - if e.IsDir() { - if isWebAssetDir(e.Name()) { - return true - } + if isHiddenName(e.Name()) { continue } - if isWebAsset(e.Name()) { + if e.IsDir() { + return false + } + if e.Type().IsRegular() && isWebAsset(e.Name()) { webCount++ - if webCount > 1 { - return true - } } } - return false + return webCount == 1 } -func isWebAssetDir(name string) bool { - lower := strings.ToLower(name) - switch lower { - case "assets", "css", "js", "images", "img", "fonts", "media", - "static", "public", "styles", "scripts", "resources": - return true - } - return false +func isHiddenName(name string) bool { + return strings.HasPrefix(name, ".") && name != "." && name != ".." } -func writeSingleFileEntry(archive *zip.Writer, dir string) error { - entries, err := os.ReadDir(dir) - if err != nil { - return err - } +func writeSingleFileEntry(archive *zip.Writer, dir string, entries []os.DirEntry) error { for _, e := range entries { - if e.IsDir() || !e.Type().IsRegular() || !isWebAsset(e.Name()) { + if isHiddenName(e.Name()) || e.IsDir() || !e.Type().IsRegular() || !isWebAsset(e.Name()) { continue } - fullPath := filepath.Join(dir, e.Name()) - sourceFile, err := os.Open(fullPath) + sourceFile, err := os.Open(filepath.Join(dir, e.Name())) if err != nil { return err } @@ -369,7 +357,7 @@ func writeSingleFileEntry(archive *zip.Writer, dir string) error { } return closeErr } - return fmt.Errorf("no web asset found in %s", dir) + return fmt.Errorf("no web asset found in directory") } func writeDirEntries(archive *zip.Writer, rootDir string) error { diff --git a/internal/cli/cli_test.go b/internal/cli/cli_test.go index 9e7fa27..849c367 100644 --- a/internal/cli/cli_test.go +++ b/internal/cli/cli_test.go @@ -466,3 +466,45 @@ func TestSingleFileSkipsWalkOnLargeDirectory(t *testing.T) { t.Fatal(err) } } + +func TestSubdirectoryWithAssetsStillWalks(t *testing.T) { + t.Parallel() + + rootDir := t.TempDir() + entryFile := filepath.Join(rootDir, "index.html") + if err := os.WriteFile(entryFile, []byte("ok"), 0o644); err != nil { + t.Fatal(err) + } + + // Non-standard subdirectory name that isWebAssetDir won't recognize + cssDir := filepath.Join(rootDir, "src", "components") + if err := os.MkdirAll(cssDir, 0o755); err != nil { + t.Fatal(err) + } + cssFile := filepath.Join(cssDir, "style.css") + if err := os.WriteFile(cssFile, []byte("body{}"), 0o644); err != nil { + t.Fatal(err) + } + + var buf bytes.Buffer + if err := writeZIPArchive(rootDir, &buf); err != nil { + t.Fatalf("writeZIPArchive failed: %v", err) + } + + zr, err := zip.NewReader(bytes.NewReader(buf.Bytes()), int64(buf.Len())) + if err != nil { + t.Fatalf("failed to read zip: %v", err) + } + + names := map[string]bool{} + for _, f := range zr.File { + names[f.Name] = true + } + + if !names["index.html"] { + t.Fatal("archive missing index.html") + } + if !names["src/components/style.css"] { + t.Fatal("archive missing src/components/style.css — subdirectory assets should be included") + } +}