[refactor] HandleRelate reads source node outside OpMu lock — inconsistent with every other write handler

[radimsem]

Summary

HandleRelate in pkg/mcp/tools/relate.go is the only write-side MCP tool that validates a node's existence (Store.GetNode) before acquiring Store.OpMu.Lock(). Every sibling write handler — pin.go, forget.go, summarize.go, write.go — acquires the lock first and reads inside the critical section. The outlier path opens a small TOCTOU window and breaks the consistency contract a future reader will rely on when scanning the package.

Location

pkg/mcp/tools/relate.go:35-55

// line 35 — read happens before lock
_, err = d.Store.GetNode(ctx, input.SourceID)
if errors.Is(err, sql.ErrNoRows) {
    return nil, nil, fmt.Errorf("source_id not found: %s", input.SourceID)
}
if err != nil {
    return nil, nil, fmt.Errorf("failed to fetch: source node %s: %w", input.SourceID, err)
}

// ... weight defaulting, ref construction ...

// line 55 — lock acquired here
d.Store.OpMu.Lock()
defer d.Store.OpMu.Unlock()

Contrast with the established pattern:

• pkg/mcp/tools/pin.go:42-50 — OpMu.Lock() at line 42, GetNode at line 46.
• pkg/mcp/tools/forget.go:31-40 — OpMu.Lock() at line 31, GetNode at line 34.
• pkg/mcp/tools/summarize.go:28-31 — OpMu.Lock() first, then GetNode.

Category

refactor (consistency)

Impact

Medium.

• Concurrency: A concurrent MemoryForget(input.SourceID) between the validation GetNode and the eventual UpsertRelation / InsertPendingRelation will trip the FK constraint on relations.source_node_id (or pending_relations.source_node_id) at the DB layer. The error surfaced to the caller becomes an opaque SQLite FK violation instead of the clean "source_id not found" the handler intended to return.
• Convention drift: HandleRelate reads as an outlier. Future contributors auditing the tools package for the write-tool locking discipline will either question this handler or, worse, copy its inverted order into a new handler.
• Symmetry with target resolution: The target side (Resolver.Resolve) is already inside the lock; only the source-side validation escapes it. Moving the source read in completes the symmetry.

Suggested direction (not applied)

Move the GetNode(ctx, input.SourceID) validation block and its error branches below d.Store.OpMu.Lock(). Keep the early input.SourceID == "" guard before the lock (no DB access). The result is HandleRelate matching the shape of pin.go / forget.go exactly.

No new abstraction needed. The change is localized to lines 35–55 of relate.go.

---

Filed by the nightly enhancement-scanner routine. Reviewed commit: 00a6c9e9.