merkle: Diff silently returns no changes when max-level metadata reads fail

[smiggleworth]

Summary

merkle.Tree.Diff can return an empty change set with no error when the two roots differ but reading the current tree's maxlevel metadata fails.

findMaxLevel currently collapses both missing metadata and every backend/decoding error to -1. diffNode then interprets -1 as an empty diff. A transient backend failure or corrupt metadata can therefore be reported as "no changes" instead of failing closed.

Affected code

• diffNode treats a negative resolved level as empty
• findMaxLevel discards the error

Reproduction

Tested against main commit 0eff19b94c4d500b4098c70c83407aae23613d34 with Go 1.26.6 on darwin/arm64.

package merkle

import (
    "bytes"
    "context"
    "errors"
    "testing"

    "github.com/fgrzl/enumerators"
    "github.com/fgrzl/kv"
    "github.com/fgrzl/lexkey"
    "github.com/stretchr/testify/require"
)

type maxLevelReadFailKV struct {
    kv.KV
    target lexkey.PrimaryKey
    err    error
}

func (s *maxLevelReadFailKV) Get(ctx context.Context, key lexkey.PrimaryKey) (*kv.Item, error) {
    if bytes.Equal(key.PartitionKey, s.target.PartitionKey) &&
        bytes.Equal(key.RowKey, s.target.RowKey) {
        return nil, s.err
    }
    return s.KV.Get(ctx, key)
}

func TestDiffShouldPropagateMaxLevelMetadataReadFailure(t *testing.T) {
    // Arrange
    ctx := context.Background()
    tree := setup(t)
    require.NoError(t, tree.Build(ctx, "previous", "accounts", leaves("A")))
    require.NoError(t, tree.Build(ctx, "current", "accounts", leaves("B")))
    wantErr := errors.New("injected max-level read failure")
    failingTree := NewTree(&maxLevelReadFailKV{
        KV:     tree.store,
        target: metaPK("current", "accounts", maxLevelKey),
        err:    wantErr,
    })

    // Act
    changes, err := enumerators.ToSlice(
        failingTree.Diff(ctx, "previous", "current", "accounts"),
    )

    // Assert
    require.ErrorIs(t, err, wantErr)
    require.Empty(t, changes)
}

Run:

go test ./pkg/merkle -run '^TestDiffShouldPropagateMaxLevelMetadataReadFailure$' -count=1

Current result:

Expected error with "injected max-level read failure" in chain but got nil.

Expected behavior

The diff enumerator should return the metadata read/decoding error. Callers must not trust a partial or empty result after any required tree read fails.

Actual behavior

enumerators.ToSlice returns an empty slice and nil error even though the roots differ and traversal could not determine the tree level.

Impact

Replication, verification, or synchronization callers can incorrectly conclude that no entities changed during a backend outage or metadata-corruption event.

Suggested direction

Change findMaxLevel to preserve absence separately from failure, for example (level int, found bool, err error), and propagate errors through the diff enumerator. A missing level for a non-empty tree should also be treated according to an explicit corruption/legacy-state policy rather than silently as no changes.

A related path may merit the same audit: walkSubtree currently converts both Get errors and missing nodes into an empty enumerator.