ReviewOS

also looking at this

craft-native/craft

feat(ios): haptic, whose reason expired when ios_config landed

#130
Closed glennmichael123 wants to merge feat/ios-haptic-reason-expired into test/ios-reply-shape-conformance
3 files +236 -78
10 days ago

Stacked on #129 — review the top commit.

A reason that was right, and stopped being right

haptic was declared .status = .unavailable, and the manifest said why:

iOS gates haptic on config.enableHaptics, which defaults to false and is not visible to Zig; serving it here would enable haptics for every app that never opted in

That was exactly right when it was written. The Taptic Engine has no readback, so a flip would have been invisible in CI and obvious on a device — and with enableHaptics defaulting to false in both the template and the SDK, the blast radius was every app that had not opted in. The module header even wrote down what unblocking it would take: "plumb the flag in (a craft_ios_set_capabilities export or a build option), then swap the refusal in haptic() for the mapping and the trigger below, both of which are written and pinned by the tests at the bottom of this file."

That condition has been met, by work that was not looking at this file. ios_config.zig reads craft.config.json out of the bundle — better than either route the header proposed — gateFor("haptic") maps the action to .haptics, and ios_dispatch.zig refuses any gated action whose flag is off before the module chain runs. The precise hazard the refusal protected against is now the one case that cannot happen: an app on the default config gets CAPABILITY_DISABLED, which is what the spec's else arm sends too.

Why this one is different from the other three

Three reasons have expired during this migration and been found by reading. This one was contradicted by another file in the same tree: the manifest said Zig could not read enableHaptics; gateFor's table said Zig reads that exact flag and refuses the action on it. Both written honestly, months apart, with nothing putting them side by side.

That is checkable, so it is now checked. ios_conformance_test.zig fails the build for a refusal whose .reason names a flag gateFor maps to that action.

Deliberately narrower than "gated and unavailable", which is not a contradiction — lockOrientation is both, and its reason is about needing a root view controller overriding supportedInterfaceOrientations, which has nothing to do with reading a flag. What cannot stand is a refusal naming a flag the gate table proves Zig reads. Restoring the original declaration verbatim fails the new test.

The change itself

The implementation was already written and pinned, as the header promised — hapticTypeForStyle maps all seven styles the way triggerHaptic(style:) does, and triggerHapticChecked is already called in production by bridge_mobile_speech.zig.

What is new is hapticTypeIn: the whole of the action's decision — body["style"] as? String ?? "medium", then the spec's switch — split out so it can be asserted on a host. The trigger cannot be; UIImpactFeedbackGenerator is not in a test runner's process, so an end-to-end call can only observe ClassNotFound, which says nothing about whether the right style was chosen. That is also the half where a mistake stays silent on a device: a heavy served as medium just feels like a device being a device.

The gate is deliberately not re-checked inside the handler. Reaching it already means enableHaptics is true, and a second copy of the rule is a second thing that can disagree.

Behaviour

configbeforeafter
enableHaptics: falseZig hands back; shim rejects CAPABILITY_DISABLEDZig's gate rejects CAPABILITY_DISABLED
enableHaptics: trueZig hands back; shim calls triggerHaptic(style:)Zig parses style and fires the same generator

hostServesItself stops handing haptic back — its list drops to three, and the manifest-derived count test drops with it. The two tests that pinned the refusal are replaced by ones that pin what the action decides, including every non-string style (absent, null, number, bool, array, object) taking the same default the spec's ?? gives it, and malformed input being the one thing that is not a style.

zig build test:ios: 5 + 1067 + 19, all green.

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

Sign in to comment on this pull request.