ReviewOS

also looking at this

craft-native/craft

fix(ios): hand the page the answers native was already sending

#222
Merged glennmichael123 wants to merge fix/ios-unanswered-posts into main
18 files +597 -164
10 days ago

Closes #207.

What was wrong

The iOS page script posted these calls with no callbackId:

  • haptic
  • vibrate
  • startListening and stopListening
  • geolocation.watchPosition
  • geolocation.clearWatch
  • the v1 location.clearWatch

Swift's resolveCallback and rejectCallback both return early on a nil id, so native's answer was thrown away and the page got undefined. That included the answer that mattered most: CAPABILITY_DISABLED, when the app was built with the capability off.

Why a callbackId alone would have made it worse

Two of them only ever answered with a rejection. An enabled haptic or speech start did its work and sent nothing, on both runtimes:

  • Swift. case "haptic" and case "startListening" called rejectCallback in their else and nothing in their if. case "stopListening" never touched the callback at all.
  • Zig. A Zig-linked app serves these first. bridge_mobile_haptics.zig and bridge_mobile_speech.zig declared .reply = .none and never called sendResultToJS.

Giving the page an id without changing native would have turned "the page hears nothing" into "the page waits for ever on every call that works". So both native halves change too:

  • Haptics. haptic resolves true once UIKit has taken the call, on both runtimes. vibrate already did.

  • Speech. startListening and stopListening resolve true once the request is taken. Zig sends that from handleMessage, while the dispatch frame still holds the page's request id. Settling on the outcome instead would hang in more ways than it fixes:

    • an authorization answer arrives after the request context is popped, so its reply carries no id and the page drops it;
    • a second start while authorization is in flight has no outcome of its own;
    • Swift re-entry can stop before answering.

    What happens after true still arrives as craftSpeech* events, as before. craft.d.ts says so, so nobody "fixes" it later by waiting for them.

  • Location. watchPosition and clearWatch already answered on both sides; only the page lacked the id.

One choice, and why: craft.haptics.* does not reject when haptics are off

enableHaptics is off by default. With the raw call now reporting the refusal, craft.haptics.impact() and its siblings would have started rejecting in every app that never turned haptics on. The review found real flows that would stop, including every example in docs/api/mobile/haptics.md that awaits a haptic before doing the work it decorates (await haptics.impact('light'); switchToTab(tabId)), and examples/notes-app.

I split the contract along the line that already exists:

  • The raw calls tell the truth. craft.haptic() and craft.vibrate() resolve true or reject with CAPABILITY_DISABLED. This is what #207 asked for: code that needs to know can find out.
  • The feedback helpers treat a capability the app left off as nothing to play. That covers craft.haptics.*, and with it the SDK's haptics, plus useHaptic in React and Vue and use:haptic in Svelte. They resolve, the same as on Android, whose wrappers always resolve, and on the web fallback, which is a no-op without a vibration motor. Any other failure, such as NATIVE_CALL_FAILED, still rejects. The refusal also stays visible in craft.capabilities.haptics and in the bridge's error history.

The issue described the old wrappers as reporting success for a refused call. That is still true for this one code, deliberately, and only for the helpers whose documented use is decoration.

Tests that fail without the fix

ios_conformance_test.zig now checks both directions, and the unanswered_posts exception list is gone:

checkfails when
every call the spec answers carries a callbackId (existing, no exceptions now)the page posts one of these without an id
every call the page waits on has a case that can resolve it (new)a Swift case only rejects, as haptic and startListening did
no Zig module declares a reply the page waits on as .none (new)the Zig manifest says nothing is sent

Each check was tried against the matching mutation: the Swift haptic resolve removed, the Zig startListening manifest set back to .none, and the page's vibrate id removed. Each fails with its own message.

packages/ios/src/page-script.test.ts (new) runs the real injected script from CraftApp.swift under Bun, with a fake native side. It checks that each call posts an id and settles with what native sends, and that the raw call rejects on a refusal. It also covers the haptics feedback split, which the E2E suite cannot reach because each run has one config. Against main's template, 12 of its 16 tests fail. The other four, the "settles with nothing played" cases, fail against a version whose wrappers pass the refusal on.

The iOS E2E suite gains six cases. The probe app now has haptics on and speech recognition off:

caseasserts
geolocation.watchPosition.resolvesTruetrue, after currentPosition has settled authorization
geolocation.clearWatch.resolvesTruetrue
haptic.resolvesTruetrue with haptics on, the path that answered nothing
vibrate.resolvesTruetrue
speech.disabled.rejectsCAPABILITY_DISABLED. On the runtime leg it must also come from Zig's own gate, so startListening is added to ZIG_REFUSED_ACTIONS
speech.stopListening.resolvesTruetrue. Ungated, so it runs with speech off and needs no microphone

The runtime leg also requires Zig to dispatch every one of these actions without handing any back to Swift.

Run locally on an iOS 26.5 simulator:

# this branch
ok   ios-shim — 12/12 cases, no zig
ok   ios-runtime — 12/12 cases, zig served checkPermission, clearWatch, clipboardRead, clipboardWrite, getCurrentPosition, getDeviceInfo, haptic, log, requestPermission, share, startListening, stopListening, vibrate, watchPosition

# main's page script (no ids), run before the stopListening case was added
FAIL ios-shim — 6/11 cases, no zig
       case geolocation.watchPosition.resolvesTrue failed: watchPosition resolved with undefined
       case geolocation.clearWatch.resolvesTrue failed: clearWatch resolved with undefined
       case haptic.resolvesTrue failed: haptic resolved with undefined
       case vibrate.resolvesTrue failed: vibrate resolved with undefined
       case speech.disabled.rejects failed: startListening resolved with undefined while speech recognition is disabled
FAIL ios-runtime — 6/11 cases, (same five)

# page fixed, the success answers removed from Swift and Zig (haptic, stopListening)
FAIL ios-shim — 10/12 cases, no zig
       case haptic.resolvesTrue failed: haptic never settled with haptics enabled
       case speech.stopListening.resolvesTrue failed: stopListening never settled
FAIL ios-runtime — 10/12 cases, (same two)

Not done here, filed

  • #219: Android's haptic, vibrate, startListening and stopListening still return nothing. craft.d.ts types them Promise<boolean> | void until then.
  • #220: craft.watchPosition / craft.clearWatch mean different things on each platform and disagree with their type, and the v1 location.watchPosition can leave a dead watch after a refusal. It is a behaviour change for iOS pages that call craft.clearWatch() with no id, so it is kept out of this PR.
  • #221: openURL, setBadge and setFlashlight never settle on a malformed argument on the Swift path.

Verification

bun run test, bun run typecheck, bun run verify:packages, Pickier, the ios/typescript/android package tests, 50 protocol tests, and both iOS E2E legs locally, with and without each half of the fix.

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

Sign in to comment on this pull request.