Repository navigation
Conversation
Threads that OAuth MCP clients delegate get ids like thread:mcp:client%3A<session>:<request>:0, so the id itself carries a "%". The thread snapshot, bounded snapshot and history requests must send that "%" as %25 and sign the same URL. Run the MCP thread id test over both id shapes and assert the exact request path instead of a substring.
Contributor
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a narrowly scoped, test-only coverage expansion confined to an ignored path, with no production behavior, product-default, or static-analysis configuration changes. Its impact is limited to validating delegated MCP thread ID encoding and signed URL requests in the test harness. Notes:
You can add or adjust custom eligibility rules. Learn more. |
macroscopeapp
Bot
dismissed
their stale review
October 11, 2026 07:05
Dismissing prior approval to re-evaluate 4489991
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
#15813 (#15813) was closed in favor of #17599 (#17599), with the note: "If there's test coverage here that #17599 doesn't have, a small follow-up PR adding it is welcome." Since then #17602 (#17602) builds the signed URL from the HttpApi contract. That covers most of what #15813 tested. One thread id shape is still untested.
Threads that OAuth MCP clients delegate get ids like
thread:mcp:client%3A<session>:<request>:0, because the server URI-encodes theclient:<session>namespace into the id. The id itself contains a%. The current test only usesmcp:<uuid>and checks that the path contains/mcp%3A. Nothing checks that a%in the id goes out as%25, so the server decodes it back to the same id, or that the proof signs that URL.Change
environmentHttpAuth.test.tsnow runs each thread loader (snapshot, bounded snapshot, history) for both id shapes:mcp:<uuid>and a delegatedthread:mcp:client%3A…id.…/threads/thread%3Amcp%3Aclient%253Asession-1%3Arequest-1%3A0/boundedand so on) instead of a substring, and still checks that the proof signs the URL that was sent.Verification
vp test run src/state/environmentHttpAuth.test.ts(client-runtime)vp run typecheck(client-runtime)vp fmt --check/vp linton the test fileTest-only change, no UI, so no screenshots.
Implemented with Claude Opus 5.5, coordinated by Claude Fable 5.1 in Claude Code.