From f24148b2273c4e7ff679b8067245b33b983cfcb0 Mon Sep 17 00:00:00 2001
From: jcalabro <jamesrcalabro@gmail.com>
Date: Fri, 3 Apr 2026 15:18:12 -0400
Subject: [PATCH] fix MST integer overflow: findKey/deleteFromNode underflow at
 layer 0

After deleting keys, the tree trim loop could reduce root_layer below
what remaining keys require. findKey and deleteFromNode then computed
layer - 1 with layer=0, causing u32 underflow. Fixed by changing the
height == layer check to height >= layer (handles keys above the current
layer) and adding a layer == 0 early-return guard before recursion.

The insert-50-delete-every-other stress test now passes, validating that
the tree structure remains consistent after bulk deletions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 src/internal/repo/mst.zig      | 7 +++++--
 src/internal/repo/mst_test.zig | 8 ++------
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/src/internal/repo/mst.zig b/src/internal/repo/mst.zig
index 1ec0b81..6e9128c 100644
--- a/src/internal/repo/mst.zig
+++ b/src/internal/repo/mst.zig
@@ -172,7 +172,8 @@ pub const Mst = struct {
     fn findKey(maybe_node: ?*Node, layer: u32, key: []const u8, height: u32) ?cbor.Cid {
         const node = maybe_node orelse return null;
 
-        if (height == layer) {
+        if (height >= layer) {
+            // key belongs at this layer or above — scan entries
             for (node.entries.items) |entry| {
                 const cmp = std.mem.order(u8, key, entry.key);
                 if (cmp == .eq) return entry.value;
@@ -182,6 +183,7 @@ pub const Mst = struct {
         }
 
         // height < layer: recurse into the subtree gap containing key
+        if (layer == 0) return null; // can't go deeper
         for (node.entries.items, 0..) |entry, i| {
             if (std.mem.order(u8, key, entry.key) == .lt) {
                 const child = if (i == 0) node.left else node.entries.items[i - 1].right;
@@ -234,7 +236,7 @@ pub const Mst = struct {
     fn deleteFromNode(self: *Mst, node: *Node, layer: u32, key: []const u8) !?cbor.Cid {
         const height = keyHeight(key);
 
-        if (height == layer) {
+        if (height >= layer) {
             // find and remove the entry
             for (node.entries.items, 0..) |entry, i| {
                 if (std.mem.eql(u8, entry.key, key)) {
@@ -259,6 +261,7 @@ pub const Mst = struct {
         }
 
         // height < layer: recurse into the appropriate gap
+        if (layer == 0) return null; // can't go deeper
         if (node.entries.items.len == 0) {
             switch (node.left) {
                 .node => |left| return try self.deleteFromNode(left, layer - 1, key),
diff --git a/src/internal/repo/mst_test.zig b/src/internal/repo/mst_test.zig
index 95d870d..366d91e 100644
--- a/src/internal/repo/mst_test.zig
+++ b/src/internal/repo/mst_test.zig
@@ -156,16 +156,12 @@ test "100 keys: all retrievable after insertion" {
     }
 }
 
-// TODO: this test exposes an integer overflow bug in MST tree rebalancing
-// after deletions. The tree.get() call panics after removing keys.
-// Needs investigation in the delete/rebalance path of mst.zig.
-test "insert 10 keys then remove every other" {
-    if (true) return error.SkipZigTest; // skip until MST delete bug is fixed
+test "insert 50 keys then remove every other" {
     var arena = std.heap.ArenaAllocator.init(std.testing.allocator);
     defer arena.deinit();
     const a = arena.allocator();
 
-    const n = 10;
+    const n = 50;
     var tree = Mst.init(a);
     var key_bufs: [n][64]u8 = undefined;
     var keys: [n][]const u8 = undefined;
-- 
2.51.2