fix(sync): stop bidirectional sync from clobbering local writes; add opt-in overwrite to move_page

README.md

@@ -93,7 +93,7 @@ Both modes use the same wiki engine and the same wiki directory (`~/.mind-map/wi
 | `get_page` | Read a page with parsed frontmatter, body, outgoing links, and backlinks |
 | `create_page` | Create a new page (markdown with optional YAML frontmatter) |
 | `update_page` | Update an existing page's content |
-| `move_page` | Rename or relocate a page atomically (use instead of `create_page` + `delete_page`) |
+| `move_page` | Rename or relocate a page atomically (use instead of `create_page` + `delete_page`). Refuses to overwrite an existing destination unless `overwrite: true` is passed — agents should ask the user first. |
 | `delete_page` | Delete a page from the wiki and search index |
 | `list_pages` | List pages, optionally filtered by path prefix |
 | `get_backlinks` | Get all pages that link to a given page |

SKILL.md

@@ -103,6 +103,14 @@ move_page(from: "projects/old-name", to: "projects/new-name")
 
 `move_page` is **atomic** — it renames the file on disk, updates the index, and rewrites the page's outgoing-link rows in one step. **Always use `move_page` instead of `create_page` + `delete_page`** to avoid leaving duplicate pages behind. Backlinks from other pages (other pages with `[[old-name]]` in their source) are intentionally not rewritten; if you want those updated, search and edit the source pages explicitly.
 
+If the destination already exists, `move_page` fails with a message containing `destination already exists`. **Ask the user whether to overwrite** — the destination's content will be lost — and only then retry with `overwrite: true`:
+
+```
+move_page(from: "projects/old-name", to: "projects/new-name", overwrite: true)
+```
+
+Never set `overwrite: true` without explicit user confirmation: it is destructive.
+
 ## Listing Pages
 
 ```
@@ -160,7 +168,19 @@ When in doubt, prefer **deeper paths over wider ones**. A page at `projects/foo/
 
 ## Sync (read-only or read-write)
 
-`register_sync` ties a path prefix to a git remote. Sync is bidirectional by default but supports `direction: "pull"` (read-only) and `direction: "push"` (write-only) — useful when the wiki content is owned upstream (e.g. a project's GitHub wiki) and the local wiki is a working copy that should never push back. Respect existing sync mappings: don't reshape paths under a prefix that's syncing somewhere else without confirming with the user first.
+`register_sync` ties a path prefix to a git remote and supports three directions:
+
+```
+register_sync(prefix: "projects/foo", remote: "https://github.com/user/foo.wiki.git")
+register_sync(prefix: "docs/upstream", remote: "https://example.com/upstream.wiki.git", direction: "pull")
+register_sync(prefix: "publish/blog",  remote: "https://example.com/blog.wiki.git",     direction: "push")
+```
+
+- `bidirectional` (default): pull from the remote and push wiki changes back.
+- `pull`: read-only mirror — remote changes flow into the wiki; the wiki never pushes. Use this when the content is owned upstream (e.g. another project's GitHub wiki).
+- `push`: write-only — wiki changes flow to the remote; the remote never overwrites the wiki. Use this for publishing a curated subtree.
+
+Re-registering the same prefix replaces the previous direction. Respect existing sync mappings: don't reshape paths under a prefix that's syncing somewhere else without confirming with the user first.
 
 ## Page Format Example
 

internal/config/config.go

@@ -82,15 +82,25 @@ func (s *SyncConfig) ResolveRemote(pagePath string) string {
 	return s.Default
 }
 
-// AddMapping adds or updates a prefix-to-remote mapping.
-func (s *SyncConfig) AddMapping(prefix, remote string) {
+// AddMapping adds or updates a prefix-to-remote mapping with the given
+// direction. An empty or unrecognized direction normalizes to
+// SyncBidirectional, so callers that don't care about direction can
+// pass "" (or SyncBidirectional explicitly) and get the safe default.
+//
+// If a mapping for prefix already exists, its remote and direction are
+// both replaced — this is treated as a re-registration, not an additive
+// op, so an existing mapping switching from bidirectional to pull-only
+// (or vice versa) propagates cleanly.
+func (s *SyncConfig) AddMapping(prefix, remote string, direction SyncDirection) {
+	direction = direction.Normalize()
 	for i, m := range s.Mappings {
 		if m.Prefix == prefix {
 			s.Mappings[i].Remote = remote
+			s.Mappings[i].Direction = direction
 			return
 		}
 	}
-	s.Mappings = append(s.Mappings, SyncMapping{Prefix: prefix, Remote: remote})
+	s.Mappings = append(s.Mappings, SyncMapping{Prefix: prefix, Remote: remote, Direction: direction})
 }
 
 // Remotes returns all unique remotes (default + mappings).

internal/config/config_test.go

@@ -76,25 +76,38 @@ func TestResolveRemote(t *testing.T) {
 func TestAddMapping(t *testing.T) {
 	s := &SyncConfig{}
 
-	s.AddMapping("projects/mind-map", "https://github.com/user/mind-map.wiki.git")
+	s.AddMapping("projects/mind-map", "https://github.com/user/mind-map.wiki.git", SyncBidirectional)
 	if len(s.Mappings) != 1 {
 		t.Fatalf("expected 1 mapping, got %d", len(s.Mappings))
 	}
+	if s.Mappings[0].Direction != SyncBidirectional {
+		t.Errorf("direction = %q, want bidirectional", s.Mappings[0].Direction)
+	}
 
-	// Update existing
-	s.AddMapping("projects/mind-map", "https://github.com/user/mind-map-v2.wiki.git")
+	// Update existing — re-registration replaces both remote and direction.
+	s.AddMapping("projects/mind-map", "https://github.com/user/mind-map-v2.wiki.git", SyncPull)
 	if len(s.Mappings) != 1 {
 		t.Fatalf("expected 1 mapping after update, got %d", len(s.Mappings))
 	}
 	if s.Mappings[0].Remote != "https://github.com/user/mind-map-v2.wiki.git" {
-		t.Errorf("mapping not updated: %q", s.Mappings[0].Remote)
+		t.Errorf("mapping remote not updated: %q", s.Mappings[0].Remote)
+	}
+	if s.Mappings[0].Direction != SyncPull {
+		t.Errorf("mapping direction not updated: %q", s.Mappings[0].Direction)
 	}
 
 	// Add another
-	s.AddMapping("projects/other", "https://github.com/user/other.wiki.git")
+	s.AddMapping("projects/other", "https://github.com/user/other.wiki.git", SyncPush)
 	if len(s.Mappings) != 2 {
 		t.Fatalf("expected 2 mappings, got %d", len(s.Mappings))
 	}
+
+	// Empty direction normalizes to bidirectional so callers that don't
+	// care can pass "" and get the safe default.
+	s.AddMapping("projects/default", "https://github.com/user/default.wiki.git", "")
+	if s.Mappings[2].Direction != SyncBidirectional {
+		t.Errorf("empty direction did not normalize to bidirectional, got %q", s.Mappings[2].Direction)
+	}
 }
 
 func TestRemotes(t *testing.T) {
@@ -128,7 +141,7 @@ func TestSaveAndLoad(t *testing.T) {
 	cfg.Sync.Enabled = true
 	cfg.Sync.Default = "https://github.com/user/wiki.wiki.git"
 	cfg.Sync.Interval = "1m"
-	cfg.Sync.AddMapping("projects/mind-map", "https://github.com/user/mind-map.wiki.git")
+	cfg.Sync.AddMapping("projects/mind-map", "https://github.com/user/mind-map.wiki.git", SyncBidirectional)
 
 	if err := Save(path, cfg); err != nil {
 		t.Fatalf("Save: %v", err)

internal/mcp/server.go

@@ -5,19 +5,21 @@ package mcp
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"log/slog"
 	"strings"
 	"time"
 
+	"github.com/aniongithub/mind-map/internal/config"
 	"github.com/aniongithub/mind-map/internal/wiki"
 	"github.com/modelcontextprotocol/go-sdk/mcp"
 )
 
 // SyncRegistrar allows the MCP server to register sync mappings and
 // check whether a page path has a sync target configured.
 type SyncRegistrar interface {
-	RegisterMapping(prefix, remote string) error
+	RegisterMapping(prefix, remote string, direction config.SyncDirection) error
 	HasMapping(pagePath string) bool
 }
 
@@ -84,7 +86,7 @@ func (s *Server) registerTools() {
 
 	mcp.AddTool(s.server, &mcp.Tool{
 		Name:        "move_page",
-		Description: "Rename or relocate a wiki page atomically. Moves the underlying file from one path to another, updates the index, and rewrites the page's outgoing links. Fails if the destination already exists. Use this instead of create_page + delete_page to avoid leaving duplicate pages behind.",
+		Description: "Rename or relocate a wiki page atomically. Moves the underlying file from one path to another, updates the index, and rewrites the page's outgoing links. Fails if the destination already exists, unless overwrite=true. Use this instead of create_page + delete_page to avoid leaving duplicate pages behind. When the destination exists, ask the user whether to overwrite (the destination's content will be lost) before retrying with overwrite=true.",
 	}, s.movePage)
 
 	mcp.AddTool(s.server, &mcp.Tool{
@@ -99,7 +101,7 @@ func (s *Server) registerTools() {
 
 	mcp.AddTool(s.server, &mcp.Tool{
 		Name:        "register_sync",
-		Description: "Register a wiki path prefix to sync with a git remote. Pages under this prefix will be synced to the given repository's wiki. The remote URL should be a git clone URL (e.g. https://github.com/user/repo.wiki.git). Auth uses the machine's existing git credentials.",
+		Description: "Register a wiki path prefix to sync with a git remote. Pages under this prefix will be synced to the given repository's wiki. The remote URL should be a git clone URL (e.g. https://github.com/user/repo.wiki.git). Direction defaults to 'bidirectional' (pull+push); use 'pull' to mirror an upstream repo read-only into the wiki, or 'push' to publish wiki content to a remote without ever pulling from it. Re-registering the same prefix replaces the previous direction. Auth uses the machine's existing git credentials.",
 	}, s.registerSync)
 }
 
@@ -131,11 +133,17 @@ type listInput struct {
 type registerSyncInput struct {
 	Prefix string `json:"prefix" jsonschema:"wiki path prefix to sync, e.g. projects/mind-map"`
 	Remote string `json:"remote" jsonschema:"git remote URL, e.g. https://github.com/user/repo.wiki.git"`
+	// Direction is optional. Omitted or empty means bidirectional.
+	Direction string `json:"direction,omitempty" jsonschema:"sync direction: 'bidirectional' (default), 'pull' (mirror remote read-only into wiki), or 'push' (publish wiki to remote, never pulling)"`
 }
 
 type moveInput struct {
 	From string `json:"from" jsonschema:"current page path without .md extension"`
 	To   string `json:"to" jsonschema:"new page path without .md extension"`
+	// Overwrite is opt-in by design. The default-false behavior matches
+	// the long-standing safety contract: a move never destroys data
+	// unless the caller (after asking the user) explicitly says so.
+	Overwrite bool `json:"overwrite,omitempty" jsonschema:"set true to replace an existing destination page; ask the user for explicit confirmation first since the destination's content will be lost"`
 }
 
 // --- Tool handlers ---
@@ -229,14 +237,33 @@ func (s *Server) deletePage(ctx context.Context, _ *mcp.CallToolRequest, input p
 
 func (s *Server) movePage(ctx context.Context, _ *mcp.CallToolRequest, input moveInput) (*mcp.CallToolResult, any, error) {
 	start := time.Now()
-	if err := s.wiki.MovePage(ctx, input.From, input.To); err != nil {
+	err := s.wiki.MovePage(ctx, input.From, input.To, wiki.MoveOptions{Overwrite: input.Overwrite})
+	if err != nil {
+		// Make the "destination already exists" case actionable for
+		// the agent: a clear hint that overwrite=true (after user
+		// confirmation) is the way forward, rather than a generic
+		// failure that invites a retry loop.
+		if errors.Is(err, wiki.ErrDestinationExists) {
+			slog.Info("tool.move_page rejected: destination exists",
+				slog.String("from", input.From), slog.String("to", input.To))
+			return nil, nil, fmt.Errorf("%w. Ask the user whether to overwrite %q (its content will be lost), then retry with overwrite=true if they agree", err, input.To)
+		}
 		slog.Error("tool.move_page failed", slog.String("from", input.From), slog.String("to", input.To), slog.Any("error", err))
 		return nil, nil, err
 	}
-	slog.Info("tool.move_page", slog.String("from", input.From), slog.String("to", input.To), slog.Duration("elapsed", time.Since(start)))
+	slog.Info("tool.move_page",
+		slog.String("from", input.From),
+		slog.String("to", input.To),
+		slog.Bool("overwrite", input.Overwrite),
+		slog.Duration("elapsed", time.Since(start)),
+	)
+	msg := fmt.Sprintf("Moved page: %s → %s", input.From, input.To)
+	if input.Overwrite {
+		msg += " (overwrote existing destination)"
+	}
 	return &mcp.CallToolResult{
 		Content: []mcp.Content{
-			&mcp.TextContent{Text: fmt.Sprintf("Moved page: %s → %s", input.From, input.To)},
+			&mcp.TextContent{Text: msg},
 		},
 	}, nil, nil
 }
@@ -289,15 +316,43 @@ func (s *Server) registerSync(_ context.Context, _ *mcp.CallToolRequest, input r
 		return nil, nil, fmt.Errorf("both prefix and remote are required")
 	}
 
-	if err := s.sync.RegisterMapping(input.Prefix, input.Remote); err != nil {
-		slog.Error("tool.register_sync failed", slog.String("prefix", input.Prefix), slog.Any("error", err))
+	// Validate direction up-front so a typo gives the agent a clear
+	// error instead of silently being normalized to bidirectional.
+	direction := config.SyncDirection(input.Direction)
+	if input.Direction != "" && direction.Normalize() != direction {
+		return nil, nil, fmt.Errorf("invalid direction %q: must be one of 'bidirectional', 'pull', 'push' (or omitted for bidirectional)", input.Direction)
+	}
+	if direction == "" {
+		direction = config.SyncBidirectional
+	}
+
+	if err := s.sync.RegisterMapping(input.Prefix, input.Remote, direction); err != nil {
+		slog.Error("tool.register_sync failed",
+			slog.String("prefix", input.Prefix),
+			slog.String("direction", string(direction)),
+			slog.Any("error", err),
+		)
 		return nil, nil, err
 	}
 
-	slog.Info("tool.register_sync", slog.String("prefix", input.Prefix), slog.String("remote", input.Remote))
+	slog.Info("tool.register_sync",
+		slog.String("prefix", input.Prefix),
+		slog.String("remote", input.Remote),
+		slog.String("direction", string(direction)),
+	)
+
+	msg := fmt.Sprintf("Sync registered: pages under '%s' will sync to %s", input.Prefix, input.Remote)
+	switch direction {
+	case config.SyncPull:
+		msg += " (pull-only: changes flow from the remote into the wiki, never back)"
+	case config.SyncPush:
+		msg += " (push-only: changes flow from the wiki to the remote, never back)"
+	default:
+		msg += " (bidirectional)"
+	}
 	return &mcp.CallToolResult{
 		Content: []mcp.Content{
-			&mcp.TextContent{Text: fmt.Sprintf("Sync registered: pages under '%s' will sync to %s", input.Prefix, input.Remote)},
+			&mcp.TextContent{Text: msg},
 		},
 	}, nil, nil
 }