feat(testkit): tool-list-changed assertion helper (MC-47)

lock_unlock_test.go

@@ -32,53 +32,23 @@ func (hwGroupCap) Attach(r *mcpkit.Registrar) error {
 // TestLockBeforeConnectThenUnlockAtRuntime proves the lazy-tier mechanism:
 // locking a group before the app ever connects keeps that group's tools out
 // of the very first tools/list, and a runtime Unlock later brings them in
-// and notifies the connected client via
-// notifications/tools/list_changed (observed here through mcpx.Client's
-// OnToolListChanged, since that requires a raw mcpx client rather than
-// testkit's harness).
+// and notifies the connected client via notifications/tools/list_changed
+// (observed here via testkit.AssertToolListChanged, MC-47).
 func TestLockBeforeConnectThenUnlockAtRuntime(t *testing.T) {
 	app, err := mcpkit.New(mcpkit.Info{Name: "lazy-tier", Version: "0.0.1"}, generic.New(), hwGroupCap{})
 	require.NoError(t, err)
 
 	require.NoError(t, app.Lock("hw"))
 
-	ctx := context.Background()
-	serverT, clientT := mcpx.InMemoryPair()
-
-	srvSess, err := app.Connect(ctx, serverT)
-	require.NoError(t, err)
-	t.Cleanup(func() { _ = srvSess.Close() })
-
-	changed := make(chan struct{}, 4)
-	client := mcpx.NewClient(mcpx.Implementation{Name: "lazy-tier-client", Version: "0.0.1"}, &mcpx.ClientOptions{
-		OnToolListChanged: func(_ context.Context) {
-			changed <- struct{}{}
-		},
-	})
-	clientSess, err := client.Connect(ctx, clientT)
-	require.NoError(t, err)
-	t.Cleanup(func() { _ = clientSess.Close() })
+	h := testkit.New(t, app)
 
-	tools, err := clientSess.ListTools(ctx)
-	require.NoError(t, err)
-	require.Len(t, tools.Tools, 1, "locked group's tool must not appear in the initial tools/list")
-	require.Equal(t, "ungrouped-tool", tools.Tools[0].Name)
+	testkit.AssertToolSet(t, h, "ungrouped-tool")
 
 	require.NoError(t, app.Unlock("hw"))
 
-	select {
-	case <-changed:
-	case <-time.After(5 * time.Second):
-		t.Fatal("did not receive tool list changed notification after Unlock")
-	}
+	testkit.AssertToolListChanged(t, h, 5*time.Second)
 
-	tools, err = clientSess.ListTools(ctx)
-	require.NoError(t, err)
-	names := make([]string, 0, len(tools.Tools))
-	for _, tool := range tools.Tools {
-		names = append(names, tool.Name)
-	}
-	require.ElementsMatch(t, []string{"hw-tool", "ungrouped-tool"}, names, "Unlock must bring the group's tool into tools/list")
+	testkit.AssertToolSet(t, h, "hw-tool", "ungrouped-tool")
 }
 
 // TestLockAtRuntimeUnregistersTool proves a runtime Lock (called after the

testkit/assert.go

@@ -38,6 +38,17 @@ func EventuallyContains(t testing.TB, timeout, interval time.Duration, fn func()
 	}, timeout, interval, "expected %q to eventually appear", want)
 }
 
+// AssertToolListChanged fails the test unless the harness observes at least
+// one notifications/tools/list_changed notification within timeout. The
+// go-sdk debounces rapid successive list changes into a single
+// notification, so this asserts "at least one arrived," never an exact
+// count.
+func AssertToolListChanged(t testing.TB, h *Harness, timeout time.Duration) {
+	t.Helper()
+	require.True(t, h.WaitForToolListChanged(timeout),
+		"expected a tools/list_changed notification within %s", timeout)
+}
+
 // AssertToolSet asserts that the app's advertised tools/list is exactly
 // want, guarding against silent drift.
 func AssertToolSet(t testing.TB, h *Harness, want ...string) {

testkit/harness.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"sync"
 	"testing"
+	"time"
 
 	"github.com/dangernoodle-io/mcpkit"
 	"github.com/dangernoodle-io/mcpkit/mcpx"
@@ -28,6 +29,15 @@ type Harness struct {
 
 	mu       sync.Mutex
 	progress map[any][]ProgressEvent
+
+	// toolListChanged is signaled (non-blocking) each time the client
+	// receives a notifications/tools/list_changed notification. Buffered so
+	// the client's receive goroutine never blocks on a slow/absent waiter;
+	// callers only need to observe that at least one arrived within a
+	// timeout, not an exact count (the go-sdk debounces bursts ~10ms apart
+	// into a single notification, so an exact-count assertion would be
+	// flaky by design).
+	toolListChanged chan struct{}
 }
 
 // New composes app over an in-memory transport pair, connects a client, and
@@ -46,10 +56,15 @@ func New(t testing.TB, app *mcpkit.App) *Harness {
 		_ = srvSess.Close()
 	})
 
-	h := &Harness{t: t, progress: make(map[any][]ProgressEvent)}
+	h := &Harness{
+		t:               t,
+		progress:        make(map[any][]ProgressEvent),
+		toolListChanged: make(chan struct{}, 8),
+	}
 
 	client := mcpx.NewClient(mcpx.Implementation{Name: "testkit", Version: "0.0.0"}, &mcpx.ClientOptions{
-		OnProgress: h.recordProgress,
+		OnProgress:        h.recordProgress,
+		OnToolListChanged: h.recordToolListChanged,
 	})
 
 	sess, err := client.Connect(ctx, clientT)
@@ -74,6 +89,31 @@ func (h *Harness) recordProgress(_ context.Context, token any, message string, p
 	})
 }
 
+// recordToolListChanged runs on the client's receive goroutine; it must
+// never block, so the signal is a non-blocking send into a buffered
+// channel.
+func (h *Harness) recordToolListChanged(_ context.Context) {
+	select {
+	case h.toolListChanged <- struct{}{}:
+	default:
+	}
+}
+
+// WaitForToolListChanged blocks until the harness observes at least one
+// notifications/tools/list_changed notification, or timeout elapses. It
+// returns true on the former, false on the latter. Because the go-sdk
+// debounces bursts of list changes into a single notification (~10ms),
+// callers should not assert an exact count — one call observes "at least
+// one arrived."
+func (h *Harness) WaitForToolListChanged(timeout time.Duration) bool {
+	select {
+	case <-h.toolListChanged:
+		return true
+	case <-time.After(timeout):
+		return false
+	}
+}
+
 // CallTool calls the named tool with args, which must be JSON-marshalable.
 func (h *Harness) CallTool(ctx context.Context, name string, args any) (*mcpx.CallToolResult, error) {
 	return h.session.CallTool(ctx, name, args)

testkit/harness_internal_test.go

@@ -0,0 +1,46 @@
+package testkit
+
+import (
+	"context"
+	"sync"
+	"testing"
+)
+
+// TestRecordToolListChanged_BufferFull proves that recordToolListChanged
+// handles a full buffer correctly by dropping signals in the default case
+// (non-blocking send). This tests the drop branch at line ~98 of harness.go.
+//
+// The test constructs a Harness with the buffered channel, calls
+// recordToolListChanged bufferSize+1 times in rapid succession (no draining),
+// and asserts it neither blocks nor panics. The first bufferSize sends succeed
+// (channel fills), and the (+1)th send hits the non-blocking default clause,
+// proving the drop branch is reachable and safe.
+func TestRecordToolListChanged_BufferFull(t *testing.T) {
+	const bufferSize = 8
+
+	// Construct a minimal Harness with the toolListChanged channel at the
+	// same buffer size as the real constructor uses (line 62 of harness.go).
+	h := &Harness{
+		t:               t,
+		mu:              sync.Mutex{},
+		progress:        make(map[any][]ProgressEvent),
+		toolListChanged: make(chan struct{}, bufferSize),
+	}
+
+	// Send bufferSize + 1 signals. The first bufferSize hit the
+	// successful send case (case h.toolListChanged <- struct{}{}:).
+	// The (+1)th hits the default: case, exercising the drop branch.
+	//
+	// If recordToolListChanged blocks on any send, this test will hang
+	// (test timeout). If it panics, the panic surfaces immediately.
+	for i := 0; i < bufferSize+1; i++ {
+		h.recordToolListChanged(context.Background())
+	}
+
+	// Verify the channel is exactly full (bufferSize sends succeeded,
+	// the (+1)th was dropped).
+	if len(h.toolListChanged) != bufferSize {
+		t.Fatalf("expected channel to contain %d signals (all successful sends before drop), got %d",
+			bufferSize, len(h.toolListChanged))
+	}
+}

testkit/harness_test.go

@@ -97,3 +97,45 @@ func TestHarnessProgress(t *testing.T) {
 	// A different token must not see this call's events.
 	require.Empty(t, h.ProgressEvents("unrelated-token"))
 }
+
+// TestHarnessToolListChanged_Timeout proves WaitForToolListChanged returns
+// false when no notifications/tools/list_changed notification arrives
+// within the timeout (nothing in this test triggers one).
+func TestHarnessToolListChanged_Timeout(t *testing.T) {
+	app, err := mcpkit.New(mcpkit.Info{Name: "no-change-test", Version: "0.0.1"}, generic.New(), pingCap{})
+	require.NoError(t, err)
+
+	h := testkit.New(t, app)
+
+	require.False(t, h.WaitForToolListChanged(20*time.Millisecond))
+}
+
+type lockedToolCap struct{}
+
+func (lockedToolCap) Attach(r *mcpkit.Registrar) error {
+	mcpkit.AddTool(r, &mcpx.Tool{
+		Name:        "locked-tool",
+		Description: "d",
+	}, mcpkit.ReadOnly, func(_ context.Context, _ *mcpx.CallToolRequest, _ struct{}) (*mcpx.CallToolResult, pingOut, error) {
+		return nil, pingOut{}, nil
+	}, mcpkit.Group("locked"))
+	return nil
+}
+
+// TestHarnessToolListChanged_Signaled proves WaitForToolListChanged (and its
+// AssertToolListChanged wrapper) observe a real
+// notifications/tools/list_changed notification fired by a runtime Unlock.
+func TestHarnessToolListChanged_Signaled(t *testing.T) {
+	app, err := mcpkit.New(mcpkit.Info{Name: "list-changed-test", Version: "0.0.1"}, generic.New(), lockedToolCap{})
+	require.NoError(t, err)
+
+	require.NoError(t, app.Lock("locked"))
+
+	h := testkit.New(t, app)
+	testkit.AssertToolSet(t, h)
+
+	require.NoError(t, app.Unlock("locked"))
+
+	testkit.AssertToolListChanged(t, h, 5*time.Second)
+	testkit.AssertToolSet(t, h, "locked-tool")
+}