Skip to content

feat: [FD-5561] implement FDv2 polling endpoint GET /sdk/poll#701

Merged
hkotian merged 8 commits intomainfrom
hkotian/FD-5561/v2-poll-support
Apr 28, 2026
Merged

feat: [FD-5561] implement FDv2 polling endpoint GET /sdk/poll#701
hkotian merged 8 commits intomainfrom
hkotian/FD-5561/v2-poll-support

Conversation

@hkotian
Copy link
Copy Markdown
Contributor

@hkotian hkotian commented Apr 27, 2026

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Related issues

Provide links to any issues in this repository or elsewhere relating to this pull request.

Describe the solution you've provided
Adds the FDv2 polling endpoint support in the dev server.

In the dev server we are only supporting full transfers or no transfers. Delta transfers would require recording the change sets, which may just be an overkill. So in the event an older basis is provided, we will always do a full transfer and a no-transfer when the sdk is caught up.

For more details of the protocol see : https://launchdarkly.atlassian.net/wiki/spaces/PD/pages/3726999667/Tech+Spec+FDv2+Server+Side+Protocol

Describe alternatives you've considered

Provide a clear and concise description of any alternative solutions or features you've considered.

Additional context

Add any other context about the pull request here.


Note

Medium Risk
Adds a new SDK-facing polling endpoint and implements FDv2 event payload construction; mistakes could cause clients to miss updates or over-fetch payloads. No auth or persistent data mutations are introduced beyond existing project/override reads.

Overview
Implements an FDv2 polling endpoint at GET /sdk/poll that returns LaunchDarkly FDv2 polling events based on the client-provided basis selector.

The server now parses basis values of the form (p:<payloadId>:<version>) and responds with either no transfer when up-to-date or a full transfer (server-intent + put-object per flag + payload-transferred) when missing/stale/mismatched; delta transfers are intentionally not supported.

Adds unit/integration-style tests covering basis parsing, full vs none transfer behavior, URL-encoded basis handling, and 404s for unknown projects.

Reviewed by Cursor Bugbot for commit 1e9ea0a. Bugbot is set up for automated code reviews on this repo. Configure here.


Related Jira issue: FD-5561: Implement poll server side endpoint handling in Launchdarkly dev server

hkotian and others added 6 commits April 24, 2026 14:27
Adds the FDv2 flag-delivery-v2 polling endpoint used by Go SDK v7.13+.
The basis param (the client's current payload version) is used only to
skip an unnecessary full transfer when the client is already up-to-date.
Stale clients always receive a full payload (xfer-full/cant-catchup) —
delta transfers are intentionally not supported, as that would require
persisting a change history which is overkill for a local dev server.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace locally-defined FDv2 wire-format types with the equivalent types
from go-server-sdk/v7/subsystems: RawEvent, PollingPayload, PutObject,
ServerIntent, Selector, and the IntentTransferFull/IntentNone/FlagKind
constants. The three reason strings (up-to-date, cant-catchup,
payload-missing) have no SDK equivalents and remain as local constants.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@launchdarkly-upra launchdarkly-upra Bot changed the title feat: implement FDv2 polling endpoint GET /sdk/poll feat: [FD-5561] implement FDv2 polling endpoint GET /sdk/poll Apr 27, 2026
@hkotian hkotian marked this pull request as ready for review April 27, 2026 17:57
@hkotian hkotian requested review from a team, jstncbllr, kinyoklion and svenkatesan-ld and removed request for a team April 27, 2026 17:58
Comment thread internal/dev_server/sdk/fdv2.go Outdated
Comment thread internal/dev_server/sdk/polling.go Outdated
return
}

basisVersion := parseBasisVersion(r.URL.Query().Get("basis"))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this URL decoded?
%28p%3A%3CpayloadId%3E%3A%3Cversion%3E%29

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is, the Query() function calls ParseQuery that should decode it. Will add a unit test to be sure

Comment thread internal/dev_server/sdk/fdv2.go Outdated

// parseBasisVersion extracts the payload version from a basis state string of the
// form "(p:<payloadId>:<version>)". Returns 0 if the string is absent or unparseable.
func parseBasisVersion(basis string) int {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parsing the selector makes me a little nervous. it would be more ideal for it to remain an opaque value.

I'm wondering if we can just pass the basis through, and pass the response as-is back out?

@jstncbllr
Copy link
Copy Markdown

jstncbllr commented Apr 28, 2026

Stepping back from the line-level comments — I want to suggest a structural rethink before this lands.

The framing this PR adopts: the dev server owns a PayloadVersion, increments it on local changes, encodes it into a selector, and answers "is the SDK current?" by parsing the SDK's basis and comparing against its own counter.

Why that's brittle:

  • The selector format is meant to be opaque. The basis parser is correct only for (p:<id>:<n>). Any selector with filters or big-segment hashes silently misparses (LastIndex(":") lands on the wrong tuple).
  • It's a "remember to bump everywhere" invariant. Today there are four call sites (CreateProject, UpdateProject, UpsertOverride, DeleteOverride). Maybe more tomorrow?
  • It conflates two different kinds of change — upstream config evolution and local override edits — into one monotonic counter that has to stand in for both.
  • Project recreate resets the counter to 1, a suspicious movement noted by bugbot, and an example of the confusion that can arise from maintaining a faux version.

The cleaner model I'd propose: the dev server doesn't author selectors at all. Identity is upstream's; overrides are a value-substitution layer applied at egress.

  • Streaming: override toggled → push a put-object for that flag at the current selector → SDK swaps value at the same version.
  • Polling: always forward the SDK's basis to the backend; take whatever it returns (full / incremental / none); regardless, layer in put-object events for any overrides changed since the last poll. The dev server tracks override mutations locally — narrower and more honest than fabricating a version space.

Backend calls become an upstream-freshness concern; override delivery is purely local.

This deletes:

  • Project.PayloadVersion
  • IncrementProjectPayloadVersion
  • parseBasisVersion
  • The >= comparison and the trap it implies

Local state narrows to what's actually local: override values, an override-change log (so we can deliver override deltas to polling SDKs regardless of what the backend says), and a cache of the last upstream value per flag (needed so override-unset can emit the original value without a synchronous backend round-trip).

One contract worth confirming with the SDK team before betting on this: that SDKs accept put-object events and apply value changes regardless of whether the resulting selector advanced. I believe that's the protocol, but worth sanity-checking that no SDK short-circuits on selector-equality.

I think we should resolve this big picture design question before merging.


Drafted in collaboration with Claude (Opus 4.7).

@hkotian
Copy link
Copy Markdown
Contributor Author

hkotian commented Apr 28, 2026

@jstncbllr : I don't think I understand this

Polling: always forward the SDK's basis to the backend; take whatever it returns (full / incremental / none); regardless, layer in put-object events for any overrides changed since the last poll. The dev server tracks override mutations locally — narrower and more honest than fabricating a version space.

the dev server is the backend that reads the flag configurations from the local store. I am not sure we would get any transfers here. If we were to drop the versioning we would do a full transfer always on polling requests. I had initially considered doing this, but then figured it'd be good to handle no transfer if none of the flags have changed.

@jstncbllr
Copy link
Copy Markdown

Polling: always forward the SDK's basis to the backend; take whatever it returns (full / incremental / none); regardless, layer in put-object events for any overrides changed since the last poll. The dev server tracks override mutations locally — narrower and more honest than fabricating a version space.

the dev server is the backend that reads the flag configurations from the local store. I am not sure we would get any transfers here. If we were to drop the versioning we would do a full transfer always on polling requests. I had initially considered doing this, but then figured it'd be good to handle no transfer if none of the flags have changed.

By "backend" i mean the FD polling service. I'm thinking of the dev server as a mitm / proxy in this context.

My version-less suggestion also avoids a full transfer, assuming that we can track which overrides have changed since the last poll from the client. If that's problematic (maybe multiple concurrent clients?), we could also resend only the overrides. Or we might associate the payload selector from the backend with a timestamp. Then if/when we see that basis from the client, we can use the timestamp to reduce the set of overrides that need to be sent (we'd need to also give each override a timestamp).

Copy link
Copy Markdown

@jstncbllr jstncbllr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my previous comments were based on bad assumption: the selector parsed in this PR is not actually from the FD backend, so there's no real concern about breaking the opacity principle. lgtm!

@hkotian hkotian requested a review from kinyoklion April 28, 2026 20:33
Comment thread internal/dev_server/sdk/fdv2.go
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ 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 1e9ea0a. Configure here.

basisPayloadID, basisVersion := parseBasis(basis)
switch {
case basisVersion == 0:
return buildFullTransferResponse(payloadID, currentVersion, flags, fdv2ReasonPayloadMissing)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version zero sentinel conflates invalid and valid basis

Low Severity

buildPollResponse checks basisVersion == 0 to detect a missing or unparseable basis, but parseBasis returns ("", 0) for invalid input and ("some-id", 0) for the valid input (p:some-id:0). This means a well-formed basis at version 0 is misclassified as payload-missing instead of falling through to the version/payloadID comparison logic. Checking basisPayloadID == "" (or using a boolean ok-return from parseBasis) would correctly distinguish parse failure from a genuine version-0 basis.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 1e9ea0a. Configure here.

Base automatically changed from hkotian/FD-5510/adding-payload-id-version-dev-server to main April 28, 2026 21:59
@hkotian hkotian merged commit 8a1cda1 into main Apr 28, 2026
11 checks passed
@hkotian hkotian deleted the hkotian/FD-5561/v2-poll-support branch April 28, 2026 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants