Remote Config v2: client core, transport and experimental API (release train) - #866
Draft
shameondev wants to merge 20 commits into
Draft
Remote Config v2: client core, transport and experimental API (release train)#866shameondev wants to merge 20 commits into
shameondev wants to merge 20 commits into
Conversation
Implement the internal Remote Config v2 transport adapter behind the existing RemoteConfigFetchTransport seam, so the resilient fetch policy can talk to the dark gateway routes without any public API change. - Bootstrap on a missing or expired session (POST /v3/remote-config-v2/session) and read the snapshot (POST /v3/remote-config-v2/snapshot) with the session header and the coordinator's exact If-None-Match validator. - A snapshot 401 drops the session and re-bootstraps exactly once; a second 401 is a typed failure, so the flow cannot loop. - The response body reaches durable admission as the exact bytes received, paired with the exact ETag: no decode, re-encode or charset round trip. - Sessions are stored per identity scope under a salted digest key, so an identity change addresses a different record and can never reuse the previous identity's token. Neither token is ever logged. - device_installed_at is sourced from PackageManager.firstInstallTime, a device fact that survives logout: the server takes min(device_installed_at, client.created_at), so a moving value would make a long-time user look new. Tests use MockWebServer (new test-only dependency, pinned to the SDK's OkHttp version) and cover the wire shape of both routes, byte equality on a non-canonical body, 304, 401 recovery and the no-loop bound, 404/503, session scoping, and the timeout path through the real coordinator. Claude-Session: https://claude.ai/code/session_018pHXqfbxkMQJFzUZ3jW4A8
Adversarial review of the adapter found four real defects and several weaker-than-claimed tests. All are addressed here. - A session or project token containing a non-printable byte reached Request.Builder.header, which throws — on an OkHttp dispatcher thread, from the mint callback and the 401 retry. That both stranded the coordinator's waiter (no completion) and put the credential into the exception message, so a token could reach a crash reporter. Tokens are now validated as HTTP header values, and request building is failure-typed instead of throwing. - The RFC3339 expiry parser only accepted exactly three fractional digits and an uppercase T/Z, so a Go gateway's RFC3339Nano timestamp parsed as "unknown" and every fetch silently re-bootstrapped forever. Replaced with a parser that accepts 0-9 fractional digits, either case, and numeric offsets. - A dead in-memory session shadowed the durable record and skipped its cleanup. - The session store is now keyed by the anonymous uid the session was minted for, not only by the snapshot scope, so a re-minted uid under an unchanged scope can never replay the previous identity's token. Also: bounded (and injectable) snapshot body read, an empty 200 body is a typed failure rather than an empty admission, Accept header, BCP-47 locale so Hebrew and Indonesian are not sent as the legacy iw/in codes, and MockWebServer pinned to the OkHttp version that actually resolves rather than the declared one. New coverage: non-printable tokens, fractional/lowercase expiry, over-budget and empty bodies, in-memory session reuse, concurrent fetches each answered exactly once, and a re-minted uid addressing a different session record. Claude-Session: https://claude.ai/code/session_018pHXqfbxkMQJFzUZ3jW4A8
Adds the customer-facing Remote Config v2 surface on top of the internal
snapshot core, fetch coordinator and gateway transport landed in the
previous slices. Nothing here re-implements them: the public types are
thin, immutable adapters over the resolution ladder, the read guard and
the coordinator's waiter model.
The surface (all marked @ExperimentalQonversionApi, so it is not yet a
stability promise):
- Qonversion.remoteConfigSnapshots() -> QRemoteConfigSnapshots, with
fetch(timeoutMs?), activate(), fetchAndActivate(), an immutable
`current` snapshot, a synchronous bundled-fallback getter and
subscribeOnConfigUpdate().
- Reads return {value, source}: raw JSON, an opaque JSON tree, or a
caller-decoded type. The decoder is the per-key validator seam, so a
rejected value falls to the previously activated release (cache) and
then to the bundled defaults.
- fetch's timeout bounds the wait, not the request: the completion
reports the best available snapshot while the request keeps running and
is still admitted when it lands.
- Activation stays a whole-release atomic swap; an immediate-policy
release performs that same swap on admission and notifies subscribers
with the changed-key diff and per-key metadata.
- Identity changes switch the scope synchronously (the previous
identity's release is never readable afterwards) and force a fetch; an
identify that only attaches an external id re-reads targeting without
dropping the served release.
The pipeline is dormant unless the app passes a QRemoteConfigV2Config:
without it no store, thread, HTTP client or base URL is constructed, and
every fetch completes with NotConfigured. There is no default endpoint.
Tests cover the contract end to end against a real MockWebServer with the
real core, guard and coordinator: timeout semantics, change detection,
all three ladder positions, raw vs typed reads, the pre-activate fallback
getter, subscription diffs, immediate auto-activation, the identity
switch (old snapshot excluded, per-identity session, install date pinned
against a real PackageManager) and main-thread delivery.
Claude-Session: https://claude.ai/code/session_018pHXqfbxkMQJFzUZ3jW4A8
QRemoteConfigV2Config demanded a `contextFingerprint`: the app had to hand the SDK the fingerprint the gateway resolves for it, and an admitted snapshot had to carry exactly that value. Nobody could supply it correctly, because it is not an app-level constant at all. configurator computes it in BuildResolvedSnapshotContextFingerprint (internal/domain/remoteconfigv2/resolved_snapshot.go) by hashing the canonical user uid, randomization id, platform, country, app version, OS version, SDK version, locale, device model, media source / campaign, install and created timestamps, purchases, active experiment uids and custom user properties. It is a per-response tag over mutable targeting inputs, not an identity binding: it rotates on any app or OS update, a language switch, a purchase, a property edit or an experiment enrollment. So the fingerprint is treated as what it is: - it is gone from the public QRemoteConfigV2Config and from the internal RemoteConfigSnapshotEnvelopeExpectation — nothing configures it and nothing compares it against a previous response; - the parser keeps validating its *shape* (64 lowercase hex, required member) and carries it through as an opaque per-response tag; a value that changes between two admissions in the same scope is normal and admitted; - it is still stored with the release, as informational data for logs and bug reports, and it still participates in the release content digest; - the KDoc on the public config, the expectation and the release all state the rule verbatim, so the "pin it across fetches" idea does not get re-invented: pinning it would freeze an identity's config until logout the first time the user updated the app or changed their language. Identity isolation is unchanged and stays where it already lives: each snapshot read travels on a session token minted for exactly one identity, the gateway routes on that session, and the snapshot / session / fetch-policy stores address each identity through its own salted scope digest. No compatibility shim: the surface is @ExperimentalQonversionApi and has never shipped as stable, so the constructor parameter is simply removed. Tests: a rotated fingerprint is admitted end to end through the public API over MockWebServer (and at the core, where the previously admitted release keeps its own tag), every malformed or missing fingerprint is still refused by the parser, and the project / environment admission boundaries are unchanged. Claude-Session: https://claude.ai/code/session_018pHXqfbxkMQJFzUZ3jW4A8
The numeric project id a v2 snapshot is admitted against was supplied by the app through QRemoteConfigV2Config, but the app is not its source: the SDK is told it by the gateway's session bootstrap. Asking for it added a public value that could only ever be typed wrong. BREAKING (experimental surface): QRemoteConfigV2Config no longer takes projectId. Callers drop the argument; nothing else changes for them. The id is now learned and pinned per project key + environment by RemoteConfigProjectIdRegistry, durably, next to the session state: the first bootstrap establishes it, every later session must agree, and one that does not is refused as the typed RemoteConfigFetchResponse .ProjectMismatch before a snapshot is ever read — never re-learned. The in-memory pin is authoritative for the process, so a storage failure cannot downgrade a conflict into a silent re-learn. A malformed id is reported apart from a conflict and stays an ordinary failure. Because the pin is established mid-fetch, the envelope expectation moved from the admission claim to the admission itself: beginAdmission takes only the scope, admitCandidate takes the project id the response was served for, and the environment is read from the admitting scope rather than restated. RemoteConfigFetchBinding was exactly a scope plus that expectation, so it is gone and the coordinator binds to the scope. A mismatch feeds the failure backoff. It is permanent until the gateway is fixed and costs a bootstrap round trip each time, and forced fetches bypass the minimum interval but not the backoff gate, so an identify/logout loop cannot turn a misrouted gateway into a request storm. The check this buys is server-vs-server consistency plus trust on first bootstrap, not proof that a snapshot belongs to the project the developer meant to target; QRemoteConfigV2Config's KDoc says so.
…ease
The gateway needs to know which release is actually serving, so an activation
that changes the active release now reports itself out of band:
POST v3/remote-config-v2/ack with the project token, the very session the
snapshot was read under, and {"release_number", "activated_at"}.
The ack is deliberately powerless over the config data path. It cannot block
or slow an activation (it is queued on a later worker task, strictly after the
completion is handed off), it never calls back into the app, it never feeds the
fetch policy, and every failure is silent — the only trace of a lost ack is a
counter, not a log line, so a flapping gateway cannot become a log storm.
Semantics:
- exactly one ack per (scope, release), which survives a restart because the
last acked release is persisted next to the pending one;
- an implicit (read-triggered) activation is acked exactly like an explicit
one, and the explicit activate() that follows reports Unchanged and stays
silent;
- at most one ack in flight per scope, newest activation wins;
- a queued ack is durable and is resumed when its identity is bound again;
- retries are bounded to three jittered attempts, then abandoned in-process
while the durable record keeps the ack owed;
- an identity change fences delivery: one identity's session may never vouch
for another identity's activation.
The route rides the existing transport seam — same bootstrap, same single
re-bootstrap-on-401 rule — which is why mint() now reports a typed outcome
instead of writing a fetch response directly. The fetch path's behaviour is
unchanged.
Claude-Session: https://claude.ai/code/session_018pHXqfbxkMQJFzUZ3jW4A8
Findings from an adversarial pass over the ack queue, in severity order: - A permanently refused ack was forgotten instead of settled, so its durable record was deleted and the very same ack was re-queued and re-POSTed on every process start and every identity binding. The likeliest permanent refusal is a 404 from a gateway that does not serve /ack yet, i.e. exactly the rollout state — so the bug was the storm the design forbids. A release is now settled by either answer, delivered or permanently refused. - The three-attempt bound was per binding, not per process: once the ladder was exhausted any bind() bought another three. Abandonment is now remembered in memory for the process, and only a newer release re-arms delivery. - The durable write ran while holding the sender lock, so a synchronous SharedPreferences commit on an OkHttp thread could park the Remote Config worker — the thread that runs activate() and the fetch. Records are now prepared under the lock and written outside it, ordered by a write stamp. - Ack retries ran their preferences read and durable write on the shared timer thread, which also releases fetch waiters; they now only wake it and do the work on the Remote Config worker. - The ack route cleared the shared gateway session on a 401. An out-of-band signal may not invalidate state the config read path depends on: it now just mints a replacement. - Full-downward jitter could put all three attempts inside milliseconds; the delay is now half the cap plus jitter. - A rejected worker submit left the activation marked as handed off, losing that ack for good, and a zero activation timestamp made the queued ack silently un-persistable. Tests follow the corrected semantics and no longer race the retry timer they arm: the 503 ladder waits for the scheduled retry instead of assuming it is already there. Claude-Session: https://claude.ai/code/session_018pHXqfbxkMQJFzUZ3jW4A8
`identity transition waits for admitted response and its callback delivery boundary` asserted that the fetch callback always runs before transitionTo() returns. The coordinator drains deliveries OUTSIDE operationLock, so a transition that takes the lock in the window between the admission releasing it and the drain reaching the waiter legitimately returns first — and the waiter is then told Superseded rather than Fetched, because its identity is already gone. Both interleavings are correct. The window is small enough that the test passed in isolation and failed in roughly two of five full-suite runs once the suite grew. What the test actually guards is kept and stated: a transition may not run while a response is being admitted, the waiter is answered exactly once, and both halves complete. The ordering assertion is replaced by an assertion on the result, which is the thing the app can observe. Claude-Session: https://claude.ai/code/session_018pHXqfbxkMQJFzUZ3jW4A8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Консолидированный PR программы Remote Config v2 (android-sdk)
Один PR вместо стека #857–#865. Вся поверхность спит:
@ExperimentalQonversionApiopt-in, активна только при явномsetRemoteConfigV2Config, прод-константы не тронуты:Верификация:
:sdk:test detektAllзелёные многократно (агенты+оркестратор), 429+ тестов, adversarial reviews (пойманы: deadlock главного потока, окно кросс-идентичности, токен в exception, RFC3339Nano). Заменяет и закрывает: #857–#865.