fix(mcp): harden unindexed rename recovery

internal/indexer/extract_source.go

@@ -14,10 +14,12 @@ import (
 // interactive recovery must not turn one MCP request into an unbounded parse.
 const maxAdHocExtractSourceBytes = 1 << 20 // 1 MiB
 
-// ExtractSource parses caller-owned bytes through the indexer's normal
-// admission, minified-content, timeout, panic-recovery, and optional
-// crash-isolation path without mutating graph state. The caller owns the
-// returned result and must call ReleaseTree when it is no longer needed.
+// ExtractSource parses caller-owned bytes through bounded admission,
+// minified-content, timeout, panic-recovery, and optional crash isolation
+// without mutating graph state. Preparation must preserve raw byte and line
+// coordinates; configured command transforms are refused because they may
+// synthesize declarations that cannot safely anchor an edit of the raw file.
+// The caller owns the result and must call ReleaseTree when finished.
 func (idx *Indexer) ExtractSource(ctx context.Context, filePath string, src []byte) (*parser.ExtractionResult, error) {
 	if idx == nil || idx.registry == nil {
 		return nil, fmt.Errorf("indexer source extraction is unavailable")
@@ -36,15 +38,26 @@ func (idx *Indexer) ExtractSource(ctx context.Context, filePath string, src []by
 		return nil, fmt.Errorf("source exceeds bounded extraction limit (%d bytes)", limit)
 	}
 
-	language, ok := idx.registry.DetectLanguageContent(filePath, src)
+	path := filePath
+	if !filepath.IsAbs(path) && idx.rootPath != "" {
+		path = filepath.Join(idx.rootPath, filepath.FromSlash(filePath))
+	}
+	prepared, err := idx.transforms.prepareCoordinateStable(path, src)
+	if err != nil {
+		return nil, fmt.Errorf("coordinate-stable source preparation: %w", err)
+	}
+	language, ok := idx.effectiveLanguage(path, prepared)
 	if !ok {
 		return nil, fmt.Errorf("unsupported source language for %s", filePath)
 	}
 	extractor, ok := idx.registry.GetByLanguage(language)
 	if !ok {
 		return nil, fmt.Errorf("no extractor registered for %s", language)
 	}
-	prepared := parser.ApplyPreParse(extractor, src)
+	prepared = parser.ApplyPreParse(extractor, prepared)
+	if !sameSourceCoordinates(src, prepared) {
+		return nil, fmt.Errorf("extractor pre-parse rewrite does not preserve source coordinates")
+	}
 	if int64(len(prepared)) > limit {
 		return nil, fmt.Errorf("prepared source exceeds bounded extraction limit (%d bytes)", limit)
 	}
@@ -55,12 +68,16 @@ func (idx *Indexer) ExtractSource(ctx context.Context, filePath string, src []by
 		pool, quarantine = idx.sharedParsePool()
 	}
 	nativeAdmission := newNativeParseExtractionAdmission(0, nil, idx.nativeParseAdmission.Load())
-	path := filePath
-	if !filepath.IsAbs(path) && idx.rootPath != "" {
-		path = filepath.Join(idx.rootPath, filepath.FromSlash(filePath))
+	rawLease, err := acquireParseAdmission(
+		ctx, int64(len(prepared)), 0, nil, idx.parseAdmission.Load(),
+	)
+	if err != nil {
+		return nil, fmt.Errorf("source admission: %w", err)
 	}
-	result, skipped, err := idx.extractFileCtx(
-		ctx, nativeAdmission, pool, quarantine,
+	defer rawLease.Release()
+
+	result, skipped, err := idx.extractFileCtxWithRawLease(
+		ctx, nativeAdmission, rawLease, pool, quarantine,
 		path, filePath, language, extractor, prepared,
 	)
 	if err != nil {

internal/indexer/extract_source_test.go

@@ -0,0 +1,118 @@
+package indexer
+
+import (
+	"context"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
+
+	"github.com/zzet/gortex/internal/config"
+	"github.com/zzet/gortex/internal/graph"
+	"github.com/zzet/gortex/internal/parser"
+	"github.com/zzet/gortex/internal/parser/languages"
+)
+
+func newExtractSourceTestIndexer(t *testing.T, cfg config.IndexConfig) *Indexer {
+	t.Helper()
+	registry := parser.NewRegistry()
+	registry.Register(languages.NewGoExtractor())
+	idx := New(graph.New(), registry, cfg, zap.NewNop())
+	t.Cleanup(idx.Close)
+	return idx
+}
+
+func TestExtractSourcePreservesRawCoordinates(t *testing.T) {
+	cfg := config.Default()
+	idx := newExtractSourceTestIndexer(t, cfg.Index)
+	source := append([]byte{0xef, 0xbb, 0xbf}, []byte("package main\n\nfunc Late() {}\n")...)
+
+	result, err := idx.ExtractSource(t.Context(), "late.go", source)
+	require.NoError(t, err)
+	defer result.ReleaseTree()
+
+	for _, node := range result.Nodes {
+		if strings.HasSuffix(node.ID, "::Late") {
+			require.Equal(t, 3, node.StartLine)
+			return
+		}
+	}
+	t.Fatal("Late declaration was not extracted")
+}
+
+func TestExtractSourceRefusesCommandTransforms(t *testing.T) {
+	cfg := config.Default()
+	cfg.Index.Transforms = []config.TransformRule{{
+		Name:       "must-not-run",
+		Extensions: []string{".go"},
+		Command:    []string{"gortex-test-transform-must-not-run"},
+	}}
+	idx := newExtractSourceTestIndexer(t, cfg.Index)
+
+	result, err := idx.ExtractSource(t.Context(), "late.go", []byte("package main\n\nfunc Late() {}\n"))
+	require.Nil(t, result)
+	require.ErrorContains(t, err, "does not guarantee coordinate preservation")
+}
+
+type unstablePreParserExtractor struct{}
+
+func (unstablePreParserExtractor) Language() string     { return "unstable" }
+func (unstablePreParserExtractor) Extensions() []string { return []string{".unstable"} }
+func (unstablePreParserExtractor) Extract(string, []byte) (*parser.ExtractionResult, error) {
+	return &parser.ExtractionResult{}, nil
+}
+func (unstablePreParserExtractor) PreParse(src []byte) []byte {
+	return append(append([]byte(nil), src...), 'x')
+}
+
+func TestExtractSourceRefusesUnstablePreParser(t *testing.T) {
+	registry := parser.NewRegistry()
+	registry.Register(unstablePreParserExtractor{})
+	cfg := config.Default()
+	idx := New(graph.New(), registry, cfg.Index, zap.NewNop())
+	t.Cleanup(idx.Close)
+
+	result, err := idx.ExtractSource(t.Context(), "late.unstable", []byte("source\n"))
+	require.Nil(t, result)
+	require.ErrorContains(t, err, "pre-parse rewrite does not preserve source coordinates")
+}
+
+func TestExtractSourceCancellationReleasesRawAdmissionWaiter(t *testing.T) {
+	cfg := config.Default()
+	idx := newExtractSourceTestIndexer(t, cfg.Index)
+	budget := newParseAdmissionBudget(1)
+	idx.parseAdmission.Store(budget)
+
+	held, err := acquireParseAdmission(t.Context(), 1, 0, nil, budget)
+	require.NoError(t, err)
+	ctx, cancel := context.WithCancel(t.Context())
+	errCh := make(chan error, 1)
+	go func() {
+		result, extractErr := idx.ExtractSource(ctx, "late.go", []byte("package main\n\nfunc Late() {}\n"))
+		if result != nil {
+			result.ReleaseTree()
+		}
+		errCh <- extractErr
+	}()
+
+	require.Eventually(t, func() bool {
+		budget.mu.Lock()
+		defer budget.mu.Unlock()
+		return len(budget.waiters) == 1
+	}, time.Second, time.Millisecond)
+	cancel()
+
+	select {
+	case extractErr := <-errCh:
+		require.ErrorIs(t, extractErr, context.Canceled)
+	case <-time.After(2 * time.Second):
+		t.Fatal("ExtractSource did not observe admission cancellation")
+	}
+	held.Release()
+	budget.mu.Lock()
+	defer budget.mu.Unlock()
+	require.Zero(t, budget.used)
+	require.Empty(t, budget.waiters)
+}

internal/indexer/transform.go

@@ -119,6 +119,71 @@ func (p *transformPipeline) run(path string, src []byte) []byte {
 	return out
 }
 
+// prepareCoordinateStable applies only source preparation whose output keeps
+// a one-to-one byte and line mapping to the caller-owned bytes. Unindexed
+// rename recovery edits the raw file, so an arbitrary command transform may
+// not synthesize or relocate a declaration and still be advertised as a
+// declaration-only fallback.
+func (p *transformPipeline) prepareCoordinateStable(path string, src []byte) ([]byte, error) {
+	if p == nil {
+		return src, nil
+	}
+	out := src
+	for _, transform := range p.prePass {
+		if !transform.matches(path) {
+			continue
+		}
+		rewritten := transform.rewrite(path, out)
+		if !sameSourceCoordinates(out, rewritten) {
+			return nil, fmt.Errorf("pre-parse transform %q does not preserve source coordinates", transform.name())
+		}
+		out = rewritten
+	}
+	for _, transform := range p.transforms {
+		if !transform.matches(path) {
+			continue
+		}
+		switch transform.(type) {
+		case bomStripTransform:
+			var err error
+			out, err = neutralizeSourceBOM(out)
+			if err != nil {
+				return nil, err
+			}
+		default:
+			return nil, fmt.Errorf("source transform %q does not guarantee coordinate preservation", transform.name())
+		}
+	}
+	return out, nil
+}
+
+func sameSourceCoordinates(raw, prepared []byte) bool {
+	if len(raw) != len(prepared) {
+		return false
+	}
+	for i := range raw {
+		rawBreak := raw[i] == '\n' || raw[i] == '\r'
+		preparedBreak := prepared[i] == '\n' || prepared[i] == '\r'
+		if (rawBreak || preparedBreak) && raw[i] != prepared[i] {
+			return false
+		}
+	}
+	return true
+}
+
+func neutralizeSourceBOM(src []byte) ([]byte, error) {
+	switch {
+	case bytes.HasPrefix(src, []byte{0xef, 0xbb, 0xbf}):
+		out := bytes.Clone(src)
+		copy(out[:3], []byte("   "))
+		return out, nil
+	case bytes.HasPrefix(src, []byte{0xff, 0xfe}), bytes.HasPrefix(src, []byte{0xfe, 0xff}):
+		return nil, fmt.Errorf("UTF-16 BOM cannot be prepared without changing source coordinates")
+	default:
+		return src, nil
+	}
+}
+
 // languageFor returns the language a transform re-types path to, or ""
 // when no transform claims it. Lets a file whose extension is not
 // natively indexed (e.g. .pdf) still reach an extractor.

internal/mcp/rename_apply.go

@@ -9,6 +9,7 @@ import (
 	"unicode/utf8"
 
 	"github.com/zzet/gortex/internal/agents"
+	"github.com/zzet/gortex/internal/indexer"
 )
 
 // renameEdit is one planned single-line replacement in a coordinated rename.
@@ -45,6 +46,10 @@ func (s *Server) unindexedRenameRecovery(ctx context.Context, id, newName string
 	}
 
 	file := s.graphPathSpelling(relPath)
+	owner, relPath := s.indexerForRel(file)
+	if owner == nil || relPath == "" {
+		return nil
+	}
 	if indexed := s.engineFor(ctx).GetFileSymbols(file); indexed != nil && indexed.TotalNodes > 0 {
 		return nil
 	}
@@ -53,7 +58,7 @@ func (s *Server) unindexedRenameRecovery(ctx context.Context, id, newName string
 	if err != nil {
 		return nil
 	}
-	extracted, err := s.indexer.ExtractSource(ctx, relPath, content)
+	extracted, err := owner.ExtractSource(ctx, relPath, content)
 	if err != nil || extracted == nil {
 		return nil
 	}
@@ -77,7 +82,7 @@ func (s *Server) unindexedRenameRecovery(ctx context.Context, id, newName string
 	}
 
 	oldLine, newLine, ok := s.verifiedDeclarationRename(
-		ctx, relPath, content, declarationLine, requestedSymbol, declarationName, newName,
+		ctx, owner, relPath, content, declarationLine, requestedSymbol, declarationName, newName,
 	)
 	if !ok {
 		return nil
@@ -129,6 +134,7 @@ const (
 // rather than at its name.
 func (s *Server) verifiedDeclarationRename(
 	ctx context.Context,
+	owner *indexer.Indexer,
 	relPath string,
 	content []byte,
 	declarationLine int,
@@ -175,7 +181,7 @@ func (s *Server) verifiedDeclarationRename(
 		candidateLines[declarationLine-1] = candidateLine
 		candidateContent := []byte(strings.Join(candidateLines, ""))
 
-		candidate, err := s.indexer.ExtractSource(ctx, relPath, candidateContent)
+		candidate, err := owner.ExtractSource(ctx, relPath, candidateContent)
 		if err == nil && candidate != nil {
 			originalPresent := false
 			expectedAtDeclaration := 0