docs: adding missing capabilities to service spec#361
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| Events: events.Endpoint().BaseURL(), | ||
| }), | ||
| dataSource)...) | ||
| client := NewSDKClient(t, c.baseSDKConfigurationPlus(dataSystem)...) |
There was a problem hiding this comment.
Disabling events test drops endpoint
Medium Severity
The disabling-events subtests still require the service-endpoints capability but no longer configure the mock events base URI (or pass the event sink configurer to the client). The scenario under test is that events stay off even when an events service endpoint is set; without that wiring, the harness no longer exercises that behavior and can pass without validating it.
Reviewed by Cursor Bugbot for commit 06851df. Configure here.
| dataSource)...) | ||
| verifyRequestHeader(t, dataSource.Endpoint()) | ||
| dataSystem)...) | ||
| verifyRequestHeader(t, dataSystem.Synchronizers[0].Endpoint()) |
There was a problem hiding this comment.
Poll option ignored in header test
Medium Severity
Subtests named for poll requests pass only DataSystemOptionPolling() into NewSDKDataSystem, but default client-side setup already defines a streaming connection mode. In that branch buildDataSystem never applies the top-level polling flag, so the SDK keeps using streaming and the subtest may assert headers on a stream connection while claiming to cover polling.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 06851df. Configure here.
| return []byte("UNSUPPORTED") | ||
| // data, _ := p.currentData.(ServerSDKData) | ||
| // flagsJSON, _ := json.Marshal(data["flags"]) | ||
| // return flagsJSON |
There was a problem hiding this comment.
PHP poll mocks return UNSUPPORTED
High Severity
PHP polling routes still use dedicated handlers, but those handlers now always write the literal body UNSUPPORTED instead of flag or segment JSON from the configured SDK data. PHP contract tests that hit /sdk/flags, /sdk/flags/{key}, or /sdk/segments/{key} will receive unusable responses.
Reviewed by Cursor Bugbot for commit 06851df. Configure here.
| // Once the directive engaged the FDv1 fallback, the FDv2 stream must | ||
| // be quiet; assert the SDK is not concurrently retrying it. | ||
| streamEndpoint.RequireNoMoreConnections(t, time.Millisecond*500) | ||
| }) |
There was a problem hiding this comment.
FDv1 fallback test missing capability gate
Medium Severity
The instance-id “FDv1 fallback directive requests” subtest is gated only on fdv1-fallback, but it drives FDv2 streaming with HTTP 403 and the X-LD-FD-Fallback header. Client-side SDKs without client-event-source-http-errors typically cannot observe that directive; the subtest should skip when that capability is absent for client-side runs, matching other FDv1 fallback coverage.
Triggered by learned rule: Gate SDK-specific test scenarios behind capability checks
Reviewed by Cursor Bugbot for commit 06851df. Configure here.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
joker23
changed the title
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
4 issues from previous reviews remain unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 06851df. Configure here.
joker23
changed the title
1 participant
Note
Low Risk
Documentation-only changes to the service spec with no runtime or test harness behavior changes.
Overview
Documents capabilities and SDK classification rules that the harness already supports but were missing from the service spec.
SDK typing: Adds
"roku"as an SDK type capability and a rule thatclient-side+mobile+rokuidentifies a Roku client-side SDK (mobile special case). The intro to the SDK type section is lightly reworded.New optional capabilities:
auto-env-attributes— client-side auto appending of environment attributes; tied toincludeEnvironmentAttributesin config.client-independence— multiple concurrent client instances with isolated state.fdv1-fallback— server-directed FDv1 fallback; gates related subtests when omitted.Fix: Closes the missing quote in the
client-per-context-summariescapability heading.Reviewed by Cursor Bugbot for commit 06851df. Bugbot is set up for automated code reviews on this repo. Configure here.