fix(cli): stop a body file with no content from wiping the page

internal/cli/body.go

@@ -75,7 +75,10 @@ func loadBody(path string, stdin io.Reader, progress io.Writer, vars map[string]
 	if len(raw) > maxBodyFileBytes {
 		return nil, nil, Errorf(ExitUsage, "body file %s is over the %d-byte limit", path, maxBodyFileBytes)
 	}
-	if strings.TrimSpace(string(raw)) == "" {
+	// isBlank rather than TrimSpace: this guard is what stands between a file
+	// with no content and a wiped page, since a replace deletes the old body
+	// whether or not the new one parsed to anything.
+	if isBlank(string(raw)) {
 		return nil, nil, Errorf(ExitUsage, "body file %s is empty", path)
 	}
 	// Before parsing, not after: the expanded text is what the author meant to
@@ -88,6 +91,13 @@ func loadBody(path string, stdin io.Reader, progress io.Writer, vars map[string]
 			return nil, nil, Errorf(ExitUsage, "%s: %v", path, err)
 		}
 		raw = []byte(expanded)
+		// Checked again after expanding, as the append path does: a file of
+		// nothing but "{{ticket}}" is content on disk and expands to nothing
+		// when the row was addressed by --page-id or --id.
+		if isBlank(string(raw)) {
+			return nil, nil, Errorf(ExitUsage,
+				"body file %s expands to nothing: every placeholder in it resolved to an empty value", path)
+		}
 	}
 	blocks, warnings, err := markdown.ToBlocks(raw)
 	if err != nil {

internal/cli/body_test.go

@@ -1186,3 +1186,83 @@ func TestPreGateMessageDoesNotClaimAMeasuredRequestSize(t *testing.T) {
 		t.Errorf("the message should name Notion's limit, got: %v", err)
 	}
 }
+
+// --body-file REPLACES: the new blocks are appended, then every old one is
+// deleted. loadBody's empty guard is the only thing standing between a file
+// with no content and a body wipe — and strings.TrimSpace does not strip
+// U+FEFF, which left Unicode's White_Space in 6.3. A BOM-only file therefore
+// reads as content, goldmark returns zero blocks, ValidateAppendable returns
+// nil on the empty slice, splitIntoRequests emits no batch so no PATCH goes
+// out, and replaceBody deletes every child anyway. The body is gone, nothing
+// replaced it, and the run exits 0. See #37.
+func TestSetBodyFileHoldingOnlyInvisibleCharactersMakesNoHTTPCall(t *testing.T) {
+	cases := []struct {
+		name    string
+		content string
+	}{
+		{"a bare BOM", "\ufeff"},
+		{"a BOM with whitespace", "\ufeff  \n\t\n"},
+		{"a zero-width space", "\u200b"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			var calls int32
+			cfg := withStubbedAPI(t, func(w http.ResponseWriter, r *http.Request) {
+				if r.URL.Path != "/v1/data_sources/ds1" {
+					atomic.AddInt32(&calls, 1)
+				}
+				w.Write([]byte(cliSchemaJSON))
+			})
+
+			p := filepath.Join(t.TempDir(), "blank.md")
+			if err := os.WriteFile(p, []byte(tc.content), 0o600); err != nil {
+				t.Fatal(err)
+			}
+
+			if code := executeArgs([]string{"set", "--ticket", "BDF-231",
+				"--body-file", p, "--config", cfg}); code != ExitUsage {
+				t.Fatalf("a body file with no content must exit %d, got %d", ExitUsage, code)
+			}
+			if n := atomic.LoadInt32(&calls); n != 0 {
+				t.Fatalf("it must be refused before anything reaches Notion, got %d calls", n)
+			}
+		})
+	}
+}
+
+// The overcorrection this fix must not make: a BOM in front of real content is
+// an ordinary file, and refusing it would break writing bodies from any editor
+// that emits one.
+func TestBodyFileKeepsAFileWhoseBOMPrefixesRealContent(t *testing.T) {
+	p := filepath.Join(t.TempDir(), "bom-content.md")
+	if err := os.WriteFile(p, []byte("\ufeff## Progress\nShipped.\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+
+	req, _, err := loadBody(p, nil, nil, nil)
+	if err != nil {
+		t.Fatalf("a BOM before real content must not make the file blank: %v", err)
+	}
+	if len(req.Blocks) == 0 {
+		t.Fatal("the content parsed to no blocks at all")
+	}
+}
+
+// The twin hole, on the same replace path: --expand can empty a file that was
+// non-blank on disk. loadAppendBody re-checks after expanding; loadBody did
+// not, so a file of nothing but "{{ticket}}" addressed by --page-id parsed to
+// zero blocks and the body was deleted with nothing to replace it.
+func TestBodyFileThatExpandsToNothingIsRefused(t *testing.T) {
+	p := filepath.Join(t.TempDir(), "expand.md")
+	if err := os.WriteFile(p, []byte("\ufeff{{ticket}}\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+
+	_, _, err := loadBody(p, nil, nil, map[string]string{"ticket": ""})
+	if err == nil {
+		t.Fatal("a body file that expands to nothing must be refused")
+	}
+	if exitCodeFor(err) != ExitUsage {
+		t.Fatalf("want a usage error, got %v (code %d)", err, exitCodeFor(err))
+	}
+}

internal/service/service.go

@@ -287,6 +287,15 @@ const sentinelAdvice = "re-run to converge"
 // happen), which the CLI surfaces even in the partial-failure JSON (spec §8).
 func (s *Service) replaceBody(ctx context.Context, pageID string, req *BodyRequest) (BodyResult, error) {
 	var res BodyResult
+	// A replace with nothing to write is a delete. The CLI already refuses a
+	// blank file, but that guard is one check in one command's argument
+	// handling and it has been wrong before (#37: it did not strip a BOM, and
+	// a BOM-only file parses to zero blocks). The page's content is not
+	// recoverable, so the refusal belongs here too, next to the deleting.
+	if len(req.Blocks) == 0 {
+		return res, errors.New(
+			"refusing to replace a body with no blocks: this would delete the page's content and write nothing in its place")
+	}
 	old, err := s.client.ListBlockChildren(ctx, pageID)
 	if err != nil {
 		return res, err

internal/service/service_test.go

@@ -1253,3 +1253,25 @@ func TestAmbiguousAppendErrorUnwrapsThroughExtraWrapping(t *testing.T) {
 		t.Fatal("errors.Is must still reach the sentinel through extra wrapping")
 	}
 }
+
+// replaceBody deletes the page's existing blocks after writing the new ones.
+// With no blocks to write, "replace" is just the delete: the body goes and
+// nothing takes its place, and the run reports success. Nothing upstream
+// should send that — loadBody rejects a blank file — but that guard is one
+// TrimSpace in one command's argument handling, and it has been wrong (#37:
+// it did not strip a BOM, and a BOM-only file parses to zero blocks). The
+// property belongs here too, where the deleting actually happens.
+func TestReplaceBodyRefusesToDeleteWhenThereIsNothingToWrite(t *testing.T) {
+	var seen []string
+	srv := bodyRoutes(t, rowJSON, `{"id":"old1","type":"paragraph"}`, &seen)
+	defer srv.Close()
+
+	s := New(notion.New("t", notion.WithBaseURL(srv.URL), notion.WithSleep(func(time.Duration) {})), testProfile())
+	_, err := s.Set(context.Background(), tracker.Fields{Ticket: "BDF-231"}, &BodyRequest{})
+	if indexOf(seen, "DELETE", "/blocks/old1") >= 0 {
+		t.Fatalf("the old body was deleted with nothing to replace it: %v", seen)
+	}
+	if err == nil {
+		t.Fatal("replacing a body with no blocks must be refused, not performed")
+	}
+}

skills/notion-track/SKILL.md

@@ -256,8 +256,10 @@ REQUEST, not the file: an append is one payload and Notion caps a payload at
 500KB, and the serialized request is always bigger than the file (Markdown is
 escaped into JSON, so every newline costs two bytes, and `--expand` runs first).
 A 450KB file of short lines can exceed it. Over the limit is exit 2 before any
-request, with the request size named — an append cannot be split, so send it in
-two runs or use `--body-file`, which is batched and allows 1 MiB. Mutually
+request. The message names the request size when it built one, and a floor on
+the file size (`is at least N bytes`) when the file alone is already too big to
+be worth serializing — an append cannot be split, so send it in two runs or use
+`--body-file`, which is batched and allows 1 MiB. Mutually
 exclusive with `--body-file` — one write picks one strategy.
 
 **Not idempotent.** Running it twice appends the note twice; there is no