Skip to content

feat: Honor the service's FDv1 fallback directive in the client - #623

Open
beekld wants to merge 2 commits into
bklimt/SDK-3035/client-fdv2-identifyfrom
bklimt/SDK-3036/client-fdv2-fdv1-fallback
Open

beekld wants to merge 2 commits into
bklimt/SDK-3035/client-fdv2-identifyfrom
bklimt/SDK-3036/client-fdv2-fdv1-fallback

Conversation

@beekld

@beekld beekld commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Honors the service's directive to move the client off FDv2 back to FDv1, and provides an FDv1 source to fall back to.

  • Adds an FDv1 adapter synchronizer that wraps an existing FDv1 client source so it can act as an FDv2 synchronizer.
  • On the fallback directive (a response header or a goodbye), applies any payload that arrived with it, then rotates from the FDv2 sources to the FDv1 fallback.
  • Retries FDv2 after the directive's TTL expires.

Note

Overview
Adds FDv1 fallback when the service tells the client to leave FDv2: a new FDv1AdapterSynchronizer exposes an existing FDv1 IDataSource as an FDv2 synchronizer (Init/Upsert/status → changesets and FDv2SourceResult).

FDv2DataSource now reacts to fdv1_fallback on initializer and synchronizer results: it still applies any accompanying payload, switches the synchronizer tier via SourceManager, starts the FDv1 fallback when configured, and schedules an FDv2 retry after the directive TTL (cancellable on close). If no FDv1 tier is configured, it reports interrupted with a clear message instead of staying on FDv2.

Unit tests cover the adapter lifecycle/conversions and orchestration (apply-before-fallback, FDv1 tier start, disconnect without fallback, TTL retry).

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

@beekld
beekld force-pushed the bklimt/SDK-3036/client-fdv2-fdv1-fallback branch from 4992afc to 9df0441 Compare October 3, 2026 01:21
@beekld
beekld force-pushed the bklimt/SDK-3036/client-fdv2-fdv1-fallback branch from 9df0441 to 468d47a Compare October 5, 2026 18:00
@beekld
beekld added this pull request to stack #627 October 5, 2026 18:00
@beekld
beekld removed this pull request from stack #627 October 5, 2026 18:05
@beekld
beekld added this pull request to stack #628 October 5, 2026 18:10
@beekld
beekld removed this pull request from stack #628 October 5, 2026 19:18
@beekld
beekld added this pull request to stack #629 October 5, 2026 19:24
@beekld
beekld force-pushed the bklimt/SDK-3036/client-fdv2-fdv1-fallback branch from 468d47a to 346c0d3 Compare October 5, 2026 23:25
@beekld
beekld force-pushed the bklimt/SDK-3036/client-fdv2-fdv1-fallback branch from 346c0d3 to f0596a8 Compare October 6, 2026 05:04
@beekld
beekld marked this pull request as ready for review October 6, 2026 16:53
@beekld
beekld requested a review from a team as a code owner October 6, 2026 16:53

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 using default effort and found 2 potential issues.

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 f0596a8. Configure here.

active_conditions_.reset();
}
StartSynchronizers();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale shutdown aborts FDv2 retry

High Severity

OnFDv2RetryTimer closes the FDv1 adapter while its Next() is still in the orchestrator's WhenAny. That unblocks the old future with Shutdown, which is posted onto the executor after the new FDv2 synchronizer has already been started. OnSynchronizerResult then treats that stale Shutdown as belonging to the replacement and resets active_synchronizer_, so the SDK is left with no live source after the TTL elapses.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f0596a8. Configure here.

DataSourceStatus::DataSourceState::kInterrupted,
DataSourceStatus::ErrorInfo::ErrorKind::kUnknown,
kNoFDv1FallbackConfigured);
return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fallback status skips shutdown fence

Medium Severity

When a fallback directive arrives and no FDv1 tier is configured, the new path writes kInterrupted through status_manager_->SetState instead of PublishState. That skips GuardShutdown, so a late write after Close can overwrite the shared status that outlives this source.

Additional Locations (1)
Fix in Cursor Fix in Web

Triggered by learned rule: Client FDv2DataSource: basis is selector; GuardShutdown fences store

Reviewed by Cursor Bugbot for commit f0596a8. Configure here.

This branch has not been deployed

No deployments
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.

1 participant