Skip to content

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

Merged
glennmichael123 merged 2 commits into
mainfrom
fix/ios-reply-shape-parity
Sep 3, 2026
Merged

fix(ios): answer the shapes the spec answers, now that a bare false survives#105
glennmichael123 merged 2 commits into
mainfrom
fix/ios-reply-shape-parity

Conversation

@chrisbbreuer

@chrisbbreuer chrisbbreuer commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

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

action was now (= 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.

…urvives

An audit of all 33 mobile modules compared every `.live` action's reply
against the Swift arm the seam now displaces. 80 actions, 61 differences.
Most are Zig being stricter — refusing embedded NULs, validating filenames,
declining overlapping picker calls — and those stay. These are 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. So the workaround was load-bearing and the shapes could not be fixed
without fixing the bug underneath: `_orEmpty` now only substitutes `{}` for a
genuinely absent payload, and `false`, `0` and `""` reach the page intact.

**Then the five shapes.** `openURL`, `share` and `requestReview` answer bare
booleans again; `setBadge`/`clearBadge` answer `true`; `setKeepAwake` answers
the observed boolean. `share` is the one that mattered: it replied
`{"completed":false}` when the user *dismissed* the share 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, ...})

Two keys called `action`, and the later one wins, so 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 — 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 its
own doc comment here said so while the code did the opposite. It skips now,
and 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, so 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 missing. Both stages reject now, as the spec does,
and log the workout id rather than fabricating a success around it.

Not included: `setBadge` still never asks for notification authorization,
where the spec rejects without it — so Zig reports `true` for a badge iOS
silently refused to set. That one converts a synchronous handler into an async
chain with a module-owned block, which is a different size of change, and it
is the next commit rather than a footnote in this one.
…e spec does

The one divergence the previous commit left out, because it is a different
kind of change: `setBadge` and `clearBadge` went from synchronous to an async
chain with a module-owned block.

Swift calls `requestAuthorization(options: .badge)` first and rejects with
"Badge permission denied" when iOS says no. Zig skipped that with a note that
it needed a *capturing* block, which no block in the repo was. That stopped
being true when `bridge_mobile_auth.zig` shipped per-slot global blocks whose
slot index is baked in at comptime — the same shape is used here. Without it
Zig set the number, proved UIKit stored it, and answered `true` for a badge
an unauthorised app never draws: a fabricated success, and the one this file
was still producing after every other module had stopped.

The order is the spec's exactly. The count is validated *before* anything is
asked, so a malformed call fails as it always did without triggering the
first-run prompt. A refusal is answered from the completion queue —
`ios_async` hops to main itself. A grant is not: the badge has to be set on
the main thread before the answer is true, which is what Swift's
`DispatchQueue.main.async` is for in this position, so the grant path hops
first and answers after. The pending entry survives the hop and is taken on
the far side, which is what makes a double fire harmless.

`UNAuthorizationOptionBadge` is `1 << 0`, read from
`UNUserNotificationCenter.h:23` and pinned by a test that cites the line — a
neighbouring module once shipped a guessed `1 << 2` for a HealthKit option
that named something else entirely.

When UserNotifications is not in the process — the `zig-slice` fixture links
only UIKit, WebKit and Foundation — there is nothing to ask, and the badge is
set directly as before, with a warning saying so. A generated app cannot
reach that path: `project.yml` links UserNotifications unconditionally.

Host tests pin the shape rather than the name of the refusal, because which
error a test binary produces depends on whether it happens to have loaded
UserNotifications. What they pin is that `setBadge` is refused there and
never resolved — a resolve would be the fabricated success this removes.
@glennmichael123
glennmichael123 force-pushed the fix/ios-reply-shape-parity branch from 41945ac to e7d2e63 Compare September 3, 2026 16:21
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

✅ Binary load time

rounds:    25 interleaved
base:      p50 25.9ms   p95 26.9ms   (25.2–27.2ms)
head:      p50 26.0ms   p95 28.1ms   (24.7–31.7ms)
delta:     +0.3%  (fails above +20.0%)

No binary load time regression.
What this measures

craft --help: process spawn, dynamic linking and argument parsing.
It never opens a window, so it cannot see a change in window or
webview startup — real startup is benchmarks/startup.bench.ts, which
needs a display.

Both binaries are measured interleaved on this runner and compared by
p50, rather than against a number recorded on another machine. On
byte-identical binaries that method reads within ~3.5%; the old one
swung 45%.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

✅ Binary Size Report

Metric Value
Current Size 14912KB (14.56MB)
Change 0KB (0%) unchanged
Size limits
  • Warning: 14.50MB
  • Maximum: 16.00MB

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants