From f03d597f157dcfd1981249f7ea025aa56899b349 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Tue, 31 Mar 2026 20:03:20 -0400 Subject: [PATCH 01/31] feat(page): upload standalone local images via api Detect standalone local markdown image lines during page upload and sync, upload those files through the official Notion API, and substitute them back into the page in document order. Reject inline local image syntax instead of silently dropping it. --- README.md | 7 ++ cmd/page.go | 70 ++++++++++-- cmd/page_local_images.go | 143 ++++++++++++++++++++++++ cmd/page_local_images_test.go | 109 +++++++++++++++++++ internal/cli/local_images.go | 175 ++++++++++++++++++++++++++++++ internal/cli/local_images_test.go | 60 ++++++++++ skills/notion/SKILL.md | 18 ++- 7 files changed, 572 insertions(+), 10 deletions(-) create mode 100644 cmd/page_local_images.go create mode 100644 cmd/page_local_images_test.go create mode 100644 internal/cli/local_images.go create mode 100644 internal/cli/local_images_test.go diff --git a/README.md b/README.md index 5f51279..af04ea7 100644 --- a/README.md +++ b/README.md @@ -86,12 +86,14 @@ notion-cli page upload ./document.md --title "Custom Title" # Explicit title notion-cli page upload ./document.md --parent "Engineering" # Parent by name or ID notion-cli page upload ./document.md --parent-db # Upload as database entry notion-cli page upload ./document.md --icon "📄" # Set emoji icon +notion-cli page upload ./document.md # Uploads standalone local images when configured # Sync a markdown file (create or update) notion-cli page sync ./document.md # Creates page, writes notion-id to frontmatter notion-cli page sync ./document.md # Updates page using notion-id from frontmatter notion-cli page sync ./document.md --parent "Engineering" # Set parent on first sync notion-cli page sync ./document.md --parent-db # Sync as database entry +notion-cli page sync ./document.md # Uploads standalone local images when configured # Edit an existing page notion-cli page edit --replace "New content" # Replace all content @@ -105,6 +107,8 @@ The `` argument accepts a URL, ID, or page name. `page view` shows open page-level comments and inline block discussions by default. Inline discussions are rendered in context, with the anchor text wrapped in `[[...]]` and the discussion shown immediately below it. Use `--no-comments` to suppress comments, `--raw` to inspect the original Notion markup, and `--json` to return the page plus a `Comments` array. +`page upload` and `page sync` support native local image upload for standalone markdown image lines like `![Alt](./diagram.png)`. When local images are present, `notion-cli` uploads those files through the official Notion API and keeps them in document order. This requires an official API token configured through `auth api setup` or `NOTION_API_TOKEN`. Inline or mixed-content local image syntax is rejected instead of being guessed. + ### Search ```bash @@ -170,6 +174,9 @@ The CLI uses Notion's remote MCP server with OAuth authentication. On first run, | Variable | Description | |----------|-------------| | `NOTION_ACCESS_TOKEN` | Access token for CI/headless usage (skips OAuth) | +| `NOTION_API_TOKEN` | Official Notion API token used for upload fallback and verification | +| `NOTION_API_BASE_URL` | Override the official Notion API base URL | +| `NOTION_API_NOTION_VERSION` | Override the official Notion API version | ## How It Works diff --git a/cmd/page.go b/cmd/page.go index 8f306a4..c56ff01 100644 --- a/cmd/page.go +++ b/cmd/page.go @@ -8,6 +8,7 @@ import ( "path/filepath" "strings" + "github.com/lox/notion-cli/internal/api" "github.com/lox/notion-cli/internal/cli" "github.com/lox/notion-cli/internal/mcp" "github.com/lox/notion-cli/internal/output" @@ -270,6 +271,12 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string) err } markdown := string(content) + bgCtx := context.Background() + markdown, localUploads, err := prepareLocalImageUploads(bgCtx, file, markdown) + if err != nil { + output.PrintError(err) + return err + } if title == "" { title = extractTitleFromMarkdown(markdown) @@ -288,8 +295,6 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string) err } defer func() { _ = client.Close() }() - bgCtx := context.Background() - req := mcp.CreatePageRequest{ Title: title, Content: markdown, @@ -321,6 +326,19 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string) err output.PrintError(err) return err } + pageID := pageIDFromCreateResponse(resp) + if err := substituteUploadedLocalImages(bgCtx, pageID, localUploads); err != nil { + finalErr := fmt.Errorf("insert uploaded local images: %w", err) + if pageID != "" { + if apiClient, apiErr := cli.RequireOfficialAPIClient(); apiErr == nil { + if cleanupErr := apiClient.TrashPage(bgCtx, pageID); cleanupErr != nil { + finalErr = fmt.Errorf("%w (cleanup failed: %v)", finalErr, cleanupErr) + } + } + } + output.PrintError(finalErr) + return finalErr + } displayTitle := title if icon != "" { @@ -329,7 +347,7 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string) err if ctx.JSON { outPage := output.Page{ - ID: resp.ID, + ID: pageID, URL: resp.URL, Title: displayTitle, Icon: icon, @@ -535,6 +553,12 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string) error content := string(raw) fm, body := cli.ParseFrontmatter(content) + bgCtx := context.Background() + body, localUploads, err := prepareLocalImageUploads(bgCtx, file, body) + if err != nil { + output.PrintError(err) + return err + } if title == "" { title = extractTitleFromMarkdown(body) @@ -552,9 +576,21 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string) error } defer func() { _ = client.Close() }() - bgCtx := context.Background() - if fm.NotionID != "" { + var snapshot *api.PageMarkdown + if len(localUploads) > 0 { + apiClient, err := cli.RequireOfficialAPIClient() + if err != nil { + output.PrintError(err) + return err + } + snapshot, err = apiClient.GetPageMarkdown(bgCtx, fm.NotionID) + if err != nil { + output.PrintError(err) + return err + } + } + req := mcp.UpdatePageRequest{ PageID: fm.NotionID, Command: "replace_content", @@ -564,6 +600,15 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string) error output.PrintError(err) return err } + if err := substituteUploadedLocalImages(bgCtx, fm.NotionID, localUploads); err != nil { + finalErr := fmt.Errorf("insert uploaded local images: %w", err) + rollbackErr := rollbackSyncedPage(bgCtx, client, fm.NotionID, snapshot) + if rollbackErr != nil { + finalErr = fmt.Errorf("%w (rollback failed: %v)", finalErr, rollbackErr) + } + output.PrintError(finalErr) + return finalErr + } displayTitle := title if icon != "" { @@ -615,9 +660,18 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string) error return err } - pageID := resp.ID - if pageID == "" && resp.URL != "" { - pageID, _ = cli.ExtractNotionUUID(resp.URL) + pageID := pageIDFromCreateResponse(resp) + if err := substituteUploadedLocalImages(bgCtx, pageID, localUploads); err != nil { + finalErr := fmt.Errorf("insert uploaded local images: %w", err) + if pageID != "" { + if apiClient, apiErr := cli.RequireOfficialAPIClient(); apiErr == nil { + if cleanupErr := apiClient.TrashPage(bgCtx, pageID); cleanupErr != nil { + finalErr = fmt.Errorf("%w (cleanup failed: %v)", finalErr, cleanupErr) + } + } + } + output.PrintError(finalErr) + return finalErr } if pageID == "" { output.PrintWarning("Page created but could not retrieve ID for frontmatter") diff --git a/cmd/page_local_images.go b/cmd/page_local_images.go new file mode 100644 index 0000000..c46c68c --- /dev/null +++ b/cmd/page_local_images.go @@ -0,0 +1,143 @@ +package cmd + +import ( + "context" + "fmt" + "os" + "strings" + + "github.com/lox/notion-cli/internal/api" + "github.com/lox/notion-cli/internal/cli" + "github.com/lox/notion-cli/internal/mcp" +) + +type uploadedLocalImage struct { + Alt string + FileUploadID string + Placeholder string + ResolvedPath string +} + +func prepareLocalImageUploads(ctx context.Context, sourceFile, markdown string) (string, []uploadedLocalImage, error) { + rewritten, placements, err := cli.RewriteStandaloneLocalImages(markdown, sourceFile) + if err != nil { + return "", nil, err + } + if len(placements) == 0 { + return markdown, nil, nil + } + + apiClient, err := cli.RequireOfficialAPIClient() + if err != nil { + return "", nil, err + } + + uploadIDByPath := make(map[string]string, len(placements)) + uploads := make([]uploadedLocalImage, 0, len(placements)) + for _, placement := range placements { + uploadID, ok := uploadIDByPath[placement.Resolved] + if !ok { + fileData, err := os.ReadFile(placement.Resolved) + if err != nil { + return "", nil, fmt.Errorf("read local image %q: %w", placement.Resolved, err) + } + uploadID, err = apiClient.UploadFile(ctx, placement.Resolved, fileData) + if err != nil { + return "", nil, fmt.Errorf("upload local image %q: %w", placement.Resolved, err) + } + uploadIDByPath[placement.Resolved] = uploadID + } + + uploads = append(uploads, uploadedLocalImage{ + Alt: placement.Alt, + FileUploadID: uploadID, + Placeholder: placement.Placeholder, + ResolvedPath: placement.Resolved, + }) + } + + return rewritten, uploads, nil +} + +func substituteUploadedLocalImages(ctx context.Context, pageID string, uploads []uploadedLocalImage) error { + if strings.TrimSpace(pageID) == "" || len(uploads) == 0 { + return nil + } + + apiClient, err := cli.RequireOfficialAPIClient() + if err != nil { + return err + } + + blocks, err := apiClient.ListAllBlockChildren(ctx, pageID) + if err != nil { + return err + } + + blocksByPlaceholder := make(map[string]api.Block, len(uploads)) + for _, block := range blocks { + if block.Type != "paragraph" || block.Paragraph == nil { + continue + } + text := paragraphPlainText(block) + if text == "" { + continue + } + blocksByPlaceholder[text] = block + } + + for _, upload := range uploads { + block, ok := blocksByPlaceholder[upload.Placeholder] + if !ok { + return fmt.Errorf("could not find placeholder block for %q", upload.ResolvedPath) + } + if err := apiClient.AppendUploadedImageAfter(ctx, pageID, block.ID, api.UploadedImageBlock{ + FileUploadID: upload.FileUploadID, + Caption: upload.Alt, + }); err != nil { + return err + } + if err := apiClient.DeleteBlock(ctx, block.ID); err != nil { + return err + } + } + + return nil +} + +func rollbackSyncedPage(ctx context.Context, client *mcp.Client, pageID string, snapshot *api.PageMarkdown) error { + if snapshot == nil || strings.TrimSpace(snapshot.Markdown) == "" { + return nil + } + return client.UpdatePage(ctx, mcp.UpdatePageRequest{ + PageID: pageID, + Command: "replace_content", + NewContent: snapshot.Markdown, + }) +} + +func pageIDFromCreateResponse(resp *mcp.CreatePageResponse) string { + if resp == nil { + return "" + } + if strings.TrimSpace(resp.ID) != "" { + return strings.TrimSpace(resp.ID) + } + if strings.TrimSpace(resp.URL) == "" { + return "" + } + id, _ := cli.ExtractNotionUUID(resp.URL) + return id +} + +func paragraphPlainText(block api.Block) string { + if block.Paragraph == nil { + return "" + } + + var builder strings.Builder + for _, part := range block.Paragraph.RichText { + builder.WriteString(part.PlainText) + } + return builder.String() +} diff --git a/cmd/page_local_images_test.go b/cmd/page_local_images_test.go new file mode 100644 index 0000000..4ac8975 --- /dev/null +++ b/cmd/page_local_images_test.go @@ -0,0 +1,109 @@ +package cmd + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestPrepareLocalImageUploadsUploadsAndDeduplicates(t *testing.T) { + tmp := t.TempDir() + doc := filepath.Join(tmp, "doc.md") + img := filepath.Join(tmp, "diagram.png") + if err := os.WriteFile(img, []byte("PNGDATA"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + createCalls := 0 + sendCalls := 0 + getCalls := 0 + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodPost && r.URL.Path == "/v1/file_uploads": + createCalls++ + _, _ = w.Write([]byte(`{"id":"upload_123","status":"pending"}`)) + case r.Method == http.MethodPost && r.URL.Path == "/v1/file_uploads/upload_123/send": + sendCalls++ + _, _ = w.Write([]byte(`{"id":"upload_123","status":"uploaded"}`)) + case r.Method == http.MethodGet && r.URL.Path == "/v1/file_uploads/upload_123": + getCalls++ + _, _ = w.Write([]byte(`{"id":"upload_123","status":"uploaded"}`)) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + })) + defer srv.Close() + + t.Setenv("HOME", t.TempDir()) + t.Setenv("NOTION_API_BASE_URL", srv.URL+"/v1") + t.Setenv("NOTION_API_TOKEN", "secret-token") + + rewritten, uploads, err := prepareLocalImageUploads(context.Background(), doc, "![One](./diagram.png)\n![Two](./diagram.png)\n") + if err != nil { + t.Fatalf("prepareLocalImageUploads: %v", err) + } + if len(uploads) != 2 { + t.Fatalf("len(uploads) = %d, want 2", len(uploads)) + } + if createCalls != 1 || sendCalls != 1 || getCalls != 1 { + t.Fatalf("unexpected call counts create=%d send=%d get=%d", createCalls, sendCalls, getCalls) + } + if uploads[0].FileUploadID != "upload_123" || uploads[1].FileUploadID != "upload_123" { + t.Fatalf("unexpected upload ids: %#v", uploads) + } + if !strings.Contains(rewritten, uploads[0].Placeholder) || !strings.Contains(rewritten, uploads[1].Placeholder) { + t.Fatalf("rewritten markdown missing placeholders: %q", rewritten) + } +} + +func TestSubstituteUploadedLocalImagesAppendsAfterPlaceholderAndDeletes(t *testing.T) { + var sawAppend bool + var sawDelete bool + + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && r.URL.Path == "/v1/blocks/page_123/children": + _, _ = w.Write([]byte(`{"results":[{"id":"block_123","type":"paragraph","paragraph":{"rich_text":[{"plain_text":"PLACEHOLDER"}]}}],"has_more":false}`)) + case r.Method == http.MethodPatch && r.URL.Path == "/v1/blocks/page_123/children": + sawAppend = true + defer func() { _ = r.Body.Close() }() + var payload map[string]any + if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { + t.Fatalf("Decode: %v", err) + } + if payload["after"] != "block_123" { + t.Fatalf("after = %#v", payload["after"]) + } + _, _ = w.Write([]byte(`{"results":[]}`)) + case r.Method == http.MethodDelete && r.URL.Path == "/v1/blocks/block_123": + sawDelete = true + w.WriteHeader(http.StatusOK) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + })) + defer srv.Close() + + t.Setenv("HOME", t.TempDir()) + t.Setenv("NOTION_API_BASE_URL", srv.URL+"/v1") + t.Setenv("NOTION_API_TOKEN", "secret-token") + + err := substituteUploadedLocalImages(context.Background(), "page_123", []uploadedLocalImage{{ + Alt: "Diagram", + FileUploadID: "upload_123", + Placeholder: "PLACEHOLDER", + ResolvedPath: "/tmp/diagram.png", + }}) + if err != nil { + t.Fatalf("substituteUploadedLocalImages: %v", err) + } + if !sawAppend || !sawDelete { + t.Fatalf("expected append and delete, saw append=%v delete=%v", sawAppend, sawDelete) + } +} diff --git a/internal/cli/local_images.go b/internal/cli/local_images.go new file mode 100644 index 0000000..7961200 --- /dev/null +++ b/internal/cli/local_images.go @@ -0,0 +1,175 @@ +package cli + +import ( + "fmt" + "net/url" + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/google/uuid" +) + +type LocalImagePlacement struct { + Alt string + Original string + Resolved string + Placeholder string +} + +var markdownImageRE = regexp.MustCompile(`!\[([^\]]*)\]\(([^)\n]+)\)`) +var standaloneMarkdownImageRE = regexp.MustCompile(`^\s*!\[([^\]]*)\]\(([^)\n]+)\)\s*$`) +var uriSchemeRE = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.-]*:`) + +func RewriteStandaloneLocalImages(markdown, sourceFile string) (string, []LocalImagePlacement, error) { + sourceFileAbs, err := filepath.Abs(sourceFile) + if err != nil { + return "", nil, fmt.Errorf("resolve source file path: %w", err) + } + sourceDir := filepath.Dir(sourceFileAbs) + + lines := strings.Split(markdown, "\n") + placements := make([]LocalImagePlacement, 0) + for i, line := range lines { + matches := markdownImageRE.FindAllStringSubmatch(line, -1) + if len(matches) == 0 { + continue + } + + standalone := standaloneMarkdownImageRE.FindStringSubmatch(line) + if standalone == nil { + for _, match := range matches { + dest, ok := parseMarkdownDestination(match[2]) + if ok && isLocalDestination(dest) { + return "", nil, fmt.Errorf("unsupported local image syntax on line %d: local images must appear on their own line", i+1) + } + } + continue + } + + dest, ok := parseMarkdownDestination(standalone[2]) + if !ok || !isLocalDestination(dest) { + continue + } + + resolvedPath, err := resolveLocalPath(dest, sourceDir) + if err != nil { + return "", nil, err + } + info, err := os.Stat(resolvedPath) + if err != nil { + return "", nil, fmt.Errorf("local image %q not found (from %s): %w", dest, sourceFile, err) + } + if info.IsDir() { + return "", nil, fmt.Errorf("local image %q resolves to a directory: %s", dest, resolvedPath) + } + + placeholder := "NOTION_CLI_LOCAL_IMAGE_" + strings.ReplaceAll(uuid.NewString(), "-", "_") + lines[i] = placeholder + placements = append(placements, LocalImagePlacement{ + Alt: standalone[1], + Original: dest, + Resolved: resolvedPath, + Placeholder: placeholder, + }) + } + + return strings.Join(lines, "\n"), placements, nil +} + +func parseMarkdownDestination(raw string) (string, bool) { + s := strings.TrimSpace(raw) + if s == "" { + return "", false + } + + if strings.HasPrefix(s, "<") { + end := strings.Index(s, ">") + if end > 1 { + return s[1:end], true + } + } + + escaped := false + for i, r := range s { + if escaped { + escaped = false + continue + } + if r == '\\' { + escaped = true + continue + } + if r == ' ' || r == '\t' || r == '\n' || r == '\r' { + s = s[:i] + break + } + } + + s = strings.TrimSpace(s) + if s == "" { + return "", false + } + return s, true +} + +func isLocalDestination(dest string) bool { + d := strings.TrimSpace(dest) + if d == "" { + return false + } + + if len(d) >= 2 && d[1] == ':' { + return true + } + + lower := strings.ToLower(d) + switch { + case strings.HasPrefix(lower, "#"): + return false + case strings.HasPrefix(lower, "http://"), + strings.HasPrefix(lower, "https://"), + strings.HasPrefix(lower, "mailto:"), + strings.HasPrefix(lower, "tel:"), + strings.HasPrefix(lower, "data:"): + return false + case strings.HasPrefix(lower, "file://"): + return true + } + + return !uriSchemeRE.MatchString(d) +} + +func resolveLocalPath(dest, sourceDir string) (string, error) { + d := strings.TrimSpace(dest) + if strings.HasPrefix(strings.ToLower(d), "file://") { + parsed, err := url.Parse(d) + if err != nil { + return "", fmt.Errorf("invalid file URL %q: %w", d, err) + } + unescaped, err := url.PathUnescape(parsed.Path) + if err != nil { + return "", fmt.Errorf("invalid file URL path %q: %w", d, err) + } + d = unescaped + } + + if strings.HasPrefix(d, "~"+string(filepath.Separator)) { + home, err := os.UserHomeDir() + if err != nil { + return "", fmt.Errorf("expand home path %q: %w", d, err) + } + d = filepath.Join(home, strings.TrimPrefix(d, "~"+string(filepath.Separator))) + } + + if !filepath.IsAbs(d) { + d = filepath.Join(sourceDir, d) + } + + abs, err := filepath.Abs(d) + if err != nil { + return "", fmt.Errorf("resolve local path %q: %w", dest, err) + } + return filepath.Clean(abs), nil +} diff --git a/internal/cli/local_images_test.go b/internal/cli/local_images_test.go new file mode 100644 index 0000000..39381e1 --- /dev/null +++ b/internal/cli/local_images_test.go @@ -0,0 +1,60 @@ +package cli + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestRewriteStandaloneLocalImagesRewritesStandaloneLocalLines(t *testing.T) { + tmp := t.TempDir() + doc := filepath.Join(tmp, "doc.md") + img := filepath.Join(tmp, "diagram.png") + if err := os.WriteFile(img, []byte("PNG"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + rewritten, placements, err := RewriteStandaloneLocalImages("# Title\n\n![Diagram](./diagram.png)\n\nDone\n", doc) + if err != nil { + t.Fatalf("RewriteStandaloneLocalImages: %v", err) + } + if len(placements) != 1 { + t.Fatalf("len(placements) = %d, want 1", len(placements)) + } + if !strings.Contains(rewritten, placements[0].Placeholder) { + t.Fatalf("rewritten markdown missing placeholder: %q", rewritten) + } + if placements[0].Resolved != img { + t.Fatalf("Resolved = %q, want %q", placements[0].Resolved, img) + } +} + +func TestRewriteStandaloneLocalImagesRejectsInlineLocalImage(t *testing.T) { + tmp := t.TempDir() + doc := filepath.Join(tmp, "doc.md") + img := filepath.Join(tmp, "diagram.png") + if err := os.WriteFile(img, []byte("PNG"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + _, _, err := RewriteStandaloneLocalImages("before ![Diagram](./diagram.png) after\n", doc) + if err == nil || !strings.Contains(err.Error(), "must appear on their own line") { + t.Fatalf("expected unsupported syntax error, got %v", err) + } +} + +func TestRewriteStandaloneLocalImagesIgnoresRemoteImages(t *testing.T) { + doc := filepath.Join(t.TempDir(), "doc.md") + + rewritten, placements, err := RewriteStandaloneLocalImages("![Diagram](https://example.test/diagram.png)\n", doc) + if err != nil { + t.Fatalf("RewriteStandaloneLocalImages: %v", err) + } + if len(placements) != 0 { + t.Fatalf("len(placements) = %d, want 0", len(placements)) + } + if rewritten != "![Diagram](https://example.test/diagram.png)\n" { + t.Fatalf("rewritten = %q", rewritten) + } +} diff --git a/skills/notion/SKILL.md b/skills/notion/SKILL.md index 8012dcb..0779f7b 100644 --- a/skills/notion/SKILL.md +++ b/skills/notion/SKILL.md @@ -26,7 +26,7 @@ Or see: https://github.com/lox/notion-cli ## Authentication -The CLI uses OAuth authentication. On first use, it opens a browser for authorization: +The CLI uses OAuth authentication for MCP-backed commands. On first use, it opens a browser for authorization: ```bash notion-cli auth login # Authenticate with Notion @@ -37,6 +37,17 @@ notion-cli auth logout # Clear credentials For CI/headless environments, set `NOTION_ACCESS_TOKEN` environment variable. +Some fallback features also use the official Notion API: + +```bash +notion-cli auth api setup +notion-cli auth api status +notion-cli auth api verify +notion-cli auth api unset +``` + +For CI/headless environments, set `NOTION_API_TOKEN`. + ## Available Commands ``` @@ -102,6 +113,8 @@ notion-cli page edit --find "old text" --replace-with "new text" notion-cli page edit --find "section" --append "additional content" ``` +For `page upload` and `page sync`, standalone local markdown image lines like `![Alt](./diagram.png)` are uploaded natively through the official API when configured. Local images must appear on their own line. Inline or mixed-content local image syntax is rejected. + `page view` shows open page-level comments and inline block discussions by default. Inline discussions are rendered beside their anchor text, with the anchor wrapped in `[[...]]` and the discussion shown immediately below it. Use `--no-comments` when you only want the page body, `--raw` to inspect the original Notion markup, and `--json` when an agent needs the page plus the `Comments` array. ### Edit mode guardrails @@ -173,4 +186,5 @@ notion-cli search "api" --json | jq '.[] | .title' 7. **Raw output** - Use `--raw` with `page view` to see the original Notion markup 8. **JSON for parsing** - Use `--json` when you need to extract specific fields, including the `Comments` array from `page view` 9. **Auth preflight** - Run `notion-cli auth status --json` before a multi-step workflow and refresh/login if needed -10. **Error handling** - If a targeted `page edit` call fails, rerun with `--replace` as a safe fallback +10. **API fallback preflight** - Run `notion-cli auth api verify` before workflows that need local image upload +11. **Error handling** - If a targeted `page edit` call fails, rerun with `--replace` as a safe fallback From e1bd99dc25bbff38c20dafd630b2b15c475d9c77 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Wed, 1 Apr 2026 00:11:18 -0400 Subject: [PATCH 02/31] fix(page): make local image upload work Send uploaded file parts with the correct MIME type, use the current Notion block insertion payload shape, and fail closed when local image uploads are attempted without a parent shared to the integration. This keeps page upload and sync working against the live API instead of relying on stale request contracts or leaving orphaned pages behind. --- cmd/page.go | 9 ++++++++ cmd/page_local_images.go | 13 ++++++++++++ cmd/page_local_images_test.go | 39 +++++++++++++++++++++++++++++++++-- internal/api/client.go | 32 +++++++++++++++++++++------- internal/api/client_test.go | 17 +++++++++++++++ 5 files changed, 101 insertions(+), 9 deletions(-) diff --git a/cmd/page.go b/cmd/page.go index c56ff01..17e1c06 100644 --- a/cmd/page.go +++ b/cmd/page.go @@ -277,6 +277,10 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string) err output.PrintError(err) return err } + if err := requireLocalImageParent(localUploads, parent, parentDB); err != nil { + output.PrintError(err) + return err + } if title == "" { title = extractTitleFromMarkdown(markdown) @@ -628,6 +632,11 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string) error return nil } + if err := requireLocalImageParent(localUploads, parent, parentDB); err != nil { + output.PrintError(err) + return err + } + req := mcp.CreatePageRequest{ Title: title, Content: body, diff --git a/cmd/page_local_images.go b/cmd/page_local_images.go index c46c68c..dceccbb 100644 --- a/cmd/page_local_images.go +++ b/cmd/page_local_images.go @@ -9,6 +9,7 @@ import ( "github.com/lox/notion-cli/internal/api" "github.com/lox/notion-cli/internal/cli" "github.com/lox/notion-cli/internal/mcp" + "github.com/lox/notion-cli/internal/output" ) type uploadedLocalImage struct { @@ -59,6 +60,18 @@ func prepareLocalImageUploads(ctx context.Context, sourceFile, markdown string) return rewritten, uploads, nil } +func requireLocalImageParent(uploads []uploadedLocalImage, parent, parentDB string) error { + if len(uploads) == 0 { + return nil + } + if strings.TrimSpace(parent) != "" || strings.TrimSpace(parentDB) != "" { + return nil + } + return &output.UserError{ + Message: "standalone local image upload requires --parent or --parent-db shared with your Notion integration", + } +} + func substituteUploadedLocalImages(ctx context.Context, pageID string, uploads []uploadedLocalImage) error { if strings.TrimSpace(pageID) == "" || len(uploads) == 0 { return nil diff --git a/cmd/page_local_images_test.go b/cmd/page_local_images_test.go index 4ac8975..4c1c84c 100644 --- a/cmd/page_local_images_test.go +++ b/cmd/page_local_images_test.go @@ -77,8 +77,19 @@ func TestSubstituteUploadedLocalImagesAppendsAfterPlaceholderAndDeletes(t *testi if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { t.Fatalf("Decode: %v", err) } - if payload["after"] != "block_123" { - t.Fatalf("after = %#v", payload["after"]) + position, ok := payload["position"].(map[string]any) + if !ok { + t.Fatalf("position = %#v", payload["position"]) + } + if position["type"] != "after_block" { + t.Fatalf("position.type = %#v", position["type"]) + } + afterBlock, ok := position["after_block"].(map[string]any) + if !ok { + t.Fatalf("position.after_block = %#v", position["after_block"]) + } + if afterBlock["id"] != "block_123" { + t.Fatalf("position.after_block.id = %#v", afterBlock["id"]) } _, _ = w.Write([]byte(`{"results":[]}`)) case r.Method == http.MethodDelete && r.URL.Path == "/v1/blocks/block_123": @@ -107,3 +118,27 @@ func TestSubstituteUploadedLocalImagesAppendsAfterPlaceholderAndDeletes(t *testi t.Fatalf("expected append and delete, saw append=%v delete=%v", sawAppend, sawDelete) } } + +func TestRequireLocalImageParent(t *testing.T) { + uploads := []uploadedLocalImage{{ + Placeholder: "PLACEHOLDER", + }} + + if err := requireLocalImageParent(nil, "", ""); err != nil { + t.Fatalf("expected nil without uploads, got %v", err) + } + if err := requireLocalImageParent(uploads, "parent-id", ""); err != nil { + t.Fatalf("expected nil with parent, got %v", err) + } + if err := requireLocalImageParent(uploads, "", "db-id"); err != nil { + t.Fatalf("expected nil with parent db, got %v", err) + } + + err := requireLocalImageParent(uploads, "", "") + if err == nil { + t.Fatal("expected error without parent or parent db") + } + if !strings.Contains(err.Error(), "--parent or --parent-db") { + t.Fatalf("unexpected error: %v", err) + } +} diff --git a/internal/api/client.go b/internal/api/client.go index d05c705..7c61fcd 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -6,8 +6,10 @@ import ( "encoding/json" "fmt" "io" + "mime" "mime/multipart" "net/http" + "net/textproto" "path/filepath" "strings" "time" @@ -188,12 +190,6 @@ func (c *Client) AppendUploadedImageAfter(ctx context.Context, parentID, afterBl } payload := map[string]any{ - "position": map[string]any{ - "type": "after_block", - "after_block": map[string]any{ - "id": afterBlockID, - }, - }, "children": []map[string]any{ { "object": "block", @@ -201,6 +197,12 @@ func (c *Client) AppendUploadedImageAfter(ctx context.Context, parentID, afterBl "image": image, }, }, + "position": map[string]any{ + "type": "after_block", + "after_block": map[string]any{ + "id": afterBlockID, + }, + }, } return c.doJSON(ctx, http.MethodPatch, "/blocks/"+parentID+"/children", payload, nil) @@ -314,7 +316,11 @@ func (c *Client) sendFileUploadPart(ctx context.Context, fileUploadID, filename var body bytes.Buffer writer := multipart.NewWriter(&body) - part, err := writer.CreateFormFile("file", filename) + header := make(textproto.MIMEHeader) + header.Set("Content-Disposition", fmt.Sprintf(`form-data; name="file"; filename="%s"`, filename)) + header.Set("Content-Type", detectUploadContentType(filename, data)) + + part, err := writer.CreatePart(header) if err != nil { return nil, fmt.Errorf("create multipart file part: %w", err) } @@ -336,6 +342,18 @@ func (c *Client) sendFileUploadPart(ctx context.Context, fileUploadID, filename return &out, nil } +func detectUploadContentType(filename string, data []byte) string { + if ext := strings.TrimSpace(filepath.Ext(filename)); ext != "" { + if contentType := strings.TrimSpace(mime.TypeByExtension(strings.ToLower(ext))); contentType != "" { + return contentType + } + } + if len(data) > 0 { + return http.DetectContentType(data) + } + return "application/octet-stream" +} + func (c *Client) waitForFileUploadUploaded(ctx context.Context, fileUploadID string) (*FileUpload, error) { id := strings.TrimSpace(fileUploadID) if id == "" { diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 5078036..d9727e0 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -4,6 +4,8 @@ import ( "context" "encoding/json" "io" + "mime" + "mime/multipart" "net/http" "net/http/httptest" "strings" @@ -59,6 +61,21 @@ func TestUploadFileAndAppendAfter(t *testing.T) { if !strings.HasPrefix(ct, "multipart/form-data;") { t.Fatalf("Content-Type = %q", ct) } + mediaType, params, err := mime.ParseMediaType(ct) + if err != nil { + t.Fatalf("ParseMediaType: %v", err) + } + if mediaType != "multipart/form-data" { + t.Fatalf("mediaType = %q", mediaType) + } + reader := multipart.NewReader(r.Body, params["boundary"]) + part, err := reader.NextPart() + if err != nil { + t.Fatalf("NextPart: %v", err) + } + if got := part.Header.Get("Content-Type"); got != "image/png" { + t.Fatalf("part Content-Type = %q", got) + } _, _ = w.Write([]byte(`{"id":"upload_123","status":"uploaded"}`)) case r.Method == http.MethodGet && r.URL.Path == "/v1/file_uploads/upload_123": getCalls++ From cf14138fd6d20f82623c761d0a804fda474571b5 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Wed, 1 Apr 2026 00:14:47 -0400 Subject: [PATCH 03/31] fix(page): escape multipart filenames Use mime.FormatMediaType for the file part content disposition so quoted filenames are encoded safely while keeping the live upload path working against the current Notion API. --- internal/api/client.go | 9 ++++++++- internal/api/client_test.go | 5 ++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index 7c61fcd..a85ee83 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -317,7 +317,14 @@ func (c *Client) sendFileUploadPart(ctx context.Context, fileUploadID, filename writer := multipart.NewWriter(&body) header := make(textproto.MIMEHeader) - header.Set("Content-Disposition", fmt.Sprintf(`form-data; name="file"; filename="%s"`, filename)) + contentDisposition := mime.FormatMediaType("form-data", map[string]string{ + "name": "file", + "filename": filename, + }) + if strings.TrimSpace(contentDisposition) == "" { + return nil, fmt.Errorf("format multipart content disposition: empty result") + } + header.Set("Content-Disposition", contentDisposition) header.Set("Content-Type", detectUploadContentType(filename, data)) part, err := writer.CreatePart(header) diff --git a/internal/api/client_test.go b/internal/api/client_test.go index d9727e0..8d676f0 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -73,6 +73,9 @@ func TestUploadFileAndAppendAfter(t *testing.T) { if err != nil { t.Fatalf("NextPart: %v", err) } + if got := part.FileName(); got != `diag"ram.png` { + t.Fatalf("part FileName = %q", got) + } if got := part.Header.Get("Content-Type"); got != "image/png" { t.Fatalf("part Content-Type = %q", got) } @@ -113,7 +116,7 @@ func TestUploadFileAndAppendAfter(t *testing.T) { t.Fatalf("NewClient: %v", err) } - uploadID, err := client.UploadFile(context.Background(), "diagram.png", []byte("PNGDATA")) + uploadID, err := client.UploadFile(context.Background(), `diag"ram.png`, []byte("PNGDATA")) if err != nil { t.Fatalf("UploadFile: %v", err) } From 582132a668218f0526ea63d3ab19c8dd969a79cb Mon Sep 17 00:00:00 2001 From: Brian Le Date: Wed, 1 Apr 2026 01:34:48 -0400 Subject: [PATCH 04/31] fix(page): handle cleanup errors and CRLF images --- cmd/page.go | 44 +++++++++++++++++-------------- internal/cli/local_images.go | 3 ++- internal/cli/local_images_test.go | 20 ++++++++++++++ 3 files changed, 46 insertions(+), 21 deletions(-) diff --git a/cmd/page.go b/cmd/page.go index 17e1c06..150e7a4 100644 --- a/cmd/page.go +++ b/cmd/page.go @@ -330,18 +330,20 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string) err output.PrintError(err) return err } - pageID := pageIDFromCreateResponse(resp) - if err := substituteUploadedLocalImages(bgCtx, pageID, localUploads); err != nil { - finalErr := fmt.Errorf("insert uploaded local images: %w", err) - if pageID != "" { - if apiClient, apiErr := cli.RequireOfficialAPIClient(); apiErr == nil { - if cleanupErr := apiClient.TrashPage(bgCtx, pageID); cleanupErr != nil { - finalErr = fmt.Errorf("%w (cleanup failed: %v)", finalErr, cleanupErr) + pageID := pageIDFromCreateResponse(resp) + if err := substituteUploadedLocalImages(bgCtx, pageID, localUploads); err != nil { + finalErr := fmt.Errorf("insert uploaded local images: %w", err) + if pageID != "" { + if apiClient, apiErr := cli.RequireOfficialAPIClient(); apiErr == nil { + if cleanupErr := apiClient.TrashPage(bgCtx, pageID); cleanupErr != nil { + finalErr = fmt.Errorf("%w (cleanup failed: %v)", finalErr, cleanupErr) + } + } else { + finalErr = fmt.Errorf("%w (cleanup client init failed: %v)", finalErr, apiErr) } } - } - output.PrintError(finalErr) - return finalErr + output.PrintError(finalErr) + return finalErr } displayTitle := title @@ -669,18 +671,20 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string) error return err } - pageID := pageIDFromCreateResponse(resp) - if err := substituteUploadedLocalImages(bgCtx, pageID, localUploads); err != nil { - finalErr := fmt.Errorf("insert uploaded local images: %w", err) - if pageID != "" { - if apiClient, apiErr := cli.RequireOfficialAPIClient(); apiErr == nil { - if cleanupErr := apiClient.TrashPage(bgCtx, pageID); cleanupErr != nil { - finalErr = fmt.Errorf("%w (cleanup failed: %v)", finalErr, cleanupErr) + pageID := pageIDFromCreateResponse(resp) + if err := substituteUploadedLocalImages(bgCtx, pageID, localUploads); err != nil { + finalErr := fmt.Errorf("insert uploaded local images: %w", err) + if pageID != "" { + if apiClient, apiErr := cli.RequireOfficialAPIClient(); apiErr == nil { + if cleanupErr := apiClient.TrashPage(bgCtx, pageID); cleanupErr != nil { + finalErr = fmt.Errorf("%w (cleanup failed: %v)", finalErr, cleanupErr) + } + } else { + finalErr = fmt.Errorf("%w (cleanup client init failed: %v)", finalErr, apiErr) } } - } - output.PrintError(finalErr) - return finalErr + output.PrintError(finalErr) + return finalErr } if pageID == "" { output.PrintWarning("Page created but could not retrieve ID for frontmatter") diff --git a/internal/cli/local_images.go b/internal/cli/local_images.go index 7961200..8cd8f9a 100644 --- a/internal/cli/local_images.go +++ b/internal/cli/local_images.go @@ -29,7 +29,8 @@ func RewriteStandaloneLocalImages(markdown, sourceFile string) (string, []LocalI } sourceDir := filepath.Dir(sourceFileAbs) - lines := strings.Split(markdown, "\n") + normalizedMarkdown := strings.NewReplacer("\r\n", "\n", "\r", "\n").Replace(markdown) + lines := strings.Split(normalizedMarkdown, "\n") placements := make([]LocalImagePlacement, 0) for i, line := range lines { matches := markdownImageRE.FindAllStringSubmatch(line, -1) diff --git a/internal/cli/local_images_test.go b/internal/cli/local_images_test.go index 39381e1..5dcaa8e 100644 --- a/internal/cli/local_images_test.go +++ b/internal/cli/local_images_test.go @@ -30,6 +30,26 @@ func TestRewriteStandaloneLocalImagesRewritesStandaloneLocalLines(t *testing.T) } } +func TestRewriteStandaloneLocalImagesHandlesCRLF(t *testing.T) { + tmp := t.TempDir() + doc := filepath.Join(tmp, "doc.md") + img := filepath.Join(tmp, "diagram.png") + if err := os.WriteFile(img, []byte("PNG"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + rewritten, placements, err := RewriteStandaloneLocalImages("# Title\r\n\r\n![Diagram](./diagram.png)\r\n", doc) + if err != nil { + t.Fatalf("RewriteStandaloneLocalImages: %v", err) + } + if len(placements) != 1 { + t.Fatalf("len(placements) = %d, want 1", len(placements)) + } + if !strings.Contains(rewritten, placements[0].Placeholder) { + t.Fatalf("rewritten markdown missing placeholder: %q", rewritten) + } +} + func TestRewriteStandaloneLocalImagesRejectsInlineLocalImage(t *testing.T) { tmp := t.TempDir() doc := filepath.Join(tmp, "doc.md") From f1db53cf34742d6d5b18e4fd414760b9e4702837 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Wed, 1 Apr 2026 02:13:19 -0400 Subject: [PATCH 05/31] feat(page): add --skip-local-images flag to upload and sync When set, standalone local image lines are silently stripped from the markdown instead of triggering the official API upload flow. Remote images are preserved. This lets users upload/sync documents with local image references even without an API token configured. --- README.md | 4 +- cmd/page.go | 84 +++++++++++++++++++++++++--------------- cmd/page_local_images.go | 23 +++++++++++ cmd/page_test.go | 49 +++++++++++++++++++++++ 4 files changed, 128 insertions(+), 32 deletions(-) create mode 100644 cmd/page_test.go diff --git a/README.md b/README.md index af04ea7..e67f4b8 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,7 @@ notion-cli page upload ./document.md --parent "Engineering" # Parent by name or notion-cli page upload ./document.md --parent-db # Upload as database entry notion-cli page upload ./document.md --icon "📄" # Set emoji icon notion-cli page upload ./document.md # Uploads standalone local images when configured +notion-cli page upload ./document.md --skip-local-images # Strips standalone local image lines instead # Sync a markdown file (create or update) notion-cli page sync ./document.md # Creates page, writes notion-id to frontmatter @@ -94,6 +95,7 @@ notion-cli page sync ./document.md # Updates page using notion-cli page sync ./document.md --parent "Engineering" # Set parent on first sync notion-cli page sync ./document.md --parent-db # Sync as database entry notion-cli page sync ./document.md # Uploads standalone local images when configured +notion-cli page sync ./document.md --skip-local-images # Strips standalone local image lines instead # Edit an existing page notion-cli page edit --replace "New content" # Replace all content @@ -107,7 +109,7 @@ The `` argument accepts a URL, ID, or page name. `page view` shows open page-level comments and inline block discussions by default. Inline discussions are rendered in context, with the anchor text wrapped in `[[...]]` and the discussion shown immediately below it. Use `--no-comments` to suppress comments, `--raw` to inspect the original Notion markup, and `--json` to return the page plus a `Comments` array. -`page upload` and `page sync` support native local image upload for standalone markdown image lines like `![Alt](./diagram.png)`. When local images are present, `notion-cli` uploads those files through the official Notion API and keeps them in document order. This requires an official API token configured through `auth api setup` or `NOTION_API_TOKEN`. Inline or mixed-content local image syntax is rejected instead of being guessed. +`page upload` and `page sync` support native local image upload for standalone markdown image lines like `![Alt](./diagram.png)`. When local images are present, `notion-cli` uploads those files through the official Notion API and keeps them in document order. This requires an official API token configured through `auth api setup` or `NOTION_API_TOKEN`. Pass `--skip-local-images` to silently remove standalone local image lines instead of uploading them. Inline or mixed-content local image syntax is rejected instead of being guessed. ### Search diff --git a/cmd/page.go b/cmd/page.go index 150e7a4..9dbf7c5 100644 --- a/cmd/page.go +++ b/cmd/page.go @@ -250,20 +250,21 @@ func runPageCreate(ctx *Context, title, parent, content string) error { } type PageUploadCmd struct { - File string `arg:"" help:"Markdown file to upload" type:"existingfile"` - Title string `help:"Page title (default: filename or first heading)" short:"t"` - Parent string `help:"Parent page URL, name, or ID" short:"p"` - ParentDB string `help:"Parent database URL, name, or ID" name:"parent-db" short:"d"` - Icon string `help:"Emoji icon for the page" short:"i"` - JSON bool `help:"Output as JSON" short:"j"` + File string `arg:"" help:"Markdown file to upload" type:"existingfile"` + Title string `help:"Page title (default: filename or first heading)" short:"t"` + Parent string `help:"Parent page URL, name, or ID" short:"p"` + ParentDB string `help:"Parent database URL, name, or ID" name:"parent-db" short:"d"` + Icon string `help:"Emoji icon for the page" short:"i"` + SkipLocalImages bool `help:"Strip local image references instead of uploading them" name:"skip-local-images"` + JSON bool `help:"Output as JSON" short:"j"` } func (c *PageUploadCmd) Run(ctx *Context) error { ctx.JSON = c.JSON - return runPageUpload(ctx, c.File, c.Title, c.Parent, c.ParentDB, c.Icon) + return runPageUpload(ctx, c.File, c.Title, c.Parent, c.ParentDB, c.Icon, c.SkipLocalImages) } -func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string) error { +func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string, skipLocalImages bool) error { content, err := os.ReadFile(file) if err != nil { output.PrintError(err) @@ -272,14 +273,23 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string) err markdown := string(content) bgCtx := context.Background() - markdown, localUploads, err := prepareLocalImageUploads(bgCtx, file, markdown) - if err != nil { - output.PrintError(err) - return err - } - if err := requireLocalImageParent(localUploads, parent, parentDB); err != nil { - output.PrintError(err) - return err + var localUploads []uploadedLocalImage + if skipLocalImages { + markdown, err = stripLocalImages(file, markdown) + if err != nil { + output.PrintError(err) + return err + } + } else { + markdown, localUploads, err = prepareLocalImageUploads(bgCtx, file, markdown) + if err != nil { + output.PrintError(err) + return err + } + if err := requireLocalImageParent(localUploads, parent, parentDB); err != nil { + output.PrintError(err) + return err + } } if title == "" { @@ -537,20 +547,21 @@ func parsePageEditProperties(props []string) (map[string]any, error) { } type PageSyncCmd struct { - File string `arg:"" help:"Markdown file to sync" type:"existingfile"` - Title string `help:"Page title (default: filename or first heading)" short:"t"` - Parent string `help:"Parent page URL, name, or ID" short:"p"` - ParentDB string `help:"Parent database URL, name, or ID" name:"parent-db" short:"d"` - Icon string `help:"Emoji icon for the page" short:"i"` - JSON bool `help:"Output as JSON" short:"j"` + File string `arg:"" help:"Markdown file to sync" type:"existingfile"` + Title string `help:"Page title (default: filename or first heading)" short:"t"` + Parent string `help:"Parent page URL, name, or ID" short:"p"` + ParentDB string `help:"Parent database URL, name, or ID" name:"parent-db" short:"d"` + Icon string `help:"Emoji icon for the page" short:"i"` + SkipLocalImages bool `help:"Strip local image references instead of uploading them" name:"skip-local-images"` + JSON bool `help:"Output as JSON" short:"j"` } func (c *PageSyncCmd) Run(ctx *Context) error { ctx.JSON = c.JSON - return runPageSync(ctx, c.File, c.Title, c.Parent, c.ParentDB, c.Icon) + return runPageSync(ctx, c.File, c.Title, c.Parent, c.ParentDB, c.Icon, c.SkipLocalImages) } -func runPageSync(ctx *Context, file, title, parent, parentDB, icon string) error { +func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipLocalImages bool) error { raw, err := os.ReadFile(file) if err != nil { output.PrintError(err) @@ -560,10 +571,19 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string) error content := string(raw) fm, body := cli.ParseFrontmatter(content) bgCtx := context.Background() - body, localUploads, err := prepareLocalImageUploads(bgCtx, file, body) - if err != nil { - output.PrintError(err) - return err + var localUploads []uploadedLocalImage + if skipLocalImages { + body, err = stripLocalImages(file, body) + if err != nil { + output.PrintError(err) + return err + } + } else { + body, localUploads, err = prepareLocalImageUploads(bgCtx, file, body) + if err != nil { + output.PrintError(err) + return err + } } if title == "" { @@ -634,9 +654,11 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string) error return nil } - if err := requireLocalImageParent(localUploads, parent, parentDB); err != nil { - output.PrintError(err) - return err + if !skipLocalImages { + if err := requireLocalImageParent(localUploads, parent, parentDB); err != nil { + output.PrintError(err) + return err + } } req := mcp.CreatePageRequest{ diff --git a/cmd/page_local_images.go b/cmd/page_local_images.go index dceccbb..5048c73 100644 --- a/cmd/page_local_images.go +++ b/cmd/page_local_images.go @@ -60,6 +60,29 @@ func prepareLocalImageUploads(ctx context.Context, sourceFile, markdown string) return rewritten, uploads, nil } +func stripLocalImages(sourceFile, markdown string) (string, error) { + rewritten, placements, err := cli.RewriteStandaloneLocalImages(markdown, sourceFile) + if err != nil { + return "", err + } + if len(placements) == 0 { + return markdown, nil + } + + lines := strings.Split(rewritten, "\n") + placeholders := make(map[string]struct{}, len(placements)) + for _, placement := range placements { + placeholders[placement.Placeholder] = struct{}{} + } + for i, line := range lines { + if _, ok := placeholders[line]; ok { + lines[i] = "" + } + } + + return strings.Join(lines, "\n"), nil +} + func requireLocalImageParent(uploads []uploadedLocalImage, parent, parentDB string) error { if len(uploads) == 0 { return nil diff --git a/cmd/page_test.go b/cmd/page_test.go new file mode 100644 index 0000000..062f9af --- /dev/null +++ b/cmd/page_test.go @@ -0,0 +1,49 @@ +package cmd + +import ( + "os" + "path/filepath" + "testing" +) + +func TestStripLocalImagesRemovesStandaloneLocalImageLines(t *testing.T) { + tmp := t.TempDir() + doc := filepath.Join(tmp, "doc.md") + img := filepath.Join(tmp, "diagram.png") + if err := os.WriteFile(img, []byte("PNGDATA"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + markdown := "# Title\n\n![Local](./diagram.png)\n\nParagraph\n" + + got, err := stripLocalImages(doc, markdown) + if err != nil { + t.Fatalf("stripLocalImages: %v", err) + } + + want := "# Title\n\n\n\nParagraph\n" + if got != want { + t.Fatalf("stripLocalImages() = %q, want %q", got, want) + } +} + +func TestStripLocalImagesPreservesRemoteImages(t *testing.T) { + tmp := t.TempDir() + doc := filepath.Join(tmp, "doc.md") + img := filepath.Join(tmp, "diagram.png") + if err := os.WriteFile(img, []byte("PNGDATA"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + markdown := "![Remote](https://example.com/remote.png)\n![Local](./diagram.png)\n" + + got, err := stripLocalImages(doc, markdown) + if err != nil { + t.Fatalf("stripLocalImages: %v", err) + } + + want := "![Remote](https://example.com/remote.png)\n\n" + if got != want { + t.Fatalf("stripLocalImages() = %q, want %q", got, want) + } +} From d96b12d0fe2e5ff2df20c215993d94e854fc8ae4 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Fri, 3 Apr 2026 19:50:33 -0400 Subject: [PATCH 06/31] fix(page): wire official api overrides through page commands --- cmd/auth.go | 11 ------- cmd/page.go | 56 +++++++++++++++++------------------ cmd/page_local_images.go | 8 ++--- cmd/page_local_images_test.go | 18 ++++++----- cmd/root.go | 13 ++++++++ 5 files changed, 55 insertions(+), 51 deletions(-) diff --git a/cmd/auth.go b/cmd/auth.go index 8649697..a1effff 100644 --- a/cmd/auth.go +++ b/cmd/auth.go @@ -198,17 +198,6 @@ type AuthAPIStatusCmd struct { JSON bool `help:"Output as JSON" short:"j"` } -func officialAPIOverrides(ctx *Context) config.APIOverrides { - if ctx == nil { - return config.APIOverrides{} - } - return config.APIOverrides{ - BaseURL: ctx.APIBaseURL, - NotionVersion: ctx.APINotionVersion, - Token: ctx.APIToken, - } -} - func (c *AuthAPIStatusCmd) Run(ctx *Context) error { ctx.JSON = c.JSON diff --git a/cmd/page.go b/cmd/page.go index 9dbf7c5..d2cad8d 100644 --- a/cmd/page.go +++ b/cmd/page.go @@ -281,7 +281,7 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string, ski return err } } else { - markdown, localUploads, err = prepareLocalImageUploads(bgCtx, file, markdown) + markdown, localUploads, err = prepareLocalImageUploads(ctx, bgCtx, file, markdown) if err != nil { output.PrintError(err) return err @@ -340,20 +340,20 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string, ski output.PrintError(err) return err } - pageID := pageIDFromCreateResponse(resp) - if err := substituteUploadedLocalImages(bgCtx, pageID, localUploads); err != nil { - finalErr := fmt.Errorf("insert uploaded local images: %w", err) - if pageID != "" { - if apiClient, apiErr := cli.RequireOfficialAPIClient(); apiErr == nil { - if cleanupErr := apiClient.TrashPage(bgCtx, pageID); cleanupErr != nil { - finalErr = fmt.Errorf("%w (cleanup failed: %v)", finalErr, cleanupErr) - } - } else { - finalErr = fmt.Errorf("%w (cleanup client init failed: %v)", finalErr, apiErr) + pageID := pageIDFromCreateResponse(resp) + if err := substituteUploadedLocalImages(ctx, bgCtx, pageID, localUploads); err != nil { + finalErr := fmt.Errorf("insert uploaded local images: %w", err) + if pageID != "" { + if apiClient, apiErr := cli.RequireOfficialAPIClient(officialAPIOverrides(ctx)); apiErr == nil { + if cleanupErr := apiClient.TrashPage(bgCtx, pageID); cleanupErr != nil { + finalErr = fmt.Errorf("%w (cleanup failed: %v)", finalErr, cleanupErr) } + } else { + finalErr = fmt.Errorf("%w (cleanup client init failed: %v)", finalErr, apiErr) } - output.PrintError(finalErr) - return finalErr + } + output.PrintError(finalErr) + return finalErr } displayTitle := title @@ -579,7 +579,7 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL return err } } else { - body, localUploads, err = prepareLocalImageUploads(bgCtx, file, body) + body, localUploads, err = prepareLocalImageUploads(ctx, bgCtx, file, body) if err != nil { output.PrintError(err) return err @@ -605,7 +605,7 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL if fm.NotionID != "" { var snapshot *api.PageMarkdown if len(localUploads) > 0 { - apiClient, err := cli.RequireOfficialAPIClient() + apiClient, err := cli.RequireOfficialAPIClient(officialAPIOverrides(ctx)) if err != nil { output.PrintError(err) return err @@ -626,7 +626,7 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL output.PrintError(err) return err } - if err := substituteUploadedLocalImages(bgCtx, fm.NotionID, localUploads); err != nil { + if err := substituteUploadedLocalImages(ctx, bgCtx, fm.NotionID, localUploads); err != nil { finalErr := fmt.Errorf("insert uploaded local images: %w", err) rollbackErr := rollbackSyncedPage(bgCtx, client, fm.NotionID, snapshot) if rollbackErr != nil { @@ -693,20 +693,20 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL return err } - pageID := pageIDFromCreateResponse(resp) - if err := substituteUploadedLocalImages(bgCtx, pageID, localUploads); err != nil { - finalErr := fmt.Errorf("insert uploaded local images: %w", err) - if pageID != "" { - if apiClient, apiErr := cli.RequireOfficialAPIClient(); apiErr == nil { - if cleanupErr := apiClient.TrashPage(bgCtx, pageID); cleanupErr != nil { - finalErr = fmt.Errorf("%w (cleanup failed: %v)", finalErr, cleanupErr) - } - } else { - finalErr = fmt.Errorf("%w (cleanup client init failed: %v)", finalErr, apiErr) + pageID := pageIDFromCreateResponse(resp) + if err := substituteUploadedLocalImages(ctx, bgCtx, pageID, localUploads); err != nil { + finalErr := fmt.Errorf("insert uploaded local images: %w", err) + if pageID != "" { + if apiClient, apiErr := cli.RequireOfficialAPIClient(officialAPIOverrides(ctx)); apiErr == nil { + if cleanupErr := apiClient.TrashPage(bgCtx, pageID); cleanupErr != nil { + finalErr = fmt.Errorf("%w (cleanup failed: %v)", finalErr, cleanupErr) } + } else { + finalErr = fmt.Errorf("%w (cleanup client init failed: %v)", finalErr, apiErr) } - output.PrintError(finalErr) - return finalErr + } + output.PrintError(finalErr) + return finalErr } if pageID == "" { output.PrintWarning("Page created but could not retrieve ID for frontmatter") diff --git a/cmd/page_local_images.go b/cmd/page_local_images.go index 5048c73..1a9946e 100644 --- a/cmd/page_local_images.go +++ b/cmd/page_local_images.go @@ -19,7 +19,7 @@ type uploadedLocalImage struct { ResolvedPath string } -func prepareLocalImageUploads(ctx context.Context, sourceFile, markdown string) (string, []uploadedLocalImage, error) { +func prepareLocalImageUploads(cmdCtx *Context, ctx context.Context, sourceFile, markdown string) (string, []uploadedLocalImage, error) { rewritten, placements, err := cli.RewriteStandaloneLocalImages(markdown, sourceFile) if err != nil { return "", nil, err @@ -28,7 +28,7 @@ func prepareLocalImageUploads(ctx context.Context, sourceFile, markdown string) return markdown, nil, nil } - apiClient, err := cli.RequireOfficialAPIClient() + apiClient, err := cli.RequireOfficialAPIClient(officialAPIOverrides(cmdCtx)) if err != nil { return "", nil, err } @@ -95,12 +95,12 @@ func requireLocalImageParent(uploads []uploadedLocalImage, parent, parentDB stri } } -func substituteUploadedLocalImages(ctx context.Context, pageID string, uploads []uploadedLocalImage) error { +func substituteUploadedLocalImages(cmdCtx *Context, ctx context.Context, pageID string, uploads []uploadedLocalImage) error { if strings.TrimSpace(pageID) == "" || len(uploads) == 0 { return nil } - apiClient, err := cli.RequireOfficialAPIClient() + apiClient, err := cli.RequireOfficialAPIClient(officialAPIOverrides(cmdCtx)) if err != nil { return err } diff --git a/cmd/page_local_images_test.go b/cmd/page_local_images_test.go index 4c1c84c..6f25df9 100644 --- a/cmd/page_local_images_test.go +++ b/cmd/page_local_images_test.go @@ -40,11 +40,12 @@ func TestPrepareLocalImageUploadsUploadsAndDeduplicates(t *testing.T) { })) defer srv.Close() - t.Setenv("HOME", t.TempDir()) - t.Setenv("NOTION_API_BASE_URL", srv.URL+"/v1") - t.Setenv("NOTION_API_TOKEN", "secret-token") + cmdCtx := &Context{ + APIToken: "secret-token", + APIBaseURL: srv.URL + "/v1", + } - rewritten, uploads, err := prepareLocalImageUploads(context.Background(), doc, "![One](./diagram.png)\n![Two](./diagram.png)\n") + rewritten, uploads, err := prepareLocalImageUploads(cmdCtx, context.Background(), doc, "![One](./diagram.png)\n![Two](./diagram.png)\n") if err != nil { t.Fatalf("prepareLocalImageUploads: %v", err) } @@ -101,11 +102,12 @@ func TestSubstituteUploadedLocalImagesAppendsAfterPlaceholderAndDeletes(t *testi })) defer srv.Close() - t.Setenv("HOME", t.TempDir()) - t.Setenv("NOTION_API_BASE_URL", srv.URL+"/v1") - t.Setenv("NOTION_API_TOKEN", "secret-token") + cmdCtx := &Context{ + APIToken: "secret-token", + APIBaseURL: srv.URL + "/v1", + } - err := substituteUploadedLocalImages(context.Background(), "page_123", []uploadedLocalImage{{ + err := substituteUploadedLocalImages(cmdCtx, context.Background(), "page_123", []uploadedLocalImage{{ Alt: "Diagram", FileUploadID: "upload_123", Placeholder: "PLACEHOLDER", diff --git a/cmd/root.go b/cmd/root.go index 9594847..f5c1db5 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -1,5 +1,7 @@ package cmd +import "github.com/lox/notion-cli/internal/config" + type Context struct { JSON bool Token string @@ -31,3 +33,14 @@ func (c *VersionCmd) Run(ctx *Context) error { println("notion-cli version " + c.Version) return nil } + +func officialAPIOverrides(ctx *Context) config.APIOverrides { + if ctx == nil { + return config.APIOverrides{} + } + return config.APIOverrides{ + BaseURL: ctx.APIBaseURL, + NotionVersion: ctx.APINotionVersion, + Token: ctx.APIToken, + } +} From ce4479d0c1bb40c9c6b5dabfac732409900052d8 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Fri, 17 Apr 2026 22:43:19 -0400 Subject: [PATCH 07/31] fix(page): strip local images without requiring source files --skip-local-images was reusing the upload-validation path, so a missing referenced file aborted the strip with a not-found error. Split validation out of the parser and wire strip mode to a non-validating helper. Inline or mixed-content local image syntax still errors so callers never silently drop characters around an inline image. --- cmd/page.go | 4 +-- cmd/page_local_images.go | 4 +-- cmd/page_test.go | 34 ++++++++++----------- internal/cli/local_images.go | 51 +++++++++++++++++++++++++------ internal/cli/local_images_test.go | 42 +++++++++++++++++++++++++ 5 files changed, 103 insertions(+), 32 deletions(-) diff --git a/cmd/page.go b/cmd/page.go index d2cad8d..0805a39 100644 --- a/cmd/page.go +++ b/cmd/page.go @@ -275,7 +275,7 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string, ski bgCtx := context.Background() var localUploads []uploadedLocalImage if skipLocalImages { - markdown, err = stripLocalImages(file, markdown) + markdown, err = stripLocalImages(markdown) if err != nil { output.PrintError(err) return err @@ -573,7 +573,7 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL bgCtx := context.Background() var localUploads []uploadedLocalImage if skipLocalImages { - body, err = stripLocalImages(file, body) + body, err = stripLocalImages(body) if err != nil { output.PrintError(err) return err diff --git a/cmd/page_local_images.go b/cmd/page_local_images.go index 1a9946e..ccf6296 100644 --- a/cmd/page_local_images.go +++ b/cmd/page_local_images.go @@ -60,8 +60,8 @@ func prepareLocalImageUploads(cmdCtx *Context, ctx context.Context, sourceFile, return rewritten, uploads, nil } -func stripLocalImages(sourceFile, markdown string) (string, error) { - rewritten, placements, err := cli.RewriteStandaloneLocalImages(markdown, sourceFile) +func stripLocalImages(markdown string) (string, error) { + rewritten, placements, err := cli.FindStandaloneLocalImageLines(markdown) if err != nil { return "", err } diff --git a/cmd/page_test.go b/cmd/page_test.go index 062f9af..658aad7 100644 --- a/cmd/page_test.go +++ b/cmd/page_test.go @@ -1,22 +1,13 @@ package cmd import ( - "os" - "path/filepath" "testing" ) func TestStripLocalImagesRemovesStandaloneLocalImageLines(t *testing.T) { - tmp := t.TempDir() - doc := filepath.Join(tmp, "doc.md") - img := filepath.Join(tmp, "diagram.png") - if err := os.WriteFile(img, []byte("PNGDATA"), 0o644); err != nil { - t.Fatalf("WriteFile: %v", err) - } - markdown := "# Title\n\n![Local](./diagram.png)\n\nParagraph\n" - got, err := stripLocalImages(doc, markdown) + got, err := stripLocalImages(markdown) if err != nil { t.Fatalf("stripLocalImages: %v", err) } @@ -28,16 +19,9 @@ func TestStripLocalImagesRemovesStandaloneLocalImageLines(t *testing.T) { } func TestStripLocalImagesPreservesRemoteImages(t *testing.T) { - tmp := t.TempDir() - doc := filepath.Join(tmp, "doc.md") - img := filepath.Join(tmp, "diagram.png") - if err := os.WriteFile(img, []byte("PNGDATA"), 0o644); err != nil { - t.Fatalf("WriteFile: %v", err) - } - markdown := "![Remote](https://example.com/remote.png)\n![Local](./diagram.png)\n" - got, err := stripLocalImages(doc, markdown) + got, err := stripLocalImages(markdown) if err != nil { t.Fatalf("stripLocalImages: %v", err) } @@ -47,3 +31,17 @@ func TestStripLocalImagesPreservesRemoteImages(t *testing.T) { t.Fatalf("stripLocalImages() = %q, want %q", got, want) } } + +func TestStripLocalImagesStripsMissingLocalFiles(t *testing.T) { + markdown := "# Title\n\n![Local](./does-not-exist.png)\n\nParagraph\n" + + got, err := stripLocalImages(markdown) + if err != nil { + t.Fatalf("stripLocalImages: %v", err) + } + + want := "# Title\n\n\n\nParagraph\n" + if got != want { + t.Fatalf("stripLocalImages() = %q, want %q", got, want) + } +} diff --git a/internal/cli/local_images.go b/internal/cli/local_images.go index 8cd8f9a..945a185 100644 --- a/internal/cli/local_images.go +++ b/internal/cli/local_images.go @@ -29,6 +29,40 @@ func RewriteStandaloneLocalImages(markdown, sourceFile string) (string, []LocalI } sourceDir := filepath.Dir(sourceFileAbs) + return scanStandaloneLocalImages(markdown, func(dest string) (string, error) { + resolvedPath, err := resolveLocalPath(dest, sourceDir) + if err != nil { + return "", err + } + info, err := os.Stat(resolvedPath) + if err != nil { + return "", fmt.Errorf("local image %q not found (from %s): %w", dest, sourceFile, err) + } + if info.IsDir() { + return "", fmt.Errorf("local image %q resolves to a directory: %s", dest, resolvedPath) + } + return resolvedPath, nil + }) +} + +// FindStandaloneLocalImageLines rewrites standalone local image lines into +// placeholders without validating that the referenced files exist on disk. It +// still rejects inline or mixed-content local image syntax with the same error +// as RewriteStandaloneLocalImages, preserving the "local images must appear on +// their own line" invariant. Use this when the caller only needs to identify +// and strip local image lines (e.g. page upload/sync --skip-local-images) and +// does not need the resolved file path. +func FindStandaloneLocalImageLines(markdown string) (string, []LocalImagePlacement, error) { + return scanStandaloneLocalImages(markdown, nil) +} + +// scanStandaloneLocalImages walks markdown line-by-line, rejecting inline local +// images and replacing each standalone local image line with a placeholder. +// When resolvePath is non-nil, it is invoked for every local destination and +// its returned path is stored on the placement's Resolved field; a non-nil +// error from resolvePath aborts the scan. When resolvePath is nil, placements +// are recorded without a Resolved path. +func scanStandaloneLocalImages(markdown string, resolvePath func(dest string) (string, error)) (string, []LocalImagePlacement, error) { normalizedMarkdown := strings.NewReplacer("\r\n", "\n", "\r", "\n").Replace(markdown) lines := strings.Split(normalizedMarkdown, "\n") placements := make([]LocalImagePlacement, 0) @@ -54,16 +88,13 @@ func RewriteStandaloneLocalImages(markdown, sourceFile string) (string, []LocalI continue } - resolvedPath, err := resolveLocalPath(dest, sourceDir) - if err != nil { - return "", nil, err - } - info, err := os.Stat(resolvedPath) - if err != nil { - return "", nil, fmt.Errorf("local image %q not found (from %s): %w", dest, sourceFile, err) - } - if info.IsDir() { - return "", nil, fmt.Errorf("local image %q resolves to a directory: %s", dest, resolvedPath) + var resolvedPath string + if resolvePath != nil { + r, err := resolvePath(dest) + if err != nil { + return "", nil, err + } + resolvedPath = r } placeholder := "NOTION_CLI_LOCAL_IMAGE_" + strings.ReplaceAll(uuid.NewString(), "-", "_") diff --git a/internal/cli/local_images_test.go b/internal/cli/local_images_test.go index 5dcaa8e..18a45a3 100644 --- a/internal/cli/local_images_test.go +++ b/internal/cli/local_images_test.go @@ -78,3 +78,45 @@ func TestRewriteStandaloneLocalImagesIgnoresRemoteImages(t *testing.T) { t.Fatalf("rewritten = %q", rewritten) } } + +func TestFindStandaloneLocalImageLinesAcceptsMissingFiles(t *testing.T) { + rewritten, placements, err := FindStandaloneLocalImageLines("# Title\n\n![Diagram](./does-not-exist.png)\n\nDone\n") + if err != nil { + t.Fatalf("FindStandaloneLocalImageLines: %v", err) + } + if len(placements) != 1 { + t.Fatalf("len(placements) = %d, want 1", len(placements)) + } + if placements[0].Resolved != "" { + t.Fatalf("Resolved = %q, want empty (strip mode does not resolve paths)", placements[0].Resolved) + } + if placements[0].Placeholder == "" { + t.Fatalf("Placeholder was not set") + } + if !strings.Contains(rewritten, placements[0].Placeholder) { + t.Fatalf("rewritten markdown missing placeholder: %q", rewritten) + } + if strings.Contains(rewritten, "does-not-exist.png") { + t.Fatalf("rewritten markdown still contains original image line: %q", rewritten) + } +} + +func TestFindStandaloneLocalImageLinesRejectsInlineLocalImage(t *testing.T) { + _, _, err := FindStandaloneLocalImageLines("before ![Diagram](./diagram.png) after\n") + if err == nil || !strings.Contains(err.Error(), "must appear on their own line") { + t.Fatalf("expected unsupported syntax error, got %v", err) + } +} + +func TestFindStandaloneLocalImageLinesIgnoresRemoteImages(t *testing.T) { + rewritten, placements, err := FindStandaloneLocalImageLines("![Diagram](https://example.test/diagram.png)\n") + if err != nil { + t.Fatalf("FindStandaloneLocalImageLines: %v", err) + } + if len(placements) != 0 { + t.Fatalf("len(placements) = %d, want 0", len(placements)) + } + if rewritten != "![Diagram](https://example.test/diagram.png)\n" { + t.Fatalf("rewritten = %q", rewritten) + } +} From ce000bff0e17f6ce6e4545dd12902da3dd37f7e8 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Fri, 17 Apr 2026 22:44:05 -0400 Subject: [PATCH 08/31] fix(page): guard sync rollback against truncated snapshots rollbackSyncedPage only short-circuited on nil or empty snapshots before. If GetPageMarkdown returned a truncated snapshot, rollback would have replayed the partial markdown back to the live page via replace_content and silently dropped content. Skip rollback when snapshot.Truncated is set and surface an explicit error so the caller still sees the original substitution failure and the fact that rollback was intentionally skipped. --- cmd/page_local_images.go | 3 +++ cmd/page_local_images_test.go | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/cmd/page_local_images.go b/cmd/page_local_images.go index ccf6296..8c2f95c 100644 --- a/cmd/page_local_images.go +++ b/cmd/page_local_images.go @@ -145,6 +145,9 @@ func rollbackSyncedPage(ctx context.Context, client *mcp.Client, pageID string, if snapshot == nil || strings.TrimSpace(snapshot.Markdown) == "" { return nil } + if snapshot.Truncated { + return fmt.Errorf("skipped rollback: page markdown snapshot was truncated; replaying would lose content") + } return client.UpdatePage(ctx, mcp.UpdatePageRequest{ PageID: pageID, Command: "replace_content", diff --git a/cmd/page_local_images_test.go b/cmd/page_local_images_test.go index 6f25df9..1f1fbc1 100644 --- a/cmd/page_local_images_test.go +++ b/cmd/page_local_images_test.go @@ -9,6 +9,8 @@ import ( "path/filepath" "strings" "testing" + + "github.com/lox/notion-cli/internal/api" ) func TestPrepareLocalImageUploadsUploadsAndDeduplicates(t *testing.T) { @@ -144,3 +146,33 @@ func TestRequireLocalImageParent(t *testing.T) { t.Fatalf("unexpected error: %v", err) } } + +// A nil *mcp.Client is passed on purpose: if the guard failed to short-circuit +// on truncated snapshots, UpdatePage would dereference the nil client and panic. +func TestRollbackSyncedPageSkipsTruncatedSnapshot(t *testing.T) { + snapshot := &api.PageMarkdown{ + Markdown: "# Title\n\nPartial content\n", + Truncated: true, + } + + err := rollbackSyncedPage(context.Background(), nil, "page-id", snapshot) + if err == nil { + t.Fatalf("rollbackSyncedPage returned nil error; expected truncation error") + } + if !strings.Contains(err.Error(), "truncated") { + t.Fatalf("rollbackSyncedPage error = %q, want it to mention truncation", err.Error()) + } +} + +func TestRollbackSyncedPageSkipsNilSnapshot(t *testing.T) { + if err := rollbackSyncedPage(context.Background(), nil, "page-id", nil); err != nil { + t.Fatalf("rollbackSyncedPage returned %v, want nil", err) + } +} + +func TestRollbackSyncedPageSkipsEmptyMarkdownSnapshot(t *testing.T) { + snapshot := &api.PageMarkdown{Markdown: " \n"} + if err := rollbackSyncedPage(context.Background(), nil, "page-id", snapshot); err != nil { + t.Fatalf("rollbackSyncedPage returned %v, want nil", err) + } +} From 35fdc88cdcfebaebba25501256aeca43aaaac658 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 10:37:51 -0400 Subject: [PATCH 09/31] fix(page): scan local images with code-block and paren awareness Replace the regex-based local-image scanner with a line-level parser that tracks fenced and indented code blocks and parses destinations with balanced parens and CommonMark backslash escapes. This prevents standalone image lines inside fenced or indented code samples from being treated as upload/strip targets and fixes "not found" errors for paths like ./diagram(1).png or ./diagram\(final\).png. --- internal/cli/local_images.go | 285 ++++++++++++++++++++++++++++-- internal/cli/local_images_test.go | 134 ++++++++++++++ 2 files changed, 402 insertions(+), 17 deletions(-) diff --git a/internal/cli/local_images.go b/internal/cli/local_images.go index 945a185..2d33db5 100644 --- a/internal/cli/local_images.go +++ b/internal/cli/local_images.go @@ -18,8 +18,6 @@ type LocalImagePlacement struct { Placeholder string } -var markdownImageRE = regexp.MustCompile(`!\[([^\]]*)\]\(([^)\n]+)\)`) -var standaloneMarkdownImageRE = regexp.MustCompile(`^\s*!\[([^\]]*)\]\(([^)\n]+)\)\s*$`) var uriSchemeRE = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.-]*:`) func RewriteStandaloneLocalImages(markdown, sourceFile string) (string, []LocalImagePlacement, error) { @@ -56,35 +54,75 @@ func FindStandaloneLocalImageLines(markdown string) (string, []LocalImagePlaceme return scanStandaloneLocalImages(markdown, nil) } -// scanStandaloneLocalImages walks markdown line-by-line, rejecting inline local -// images and replacing each standalone local image line with a placeholder. -// When resolvePath is non-nil, it is invoked for every local destination and -// its returned path is stored on the placement's Resolved field; a non-nil -// error from resolvePath aborts the scan. When resolvePath is nil, placements -// are recorded without a Resolved path. +// scanStandaloneLocalImages walks markdown line-by-line, skipping fenced and +// indented code blocks, rejecting inline local images, and replacing each +// standalone local image line with a placeholder. When resolvePath is non-nil, +// it is invoked for every local destination and its returned path is stored on +// the placement's Resolved field; a non-nil error from resolvePath aborts the +// scan. When resolvePath is nil, placements are recorded without a Resolved +// path. func scanStandaloneLocalImages(markdown string, resolvePath func(dest string) (string, error)) (string, []LocalImagePlacement, error) { normalizedMarkdown := strings.NewReplacer("\r\n", "\n", "\r", "\n").Replace(markdown) lines := strings.Split(normalizedMarkdown, "\n") placements := make([]LocalImagePlacement, 0) + + var inFence bool + var fenceChar byte + var fenceLen int + var inIndented bool + prevBlank := true + for i, line := range lines { - matches := markdownImageRE.FindAllStringSubmatch(line, -1) + isBlank := strings.TrimSpace(line) == "" + + if inFence { + if closesFencedCodeBlock(line, fenceChar, fenceLen) { + inFence = false + } + prevBlank = isBlank + continue + } + if c, n := opensFencedCodeBlock(line); n > 0 { + inFence = true + fenceChar = c + fenceLen = n + prevBlank = false + continue + } + + if inIndented { + if isBlank || startsWithCodeIndent(line) { + prevBlank = isBlank + continue + } + inIndented = false + } else if prevBlank && !isBlank && startsWithCodeIndent(line) { + inIndented = true + prevBlank = false + continue + } + + matches := findMarkdownImages(line) if len(matches) == 0 { + prevBlank = isBlank continue } - standalone := standaloneMarkdownImageRE.FindStringSubmatch(line) - if standalone == nil { - for _, match := range matches { - dest, ok := parseMarkdownDestination(match[2]) + if !isStandaloneImageLine(line, matches) { + for _, m := range matches { + dest, ok := parseMarkdownDestination(m.dest) if ok && isLocalDestination(dest) { return "", nil, fmt.Errorf("unsupported local image syntax on line %d: local images must appear on their own line", i+1) } } + prevBlank = isBlank continue } - dest, ok := parseMarkdownDestination(standalone[2]) + m := matches[0] + dest, ok := parseMarkdownDestination(m.dest) if !ok || !isLocalDestination(dest) { + prevBlank = isBlank continue } @@ -100,16 +138,201 @@ func scanStandaloneLocalImages(markdown string, resolvePath func(dest string) (s placeholder := "NOTION_CLI_LOCAL_IMAGE_" + strings.ReplaceAll(uuid.NewString(), "-", "_") lines[i] = placeholder placements = append(placements, LocalImagePlacement{ - Alt: standalone[1], + Alt: m.alt, Original: dest, Resolved: resolvedPath, Placeholder: placeholder, }) + prevBlank = false } return strings.Join(lines, "\n"), placements, nil } +type markdownImageMatch struct { + start int + end int + alt string + dest string +} + +// findMarkdownImages returns every `![alt](dest)` span on a single line. +// Destinations may contain balanced parentheses and `\)` / `\(` escapes, and +// may optionally be wrapped in angle brackets (``). The returned `dest` +// is the raw text between the opening `(` and the matching `)`, preserving +// escapes for parseMarkdownDestination to normalize. +func findMarkdownImages(line string) []markdownImageMatch { + var matches []markdownImageMatch + for i := 0; i < len(line); { + if line[i] != '!' || i+1 >= len(line) || line[i+1] != '[' { + i++ + continue + } + altStart := i + 2 + altEnd, ok := findLinkTextEnd(line, altStart) + if !ok { + i = altStart + continue + } + if altEnd+1 >= len(line) || line[altEnd+1] != '(' { + i = altEnd + 1 + continue + } + destStart := altEnd + 2 + destEnd, ok := findDestinationEnd(line, destStart) + if !ok { + i = altEnd + 1 + continue + } + matches = append(matches, markdownImageMatch{ + start: i, + end: destEnd + 1, + alt: line[altStart:altEnd], + dest: line[destStart:destEnd], + }) + i = destEnd + 1 + } + return matches +} + +// findLinkTextEnd walks `line` from `start` and returns the offset of the +// unescaped `]` that closes the link text, honoring `\]` escapes. +func findLinkTextEnd(line string, start int) (int, bool) { + for i := start; i < len(line); i++ { + c := line[i] + if c == '\\' && i+1 < len(line) { + i++ + continue + } + if c == ']' { + return i, true + } + } + return 0, false +} + +// findDestinationEnd returns the offset of the `)` that closes a markdown +// destination starting at `start`. Balanced parens and `\(` / `\)` escapes +// inside the destination are preserved; angle-bracketed destinations end at +// the first unescaped `>` followed immediately by `)`. +func findDestinationEnd(line string, start int) (int, bool) { + if start < len(line) && line[start] == '<' { + for i := start + 1; i < len(line); i++ { + c := line[i] + if c == '\\' && i+1 < len(line) { + i++ + continue + } + if c == '>' { + if i+1 < len(line) && line[i+1] == ')' { + return i + 1, true + } + return 0, false + } + } + return 0, false + } + depth := 0 + for i := start; i < len(line); i++ { + c := line[i] + switch c { + case '\\': + if i+1 < len(line) { + i++ + continue + } + case '(': + depth++ + case ')': + if depth == 0 { + return i, true + } + depth-- + } + } + return 0, false +} + +// isStandaloneImageLine reports whether `line` contains exactly one image and +// no other non-whitespace content. +func isStandaloneImageLine(line string, matches []markdownImageMatch) bool { + if len(matches) != 1 { + return false + } + m := matches[0] + return strings.TrimSpace(line[:m.start]) == "" && strings.TrimSpace(line[m.end:]) == "" +} + +// opensFencedCodeBlock reports whether `line` opens a CommonMark fenced code +// block. It returns the fence character (`` ` `` or `~`) and the fence length +// (>= 3) on a match, and (0, 0) otherwise. +func opensFencedCodeBlock(line string) (byte, int) { + i := 0 + for i < len(line) && i < 4 && line[i] == ' ' { + i++ + } + if i >= 4 || i >= len(line) { + return 0, 0 + } + c := line[i] + if c != '`' && c != '~' { + return 0, 0 + } + n := 0 + for i < len(line) && line[i] == c { + i++ + n++ + } + if n < 3 { + return 0, 0 + } + // Backtick info strings may not contain additional backticks. + if c == '`' && strings.ContainsRune(line[i:], '`') { + return 0, 0 + } + return c, n +} + +// closesFencedCodeBlock reports whether `line` closes a fenced code block that +// was opened with `openChar` repeated `openLen` times. A closing fence must be +// the same character, at least as long as the opener, indented no more than 3 +// spaces, and followed only by whitespace. +func closesFencedCodeBlock(line string, openChar byte, openLen int) bool { + i := 0 + for i < len(line) && i < 4 && line[i] == ' ' { + i++ + } + if i >= 4 || i >= len(line) || line[i] != openChar { + return false + } + n := 0 + for i < len(line) && line[i] == openChar { + i++ + n++ + } + if n < openLen { + return false + } + for ; i < len(line); i++ { + if line[i] != ' ' && line[i] != '\t' { + return false + } + } + return true +} + +// startsWithCodeIndent reports whether `line` begins with an indented-code +// indentation (a tab or four spaces). +func startsWithCodeIndent(line string) bool { + if strings.HasPrefix(line, "\t") { + return true + } + if len(line) >= 4 && line[:4] == " " { + return true + } + return false +} + func parseMarkdownDestination(raw string) (string, bool) { s := strings.TrimSpace(raw) if s == "" { @@ -119,7 +342,7 @@ func parseMarkdownDestination(raw string) (string, bool) { if strings.HasPrefix(s, "<") { end := strings.Index(s, ">") if end > 1 { - return s[1:end], true + return unescapeMarkdownPunctuation(s[1:end]), true } } @@ -143,7 +366,35 @@ func parseMarkdownDestination(raw string) (string, bool) { if s == "" { return "", false } - return s, true + return unescapeMarkdownPunctuation(s), true +} + +// unescapeMarkdownPunctuation turns CommonMark backslash escapes of ASCII +// punctuation (e.g. `\)`, `\(`, `\\`) into their literal characters. +func unescapeMarkdownPunctuation(s string) string { + if !strings.Contains(s, `\`) { + return s + } + var b strings.Builder + b.Grow(len(s)) + for i := 0; i < len(s); i++ { + if s[i] == '\\' && i+1 < len(s) && isASCIIPunctuation(s[i+1]) { + b.WriteByte(s[i+1]) + i++ + continue + } + b.WriteByte(s[i]) + } + return b.String() +} + +func isASCIIPunctuation(b byte) bool { + switch b { + case '!', '"', '#', '$', '%', '&', '\'', '(', ')', '*', '+', ',', '-', '.', '/', + ':', ';', '<', '=', '>', '?', '@', '[', '\\', ']', '^', '_', '`', '{', '|', '}', '~': + return true + } + return false } func isLocalDestination(dest string) bool { diff --git a/internal/cli/local_images_test.go b/internal/cli/local_images_test.go index 18a45a3..ae1fda4 100644 --- a/internal/cli/local_images_test.go +++ b/internal/cli/local_images_test.go @@ -120,3 +120,137 @@ func TestFindStandaloneLocalImageLinesIgnoresRemoteImages(t *testing.T) { t.Fatalf("rewritten = %q", rewritten) } } + +func TestRewriteStandaloneLocalImagesSupportsBalancedParensInDestination(t *testing.T) { + tmp := t.TempDir() + doc := filepath.Join(tmp, "doc.md") + img := filepath.Join(tmp, "diagram(1).png") + if err := os.WriteFile(img, []byte("PNG"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + rewritten, placements, err := RewriteStandaloneLocalImages("![Diagram](./diagram(1).png)\n", doc) + if err != nil { + t.Fatalf("RewriteStandaloneLocalImages: %v", err) + } + if len(placements) != 1 { + t.Fatalf("len(placements) = %d, want 1", len(placements)) + } + if placements[0].Resolved != img { + t.Fatalf("Resolved = %q, want %q", placements[0].Resolved, img) + } + if !strings.Contains(rewritten, placements[0].Placeholder) { + t.Fatalf("rewritten markdown missing placeholder: %q", rewritten) + } +} + +func TestRewriteStandaloneLocalImagesSupportsEscapedParensInDestination(t *testing.T) { + tmp := t.TempDir() + doc := filepath.Join(tmp, "doc.md") + img := filepath.Join(tmp, "diagram(final).png") + if err := os.WriteFile(img, []byte("PNG"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + rewritten, placements, err := RewriteStandaloneLocalImages(`![Diagram](./diagram\(final\).png)`+"\n", doc) + if err != nil { + t.Fatalf("RewriteStandaloneLocalImages: %v", err) + } + if len(placements) != 1 { + t.Fatalf("len(placements) = %d, want 1", len(placements)) + } + if placements[0].Resolved != img { + t.Fatalf("Resolved = %q, want %q", placements[0].Resolved, img) + } + if placements[0].Original != "./diagram(final).png" { + t.Fatalf("Original = %q, want unescaped destination", placements[0].Original) + } + if !strings.Contains(rewritten, placements[0].Placeholder) { + t.Fatalf("rewritten markdown missing placeholder: %q", rewritten) + } +} + +func TestFindStandaloneLocalImageLinesSkipsBacktickFencedCodeBlock(t *testing.T) { + markdown := strings.Join([]string{ + "before", + "", + "```", + "![Demo](./example.png)", + "```", + "", + "after", + "", + }, "\n") + + rewritten, placements, err := FindStandaloneLocalImageLines(markdown) + if err != nil { + t.Fatalf("FindStandaloneLocalImageLines: %v", err) + } + if len(placements) != 0 { + t.Fatalf("len(placements) = %d, want 0 (image inside fenced code block should be ignored)", len(placements)) + } + if rewritten != markdown { + t.Fatalf("rewritten mutated code block content:\n%s", rewritten) + } +} + +func TestFindStandaloneLocalImageLinesSkipsTildeFencedCodeBlock(t *testing.T) { + markdown := strings.Join([]string{ + "~~~markdown", + "![Demo](./example.png)", + "~~~", + "", + }, "\n") + + _, placements, err := FindStandaloneLocalImageLines(markdown) + if err != nil { + t.Fatalf("FindStandaloneLocalImageLines: %v", err) + } + if len(placements) != 0 { + t.Fatalf("len(placements) = %d, want 0 (image inside tilde-fenced code block should be ignored)", len(placements)) + } +} + +func TestFindStandaloneLocalImageLinesSkipsIndentedCodeBlock(t *testing.T) { + markdown := strings.Join([]string{ + "before paragraph", + "", + " ![Demo](./example.png)", + "", + "after paragraph", + "", + }, "\n") + + rewritten, placements, err := FindStandaloneLocalImageLines(markdown) + if err != nil { + t.Fatalf("FindStandaloneLocalImageLines: %v", err) + } + if len(placements) != 0 { + t.Fatalf("len(placements) = %d, want 0 (image inside indented code block should be ignored)", len(placements)) + } + if rewritten != markdown { + t.Fatalf("rewritten mutated indented code block:\n%s", rewritten) + } +} + +func TestFindStandaloneLocalImageLinesTreatsImageOutsideFenceAsStandalone(t *testing.T) { + markdown := strings.Join([]string{ + "```", + "fenced example", + "```", + "", + "![Real](./diagram.png)", + "", + }, "\n") + + rewritten, placements, err := FindStandaloneLocalImageLines(markdown) + if err != nil { + t.Fatalf("FindStandaloneLocalImageLines: %v", err) + } + if len(placements) != 1 { + t.Fatalf("len(placements) = %d, want 1", len(placements)) + } + if strings.Contains(rewritten, "./diagram.png") { + t.Fatalf("rewritten should have replaced post-fence image line with a placeholder: %q", rewritten) + } +} From 5ff612a759814ac84a48c4738f80dacffef95680 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 10:38:00 -0400 Subject: [PATCH 10/31] fix(page): restore empty snapshots during local-image rollback Drop the early-return that treated a blank snapshot markdown as "nothing to restore". An empty or whitespace-only snapshot is a valid prior page state, so rollback should still issue replace_content instead of leaving placeholder-filled content on the page when image substitution fails after a replace on a previously empty page. Route rollback through a pageUpdater interface so the behavior is covered by tests without a live MCP client. --- cmd/page_local_images.go | 8 ++++-- cmd/page_local_images_test.go | 54 +++++++++++++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/cmd/page_local_images.go b/cmd/page_local_images.go index 8c2f95c..5b60b18 100644 --- a/cmd/page_local_images.go +++ b/cmd/page_local_images.go @@ -141,8 +141,12 @@ func substituteUploadedLocalImages(cmdCtx *Context, ctx context.Context, pageID return nil } -func rollbackSyncedPage(ctx context.Context, client *mcp.Client, pageID string, snapshot *api.PageMarkdown) error { - if snapshot == nil || strings.TrimSpace(snapshot.Markdown) == "" { +type pageUpdater interface { + UpdatePage(ctx context.Context, req mcp.UpdatePageRequest) error +} + +func rollbackSyncedPage(ctx context.Context, client pageUpdater, pageID string, snapshot *api.PageMarkdown) error { + if snapshot == nil { return nil } if snapshot.Truncated { diff --git a/cmd/page_local_images_test.go b/cmd/page_local_images_test.go index 1f1fbc1..72bc07c 100644 --- a/cmd/page_local_images_test.go +++ b/cmd/page_local_images_test.go @@ -11,8 +11,19 @@ import ( "testing" "github.com/lox/notion-cli/internal/api" + "github.com/lox/notion-cli/internal/mcp" ) +type fakePageUpdater struct { + calls []mcp.UpdatePageRequest + err error +} + +func (f *fakePageUpdater) UpdatePage(_ context.Context, req mcp.UpdatePageRequest) error { + f.calls = append(f.calls, req) + return f.err +} + func TestPrepareLocalImageUploadsUploadsAndDeduplicates(t *testing.T) { tmp := t.TempDir() doc := filepath.Join(tmp, "doc.md") @@ -170,9 +181,48 @@ func TestRollbackSyncedPageSkipsNilSnapshot(t *testing.T) { } } -func TestRollbackSyncedPageSkipsEmptyMarkdownSnapshot(t *testing.T) { +func TestRollbackSyncedPageReplacesEmptyMarkdownSnapshot(t *testing.T) { + snapshot := &api.PageMarkdown{Markdown: ""} + updater := &fakePageUpdater{} + + if err := rollbackSyncedPage(context.Background(), updater, "page-id", snapshot); err != nil { + t.Fatalf("rollbackSyncedPage returned %v, want nil", err) + } + if len(updater.calls) != 1 { + t.Fatalf("len(updater.calls) = %d, want 1 (empty snapshot should still restore previous empty state)", len(updater.calls)) + } + got := updater.calls[0] + if got.PageID != "page-id" || got.Command != "replace_content" || got.NewContent != "" { + t.Fatalf("unexpected UpdatePage request: %#v", got) + } +} + +func TestRollbackSyncedPageReplacesWhitespaceSnapshot(t *testing.T) { snapshot := &api.PageMarkdown{Markdown: " \n"} - if err := rollbackSyncedPage(context.Background(), nil, "page-id", snapshot); err != nil { + updater := &fakePageUpdater{} + + if err := rollbackSyncedPage(context.Background(), updater, "page-id", snapshot); err != nil { + t.Fatalf("rollbackSyncedPage returned %v, want nil", err) + } + if len(updater.calls) != 1 { + t.Fatalf("len(updater.calls) = %d, want 1", len(updater.calls)) + } + if got := updater.calls[0].NewContent; got != " \n" { + t.Fatalf("NewContent = %q, want whitespace snapshot preserved verbatim", got) + } +} + +func TestRollbackSyncedPageReplacesNonEmptySnapshot(t *testing.T) { + snapshot := &api.PageMarkdown{Markdown: "# Previous\n\nbody\n"} + updater := &fakePageUpdater{} + + if err := rollbackSyncedPage(context.Background(), updater, "page-id", snapshot); err != nil { t.Fatalf("rollbackSyncedPage returned %v, want nil", err) } + if len(updater.calls) != 1 { + t.Fatalf("len(updater.calls) = %d, want 1", len(updater.calls)) + } + if got := updater.calls[0].NewContent; got != "# Previous\n\nbody\n" { + t.Fatalf("NewContent = %q, want snapshot markdown", got) + } } From 729b00e2b9759c4ba82d48c2f9c1f57e50d1875a Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 11:23:27 -0400 Subject: [PATCH 11/31] fix(page): handle nested brackets and protocol-relative image URLs Track `[`/`]` depth inside alt-text parsing so images with nested brackets like `![Architecture [v2]](./diagram.png)` are recognized as a single standalone image, and treat `//cdn.example.com/...` destinations as remote so protocol-relative URLs are not mistaken for local paths. --- internal/cli/local_images.go | 21 ++++++++++++----- internal/cli/local_images_test.go | 39 +++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/internal/cli/local_images.go b/internal/cli/local_images.go index 2d33db5..8e944b0 100644 --- a/internal/cli/local_images.go +++ b/internal/cli/local_images.go @@ -196,16 +196,25 @@ func findMarkdownImages(line string) []markdownImageMatch { } // findLinkTextEnd walks `line` from `start` and returns the offset of the -// unescaped `]` that closes the link text, honoring `\]` escapes. +// unescaped `]` that closes the link text, honoring `\]` escapes and balanced +// nested `[`/`]` pairs so alt text like `Architecture [v2]` parses as a single +// image. func findLinkTextEnd(line string, start int) (int, bool) { + depth := 0 for i := start; i < len(line); i++ { c := line[i] if c == '\\' && i+1 < len(line) { i++ continue } - if c == ']' { - return i, true + switch c { + case '[': + depth++ + case ']': + if depth == 0 { + return i, true + } + depth-- } } return 0, false @@ -409,9 +418,9 @@ func isLocalDestination(dest string) bool { lower := strings.ToLower(d) switch { - case strings.HasPrefix(lower, "#"): - return false - case strings.HasPrefix(lower, "http://"), + case strings.HasPrefix(lower, "#"), + strings.HasPrefix(lower, "//"), + strings.HasPrefix(lower, "http://"), strings.HasPrefix(lower, "https://"), strings.HasPrefix(lower, "mailto:"), strings.HasPrefix(lower, "tel:"), diff --git a/internal/cli/local_images_test.go b/internal/cli/local_images_test.go index ae1fda4..d5b5bb9 100644 --- a/internal/cli/local_images_test.go +++ b/internal/cli/local_images_test.go @@ -233,6 +233,45 @@ func TestFindStandaloneLocalImageLinesSkipsIndentedCodeBlock(t *testing.T) { } } +func TestRewriteStandaloneLocalImagesSupportsNestedBracketsInAltText(t *testing.T) { + tmp := t.TempDir() + doc := filepath.Join(tmp, "doc.md") + img := filepath.Join(tmp, "diagram.png") + if err := os.WriteFile(img, []byte("PNG"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + rewritten, placements, err := RewriteStandaloneLocalImages("![Architecture [v2]](./diagram.png)\n", doc) + if err != nil { + t.Fatalf("RewriteStandaloneLocalImages: %v", err) + } + if len(placements) != 1 { + t.Fatalf("len(placements) = %d, want 1", len(placements)) + } + if placements[0].Alt != "Architecture [v2]" { + t.Fatalf("Alt = %q, want \"Architecture [v2]\"", placements[0].Alt) + } + if placements[0].Resolved != img { + t.Fatalf("Resolved = %q, want %q", placements[0].Resolved, img) + } + if strings.Contains(rewritten, "./diagram.png") { + t.Fatalf("rewritten should have replaced nested-bracket image line: %q", rewritten) + } +} + +func TestFindStandaloneLocalImageLinesIgnoresProtocolRelativeURLs(t *testing.T) { + rewritten, placements, err := FindStandaloneLocalImageLines("![CDN](//cdn.example.com/image.png)\n") + if err != nil { + t.Fatalf("FindStandaloneLocalImageLines: %v", err) + } + if len(placements) != 0 { + t.Fatalf("len(placements) = %d, want 0 (protocol-relative URL should be treated as remote)", len(placements)) + } + if rewritten != "![CDN](//cdn.example.com/image.png)\n" { + t.Fatalf("rewritten = %q, want input unchanged", rewritten) + } +} + func TestFindStandaloneLocalImageLinesTreatsImageOutsideFenceAsStandalone(t *testing.T) { markdown := strings.Join([]string{ "```", From 315f5317d349b9b63152f9d5b4abfd60396d234f Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 11:23:27 -0400 Subject: [PATCH 12/31] fix(page): fail local-image substitution when page ID is missing Previously substituteUploadedLocalImages silently returned success when pageID was empty but uploads were pending, leaving `NOTION_CLI_LOCAL_IMAGE_*` placeholders in the published page whenever CreatePage's fallback path could not extract an ID. Return an explicit error in that case so upload/sync surfaces the failure instead of reporting success with corrupted content. --- cmd/page_local_images.go | 5 ++++- cmd/page_local_images_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/cmd/page_local_images.go b/cmd/page_local_images.go index 5b60b18..64c96bf 100644 --- a/cmd/page_local_images.go +++ b/cmd/page_local_images.go @@ -96,9 +96,12 @@ func requireLocalImageParent(uploads []uploadedLocalImage, parent, parentDB stri } func substituteUploadedLocalImages(cmdCtx *Context, ctx context.Context, pageID string, uploads []uploadedLocalImage) error { - if strings.TrimSpace(pageID) == "" || len(uploads) == 0 { + if len(uploads) == 0 { return nil } + if strings.TrimSpace(pageID) == "" { + return fmt.Errorf("cannot substitute %d local image(s): missing page ID", len(uploads)) + } apiClient, err := cli.RequireOfficialAPIClient(officialAPIOverrides(cmdCtx)) if err != nil { diff --git a/cmd/page_local_images_test.go b/cmd/page_local_images_test.go index 72bc07c..4862290 100644 --- a/cmd/page_local_images_test.go +++ b/cmd/page_local_images_test.go @@ -134,6 +134,31 @@ func TestSubstituteUploadedLocalImagesAppendsAfterPlaceholderAndDeletes(t *testi } } +func TestSubstituteUploadedLocalImagesErrorsWhenPageIDMissing(t *testing.T) { + cmdCtx := &Context{APIToken: "secret-token"} + uploads := []uploadedLocalImage{{ + Alt: "Diagram", + FileUploadID: "upload_123", + Placeholder: "PLACEHOLDER", + ResolvedPath: "/tmp/diagram.png", + }} + + err := substituteUploadedLocalImages(cmdCtx, context.Background(), " ", uploads) + if err == nil { + t.Fatalf("expected error when pageID is empty with pending uploads") + } + if !strings.Contains(err.Error(), "missing page ID") { + t.Fatalf("error = %v, want missing-page-ID message", err) + } +} + +func TestSubstituteUploadedLocalImagesSkipsWithoutUploads(t *testing.T) { + cmdCtx := &Context{APIToken: "secret-token"} + if err := substituteUploadedLocalImages(cmdCtx, context.Background(), "", nil); err != nil { + t.Fatalf("expected nil when no uploads, got %v", err) + } +} + func TestRequireLocalImageParent(t *testing.T) { uploads := []uploadedLocalImage{{ Placeholder: "PLACEHOLDER", From e8ec25b641bb65ca7d68e743756e86f8d44d41bb Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 11:42:11 -0400 Subject: [PATCH 13/31] fix(page): skip escaped image markers and titles in destinations Treat `\![...]` as literal text so documentation that shows image syntax doesn't trip the unsupported-inline-syntax error, and accept an optional whitespace-separated title (in `"..."`, `'...'`, or `(...)` form) after angle-bracketed destinations so valid markdown like `![Alt](<./diagram 1.png> "caption")` parses correctly instead of being silently skipped. --- internal/cli/local_images.go | 71 +++++++++++++++++++++++++++---- internal/cli/local_images_test.go | 37 ++++++++++++++++ 2 files changed, 99 insertions(+), 9 deletions(-) diff --git a/internal/cli/local_images.go b/internal/cli/local_images.go index 8e944b0..28a6b45 100644 --- a/internal/cli/local_images.go +++ b/internal/cli/local_images.go @@ -158,12 +158,18 @@ type markdownImageMatch struct { // findMarkdownImages returns every `![alt](dest)` span on a single line. // Destinations may contain balanced parentheses and `\)` / `\(` escapes, and -// may optionally be wrapped in angle brackets (``). The returned `dest` -// is the raw text between the opening `(` and the matching `)`, preserving -// escapes for parseMarkdownDestination to normalize. +// may optionally be wrapped in angle brackets (``). A backslash before +// `!` escapes the image marker so `\![...]` is treated as literal text. +// The returned `dest` is the raw text between the opening `(` and the +// matching `)`, preserving escapes for parseMarkdownDestination to normalize. func findMarkdownImages(line string) []markdownImageMatch { var matches []markdownImageMatch - for i := 0; i < len(line); { + i := 0 + for i < len(line) { + if line[i] == '\\' && i+1 < len(line) { + i += 2 + continue + } if line[i] != '!' || i+1 >= len(line) || line[i+1] != '[' { i++ continue @@ -223,7 +229,8 @@ func findLinkTextEnd(line string, start int) (int, bool) { // findDestinationEnd returns the offset of the `)` that closes a markdown // destination starting at `start`. Balanced parens and `\(` / `\)` escapes // inside the destination are preserved; angle-bracketed destinations end at -// the first unescaped `>` followed immediately by `)`. +// the first unescaped `>`, optionally followed by a whitespace-separated title +// in `"..."`, `'...'`, or `(...)` form before the closing `)`. func findDestinationEnd(line string, start int) (int, bool) { if start < len(line) && line[start] == '<' { for i := start + 1; i < len(line); i++ { @@ -233,10 +240,7 @@ func findDestinationEnd(line string, start int) (int, bool) { continue } if c == '>' { - if i+1 < len(line) && line[i+1] == ')' { - return i + 1, true - } - return 0, false + return skipOptionalTitleAndClose(line, i+1) } } return 0, false @@ -262,6 +266,55 @@ func findDestinationEnd(line string, start int) (int, bool) { return 0, false } +// skipOptionalTitleAndClose returns the offset of the `)` that terminates a +// markdown image after an optional whitespace-separated title, starting at +// the first character after the destination (or after the closing `>` of an +// angle-bracketed destination). +func skipOptionalTitleAndClose(line string, i int) (int, bool) { + for i < len(line) && (line[i] == ' ' || line[i] == '\t') { + i++ + } + if i >= len(line) { + return 0, false + } + if line[i] == ')' { + return i, true + } + var closeQuote byte + switch line[i] { + case '"': + closeQuote = '"' + case '\'': + closeQuote = '\'' + case '(': + closeQuote = ')' + default: + return 0, false + } + for i++; i < len(line); i++ { + if line[i] == '\\' && i+1 < len(line) { + i++ + continue + } + if line[i] == closeQuote { + break + } + } + if i >= len(line) { + return 0, false + } + for i++; i < len(line); i++ { + if line[i] == ' ' || line[i] == '\t' { + continue + } + if line[i] == ')' { + return i, true + } + return 0, false + } + return 0, false +} + // isStandaloneImageLine reports whether `line` contains exactly one image and // no other non-whitespace content. func isStandaloneImageLine(line string, matches []markdownImageMatch) bool { diff --git a/internal/cli/local_images_test.go b/internal/cli/local_images_test.go index d5b5bb9..f21c00d 100644 --- a/internal/cli/local_images_test.go +++ b/internal/cli/local_images_test.go @@ -272,6 +272,43 @@ func TestFindStandaloneLocalImageLinesIgnoresProtocolRelativeURLs(t *testing.T) } } +func TestRewriteStandaloneLocalImagesSupportsAngleBracketDestinationWithTitle(t *testing.T) { + tmp := t.TempDir() + doc := filepath.Join(tmp, "doc.md") + img := filepath.Join(tmp, "diagram 1.png") + if err := os.WriteFile(img, []byte("PNG"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + rewritten, placements, err := RewriteStandaloneLocalImages(`![Alt](<./diagram 1.png> "caption")`+"\n", doc) + if err != nil { + t.Fatalf("RewriteStandaloneLocalImages: %v", err) + } + if len(placements) != 1 { + t.Fatalf("len(placements) = %d, want 1", len(placements)) + } + if placements[0].Resolved != img { + t.Fatalf("Resolved = %q, want %q", placements[0].Resolved, img) + } + if strings.Contains(rewritten, "./diagram 1.png") { + t.Fatalf("rewritten should have replaced angle-bracket image line: %q", rewritten) + } +} + +func TestFindStandaloneLocalImageLinesIgnoresEscapedImageMarker(t *testing.T) { + input := `\![Demo](./example.png)` + "\n" + rewritten, placements, err := FindStandaloneLocalImageLines(input) + if err != nil { + t.Fatalf("FindStandaloneLocalImageLines: %v", err) + } + if len(placements) != 0 { + t.Fatalf("len(placements) = %d, want 0 (escaped ! should be literal text)", len(placements)) + } + if rewritten != input { + t.Fatalf("rewritten = %q, want input unchanged", rewritten) + } +} + func TestFindStandaloneLocalImageLinesTreatsImageOutsideFenceAsStandalone(t *testing.T) { markdown := strings.Join([]string{ "```", From d31965d973bd6ed40a0a9c1b698acfd480afec4a Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 11:42:11 -0400 Subject: [PATCH 14/31] fix(page): check local-image parent before uploading page upload and create-mode page sync now verify --parent/--parent-db is set before prepareLocalImageUploads runs, so a missing-parent user error is raised up front instead of after local files have already been uploaded to Notion. Replaces requireLocalImageParent with a pre-upload checkLocalImageParent helper that scans markdown via FindStandaloneLocalImageLines. --- cmd/page.go | 19 +++++++++---------- cmd/page_local_images.go | 12 ++++++++++-- cmd/page_local_images_test.go | 17 ++++++++--------- 3 files changed, 27 insertions(+), 21 deletions(-) diff --git a/cmd/page.go b/cmd/page.go index 0805a39..3ac1b30 100644 --- a/cmd/page.go +++ b/cmd/page.go @@ -281,12 +281,12 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string, ski return err } } else { - markdown, localUploads, err = prepareLocalImageUploads(ctx, bgCtx, file, markdown) - if err != nil { + if err := checkLocalImageParent(markdown, parent, parentDB); err != nil { output.PrintError(err) return err } - if err := requireLocalImageParent(localUploads, parent, parentDB); err != nil { + markdown, localUploads, err = prepareLocalImageUploads(ctx, bgCtx, file, markdown) + if err != nil { output.PrintError(err) return err } @@ -579,6 +579,12 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL return err } } else { + if fm.NotionID == "" { + if err := checkLocalImageParent(body, parent, parentDB); err != nil { + output.PrintError(err) + return err + } + } body, localUploads, err = prepareLocalImageUploads(ctx, bgCtx, file, body) if err != nil { output.PrintError(err) @@ -654,13 +660,6 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL return nil } - if !skipLocalImages { - if err := requireLocalImageParent(localUploads, parent, parentDB); err != nil { - output.PrintError(err) - return err - } - } - req := mcp.CreatePageRequest{ Title: title, Content: body, diff --git a/cmd/page_local_images.go b/cmd/page_local_images.go index 64c96bf..fcc9c3f 100644 --- a/cmd/page_local_images.go +++ b/cmd/page_local_images.go @@ -83,8 +83,16 @@ func stripLocalImages(markdown string) (string, error) { return strings.Join(lines, "\n"), nil } -func requireLocalImageParent(uploads []uploadedLocalImage, parent, parentDB string) error { - if len(uploads) == 0 { +// checkLocalImageParent scans markdown for standalone local images without +// performing remote uploads and returns a parent-required error when the user +// provided neither --parent nor --parent-db. Call before prepareLocalImageUploads +// so missing-parent failures don't leave orphaned file uploads in Notion. +func checkLocalImageParent(markdown, parent, parentDB string) error { + _, placements, err := cli.FindStandaloneLocalImageLines(markdown) + if err != nil { + return err + } + if len(placements) == 0 { return nil } if strings.TrimSpace(parent) != "" || strings.TrimSpace(parentDB) != "" { diff --git a/cmd/page_local_images_test.go b/cmd/page_local_images_test.go index 4862290..1e48517 100644 --- a/cmd/page_local_images_test.go +++ b/cmd/page_local_images_test.go @@ -159,22 +159,21 @@ func TestSubstituteUploadedLocalImagesSkipsWithoutUploads(t *testing.T) { } } -func TestRequireLocalImageParent(t *testing.T) { - uploads := []uploadedLocalImage{{ - Placeholder: "PLACEHOLDER", - }} +func TestCheckLocalImageParent(t *testing.T) { + const markdownWithImage = "![Diagram](./diagram.png)\n" + const markdownWithoutImage = "# Just text\n" - if err := requireLocalImageParent(nil, "", ""); err != nil { - t.Fatalf("expected nil without uploads, got %v", err) + if err := checkLocalImageParent(markdownWithoutImage, "", ""); err != nil { + t.Fatalf("expected nil without local images, got %v", err) } - if err := requireLocalImageParent(uploads, "parent-id", ""); err != nil { + if err := checkLocalImageParent(markdownWithImage, "parent-id", ""); err != nil { t.Fatalf("expected nil with parent, got %v", err) } - if err := requireLocalImageParent(uploads, "", "db-id"); err != nil { + if err := checkLocalImageParent(markdownWithImage, "", "db-id"); err != nil { t.Fatalf("expected nil with parent db, got %v", err) } - err := requireLocalImageParent(uploads, "", "") + err := checkLocalImageParent(markdownWithImage, "", "") if err == nil { t.Fatal("expected error without parent or parent db") } From c4f38245fa095e3e0465b343acba739c046d9576 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 12:52:58 -0400 Subject: [PATCH 15/31] refactor(page): stream uploads, parallelize, share constants --- cmd/page.go | 32 +++--------- cmd/page_local_images.go | 100 +++++++++++++++++++++++++++++------- go.mod | 1 + go.sum | 2 + internal/api/client.go | 96 +++++++++++++++++++++++----------- internal/api/client_test.go | 4 +- 6 files changed, 158 insertions(+), 77 deletions(-) diff --git a/cmd/page.go b/cmd/page.go index 3ac1b30..1f6b2af 100644 --- a/cmd/page.go +++ b/cmd/page.go @@ -341,19 +341,9 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string, ski return err } pageID := pageIDFromCreateResponse(resp) - if err := substituteUploadedLocalImages(ctx, bgCtx, pageID, localUploads); err != nil { - finalErr := fmt.Errorf("insert uploaded local images: %w", err) - if pageID != "" { - if apiClient, apiErr := cli.RequireOfficialAPIClient(officialAPIOverrides(ctx)); apiErr == nil { - if cleanupErr := apiClient.TrashPage(bgCtx, pageID); cleanupErr != nil { - finalErr = fmt.Errorf("%w (cleanup failed: %v)", finalErr, cleanupErr) - } - } else { - finalErr = fmt.Errorf("%w (cleanup client init failed: %v)", finalErr, apiErr) - } - } - output.PrintError(finalErr) - return finalErr + if err := substituteOrCleanup(ctx, bgCtx, pageID, localUploads); err != nil { + output.PrintError(err) + return err } displayTitle := title @@ -693,19 +683,9 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL } pageID := pageIDFromCreateResponse(resp) - if err := substituteUploadedLocalImages(ctx, bgCtx, pageID, localUploads); err != nil { - finalErr := fmt.Errorf("insert uploaded local images: %w", err) - if pageID != "" { - if apiClient, apiErr := cli.RequireOfficialAPIClient(officialAPIOverrides(ctx)); apiErr == nil { - if cleanupErr := apiClient.TrashPage(bgCtx, pageID); cleanupErr != nil { - finalErr = fmt.Errorf("%w (cleanup failed: %v)", finalErr, cleanupErr) - } - } else { - finalErr = fmt.Errorf("%w (cleanup client init failed: %v)", finalErr, apiErr) - } - } - output.PrintError(finalErr) - return finalErr + if err := substituteOrCleanup(ctx, bgCtx, pageID, localUploads); err != nil { + output.PrintError(err) + return err } if pageID == "" { output.PrintWarning("Page created but could not retrieve ID for frontmatter") diff --git a/cmd/page_local_images.go b/cmd/page_local_images.go index fcc9c3f..125baf9 100644 --- a/cmd/page_local_images.go +++ b/cmd/page_local_images.go @@ -5,6 +5,9 @@ import ( "fmt" "os" "strings" + "sync" + + "golang.org/x/sync/errgroup" "github.com/lox/notion-cli/internal/api" "github.com/lox/notion-cli/internal/cli" @@ -12,6 +15,9 @@ import ( "github.com/lox/notion-cli/internal/output" ) +// localImageUploadConcurrency caps concurrent local image uploads. +const localImageUploadConcurrency = 4 + type uploadedLocalImage struct { Alt string FileUploadID string @@ -33,25 +39,46 @@ func prepareLocalImageUploads(cmdCtx *Context, ctx context.Context, sourceFile, return "", nil, err } - uploadIDByPath := make(map[string]string, len(placements)) - uploads := make([]uploadedLocalImage, 0, len(placements)) + // Collect the unique set of resolved paths to upload so we do one upload + // per distinct file even when the same image appears in multiple + // placements. + distinctPaths := make([]string, 0, len(placements)) + seen := make(map[string]struct{}, len(placements)) for _, placement := range placements { - uploadID, ok := uploadIDByPath[placement.Resolved] - if !ok { - fileData, err := os.ReadFile(placement.Resolved) - if err != nil { - return "", nil, fmt.Errorf("read local image %q: %w", placement.Resolved, err) - } - uploadID, err = apiClient.UploadFile(ctx, placement.Resolved, fileData) + if _, ok := seen[placement.Resolved]; ok { + continue + } + seen[placement.Resolved] = struct{}{} + distinctPaths = append(distinctPaths, placement.Resolved) + } + + uploadIDByPath := make(map[string]string, len(distinctPaths)) + var mu sync.Mutex + + group, gctx := errgroup.WithContext(ctx) + group.SetLimit(localImageUploadConcurrency) + for _, path := range distinctPaths { + path := path + group.Go(func() error { + uploadID, err := uploadLocalImage(gctx, apiClient, path) if err != nil { - return "", nil, fmt.Errorf("upload local image %q: %w", placement.Resolved, err) + return err } - uploadIDByPath[placement.Resolved] = uploadID - } + mu.Lock() + uploadIDByPath[path] = uploadID + mu.Unlock() + return nil + }) + } + if err := group.Wait(); err != nil { + return "", nil, err + } + uploads := make([]uploadedLocalImage, 0, len(placements)) + for _, placement := range placements { uploads = append(uploads, uploadedLocalImage{ Alt: placement.Alt, - FileUploadID: uploadID, + FileUploadID: uploadIDByPath[placement.Resolved], Placeholder: placement.Placeholder, ResolvedPath: placement.Resolved, }) @@ -60,6 +87,25 @@ func prepareLocalImageUploads(cmdCtx *Context, ctx context.Context, sourceFile, return rewritten, uploads, nil } +func uploadLocalImage(ctx context.Context, apiClient *api.Client, path string) (string, error) { + f, err := os.Open(path) + if err != nil { + return "", fmt.Errorf("open local image %q: %w", path, err) + } + defer func() { _ = f.Close() }() + + info, err := f.Stat() + if err != nil { + return "", fmt.Errorf("stat local image %q: %w", path, err) + } + + uploadID, err := apiClient.UploadFile(ctx, path, info.Size(), f) + if err != nil { + return "", fmt.Errorf("upload local image %q: %w", path, err) + } + return uploadID, nil +} + func stripLocalImages(markdown string) (string, error) { rewritten, placements, err := cli.FindStandaloneLocalImageLines(markdown) if err != nil { @@ -83,10 +129,9 @@ func stripLocalImages(markdown string) (string, error) { return strings.Join(lines, "\n"), nil } -// checkLocalImageParent scans markdown for standalone local images without -// performing remote uploads and returns a parent-required error when the user -// provided neither --parent nor --parent-db. Call before prepareLocalImageUploads -// so missing-parent failures don't leave orphaned file uploads in Notion. +// checkLocalImageParent returns a parent-required error when markdown has +// standalone local images but the caller provided neither --parent nor +// --parent-db. func checkLocalImageParent(markdown, parent, parentDB string) error { _, placements, err := cli.FindStandaloneLocalImageLines(markdown) if err != nil { @@ -103,6 +148,25 @@ func checkLocalImageParent(markdown, parent, parentDB string) error { } } +// substituteOrCleanup substitutes uploaded local images into the page and +// trashes the page on failure so it doesn't linger with placeholders. +func substituteOrCleanup(cmdCtx *Context, ctx context.Context, pageID string, uploads []uploadedLocalImage) error { + if err := substituteUploadedLocalImages(cmdCtx, ctx, pageID, uploads); err != nil { + finalErr := fmt.Errorf("insert uploaded local images: %w", err) + if pageID != "" { + if apiClient, apiErr := cli.RequireOfficialAPIClient(officialAPIOverrides(cmdCtx)); apiErr == nil { + if cleanupErr := apiClient.TrashPage(ctx, pageID); cleanupErr != nil { + finalErr = fmt.Errorf("%w (cleanup failed: %v)", finalErr, cleanupErr) + } + } else { + finalErr = fmt.Errorf("%w (cleanup client init failed: %v)", finalErr, apiErr) + } + } + return finalErr + } + return nil +} + func substituteUploadedLocalImages(cmdCtx *Context, ctx context.Context, pageID string, uploads []uploadedLocalImage) error { if len(uploads) == 0 { return nil @@ -123,7 +187,7 @@ func substituteUploadedLocalImages(cmdCtx *Context, ctx context.Context, pageID blocksByPlaceholder := make(map[string]api.Block, len(uploads)) for _, block := range blocks { - if block.Type != "paragraph" || block.Paragraph == nil { + if block.Type != api.BlockTypeParagraph || block.Paragraph == nil { continue } text := paragraphPlainText(block) diff --git a/go.mod b/go.mod index 4581ec5..cdd1d87 100644 --- a/go.mod +++ b/go.mod @@ -42,6 +42,7 @@ require ( github.com/yosida95/uritemplate/v3 v3.0.2 // indirect github.com/yuin/goldmark v1.7.8 // indirect github.com/yuin/goldmark-emoji v1.0.5 // indirect + golang.org/x/sync v0.20.0 // indirect golang.org/x/sys v0.40.0 // indirect golang.org/x/text v0.33.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect diff --git a/go.sum b/go.sum index e312146..c56b2ac 100644 --- a/go.sum +++ b/go.sum @@ -102,6 +102,8 @@ golang.org/x/exp v0.0.0-20220909182711-5c715a9e8561 h1:MDc5xs78ZrZr3HMQugiXOAkSZ golang.org/x/exp v0.0.0-20220909182711-5c715a9e8561/go.mod h1:cyybsKvd6eL0RnXn6p/Grxp8F5bW7iYuBgsNCOHpMYE= golang.org/x/net v0.49.0 h1:eeHFmOGUTtaaPSGNmjBKpbng9MulQsJURQUAfUwY++o= golang.org/x/net v0.49.0/go.mod h1:/ysNB2EvaqvesRkuLAyjI1ycPZlQHM3q01F02UY/MV8= +golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4= +golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.40.0 h1:DBZZqJ2Rkml6QMQsZywtnjnnGvHza6BTfYFWY9kjEWQ= diff --git a/internal/api/client.go b/internal/api/client.go index a85ee83..8e35cac 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -1,6 +1,7 @@ package api import ( + "bufio" "bytes" "context" "encoding/json" @@ -19,6 +20,13 @@ import ( const defaultHTTPTimeout = 20 * time.Second +// Notion block type strings used when appending or inspecting blocks. +const ( + BlockTypeParagraph = "paragraph" + BlockTypeImage = "image" + BlockTypeFileUpload = "file_upload" +) + var ( fileUploadPollInterval = 250 * time.Millisecond fileUploadMaxChecks = 240 @@ -126,19 +134,26 @@ func (c *Client) GetPageMarkdown(ctx context.Context, pageID string) (*PageMarkd return &out, nil } -func (c *Client) UploadFile(ctx context.Context, filename string, data []byte) (string, error) { - filename = strings.TrimSpace(filepath.Base(filename)) - if filename == "" { +// UploadFile streams a file upload to the Notion API without buffering the whole +// payload in memory. The reader is consumed through a multipart writer piped +// directly into the HTTP request body. Pass the exact byte size so the server +// can validate the stream. +func (c *Client) UploadFile(ctx context.Context, name string, size int64, body io.Reader) (string, error) { + name = strings.TrimSpace(filepath.Base(name)) + if name == "" { return "", fmt.Errorf("filename is required") } - if len(data) == 0 { - return "", fmt.Errorf("file data is required") + if size <= 0 { + return "", fmt.Errorf("file size must be positive") + } + if body == nil { + return "", fmt.Errorf("file body is required") } var created FileUpload createPayload := map[string]any{ "mode": "single_part", - "filename": filename, + "filename": name, } if err := c.doJSON(ctx, http.MethodPost, "/file_uploads", createPayload, &created); err != nil { return "", err @@ -147,7 +162,7 @@ func (c *Client) UploadFile(ctx context.Context, filename string, data []byte) ( return "", fmt.Errorf("create file upload failed: empty upload ID") } - if _, err := c.sendFileUploadPart(ctx, created.ID, filename, data); err != nil { + if _, err := c.sendFileUploadPart(ctx, created.ID, name, size, body); err != nil { return "", err } @@ -158,6 +173,12 @@ func (c *Client) UploadFile(ctx context.Context, filename string, data []byte) ( return uploaded.ID, nil } +// UploadFileBytes is a convenience wrapper for callers that already have the +// file contents in memory. +func (c *Client) UploadFileBytes(ctx context.Context, name string, data []byte) (string, error) { + return c.UploadFile(ctx, name, int64(len(data)), bytes.NewReader(data)) +} + func (c *Client) AppendUploadedImageAfter(ctx context.Context, parentID, afterBlockID string, block UploadedImageBlock) error { parentID = strings.TrimSpace(parentID) afterBlockID = strings.TrimSpace(afterBlockID) @@ -173,8 +194,8 @@ func (c *Client) AppendUploadedImageAfter(ctx context.Context, parentID, afterBl } image := map[string]any{ - "type": "file_upload", - "file_upload": map[string]any{ + "type": BlockTypeFileUpload, + BlockTypeFileUpload: map[string]any{ "id": fileUploadID, }, } @@ -189,14 +210,13 @@ func (c *Client) AppendUploadedImageAfter(ctx context.Context, parentID, afterBl } } + child := map[string]any{ + "object": "block", + "type": BlockTypeImage, + BlockTypeImage: image, + } payload := map[string]any{ - "children": []map[string]any{ - { - "object": "block", - "type": "image", - "image": image, - }, - }, + "children": []map[string]any{child}, "position": map[string]any{ "type": "after_block", "after_block": map[string]any{ @@ -312,9 +332,15 @@ func (c *Client) doRequest(ctx context.Context, method, path string, body io.Rea return nil } -func (c *Client) sendFileUploadPart(ctx context.Context, fileUploadID, filename string, data []byte) (*FileUpload, error) { - var body bytes.Buffer - writer := multipart.NewWriter(&body) +func (c *Client) sendFileUploadPart(ctx context.Context, fileUploadID, filename string, size int64, body io.Reader) (*FileUpload, error) { + // Peek the first 512 bytes so we can detect the content type without + // buffering the full payload. + br := bufio.NewReaderSize(body, 4096) + peek, _ := br.Peek(512) + contentType := detectUploadContentType(filename, peek) + + pr, pw := io.Pipe() + writer := multipart.NewWriter(pw) header := make(textproto.MIMEHeader) contentDisposition := mime.FormatMediaType("form-data", map[string]string{ @@ -322,27 +348,35 @@ func (c *Client) sendFileUploadPart(ctx context.Context, fileUploadID, filename "filename": filename, }) if strings.TrimSpace(contentDisposition) == "" { + _ = pw.Close() return nil, fmt.Errorf("format multipart content disposition: empty result") } header.Set("Content-Disposition", contentDisposition) - header.Set("Content-Type", detectUploadContentType(filename, data)) + header.Set("Content-Type", contentType) - part, err := writer.CreatePart(header) - if err != nil { - return nil, fmt.Errorf("create multipart file part: %w", err) - } - if _, err := part.Write(data); err != nil { - return nil, fmt.Errorf("write multipart file data: %w", err) - } - if err := writer.Close(); err != nil { - return nil, fmt.Errorf("close multipart writer: %w", err) - } + go func() { + part, err := writer.CreatePart(header) + if err != nil { + _ = pw.CloseWithError(fmt.Errorf("create multipart file part: %w", err)) + return + } + if _, err := io.Copy(part, br); err != nil { + _ = pw.CloseWithError(fmt.Errorf("write multipart file data: %w", err)) + return + } + if err := writer.Close(); err != nil { + _ = pw.CloseWithError(fmt.Errorf("close multipart writer: %w", err)) + return + } + _ = pw.Close() + }() var out FileUpload path := "/file_uploads/" + strings.TrimSpace(fileUploadID) + "/send" - if err := c.doRequest(ctx, http.MethodPost, path, bytes.NewReader(body.Bytes()), writer.FormDataContentType(), &out); err != nil { + if err := c.doRequest(ctx, http.MethodPost, path, pr, writer.FormDataContentType(), &out); err != nil { return nil, err } + _ = size // size is currently advisory; the multipart writer sets its own framing. if strings.TrimSpace(out.ID) == "" { out.ID = strings.TrimSpace(fileUploadID) } diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 8d676f0..95e285e 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -116,7 +116,7 @@ func TestUploadFileAndAppendAfter(t *testing.T) { t.Fatalf("NewClient: %v", err) } - uploadID, err := client.UploadFile(context.Background(), `diag"ram.png`, []byte("PNGDATA")) + uploadID, err := client.UploadFileBytes(context.Background(), `diag"ram.png`, []byte("PNGDATA")) if err != nil { t.Fatalf("UploadFile: %v", err) } @@ -172,7 +172,7 @@ func TestUploadFileRetriesEmptyAndPendingStatuses(t *testing.T) { t.Fatalf("NewClient: %v", err) } - uploadID, err := client.UploadFile(context.Background(), "diagram.png", []byte("PNGDATA")) + uploadID, err := client.UploadFileBytes(context.Background(), "diagram.png", []byte("PNGDATA")) if err != nil { t.Fatalf("UploadFile: %v", err) } From bf87d08f779f9086091ff0ed9d42db297636fc4d Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 13:39:31 -0400 Subject: [PATCH 16/31] fix(page): gate sync on truncated snapshot and strengthen scanner Three fixes motivated by reviewer findings on the local image flow. runPageSync now scans for local images up front, fetches the page markdown snapshot, and fails fast when the snapshot is truncated. Previously we uploaded local images first and only noticed truncation during rollback, leaving every upload orphaned on the official API and the page stuck with placeholder paragraphs. The local image scanner masks inline code spans before looking for image syntax, so documentation text like backtick-![Demo](./x.png)- backtick no longer trips the "must appear on its own line" error and no longer needs a fenced or indented block to survive. The placeholder substitution keeps the original leading whitespace on the image line instead of collapsing to column zero, so images written as indented list continuations are not silently lifted out of their container before upload. --- cmd/page.go | 45 +++++++++++++++++++--------- internal/cli/local_images.go | 49 +++++++++++++++++++++++++++++-- internal/cli/local_images_test.go | 31 +++++++++++++++++++ 3 files changed, 109 insertions(+), 16 deletions(-) diff --git a/cmd/page.go b/cmd/page.go index 1f6b2af..d125488 100644 --- a/cmd/page.go +++ b/cmd/page.go @@ -562,6 +562,7 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL fm, body := cli.ParseFrontmatter(content) bgCtx := context.Background() var localUploads []uploadedLocalImage + var snapshot *api.PageMarkdown if skipLocalImages { body, err = stripLocalImages(body) if err != nil { @@ -569,12 +570,42 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL return err } } else { + // Dry-run scan so we know whether uploads will happen before anything + // goes over the wire. This lets us validate the parent flags and, + // for in-place sync, fetch the rollback snapshot (and gate on + // truncation) before we touch the official API. + _, placements, scanErr := cli.FindStandaloneLocalImageLines(body) + if scanErr != nil { + output.PrintError(scanErr) + return scanErr + } + hasLocalImages := len(placements) > 0 + if fm.NotionID == "" { if err := checkLocalImageParent(body, parent, parentDB); err != nil { output.PrintError(err) return err } } + + if hasLocalImages && fm.NotionID != "" { + apiClient, err := cli.RequireOfficialAPIClient(officialAPIOverrides(ctx)) + if err != nil { + output.PrintError(err) + return err + } + snapshot, err = apiClient.GetPageMarkdown(bgCtx, fm.NotionID) + if err != nil { + output.PrintError(err) + return err + } + if snapshot.Truncated { + finalErr := fmt.Errorf("cannot sync local images safely: page %s markdown snapshot is truncated, so rollback on a failed substitution would leave placeholders in the page. Retry without local images or reduce the page before syncing", fm.NotionID) + output.PrintError(finalErr) + return finalErr + } + } + body, localUploads, err = prepareLocalImageUploads(ctx, bgCtx, file, body) if err != nil { output.PrintError(err) @@ -599,20 +630,6 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL defer func() { _ = client.Close() }() if fm.NotionID != "" { - var snapshot *api.PageMarkdown - if len(localUploads) > 0 { - apiClient, err := cli.RequireOfficialAPIClient(officialAPIOverrides(ctx)) - if err != nil { - output.PrintError(err) - return err - } - snapshot, err = apiClient.GetPageMarkdown(bgCtx, fm.NotionID) - if err != nil { - output.PrintError(err) - return err - } - } - req := mcp.UpdatePageRequest{ PageID: fm.NotionID, Command: "replace_content", diff --git a/internal/cli/local_images.go b/internal/cli/local_images.go index 28a6b45..cc3ac24 100644 --- a/internal/cli/local_images.go +++ b/internal/cli/local_images.go @@ -102,7 +102,8 @@ func scanStandaloneLocalImages(markdown string, resolvePath func(dest string) (s continue } - matches := findMarkdownImages(line) + scanLine := maskInlineCodeSpans(line) + matches := findMarkdownImages(scanLine) if len(matches) == 0 { prevBlank = isBlank continue @@ -136,7 +137,10 @@ func scanStandaloneLocalImages(markdown string, resolvePath func(dest string) (s } placeholder := "NOTION_CLI_LOCAL_IMAGE_" + strings.ReplaceAll(uuid.NewString(), "-", "_") - lines[i] = placeholder + // Preserve the whitespace that surrounded the image so block context + // (list continuations, blockquote markers, etc.) is not dropped when + // the placeholder is substituted back. + lines[i] = line[:m.start] + placeholder + line[m.end:] placements = append(placements, LocalImagePlacement{ Alt: m.alt, Original: dest, @@ -156,6 +160,47 @@ type markdownImageMatch struct { dest string } +// maskInlineCodeSpans replaces the contents of every inline code span in +// `line` with spaces so the image scanner does not pick up markdown tokens +// inside the span. Length is preserved so match offsets still map back to +// the original line. An opening run of N backticks closes on the first +// matching run of exactly N backticks (CommonMark rule). Backticks inside +// a span cannot be escaped. +func maskInlineCodeSpans(line string) string { + b := []byte(line) + i := 0 + for i < len(b) { + if b[i] != '`' { + i++ + continue + } + runStart := i + for i < len(b) && b[i] == '`' { + i++ + } + runLen := i - runStart + for i < len(b) { + if b[i] != '`' { + i++ + continue + } + closeStart := i + for i < len(b) && b[i] == '`' { + i++ + } + if i-closeStart == runLen { + for k := runStart + runLen; k < closeStart; k++ { + if b[k] != '\n' { + b[k] = ' ' + } + } + break + } + } + } + return string(b) +} + // findMarkdownImages returns every `![alt](dest)` span on a single line. // Destinations may contain balanced parentheses and `\)` / `\(` escapes, and // may optionally be wrapped in angle brackets (``). A backslash before diff --git a/internal/cli/local_images_test.go b/internal/cli/local_images_test.go index f21c00d..6bdd4e8 100644 --- a/internal/cli/local_images_test.go +++ b/internal/cli/local_images_test.go @@ -170,6 +170,37 @@ func TestRewriteStandaloneLocalImagesSupportsEscapedParensInDestination(t *testi } } +func TestFindStandaloneLocalImageLinesIgnoresInlineCodeSpans(t *testing.T) { + input := "Use `![Demo](./example.png)` in docs.\n" + + rewritten, placements, err := FindStandaloneLocalImageLines(input) + if err != nil { + t.Fatalf("FindStandaloneLocalImageLines: %v", err) + } + if len(placements) != 0 { + t.Fatalf("len(placements) = %d, want 0 (image inside inline code span should be ignored)", len(placements)) + } + if rewritten != input { + t.Fatalf("rewritten = %q, want input unchanged", rewritten) + } +} + +func TestFindStandaloneLocalImageLinesPreservesLeadingIndentation(t *testing.T) { + input := " ![img](./a.png)\n" + + rewritten, placements, err := FindStandaloneLocalImageLines(input) + if err != nil { + t.Fatalf("FindStandaloneLocalImageLines: %v", err) + } + if len(placements) != 1 { + t.Fatalf("len(placements) = %d, want 1", len(placements)) + } + want := " " + placements[0].Placeholder + "\n" + if rewritten != want { + t.Fatalf("rewritten = %q, want %q", rewritten, want) + } +} + func TestFindStandaloneLocalImageLinesSkipsBacktickFencedCodeBlock(t *testing.T) { markdown := strings.Join([]string{ "before", From 0838c60a7da1fb2c2b22f59d2057a7375341773b Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 13:51:29 -0400 Subject: [PATCH 17/31] fix(page): strip indented placeholders in --skip-local-images mode scanStandaloneLocalImages preserves the whitespace that surrounded the image line (added with the indentation fix), so an input like " ![](./x.png)" now becomes " NOTION_CLI_LOCAL_IMAGE_*". The equality check in stripLocalImages only matched the bare placeholder token, so these indented lines survived intact and the command published the raw placeholder text instead of stripping the image. Match on the trimmed token so leading or trailing whitespace still strips correctly. --- cmd/page_local_images.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cmd/page_local_images.go b/cmd/page_local_images.go index 125baf9..c9a7cb9 100644 --- a/cmd/page_local_images.go +++ b/cmd/page_local_images.go @@ -121,7 +121,12 @@ func stripLocalImages(markdown string) (string, error) { placeholders[placement.Placeholder] = struct{}{} } for i, line := range lines { - if _, ok := placeholders[line]; ok { + // scanStandaloneLocalImages preserves the whitespace that surrounded + // the image so block context is not lost during upload. In strip + // mode we need to match on the trimmed placeholder so indented or + // trailing-whitespace image lines are also cleared rather than left + // as NOTION_CLI_LOCAL_IMAGE_* text in the published page. + if _, ok := placeholders[strings.TrimSpace(line)]; ok { lines[i] = "" } } From 9bd9b15e84e89bd50cfc33ae95080947750febe2 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 15:13:11 -0400 Subject: [PATCH 18/31] fix(page): force placeholders to column zero so substitution lands in paragraph Indented standalone image lines used to carry their leading whitespace into the placeholder line, which kept the block context (list continuation, blockquote, etc.) intact on Notion's side. That turned into a hard failure: substituteUploadedLocalImages only indexes paragraph blocks when it searches for placeholder text, so a placeholder that landed inside a list_item or quote block never matched and the command fell back to rolling back the entire page. Drop the surrounding whitespace before replace_content so every placeholder sits in a paragraph block and substitution always finds it. The tradeoff is cosmetic: uploaded images render as standalone blocks rather than nested inside the list or quote they were written under. Strip mode already matches on the trimmed placeholder so no change is needed there. --- internal/cli/local_images.go | 16 ++++++++++++---- internal/cli/local_images_test.go | 11 ++++++++--- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/internal/cli/local_images.go b/internal/cli/local_images.go index cc3ac24..55eb963 100644 --- a/internal/cli/local_images.go +++ b/internal/cli/local_images.go @@ -137,10 +137,18 @@ func scanStandaloneLocalImages(markdown string, resolvePath func(dest string) (s } placeholder := "NOTION_CLI_LOCAL_IMAGE_" + strings.ReplaceAll(uuid.NewString(), "-", "_") - // Preserve the whitespace that surrounded the image so block context - // (list continuations, blockquote markers, etc.) is not dropped when - // the placeholder is substituted back. - lines[i] = line[:m.start] + placeholder + line[m.end:] + // Replace the entire line with the bare placeholder at column 0. We + // deliberately drop any surrounding whitespace so the placeholder + // always lands in a paragraph block after replace_content, even + // when the original image sat inside a list continuation or + // blockquote. Keeping the leading whitespace would let Notion nest + // the placeholder inside the enclosing block (list_item, quote, + // etc.), and substituteUploadedLocalImages only indexes paragraph + // blocks, so substitution would fail and force rollback. The + // tradeoff is cosmetic: uploaded images render as standalone + // blocks rather than nested under the list or quote they were + // written under. + lines[i] = placeholder placements = append(placements, LocalImagePlacement{ Alt: m.alt, Original: dest, diff --git a/internal/cli/local_images_test.go b/internal/cli/local_images_test.go index 6bdd4e8..32c71a5 100644 --- a/internal/cli/local_images_test.go +++ b/internal/cli/local_images_test.go @@ -185,7 +185,12 @@ func TestFindStandaloneLocalImageLinesIgnoresInlineCodeSpans(t *testing.T) { } } -func TestFindStandaloneLocalImageLinesPreservesLeadingIndentation(t *testing.T) { +func TestFindStandaloneLocalImageLinesNormalizesToColumnZero(t *testing.T) { + // Indented standalone image lines have their leading whitespace dropped + // so the placeholder always lands in a paragraph block after + // replace_content. substituteUploadedLocalImages only indexes paragraph + // blocks, so keeping the indent would let Notion nest the placeholder + // inside the surrounding list_item/quote and break substitution. input := " ![img](./a.png)\n" rewritten, placements, err := FindStandaloneLocalImageLines(input) @@ -195,9 +200,9 @@ func TestFindStandaloneLocalImageLinesPreservesLeadingIndentation(t *testing.T) if len(placements) != 1 { t.Fatalf("len(placements) = %d, want 1", len(placements)) } - want := " " + placements[0].Placeholder + "\n" + want := placements[0].Placeholder + "\n" if rewritten != want { - t.Fatalf("rewritten = %q, want %q", rewritten, want) + t.Fatalf("rewritten = %q, want %q (placeholder must sit at column 0)", rewritten, want) } } From 391db41b765ac5daad7aa55720b470e9f53217c8 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 15:42:14 -0400 Subject: [PATCH 19/31] fix(page): resolve file:// hosts and ~/ paths portably Two small portability fixes in resolveLocalPath. Preserve parsed.Host when the file URL carries authority data so UNC paths like file://server/share/a.png resolve to the right share instead of the local drive. localhost is still treated as empty. Accept both `~/` and `~` when expanding home-directory paths. markdown commonly uses forward slashes regardless of OS, and matching only filepath.Separator skipped expansion on Windows for otherwise valid paths. --- internal/cli/local_images.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/internal/cli/local_images.go b/internal/cli/local_images.go index 55eb963..a38e614 100644 --- a/internal/cli/local_images.go +++ b/internal/cli/local_images.go @@ -550,15 +550,24 @@ func resolveLocalPath(dest, sourceDir string) (string, error) { if err != nil { return "", fmt.Errorf("invalid file URL path %q: %w", d, err) } - d = unescaped + // Preserve the authority for UNC-style file URLs + // (file://server/share/path) so the resolved local path points at + // the right share instead of silently dropping the host. + if parsed.Host != "" && parsed.Host != "localhost" { + d = "//" + parsed.Host + unescaped + } else { + d = unescaped + } } - if strings.HasPrefix(d, "~"+string(filepath.Separator)) { + // Accept both `~/` and `~\` so markdown paths written with forward + // slashes still expand on Windows, where filepath.Separator is `\`. + if strings.HasPrefix(d, "~/") || strings.HasPrefix(d, "~"+string(filepath.Separator)) { home, err := os.UserHomeDir() if err != nil { return "", fmt.Errorf("expand home path %q: %w", d, err) } - d = filepath.Join(home, strings.TrimPrefix(d, "~"+string(filepath.Separator))) + d = filepath.Join(home, d[2:]) } if !filepath.IsAbs(d) { From 1ef067c45280fed3d1f05c95d5824540a6816b68 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 16:17:53 -0400 Subject: [PATCH 20/31] fix(page): gate sync when snapshot has unknown block IDs rollbackSyncedPage previously guarded only on snapshot.Truncated. If a page contains blocks that cannot be represented in markdown, the PageMarkdown snapshot carries their IDs in UnknownBlockIDs, and a replace_content rollback would replay a lossy markdown body and permanently drop those blocks. Treat non-empty UnknownBlockIDs as an unsafe rollback source and fail early at the pre-sync gate with the same pattern used for truncation, so page sync does not silently lose content on recovery. --- cmd/page.go | 5 +++++ cmd/page_local_images.go | 3 +++ cmd/page_local_images_test.go | 15 +++++++++++++++ 3 files changed, 23 insertions(+) diff --git a/cmd/page.go b/cmd/page.go index d125488..50cf519 100644 --- a/cmd/page.go +++ b/cmd/page.go @@ -604,6 +604,11 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL output.PrintError(finalErr) return finalErr } + if len(snapshot.UnknownBlockIDs) > 0 { + finalErr := fmt.Errorf("cannot sync local images safely: page %s contains %d block(s) that cannot be represented in markdown, so rollback on a failed substitution would drop them. Retry without local images or remove the unsupported blocks before syncing", fm.NotionID, len(snapshot.UnknownBlockIDs)) + output.PrintError(finalErr) + return finalErr + } } body, localUploads, err = prepareLocalImageUploads(ctx, bgCtx, file, body) diff --git a/cmd/page_local_images.go b/cmd/page_local_images.go index c9a7cb9..f60ae4b 100644 --- a/cmd/page_local_images.go +++ b/cmd/page_local_images.go @@ -232,6 +232,9 @@ func rollbackSyncedPage(ctx context.Context, client pageUpdater, pageID string, if snapshot.Truncated { return fmt.Errorf("skipped rollback: page markdown snapshot was truncated; replaying would lose content") } + if len(snapshot.UnknownBlockIDs) > 0 { + return fmt.Errorf("skipped rollback: page markdown snapshot omits %d block(s) that cannot be represented in markdown; replaying would drop them", len(snapshot.UnknownBlockIDs)) + } return client.UpdatePage(ctx, mcp.UpdatePageRequest{ PageID: pageID, Command: "replace_content", diff --git a/cmd/page_local_images_test.go b/cmd/page_local_images_test.go index 1e48517..bc4932d 100644 --- a/cmd/page_local_images_test.go +++ b/cmd/page_local_images_test.go @@ -199,6 +199,21 @@ func TestRollbackSyncedPageSkipsTruncatedSnapshot(t *testing.T) { } } +func TestRollbackSyncedPageSkipsUnknownBlocks(t *testing.T) { + snapshot := &api.PageMarkdown{ + Markdown: "# Title\n\nLossy content\n", + UnknownBlockIDs: []string{"block-1", "block-2"}, + } + + err := rollbackSyncedPage(context.Background(), nil, "page-id", snapshot) + if err == nil { + t.Fatalf("rollbackSyncedPage returned nil error; expected unknown-blocks error") + } + if !strings.Contains(err.Error(), "cannot be represented in markdown") { + t.Fatalf("rollbackSyncedPage error = %q, want it to mention unrepresentable blocks", err.Error()) + } +} + func TestRollbackSyncedPageSkipsNilSnapshot(t *testing.T) { if err := rollbackSyncedPage(context.Background(), nil, "page-id", nil); err != nil { t.Fatalf("rollbackSyncedPage returned %v, want nil", err) From 0fd1e95d366defe44dc8bebe411b02a7e5164b3e Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 17:13:35 -0400 Subject: [PATCH 21/31] fix(page): resolve parent IDs before uploading local images runPageUpload and the create path of runPageSync previously uploaded local images via prepareLocalImageUploads before resolving --parent or --parent-db through the official API. An invalid parent reference returned an error only after bytes were already sent, leaving orphaned uploads behind on a command users expected to be a failed no-op. Resolve parent IDs first so any parent-resolution failure aborts before touching the file-upload path. Pre-resolved IDs are threaded into the CreatePage request so the resolution still only happens once. --- cmd/page.go | 90 +++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 67 insertions(+), 23 deletions(-) diff --git a/cmd/page.go b/cmd/page.go index 50cf519..d485e28 100644 --- a/cmd/page.go +++ b/cmd/page.go @@ -273,7 +273,6 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string, ski markdown := string(content) bgCtx := context.Background() - var localUploads []uploadedLocalImage if skipLocalImages { markdown, err = stripLocalImages(markdown) if err != nil { @@ -285,22 +284,6 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string, ski output.PrintError(err) return err } - markdown, localUploads, err = prepareLocalImageUploads(ctx, bgCtx, file, markdown) - if err != nil { - output.PrintError(err) - return err - } - } - - if title == "" { - title = extractTitleFromMarkdown(markdown) - } - if title == "" { - title = strings.TrimSuffix(filepath.Base(file), filepath.Ext(file)) - } - - if icon == "" { - icon, title = extractEmojiFromTitle(title) } client, err := cli.RequireClient() @@ -309,11 +292,9 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string, ski } defer func() { _ = client.Close() }() - req := mcp.CreatePageRequest{ - Title: title, - Content: markdown, - } - + // Resolve parent IDs before any upload side effects so an invalid + // --parent/--parent-db doesn't leave orphaned file uploads behind. + req := mcp.CreatePageRequest{} if parentDB != "" { dbID, err := cli.ResolveDatabaseID(bgCtx, client, parentDB) if err != nil { @@ -335,6 +316,29 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string, ski req.ParentPageID = parentID } + var localUploads []uploadedLocalImage + if !skipLocalImages { + markdown, localUploads, err = prepareLocalImageUploads(ctx, bgCtx, file, markdown) + if err != nil { + output.PrintError(err) + return err + } + } + + if title == "" { + title = extractTitleFromMarkdown(markdown) + } + if title == "" { + title = strings.TrimSuffix(filepath.Base(file), filepath.Ext(file)) + } + + if icon == "" { + icon, title = extractEmojiFromTitle(title) + } + + req.Title = title + req.Content = markdown + resp, err := client.CreatePage(bgCtx, req) if err != nil { output.PrintError(err) @@ -563,6 +567,7 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL bgCtx := context.Background() var localUploads []uploadedLocalImage var snapshot *api.PageMarkdown + var resolvedParentPageID, resolvedParentDatabaseID string if skipLocalImages { body, err = stripLocalImages(body) if err != nil { @@ -611,6 +616,39 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL } } + // For the create path, resolve parent IDs before any uploads so an + // invalid --parent/--parent-db doesn't leave orphaned file uploads. + if hasLocalImages && fm.NotionID == "" { + resolveClient, err := cli.RequireClient() + if err != nil { + return err + } + if parentDB != "" { + dbID, err := cli.ResolveDatabaseID(bgCtx, resolveClient, parentDB) + if err != nil { + _ = resolveClient.Close() + output.PrintError(err) + return err + } + dbID, err = resolveClient.ResolveDataSourceID(bgCtx, dbID) + if err != nil { + _ = resolveClient.Close() + output.PrintError(err) + return err + } + resolvedParentDatabaseID = dbID + } else if parent != "" { + parentID, err := cli.ResolvePageID(bgCtx, resolveClient, parent) + if err != nil { + _ = resolveClient.Close() + output.PrintError(err) + return err + } + resolvedParentPageID = parentID + } + _ = resolveClient.Close() + } + body, localUploads, err = prepareLocalImageUploads(ctx, bgCtx, file, body) if err != nil { output.PrintError(err) @@ -677,7 +715,13 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL Content: body, } - if parentDB != "" { + // Reuse parent IDs pre-resolved above when local images were involved; + // otherwise resolve here for the no-upload create path. + if resolvedParentDatabaseID != "" { + req.ParentDatabaseID = resolvedParentDatabaseID + } else if resolvedParentPageID != "" { + req.ParentPageID = resolvedParentPageID + } else if parentDB != "" { dbID, err := cli.ResolveDatabaseID(bgCtx, client, parentDB) if err != nil { output.PrintError(err) From b7a5954e84a7bedbf7a0202edac95440cb92f866 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 17:44:40 -0400 Subject: [PATCH 22/31] fix(page): report orphan page URL when create ID is unparsable When CreatePage succeeds but pageIDFromCreateResponse cannot extract an ID (empty resp.ID and unparseable resp.URL), substituteUploadedLocalImages returns a 'missing page ID' error and substituteOrCleanup then skips its TrashPage branch because pageID is empty, leaving a newly created page in the workspace with raw NOTION_CLI_LOCAL_IMAGE_* placeholders and no automatic recovery path. Pass the create URL through and, when the ID is missing with uploads pending, wrap the error with the create URL so the user can delete the orphan manually. --- cmd/page.go | 4 ++-- cmd/page_local_images.go | 13 +++++++++++-- cmd/page_local_images_test.go | 22 ++++++++++++++++++++++ 3 files changed, 35 insertions(+), 4 deletions(-) diff --git a/cmd/page.go b/cmd/page.go index d485e28..50b7eb5 100644 --- a/cmd/page.go +++ b/cmd/page.go @@ -345,7 +345,7 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string, ski return err } pageID := pageIDFromCreateResponse(resp) - if err := substituteOrCleanup(ctx, bgCtx, pageID, localUploads); err != nil { + if err := substituteOrCleanup(ctx, bgCtx, pageID, resp.URL, localUploads); err != nil { output.PrintError(err) return err } @@ -749,7 +749,7 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL } pageID := pageIDFromCreateResponse(resp) - if err := substituteOrCleanup(ctx, bgCtx, pageID, localUploads); err != nil { + if err := substituteOrCleanup(ctx, bgCtx, pageID, resp.URL, localUploads); err != nil { output.PrintError(err) return err } diff --git a/cmd/page_local_images.go b/cmd/page_local_images.go index f60ae4b..e92facd 100644 --- a/cmd/page_local_images.go +++ b/cmd/page_local_images.go @@ -154,8 +154,11 @@ func checkLocalImageParent(markdown, parent, parentDB string) error { } // substituteOrCleanup substitutes uploaded local images into the page and -// trashes the page on failure so it doesn't linger with placeholders. -func substituteOrCleanup(cmdCtx *Context, ctx context.Context, pageID string, uploads []uploadedLocalImage) error { +// trashes the page on failure so it doesn't linger with placeholders. When +// pageID is empty (the create response omitted a parseable ID), we cannot +// trash the page automatically, so the error message includes createdURL so +// the user can delete the orphan manually. +func substituteOrCleanup(cmdCtx *Context, ctx context.Context, pageID, createdURL string, uploads []uploadedLocalImage) error { if err := substituteUploadedLocalImages(cmdCtx, ctx, pageID, uploads); err != nil { finalErr := fmt.Errorf("insert uploaded local images: %w", err) if pageID != "" { @@ -166,6 +169,12 @@ func substituteOrCleanup(cmdCtx *Context, ctx context.Context, pageID string, up } else { finalErr = fmt.Errorf("%w (cleanup client init failed: %v)", finalErr, apiErr) } + } else if len(uploads) > 0 { + ref := strings.TrimSpace(createdURL) + if ref == "" { + ref = "(no URL returned; check your workspace)" + } + finalErr = fmt.Errorf("%w (page was created but its ID could not be parsed, so placeholders remain and automatic cleanup is not possible; delete it manually: %s)", finalErr, ref) } return finalErr } diff --git a/cmd/page_local_images_test.go b/cmd/page_local_images_test.go index bc4932d..ca7d07e 100644 --- a/cmd/page_local_images_test.go +++ b/cmd/page_local_images_test.go @@ -152,6 +152,28 @@ func TestSubstituteUploadedLocalImagesErrorsWhenPageIDMissing(t *testing.T) { } } +func TestSubstituteOrCleanupReportsOrphanURLWhenPageIDMissing(t *testing.T) { + cmdCtx := &Context{APIToken: "secret-token"} + uploads := []uploadedLocalImage{{ + Alt: "Diagram", + FileUploadID: "upload_123", + Placeholder: "PLACEHOLDER", + ResolvedPath: "/tmp/diagram.png", + }} + + orphanURL := "https://www.notion.so/Page-1234567890abcdef1234567890abcdef" + err := substituteOrCleanup(cmdCtx, context.Background(), "", orphanURL, uploads) + if err == nil { + t.Fatalf("expected error when pageID is empty with pending uploads") + } + if !strings.Contains(err.Error(), orphanURL) { + t.Fatalf("error = %v, want orphan URL in message", err) + } + if !strings.Contains(err.Error(), "delete it manually") { + t.Fatalf("error = %v, want manual-delete guidance", err) + } +} + func TestSubstituteUploadedLocalImagesSkipsWithoutUploads(t *testing.T) { cmdCtx := &Context{APIToken: "secret-token"} if err := substituteUploadedLocalImages(cmdCtx, context.Background(), "", nil); err != nil { From 71d2800b54fcae5866922e45fdaf3c19e9c16b14 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 17:44:41 -0400 Subject: [PATCH 23/31] fix(images): restrict Windows drive detection to avoid URI false positives isLocalDestination previously treated any destination whose second character was ':' as a Windows drive path, so valid single-letter URI schemes (for example 'a://example.com/img.png' or 'a:foo') were classified as filesystem paths. That caused 'local image not found' errors during sync and --skip-local-images incorrectly stripped those lines. Require the leading character to be an ASCII letter, require a path separator (or end-of-string) after the colon, and exclude 'x://...' authority-style URIs so one-letter schemes fall through to the existing scheme regex. --- internal/cli/local_images.go | 21 +++++++++++++++++---- internal/cli/local_images_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/internal/cli/local_images.go b/internal/cli/local_images.go index a38e614..fc56ad4 100644 --- a/internal/cli/local_images.go +++ b/internal/cli/local_images.go @@ -518,10 +518,6 @@ func isLocalDestination(dest string) bool { return false } - if len(d) >= 2 && d[1] == ':' { - return true - } - lower := strings.ToLower(d) switch { case strings.HasPrefix(lower, "#"), @@ -536,9 +532,26 @@ func isLocalDestination(dest string) bool { return true } + // Windows drive paths like `C:`, `C:\foo`, or `C:/foo`. Require the + // leading character to be an ASCII letter and the character after the + // colon to be a separator (or end-of-string) so one-letter URI schemes + // such as `a:foo` fall through to the scheme check below instead of + // being misclassified as filesystem paths. Exclude `x://...` because + // that is a URI with authority, not a Windows drive path. + if len(d) >= 2 && isASCIILetter(d[0]) && d[1] == ':' { + hasAuthority := len(d) >= 4 && d[2] == '/' && d[3] == '/' + if !hasAuthority && (len(d) == 2 || d[2] == '\\' || d[2] == '/') { + return true + } + } + return !uriSchemeRE.MatchString(d) } +func isASCIILetter(b byte) bool { + return (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') +} + func resolveLocalPath(dest, sourceDir string) (string, error) { d := strings.TrimSpace(dest) if strings.HasPrefix(strings.ToLower(d), "file://") { diff --git a/internal/cli/local_images_test.go b/internal/cli/local_images_test.go index 32c71a5..c6bc87a 100644 --- a/internal/cli/local_images_test.go +++ b/internal/cli/local_images_test.go @@ -366,3 +366,29 @@ func TestFindStandaloneLocalImageLinesTreatsImageOutsideFenceAsStandalone(t *tes t.Fatalf("rewritten should have replaced post-fence image line with a placeholder: %q", rewritten) } } + +func TestIsLocalDestinationHandlesWindowsAndURISchemes(t *testing.T) { + cases := []struct { + name string + in string + want bool + }{ + {"windows drive only", `C:`, true}, + {"windows backslash path", `C:\Users\foo\img.png`, true}, + {"windows forward-slash path", `C:/Users/foo/img.png`, true}, + {"lowercase drive", `d:\tmp\img.png`, true}, + {"single-letter URI scheme", `a://example.com/img.png`, false}, + {"single-letter URI no authority", `a:example`, false}, + {"non-letter colon prefix", `::foo`, true}, + {"digit colon prefix", `1:relative`, true}, + {"empty string", ``, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := isLocalDestination(tc.in) + if got != tc.want { + t.Fatalf("isLocalDestination(%q) = %v, want %v", tc.in, got, tc.want) + } + }) + } +} From 3a9eb0aa528db6eb2f1930dc58d474d1462b77a4 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 18:14:13 -0400 Subject: [PATCH 24/31] fix(api): reject >20MB single_part uploads with a clear error UploadFile always sends mode=\"single_part\" but Notion's single-part upload limit is ~20MB; anything larger fails on the server with an opaque error. The client already knows the file size, so add an explicit SinglePartUploadMaxBytes constant and fail the upload early with a message that names the limit and notes that multi_part uploads are not yet supported. Full multipart support is out of scope for this PR and tracked as follow-up work. --- internal/api/client.go | 8 ++++++++ internal/api/client_test.go | 21 +++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/internal/api/client.go b/internal/api/client.go index 8e35cac..9c1dd01 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -134,6 +134,11 @@ func (c *Client) GetPageMarkdown(ctx context.Context, pageID string) (*PageMarkd return &out, nil } +// SinglePartUploadMaxBytes is Notion's single_part file-upload size limit. +// Files larger than this require the multi_part upload flow, which this +// client does not yet implement. +const SinglePartUploadMaxBytes = 20 * 1024 * 1024 + // UploadFile streams a file upload to the Notion API without buffering the whole // payload in memory. The reader is consumed through a multipart writer piped // directly into the HTTP request body. Pass the exact byte size so the server @@ -146,6 +151,9 @@ func (c *Client) UploadFile(ctx context.Context, name string, size int64, body i if size <= 0 { return "", fmt.Errorf("file size must be positive") } + if size > SinglePartUploadMaxBytes { + return "", fmt.Errorf("file %q is %d bytes; Notion's single_part upload limit is %d bytes (~20MB) and multi_part uploads are not yet supported by this client", name, size, SinglePartUploadMaxBytes) + } if body == nil { return "", fmt.Errorf("file body is required") } diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 95e285e..1134ca4 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -260,3 +260,24 @@ func TestTrashPageUsesPatch(t *testing.T) { t.Fatalf("TrashPage: %v", err) } } + +func TestUploadFileRejectsOversizedSinglePart(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + })) + defer srv.Close() + + client, err := NewClient(config.APIConfig{BaseURL: srv.URL + "/v1"}, "secret-token") + if err != nil { + t.Fatalf("NewClient: %v", err) + } + + oversize := int64(SinglePartUploadMaxBytes + 1) + _, err = client.UploadFile(context.Background(), "big.png", oversize, strings.NewReader("")) + if err == nil { + t.Fatalf("UploadFile returned nil error; expected size-limit error") + } + if !strings.Contains(err.Error(), "single_part upload limit") { + t.Fatalf("UploadFile error = %q, want single_part limit message", err.Error()) + } +} From ae15b56cc34f8a7721015d0183a526a4a02a2b69 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 18:14:13 -0400 Subject: [PATCH 25/31] fix(images): honor \> escapes inside angle-bracket image destinations parseMarkdownDestination stopped at the first '>' even when escaped, while the scanner's findDestinationEnd already walks past '\\>' the way CommonMark requires. That made inputs like '<./foo\\>bar.png>' parse as './foo\\' and fail local-file resolution. Walk character by character in the angle-bracket branch, skip escape sequences, and stop only at the first unescaped '>' so the two halves of the parser agree. --- internal/cli/local_images.go | 17 ++++++++++++++--- internal/cli/local_images_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/internal/cli/local_images.go b/internal/cli/local_images.go index fc56ad4..c28abc8 100644 --- a/internal/cli/local_images.go +++ b/internal/cli/local_images.go @@ -455,9 +455,20 @@ func parseMarkdownDestination(raw string) (string, bool) { } if strings.HasPrefix(s, "<") { - end := strings.Index(s, ">") - if end > 1 { - return unescapeMarkdownPunctuation(s[1:end]), true + // Match the scanner's findDestinationEnd: walk past `\>` escapes + // so inputs like `<./foo\>bar.png>` keep the full destination + // instead of truncating at the first `>`. + for i := 1; i < len(s); i++ { + if s[i] == '\\' && i+1 < len(s) { + i++ + continue + } + if s[i] == '>' { + if i > 1 { + return unescapeMarkdownPunctuation(s[1:i]), true + } + break + } } } diff --git a/internal/cli/local_images_test.go b/internal/cli/local_images_test.go index c6bc87a..632ad66 100644 --- a/internal/cli/local_images_test.go +++ b/internal/cli/local_images_test.go @@ -331,6 +331,30 @@ func TestRewriteStandaloneLocalImagesSupportsAngleBracketDestinationWithTitle(t } } +func TestRewriteStandaloneLocalImagesHandlesEscapedGTInAngleBrackets(t *testing.T) { + tmp := t.TempDir() + doc := filepath.Join(tmp, "doc.md") + img := filepath.Join(tmp, "foo>bar.png") + if err := os.WriteFile(img, []byte("PNG"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + // CommonMark allows `\>` inside angle-bracket destinations. + rewritten, placements, err := RewriteStandaloneLocalImages(`![Alt](<./foo\>bar.png>)`+"\n", doc) + if err != nil { + t.Fatalf("RewriteStandaloneLocalImages: %v", err) + } + if len(placements) != 1 { + t.Fatalf("len(placements) = %d, want 1", len(placements)) + } + if placements[0].Resolved != img { + t.Fatalf("Resolved = %q, want %q", placements[0].Resolved, img) + } + if strings.Contains(rewritten, `foo\>bar.png`) { + t.Fatalf("rewritten should have replaced image line, got %q", rewritten) + } +} + func TestFindStandaloneLocalImageLinesIgnoresEscapedImageMarker(t *testing.T) { input := `\![Demo](./example.png)` + "\n" rewritten, placements, err := FindStandaloneLocalImageLines(input) From 38b68767e85ecb19bddef486ef8584f3f93360ce Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 18:42:50 -0400 Subject: [PATCH 26/31] fix(images): decode escaped spaces and skip titles in destinations Two scanner fixes that together let standalone local images with quoted titles and escaped whitespace round-trip correctly: - parseMarkdownDestination already walked past '\\ ' so unbracketed destinations like './diagram\\ 1.png' parsed as one token, but unescapeMarkdownPunctuation only decoded ASCII punctuation; the backslash was left in the resolved path and resolveLocalPath failed to find the real file. Decode '\\ ' to ' ' during unescape. - findDestinationEnd closed the image span at the first unescaped ')' at depth 0, including ')' characters inside a quoted title like '![Alt](./img.png \"v1) draft\")'. Treat depth-0 whitespace as the destination terminator and hand off to skipOptionalTitleAndClose so a ')' inside the title can no longer be mistaken for the image's closing paren. --- internal/cli/local_images.go | 16 +++++++++-- internal/cli/local_images_test.go | 44 +++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 2 deletions(-) diff --git a/internal/cli/local_images.go b/internal/cli/local_images.go index c28abc8..46b4a4c 100644 --- a/internal/cli/local_images.go +++ b/internal/cli/local_images.go @@ -314,6 +314,14 @@ func findDestinationEnd(line string, start int) (int, bool) { return i, true } depth-- + case ' ', '\t': + // Whitespace at depth 0 terminates a non-angle destination; the + // rest is an optional quoted title, then the closing `)`. Skip + // through the title so a `)` inside it doesn't look like the end + // of the image span. + if depth == 0 { + return skipOptionalTitleAndClose(line, i) + } } } return 0, false @@ -496,7 +504,11 @@ func parseMarkdownDestination(raw string) (string, bool) { } // unescapeMarkdownPunctuation turns CommonMark backslash escapes of ASCII -// punctuation (e.g. `\)`, `\(`, `\\`) into their literal characters. +// punctuation (e.g. `\)`, `\(`, `\\`) into their literal characters. It also +// decodes `\` because the destination scanner treats a backslash as +// an escape across whitespace so users can embed spaces in unbracketed paths +// like `./diagram\ 1.png`; if we left the backslash in place, the resolved +// path would not match the real filename. func unescapeMarkdownPunctuation(s string) string { if !strings.Contains(s, `\`) { return s @@ -504,7 +516,7 @@ func unescapeMarkdownPunctuation(s string) string { var b strings.Builder b.Grow(len(s)) for i := 0; i < len(s); i++ { - if s[i] == '\\' && i+1 < len(s) && isASCIIPunctuation(s[i+1]) { + if s[i] == '\\' && i+1 < len(s) && (isASCIIPunctuation(s[i+1]) || s[i+1] == ' ') { b.WriteByte(s[i+1]) i++ continue diff --git a/internal/cli/local_images_test.go b/internal/cli/local_images_test.go index 632ad66..eda79f3 100644 --- a/internal/cli/local_images_test.go +++ b/internal/cli/local_images_test.go @@ -391,6 +391,50 @@ func TestFindStandaloneLocalImageLinesTreatsImageOutsideFenceAsStandalone(t *tes } } +func TestRewriteStandaloneLocalImagesDecodesEscapedSpaceInDestination(t *testing.T) { + tmp := t.TempDir() + doc := filepath.Join(tmp, "doc.md") + img := filepath.Join(tmp, "diagram 1.png") + if err := os.WriteFile(img, []byte("PNG"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + rewritten, placements, err := RewriteStandaloneLocalImages(`![Alt](./diagram\ 1.png)`+"\n", doc) + if err != nil { + t.Fatalf("RewriteStandaloneLocalImages: %v", err) + } + if len(placements) != 1 { + t.Fatalf("len(placements) = %d, want 1", len(placements)) + } + if placements[0].Resolved != img { + t.Fatalf("Resolved = %q, want %q", placements[0].Resolved, img) + } + if strings.Contains(rewritten, `diagram\ 1.png`) { + t.Fatalf("rewritten still contains escaped destination: %q", rewritten) + } +} + +func TestRewriteStandaloneLocalImagesHandlesParenInTitle(t *testing.T) { + tmp := t.TempDir() + doc := filepath.Join(tmp, "doc.md") + img := filepath.Join(tmp, "img.png") + if err := os.WriteFile(img, []byte("PNG"), 0o644); err != nil { + t.Fatalf("WriteFile: %v", err) + } + + // The `)` inside the title must not terminate the image span. + rewritten, placements, err := RewriteStandaloneLocalImages(`![Alt](./img.png "v1) draft")`+"\n", doc) + if err != nil { + t.Fatalf("RewriteStandaloneLocalImages: %v", err) + } + if len(placements) != 1 { + t.Fatalf("len(placements) = %d, want 1; rewritten=%q", len(placements), rewritten) + } + if placements[0].Resolved != img { + t.Fatalf("Resolved = %q, want %q", placements[0].Resolved, img) + } +} + func TestIsLocalDestinationHandlesWindowsAndURISchemes(t *testing.T) { cases := []struct { name string From e1b03abf2bb8d547a98017fa6e0cede37d9d1124 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 19:35:26 -0400 Subject: [PATCH 27/31] fix(page): serialize local image uploads to respect Notion rate limits prepareLocalImageUploads fanned out up to four uploads in parallel, but each upload fires three sequential official-API calls (create, send, then poll), so even two in-flight uploads easily exceed Notion's documented ~3 req/sec average limit. Without a shared rate limiter or 429/Retry-After handling, multi-image sync hits HTTP 429 nondeterministically. Drop the concurrency limit to 1 so uploads proceed serially and stay under the documented budget; full rate-limit/backoff infrastructure can land separately. --- cmd/page_local_images.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cmd/page_local_images.go b/cmd/page_local_images.go index e92facd..c025de7 100644 --- a/cmd/page_local_images.go +++ b/cmd/page_local_images.go @@ -15,8 +15,13 @@ import ( "github.com/lox/notion-cli/internal/output" ) -// localImageUploadConcurrency caps concurrent local image uploads. -const localImageUploadConcurrency = 4 +// localImageUploadConcurrency caps concurrent local image uploads. Each +// upload fires at least three sequential Notion API calls (create file +// upload, send, then poll for upload status), so even serial work lives +// right at Notion's documented ~3 req/sec average limit. Fanning out +// in parallel without a shared rate limiter and 429/Retry-After backoff +// reliably trips rate limits on pages with several images. +const localImageUploadConcurrency = 1 type uploadedLocalImage struct { Alt string From 0fba6bc98a04b448bd31689333c69452db52c3c4 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Sat, 18 Apr 2026 19:53:21 -0400 Subject: [PATCH 28/31] fix(images): skip blockquoted lines so quoted fences don't upload The fence detector only looked for up to three leading spaces before checking for triple-backtick/tilde, so a quoted fence like '> \`\`\`md' never set the in-fence state. A subsequent quoted example line such as '> ![Demo](./img.png)' was then scanned as a real local image and hit the 'must appear on their own line' error path, failing page upload and page sync on ordinary documentation markdown. Treat any blockquote line (up to three spaces of indent then '>') as opaque to image scanning; blockquoted images already wouldn't round-trip cleanly through the substitute-placeholder path anyway. --- internal/cli/local_images.go | 21 +++++++++++++++ internal/cli/local_images_test.go | 44 +++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/internal/cli/local_images.go b/internal/cli/local_images.go index 46b4a4c..a60eb40 100644 --- a/internal/cli/local_images.go +++ b/internal/cli/local_images.go @@ -89,6 +89,17 @@ func scanStandaloneLocalImages(markdown string, resolvePath func(dest string) (s prevBlank = false continue } + // Blockquotes typically hold documentation, examples, or quoted + // code including fenced blocks with literal `![...](...)` samples. + // The fence detector above ignores `>`-prefixed lines, so any + // image syntax inside a quoted fence would otherwise be scanned + // as a real local image and either uploaded by mistake or hit + // the "must appear on their own line" error path. Treat the + // whole blockquote line as opaque to image scanning. + if isBlockquoteLine(line) { + prevBlank = isBlank + continue + } if inIndented { if isBlank || startsWithCodeIndent(line) { @@ -386,6 +397,16 @@ func isStandaloneImageLine(line string, matches []markdownImageMatch) bool { return strings.TrimSpace(line[:m.start]) == "" && strings.TrimSpace(line[m.end:]) == "" } +// isBlockquoteLine reports whether `line` opens or continues a CommonMark +// blockquote (a `>` optionally preceded by up to three spaces of indent). +func isBlockquoteLine(line string) bool { + i := 0 + for i < len(line) && i < 4 && line[i] == ' ' { + i++ + } + return i < len(line) && line[i] == '>' +} + // opensFencedCodeBlock reports whether `line` opens a CommonMark fenced code // block. It returns the fence character (`` ` `` or `~`) and the fence length // (>= 3) on a match, and (0, 0) otherwise. diff --git a/internal/cli/local_images_test.go b/internal/cli/local_images_test.go index eda79f3..4400e4b 100644 --- a/internal/cli/local_images_test.go +++ b/internal/cli/local_images_test.go @@ -435,6 +435,50 @@ func TestRewriteStandaloneLocalImagesHandlesParenInTitle(t *testing.T) { } } +func TestFindStandaloneLocalImageLinesIgnoresQuotedFencedCodeBlock(t *testing.T) { + // Image syntax inside a quoted fenced code block must not be + // scanned; it's documentation, not an upload target. + markdown := strings.Join([]string{ + "> ```md", + "> ![Demo](./img.png)", + "> ```", + "", + "![Real](./real.png)", + "", + }, "\n") + + rewritten, placements, err := FindStandaloneLocalImageLines(markdown) + if err != nil { + t.Fatalf("FindStandaloneLocalImageLines: %v", err) + } + if len(placements) != 1 { + t.Fatalf("len(placements) = %d, want 1 (quoted fence contents should be ignored)", len(placements)) + } + if !strings.Contains(rewritten, "> ![Demo](./img.png)") { + t.Fatalf("rewritten lost the quoted example: %q", rewritten) + } + if strings.Contains(rewritten, "./real.png") { + t.Fatalf("rewritten should have replaced the real image line: %q", rewritten) + } +} + +func TestFindStandaloneLocalImageLinesIgnoresBlockquotedImage(t *testing.T) { + // A blockquoted image line isn't standalone; the `>` marker means + // the user is documenting rather than uploading. + markdown := "> ![Quoted](./quoted.png)\n" + + rewritten, placements, err := FindStandaloneLocalImageLines(markdown) + if err != nil { + t.Fatalf("FindStandaloneLocalImageLines: %v", err) + } + if len(placements) != 0 { + t.Fatalf("len(placements) = %d, want 0 (blockquoted image should be ignored)", len(placements)) + } + if rewritten != markdown { + t.Fatalf("rewritten = %q, want input unchanged", rewritten) + } +} + func TestIsLocalDestinationHandlesWindowsAndURISchemes(t *testing.T) { cases := []struct { name string From ce2ca0b049a2ad02a3944684b95ee9dc0fd9f480 Mon Sep 17 00:00:00 2001 From: Lachlan Donald Date: Mon, 27 Apr 2026 21:00:22 +1000 Subject: [PATCH 29/31] fix(page): preflight MCP before local image uploads --- cmd/page.go | 33 ++++++++++++++--------- cmd/page_local_images_test.go | 49 +++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 12 deletions(-) diff --git a/cmd/page.go b/cmd/page.go index 50b7eb5..8529331 100644 --- a/cmd/page.go +++ b/cmd/page.go @@ -26,6 +26,7 @@ type PageCmd struct { var loadPageViewCommentsFn = loadPageViewComments var printViewedPageFn = output.PrintViewedPage var printWarningFn = output.PrintWarning +var requirePageClientFn = cli.RequireClient type PageListCmd struct { Query string `help:"Filter pages by name" short:"q"` @@ -568,6 +569,12 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL var localUploads []uploadedLocalImage var snapshot *api.PageMarkdown var resolvedParentPageID, resolvedParentDatabaseID string + var client *mcp.Client + defer func() { + if client != nil { + _ = client.Close() + } + }() if skipLocalImages { body, err = stripLocalImages(body) if err != nil { @@ -594,6 +601,11 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL } if hasLocalImages && fm.NotionID != "" { + client, err = requirePageClientFn() + if err != nil { + return err + } + apiClient, err := cli.RequireOfficialAPIClient(officialAPIOverrides(ctx)) if err != nil { output.PrintError(err) @@ -619,34 +631,30 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL // For the create path, resolve parent IDs before any uploads so an // invalid --parent/--parent-db doesn't leave orphaned file uploads. if hasLocalImages && fm.NotionID == "" { - resolveClient, err := cli.RequireClient() + client, err = requirePageClientFn() if err != nil { return err } if parentDB != "" { - dbID, err := cli.ResolveDatabaseID(bgCtx, resolveClient, parentDB) + dbID, err := cli.ResolveDatabaseID(bgCtx, client, parentDB) if err != nil { - _ = resolveClient.Close() output.PrintError(err) return err } - dbID, err = resolveClient.ResolveDataSourceID(bgCtx, dbID) + dbID, err = client.ResolveDataSourceID(bgCtx, dbID) if err != nil { - _ = resolveClient.Close() output.PrintError(err) return err } resolvedParentDatabaseID = dbID } else if parent != "" { - parentID, err := cli.ResolvePageID(bgCtx, resolveClient, parent) + parentID, err := cli.ResolvePageID(bgCtx, client, parent) if err != nil { - _ = resolveClient.Close() output.PrintError(err) return err } resolvedParentPageID = parentID } - _ = resolveClient.Close() } body, localUploads, err = prepareLocalImageUploads(ctx, bgCtx, file, body) @@ -666,11 +674,12 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string, skipL icon, title = extractEmojiFromTitle(title) } - client, err := cli.RequireClient() - if err != nil { - return err + if client == nil { + client, err = requirePageClientFn() + if err != nil { + return err + } } - defer func() { _ = client.Close() }() if fm.NotionID != "" { req := mcp.UpdatePageRequest{ diff --git a/cmd/page_local_images_test.go b/cmd/page_local_images_test.go index ca7d07e..11b8fb2 100644 --- a/cmd/page_local_images_test.go +++ b/cmd/page_local_images_test.go @@ -3,6 +3,7 @@ package cmd import ( "context" "encoding/json" + "errors" "net/http" "net/http/httptest" "os" @@ -76,6 +77,54 @@ func TestPrepareLocalImageUploadsUploadsAndDeduplicates(t *testing.T) { } } +func TestRunPageSyncPreflightsMCPBeforeLocalImageUpload(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + + tmp := t.TempDir() + doc := filepath.Join(tmp, "doc.md") + img := filepath.Join(tmp, "diagram.png") + if err := os.WriteFile(img, []byte("PNGDATA"), 0o644); err != nil { + t.Fatalf("WriteFile image: %v", err) + } + if err := os.WriteFile(doc, []byte("---\nnotion-id: page_123\n---\n\n![Diagram](./diagram.png)\n"), 0o644); err != nil { + t.Fatalf("WriteFile doc: %v", err) + } + + uploadCalls := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && r.URL.Path == "/v1/pages/page_123/markdown": + _, _ = w.Write([]byte(`{"markdown":"# Previous\n","truncated":false}`)) + case r.Method == http.MethodPost && strings.HasPrefix(r.URL.Path, "/v1/file_uploads"): + uploadCalls++ + http.Error(w, "upload should not start before MCP auth", http.StatusInternalServerError) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + })) + defer srv.Close() + + wantErr := errors.New("mcp unavailable") + oldRequirePageClientFn := requirePageClientFn + requirePageClientFn = func() (*mcp.Client, error) { + return nil, wantErr + } + t.Cleanup(func() { + requirePageClientFn = oldRequirePageClientFn + }) + + err := runPageSync(&Context{ + APIToken: "secret-token", + APIBaseURL: srv.URL + "/v1", + }, doc, "", "", "", "", false) + if !errors.Is(err, wantErr) { + t.Fatalf("runPageSync error = %v, want %v", err, wantErr) + } + if uploadCalls != 0 { + t.Fatalf("uploadCalls = %d, want 0", uploadCalls) + } +} + func TestSubstituteUploadedLocalImagesAppendsAfterPlaceholderAndDeletes(t *testing.T) { var sawAppend bool var sawDelete bool From 688d6ff70c2b9efec56a5635d815d93071279bf4 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Thu, 30 Apr 2026 12:05:55 -0400 Subject: [PATCH 30/31] fix(images): honor blockquote indent limit --- internal/cli/local_images.go | 2 +- internal/cli/local_images_test.go | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/internal/cli/local_images.go b/internal/cli/local_images.go index a60eb40..16c7d97 100644 --- a/internal/cli/local_images.go +++ b/internal/cli/local_images.go @@ -401,7 +401,7 @@ func isStandaloneImageLine(line string, matches []markdownImageMatch) bool { // blockquote (a `>` optionally preceded by up to three spaces of indent). func isBlockquoteLine(line string) bool { i := 0 - for i < len(line) && i < 4 && line[i] == ' ' { + for i < len(line) && i < 3 && line[i] == ' ' { i++ } return i < len(line) && line[i] == '>' diff --git a/internal/cli/local_images_test.go b/internal/cli/local_images_test.go index 4400e4b..7b92c18 100644 --- a/internal/cli/local_images_test.go +++ b/internal/cli/local_images_test.go @@ -479,6 +479,26 @@ func TestFindStandaloneLocalImageLinesIgnoresBlockquotedImage(t *testing.T) { } } +func TestFindStandaloneLocalImageLinesHonorsBlockquoteIndentLimit(t *testing.T) { + blockquote := " > ![Quoted](./quoted.png)\n" + + rewritten, placements, err := FindStandaloneLocalImageLines(blockquote) + if err != nil { + t.Fatalf("FindStandaloneLocalImageLines: %v", err) + } + if len(placements) != 0 { + t.Fatalf("len(placements) = %d, want 0", len(placements)) + } + if rewritten != blockquote { + t.Fatalf("rewritten = %q, want input unchanged", rewritten) + } + + _, _, err = FindStandaloneLocalImageLines("text\n > ![Code](./code.png)\n") + if err == nil { + t.Fatal("expected local image syntax error for 4-space-indented > line") + } +} + func TestIsLocalDestinationHandlesWindowsAndURISchemes(t *testing.T) { cases := []struct { name string From d866ccf6e5a60f4ba03391de3c4259cbe1b12b80 Mon Sep 17 00:00:00 2001 From: Brian Le Date: Thu, 30 Apr 2026 12:58:08 -0400 Subject: [PATCH 31/31] fix(images): skip raw html blocks --- internal/cli/local_images.go | 140 ++++++++++++++++++++++++++++-- internal/cli/local_images_test.go | 56 ++++++++++++ 2 files changed, 188 insertions(+), 8 deletions(-) diff --git a/internal/cli/local_images.go b/internal/cli/local_images.go index 16c7d97..8d3c1b4 100644 --- a/internal/cli/local_images.go +++ b/internal/cli/local_images.go @@ -55,12 +55,12 @@ func FindStandaloneLocalImageLines(markdown string) (string, []LocalImagePlaceme } // scanStandaloneLocalImages walks markdown line-by-line, skipping fenced and -// indented code blocks, rejecting inline local images, and replacing each -// standalone local image line with a placeholder. When resolvePath is non-nil, -// it is invoked for every local destination and its returned path is stored on -// the placement's Resolved field; a non-nil error from resolvePath aborts the -// scan. When resolvePath is nil, placements are recorded without a Resolved -// path. +// indented code blocks and raw HTML blocks, rejecting inline local images, and +// replacing each standalone local image line with a placeholder. When +// resolvePath is non-nil, it is invoked for every local destination and its +// returned path is stored on the placement's Resolved field; a non-nil error +// from resolvePath aborts the scan. When resolvePath is nil, placements are +// recorded without a Resolved path. func scanStandaloneLocalImages(markdown string, resolvePath func(dest string) (string, error)) (string, []LocalImagePlacement, error) { normalizedMarkdown := strings.NewReplacer("\r\n", "\n", "\r", "\n").Replace(markdown) lines := strings.Split(normalizedMarkdown, "\n") @@ -70,11 +70,19 @@ func scanStandaloneLocalImages(markdown string, resolvePath func(dest string) (s var fenceChar byte var fenceLen int var inIndented bool + var htmlEnd htmlBlockEnd prevBlank := true for i, line := range lines { isBlank := strings.TrimSpace(line) == "" + if htmlEnd != htmlBlockEndNone { + if closesHTMLBlock(line, htmlEnd) { + htmlEnd = htmlBlockEndNone + } + prevBlank = isBlank + continue + } if inFence { if closesFencedCodeBlock(line, fenceChar, fenceLen) { inFence = false @@ -89,6 +97,13 @@ func scanStandaloneLocalImages(markdown string, resolvePath func(dest string) (s prevBlank = false continue } + if end := opensHTMLBlock(line); end != htmlBlockEndNone { + if !closesHTMLBlock(line, end) { + htmlEnd = end + } + prevBlank = isBlank + continue + } // Blockquotes typically hold documentation, examples, or quoted // code including fenced blocks with literal `![...](...)` samples. // The fence detector above ignores `>`-prefixed lines, so any @@ -407,9 +422,118 @@ func isBlockquoteLine(line string) bool { return i < len(line) && line[i] == '>' } +type htmlBlockEnd int + +const ( + htmlBlockEndNone htmlBlockEnd = iota + htmlBlockEndBlank + htmlBlockEndComment + htmlBlockEndProcessingInstruction + htmlBlockEndDeclaration + htmlBlockEndCDATA + htmlBlockEndScript + htmlBlockEndPre + htmlBlockEndStyle +) + +func opensHTMLBlock(line string) htmlBlockEnd { + trimmed := trimMarkdownBlockStart(line) + lower := strings.ToLower(trimmed) + switch { + case strings.HasPrefix(lower, "") + case htmlBlockEndProcessingInstruction: + return strings.Contains(line, "?>") + case htmlBlockEndDeclaration: + return strings.Contains(line, ">") + case htmlBlockEndCDATA: + return strings.Contains(line, "]]>") + case htmlBlockEndScript: + return strings.Contains(lower, "") + case htmlBlockEndPre: + return strings.Contains(lower, "") + case htmlBlockEndStyle: + return strings.Contains(lower, "") + default: + return true + } +} + +func trimMarkdownBlockStart(line string) string { + i := 0 + for i < len(line) && i < 3 && line[i] == ' ' { + i++ + } + return strings.TrimSpace(line[i:]) +} + +func startsBlockHTMLTag(s string) bool { + if !strings.HasPrefix(s, "<") { + return false + } + nameStart := 1 + if len(s) > 1 && s[1] == '/' { + nameStart = 2 + } + nameEnd := nameStart + for nameEnd < len(s) && isHTMLTagNameByte(s[nameEnd]) { + nameEnd++ + } + if nameEnd == nameStart { + return false + } + if nameEnd >= len(s) { + return false + } + next := s[nameEnd] + if next != ' ' && next != '\t' && next != '>' && next != '/' { + return false + } + return isBlockHTMLTag(strings.ToLower(s[nameStart:nameEnd])) +} + +func isHTMLTagNameByte(b byte) bool { + return (b >= 'a' && b <= 'z') || (b >= 'A' && b <= 'Z') || (b >= '0' && b <= '9') || b == '-' +} + +func isBlockHTMLTag(name string) bool { + switch name { + case "address", "article", "aside", "base", "basefont", "blockquote", "body", "caption", "center", "col", "colgroup", "dd", "details", "dialog", "dir", "div", "dl", "dt", "fieldset", "figcaption", "figure", "footer", "form", "frame", "frameset", "h1", "h2", "h3", "h4", "h5", "h6", "head", "header", "hr", "html", "iframe", "legend", "li", "link", "main", "menu", "menuitem", "nav", "noframes", "ol", "optgroup", "option", "p", "param", "search", "section", "summary", "table", "tbody", "td", "tfoot", "th", "thead", "title", "tr", "track", "ul": + return true + default: + return false + } +} + // opensFencedCodeBlock reports whether `line` opens a CommonMark fenced code -// block. It returns the fence character (`` ` `` or `~`) and the fence length -// (>= 3) on a match, and (0, 0) otherwise. +// block. It returns the fence character (backtick or `~`) and the fence +// length (>= 3) on a match, and (0, 0) otherwise. func opensFencedCodeBlock(line string) (byte, int) { i := 0 for i < len(line) && i < 4 && line[i] == ' ' { diff --git a/internal/cli/local_images_test.go b/internal/cli/local_images_test.go index 7b92c18..74d9760 100644 --- a/internal/cli/local_images_test.go +++ b/internal/cli/local_images_test.go @@ -499,6 +499,62 @@ func TestFindStandaloneLocalImageLinesHonorsBlockquoteIndentLimit(t *testing.T) } } +func TestFindStandaloneLocalImageLinesIgnoresRawHTMLBlock(t *testing.T) { + markdown := strings.Join([]string{ + "
", + "![Demo](./example.png)", + "
", + "", + "![Real](./real.png)", + "", + }, "\n") + + rewritten, placements, err := FindStandaloneLocalImageLines(markdown) + if err != nil { + t.Fatalf("FindStandaloneLocalImageLines: %v", err) + } + if len(placements) != 1 { + t.Fatalf("len(placements) = %d, want 1", len(placements)) + } + if !strings.Contains(rewritten, "![Demo](./example.png)") { + t.Fatalf("rewritten lost raw HTML image syntax: %q", rewritten) + } + if strings.Contains(rewritten, "![Real](./real.png)") { + t.Fatalf("rewritten should have replaced the real image line: %q", rewritten) + } +} + +func TestRewriteStandaloneLocalImagesIgnoresMissingImageInRawHTMLBlock(t *testing.T) { + dir := t.TempDir() + source := filepath.Join(dir, "doc.md") + real := filepath.Join(dir, "real.png") + if err := os.WriteFile(source, nil, 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(real, []byte("image"), 0o644); err != nil { + t.Fatal(err) + } + markdown := strings.Join([]string{ + "
", + "![Missing](./missing.png)", + "
", + "", + "![Real](./real.png)", + "", + }, "\n") + + _, placements, err := RewriteStandaloneLocalImages(markdown, source) + if err != nil { + t.Fatalf("RewriteStandaloneLocalImages: %v", err) + } + if len(placements) != 1 { + t.Fatalf("len(placements) = %d, want 1", len(placements)) + } + if placements[0].Resolved != real { + t.Fatalf("Resolved = %q, want %q", placements[0].Resolved, real) + } +} + func TestIsLocalDestinationHandlesWindowsAndURISchemes(t *testing.T) { cases := []struct { name string