Draft: Cloud Sync V2 work in progress and build fixes - #2
Draft
Xare123 wants to merge 92 commits into
Draft
Conversation
Draft snapshot so CI can build this work on runners that do not share this
development host's limits. History will be reorganised before this is proposed
for merge.
Build fixes in this snapshot:
CargoKit located Flutter's Gradle plugin by comparing the fully qualified class
name to "FlutterPlugin". Flutter 3.44.8 ships the Kotlin rewrite as
com.flutter.gradle.FlutterPlugin, so the comparison failed, CargoKit skipped
itself, and every Android package was built with no librust_lib_bluebubbles.so
while still reporting success. It now accepts a package-qualified name.
build_verified_alpha.ps1 required lib/arm64-v8a/libapp.so in every mode, but a
debug package carries interpreted Dart in its asset bundle. That entry is now
required only for profile and release.
verify_foundation.ps1 selected an objectbox.dll by presence alone and picked a
stale 4.0.2 library out of a previous build directory, which failed 63 Cloud
Sync tests with an unrelated LateInitializationError. It now derives the host
architecture from dart.exe, reads the pinned version from pubspec.yaml, and
refuses a library that does not match both.
check_sensitive_rust_logs.ps1 exited without setting a status on success, so a
passing scan aborted the verification gate in a fresh shell.
CloudSemanticIdentifierHasher moved to cloud_sync_semantic_identity.rs so the
protector and its dependency-light standalone harness can both link it without
the Apple record parsing stack. Hash domains are unchanged.
Correctness fixes in the CloudKit download path:
An unknown Apple reaction type at or above 2008 indexed past ReactionTypes and
threw a RangeError, aborting the whole message. Unknown types now yield no
reaction row.
Attachment owner parsing used split("_")[2], truncating any message GUID
containing an underscore and throwing on fewer than three segments. One tested
parser in lib/utils/attachment_guid_utils.dart now backs all five former copies
and Attachment.applyFromCloud.
.gitmodules resolves rustpush from the Xare123 fork for the same reason its own
.gitmodules points at the apple-private-apis fork: the icloud-auth rustls
removal that unblocks aarch64-pc-windows-msvc exists only there. Revert both
URLs before proposing this upstream.
…r SDK Two CI lanes failed for reasons unrelated to the code under test. The APK job ran the Cloud Sync suite with no ObjectBox C library on the runner, so 63 ObjectBox-backed tests failed with "Failed to load dynamic library 'libobjectbox.so'". The new step downloads the release matching the objectbox version pinned in pubspec.yaml, verifies a pinned SHA-256 because the upstream download script is unpinned, and exports LD_LIBRARY_PATH. It also fails if the pubspec pin and the installed version drift apart. The Windows ARM64 job installed an x64 Flutter SDK and then tried to swap in an ARM64 Dart SDK by calling Flutter's private bin\internal\update_dart_sdk.ps1. Flutter 3.44.8 no longer ships that script, so the step failed with a command-not-found before any build work. Flutter 3.44 publishes a native Windows ARM64 Dart SDK and engine, so the setup action now requests the architecture matching the runner. The ARM64 Dart and engine assertions are kept so a silent fallback still fails the job.
The previous commit assumed Flutter publishes a native Windows ARM64 SDK archive and asked the setup action for it. It does not: every entry in releases_windows.json reports dart_sdk_arch x64, so the request failed during setup. The original swap approach was right and the real defect was a path. The step resolved bin\internal\update_dart_sdk.ps1 against the action's cache-path output, but the archive unpacks into a single top-level flutter directory, so the script lives one level below that. FLUTTER_ROOT already points at the SDK root, so the step now uses it and asserts the script exists before running, turning a future layout change into a clear failure instead of command-not-found. Verified against the archive CI actually downloads: it contains flutter/bin/internal/update_dart_sdk.ps1 and unpacks to a single flutter directory.
The protected-data logging scan failed with "pwsh: command not found". It is a .ps1 and ubuntu-latest has moved to an image that no longer ships PowerShell. The failure was latent: before the ObjectBox fix the job never reached this step. Pin the APK and bindings jobs to ubuntu-24.04 so an image change cannot drift the gates, which the production-readiness notes already ask for on the bindings job. Also install pwsh when it is absent, guarded so an image that ships it costs nothing, rather than reimplementing a security scan in shell where the two copies could disagree.
…ly produces Two more steps that had never executed, both exposed as earlier fixes unblocked the pipeline. The Android cold-start step ran ./gradlew and exited 127. android/.gitignore excludes /gradlew and gradle-wrapper.jar and upstream tracks neither, so the wrapper does not exist in a fresh checkout. Only gradle-wrapper.properties is tracked, so install exactly the distribution it pins, cross-check the version against that file so the two cannot drift, verify the archive against Gradle's published SHA-256, and invoke gradle directly. The ARM64 job asserted a windows-arm64-release engine directory after "flutter precache --windows". precache fetches only the host debug engine; profile and release engines download on demand during the build, so that assertion could never pass. Confirmed by reproducing the whole sequence on an ARM64 host: the x64 archive plus the Dart SDK swap plus precache yields windows-arm64 next to the archive's x64 engines and no windows-arm64-release. The check now requires a native ARM64 engine and reports which engines are present when it fails, which is what the step was really guarding.
The ARM64 job failed with "Snapshot not compatible with the current VM configuration: the snapshot requires ... x64 windows ... but the VM has ... arm64 windows". flutter_tools.snapshot ships compiled for the archive's x64 Dart VM, and the setup action's cached bin/cache carries a stamp that still matches it, so nothing triggers a rebuild after the Dart SDK swap. Every later flutter command then fails and no ARM64 engine is fetched, which is what the engine assertion reported as "Present: windows-x64, windows-x64-profile, windows-x64-release". Removing the snapshot and its stamp makes the next flutter invocation recompile the tool against the ARM64 VM. Verified on an ARM64 host: after deleting both, flutter prints "Building flutter tool" and produces a 42,879,680-byte snapshot where the shipped x64 one is 44,665,376, then runs normally. The snapshot is a regenerable cache artifact, so the only cost when a rebuild was unnecessary is the compile time.
The manual shadow sampler could never reach the network. The interlock acquires a coordinator lease on a sentinel fence scope, writes it as a durable row, then runs the guarded work while that row is live. The guarded work's first act is a preflight whose lease probe read every unexpired lease in the box, found the interlock's own fence, and threw coordinator_active. No network call, no report file, every time. The probe now ignores the fence scope key. A mutual-exclusion fence is not another sync coordinator, and an operation must not observe itself as one. Every other scope, including an unrecognized one, still blocks. This was never caught because the probe and the interlock were only ever tested apart. The probe's existing test inserts an arbitrary-scope lease and still passes unchanged; two new tests cover the fence scope alone and a real coordinator lease sitting alongside it.
Adds a developer-only beta APK built with the Cloud Sync V2 sampler compile gate enabled, so the first live read-only run does not depend on a developer workstation. The beta flavour is a separate applicationId and installs beside alpha without touching its database. Debug rather than release: the beta release config needs a signing keystore, and the shadow report has no in-app export, so it must be pulled with adb run-as, which a release build does not permit. The APK is checked for lib/arm64-v8a/librust_lib_bluebubbles.so before upload. CargoKit silently skipped the Rust build on Flutter 3.44.8 until the plugin detection fix, producing packages that installed and could not work while the build still reported success. This makes that failure mode loud.
The APK job now spends most of its time in CargoKit. Every flavour and build type pays a separate Rust and vendored-OpenSSL compile, and the beta sampler build sat at the end of that chain behind the full test suite and both alpha APKs, so it was over an hour from being produced. Moving it to its own job lets it build in parallel. Setup mirrors the APK job, including the Fairplay fixtures the Rust build needs, which the first version of this job omitted. The added time in the APK job is left alone. It is the cost of those packages containing a native library at all; before the CargoKit plugin-detection fix they were uploaded without one.
The rollout plan describes phases and the gate documents describe conditions, but nothing stated in order what is left and which parts cannot be finished by writing code. Separates the remainder into code work, work that requires live Apple access, and work blocked by licensing or missing hardware. Records the measured state as of today rather than repeating stale claims, and names the one open question that would reorder the plan: whether messageUpdateZone and recoverableMessageDeleteZone carry edits and recoverable deletes, which would put zone coverage ahead of semantic apply.
Both parsers required a `p:<part>/<guid>` wrapper. Apple omits the wrapper entirely when a reaction targets no particular part, so the value is a bare GUID, and the converter turned that parse failure into a hard quarantine. Every partless reaction was therefore dropped, with a MalformedParent reason that reads as corrupt data rather than a parser gap. A bare value must carry no structure of its own. The identifier validators only reject empty, oversized, and NUL-bearing values, so without an explicit check a malformed wrapper such as `0/<guid>` or `bp:0/<guid>` would be accepted whole as the parent identifier. The two parsers also disagreed on leading zeros: the native side rejected them and Dart normalised them, so `p:0003/<guid>` converted on one side of the bridge and quarantined on the other. Dart now rejects them too. parent_part becomes optional through ParsedAssociatedParent and CloudCanonicalParentReference. Tests that asserted the bare form was invalid, and that `p:0003/` normalised to part 3, asserted the wrong grammar and are corrected.
CloudKit distinguishes "present but empty" from a field being absent using its own wire type, and the V2 merge contract depends on that distinction. The presence extractor collapsed every valued field to PresentWithValue and never read the type tag, so the evidence was destroyed at the transport boundary. The practical consequence was that the first live fetch could not answer whether Apple emits EMPTY_LIST at all, which is the question the tri-state exists to settle. The prior art cannot answer it either: the vendored client encodes an empty collection as an omitted field and decodes EMPTY_LIST identically to an empty list, so it can neither emit nor detect the distinction. Recorded as evidence only. No conversion decision reads it. Whether an empty list means "clear" is exactly what a live run has to establish, and inferring a meaning here would repeat the existing mistake of inferring one from an empty message-summary dictionary.
A present-but-empty ec, ep, otr, or rp in the message summary was converted to ExplicitClear, meaning "discard every edit" and "un-retract every part". Apple's summary plist omits empty collections rather than sending them, so that shape carries no instruction, and acting on it would wipe local edit history and retraction state on a guess. It also applied a weaker standard than the rest of this file. A record-level clear requires an authoritative marker from a pre-typed decoder and errors with ExplicitClearWithoutPresence otherwise, but the summary path inferred one straight from dictionary key presence. Empty collections now read as Absent. attributedBody is left alone: it is protobuf optional bytes, where present-and-empty is genuinely distinguishable from absent, so a clear there is well founded. reaction_cannot_smuggle_edit_or_retraction_state was passing only because the all-empty fixture produced that fabricated clear, so it never exercised smuggled state. It now carries a real edit with a matching edited-part index, and still quarantines.
sync_directory opened the directory read-only with backup semantics and called sync_all, which issues FlushFileBuffers. That call requires write access and is not supported on a directory handle, so on Windows it always failed and took every enclosing store operation with it. Eighteen of the lease, recovery, and garbage-collection tests failed on this host for that single reason, each reporting an opaque Io because the underlying error is discarded by map_err. With this fixed the cloud_sync Rust suite is 100 passed, 0 failed, where it was 81 passed, 19 failed. Windows offers no directory-sync primitive, so a no-op is the honest implementation rather than pretending to a guarantee the platform does not provide. Durability of a rename on Windows rests on startup reconciliation, which treats the database as authoritative and repairs the staging directory against it. This was pre-existing and unrelated to the canonical-converter work; it was confirmed against a stashed tree before being changed.
The open-source review states the rule that every borrowed idea must be recorded with its project, licence, whether code or only a concept was taken, and the implementing file. It held a per-project table but none of those entries. The ledger adds them, covering the protocol facts this work took from the SSPL-licensed rustpush submodule. The distinction the ledger turns on is that a field name, a wire type, a zone name, and an identifier grammar are facts about Apple's protocol, while struct definitions, derive macros, and .proto files are expression carrying their project's licence. Every entry states which was taken. Two entries name no implementing file because the fact prevented work rather than causing it. The ObjectBox record exists because Cloud Sync V2 is about to make that store the durability boundary for reconciled message data. It also corrects a claim worth not inheriting: published warnings concern the archived community Rust binding, and this repository has no Rust dependency on ObjectBox at all. The real exposure is narrower, that the native library is proprietary while the bindings are Apache-2.0, and it is already present rather than introduced here. Both note that the pending redistribution review does not currently list ObjectBox alongside libmpv and FFmpeg, and that the repository has a LICENSE but no third-party notices file.
applyFromCloud stored proto1.associatedMessageGuid verbatim. Apple sends that field as `p:<part>/<guid>`, so the stored value never matched Message_.guid, and the lookup in Message.handleAssociatedMessages compares it directly against that column. Every reaction arriving over CloudKit was therefore left permanently orphaned from its parent. The part was separately derived from the reaction's own attributed body by matching the associated range. The canonical mapping forbids this: the range is validation evidence, not the source of the part. Both now come from CloudAssociatedMessageParentReference, which already handles the wrapped form, the bare-GUID partless form Apple uses when no part is targeted, and rejects structured values that are not either. That parser existed and was tested but had no caller outside its own test. An unparseable parent leaves the reaction unassociated rather than aborting the download, matching the surrounding fail-open behaviour for unknown reaction types. Note the equivalent JSON path in fromMap already parsed this correctly, so the two ingestion paths disagreed.
Field ownership is the prerequisite for the semantic adapter. Without it, "the server wins" gets implemented field by field from memory, and the first time that is wrong the symptom is a user's local state being silently overwritten, discovered from a bug report rather than a test. Classes are server-immutable, server-mutable, and device-owned, each with its merge rule. The server-owned set was derived from what applyFromCloud actually writes rather than from what the schema could carry. Two existing locks, lockChatName and lockChatIcon, are called out as the one place a device decision outranks server authority. It also records why a per-row sequence number is unnecessary: the checkpoint, inbox status, and projected rows already commit in one ObjectBox transaction, so exactly-once follows from the transaction rather than a version column. The residual risk is ordering within the server-mutable set, which the monotonic rules cover, and field ownership already partitions by zone. Separately, _advanceContiguousApplied stops at the first inbox row that is not applied, so a quarantined row blocks the checkpoint permanently. That is deliberate, but unbounded: the journal fills, fetching stops, and if the stall outlasts the undocumented change-token lifetime the cost is a full zone re-bootstrap. Both options are written down rather than defaulted into, since this is dormant Phase 2 code and changing the safety model unilaterally would be the wrong call.
Both new parsers rejected `bp:<part>/<guid>`. That is a real shape: this app's own JSON ingestion path in Message.fromMap has always stripped the `bp:` prefix, in both the io and html variants. Rejecting it was a regression introduced with the bare-GUID work, and a worse failure than the bug it sat beside. Dart stored the association as null, which is unrecoverable, and Rust quarantined the whole record. Both parsers now accept it; `bpdi:`, the balloon payload reference, stays rejected. Tests that asserted `bp:` must be rejected encoded the gap as intended behaviour and are corrected. The CI changes matter more than any single fix in this branch. cargo check --lib does not compile #[cfg(test)] code, and it was the only cargo invocation in any workflow. Every Rust test here was unenforced: the canonical converter, the DTO parsers, the protected store, the identity hasher. The 100-passing figure quoted in recent commits was a local result that no build could ever contradict. Both the library suite and the protector harness now run. The Dart job enumerated individual test files, and that list had drifted badly: eleven test files were executed by no workflow at all, including the only coverage for the attachment-GUID parser and the reaction-type mapper, both rewritten in this branch. It now runs the whole suite, ordered after the ObjectBox install the Cloud Sync tests depend on.
Upload could not emit the partless parent. "p:$associatedMessagePart/..." interpolated a null part into the literal "p:null/<guid>", a shape Apple never sends and which both of this app's parsers now reject, so a round trip through the uploader could not be read back. It emits the bare GUID instead. Partless and part-zero parents are now pinned by a test rather than left implicit. They share a parent hash, because the hash covers only the GUID, but remain distinct payload values. That is the truthful representation: for a single-part message the two may mean the same thing, but for a multi-part message "the message" and "its first part" are different targets and nothing available offline settles which Apple intends. Dart keeps the same distinction in storage and only coerces when matching for display. The EMPTY_LIST accessors stay unwired on purpose. Surfacing them means adding a transient DTO field, and that DTO should be widened once and deliberately for semantic apply rather than regenerated for one diagnostic. They are marked pending, the name list is now sorted so a future report cannot vary between runs, and carrying the observation across is recorded as a required item for that widening. The roadmap also carried my own error forward, claiming bp/bpdi handling should be removed from the record layer. Corrected, with the reason: that finding was about field names in the IDS payload and said nothing about the bp: prefix inside associatedMessageGuid, which is a real shape.
actions/setup-java v1 through v4 are deprecated and the runner now warns on every run. Bump both jobs to v5. The java-version and distribution inputs are unchanged, so this is a version bump and nothing else; the JDK is still Temurin 21. actions-rs/toolchain is a harder problem than a warning. The actions-rs organisation was archived in October 2023, so that action is pinned to a Node runtime GitHub is actively retiring and will not be updated when it breaks. bridge-bindings.yml already uses dtolnay/rust-toolchain, which is the maintained replacement, so this only brings build.yml in line with a choice this repository had already made. dtolnay encodes the toolchain in the ref rather than an input, which is why the `with: toolchain: stable` block goes away without losing the pin.
Message.save() and bulkSave() caught UniqueViolationException and discarded it. save() is the one that matters: on that path `id` is left null and the message is returned as though it had been persisted, so no caller can distinguish a saved message from a dropped one. That is the exact mechanism by which legacy and Cloud Sync V2 coexistence would lose a message silently rather than loudly, and the first live sampler run depends on its own report being truthful about what was written. Control flow is unchanged, because callers rely on save() not throwing. Only the silence goes away. replaceMessage already logs this same constraint violation as an error a hundred lines below, so these two sites were the outliers against the file's own convention rather than a new policy being introduced here. Verified: 394 Dart tests pass, analyzer reports no new issues.
I previously reported that conversion failures "retry forever with no dead-letter path". Reading the driver rather than the catch block, that was overstated. CloudMessageUploadBatchResult.madeProgress is false when nothing converted in a pass, and rustpush_service breaks out of the upload loop and logs how many messages stayed queued. A wholly unconvertible backlog terminates; it does not spin. The genuine gap is narrower and worth stating accurately. Nothing persists the fact that a given message has failed conversion, so a permanently unconvertible message is invisible across sessions, and a pass that converts some messages but not others never trips the break, so the stuck ones ride along unnoticed. Closing that needs a durable per-message attempt count, which is a schema change and is not worth making ahead of the first live run. The read-only sampler does not exercise this direction at all. Also records why this branch diverges from upstream here. Upstream sets ckSyncState = true in a `finally`, so a message that fails to convert is marked crawled and silently never reaches CloudKit. This branch keeps it unsynced instead. That is a deliberate trade, not an accident, and a reviewer would otherwise have to infer it from the diff.
MentionTextEditingController.notifyListeners() ran on every change to the message field and logged both the previous and current full text. On the test device this produced roughly a megabyte of log per day and wrote every character typed into a message, including its final content, to a plaintext file under app_flutter/logs. Two costs, one of which is not obvious. The volume rotates real diagnostics out of the retained logs, which is how it was found: the composer chatter crowded out everything else while looking for the cause of an unrelated bug. The other is that message text lands on disk in the clear for anyone who can read the app's files. The line is a leftover debug print, tagged "a", with no caller. The Caret diff and Invalid changed annotations logs immediately below it are deliberate and stay.
The earlier fix taught CargoKit to recognise Flutter 3.44.8's Kotlin plugin, which it had been failing to find. That was correct as far as it went, and it turned a silent skip into a hang: once CargoKit finds the plugin it immediately calls Groovy-era APIs that the Kotlin rewrite no longer has. plugin.project -> now `private var project: Project? = null` plugin.getTargetPlatforms -> removed, moved to FlutterPluginUtils Both are called from inside `applicationVariants.all`, so they throw once per variant. This project has five flavours across two build types, so Gradle collected roughly ten failures and handed them to MultipleBuildFailuresExceptionAnalyser. DefaultFailureFactory then recurses over the combined cause graph, constructing a Throwable and filling in a stack trace at every node, and never returns. The build had already failed; Gradle simply could not finish rendering the error. That is why no output ever appeared. A cancelled CI run shows the last log line at 17:35 and then a 302 minute gap of complete silence. Locally it pins one core at 100% with no cargo or rustc process anywhere, and three separate thread dumps land in the same frame. The fix stops holding the plugin instance, which was only ever used to reach its project, and resolves that project directly during the search. Target platforms are read from the -Ptarget-platform property, which is what FlutterPluginUtils.getTargetPlatforms does, with the same default list when the property is absent. `print` also becomes `println`, because the un-terminated line left the skip message glued to whatever Gradle logged next. Verified: :app:assembleAlphaDebug now completes in about 90 seconds rather than hanging indefinitely, and the CargoKit task registers and runs. It currently fails on a separate Windows path resolution error, "The system cannot find the path specified", which is visible and does not block the build. That is a different bug and is not fixed here.
The beta sampler APK installed and then hung on launch. The visible error was a path_provider channel failure inside FilesystemService.init, which names nothing about the real cause. The real cause is two packages up the chain. irondash_engine_context and super_native_extensions each vendor their own copy of CargoKit, and neither can see Flutter 3.44.8's Kotlin plugin, so their Rust is never built and their native libraries are absent from the APK. At startup IrondashEngineContextPlugin's static initializer calls System.loadLibrary and throws UnsatisfiedLinkError. That throw happens inside GeneratedPluginRegistrant.registerWith, so registration aborts for EVERY plugin, not just that one. path_provider is simply the first casualty loud enough to notice. Fixing the class-name match alone is not sufficient and is worse than doing nothing: once CargoKit finds the Kotlin plugin it calls plugin.project, now private, and plugin.getTargetPlatforms(), now moved to FlutterPluginUtils. Both throw once per variant, and Gradle 8.9 wedges indefinitely trying to format the resulting pile of failures. The patch therefore also stops reaching through the plugin instance and resolves the Flutter Project directly, matching the fix already applied to rust_builder. These copies live in the pub cache and are restored by pub get, so a committed edit cannot reach them. The patch runs as a build step instead. It is idempotent and re-running reports the files as already current. The beta APK check now also asserts libirondash_engine_context_native.so is present. Its absence does not fail any build; it produces a package that installs cleanly and dies on launch, so only an explicit assertion catches it.
GeneratedPluginRegistrant registers two CargoKit-backed plugins, IrondashEngineContextPlugin and SuperNativeExtensionsPlugin, and the app loads its own bridge on top of that. The previous assertion covered only the first of those because it was written from the crash that happened to surface first. Checking the whole set matters more than usual here. A missing entry fails no build at all. It produces an APK that installs cleanly and dies on launch, because System.loadLibrary throws from a static initializer inside GeneratedPluginRegistrant.registerWith and takes every other plugin down with it, surfacing as an unrelated channel error. libdartjni.so and libsqlite3.so are deliberately absent from this list. Both turn up in a pub-cache-wide scan for System.loadLibrary, but jni and sqlite3_flutter_libs are not dependencies of this app; they are unrelated packages sharing the cache. pubspec.lock and the registrant are the authority, not the cache.
The step failed with "Permission denied" and exit 126. The script was committed from a Windows host, where git records mode 100644 regardless of the local chmod, so the checked-out file was not executable. Fixed twice over, deliberately. The mode is now 100755 in the index, and the workflow invokes the script through `bash` rather than relying on the exec bit. The second guard matters because this repository is developed on Windows: any future edit that recreates the file will silently drop the mode again, and a build step whose only protection is a bit that the dev platform cannot represent will keep breaking.
A message with several links rendered the first preview correctly and the rest as empty grey boxes. Three separate causes, all of which assumed one link per message. MetadataHelper.fetchMetadata always resolved `message.url`, which is the first URL in the whole message text, so every preview in the message fetched the same page. Its cache was keyed by message GUID alone, so all the previews in one message shared a single entry and raced for it. And LegacyUrlPreview derived its site label from `Uri.tryParse(message.text)`, which is not a parseable URI once the text holds more than one link, so the host came back null and the card had nothing to show. fetchMetadata now takes the URL it should resolve and keys the cache on message and URL together. LegacyUrlPreview takes the URL it is previewing and uses it for both the label and the tap target. InteractiveHolder already knew which MessagePart it was drawing, so it passes part.url. message.metadata stays reserved for the message's primary URL. It is a single blob and cannot describe more than one link, so letting every preview write it meant whichever link resolved last overwrote the others and all of them then rendered from that one on reload. The tap target is also parsed with tryParse instead of Uri.parse, since the previous version would throw on a message whose text is not a URI. Verified: 394 Dart tests pass, analyzer clean on all three files. Not yet confirmed against a real multi-link message on the device.
A link straight to a .jpg has no HTML metadata to scrape, so the image fallback is the only thing that can give it a preview. It could never fire. The condition tested data.url, which is not assigned until twenty lines further down, so it was still null at that point and the whole check short-circuited. Bare image links therefore rendered as empty cards. The pattern now matches the URL that was actually requested. Two smaller faults went with it. It was anchored with $ against the full URL, so any query string defeated it, and photo.jpg?size=large did not count as an image; it now matches against the path. And the dots were unescaped, so `.jpg` matched any character before "jpg" and something ending "Xjpg" counted as an image. webp, heic and bmp are added to the list, and the match is now case-insensitive, since a URL ending .JPG is not unusual. Verified: 394 Dart tests pass, analyzer clean.
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.
This draft is a CI and review snapshot. It is not ready to merge. Commit history and upstream-facing scope still need to be split into reviewable PRs.
Current scope
This branch combines production-path fixes, Cloud Sync V2 safety scaffolding, Windows ARM64 enablement, Android native-library delivery checks, and FaceTime diagnostics/tests. It exists on the fork so the complete matrix can be exercised before any upstream proposal.
High-impact fixes
Android and Windows builds
com.flutter.gradle.FlutterPlugin. Before this change, Android builds could report success while omittinglibrust_lib_bluebubbles.so.ring 0.16.20and blocked Windows ARM64 Rust builds.actions/setup-java@v5.Existing CloudKit path
p:<part>/<guid>,bp:<part>/<guid>, and bare parent references instead of storing an orphaned raw value.p:null/<guid>references.RangeError.UniqueViolationExceptionmessage-save failures are now observable.Cloud Sync V2 safety
FaceTime reliability and diagnostics
Verified on head
d88f4383dCI runs:
Downloaded Beta artifact verification:
com.bluebubbles.messaging.beta0C68FA27ACF275D49F012C3283EB0A9FD166D82DE6CAFE6395CB2963A1C56129lib/arm64-v8a/libflutter.so,libobjectbox-jni.so, andlibrust_lib_bluebubbles.soLive validation still required
Automated tests cannot prove Apple's private production service will accept the full exchange. Before calling FaceTime fixed, test exactly one outgoing and one incoming call while recording these boundaries: WebView load, patch installation, permissions, correlated admission, ICE connected/completed, live remote audio/video tracks, and increasing media bytes. Stop at the first failed boundary instead of repeating blind calls.
Cloud Sync V2 remains gated on a read-only sampler run, reconciliation review, controlled test-account validation, and a soak. It must not be enabled against real message history yet.
Fork and upstream constraints
.gitmodulescurrently points at theXare123/rustpushfork, whose nestedapple-private-apisdependency also points at the fork. Those URLs and commit dependencies must be handled deliberately when the work is split for upstream review.Companion branches:
Xare123/rustpush@agent/cloud-sync-v2-draftXare123/apple-private-apis@agent/cloud-sync-v2-draftAndroid release signing is not configured. The validated artifacts are debug/profile test packages. The official ANGLE native-media build remains a separately gated licensing/provenance task.
Non-blocking CI warnings remain for
actions/checkout@v4andactions/upload-artifact@v4being forced from Node 20 to Node 24, plus Android's deprecatedonSurfaceDestroyed()callback. They should be handled in narrow follow-up changes rather than mixed into this draft.