Skip to content

fix!: derive provider status from a single ordered event relay - #259

Closed
mtonko-flx wants to merge 2 commits into
open-feature:mainfrom
mtonko-flx:fix/single-ordered-event-relay
Closed

fix!: derive provider status from a single ordered event relay#259
mtonko-flx wants to merge 2 commits into
open-feature:mainfrom
mtonko-flx:fix/single-ordered-event-relay

Conversation

@mtonko-flx

Copy link
Copy Markdown

Stacked on #258. Base is main because the branch lives in a fork and GitHub requires a base branch in the upstream repository, so the diff shown here includes #258's commit. Review only the second commit, fix!: derive provider status from a single ordered event relay, or merge #258 first and this diff will reduce to it.

Intent

Provider status and provider events are two independent subscriptions to the same provider stream. handleProviderEvents collects it to update the status, and observe<T>() hands application code its own subscription — so nothing orders the two, and a handler can observe an event before the status reflects it. SDK-inferred transitions also never appear in the event stream at all. This PR makes the SDK collect the provider once, write the status, and only then republish, so the status is always current by the time subscribers see the event that caused it.

Motivation

Requirement 5.3.5 requires the status be updated before the handlers for that event run; with two independent subscriptions that cannot be guaranteed. Requirement 5.3.3 requires a handler attached when the provider is already in a state to run immediately, which nothing currently provides. Both are prerequisites for spec #385, where the status is derived entirely from provider events.

It also closes the gap raised on spec #365 that Kotlin's observe is a straight passthrough to the provider, so lifecycle events around setProvider/setEvaluationContext never appear in it.

Spec Requirements

Requirement Relationship
5.3.5 — the SDK MUST update the status "before invoking any event handlers for that event" Fixes an existing violation
5.3.3 — "Handlers attached after the provider is already in the associated state, MUST run immediately" Fixes an existing violation
5.2.3 — event details MUST contain the provider name Fixes an existing violation — stamped by the relay
1.7.1, 1.7.2.1 Satisfied — status accessor incl. RECONCILING
1.7.3, 1.7.4, 1.7.5 Satisfied — derived from provider events via the 5.3.5 table
1.7.6NOT_READY once shutdown terminates Satisfied — inferred by the SDK, status only, no event (5.1.1 has no PROVIDER_NOT_READY)
5.2.6 — handlers MUST persist across provider changes Improved — an SDK-owned stream outlives provider swaps
1.4.9, 1.4.10 Preserved — see NOT_READY/FATAL short-circuit below

Changes

Implementation

  • One internal collector on provider.observe(): stamps the provider name, writes the status from the event, then republishes onto an SDK-owned MutableSharedFlow that observe<T>() reads.
  • Republication uses tryEmit against a bounded buffer, so a slow application subscriber can never stall the SDK's own status derivation — requirement 1.1.2.4 notes application handlers need not complete. Overflow drops the oldest and logs.
  • New subscribers receive an event synthesised from the current status, not a replay of the last raw event. Replaying the last event would resurface a stateless PROVIDER_CONFIGURATION_CHANGED, or an event belonging to a provider that has since been replaced.
  • statusFlow becomes a StateFlow; getStatus() reads .value instead of a replay cache.

NOT_READY/FATAL short-circuit

Requirements 1.7.6/1.7.7 as they stood — the client short-circuiting evaluation while NOT_READY/FATAL — were removed by #385 and retagged @spec-2.2.7, on the grounds that reading the status and then evaluating is itself a time-of-check/time-of-use gap (spec #369). They were removed rather than inverted, so short-circuiting remains permitted. This SDK keeps it: dropping it would change the error codes callers already depend on, and EvaluationState snapshots the provider and context together, which bounds the staleness. Only the KDoc changes, to record that this is now deliberate policy rather than a requirement.

Testing

  • A handler reads the matching status at the moment it runs (5.3.5).
  • A subscriber attaching while the provider is already READY — and separately while STALE — receives that state's event immediately; a stateless CONFIGURATION_CHANGED is not resurfaced, nor is an event from a replaced provider (5.3.3).
  • Every relayed event carries the emitting provider's name, synthesised ones included (5.2.3).
  • shutdown reverts to NOT_READY without putting an event into the stream (1.7.6).
  • 241 tests, ktlintCheck and apiCheck green, apiDump regenerated.

Two pre-existing test weaknesses surfaced and were corrected rather than worked around: several event tests launched a collector and then registered a provider before it had subscribed, passing only because the provider's own flow replayed; and status-sequence assertions relied on the relay running in real time. Both now subscribe first and drive the relay on the test dispatcher.

Breaking Changes

statusFlow is now StateFlow<OpenFeatureStatus> rather than Flow<OpenFeatureStatus>, and conflates consecutive equal values — status is a snapshot, and the transition sequence belongs to observe<T>(). Source-compatible for collection; binary-incompatible, and code asserting on every intermediate status should move to the event stream.

Spec requirement 5.1.1 requires the provider event set to include
PROVIDER_RECONCILING and PROVIDER_CONTEXT_CHANGED, and 5.2.3 requires event
details to carry the provider name. Neither existed, so the SDK could not
represent context reconciliation as events nor attribute an event to its
provider.

Signed-off-by: Mark Tonkonoh <mark.tonkonoh@fluxon.com>
@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Comment @coderabbitai help to get the list of available commands.

BREAKING CHANGE: statusFlow is now StateFlow<OpenFeatureStatus> and conflates
consecutive equal values.

Signed-off-by: Mark Tonkonoh <mark.tonkonoh@fluxon.com>
@mtonko-flx

Copy link
Copy Markdown
Author

Superseded by #262, which delivers this work as a single change. Closing the stack.

@mtonko-flx mtonko-flx closed this Aug 28, 2026
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