Skip to content

fix: critical bugs, stale removal, hot reload, E2E tests#12

Merged
msilverblatt merged 31 commits intomasterfrom
fix/critical-fixes-and-testing
Mar 21, 2026
Merged

fix: critical bugs, stale removal, hot reload, E2E tests#12
msilverblatt merged 31 commits intomasterfrom
fix/critical-fixes-and-testing

Conversation

@msilverblatt
Copy link
Owner

Summary

Comprehensive fix pass addressing 9 critical issues found during repo review, plus exhaustive real-scenario E2E tests for advanced features.

Bug Fixes

  • Python SDK: Add missing import os in transport.py (runtime crash on large payloads)
  • Rust SDK: Fix test isolation — add missing clear functions, clear_all_registries() helper, mutex poison recovery
  • Go SDK distribution: Add go.work for local dev, remove replace directives, add sdk/go/vX.Y.Z submodule tagging in CI
  • Tool groups: Change default strategy from union to separate (oneOf not supported by Claude Code / most MCP clients)
  • Bridge stale removal: Track registered tools/resources/prompts and call RemoveTools/RemoveResources/RemovePrompts on sync
  • Hot reload: Watch entire directory tree recursively (not just single file), auto-watch new subdirectories
  • Version injection: Inject version via ldflags instead of hardcoded "1.0.0"
  • Rust dispatch deadlock: Release GROUP_REGISTRY lock before calling handlers
  • Bridge atomic sync: Add SyncAll() for atomic tool/resource/prompt sync during reload

E2E Tests (5 new integration tests)

  • Tool groups with separate strategy
  • Multi-step workflow (review → approve → deploy state machine)
  • Middleware (audit logging + arg injection)
  • Sidecar with HTTP health check
  • Hot reload verifying tool addition and stale removal

Bugs Found During Testing

  • Process manager Reload channel routing bug (ToolListResponse routed to wrong channel)
  • Python discovery not clearing tool/workflow registries on reload
  • TypeScript group tests not updated for new default strategy

Test Results

Suite Tests Status
Go (11 packages) All
Python SDK 146
Rust SDK 73
TypeScript SDK 100
E2E Integration 9

Test plan

  • Go test suite passes (go test ./...)
  • Python SDK tests pass (pytest tests/)
  • Rust SDK tests pass with parallel execution (cargo test)
  • TypeScript SDK tests pass (vitest run)
  • All 9 E2E integration tests pass
  • Verify brew install on clean machine after release

Add clear_prompt_registry, clear_resource_registry, clear_resource_template_registry.
Export from lib.rs with clear_all_registries() helper. Add process-wide TEST_LOCK
mutex so tests with global registry state serialize correctly under default parallelism.
Use unwrap_or_else(|e| e.into_inner()) on all Mutex lock calls to survive poisoning.
The oneOf discriminated union schema is not understood by Claude Code
or most MCP clients. 'separate' creates individual tools per action
which works universally. 'union' remains available for clients that
support oneOf.
Add go.work workspace with replace directive for sdk/go pre-tag local dev,
remove replace directives from sdk/go and examples go.mod files, set real
version requirements, and add CI step to push sdk/go submodule tag on release.
Hot reload now watches the entry-point file's parent directory
recursively, filtering by file extension. Changes to any source file
in the project trigger a reload.
Previously syncTools/syncResources/syncPrompts only called Add, never
Remove. Tools that were deleted or renamed during hot reload persisted
as stale entries in the MCP server.
Also fixes two bugs discovered during testing:
- Fix process manager Reload to read ToolListResponse from pending channel
  (Python SDK sends it with request_id, not as unsolicited message)
- Fix Python discovery to clear tool and workflow registries on hot reload
  (previously only cleared group registry, leaving stale tools)
Task 4 changed the default strategy from 'union' to 'separate' but
the TypeScript group tests were not updated.
Prevents potential deadlock if a handler accesses the registry.
Prevents partial sync when concurrent tool mutations interleave
with hot reload's sequential SyncTools + SyncResources + SyncPrompts.
The protocol-based ReloadRequest only works for scripts using the
discovery module with hot_reload=True. Simple single-file scripts
don't re-import on ReloadRequest, so the tool list never changes.

Replace with a stop-and-restart approach that kills the old process,
resets internal state, and spawns a fresh process. This works for
all languages and all script structures.
Breaking change: default tool group strategy is now "separate"
(each action becomes its own tool) instead of "union" (oneOf).
Users passing strategy="union" explicitly are unaffected.
…and-testing

# Conflicts:
#	.gitignore
#	README.md
#	sdk/python/pyproject.toml
#	sdk/python/src/protomcp/discovery.py
#	sdk/rust/Cargo.toml
#	sdk/rust/src/group.rs
#	sdk/rust/src/local_middleware.rs
#	sdk/rust/src/middleware.rs
#	sdk/rust/src/server_context.rs
#	sdk/rust/src/sidecar.rs
#	sdk/rust/src/telemetry.rs
#	sdk/rust/src/tool.rs
#	sdk/rust/src/workflow.rs
#	sdk/typescript/package.json
#	test/e2e/e2e_test.go
@msilverblatt msilverblatt merged commit e0b62e9 into master Mar 21, 2026
4 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant