feat: Add contract tests for client-side secure mode hash (h query param)#358
feat: Add contract tests for client-side secure mode hash (h query param)#358joker23 wants to merge 2 commits into
Conversation
…ram) Adds streaming and polling contract tests verifying that client-side SDKs include the h query parameter when a secure mode hash is configured, and omit it when no hash is configured. Tests gate on the existing secure-mode-hash capability alongside the implicit client-side requirement. Adds Hash o.Maybe[string] to SDKConfigClientSideParams so the harness can pass a hash value through the test protocol to client-side SDK test services.
joker23
force-pushed
the
skz/sdk-2512/secure-mode-client-side
branch
from
cbdf3c2 to
c7b8d98
Compare
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c7b8d98. Configure here.
| // m.AllOf() is used as "match any value" since this matchers library has no Anything() function. | ||
| m.In(t).For("h query parameter").Assert(request.URL.RawQuery, | ||
| UniqueQueryParameters().Should(m.Not(m.MapIncluding(m.KV("h", m.AllOf()))))) | ||
| }) |
There was a problem hiding this comment.
Streaming tests skip JS polling
Medium Severity
The streaming secure-mode hash cases only assert h on the streaming mock endpoint, but JS client SDKs in streaming mode also issue an initial polling request via CommonStreamingTests.setupDataSources. The service spec requires h on both streaming and polling requests when hash is set, so a JS SDK could omit or mis-set h on that bootstrap poll and still pass these tests.
Reviewed by Cursor Bugbot for commit c7b8d98. Configure here.
There was a problem hiding this comment.
this would break other client side sdks that do not have this. I think the polling tests would cover this gap.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
1 participant
Adds streaming and polling contract tests verifying that client-side SDKs
include the h query parameter when a secure mode hash is configured, and
omit it when no hash is configured. Tests gate on the existing
secure-mode-hash capability alongside the implicit client-side requirement.
Adds Hash o.Maybe[string] to SDKConfigClientSideParams so the harness can
pass a hash value through the test protocol to client-side SDK test services.
Note
Low Risk
Test harness and documentation changes only; no production SDK runtime behavior in this repo.
Overview
Adds client-side contract coverage for secure mode: when
clientSide.hashis set, streaming and polling connections must send it as thehquery parameter; when unset,hmust not appear. Tests run only for services with the existingsecure-mode-hashcapability.The harness now exposes
hashonSDKConfigClientSideParamsand documents it in the service spec. AMapHasKeymatcher supports asserting that a query param key is absent. The new suite is registered under the client-side test entry point.Reviewed by Cursor Bugbot for commit 056e28f. Bugbot is set up for automated code reviews on this repo. Configure here.