feat: bundle mode with atomic deploys, seed assets, and version-gated caching#482
Open
divyanshub024 wants to merge 2 commits into
Open
feat: bundle mode with atomic deploys, seed assets, and version-gated caching#482divyanshub024 wants to merge 2 commits into
divyanshub024 wants to merge 2 commits into
Conversation
Contributor
📝 WalkthroughWalkthroughAdds bundle mode to ChangesStac package bundle mode
stac_cli atomic bundle deployment
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
- StacBundle/StacBundleConfig models; SharedPreferences bundle store with schema-version guard - StacBundleService: conditional sync (ETag/since), seed-asset hydration (highest version wins), updates stream, offline fallback - StacBundleUpdater: sync on app-resume + optional foreground polling - StacCloud serves screens/themes from the bundle when enabled (opt-in; falls through to legacy per-artifact fetch) - CLI: single atomic POST /bundles with checksum + payload size log, seed asset emission with pubspec warning, --legacy fallback flag; build now cleans stale .build outputs
divyanshub024
force-pushed
the
feat/bundle-deploy
branch
from
11bd5a3 to
3b58309
Compare
divyanshub024
changed the title
![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: 2
🧹 Nitpick comments (2)
packages/stac/lib/src/services/stac_bundle_service.dart (2)
104-167: 🩺 Stability & Availability | 🔵 TrivialNo backoff on repeated sync failures.
On non-2xx/non-304/204 statuses or network errors,
sync()simply returns the stale bundle without recording failure state. Combined withStacBundleUpdater's fixed polling interval and resume-triggered checks, a persistently failing/rate-limiting server will be hit at the same fixed cadence indefinitely. Consider adding lightweight backoff (e.g., skip the next N scheduled checks after consecutive failures) for resilience during outages.🤖 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 `@packages/stac/lib/src/services/stac_bundle_service.dart` around lines 104 - 167, Add lightweight failure backoff to the bundle sync flow so repeated errors do not keep hitting the server at a fixed cadence. Update StacBundleService._syncInternal to record consecutive failures for non-2xx/304/204 responses and caught exceptions, then have StacBundleUpdater skip the next scheduled sync attempts for a short backoff window after repeated failures. Reset the failure state after a successful sync so normal polling resumes.
169-194: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winChecksum is stored but never verified.
_bundleFromResponsecaptureschecksumfrom the response into the persistedStacBundle, but nothing in this file (or the reviewed cohort) compares it against a locally computed hash ofscreens/themesbefore trusting and persisting the payload on a200. If the intent (per the CLI'scomputeBundleChecksum) is end-to-end integrity verification, a corrupted/incomplete-but-valid-JSON response would currently be accepted and cached without detection.If checksum verification is intentionally deferred to the model layer or CLI-only (deploy-time verification), this is fine — otherwise consider validating it here before swapping in the new bundle.
♻️ Possible verification step
final bundle = _bundleFromResponse( Map<String, dynamic>.from(data), projectId: projectId, etagHeader: response.headers.value('etag'), ); if (bundle == null) { Log.w('StacBundleService: Bundle response is missing a version'); return cached; } + if (bundle.checksum != null && + !_matchesChecksum(bundle)) { + Log.w('StacBundleService: Bundle checksum mismatch, discarding'); + return cached; + }Please confirm (or search) whether checksum verification against the bundle payload is implemented elsewhere in the
StacBundlemodel, or whether it's deploy-time-only by design.🤖 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 `@packages/stac/lib/src/services/stac_bundle_service.dart` around lines 169 - 194, The checksum captured in _bundleFromResponse is persisted but never validated against the bundle payload before accepting a 200 response. Check whether StacBundle or any related service already verifies the checksum; if not, add verification in StacBundleService._bundleFromResponse before constructing the StacBundle, comparing the response checksum to a locally computed hash of screens/themes and rejecting mismatches. If checksum validation is intentionally handled elsewhere, document that design clearly in this path.
🤖 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 `@packages/stac_cli/lib/src/services/deploy_service.dart`:
- Around line 137-182: The deploy flow in deploy_service.dart currently writes
the seed asset even when the decoded 2xx response has no version, which can
overwrite a valid bundle with an invalid empty seed. Update the deploy handling
around the response body/version check in the deploy method to validate that
body['version'] is present before calling _writeSeedAsset, and either skip the
write or throw a StacException for malformed success responses; keep the
existing success/info logging and checksum handling in place.
In `@packages/stac/lib/src/framework/stac_service.dart`:
- Around line 202-207: The bundle initialization path in StacService should use
the caller’s dio and validate options up front: pass the same dio used by
StacNetworkService into the bundle sync flow so StacBundleService.sync honors
auth/interceptors/proxy settings, and guard the _bundleConfig.enabled branch so
StacBundleUpdater.start() and prefetch only run when options is present. If
bundle mode is enabled without options, fail fast during initialization with a
clear validation error instead of letting the later _projectId access in
StacBundleService or the updater throw asynchronously.
---
Nitpick comments:
In `@packages/stac/lib/src/services/stac_bundle_service.dart`:
- Around line 104-167: Add lightweight failure backoff to the bundle sync flow
so repeated errors do not keep hitting the server at a fixed cadence. Update
StacBundleService._syncInternal to record consecutive failures for
non-2xx/304/204 responses and caught exceptions, then have StacBundleUpdater
skip the next scheduled sync attempts for a short backoff window after repeated
failures. Reset the failure state after a successful sync so normal polling
resumes.
- Around line 169-194: The checksum captured in _bundleFromResponse is persisted
but never validated against the bundle payload before accepting a 200 response.
Check whether StacBundle or any related service already verifies the checksum;
if not, add verification in StacBundleService._bundleFromResponse before
constructing the StacBundle, comparing the response checksum to a locally
computed hash of screens/themes and rejecting mismatches. If checksum validation
is intentionally handled elsewhere, document that design clearly in this path.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 356aed86-6eb8-47f6-ae9f-429fd5ea4c48
⛔ Files ignored due to path filters (1)
packages/stac_cli/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
packages/stac/lib/src/framework/stac.dartpackages/stac/lib/src/framework/stac_service.dartpackages/stac/lib/src/models/models.dartpackages/stac/lib/src/models/stac_bundle.dartpackages/stac/lib/src/models/stac_bundle.g.dartpackages/stac/lib/src/models/stac_bundle_config.dartpackages/stac/lib/src/services/services.dartpackages/stac/lib/src/services/stac_bundle_service.dartpackages/stac/lib/src/services/stac_bundle_store.dartpackages/stac/lib/src/services/stac_bundle_updater.dartpackages/stac/lib/src/services/stac_cloud.dartpackages/stac/test/models/stac_bundle_test.dartpackages/stac/test/services/stac_bundle_service_test.dartpackages/stac_cli/CHANGELOG.mdpackages/stac_cli/lib/src/commands/deploy_command.dartpackages/stac_cli/lib/src/services/build_service.dartpackages/stac_cli/lib/src/services/deploy_service.dartpackages/stac_cli/pubspec.yamlpackages/stac_cli/test/services/deploy_service_test.dart
Comment on lines
+202
to
+207
| if (_bundleConfig.enabled) { | ||
| StacBundleUpdater.start(); | ||
| if (_bundleConfig.prefetchOnInit && options != null) { | ||
| unawaited(StacBundleService.sync()); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline packages/stac/lib/src/services/stac_bundle_service.dart
rg -n -B2 -A5 'static\s+Dio\s+(get\s+)?dio' packages/stac/lib/src/services/stac_bundle_service.dartRepository: StacDev/stac
Length of output: 220
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## relevant files\n'
git ls-files | rg 'packages/stac/lib/src/.*(stac_service\.dart|bundle|updater|network|service)'
printf '\n## locate bundle service/updater symbols\n'
rg -n 'class StacBundleService|class StacBundleUpdater|static .*dio|dio\s*=' packages/stac/lib/src
printf '\n## inspect initialize block\n'
sed -n '180,240p' packages/stac/lib/src/framework/stac_service.dart
printf '\n## inspect bundle-related implementations\n'
for f in $(git ls-files | rg 'packages/stac/lib/src/.*(bundle|updater|network|service).*\.dart'); do
echo
echo "### $f"
wc -l "$f"
doneRepository: StacDev/stac
Length of output: 5339
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## StacBundleService relevant sections\n'
sed -n '1,120p' packages/stac/lib/src/services/stac_bundle_service.dart
printf '\n---\n'
sed -n '120,240p' packages/stac/lib/src/services/stac_bundle_service.dart
printf '\n---\n'
sed -n '240,330p' packages/stac/lib/src/services/stac_bundle_service.dart
printf '\n## StacBundleUpdater relevant sections\n'
sed -n '1,180p' packages/stac/lib/src/services/stac_bundle_updater.dartRepository: StacDev/stac
Length of output: 13675
Propagate dio to bundle mode, and fail fast when options is missing. packages/stac/lib/src/framework/stac_service.dart:202-207
dio:only reachesStacNetworkService;StacBundleService.sync()still uses its own default client, so bundle requests won’t honor caller auth/interceptors/proxy settings.- When
bundleConfig.enabledis set withoutoptions,StacBundleUpdater.start()still runs and the first resume/poll sync throws from_projectIdoutside thetry, instead of surfacing as an init-time validation error.
🤖 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 `@packages/stac/lib/src/framework/stac_service.dart` around lines 202 - 207,
The bundle initialization path in StacService should use the caller’s dio and
validate options up front: pass the same dio used by StacNetworkService into the
bundle sync flow so StacBundleService.sync honors auth/interceptors/proxy
settings, and guard the _bundleConfig.enabled branch so
StacBundleUpdater.start() and prefetch only run when options is present. If
bundle mode is enabled without options, fail fast during initialization with a
clear validation error instead of letting the later _projectId access in
StacBundleService or the updater throw asynchronously.
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.
What
Adds opt-in bundle mode: instead of fetching every screen/theme with its own request, apps download all artifacts as a single versioned bundle, cache it, and only re-download when the server has a newer version.
Client (
packages/stac)StacBundleConfigonStac.initialize(enabled: falseby default — zero behavior change unless opted in):seedAsset,baseUrl,checkOnResume,pollingInterval.StacBundleService: conditional sync (ETag/If-None-Match+?sincefallback) —304/204keep the cache,200swaps and persists; offline falls back to the cached bundle;updatesstream notifies hosts when a new version lands.StacBundleUpdater: re-checks on app-resume (default on) and on an optional foreground polling interval.CLI (
packages/stac_cli)stac deployis now atomic: one POST with all screens/themes + checksum; any failure exits non-zero with nothing partially applied. Logs artifact counts and payload size.--legacykeeps the old per-file behavior for one release.assets/stac_bundle.json) stamped with the server-authoritative version after every deploy, and warns if pubspec doesn't declare it.stac buildnow cleans stale outputs so deleted screens can't linger in.build.Tests
packages/stac), 8 deploy service tests (packages/stac_cli) — first tests in the CLI package (test: Write tests for stac_cli #459 start).dart analyzeclean in both packages.Notes
/bundlesendpoint (rolling out server-side).Summary by CodeRabbit
--legacyflag to keep the previous upload flow.screens/themes, preventing stale JSON from being redeployed.