feat: unify minute artifacts output to ./minutes/{minute_token}/ (#604)

* feat: unify minute artifacts output to ./minutes/{minute_token}/

* fix: tighten path validation and batch-mode --output rejection

* style: translate comments to english and trim historical context

* style: translate leftover chinese comments in vc_notes

* refactor: address review findings across validate ordering, error types, JSON, tests

* fix: sanitize server-provided filename to prevent escape from artifact dir

* style: tighten flag help text for minutes/vc output flags

* docs: update minutes/vc skill docs for unified artifact layout

Author: Ren1104

shortcuts/common/artifact_path.go

@@ -0,0 +1,30 @@
+// Copyright (c) 2026 Lark Technologies Pte. Ltd.
+// SPDX-License-Identifier: MIT
+
+// This file defines artifact-path conventions shared between
+// `minutes +download` and `vc +notes`. Callers outside those two shortcuts
+// should not take a dependency on these symbols.
+
+package common
+
+import "path/filepath"
+
+// DefaultMinuteArtifactSubdir is the top-level directory for minute-scoped
+// artifacts under the default layout.
+const DefaultMinuteArtifactSubdir = "minutes"
+
+// DefaultTranscriptFileName is the fixed transcript filename under the
+// default layout. Recording files keep the server-provided name.
+const DefaultTranscriptFileName = "transcript.txt"
+
+// ArtifactTypeRecording is the artifact_type value emitted by
+// `minutes +download` so that callers can index results by kind without
+// parsing saved_path.
+const ArtifactTypeRecording = "recording"
+
+// DefaultMinuteArtifactDir returns the default output directory for an
+// artifact keyed by minuteToken. The same path is shared across commands so
+// that related artifacts of one meeting land together.
+func DefaultMinuteArtifactDir(minuteToken string) string {
+	return filepath.Join(DefaultMinuteArtifactSubdir, minuteToken)
+}

shortcuts/common/validate.go

@@ -83,13 +83,12 @@ func ParseIntBounded(rt *RuntimeContext, name string, min, max int) int {
 	return v
 }
 
-// ValidateSafeOutputDir ensures outputDir is a relative path that resolves
-// within the current working directory, preventing path traversal attacks
-// (including symlink-based escape).
-// It delegates all validation to FileIO.ResolvePath which already performs
-// cwd-boundary checks, symlink resolution, and control-character rejection.
-func ValidateSafeOutputDir(fio fileio.FileIO, outputDir string) error {
-	_, err := fio.ResolvePath(outputDir)
+// ValidateSafePath ensures path is relative and resolves within the current
+// working directory. It catches traversal, symlink escape, and control
+// characters by delegating to FileIO.ResolvePath. Works for both file and
+// directory paths.
+func ValidateSafePath(fio fileio.FileIO, path string) error {
+	_, err := fio.ResolvePath(path)
 	return err
 }
 

shortcuts/common/validate_test.go

@@ -172,7 +172,7 @@ func TestParseIntBounded(t *testing.T) {
 }
 
 // ---------------------------------------------------------------------------
-// ValidateSafeOutputDir — symlink escape prevention
+// ValidateSafePath — symlink escape prevention
 // ---------------------------------------------------------------------------
 
 // chdirForTest changes CWD to dir and restores the original CWD on cleanup.
@@ -188,9 +188,9 @@ func chdirForTest(t *testing.T, dir string) {
 	t.Cleanup(func() { os.Chdir(orig) })
 }
 
-// TestValidateSafeOutputDir_RejectsSymlinkEscape verifies that a relative path
+// TestValidateSafePath_RejectsSymlinkEscape verifies that a relative path
 // that resolves to a symlink pointing outside CWD is rejected.
-func TestValidateSafeOutputDir_RejectsSymlinkEscape(t *testing.T) {
+func TestValidateSafePath_RejectsSymlinkEscape(t *testing.T) {
 	outside := t.TempDir() // target outside CWD
 	workDir := t.TempDir()
 	chdirForTest(t, workDir)
@@ -200,29 +200,29 @@ func TestValidateSafeOutputDir_RejectsSymlinkEscape(t *testing.T) {
 		t.Fatalf("Symlink: %v", err)
 	}
 
-	if err := ValidateSafeOutputDir(&localfileio.LocalFileIO{}, "evil_out"); err == nil {
+	if err := ValidateSafePath(&localfileio.LocalFileIO{}, "evil_out"); err == nil {
 		t.Fatal("expected error for symlink pointing outside CWD, got nil")
 	}
 }
 
-// TestValidateSafeOutputDir_RejectsDanglingSymlink verifies that a dangling
+// TestValidateSafePath_RejectsDanglingSymlink verifies that a dangling
 // symlink (target does not exist) is rejected to prevent future escapes.
-func TestValidateSafeOutputDir_RejectsDanglingSymlink(t *testing.T) {
+func TestValidateSafePath_RejectsDanglingSymlink(t *testing.T) {
 	workDir := t.TempDir()
 	chdirForTest(t, workDir)
 
 	if err := os.Symlink("/nonexistent/outside/target", filepath.Join(workDir, "dangling")); err != nil {
 		t.Fatalf("Symlink: %v", err)
 	}
 
-	if err := ValidateSafeOutputDir(&localfileio.LocalFileIO{}, "dangling"); err == nil {
+	if err := ValidateSafePath(&localfileio.LocalFileIO{}, "dangling"); err == nil {
 		t.Fatal("expected error for dangling symlink, got nil")
 	}
 }
 
-// TestValidateSafeOutputDir_AllowsNormalSubdir verifies that an existing real
+// TestValidateSafePath_AllowsNormalSubdir verifies that an existing real
 // subdirectory within CWD is accepted.
-func TestValidateSafeOutputDir_AllowsNormalSubdir(t *testing.T) {
+func TestValidateSafePath_AllowsNormalSubdir(t *testing.T) {
 	workDir := t.TempDir()
 	chdirForTest(t, workDir)
 
@@ -231,18 +231,18 @@ func TestValidateSafeOutputDir_AllowsNormalSubdir(t *testing.T) {
 		t.Fatalf("Mkdir: %v", err)
 	}
 
-	if err := ValidateSafeOutputDir(&localfileio.LocalFileIO{}, "output"); err != nil {
+	if err := ValidateSafePath(&localfileio.LocalFileIO{}, "output"); err != nil {
 		t.Fatalf("expected no error for real subdir, got: %v", err)
 	}
 }
 
-// TestValidateSafeOutputDir_AllowsNonExistentPath verifies that a path that
+// TestValidateSafePath_AllowsNonExistentPath verifies that a path that
 // does not yet exist (new output directory) is accepted.
-func TestValidateSafeOutputDir_AllowsNonExistentPath(t *testing.T) {
+func TestValidateSafePath_AllowsNonExistentPath(t *testing.T) {
 	workDir := t.TempDir()
 	chdirForTest(t, workDir)
 
-	if err := ValidateSafeOutputDir(&localfileio.LocalFileIO{}, "new_output_dir"); err != nil {
+	if err := ValidateSafePath(&localfileio.LocalFileIO{}, "new_output_dir"); err != nil {
 		t.Fatalf("expected no error for non-existent path, got: %v", err)
 	}
 }

shortcuts/minutes/minutes_download.go

@@ -5,10 +5,13 @@ package minutes
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"mime"
 	"net/http"
+	"path"
 	"path/filepath"
 	"regexp"
 	"strings"
@@ -43,7 +46,8 @@ var MinutesDownload = common.Shortcut{
 	HasFormat:   true,
 	Flags: []common.Flag{
 		{Name: "minute-tokens", Desc: "minute tokens, comma-separated for batch download (max 50)", Required: true},
-		{Name: "output", Desc: "output path: file path for single token, directory for batch (default: current dir)"},
+		{Name: "output", Desc: "output file path (single token)"},
+		{Name: "output-dir", Desc: "output directory (default: ./minutes/{minute_token}/)"},
 		{Name: "overwrite", Type: "bool", Desc: "overwrite existing output file"},
 		{Name: "url-only", Type: "bool", Desc: "only print the download URL(s) without downloading"},
 	},
@@ -60,6 +64,22 @@ var MinutesDownload = common.Shortcut{
 				return output.ErrValidation("invalid minute token %q: must contain only lowercase alphanumeric characters (e.g. obcnq3b9jl72l83w4f149w9c)", token)
 			}
 		}
+		// Cheap checks first, then path-safety resolution.
+		out := runtime.Str("output")
+		outDir := runtime.Str("output-dir")
+		if out != "" && outDir != "" {
+			return output.ErrValidation("--output and --output-dir cannot both be set")
+		}
+		if out != "" {
+			if err := common.ValidateSafePath(runtime.FileIO(), out); err != nil {
+				return err
+			}
+		}
+		if outDir != "" {
+			if err := common.ValidateSafePath(runtime.FileIO(), outDir); err != nil {
+				return err
+			}
+		}
 		return nil
 	},
 	DryRun: func(ctx context.Context, runtime *common.RuntimeContext) *common.DryRunAPI {
@@ -70,29 +90,53 @@ var MinutesDownload = common.Shortcut{
 	},
 	Execute: func(ctx context.Context, runtime *common.RuntimeContext) error {
 		tokens := common.SplitCSV(runtime.Str("minute-tokens"))
-		outputPath := runtime.Str("output")
+		rawOutput := runtime.Str("output")
+		rawOutputDir := runtime.Str("output-dir")
 		overwrite := runtime.Bool("overwrite")
 		urlOnly := runtime.Bool("url-only")
 		errOut := runtime.IO().ErrOut
 		single := len(tokens) == 1
 
-		// Batch mode: --output must be a directory, not an existing file.
-		if !single && outputPath != "" {
-			if fi, err := runtime.FileIO().Stat(outputPath); err == nil && !fi.IsDir() {
-				return output.ErrValidation("--output %q is a file; batch mode expects a directory path", outputPath)
+		// Re-interpret --output based on what the path points to. An existing
+		// directory is promoted to --output-dir so single-token cp semantics
+		// work. An existing file is rejected in batch mode (the flag carries
+		// directory semantics there). Unknown filesystem errors are surfaced
+		// eagerly rather than deferred to Save.
+		explicitOutputPath := rawOutput
+		explicitOutputDir := rawOutputDir
+		if explicitOutputPath != "" {
+			fi, statErr := runtime.FileIO().Stat(explicitOutputPath)
+			switch {
+			case statErr == nil && fi.IsDir():
+				explicitOutputDir = explicitOutputPath
+				explicitOutputPath = ""
+			case statErr == nil && !fi.IsDir():
+				if !single {
+					return output.ErrValidation("--output %q is a file; batch mode expects a directory (use --output-dir)", explicitOutputPath)
+				}
+			case errors.Is(statErr, fs.ErrNotExist):
+				if !single {
+					explicitOutputDir = explicitOutputPath
+					explicitOutputPath = ""
+				}
+			default:
+				return output.Errorf(output.ExitAPI, "io_error", "cannot access --output %q: %s", explicitOutputPath, statErr)
 			}
 		}
 
+		useDefaultLayout := explicitOutputPath == "" && explicitOutputDir == ""
+
 		if !single {
 			fmt.Fprintf(errOut, "[minutes +download] batch: %d token(s)\n", len(tokens))
 		}
 
 		type result struct {
-			MinuteToken string `json:"minute_token"`
-			SavedPath   string `json:"saved_path,omitempty"`
-			SizeBytes   int64  `json:"size_bytes,omitempty"`
-			DownloadURL string `json:"download_url,omitempty"`
-			Error       string `json:"error,omitempty"`
+			MinuteToken  string `json:"minute_token"`
+			ArtifactType string `json:"artifact_type,omitempty"`
+			SavedPath    string `json:"saved_path,omitempty"`
+			SizeBytes    int64  `json:"size_bytes,omitempty"`
+			DownloadURL  string `json:"download_url,omitempty"`
+			Error        string `json:"error,omitempty"`
 		}
 
 		results := make([]result, len(tokens))
@@ -160,20 +204,31 @@ var MinutesDownload = common.Shortcut{
 
 			fmt.Fprintf(errOut, "Downloading media: %s\n", common.MaskToken(token))
 
-			// single token: --output is a file path; batch: --output is a directory
-			opts := downloadOpts{fio: runtime.FileIO(), overwrite: overwrite, usedNames: usedNames}
-			if single {
-				opts.outputPath = outputPath
-			} else {
-				opts.outputDir = outputPath
+			opts := downloadOpts{fio: runtime.FileIO(), overwrite: overwrite}
+			switch {
+			case useDefaultLayout:
+				// Per-token subdirectory guarantees unique paths, so no dedup map.
+				opts.outputDir = common.DefaultMinuteArtifactDir(token)
+			case explicitOutputPath != "" && single:
+				opts.outputPath = explicitOutputPath
+			default:
+				opts.outputDir = explicitOutputDir
+				if !single {
+					opts.usedNames = usedNames
+				}
 			}
 
 			dl, err := downloadMediaFile(ctx, dlClient, downloadURL, token, opts)
 			if err != nil {
 				results[i] = result{MinuteToken: token, Error: err.Error()}
 				continue
 			}
-			results[i] = result{MinuteToken: token, SavedPath: dl.savedPath, SizeBytes: dl.sizeBytes}
+			results[i] = result{
+				MinuteToken:  token,
+				ArtifactType: common.ArtifactTypeRecording,
+				SavedPath:    dl.savedPath,
+				SizeBytes:    dl.sizeBytes,
+			}
 		}
 
 		// output
@@ -183,9 +238,17 @@ var MinutesDownload = common.Shortcut{
 				return output.ErrAPI(0, r.Error, nil)
 			}
 			if urlOnly {
-				runtime.Out(map[string]interface{}{"download_url": r.DownloadURL}, nil)
+				runtime.Out(map[string]interface{}{
+					"minute_token": r.MinuteToken,
+					"download_url": r.DownloadURL,
+				}, nil)
 			} else {
-				runtime.Out(map[string]interface{}{"saved_path": r.SavedPath, "size_bytes": r.SizeBytes}, nil)
+				runtime.Out(map[string]interface{}{
+					"minute_token":  r.MinuteToken,
+					"artifact_type": r.ArtifactType,
+					"saved_path":    r.SavedPath,
+					"size_bytes":    r.SizeBytes,
+				}, nil)
 			}
 			return nil
 		}
@@ -230,7 +293,7 @@ type downloadResult struct {
 type downloadOpts struct {
 	fio        fileio.FileIO // file I/O abstraction
 	outputPath string        // explicit output file path (single mode only)
-	outputDir  string        // output directory (batch mode)
+	outputDir  string        // output directory (single or batch)
 	overwrite  bool
 	usedNames  map[string]bool // tracks used filenames to deduplicate in batch mode
 }
@@ -300,7 +363,7 @@ func downloadMediaFile(ctx context.Context, client *http.Client, downloadURL, mi
 func resolveFilenameFromResponse(resp *http.Response, minuteToken string) string {
 	if cd := resp.Header.Get("Content-Disposition"); cd != "" {
 		if _, params, err := mime.ParseMediaType(cd); err == nil {
-			if filename := params["filename"]; filename != "" {
+			if filename := sanitizeServerFilename(params["filename"]); filename != "" {
 				return filename
 			}
 		}
@@ -311,6 +374,20 @@ func resolveFilenameFromResponse(resp *http.Response, minuteToken string) string
 	return minuteToken + ".media"
 }
 
+// sanitizeServerFilename reduces a server-provided filename to its basename,
+// defending against Content-Disposition payloads that embed directory
+// separators (e.g. "../other.mp4") and would otherwise escape the intended
+// artifact directory after filepath.Join. Empty or dot-only names return ""
+// so the caller can fall back to the next naming strategy.
+func sanitizeServerFilename(filename string) string {
+	filename = strings.ReplaceAll(filename, "\\", "/")
+	filename = path.Base(filename)
+	if filename == "" || filename == "." || filename == ".." {
+		return ""
+	}
+	return filename
+}
+
 // preferredExt overrides Go's mime.ExtensionsByType which returns alphabetically sorted
 // results (e.g. .m4v before .mp4 for video/mp4).
 var preferredExt = map[string]string{