Skip to content

fix(mcp): surface non-ErrNoRows GetNode errors in MemoryWrite - #250

Merged
radimsem merged 1 commit into
devfrom
fix/write-surfaces-getnode-error
Jun 6, 2026
Merged

fix(mcp): surface non-ErrNoRows GetNode errors in MemoryWrite#250
radimsem merged 1 commit into
devfrom
fix/write-surfaces-getnode-error

Conversation

@radimsem

@radimsem radimsem commented Jun 6, 2026

Copy link
Copy Markdown
Owner

Closes #218.

Problem

HandleWrite discarded the GetNode error with existing, _ := d.Store.GetNode(ctx, nodeID). Any non-sql.ErrNoRows failure (DB locked, ctx cancelled, corrupted page, broken connection) was silently swallowed, and the handler proceeded to construct a brand-new node from payload defaults. When the operator explicitly supplied an Anchor, that turns a transient store failure into a silent defaulted-metadata create instead of a loud, recoverable error.

Fix

Surface non-ErrNoRows errors while still falling through to the create branch on sql.ErrNoRows, matching the existing pattern in forget.go and summarize.go:

existing, err := d.Store.GetNode(ctx, nodeID)
if err != nil && !errors.Is(err, sql.ErrNoRows) {
    return nil, nil, fmt.Errorf("failed to fetch: node %s: %w", nodeID, err)
}

The early return is before the sole emitNodeChange site, so the one-snapshot-per-call invariant holds and no snapshot is created on the error path.

Test

Adds TestHandleWrite_SurfacesGetNodeError: a cancelled context forces GetNode to return context.Canceled (non-ErrNoRows). Verified regression guard — passes with the fix, fails without (the swallowed path surfaces a later, unrelated emit failure: "failed to write: failed to fetch: head snapshot id").

Verification

  • make build clean · go vet ./pkg/mcp/... exit 0 · make test green (220 mcp tests) · make lint 0 issues
  • Reviewed by mcp-surface, go-style, and Codex — 0 actionable findings
  • Semantics-only fix: tool name/signature/return/locking unchanged, no public-skill update needed

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: radimsem <me@radimsemerak.cz>
@radimsem radimsem self-assigned this Jun 6, 2026
@radimsem radimsem added the bug Something isn't working label Jun 6, 2026
@radimsem
radimsem merged commit 27aae33 into dev Jun 6, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[enhancement] HandleWrite swallows GetNode errors with blank identifier — non-ErrNoRows DB failures silently fall into the create path

1 participant