feat(mcp): complete protocol proxy surface - #70
Conversation
📝 WalkthroughWalkthroughMiftah now proxies MCP resource templates, subscriptions, aggregate pagination, list-change notifications, progress, and cancellation across STDIO and Streamable HTTP upstreams. Registries, pipelines, transports, identity verification, tests, fixtures, error mappings, and documentation were updated. ChangesMCP protocol conformance
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review The initial CodeRabbit status has remained pending as Review in progress since 01:34 UTC with no review output, while all current-head Linux, macOS, Windows, quality, and package checks are green. Requesting a fresh review run. |
|
✅ Action performedFull review finished. You're currently rate limited under our Fair Usage Limits Policy. Your recent PR review activity is in the 95th percentile or higher among CodeRabbit users, so adaptive limits apply. Your next review will be available in 44 minutes. |
|
Merge-gate exception documented: CodeRabbit confirmed that its original review stalled, and its requested retry is unavailable for 44 minutes because of an adaptive rate limit. A thread-aware audit found zero review threads and no human reviews. All current-head required GitHub Actions checks are green. Proceeding under the repository rule that permits a demonstrably stuck CodeRabbit review to be ignored only after documenting the evidence. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mcp/server/miftah-server.ts`:
- Around line 494-528: The subscription-capability probe in
configureResourceSubscriptionCapability currently starts and stops every
profile/upstream serially, adding substantial startup cost. Reduce the probing
work while preserving the documented maxConcurrentProfiles limit and accurate
resource-subscription detection, reusing existing upstream/session management
symbols such as this.upstreams, get, closeProfile, and
resourceSubscriptionsAvailable.
In `@src/mcp/server/resource-prompt-registry.ts`:
- Around line 251-273: The resource-template registration path must detect
upstream template renames instead of reusing a route with a stale exposed name.
In the loop handling discovered templates, update the existing-route logic
around `existing`, `exposedName`, and `registerResourceTemplateRoute` to
validate that `existing.exposedName` matches the newly computed name and raise
`RESOURCE_TEMPLATE_COLLISION` when it differs, while preserving reuse for
unchanged templates.
In `@src/upstream/progress-preserving-transport.ts`:
- Around line 84-88: Update the finally-path in the progress-preserving
transport to reschedule draining when a synchronous notification handler failure
leaves pendingMessages non-empty, while preserving the existing responseDeferred
behavior and close propagation. Extend the recovery test to enqueue a response
before the handler throws, and verify that both the response and any requested
close are eventually delivered.
In `@tests/resource-prompt-registry.test.ts`:
- Around line 199-212: Correct the URI uniqueness assertion in the aggregate
pagination test by checking the Set’s size rather than using toHaveLength on the
Set. Keep the existing combined-resource mapping and expected count of 3 so the
test genuinely detects duplicate URIs.
- Around line 394-429: The test’s resolution expectation should assert the
expanded alpha template URI rather than the template name. Update the assertion
in the “removes failed template routes…” test around alphaUri so the expected
originalUri matches the substituted account://alpha/one value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a7baa5fa-1a46-4786-8ccf-3b7c5d2a425d
📒 Files selected for processing (30)
CHANGELOG.mdREADME.mddocs/architecture.mddocs/config.mddocs/library-api.mdsrc/audit/audit-journal.tssrc/cli/exit-codes.tssrc/identity/identity-manager.tssrc/mcp/server/miftah-server.tssrc/mcp/server/operation-pipeline.tssrc/mcp/server/resource-prompt-registry.tssrc/mcp/server/tool-registry.tssrc/upstream/progress-preserving-transport.tssrc/upstream/upstream-process-manager.tssrc/upstream/upstream-session.tssrc/utils/errors.tstests/audit.test.tstests/cli-exit-codes.test.tstests/fixtures/back-to-back-progress-upstream.mjstests/fixtures/fake-upstream.mjstests/helpers/fake-remote-upstream.tstests/identity-manager.test.tstests/mcp-wrapper.test.tstests/multi-upstream.test.tstests/package-contract.test.tstests/progress-preserving-transport.test.tstests/public-api.test.tstests/remote-transport.test.tstests/resource-prompt-registry.test.tstests/upstream-manager.test.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/upstream/progress-preserving-transport.ts`:
- Around line 130-132: Update asError to preserve the original thrown value when
it is not an Error, deriving the resulting Error message from that value instead
of always using the fixed generic message; retain existing Error instances
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 097f1b3b-53e4-4785-8ed8-363b867f066f
📒 Files selected for processing (3)
src/upstream/progress-preserving-transport.tstests/progress-preserving-transport.test.tstests/resource-prompt-registry.test.ts
1 participant
Summary
Validation
npm test(67 files, 907 passed, 11 platform skips)npm run test:core(17 files, 329 passed, 11 platform skips)npm run test:coverage(all enforced thresholds pass)npm run lintnpm run typechecknpm run buildnpm run smoke:clinpm run check:packnpm run test:packageCloses #32
Summary by CodeRabbit
New Features
Bug Fixes
Documentation