feat(doc): add --selection-with-ellipsis position flag to +media-insert (#335)

* feat(doc): add --after-keyword/--before-keyword flags to +media-insert Allows inserting images/files at a position relative to the first block whose plain text matches a keyword (case-insensitive substring match). - Add --after-keyword: insert after the matched root-level block - Add --before-keyword: insert before the matched root-level block - Flags are mutually exclusive; default behavior (append to end) unchanged - fetchAllBlocks: paginated block listing (up to 50 pages × 200 blocks) - extractBlockPlainText: covers text, heading1-9, bullet, ordered, todo, code, quote - findInsertIndexByKeyword: walks parent_id chain to resolve nested blocks to their root-level ancestor - DryRun updated to show block-listing step when keyword flag is set * test(doc): add fetchAllBlocks pagination and keyword dry-run coverage - TestFetchAllBlocksPaginationViaExecute: exercises fetchAllBlocks via a full Execute flow with --after-keyword, covering multi-page block listing (fetchAllBlocks was previously at 0% coverage) - TestDocMediaInsertDryRunWithAfterKeyword: verifies that the dry-run output includes a block-listing step and mentions "search blocks" in the description when --after-keyword is provided fetchAllBlocks coverage: 0% → 76.2% * refactor(doc): use MCP locate-doc for keyword-based block positioning Replace fetchAllBlocks + keyword scan with MCP locate-doc tool, consistent with DriveAddComment. Flags changed from --after-keyword / --before-keyword to --selection-with-ellipsis + --before. * fix(doc): show <locate_index> in dry-run create-block when selection is set When --selection-with-ellipsis is provided, the create-block step in dry-run now shows index: "<locate_index>" instead of "<children_len>" to accurately reflect that the insertion position is computed from MCP locate-doc, not appended to end. * fix(doc): address CodeRabbit review on +media-insert selection feature - Validate: reject blank/whitespace --selection-with-ellipsis unconditionally so a mis-typed empty value cannot silently fall back to append-mode. - Redact the raw selection string when logging to stderr and when emitting error messages. --selection-with-ellipsis is copied verbatim from document content and may contain confidential text; the new redactSelection helper keeps a short prefix and rune count so operators can still identify the failing selection. - Harden the after/before mode tests: root children now have three entries so the two modes land on different indices, and the tests decode the create-block request body to assert the computed `index` actually reaches the /children API. A regression that ignored --before would now fail. - Harden the nested-block test so it exercises the fallback parent-walk: the anchor is now two levels deep (blk_grandchild under blk_section_child under blk_section), which forces the walk to fetch the intermediate block via GET /blocks/{id} to discover the root-level ancestor. * fix(doc): harden +media-insert selection UX on top of #335 (#577) Follow-up to #335 review: closes a handful of UX and robustness gaps in the new --selection-with-ellipsis flow. - Flag description rewritten to make the "insert at the top-level ancestor" semantics explicit — when the selection is inside a callout, table cell, or nested list, media lands outside that container, not inside. Also calls out the 'start...end' disambiguator. - locate-doc is now called with limit=2 so an ambiguous selection (same phrase in more than one block) surfaces a stderr warning pointing at 'start...end', instead of silently picking the first match. The first-match return behaviour is unchanged. - When the anchor is nested below the root, locateInsertIndex now logs a note to stderr naming the walk depth and the root-level ancestor's insert index. Users don't have to guess why the image landed outside the callout they were editing. - maxDepth bumped 8 → 32 with a comment explaining the invariants: `visited` is the real cycle guard, `maxDepth` is belt-and-suspenders. 32 comfortably exceeds real docx nesting depth so a deeply-nested but well-formed anchor is no longer silently rejected. - Comment added before the parent-walk loop noting why the API calls are serial (each level's parent_id is only known after the previous GET returns; can't be batched or parallelised). Tests: - TestLocateInsertIndexWarnsOnMultipleMatches: stubs two matches, asserts the stderr warning names the ambiguity and mentions 'start...end', and that the first-match insert index is unchanged. - TestLocateInsertIndexLogsNestedAnchor: anchor two levels below root, asserts stderr carries the "nested … top-level ancestor" note. - TestLocateInsertIndexCycleDetection: malformed parent chain with blk_x.parent = blk_y and blk_y.parent = blk_x, neither reachable from root. Registering a single GET /blocks/blk_y stub also bounds the call count — a regression that broke `visited` tracking would either hang or fail via httpmock's extra-call guard. Co-authored-by: fangshuyu-768 <shuyufang768@outlook.com>
2026-07-03 14:02:43 +08:00 · 2026-04-20 23:24:11 +08:00
parent 9e891b758e
commit 9057299430
2 changed files with 848 additions and 14 deletions
--- a/shortcuts/doc/doc_media_insert.go
+++ b/shortcuts/doc/doc_media_insert.go
@@ -7,6 +7,7 @@ import (
 	"context"
 	"fmt"
 	"path/filepath"
+	"strings"

 	"github.com/larksuite/cli/extension/fileio"
 	"github.com/larksuite/cli/internal/output"
@@ -35,7 +36,7 @@ var fileViewMap = map[string]int{
 var DocMediaInsert = common.Shortcut{
 	Service:     "docs",
 	Command:     "+media-insert",
-	Description: "Insert a local image or file at the end of a Lark document (4-step orchestration + auto-rollback)",
+	Description: "Insert a local image or file into a Lark document (4-step orchestration + auto-rollback); appends to end by default, or inserts relative to a text selection with --selection-with-ellipsis",
 	Risk:        "write",
 	Scopes:      []string{"docs:document.media:upload", "docx:document:write_only", "docx:document:readonly"},
 	AuthTypes:   []string{"user", "bot"},
@@ -45,6 +46,8 @@ var DocMediaInsert = common.Shortcut{
 		{Name: "type", Default: "image", Desc: "type: image | file"},
 		{Name: "align", Desc: "alignment: left | center | right"},
 		{Name: "caption", Desc: "image caption text"},
+		{Name: "selection-with-ellipsis", Desc: "plain text (or 'start...end' to disambiguate) matching the target block's content. Media is inserted at the top-level ancestor of the matched block — i.e., when the selection is inside a callout, table cell, or nested list, media lands outside that container, not inside it. Pass 'start...end' (a unique prefix and suffix separated by '...') when the plain text appears in more than one block"},
+		{Name: "before", Type: "bool", Desc: "insert before the matched block instead of after (requires --selection-with-ellipsis)"},
 		{Name: "file-view", Desc: "file block rendering: card (default) | preview | inline; only applies when --type=file. preview renders audio/video as an inline player"},
 	},
 	Validate: func(ctx context.Context, runtime *common.RuntimeContext) error {
@@ -55,6 +58,18 @@ var DocMediaInsert = common.Shortcut{
 		if docRef.Kind == "doc" {
 			return output.ErrValidation("docs +media-insert only supports docx documents; use a docx token/URL or a wiki URL that resolves to docx")
 		}
+		rawSelection := runtime.Str("selection-with-ellipsis")
+		trimmedSelection := strings.TrimSpace(rawSelection)
+		// Explicitly reject a flag that was supplied but blank: runtime.Str cannot
+		// distinguish "omitted" from "provided as empty/whitespace", and a silent
+		// trim-to-empty would make +media-insert fall back to append-mode and
+		// write at the wrong location.
+		if rawSelection != "" && trimmedSelection == "" {
+			return output.ErrValidation("--selection-with-ellipsis must not be blank or whitespace-only")
+		}
+		if runtime.Bool("before") && trimmedSelection == "" {
+			return output.ErrValidation("--before requires --selection-with-ellipsis")
+		}
 		if view := runtime.Str("file-view"); view != "" {
 			if _, ok := fileViewMap[view]; !ok {
 				return output.ErrValidation("invalid --file-view value %q, expected one of: card | preview | inline", view)
@@ -76,30 +91,71 @@ var DocMediaInsert = common.Shortcut{
 		filePath := runtime.Str("file")
 		mediaType := runtime.Str("type")
 		caption := runtime.Str("caption")
+		selection := strings.TrimSpace(runtime.Str("selection-with-ellipsis"))
+		hasSelection := selection != ""
 		fileViewType := fileViewMap[runtime.Str("file-view")]

 		parentType := parentTypeForMediaType(mediaType)
 		createBlockData := buildCreateBlockData(mediaType, 0, fileViewType)
-		createBlockData["index"] = "<children_len>"
+		if hasSelection {
+			createBlockData["index"] = "<locate_index>"
+		} else {
+			createBlockData["index"] = "<children_len>"
+		}
 		batchUpdateData := buildBatchUpdateData("<new_block_id>", mediaType, "<file_token>", runtime.Str("align"), caption)

 		d := common.NewDryRunAPI()
+		totalSteps := 4
+		if docRef.Kind == "wiki" {
+			totalSteps++
+		}
+		if hasSelection {
+			totalSteps++
+		}
+
+		positionLabel := map[bool]string{true: "before", false: "after"}[runtime.Bool("before")]
+
 		if docRef.Kind == "wiki" {
 			documentID = "<resolved_docx_token>"
 			stepBase = 2
-			d.Desc("5-step orchestration: resolve wiki → query root → create block → upload file → bind to block (auto-rollback on failure)").
+			d.Desc(fmt.Sprintf("%d-step orchestration: resolve wiki → query root →%s create block → upload file → bind to block (auto-rollback on failure)",
+				totalSteps, map[bool]string{true: " locate-doc →", false: ""}[hasSelection])).
 				GET("/open-apis/wiki/v2/spaces/get_node").
 				Desc("[1] Resolve wiki node to docx document").
 				Params(map[string]interface{}{"token": docRef.Token})
 		} else {
-			d.Desc("4-step orchestration: query root → create block → upload file → bind to block (auto-rollback on failure)")
+			d.Desc(fmt.Sprintf("%d-step orchestration: query root →%s create block → upload file → bind to block (auto-rollback on failure)",
+				totalSteps, map[bool]string{true: " locate-doc →", false: ""}[hasSelection]))
 		}

 		d.
 			GET("/open-apis/docx/v1/documents/:document_id/blocks/:document_id").
-			Desc(fmt.Sprintf("[%d] Get document root block", stepBase)).
+			Desc(fmt.Sprintf("[%d] Get document root block", stepBase))
+
+		if hasSelection {
+			mcpEndpoint := common.MCPEndpoint(runtime.Config.Brand)
+			mcpArgs := map[string]interface{}{
+				"doc_id":                  documentID,
+				"selection_with_ellipsis": selection,
+				"limit":                   1,
+			}
+			d.POST(mcpEndpoint).
+				Desc(fmt.Sprintf("[%d] MCP locate-doc: find block matching selection (%s)", stepBase+1, positionLabel)).
+				Body(map[string]interface{}{
+					"method": "tools/call",
+					"params": map[string]interface{}{
+						"name":      "locate-doc",
+						"arguments": mcpArgs,
+					},
+				}).
+				Set("mcp_tool", "locate-doc").
+				Set("args", mcpArgs)
+			stepBase++
+		}
+
+		d.
 			POST("/open-apis/docx/v1/documents/:document_id/blocks/:document_id/children").
-			Desc(fmt.Sprintf("[%d] Create empty block at document end", stepBase+1)).
+			Desc(fmt.Sprintf("[%d] Create empty block at target position", stepBase+1)).
 			Body(createBlockData)
 		appendDocMediaInsertUploadDryRun(d, runtime.FileIO(), filePath, parentType, stepBase+2)
 		d.PATCH("/open-apis/docx/v1/documents/:document_id/blocks/batch_update").
@@ -144,13 +200,31 @@ var DocMediaInsert = common.Shortcut{
 			return err
 		}

-		parentBlockID, insertIndex, err := extractAppendTarget(rootData, documentID)
+		parentBlockID, insertIndex, rootChildren, err := extractAppendTarget(rootData, documentID)
 		if err != nil {
 			return err
 		}
 		fmt.Fprintf(runtime.IO().ErrOut, "Root block ready: %s (%d children)\n", parentBlockID, insertIndex)

-		// Step 2: Create an empty block at the end of the document
+		selection := strings.TrimSpace(runtime.Str("selection-with-ellipsis"))
+		if selection != "" {
+			before := runtime.Bool("before")
+			// Redact the selection when logging — it is copied verbatim from
+			// document content and may contain confidential text.
+			fmt.Fprintf(runtime.IO().ErrOut, "Locating block matching selection (%s)\n", redactSelection(selection))
+			idx, err := locateInsertIndex(runtime, documentID, selection, rootChildren, before)
+			if err != nil {
+				return err
+			}
+			insertIndex = idx
+			posLabel := "after"
+			if before {
+				posLabel = "before"
+			}
+			fmt.Fprintf(runtime.IO().ErrOut, "locate-doc matched: inserting %s at index %d\n", posLabel, insertIndex)
+		}
+
+		// Step 2: Create an empty block at the target position
 		fmt.Fprintf(runtime.IO().ErrOut, "Creating block at index %d\n", insertIndex)

 		createData, err := runtime.CallAPI("POST",
@@ -224,6 +298,20 @@ func blockTypeForMediaType(mediaType string) int {
 	return 27
 }

+// redactSelection summarizes --selection-with-ellipsis values for logging and
+// error messages without echoing raw document text. Returns the rune count and,
+// for longer strings, a short prefix so operators can still identify which
+// selection failed without leaking confidential content into terminals or CI
+// logs.
+func redactSelection(s string) string {
+	const prefixRunes = 8
+	runes := []rune(s)
+	if len(runes) <= prefixRunes {
+		return fmt.Sprintf("%d chars", len(runes))
+	}
+	return fmt.Sprintf("%q… %d chars total", string(runes[:prefixRunes]), len(runes))
+}
+
 func parentTypeForMediaType(mediaType string) string {
 	if mediaType == "file" {
 		return "docx_file"
@@ -332,19 +420,150 @@ func buildBatchUpdateData(blockID, mediaType, fileToken, alignStr, caption strin
 	}
 }

-func extractAppendTarget(rootData map[string]interface{}, fallbackBlockID string) (string, int, error) {
+func extractAppendTarget(rootData map[string]interface{}, fallbackBlockID string) (parentBlockID string, insertIndex int, children []interface{}, err error) {
 	block, _ := rootData["block"].(map[string]interface{})
 	if len(block) == 0 {
-		return "", 0, output.Errorf(output.ExitAPI, "api_error", "failed to query document root block")
+		return "", 0, nil, output.Errorf(output.ExitAPI, "api_error", "failed to query document root block")
 	}

-	parentBlockID := fallbackBlockID
+	parentBlockID = fallbackBlockID
 	if blockID, _ := block["block_id"].(string); blockID != "" {
 		parentBlockID = blockID
 	}

-	children, _ := block["children"].([]interface{})
-	return parentBlockID, len(children), nil
+	children, _ = block["children"].([]interface{})
+	return parentBlockID, len(children), children, nil
+}
+
+// locateInsertIndex uses the MCP locate-doc tool to find the root-level index
+// at which to insert relative to the block matching selection. It walks the
+// parent_id chain (using single-block GET calls when needed) to resolve nested
+// blocks to their top-level ancestor in rootChildren.
+func locateInsertIndex(runtime *common.RuntimeContext, documentID string, selection string, rootChildren []interface{}, before bool) (int, error) {
+	// Ask for 2 matches so we can warn when the selection is ambiguous. locate-doc
+	// orders matches by document position, so matches[0] is still deterministic.
+	args := map[string]interface{}{
+		"doc_id":                  documentID,
+		"selection_with_ellipsis": selection,
+		"limit":                   2,
+	}
+	result, err := common.CallMCPTool(runtime, "locate-doc", args)
+	if err != nil {
+		return 0, err
+	}
+
+	matches := common.GetSlice(result, "matches")
+	if len(matches) == 0 {
+		return 0, output.ErrWithHint(
+			output.ExitValidation,
+			"no_match",
+			fmt.Sprintf("locate-doc did not find any block matching selection (%s)", redactSelection(selection)),
+			"check spelling or use 'start...end' syntax to narrow the selection",
+		)
+	}
+	if len(matches) > 1 {
+		// Silently picking the first match surprises users whose selection appears
+		// in more than one block (e.g. the same phrase in a title and a paragraph).
+		// Surface that another match exists and point at the 'start...end' disambiguator.
+		fmt.Fprintf(runtime.IO().ErrOut,
+			"warning: selection (%s) matched more than one block; inserting relative to the first. "+
+				"Pass --selection-with-ellipsis 'start...end' to narrow.\n",
+			redactSelection(selection))
+	}
+
+	matchMap, _ := matches[0].(map[string]interface{})
+	anchorBlockID := common.GetString(matchMap, "anchor_block_id")
+	if anchorBlockID == "" {
+		// Fall back to first block entry if anchor_block_id is absent.
+		blocks := common.GetSlice(matchMap, "blocks")
+		if len(blocks) > 0 {
+			if b, ok := blocks[0].(map[string]interface{}); ok {
+				anchorBlockID = common.GetString(b, "block_id")
+			}
+		}
+	}
+	if anchorBlockID == "" {
+		return 0, output.Errorf(output.ExitAPI, "api_error", "locate-doc response missing anchor_block_id")
+	}
+	parentBlockID := common.GetString(matchMap, "parent_block_id")
+
+	// Build root children set for O(1) lookup.
+	rootSet := make(map[string]int, len(rootChildren))
+	for i, c := range rootChildren {
+		if id, ok := c.(string); ok {
+			rootSet[id] = i
+		}
+	}
+
+	// Walk up the parent chain to the top-level ancestor in rootChildren. This
+	// is serial by nature: each level's parent_id is only known after the
+	// previous level's GET /blocks/{id} response arrives, so the calls cannot
+	// be batched or parallelised.
+	//
+	// visited is the real cycle guard — it stops an A→B→A parent-id loop (seen
+	// on malformed API responses) after one lap. maxDepth is belt-and-suspenders
+	// in case both visited tracking and parent_id sanity simultaneously break;
+	// 32 comfortably exceeds the deepest real docx nesting (~6–8 levels for
+	// quote/callout/list combinations) without letting a bug run unbounded.
+	cur := anchorBlockID
+	nextParent := parentBlockID
+	visited := map[string]bool{}
+	const maxDepth = 32
+	walkDepth := 0
+	for depth := 0; depth < maxDepth; depth++ {
+		if visited[cur] {
+			break
+		}
+		visited[cur] = true
+
+		if idx, ok := rootSet[cur]; ok {
+			if walkDepth > 0 {
+				// The anchor was nested inside a callout / table cell / list and
+				// got resolved to its top-level ancestor. Surface this so users
+				// don't misread "insert before 'X'" as "insert right next to X"
+				// when X is buried several levels deep.
+				posLabel := "after"
+				if before {
+					posLabel = "before"
+				}
+				fmt.Fprintf(runtime.IO().ErrOut,
+					"note: selection (%s) was nested %d level(s) deep; inserting %s its top-level ancestor at index %d\n",
+					redactSelection(selection), walkDepth, posLabel, idx)
+			}
+			if before {
+				return idx, nil
+			}
+			return idx + 1, nil
+		}
+
+		// Advance: use the parent hint we already have, or fetch from API.
+		parent := nextParent
+		nextParent = "" // clear hint after first use
+		if parent == "" || parent == cur {
+			// Need to fetch this block to find its parent.
+			data, err := runtime.CallAPI("GET",
+				fmt.Sprintf("/open-apis/docx/v1/documents/%s/blocks/%s",
+					validate.EncodePathSegment(documentID), validate.EncodePathSegment(cur)),
+				nil, nil)
+			if err != nil {
+				return 0, err
+			}
+			block := common.GetMap(data, "block")
+			parent = common.GetString(block, "parent_id")
+		}
+		if parent == "" || parent == cur {
+			break
+		}
+		cur = parent
+		walkDepth++
+	}
+
+	return 0, output.ErrWithHint(
+		output.ExitValidation,
+		"block_not_reachable",
+		fmt.Sprintf("block matching selection (%s) is not reachable from document root", redactSelection(selection)),
+		"try a top-level heading or paragraph as the selection",
+	)
 }

 func extractCreatedBlockTargets(createData map[string]interface{}, mediaType string) (blockID, uploadParentNode, replaceBlockID string) {
--- a/shortcuts/doc/doc_media_insert_test.go
+++ b/shortcuts/doc/doc_media_insert_test.go
@@ -5,12 +5,15 @@ package doc

 import (
 	"context"
+	"encoding/json"
 	"reflect"
 	"strings"
 	"testing"

 	"github.com/spf13/cobra"

+	"github.com/larksuite/cli/internal/cmdutil"
+	"github.com/larksuite/cli/internal/httpmock"
 	"github.com/larksuite/cli/shortcuts/common"
 )

@@ -222,7 +225,7 @@ func TestExtractAppendTargetUsesRootChildrenCount(t *testing.T) {
 		},
 	}

-	blockID, index, err := extractAppendTarget(rootData, "fallback")
+	blockID, index, children, err := extractAppendTarget(rootData, "fallback")
 	if err != nil {
 		t.Fatalf("extractAppendTarget() unexpected error: %v", err)
 	}
@@ -232,6 +235,365 @@ func TestExtractAppendTargetUsesRootChildrenCount(t *testing.T) {
 	if index != 3 {
 		t.Fatalf("extractAppendTarget() index = %d, want 3", index)
 	}
+	if len(children) != 3 {
+		t.Fatalf("extractAppendTarget() children len = %d, want 3", len(children))
+	}
+}
+
+// buildLocateDocMCPResponse builds a JSON-RPC 2.0 response for a locate-doc MCP call.
+func buildLocateDocMCPResponse(matches []map[string]interface{}) map[string]interface{} {
+	resultJSON, _ := json.Marshal(map[string]interface{}{"matches": matches})
+	return map[string]interface{}{
+		"jsonrpc": "2.0",
+		"id":      "test-id",
+		"result": map[string]interface{}{
+			"content": []interface{}{
+				map[string]interface{}{
+					"type": "text",
+					"text": string(resultJSON),
+				},
+			},
+		},
+	}
+}
+
+// registerInsertWithSelectionStubs wires the minimal stub set for the
+// --selection-with-ellipsis happy path. Returns the create-block stub so
+// callers can inspect the request body (e.g. to verify the computed index).
+func registerInsertWithSelectionStubs(reg interface {
+	Register(*httpmock.Stub)
+}, docID, anchorBlockID, parentBlockID string, rootChildren []interface{}) *httpmock.Stub {
+	// Root block
+	reg.Register(&httpmock.Stub{
+		Method: "GET",
+		URL:    "/open-apis/docx/v1/documents/" + docID + "/blocks/" + docID,
+		Body: map[string]interface{}{
+			"code": 0, "msg": "ok",
+			"data": map[string]interface{}{
+				"block": map[string]interface{}{
+					"block_id": docID,
+					"children": rootChildren,
+				},
+			},
+		},
+	})
+	// MCP locate-doc
+	reg.Register(&httpmock.Stub{
+		Method: "POST",
+		URL:    "mcp.feishu.cn/mcp",
+		Body: buildLocateDocMCPResponse([]map[string]interface{}{
+			{"anchor_block_id": anchorBlockID, "parent_block_id": parentBlockID},
+		}),
+	})
+	// Create block — returned so the test can inspect index in CapturedBody.
+	createStub := &httpmock.Stub{
+		Method: "POST",
+		URL:    "/open-apis/docx/v1/documents/" + docID + "/blocks/" + docID + "/children",
+		Body: map[string]interface{}{
+			"code": 0, "msg": "ok",
+			"data": map[string]interface{}{
+				"children": []interface{}{
+					map[string]interface{}{"block_id": "blk_new", "block_type": 27, "image": map[string]interface{}{}},
+				},
+			},
+		},
+	}
+	reg.Register(createStub)
+	// Upload
+	reg.Register(&httpmock.Stub{
+		Method: "POST",
+		URL:    "/open-apis/drive/v1/medias/upload_all",
+		Body: map[string]interface{}{
+			"code": 0, "msg": "ok",
+			"data": map[string]interface{}{"file_token": "ftok_test"},
+		},
+	})
+	// Batch update
+	reg.Register(&httpmock.Stub{
+		Method: "PATCH",
+		URL:    "/open-apis/docx/v1/documents/" + docID + "/blocks/batch_update",
+		Body:   map[string]interface{}{"code": 0, "msg": "ok", "data": map[string]interface{}{}},
+	})
+	return createStub
+}
+
+// assertCreateBlockIndex decodes the create-block request body and asserts the
+// `index` field equals want. Fails the test if the body is missing or wrong.
+func assertCreateBlockIndex(t *testing.T, stub *httpmock.Stub, want int) {
+	t.Helper()
+	if stub.CapturedBody == nil {
+		t.Fatalf("create-block stub captured no body")
+	}
+	var body map[string]interface{}
+	if err := json.Unmarshal(stub.CapturedBody, &body); err != nil {
+		t.Fatalf("decode create-block body: %v (raw: %s)", err, stub.CapturedBody)
+	}
+	got, _ := body["index"].(float64)
+	if int(got) != want {
+		t.Fatalf("create-block index = %v, want %d (body: %s)", body["index"], want, stub.CapturedBody)
+	}
+}
+
+// TestLocateInsertIndexAfterModeViaExecute verifies that
+// --selection-with-ellipsis (default after-mode) places the new block
+// immediately after the matched root-level block. Uses three root children so
+// the after-index (2) differs from what --before would produce (1), and
+// inspects the create-block request body to prove the computed index actually
+// reaches the /children API.
+func TestLocateInsertIndexAfterModeViaExecute(t *testing.T) {
+	f, _, _, reg := cmdutil.TestFactory(t, docsTestConfigWithAppID("locate-after-app"))
+	createStub := registerInsertWithSelectionStubs(reg, "doxcnSEL", "blk_b", "doxcnSEL",
+		[]interface{}{"blk_a", "blk_b", "blk_c"})
+
+	tmpDir := t.TempDir()
+	withDocsWorkingDir(t, tmpDir)
+	writeSizedDocTestFile(t, "img.png", 100)
+
+	err := mountAndRunDocs(t, DocMediaInsert, []string{
+		"+media-insert",
+		"--doc", "doxcnSEL",
+		"--file", "img.png",
+		"--selection-with-ellipsis", "Introduction",
+		"--as", "bot",
+	}, f, nil)
+	if err != nil {
+		t.Fatalf("Execute() error: %v", err)
+	}
+	// after blk_b (index 1) → insert at index 2, between blk_b and blk_c.
+	assertCreateBlockIndex(t, createStub, 2)
+}
+
+// TestLocateInsertIndexBeforeModeViaExecute verifies that --before inserts
+// before the matched root-level block. Pairs with the after-mode test above:
+// same fixture, same anchor, but --before should flip the index from 2 to 1.
+// A regression that ignored --before would still pass the success check alone,
+// so we assert the create-block body explicitly.
+func TestLocateInsertIndexBeforeModeViaExecute(t *testing.T) {
+	f, _, _, reg := cmdutil.TestFactory(t, docsTestConfigWithAppID("locate-before-app"))
+	createStub := registerInsertWithSelectionStubs(reg, "doxcnSEL2", "blk_b", "doxcnSEL2",
+		[]interface{}{"blk_a", "blk_b", "blk_c"})
+
+	tmpDir := t.TempDir()
+	withDocsWorkingDir(t, tmpDir)
+	writeSizedDocTestFile(t, "img.png", 100)
+
+	err := mountAndRunDocs(t, DocMediaInsert, []string{
+		"+media-insert",
+		"--doc", "doxcnSEL2",
+		"--file", "img.png",
+		"--selection-with-ellipsis", "Architecture",
+		"--before",
+		"--as", "bot",
+	}, f, nil)
+	if err != nil {
+		t.Fatalf("Execute() error: %v", err)
+	}
+	// before blk_b (index 1) → insert at index 1, between blk_a and blk_b.
+	assertCreateBlockIndex(t, createStub, 1)
+}
+
+// TestLocateInsertIndexNestedBlockViaExecute verifies that a deeply-nested
+// anchor (2+ levels below root) walks up through an intermediate block via
+// the GET /blocks/{id} API to find the root-level ancestor. This exercises
+// the fallback ancestor-walk path in locateInsertIndex — the parent_block_id
+// hint from locate-doc is only good for one level, so deeper nesting must hit
+// the block-fetch loop.
+func TestLocateInsertIndexNestedBlockViaExecute(t *testing.T) {
+	f, _, _, reg := cmdutil.TestFactory(t, docsTestConfigWithAppID("locate-nested-app"))
+
+	docID := "doxcnNESTED"
+	// Root children: blk_section (index 0), blk_other (index 1).
+	// Anchor blk_grandchild is nested two levels deep:
+	//   root → blk_section → blk_section_child → blk_grandchild
+	// locate-doc gives us parent_block_id = blk_section_child (one level up);
+	// the walk must fetch blk_section_child to discover its parent = blk_section
+	// before it can land on a root child.
+	reg.Register(&httpmock.Stub{
+		Method: "GET",
+		URL:    "/open-apis/docx/v1/documents/" + docID + "/blocks/" + docID,
+		Body: map[string]interface{}{
+			"code": 0, "msg": "ok",
+			"data": map[string]interface{}{
+				"block": map[string]interface{}{
+					"block_id": docID,
+					"children": []interface{}{"blk_section", "blk_other"},
+				},
+			},
+		},
+	})
+	reg.Register(&httpmock.Stub{
+		Method: "POST",
+		URL:    "mcp.feishu.cn/mcp",
+		Body: buildLocateDocMCPResponse([]map[string]interface{}{
+			{"anchor_block_id": "blk_grandchild", "parent_block_id": "blk_section_child"},
+		}),
+	})
+	// Intermediate block lookup — this is the key step that exercises the
+	// fallback walk. Without this stub the test would fail.
+	intermediateStub := &httpmock.Stub{
+		Method: "GET",
+		URL:    "/open-apis/docx/v1/documents/" + docID + "/blocks/blk_section_child",
+		Body: map[string]interface{}{
+			"code": 0, "msg": "ok",
+			"data": map[string]interface{}{
+				"block": map[string]interface{}{
+					"block_id":  "blk_section_child",
+					"parent_id": "blk_section",
+				},
+			},
+		},
+	}
+	reg.Register(intermediateStub)
+	createStub := &httpmock.Stub{
+		Method: "POST",
+		URL:    "/open-apis/docx/v1/documents/" + docID + "/blocks/" + docID + "/children",
+		Body: map[string]interface{}{
+			"code": 0, "msg": "ok",
+			"data": map[string]interface{}{
+				"children": []interface{}{
+					map[string]interface{}{"block_id": "blk_new", "block_type": 27, "image": map[string]interface{}{}},
+				},
+			},
+		},
+	}
+	reg.Register(createStub)
+	reg.Register(&httpmock.Stub{
+		Method: "POST",
+		URL:    "/open-apis/drive/v1/medias/upload_all",
+		Body: map[string]interface{}{
+			"code": 0, "msg": "ok",
+			"data": map[string]interface{}{"file_token": "ftok_nested"},
+		},
+	})
+	reg.Register(&httpmock.Stub{
+		Method: "PATCH",
+		URL:    "/open-apis/docx/v1/documents/" + docID + "/blocks/batch_update",
+		Body:   map[string]interface{}{"code": 0, "msg": "ok", "data": map[string]interface{}{}},
+	})
+
+	tmpDir := t.TempDir()
+	withDocsWorkingDir(t, tmpDir)
+	writeSizedDocTestFile(t, "img.png", 100)
+
+	err := mountAndRunDocs(t, DocMediaInsert, []string{
+		"+media-insert",
+		"--doc", docID,
+		"--file", "img.png",
+		"--selection-with-ellipsis", "nested content",
+		"--as", "bot",
+	}, f, nil)
+	if err != nil {
+		t.Fatalf("Execute() error: %v", err)
+	}
+	// Confirm the ancestor-walk actually fired — without this assertion a
+	// regression that short-circuited the walk would still pass.
+	if intermediateStub.CapturedBody == nil && intermediateStub.CapturedHeaders == nil {
+		t.Errorf("expected GET /blocks/blk_section_child to be invoked by the parent-walk; stub was not hit")
+	}
+	// after blk_section (index 0) → insert at index 1, between blk_section and blk_other.
+	assertCreateBlockIndex(t, createStub, 1)
+}
+
+// TestLocateInsertIndexNoMatchReturnsError verifies that when locate-doc returns
+// no matches, Execute returns a descriptive error.
+func TestLocateInsertIndexNoMatchReturnsError(t *testing.T) {
+	f, _, _, reg := cmdutil.TestFactory(t, docsTestConfigWithAppID("locate-nomatch-app"))
+
+	docID := "doxcnNOMATCH"
+	reg.Register(&httpmock.Stub{
+		Method: "GET",
+		URL:    "/open-apis/docx/v1/documents/" + docID + "/blocks/" + docID,
+		Body: map[string]interface{}{
+			"code": 0, "msg": "ok",
+			"data": map[string]interface{}{
+				"block": map[string]interface{}{
+					"block_id": docID,
+					"children": []interface{}{"blk_a"},
+				},
+			},
+		},
+	})
+	reg.Register(&httpmock.Stub{
+		Method: "POST",
+		URL:    "mcp.feishu.cn/mcp",
+		Body:   buildLocateDocMCPResponse([]map[string]interface{}{}),
+	})
+
+	tmpDir := t.TempDir()
+	withDocsWorkingDir(t, tmpDir)
+	writeSizedDocTestFile(t, "img.png", 100)
+
+	err := mountAndRunDocs(t, DocMediaInsert, []string{
+		"+media-insert",
+		"--doc", docID,
+		"--file", "img.png",
+		"--selection-with-ellipsis", "nonexistent text",
+		"--as", "bot",
+	}, f, nil)
+	if err == nil {
+		t.Fatal("expected no-match error, got nil")
+	}
+	if !strings.Contains(err.Error(), "no_match") && !strings.Contains(err.Error(), "did not find") {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
+// TestLocateInsertIndexDryRunIncludesMCPStep verifies that the dry-run output
+// includes a locate-doc MCP step when --selection-with-ellipsis is provided.
+func TestLocateInsertIndexDryRunIncludesMCPStep(t *testing.T) {
+	t.Parallel()
+
+	cmd := &cobra.Command{Use: "docs +media-insert"}
+	cmd.Flags().String("file", "", "")
+	cmd.Flags().String("doc", "", "")
+	cmd.Flags().String("type", "image", "")
+	cmd.Flags().String("align", "", "")
+	cmd.Flags().String("caption", "", "")
+	cmd.Flags().String("selection-with-ellipsis", "", "")
+	cmd.Flags().Bool("before", false, "")
+	_ = cmd.Flags().Set("file", "img.png")
+	_ = cmd.Flags().Set("doc", "doxcnABCDEF")
+	_ = cmd.Flags().Set("selection-with-ellipsis", "Introduction")
+
+	rt := common.TestNewRuntimeContext(cmd, docsTestConfigWithAppID("dry-run-app"))
+	dryAPI := DocMediaInsert.DryRun(context.Background(), rt)
+	raw, _ := json.Marshal(dryAPI)
+
+	var dry struct {
+		Description string `json:"description"`
+		API         []struct {
+			Desc string                 `json:"desc"`
+			URL  string                 `json:"url"`
+			Body map[string]interface{} `json:"body"`
+		} `json:"api"`
+	}
+	if err := json.Unmarshal(raw, &dry); err != nil {
+		t.Fatalf("decode dry-run: %v", err)
+	}
+
+	foundMCP := false
+	for _, step := range dry.API {
+		if strings.Contains(step.Desc, "locate-doc") {
+			foundMCP = true
+		}
+	}
+	if !foundMCP {
+		t.Fatalf("dry-run should include a locate-doc step, got: %+v", dry.API)
+	}
+	if !strings.Contains(dry.Description, "locate-doc") {
+		t.Fatalf("dry-run description should mention 'locate-doc', got: %s", dry.Description)
+	}
+
+	// Verify create-block step shows <locate_index> not <children_len>
+	for _, step := range dry.API {
+		if strings.Contains(step.URL, "/children") && step.Body != nil {
+			if idx, ok := step.Body["index"]; ok {
+				if idx != "<locate_index>" {
+					t.Fatalf("create-block index in selection mode = %q, want <locate_index>", idx)
+				}
+			}
+		}
+	}
 }

 func TestExtractCreatedBlockTargetsForImage(t *testing.T) {
@@ -369,3 +731,256 @@ func TestDocMediaInsertValidateFileView(t *testing.T) {
 		})
 	}
 }
+
+// TestLocateInsertIndexWarnsOnMultipleMatches verifies that when locate-doc
+// returns more than one match, a warning is written to stderr pointing the user
+// at the 'start...end' disambiguation syntax. Silently picking the first match
+// of an ambiguous selection is a real UX trap — users who edit documents with
+// repeated phrases (a heading that also appears in the TOC, for example) get
+// no signal that another match existed.
+func TestLocateInsertIndexWarnsOnMultipleMatches(t *testing.T) {
+	f, _, stderr, reg := cmdutil.TestFactory(t, docsTestConfigWithAppID("locate-multi-app"))
+
+	docID := "doxcnMULTI"
+	reg.Register(&httpmock.Stub{
+		Method: "GET",
+		URL:    "/open-apis/docx/v1/documents/" + docID + "/blocks/" + docID,
+		Body: map[string]interface{}{
+			"code": 0, "msg": "ok",
+			"data": map[string]interface{}{
+				"block": map[string]interface{}{
+					"block_id": docID,
+					"children": []interface{}{"blk_a", "blk_b"},
+				},
+			},
+		},
+	})
+	// Two matches — same selection appears in two different root-level blocks.
+	// locate-doc orders matches by document position, so matches[0] is still
+	// deterministic (blk_a) even with limit=2.
+	reg.Register(&httpmock.Stub{
+		Method: "POST",
+		URL:    "mcp.feishu.cn/mcp",
+		Body: buildLocateDocMCPResponse([]map[string]interface{}{
+			{"anchor_block_id": "blk_a", "parent_block_id": docID},
+			{"anchor_block_id": "blk_b", "parent_block_id": docID},
+		}),
+	})
+	createStub := &httpmock.Stub{
+		Method: "POST",
+		URL:    "/open-apis/docx/v1/documents/" + docID + "/blocks/" + docID + "/children",
+		Body: map[string]interface{}{
+			"code": 0, "msg": "ok",
+			"data": map[string]interface{}{
+				"children": []interface{}{
+					map[string]interface{}{"block_id": "blk_new", "block_type": 27, "image": map[string]interface{}{}},
+				},
+			},
+		},
+	}
+	reg.Register(createStub)
+	reg.Register(&httpmock.Stub{
+		Method: "POST",
+		URL:    "/open-apis/drive/v1/medias/upload_all",
+		Body: map[string]interface{}{
+			"code": 0, "msg": "ok",
+			"data": map[string]interface{}{"file_token": "ftok_multi"},
+		},
+	})
+	reg.Register(&httpmock.Stub{
+		Method: "PATCH",
+		URL:    "/open-apis/docx/v1/documents/" + docID + "/blocks/batch_update",
+		Body:   map[string]interface{}{"code": 0, "msg": "ok", "data": map[string]interface{}{}},
+	})
+
+	tmpDir := t.TempDir()
+	withDocsWorkingDir(t, tmpDir)
+	writeSizedDocTestFile(t, "img.png", 100)
+
+	err := mountAndRunDocs(t, DocMediaInsert, []string{
+		"+media-insert",
+		"--doc", docID,
+		"--file", "img.png",
+		"--selection-with-ellipsis", "Repeated phrase",
+		"--as", "bot",
+	}, f, nil)
+	if err != nil {
+		t.Fatalf("Execute() error: %v", err)
+	}
+
+	// Warning should name the ambiguity and point at 'start...end'.
+	stderrOut := stderr.String()
+	if !strings.Contains(stderrOut, "matched more than one block") {
+		t.Errorf("stderr missing multi-match warning; got:\n%s", stderrOut)
+	}
+	if !strings.Contains(stderrOut, "start...end") {
+		t.Errorf("stderr missing 'start...end' disambiguation hint; got:\n%s", stderrOut)
+	}
+	// Should still insert at the first match (blk_a at index 0) → after ⇒ 1.
+	assertCreateBlockIndex(t, createStub, 1)
+}
+
+// TestLocateInsertIndexLogsNestedAnchor verifies that when the matched block is
+// nested (not a direct root child), a note is written to stderr explaining that
+// the media lands at the top-level ancestor. This protects users from being
+// surprised when selecting text inside a callout or table cell and seeing the
+// image appear outside that container.
+func TestLocateInsertIndexLogsNestedAnchor(t *testing.T) {
+	f, _, stderr, reg := cmdutil.TestFactory(t, docsTestConfigWithAppID("locate-nested-log-app"))
+
+	docID := "doxcnNESTEDLOG"
+	// Same shape as TestLocateInsertIndexNestedBlockViaExecute: anchor is two
+	// levels below root, so walkDepth == 2 when we hit the root ancestor.
+	reg.Register(&httpmock.Stub{
+		Method: "GET",
+		URL:    "/open-apis/docx/v1/documents/" + docID + "/blocks/" + docID,
+		Body: map[string]interface{}{
+			"code": 0, "msg": "ok",
+			"data": map[string]interface{}{
+				"block": map[string]interface{}{
+					"block_id": docID,
+					"children": []interface{}{"blk_section", "blk_other"},
+				},
+			},
+		},
+	})
+	reg.Register(&httpmock.Stub{
+		Method: "POST",
+		URL:    "mcp.feishu.cn/mcp",
+		Body: buildLocateDocMCPResponse([]map[string]interface{}{
+			{"anchor_block_id": "blk_grandchild", "parent_block_id": "blk_section_child"},
+		}),
+	})
+	reg.Register(&httpmock.Stub{
+		Method: "GET",
+		URL:    "/open-apis/docx/v1/documents/" + docID + "/blocks/blk_section_child",
+		Body: map[string]interface{}{
+			"code": 0, "msg": "ok",
+			"data": map[string]interface{}{
+				"block": map[string]interface{}{
+					"block_id":  "blk_section_child",
+					"parent_id": "blk_section",
+				},
+			},
+		},
+	})
+	reg.Register(&httpmock.Stub{
+		Method: "POST",
+		URL:    "/open-apis/docx/v1/documents/" + docID + "/blocks/" + docID + "/children",
+		Body: map[string]interface{}{
+			"code": 0, "msg": "ok",
+			"data": map[string]interface{}{
+				"children": []interface{}{
+					map[string]interface{}{"block_id": "blk_new", "block_type": 27, "image": map[string]interface{}{}},
+				},
+			},
+		},
+	})
+	reg.Register(&httpmock.Stub{
+		Method: "POST",
+		URL:    "/open-apis/drive/v1/medias/upload_all",
+		Body: map[string]interface{}{
+			"code": 0, "msg": "ok",
+			"data": map[string]interface{}{"file_token": "ftok_nested_log"},
+		},
+	})
+	reg.Register(&httpmock.Stub{
+		Method: "PATCH",
+		URL:    "/open-apis/docx/v1/documents/" + docID + "/blocks/batch_update",
+		Body:   map[string]interface{}{"code": 0, "msg": "ok", "data": map[string]interface{}{}},
+	})
+
+	tmpDir := t.TempDir()
+	withDocsWorkingDir(t, tmpDir)
+	writeSizedDocTestFile(t, "img.png", 100)
+
+	err := mountAndRunDocs(t, DocMediaInsert, []string{
+		"+media-insert",
+		"--doc", docID,
+		"--file", "img.png",
+		"--selection-with-ellipsis", "nested content",
+		"--as", "bot",
+	}, f, nil)
+	if err != nil {
+		t.Fatalf("Execute() error: %v", err)
+	}
+
+	stderrOut := stderr.String()
+	if !strings.Contains(stderrOut, "nested") || !strings.Contains(stderrOut, "top-level ancestor") {
+		t.Errorf("stderr missing nested-anchor note; got:\n%s", stderrOut)
+	}
+}
+
+// TestLocateInsertIndexCycleDetection verifies that a malformed parent chain
+// (blk_x.parent = blk_y and blk_y.parent = blk_x, neither reachable from root)
+// does not spin the locate-doc walk forever. The `visited` map must break the
+// cycle, and the user must see the "not reachable from document root" error
+// rather than the process hanging. Without this test, a regression that broke
+// cycle protection would only surface in production with a stalled CLI.
+func TestLocateInsertIndexCycleDetection(t *testing.T) {
+	f, _, _, reg := cmdutil.TestFactory(t, docsTestConfigWithAppID("locate-cycle-app"))
+
+	docID := "doxcnCYCLE"
+	// Root has unrelated children — neither blk_x nor blk_y reach root.
+	reg.Register(&httpmock.Stub{
+		Method: "GET",
+		URL:    "/open-apis/docx/v1/documents/" + docID + "/blocks/" + docID,
+		Body: map[string]interface{}{
+			"code": 0, "msg": "ok",
+			"data": map[string]interface{}{
+				"block": map[string]interface{}{
+					"block_id": docID,
+					"children": []interface{}{"blk_unrelated_a", "blk_unrelated_b"},
+				},
+			},
+		},
+	})
+	// locate-doc hints parent_block_id = blk_y for anchor blk_x (first hop consumed).
+	reg.Register(&httpmock.Stub{
+		Method: "POST",
+		URL:    "mcp.feishu.cn/mcp",
+		Body: buildLocateDocMCPResponse([]map[string]interface{}{
+			{"anchor_block_id": "blk_x", "parent_block_id": "blk_y"},
+		}),
+	})
+	// blk_y claims blk_x as parent — closes the cycle. The walk must land here
+	// exactly once before visited[blk_x] triggers a break.
+	blkYStub := &httpmock.Stub{
+		Method: "GET",
+		URL:    "/open-apis/docx/v1/documents/" + docID + "/blocks/blk_y",
+		Body: map[string]interface{}{
+			"code": 0, "msg": "ok",
+			"data": map[string]interface{}{
+				"block": map[string]interface{}{
+					"block_id":  "blk_y",
+					"parent_id": "blk_x",
+				},
+			},
+		},
+	}
+	reg.Register(blkYStub)
+
+	tmpDir := t.TempDir()
+	withDocsWorkingDir(t, tmpDir)
+	writeSizedDocTestFile(t, "img.png", 100)
+
+	err := mountAndRunDocs(t, DocMediaInsert, []string{
+		"+media-insert",
+		"--doc", docID,
+		"--file", "img.png",
+		"--selection-with-ellipsis", "cyclic anchor",
+		"--as", "bot",
+	}, f, nil)
+	if err == nil {
+		t.Fatal("expected 'block_not_reachable' error from cyclic parent chain; got nil")
+	}
+	if !strings.Contains(err.Error(), "not reachable") && !strings.Contains(err.Error(), "block_not_reachable") {
+		t.Fatalf("unexpected error — want cycle-bounded 'not reachable', got: %v", err)
+	}
+	// blk_y should be fetched exactly once. Registering just one stub for it
+	// already enforces an upper bound (httpmock errors on extra calls), so if
+	// the walk looped more than once the test harness would fail differently.
+	if blkYStub.CapturedHeaders == nil && blkYStub.CapturedBody == nil {
+		t.Errorf("expected the walk to fetch blk_y once; stub was not hit")
+	}
+}