feat: ENG-1123: setup() + command queue foundation#2
Merged
Conversation
pandeymangg
changed the title
pandeymangg
changed the title
Contributor
WalkthroughThis pull request implements the complete Formbricks Flutter SDK initialization infrastructure. It introduces a serialized command queue for operation ordering, persistent configuration management backed by SharedPreferences, HTTP API communication with error normalization, and lifecycle-aware expiry checking. The setup() function orchestrates workspace validation, caches config with error-cooldown semantics, and optionally starts an expiry ticker. A public Formbricks facade exposes setup() and debugStoredConfig() via the command queue. The playground app now reads environment variables and calls Formbricks.setup on launch, displaying status. Dependencies (http, shared_preferences, clock, mocktail, fake_async) are added, and the entire implementation is covered with 40+ tests verifying type safety, persistence, error handling, and lifecycle behavior. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
README.md (1)
8-10:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale status note —
welcome()no longer exists.This PR replaces the placeholder
welcome()with theFormbricks.setupfacade, so the "currently exposes only a placeholderwelcome()" status is now inaccurate. Update this note (and consider marking thesetup+ command queue roadmap row done at Line 183) to reflect the new public surface.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 8 - 10, Update the README status note to remove the obsolete reference to the placeholder welcome() and instead mention the new public facade Formbricks.setup as the current exposed API; also update the Roadmap row about "setup + command queue" to marked done/completed to reflect that Formbricks.setup and the command queue implementation exist now. Ensure the phrasing replaces "welcome()" with "Formbricks.setup" and adjusts any tense/visibility language so the public surface description is accurate.tool/run.sh (1)
70-71:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix Android
adb shellboot-wait portability by removing Bash[[ ... ]].
tool/run.shstill containswhile [[ -z $(getprop sys.boot_completed) ]]; ...inside the remoteadb shellcommand (lines 70-71). Android’s default/system/bin/shwon’t parse[[ ... ]], and the syntax failure is masked by|| true.Suggested fix
- "$adb" -s "$(booted_serial)" wait-for-device shell \ - 'while [[ -z $(getprop sys.boot_completed) ]]; do sleep 1; done' 2>/dev/null || true + "$adb" -s "$(booted_serial)" wait-for-device shell \ + 'while [ "$(getprop sys.boot_completed)" != "1" ]; do sleep 1; done' 2>/dev/null || true🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tool/run.sh` around lines 70 - 71, The remote adb shell loop uses Bash-specific [[ ... ]] which /system/bin/sh doesn't support; update the command invoked in tool/run.sh (the "$adb" -s "$(booted_serial)" wait-for-device shell ... line) to use POSIX test syntax (for example: while [ -z "$(getprop sys.boot_completed)" ]; do sleep 1; done) so the loop runs on Android's default sh; ensure quoting around $(getprop ...) is present and remove or keep the trailing || true as appropriate for error handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/FLUTTER_SDK_PLAN.md`:
- Line 310: The fenced code block showing the project tree under
"formbricks_flutter/" lacks a language identifier which triggers markdownlint
MD040; update the opening fence from ``` to ```text (i.e., add the "text"
language identifier) for that code block so the block becomes ```text ... ```
while keeping the existing closing backticks unchanged.
In `@packages/formbricks_flutter/lib/src/common/api_client.dart`:
- Line 114: Replace the non-testable DateTime.now() used in the cache-busting
query param of the environment endpoint string
('/api/v2/client/$workspaceId/environment?rand=${DateTime.now().millisecondsSinceEpoch}')
with clock.now().millisecondsSinceEpoch and add the package:clock import (import
'package:clock/clock.dart') at the top of the file so the call is mockable in
tests; update the exact string expression in api_client.dart where that URL is
built.
In `@packages/formbricks_flutter/lib/src/common/logger.dart`:
- Around line 29-31: The configure method currently no-ops when level is null,
leaving a previously set verbosity sticky; update the static
configure({LogLevel? level}) in logger.dart so that when level is null it resets
_instance._level to the library’s default log level (e.g. LogLevel.info) instead
of doing nothing — change the branch that checks level to assign either the
provided level or the default to _instance._level (referencing configure,
_instance._level, and LogLevel).
In `@packages/formbricks_flutter/lib/src/common/setup.dart`:
- Around line 98-143: The ApiClient created as api must be closed on all
error/early-return paths to avoid leaking the underlying http.Client; modify the
setup flow so that ApiClient.close() is called if _syncExistingConfig(config,
api, existing) returns an Err or if api.getWorkspaceState() returns an Err
(before returning or throwing in _handleErrorOnFirstSetup), and ensure it's also
closed in the fresh-setup success/error branch and before returning when
startTicker is false—use a try/finally around the code that uses api (including
calls to _syncExistingConfig and getWorkspaceState) or explicitly call
api.close() in the Err branches; reference ApiClient, _syncExistingConfig,
getWorkspaceState, and _handleErrorOnFirstSetup to locate the relevant spots.
In `@packages/formbricks_flutter/lib/src/types/config.dart`:
- Around line 133-137: The copyWith implementations (e.g. TUserState.copyWith)
use "param ?? this.field" so callers cannot clear fields by passing null; change
these to use a sentinel default value (e.g. a private const _unset = Object())
and make parameters default to that sentinel (e.g. Object? expiresAt = _unset,
Object? data = _unset), then inside copyWith check "expiresAt == _unset ?
this.expiresAt : expiresAt as DateTime?" and similarly for data; apply the same
sentinel pattern to the other copyWith methods that touch workspace,
workspaceId, and appUrl so callers can pass null to explicitly clear those
nullable fields.
In `@packages/formbricks_flutter/README.md`:
- Around line 31-32: The README text is ambiguous about whether setup returns a
Result or throws an exception; update the wording around the setup contract to
consistently state the chosen error handling approach (either "setup returns
Result with Err(FormbricksSetupError)" or "setup throws FormbricksSetupError"),
and make related lines mention the same behavior (including the 10-minute
cooldown) while aligning examples (the earlier Err(...) example or try/catch) to
use the same contract; reference the setup function name and the
FormbricksSetupError type and update any sample code and sentence that mentions
Err(...) or throws to match the single chosen contract.
---
Outside diff comments:
In `@README.md`:
- Around line 8-10: Update the README status note to remove the obsolete
reference to the placeholder welcome() and instead mention the new public facade
Formbricks.setup as the current exposed API; also update the Roadmap row about
"setup + command queue" to marked done/completed to reflect that
Formbricks.setup and the command queue implementation exist now. Ensure the
phrasing replaces "welcome()" with "Formbricks.setup" and adjusts any
tense/visibility language so the public surface description is accurate.
In `@tool/run.sh`:
- Around line 70-71: The remote adb shell loop uses Bash-specific [[ ... ]]
which /system/bin/sh doesn't support; update the command invoked in tool/run.sh
(the "$adb" -s "$(booted_serial)" wait-for-device shell ... line) to use POSIX
test syntax (for example: while [ -z "$(getprop sys.boot_completed)" ]; do sleep
1; done) so the loop runs on Android's default sh; ensure quoting around
$(getprop ...) is present and remove or keep the trailing || true as appropriate
for error handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0cd4cd1c-f508-4fbc-9fe8-13d96fce2d66
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (30)
.gitignoreREADME.mdapps/playground/.env.exampleapps/playground/README.mdapps/playground/ios/Runner/Assets.xcassets/LaunchImage.imageset/README.mdapps/playground/lib/main.dartdocs/FLUTTER_SDK_PLAN.mdpackages/formbricks_flutter/README.mdpackages/formbricks_flutter/lib/formbricks_flutter.dartpackages/formbricks_flutter/lib/src/common/api_client.dartpackages/formbricks_flutter/lib/src/common/command_queue.dartpackages/formbricks_flutter/lib/src/common/config.dartpackages/formbricks_flutter/lib/src/common/expiry_ticker.dartpackages/formbricks_flutter/lib/src/common/logger.dartpackages/formbricks_flutter/lib/src/common/result.dartpackages/formbricks_flutter/lib/src/common/setup.dartpackages/formbricks_flutter/lib/src/common/time.dartpackages/formbricks_flutter/lib/src/types/config.dartpackages/formbricks_flutter/lib/src/types/errors.dartpackages/formbricks_flutter/pubspec.yamlpackages/formbricks_flutter/test/common/api_client_test.dartpackages/formbricks_flutter/test/common/command_queue_test.dartpackages/formbricks_flutter/test/common/config_test.dartpackages/formbricks_flutter/test/common/expiry_ticker_test.dartpackages/formbricks_flutter/test/common/result_test.dartpackages/formbricks_flutter/test/common/setup_test.dartpackages/formbricks_flutter/test/formbricks_flutter_test.dartpackages/formbricks_flutter/test/types/config_test.dartpackages/formbricks_flutter/test/types/errors_test.darttool/run.sh
itsjavi
requested changes
Jun 3, 2026
Member
itsjavi
left a comment
itsjavi
left a comment
There was a problem hiding this comment.
thank you @pandeymangg tests and build passes locally,
I left a couple of remarks that would be good to consider.
|
itsjavi
approved these changes
Jun 4, 2026
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
2 participants
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.
What
Lands the four foundations the rest of the Flutter SDK builds on (ENG-1123):
FormbricksConfig—SharedPreferences-backed singleton (keyformbricks-flutter); init-once, synchronousget(), awaitedupdate()/reset().ApiClient— thinpackage:httpwrapper for the two client endpoints, returning typedResult<T, ApiErrorResponse>.Formbricks.setup({appUrl, workspaceId})— validates input, hydrates cached config, syncs workspace (+ user when one is cached and expired), installs a lifecycle-aware expiry ticker, marks setup complete.CommandQueue— FIFO single-in-flight worker so the public API can't race; eachaddreturns its ownFuture.Plus the supporting
Result<T,E>sealed type, error hierarchy, config models, and aLogger. The playground demo now callsFormbricks.setupon launch (credentials via--dart-define/ a local.env) and shows the result.welcome()is removed — the real API replaces it.Deliberate divergences from the RN SDK
environmentIdalias / legacy migration —workspaceIdonly.get()after (RN re-read storage on every access).update()awaits the disk write before completing (RN fire-and-forget could lose state on crash).paused/inactive, immediate check onresumed.clock.now()everywhere (neverDateTime.now()) so expiry logic is deterministic in tests._kErrorCooldownconstant instead of an inline10 * 60000.Loggerusesdart:developer log(), notprint(avoid_printis on).Source → test mapping
lib/src/common/result.darttest/common/result_test.dartlib/src/common/command_queue.darttest/common/command_queue_test.dartlib/src/common/config.darttest/common/config_test.dartlib/src/common/api_client.darttest/common/api_client_test.dartlib/src/common/setup.darttest/common/setup_test.dartlib/src/common/expiry_ticker.darttest/common/expiry_ticker_test.dartlib/src/types/config.darttest/types/config_test.dartlib/src/types/errors.darttest/types/errors_test.dartlib/formbricks_flutter.dart(facade)test/formbricks_flutter_test.dartlib/src/common/logger.dartconfig_test/setup_test(no dedicated file)lib/src/common/time.dartconfig_test/setup_test/expiry_ticker_testVerification
dart analyze --fatal-infos .→ clean.melos run format-check→ clean.flutter test→ all green (SDK + playground).FormbricksError(network_error), proving the full path (queue → ApiClient → http → error-state persisted). With real staging creds it shows "setup complete ✓".Out of scope (follow-ups)
filterSurveys/displayPercentage(thefilteredSurveysfield stays[]),track/ identify /UpdateQueue, the survey WebView +SurveyStore(ENG-1127), GitHub Actions CI (ENG-1125), SonarQube (ENG-1124), encrypted-storage opt-in.🤖 Generated with Claude Code