ReviewOS

also looking at this

craft-native/craft

fix(ios): answer the shapes the spec answers, now that a bare false survives

#105
Merged chris wants to merge fix/ios-reply-shape-parity into main
7 files +423 -82
11 days ago

An audit of all 33 mobile modules compared every .live action's reply against the Swift arm the seam (#103) now displaces: 80 actions, 61 observable differences, each confirmed by two independent refuters. Most are Zig being stricter — refusing embedded NULs, validating filenames, declining overlapping picker calls — and those stay. This PR is the ones where a page written against the spec gets a different answer.

The root cause, first

craft-bridge.js settled with payload || {}, which turns a truthful false into a truthy {}. Five handlers wrapped their booleans in objects specifically to survive that, and said so in their comments:

The reply is an object, not the bare true Swift resolves: craft-bridge.js settles with payload || {}, so any falsy scalar arrives at the page as {} and carries nothing.

So the workaround was load-bearing, and the shapes could not be fixed without fixing the bug underneath. _orEmpty now substitutes {} only for a genuinely absent payload; false, 0 and "" reach the page intact.

Then the five shapes

actionwasnow (= spec)
openURL{"opened":bool}true / false
share{"completed":bool}true / false
requestReview{"requested":true}true
setBadge / clearBadge{"count":N}true
setKeepAwake{"enabled":bool}true / false

share is the one that mattered. It replied {"completed":false} when the user dismissed the sheet — and every page doing if (await craft.share(...)) read that object as success.

registerSiriShortcut and removeSiriShortcut never worked

Not a regression from the seam. A duplicate key in the page's own JS:

postMessage({action: 'registerSiriShortcut', phrase: phrase, action: action, callbackId: id})

Two keys called action; the later one wins. Every call arrived labelled with the shortcut's identifier instead of the bridge action, no switch arm matched, nothing answered — and these two wrappers park in _callbacks with no timeout, so the promise never settled at all. The payload field is shortcutAction now, in the wrapper, the Swift arms and the Zig reader.

Two behaviour regressions

  • recognizeText rejected the whole call when any observation had no top candidate. The spec skips that observation, and this handler's own doc comment described the skip while the code did the opposite. It skips now; the separator is keyed off what was written rather than the loop index, so a skip cannot leave a doubled comma.
  • saveHealthWorkout resolved {"id":…,"routeId":""} when a route stage failed — the same shape a successful finish with a nil route produces. A page could not tell "saved with no route id" from "the route did not save", and would file the workout as complete with its locations silently missing. Both stages reject now, as the spec does, and log the workout id rather than fabricating a success around it.

And the one that was going to be a separate PR

setBadge / clearBadge now request badge authorization first and reject with PERMISSION_DENIED when iOS refuses — the spec's "Badge permission denied". It was skipped originally with a note that it needed a capturing block; bridge_mobile_auth.zig's per-slot global blocks (slot index baked in at comptime) are exactly the shape that makes it unnecessary, so it is a second commit here rather than a footnote. Without it Zig answered true for a badge an unauthorised app never draws.

Verification

zig build test: 150/150 steps, 2482/2494 passed, 12 skipped, 0 failed (the second commit adds the badge-authorization tests), on Zig 0.17.0-dev.1963.

7 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.