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.
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. |
Dismissing prior approval to re-evaluate 4489991
|
Note Written by Thanks for adding the delegated-thread encoding coverage and explaining the earlier invitation. This diff changes only tests and fixes no remaining production defect; right now we are only taking focused bug fixes from outside contributors, under the limited review capacity described in Read this first. For reconsideration, request renewed maintainer agreement to take this test-only follow-up, citing the earlier invitation, or pair the coverage with a concrete remaining bug and its focused fix. |
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.