diff --git a/README.md b/README.md index 5f51279..e67f4b8 100644 --- a/README.md +++ b/README.md @@ -86,12 +86,16 @@ 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 +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 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 +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 @@ -105,6 +109,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`. 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 ```bash @@ -170,6 +176,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/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 8f306a4..8529331 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" @@ -25,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"` @@ -249,20 +251,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) @@ -270,16 +273,18 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string) err } markdown := string(content) - - if title == "" { - title = extractTitleFromMarkdown(markdown) - } - if title == "" { - title = strings.TrimSuffix(filepath.Base(file), filepath.Ext(file)) - } - - if icon == "" { - icon, title = extractEmojiFromTitle(title) + bgCtx := context.Background() + if skipLocalImages { + markdown, err = stripLocalImages(markdown) + if err != nil { + output.PrintError(err) + return err + } + } else { + if err := checkLocalImageParent(markdown, parent, parentDB); err != nil { + output.PrintError(err) + return err + } } client, err := cli.RequireClient() @@ -288,13 +293,9 @@ 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, - } - + // 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 { @@ -316,11 +317,39 @@ func runPageUpload(ctx *Context, file, title, parent, parentDB, icon string) err 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) return err } + pageID := pageIDFromCreateResponse(resp) + if err := substituteOrCleanup(ctx, bgCtx, pageID, resp.URL, localUploads); err != nil { + output.PrintError(err) + return err + } displayTitle := title if icon != "" { @@ -329,7 +358,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, @@ -513,20 +542,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) @@ -535,6 +565,104 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string) error content := string(raw) fm, body := cli.ParseFrontmatter(content) + bgCtx := context.Background() + 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 { + output.PrintError(err) + 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 != "" { + client, err = requirePageClientFn() + if err != nil { + return err + } + + 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 + } + 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 + } + } + + // 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 == "" { + client, err = requirePageClientFn() + if err != nil { + return err + } + if parentDB != "" { + dbID, err := cli.ResolveDatabaseID(bgCtx, client, parentDB) + if err != nil { + output.PrintError(err) + return err + } + dbID, err = client.ResolveDataSourceID(bgCtx, dbID) + if err != nil { + output.PrintError(err) + return err + } + resolvedParentDatabaseID = dbID + } else if parent != "" { + parentID, err := cli.ResolvePageID(bgCtx, client, parent) + if err != nil { + output.PrintError(err) + return err + } + resolvedParentPageID = parentID + } + } + + body, localUploads, err = prepareLocalImageUploads(ctx, bgCtx, file, body) + if err != nil { + output.PrintError(err) + return err + } + } if title == "" { title = extractTitleFromMarkdown(body) @@ -546,13 +674,12 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string) error 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() }() - - bgCtx := context.Background() if fm.NotionID != "" { req := mcp.UpdatePageRequest{ @@ -564,6 +691,15 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string) error output.PrintError(err) return err } + 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 { + finalErr = fmt.Errorf("%w (rollback failed: %v)", finalErr, rollbackErr) + } + output.PrintError(finalErr) + return finalErr + } displayTitle := title if icon != "" { @@ -588,7 +724,13 @@ func runPageSync(ctx *Context, file, title, parent, parentDB, icon string) error 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) @@ -615,9 +757,10 @@ 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 := substituteOrCleanup(ctx, bgCtx, pageID, resp.URL, 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 new file mode 100644 index 0000000..c025de7 --- /dev/null +++ b/cmd/page_local_images.go @@ -0,0 +1,283 @@ +package cmd + +import ( + "context" + "fmt" + "os" + "strings" + "sync" + + "golang.org/x/sync/errgroup" + + "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" +) + +// 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 + FileUploadID string + Placeholder string + ResolvedPath string +} + +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 + } + if len(placements) == 0 { + return markdown, nil, nil + } + + apiClient, err := cli.RequireOfficialAPIClient(officialAPIOverrides(cmdCtx)) + if err != nil { + return "", nil, err + } + + // 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 { + 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 err + } + 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: uploadIDByPath[placement.Resolved], + Placeholder: placement.Placeholder, + ResolvedPath: placement.Resolved, + }) + } + + 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 { + 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 { + // 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] = "" + } + } + + return strings.Join(lines, "\n"), nil +} + +// 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 { + return err + } + if len(placements) == 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", + } +} + +// substituteOrCleanup substitutes uploaded local images into the page and +// 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 != "" { + 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) + } + } 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 + } + return nil +} + +func substituteUploadedLocalImages(cmdCtx *Context, ctx context.Context, pageID string, uploads []uploadedLocalImage) error { + 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 { + 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 != api.BlockTypeParagraph || 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 +} + +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 { + 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", + 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..11b8fb2 --- /dev/null +++ b/cmd/page_local_images_test.go @@ -0,0 +1,338 @@ +package cmd + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "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") + 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() + + cmdCtx := &Context{ + APIToken: "secret-token", + APIBaseURL: srv.URL + "/v1", + } + + rewritten, uploads, err := prepareLocalImageUploads(cmdCtx, 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 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 + + 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) + } + 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": + sawDelete = true + w.WriteHeader(http.StatusOK) + default: + t.Fatalf("unexpected request: %s %s", r.Method, r.URL.Path) + } + })) + defer srv.Close() + + cmdCtx := &Context{ + APIToken: "secret-token", + APIBaseURL: srv.URL + "/v1", + } + + err := substituteUploadedLocalImages(cmdCtx, 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) + } +} + +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 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 { + t.Fatalf("expected nil when no uploads, got %v", err) + } +} + +func TestCheckLocalImageParent(t *testing.T) { + const markdownWithImage = "![Diagram](./diagram.png)\n" + const markdownWithoutImage = "# Just text\n" + + if err := checkLocalImageParent(markdownWithoutImage, "", ""); err != nil { + t.Fatalf("expected nil without local images, got %v", err) + } + if err := checkLocalImageParent(markdownWithImage, "parent-id", ""); err != nil { + t.Fatalf("expected nil with parent, got %v", err) + } + if err := checkLocalImageParent(markdownWithImage, "", "db-id"); err != nil { + t.Fatalf("expected nil with parent db, got %v", err) + } + + err := checkLocalImageParent(markdownWithImage, "", "") + 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) + } +} + +// 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 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) + } +} + +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"} + 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) + } +} diff --git a/cmd/page_test.go b/cmd/page_test.go new file mode 100644 index 0000000..658aad7 --- /dev/null +++ b/cmd/page_test.go @@ -0,0 +1,47 @@ +package cmd + +import ( + "testing" +) + +func TestStripLocalImagesRemovesStandaloneLocalImageLines(t *testing.T) { + markdown := "# Title\n\n![Local](./diagram.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) + } +} + +func TestStripLocalImagesPreservesRemoteImages(t *testing.T) { + markdown := "![Remote](https://example.com/remote.png)\n![Local](./diagram.png)\n" + + got, err := stripLocalImages(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) + } +} + +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/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, + } +} 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 d05c705..9c1dd01 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -1,13 +1,16 @@ package api import ( + "bufio" "bytes" "context" "encoding/json" "fmt" "io" + "mime" "mime/multipart" "net/http" + "net/textproto" "path/filepath" "strings" "time" @@ -17,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 @@ -124,19 +134,34 @@ 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 == "" { +// 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 +// 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 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") } 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 @@ -145,7 +170,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 } @@ -156,6 +181,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) @@ -171,8 +202,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, }, } @@ -187,20 +218,19 @@ 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{child}, "position": map[string]any{ "type": "after_block", "after_block": map[string]any{ "id": afterBlockID, }, }, - "children": []map[string]any{ - { - "object": "block", - "type": "image", - "image": image, - }, - }, } return c.doJSON(ctx, http.MethodPatch, "/blocks/"+parentID+"/children", payload, nil) @@ -310,32 +340,69 @@ 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) - part, err := writer.CreateFormFile("file", filename) - 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) + pr, pw := io.Pipe() + writer := multipart.NewWriter(pw) + + header := make(textproto.MIMEHeader) + contentDisposition := mime.FormatMediaType("form-data", map[string]string{ + "name": "file", + "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", contentType) + + 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) } 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..1134ca4 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,24 @@ 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.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) + } _, _ = w.Write([]byte(`{"id":"upload_123","status":"uploaded"}`)) case r.Method == http.MethodGet && r.URL.Path == "/v1/file_uploads/upload_123": getCalls++ @@ -96,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.UploadFileBytes(context.Background(), `diag"ram.png`, []byte("PNGDATA")) if err != nil { t.Fatalf("UploadFile: %v", err) } @@ -152,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) } @@ -240,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()) + } +} diff --git a/internal/cli/local_images.go b/internal/cli/local_images.go new file mode 100644 index 0000000..8d3c1b4 --- /dev/null +++ b/internal/cli/local_images.go @@ -0,0 +1,763 @@ +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 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) + + 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, skipping fenced and +// 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") + placements := make([]LocalImagePlacement, 0) + + var inFence bool + 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 + } + prevBlank = isBlank + continue + } + if c, n := opensFencedCodeBlock(line); n > 0 { + inFence = true + fenceChar = c + fenceLen = n + 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 + // 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) { + prevBlank = isBlank + continue + } + inIndented = false + } else if prevBlank && !isBlank && startsWithCodeIndent(line) { + inIndented = true + prevBlank = false + continue + } + + scanLine := maskInlineCodeSpans(line) + matches := findMarkdownImages(scanLine) + if len(matches) == 0 { + prevBlank = isBlank + continue + } + + 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 + } + + m := matches[0] + dest, ok := parseMarkdownDestination(m.dest) + if !ok || !isLocalDestination(dest) { + prevBlank = isBlank + continue + } + + 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(), "-", "_") + // 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, + Resolved: resolvedPath, + Placeholder: placeholder, + }) + prevBlank = false + } + + return strings.Join(lines, "\n"), placements, nil +} + +type markdownImageMatch struct { + start int + end int + alt string + 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 +// `!` 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 + 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 + } + 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 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 + } + switch c { + case '[': + depth++ + case ']': + if depth == 0 { + return i, true + } + depth-- + } + } + 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 `>`, 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++ { + c := line[i] + if c == '\\' && i+1 < len(line) { + i++ + continue + } + if c == '>' { + return skipOptionalTitleAndClose(line, i+1) + } + } + 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-- + 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 +} + +// 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 { + if len(matches) != 1 { + return false + } + m := matches[0] + 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 < 3 && line[i] == ' ' { + i++ + } + 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 (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] == ' ' { + 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 == "" { + return "", false + } + + if strings.HasPrefix(s, "<") { + // 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 + } + } + } + + 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 unescapeMarkdownPunctuation(s), true +} + +// unescapeMarkdownPunctuation turns CommonMark backslash escapes of ASCII +// 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 + } + 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]) || 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 { + d := strings.TrimSpace(dest) + if d == "" { + return false + } + + lower := strings.ToLower(d) + switch { + case strings.HasPrefix(lower, "#"), + strings.HasPrefix(lower, "//"), + 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 + } + + // 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://") { + 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) + } + // 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 + } + } + + // 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, d[2:]) + } + + 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..74d9760 --- /dev/null +++ b/internal/cli/local_images_test.go @@ -0,0 +1,582 @@ +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 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") + 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) + } +} + +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) + } +} + +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 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 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) + 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 (placeholder must sit at column 0)", rewritten, want) + } +} + +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 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 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 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) + 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{ + "```", + "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) + } +} + +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 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 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 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 + 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) + } + }) + } +} 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