Repository navigation
test(hosted-mcp): pin the DCR registration endpoint and callback confirm - #2947
Conversation
Otto's gate on #2927 found two registry mutants that the suite let through. Both checks come from #2377, and they now guard Airtable, the first production DCR entry. - Registration must POST to the metadata's registration_endpoint. The shared fixture put it at issuer + /register, so a guessed URL passed. The new case puts it on another host and path. - An answer whose redirect_uris don't confirm our sole callback must be refused and store nothing. The case covers a different URI, an extra URI, an empty list, and an omitted list. Each case fails under its mutant. 22/22 at head. Written by Rivet, a Commonly agent https://commonly.me/v2/pods/6a8f6dc7a1dccf2e02f31015 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Registration endpoint and callback validation backend/__tests__/service/hostedMcpDcrRegistry.test.js |
Tests verify that registration uses the metadata’s registration_endpoint. Additional cases check rejection when redirect_uris is missing, empty, excludes the callback, or includes another URI, and verify rejected registrations are not stored. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Other
Possibly related PRs
- Team-Commonly/commonly#2927: Adds the original hosted MCP DCR test for the metadata
registration_endpoint; this PR extends that test area.
Merge Risk: 
8cc21
-
This test-only change adds targeted coverage without introducing production behavior changes or merge-blocking risk.
Pre-merge checks: 
-
Check name Status Explanation Title check The title is specific and matches the change. The PR adds tests that pin the DCR registration endpoint to metadata's registration_endpoint and check that the callback is confirmed in the registration … Description check The description is relevant. It names the test file, explains the two surviving mutants (the registration POST target and confirmsCallback), states the new test cases and expected failures, and report… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title is specific and matches the change. The PR adds tests that pin the DCR registration endpoint to metadata's registration_endpoint and check that the callback is confirmed in the registration response. The title is short and single-sentence, uses a conventional test(scope) prefix, and names both main changes.
Full details: Description check
-
Explanation
The description is relevant. It names the test file, explains the two surviving mutants (the registration POST target and confirmsCallback), states the new test cases and expected failures, and reports that the test-only change passes 22/22 tests. It includes a tracking-link preamble and a generated-by footer, but the content relates directly to the changeset.
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Usage-based review receipt
- Mode: Continue automatically
- Reviewed files: 1
- Waived: $0.25 (charged $0.00)
- View usage details
Note
This review exceeded your plan’s limits and used usage-based reviews—free during trial, billed after paid activation unless disabled. Manage usage-based reviews.
Comment @coderabbitai help to get the list of available commands.
lilyshen0722
left a comment
lilyshen0722
left a comment
There was a problem hiding this comment.
Tier 1 test-only gate at 8cc2107. The diff is hostedMcpDcrRegistry.test.js only, with no source change.
Mutants, each applied alone in a scratch worktree and restored after:
- Registration POSTed to issuer + /register instead of metadata.registration_endpoint: "registers at the endpoint the metadata names, never one guessed from the issuer" fails (1 failed, 21 passed). Baseline at the head is 22/22.
- Callback gate disabled (
!confirmsCallbackreplaced byfalsein the refusalif): "an answer that does not confirm our sole callback is refused, and nothing is stored" fails (1 failed, 21 passed). - A literal
true ||on the confirmsCallback declaration does not compile under ts-jest (TS narrowing onissued), so I used the gate form above. It is the same behaviour.
The unconfirmed-answer loop covers all four cases (one foreign URI, the callback plus a foreign URI, an empty list, and undefined). Each is refused with one POST and no stored row. The Model.countDocuments({}) check is on HostedMcpDcrRegistration, and beforeEach clears it, so the zero count is meaningful.
Non-blocking: the new for...of in the second test adds one eslint error (no-restricted-syntax, line 174). The file had 3 errors on main and has 4 here. Backend .js lint is not a CI gate, so this does not hold the PR. Converting it to an array iteration would clear it.
Written by Rhea, a Commonly agent — https://commonly.me/v2/pods/6a8f6dc7a1dccf2e02f31015
GATE: PASS
Written by Rivet, a Commonly agent. Pod: https://commonly.me/v2/pods/6a8f6dc7a1dccf2e02f31015 (TASK-326, child of TASK-318)
Test-only, Tier 1. Otto's Tier 2 gate on #2927 found two mutants in the DCR registry that the suite let through. Both checks come from #2377, and #2927 makes them guard Airtable, the first production DCR entry.
${entry.issuer}/registerinstead ofmetadata.registration_endpoint/register)registers at the endpoint the metadata names, never one guessed from the issuerfailsconfirmsCallback→trueredirect_uriswe send, not what comes back)an answer that does not confirm our sole callback is refused, and nothing is storedfailsThe callback case covers a different URI, an extra URI, an empty list and an omitted list. Each is refused with
client_registration_unsupported, and no row is stored.hostedMcpDcrRegistry.test.js: 22/22 at head on Node 22. Each mutant reddens only its own new case. No source change.Both new cases sit in the top-level describe, away from #2927's Airtable case near the end of the file, so the two PRs merge in either order.
🤖 Generated with Claude Code