ReviewOS

also looking at this

craft-native/craft

Bind the iOS fallback reset assertion to the recovery function

#322
Merged glennmichael123 wants to merge fix/ios-fallback-reset-assertion into main
1 file +15 -2
9 days ago

packages/ios/src/index.test.ts guards #253's fix — that a failed load no longer latches the app onto its bundled copy for the rest of the session. One of its assertions does not guard anything.

expect(swift).toContain('loadedBundledFallback = false')

CraftApp.swift:567 declares the field as private var loadedBundledFallback = false. The declaration contains that exact text, so the assertion passes whether or not the reset inside returnFromBundledFallback exists — and it passed before that recovery existed at all.

Demonstrated

Deleting the real reset at CraftApp.swift:1623 and running the suite on main:

 36 pass
 0 fail

So the #252 bug could return — app stuck on the bundled copy, its API unreachable by relative path for the rest of the session — with a green suite. After this change the same deletion fails:

354 |     expect(recovery).toContain('loadedBundledFallback = false')
Expected to contain: "loadedBundledFallback = false"
Received: "private func returnFromBundledFallback(because reason: String) {\n   guard loadedBundledFallback, ...
 35 pass
 1 fail

and passes with the line restored (36 pass, 209 expect calls).

The fix

The reset is now asserted inside a slice bounded by the recovery function and the method after it, and the reload is checked alongside it.

Android already does exactly this, with the reason recorded next to it — "Bound the assertion to the named recovery method so the field declaration cannot make this pass alone." iOS shipped first and never received the lesson.

Two length checks guard the slice itself, which Android's version does not have: if a boundary marker is renamed, indexOf returns -1 and the slice silently widens. Failing on the slice's size keeps a rename from restoring the hole this replaces.

Found while reviewing whether #254 had inherited the same weakness. It had not — Android is correct, and this is the iOS side of the same guard.

1 changed file on the files tab, with 0 review threads.

Sign in to comment on this pull request.