fix(gmail): respect --name for directory --out

Co-authored-by: zerone0x <hi@trine.dev>

Author: steipete

CHANGELOG.md

@@ -6,6 +6,9 @@
 - Forms: add `forms` command group (create/get forms, list/get responses).
 - Apps Script: add `appscript` command group (create/get projects, fetch content, run deployed functions).
 
+### Fixed
+- Gmail: when `gmail attachment --out` points to a directory (or ends with a trailing slash), combine with `--name` and avoid false cache hits on directories. (#248) — thanks @zerone0x.
+
 ## 0.10.0 - 2026-02-14
 
 ### Added

internal/cmd/gmail_attachment.go

@@ -20,11 +20,47 @@ type GmailAttachmentCmd struct {
 	MessageID    string         `arg:"" name:"messageId" help:"Message ID"`
 	AttachmentID string         `arg:"" name:"attachmentId" help:"Attachment ID"`
 	Output       OutputPathFlag `embed:""`
-	Name         string         `name:"name" help:"Filename (only used when --out is empty)"`
+	Name         string         `name:"name" help:"Filename (used with default dir or when --out is a directory)"`
 }
 
 const defaultGmailAttachmentFilename = "attachment.bin"
 
+func sanitizeAttachmentFilename(name string) string {
+	filename := strings.TrimSpace(name)
+	if filename == "" {
+		filename = defaultGmailAttachmentFilename
+	}
+
+	// Ensure users can't escape the output dir when `--out` is a directory.
+	safeFilename := filepath.Base(filename)
+	if safeFilename == "" || safeFilename == "." || safeFilename == ".." {
+		safeFilename = defaultGmailAttachmentFilename
+	}
+	return safeFilename
+}
+
+func resolveAttachmentOutPath(outPathFlag string, name string) (string, error) {
+	outPath, err := config.ExpandPath(outPathFlag)
+	if err != nil {
+		return "", err
+	}
+
+	// Directory intent:
+	// - existing directory path
+	// - or explicit trailing slash for a (possibly non-existent) directory
+	isDir := strings.HasSuffix(outPathFlag, "/") || strings.HasSuffix(outPathFlag, "\\")
+	if !isDir {
+		if info, statErr := os.Stat(outPath); statErr == nil && info.IsDir() {
+			isDir = true
+		}
+	}
+	if isDir {
+		outPath = filepath.Join(outPath, sanitizeAttachmentFilename(name))
+	}
+
+	return outPath, nil
+}
+
 func (c *GmailAttachmentCmd) Run(ctx context.Context, flags *RootFlags) error {
 	u := ui.FromContext(ctx)
 	messageID := normalizeGmailMessageID(c.MessageID)
@@ -40,21 +76,14 @@ func (c *GmailAttachmentCmd) Run(ctx context.Context, flags *RootFlags) error {
 		if dirErr != nil {
 			return dirErr
 		}
-		filename := strings.TrimSpace(c.Name)
-		if filename == "" {
-			filename = defaultGmailAttachmentFilename
-		}
-		safeFilename := filepath.Base(filename)
-		if safeFilename == "" || safeFilename == "." || safeFilename == ".." {
-			safeFilename = defaultGmailAttachmentFilename
-		}
+		safeFilename := sanitizeAttachmentFilename(c.Name)
 		shortID := attachmentID
 		if len(shortID) > 8 {
 			shortID = shortID[:8]
 		}
 		destPath = filepath.Join(dir, fmt.Sprintf("%s_%s_%s", messageID, shortID, safeFilename))
 	} else {
-		outPath, err := config.ExpandPath(outPathFlag)
+		outPath, err := resolveAttachmentOutPath(outPathFlag, c.Name)
 		if err != nil {
 			return err
 		}
@@ -80,19 +109,12 @@ func (c *GmailAttachmentCmd) Run(ctx context.Context, flags *RootFlags) error {
 		return err
 	}
 
-	if strings.TrimSpace(c.Output.Path) == "" {
+	if outPathFlag == "" {
 		dir, dirErr := config.EnsureGmailAttachmentsDir()
 		if dirErr != nil {
 			return dirErr
 		}
-		filename := strings.TrimSpace(c.Name)
-		if filename == "" {
-			filename = defaultGmailAttachmentFilename
-		}
-		safeFilename := filepath.Base(filename)
-		if safeFilename == "" || safeFilename == "." || safeFilename == ".." {
-			safeFilename = defaultGmailAttachmentFilename
-		}
+		safeFilename := sanitizeAttachmentFilename(c.Name)
 		shortID := attachmentID
 		if len(shortID) > 8 {
 			shortID = shortID[:8]
@@ -111,7 +133,7 @@ func (c *GmailAttachmentCmd) Run(ctx context.Context, flags *RootFlags) error {
 		return nil
 	}
 
-	outPath, err := config.ExpandPath(c.Output.Path)
+	outPath, err := resolveAttachmentOutPath(outPathFlag, c.Name)
 	if err != nil {
 		return err
 	}
@@ -141,11 +163,11 @@ func downloadAttachmentToPath(
 	}
 
 	if expectedSize > 0 {
-		if st, err := os.Stat(outPath); err == nil && st.Size() == expectedSize {
+		if st, err := os.Stat(outPath); err == nil && st.Mode().IsRegular() && st.Size() == expectedSize {
 			return outPath, true, st.Size(), nil
 		}
 	} else if expectedSize == -1 {
-		if st, err := os.Stat(outPath); err == nil && st.Size() > 0 {
+		if st, err := os.Stat(outPath); err == nil && st.Mode().IsRegular() && st.Size() > 0 {
 			return outPath, true, st.Size(), nil
 		}
 	}

internal/cmd/gmail_attachment_more_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/base64"
 	"encoding/json"
+	"errors"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -13,6 +14,8 @@ import (
 
 	"google.golang.org/api/gmail/v1"
 	"google.golang.org/api/option"
+
+	"github.com/steipete/gogcli/internal/outfmt"
 )
 
 func TestDownloadAttachmentToPath_MissingOutPath(t *testing.T) {
@@ -94,6 +97,120 @@ func TestDownloadAttachmentToPath_EmptyData(t *testing.T) {
 	}
 }
 
+func TestDownloadAttachmentToPath_DirectoryNotCached(t *testing.T) {
+	dir := t.TempDir()
+	// A directory should not be treated as a cached attachment even though
+	// os.Stat succeeds and Size() > 0 on directories.
+	srv := httptestServerForAttachment(t, base64.RawURLEncoding.EncodeToString([]byte("data")))
+	gsvc, err := gmail.NewService(context.Background(),
+		option.WithoutAuthentication(),
+		option.WithHTTPClient(srv.Client()),
+		option.WithEndpoint(srv.URL+"/"),
+	)
+	if err != nil {
+		t.Fatalf("NewService: %v", err)
+	}
+
+	outPath := filepath.Join(dir, "subdir")
+	if err := os.Mkdir(outPath, 0o700); err != nil {
+		t.Fatalf("Mkdir: %v", err)
+	}
+
+	// With expectedSize == -1, a directory should NOT be returned as cached.
+	// Instead it should attempt download and fail trying to write to the dir path.
+	_, _, _, dlErr := downloadAttachmentToPath(context.Background(), gsvc, "m1", "a1", outPath, -1)
+	// We expect an error because outPath is a directory and WriteFile to a dir fails.
+	if dlErr == nil {
+		t.Fatalf("expected error when outPath is a directory")
+	}
+}
+
+func TestDownloadAttachmentToPath_DirectoryNotCachedBySize(t *testing.T) {
+	dir := t.TempDir()
+	// Even with a positive expectedSize matching the directory's metadata
+	// size, a directory should not be treated as a cached file.
+	srv := httptestServerForAttachment(t, base64.RawURLEncoding.EncodeToString([]byte("data")))
+	gsvc, err := gmail.NewService(context.Background(),
+		option.WithoutAuthentication(),
+		option.WithHTTPClient(srv.Client()),
+		option.WithEndpoint(srv.URL+"/"),
+	)
+	if err != nil {
+		t.Fatalf("NewService: %v", err)
+	}
+
+	// Pass the directory size as expectedSize - should NOT match because
+	// the path is a directory, not a regular file.
+	info, _ := os.Stat(dir)
+	_, cached, _, dlErr := downloadAttachmentToPath(context.Background(), gsvc, "m1", "a1", dir, info.Size())
+	// It should not return cached=true; it will try to download and fail
+	// because WriteFile to a directory fails.
+	if dlErr == nil && cached {
+		t.Fatalf("directory should not be treated as cached file")
+	}
+}
+
+func TestGmailAttachmentCmd_DryRun_OutDir_UsesName(t *testing.T) {
+	outDir := t.TempDir()
+	ctx := outfmt.WithMode(context.Background(), outfmt.Mode{JSON: true})
+
+	out := captureStdout(t, func() {
+		err := runKong(t, &GmailAttachmentCmd{}, []string{"m1", "a1", "--out", outDir, "--name", "invoice.pdf"}, ctx, &RootFlags{DryRun: true})
+		var exitErr *ExitError
+		if !errors.As(err, &exitErr) || exitErr.Code != 0 {
+			t.Fatalf("expected exit code 0, got: %v", err)
+		}
+	})
+
+	var got map[string]any
+	if err := json.Unmarshal([]byte(out), &got); err != nil {
+		t.Fatalf("unmarshal: %v\noutput=%q", err, out)
+	}
+	req, ok := got["request"].(map[string]any)
+	if !ok {
+		t.Fatalf("expected request object, got=%T", got["request"])
+	}
+	path, ok := req["path"].(string)
+	if !ok {
+		t.Fatalf("expected request.path string, got=%T", req["path"])
+	}
+	want := filepath.Join(outDir, "invoice.pdf")
+	if path != want {
+		t.Fatalf("unexpected path: got=%q want=%q", path, want)
+	}
+}
+
+func TestGmailAttachmentCmd_DryRun_OutDirTrailingSlash_UsesNameEvenIfMissing(t *testing.T) {
+	base := t.TempDir()
+	outDir := filepath.Join(base, "newdir") + string(os.PathSeparator)
+	ctx := outfmt.WithMode(context.Background(), outfmt.Mode{JSON: true})
+
+	out := captureStdout(t, func() {
+		err := runKong(t, &GmailAttachmentCmd{}, []string{"m1", "a1", "--out", outDir, "--name", "invoice.pdf"}, ctx, &RootFlags{DryRun: true})
+		var exitErr *ExitError
+		if !errors.As(err, &exitErr) || exitErr.Code != 0 {
+			t.Fatalf("expected exit code 0, got: %v", err)
+		}
+	})
+
+	var got map[string]any
+	if err := json.Unmarshal([]byte(out), &got); err != nil {
+		t.Fatalf("unmarshal: %v\noutput=%q", err, out)
+	}
+	req, ok := got["request"].(map[string]any)
+	if !ok {
+		t.Fatalf("expected request object, got=%T", got["request"])
+	}
+	path, ok := req["path"].(string)
+	if !ok {
+		t.Fatalf("expected request.path string, got=%T", req["path"])
+	}
+	want := filepath.Join(filepath.Join(base, "newdir"), "invoice.pdf")
+	if path != want {
+		t.Fatalf("unexpected path: got=%q want=%q", path, want)
+	}
+}
+
 func httptestServerForAttachment(t *testing.T, data string) *httptest.Server {
 	t.Helper()
 	return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {