also looking at this
feat(ios): haptic, whose reason expired when ios_config landed
#130
3 files
+236
-78
| @@ -1,10 +1,34 @@ | ||
| 1 | 1 | //! The haptics half of the `mobile` namespace: `haptic` and `vibrate`. |
| 2 | 2 | //! |
| 3 | 3 | //! Split out of `bridge_mobile.zig` because the two actions have opposite |
| 4 | //! contracts — `haptic` is fire-and-forget and gated on a config flag, while | |
| 5 | //! `vibrate` replies and is gated on nothing — and because one of them cannot | |
| 6 | //! be served honestly from Zig yet. Keeping that admission next to the working | |
| 7 | //! action, rather than folded into the namespace's main file, is the point. | |
| 4 | //! contracts: `haptic` is fire-and-forget and gated on `config.enableHaptics`, | |
| 5 | //! `vibrate` replies and is gated on nothing. | |
| 6 | //! | |
| 7 | //! ## The refusal that used to live here | |
| 8 | //! | |
| 9 | //! `haptic` was declared `.status = .unavailable` for one reason, quoted from | |
| 10 | //! the manifest it carried: the flag "is not visible to Zig; serving it here | |
| 11 | //! would enable haptics for every app that never opted in". That was exactly | |
| 12 | //! right when it was written. The Taptic Engine has no readback, so a flip | |
| 13 | //! would have been invisible in CI and obvious on a device — and the default | |
| 14 | //! config has `enableHaptics` false, so the blast radius was every app that | |
| 15 | //! had not opted in. | |
| 16 | //! | |
| 17 | //! The condition that reason set for itself has since been met, by work that | |
| 18 | //! was not looking at this file. `ios_config.zig` reads `craft.config.json` | |
| 19 | //! out of the bundle, `gateFor("haptic")` maps the action to `.haptics`, and | |
| 20 | //! `ios_dispatch.zig` refuses any gated action whose flag is off *before* the | |
| 21 | //! module chain runs. The precise hazard the refusal was protecting against — | |
| 22 | //! an app on the default config buzzing on `craft.haptics.impact()` — is now | |
| 23 | //! the one case that cannot happen: the gate rejects it with | |
| 24 | //! `CAPABILITY_DISABLED`, which is what the spec's `else` arm sends too. | |
| 25 | //! | |
| 26 | //! Nothing re-checked that. The reason sat here, correct on the day it was | |
| 27 | //! written and wrong for every day after `ios_config.zig` landed, and the only | |
| 28 | //! way it surfaced was someone reading it again. `ios_conformance_test.zig` | |
| 29 | //! now fails the build for a gated action declared `.unavailable`, because a | |
| 30 | //! module claiming both "Zig can enforce this gate" and "Zig cannot read this | |
| 31 | //! gate" is stating a contradiction that a test can see. | |
| 8 | 32 | |
| 9 | 33 | const std = ; |
| 10 | 34 | const builtin = ; |
| @@ -25,53 +49,46 @@ pub const A = struct { | ||
| 25 | 49 | pub const vibrate = "vibrate"; |
| 26 | 50 | }; |
| 27 | 51 | |
| 28 | /// `haptic` is declared `unavailable`, and that is a decision, not an oversight. | |
| 29 | /// | |
| 30 | /// The Swift it replaces does not run `triggerHaptic` unconditionally — it runs | |
| 31 | /// it `if config.enableHaptics` (`CraftApp.swift:537`), and that flag defaults | |
| 32 | /// to **false** in both the template (`CraftApp.swift:190`) and the SDK | |
| 33 | /// (`packages/ios/src/index.ts:118`). It reaches Zig through nothing: no | |
| 34 | /// `craft_ios_*` export carries it, no build option encodes it. So serving | |
| 35 | /// `haptic` from here means serving a *different action* from the one the spec | |
| 36 | /// defines — every app on the default config would start buzzing on | |
| 37 | /// `craft.haptics.impact()`, which is the entire shipped haptics surface, and | |
| 38 | /// no test in this repo could see it because the Taptic Engine has no readback. | |
| 52 | /// The whole of `haptic`'s decision: `body["style"] as? String ?? "medium"`, | |
| 53 | /// then the spec's switch. | |
| 39 | 54 | /// |
| 40 | /// The three ways to make that go away are all worse than saying so: | |
| 41 | /// - fire anyway → a behaviour change that is invisible in CI and obvious on | |
| 42 | /// a device; | |
| 43 | /// - hardcode the gate false → a permanent no-op that reports nothing, which | |
| 44 | /// is the shape of the nine Android handlers that were deleted; | |
| 45 | /// - reply `{"success":true}` → fabricated success, the thing this migration | |
| 46 | /// exists to stop. | |
| 47 | /// | |
| 48 | /// So the handler refuses and the manifest says why. Unblocking it is small and | |
| 49 | /// deliberate: plumb the flag in (a `craft_ios_set_capabilities` export or a | |
| 50 | /// build option), then swap the refusal in `haptic()` for the mapping and the | |
| 51 | /// trigger below, both of which are written and pinned by the tests at the | |
| 52 | /// bottom of this file. What must not happen is the flip happening silently. | |
| 53 | /// | |
| 54 | /// Note the inconsistency this preserves: `case "vibrate"` has no | |
| 55 | /// `enableHaptics` gate in the spec either, so `craft.vibrate([100])` already | |
| 56 | /// fires impacts on an app with haptics "disabled". `vibrate` stays ungated to | |
| 57 | /// match; the asymmetry is the spec's, not ours. | |
| 58 | const haptic_unavailable_reason = | |
| 59 | "iOS gates haptic on config.enableHaptics, which defaults to false and is " ++ | |
| 60 | "not visible to Zig; serving it here would enable haptics for every app " ++ | |
| 61 | "that never opted in"; | |
| 55 | /// Split out from `haptic` so it can be asserted on a host. The trigger cannot | |
| 56 | /// — `UIImpactFeedbackGenerator` is not in a test runner's process — so a test | |
| 57 | /// calling `haptic` end-to-end can only observe `ClassNotFound`, which says | |
| 58 | /// nothing about whether the right style was chosen. This is the half where a | |
| 59 | /// mistake would be silent on a device too: the Taptic Engine has no readback, | |
| 60 | /// so a `heavy` served as `medium` feels like a device being a device. | |
| 61 | fn hapticTypeIn(allocator: std.mem.Allocator, data: []const u8) !HapticType { | |
| 62 | const parsed = std.json.parseFromSlice(std.json.Value, allocator, data, .{}) catch | |
| 63 | return bridge_error.BridgeError.InvalidJSON; | |
| 64 | defer parsed.deinit(); | |
| 65 | ||
| 66 | const root = switch (parsed.value) { | |
| 67 | .object => |obj| obj, | |
| 68 | else => return bridge_error.BridgeError.InvalidJSON, | |
| 69 | }; | |
| 62 | 70 | |
| 71 | // Swift's `as? String` fails for every non-string, and `?? "medium"` sends | |
| 72 | // all of them to the same place: absent, null, number, bool, array, object. | |
| 73 | const style = switch (root.get("style") orelse std.json.Value{ .null = {} }) { | |
| 74 | .string => |text| text, | |
| 75 | else => "medium", | |
| 76 | }; | |
| 77 | return hapticTypeForStyle(style); | |
| 78 | } | |
| 79 | ||
| 80 | /// `vibrate` stays ungated, and that is the spec's asymmetry rather than ours. | |
| 81 | /// | |
| 82 | /// `case "vibrate"` has no `enableHaptics` check in the spec, so | |
| 83 | /// `craft.vibrate([100])` fires impacts on an app with haptics "disabled". | |
| 84 | /// Adding a gate here would be a divergence dressed as a tidy-up. | |
| 63 | 85 | pub const capability_actions = [_]capabilities.ActionDecl{ |
| 64 | // `.none` is the action's contract regardless of status: the page's | |
| 65 | // `craft.haptic()` returns undefined and the spec's `case "haptic"` never | |
| 66 | // touches `resolveCallback`. An error still reaches the page — with no | |
| 67 | // pending entry to settle, `craft-bridge.js` reports it to the console — | |
| 68 | // so a refusal is logged rather than swallowed. | |
| 69 | .{ | |
| 70 | .name = A.haptic, | |
| 71 | .reply = .none, | |
| 72 | .status = .unavailable, | |
| 73 | .reason = haptic_unavailable_reason, | |
| 74 | }, | |
| 86 | // `.none` is the contract: the page's `craft.haptic()` returns undefined | |
| 87 | // and the spec's `case "haptic"` never touches `resolveCallback`. An error | |
| 88 | // still reaches the page — with no pending entry to settle, | |
| 89 | // `craft-bridge.js` reports it to the console — so a refusal is logged | |
| 90 | // rather than swallowed. | |
| 91 | .{ .name = A.haptic, .reply = .none }, | |
| 75 | 92 | // `.result` carrying the JSON literal `true`. Not a shape worth improving: |
| 76 | 93 | // the hand-off path already delivers exactly this today, and `.none` would |
| 77 | 94 | // park any `_req`-based caller for the full 30s timeout. |
| @@ -104,19 +121,27 @@ pub const HapticsBridge = struct { | ||
| 104 | 121 | return bridge_error.BridgeError.UnknownAction; |
| 105 | 122 | } |
| 106 | 123 | |
| 107 | /// Refuses, for the reason recorded on `haptic_unavailable_reason`. | |
| 124 | /// `case "haptic"` (`CraftApp.swift:576-582`). | |
| 125 | /// | |
| 126 | /// The gate is not checked here. `ios_dispatch.route` refuses every gated | |
| 127 | /// action whose flag is off before any module is asked, so reaching this | |
| 128 | /// function already means `enableHaptics` is true — and checking twice | |
| 129 | /// would put a second copy of the rule where the two could disagree. | |
| 108 | 130 | /// |
| 109 | /// The payload is not parsed first. Reporting a malformed `style` would | |
| 110 | /// imply the style was going to be used, and it is not — a refusal that | |
| 111 | /// sometimes reports a different cause is harder to act on than one that | |
| 112 | /// always says the same thing. | |
| 131 | /// `style` is read the way Swift reads it: `body["style"] as? String ?? | |
| 132 | /// "medium"`. A missing key, a number, an object — every failed cast takes | |
| 133 | /// the same default, so this reads the string when there is one and hands | |
| 134 | /// `hapticTypeForStyle` the literal `"medium"` otherwise. That is one | |
| 135 | /// spelling of the default rather than two: `hapticTypeForStyle` maps an | |
| 136 | /// unrecognised style to `.impact_medium` as well, matching the spec's | |
| 137 | /// `default:` arm, and this path exercises the same branch. | |
| 113 | 138 | /// |
| 114 | /// The error name is what lands in the device log; the page sees | |
| 115 | /// `NATIVE_CALL_FAILED`, `ios_dispatch.asBridgeError`'s catch-all, because | |
| 116 | /// the wire protocol has no code for "declared unavailable". The manifest | |
| 117 | /// is where an app reads the actual reason. | |
| 118 | fn haptic(_: *Self, _: []const u8) !void { | |
| 119 | return error.HapticsGateNotVisibleToZig; | |
| 139 | /// No reply on success, matching `.none` and the spec. The error from | |
| 140 | /// `triggerHapticChecked` is the one thing that can come back, and it means | |
| 141 | /// UIKit was not there to accept the call — never that the device stayed | |
| 142 | /// still, which no iOS API reports. | |
| 143 | fn haptic(self: *Self, data: []const u8) !void { | |
| 144 | try triggerHapticChecked(try hapticTypeIn(self.allocator, data)); | |
| 120 | 145 | } |
| 121 | 146 | |
| 122 | 147 | /// `case "vibrate"` (`CraftApp.swift:685-691`). |
| @@ -472,7 +497,9 @@ test "an unavailable action carries the reason with it" { | ||
| 472 | 497 | try testing.expect(decl.reason != null); |
| 473 | 498 | try testing.expect(decl.reason.?.len > 0); |
| 474 | 499 | } |
| 475 | try testing.expectEqual(capabilities.ActionStatus.unavailable, capability_actions[0].status); | |
| 500 | // Both live now. `haptic` was `.unavailable` until `ios_config.zig` made | |
| 501 | // the flag its reason blamed readable; see the module header. | |
| 502 | try testing.expectEqual(capabilities.ActionStatus.live, capability_actions[0].status); | |
| 476 | 503 | try testing.expectEqual(capabilities.ActionStatus.live, capability_actions[1].status); |
| 477 | 504 | } |
| 478 | 505 | |
| @@ -508,18 +535,36 @@ test "an action the namespace does not serve is reported, not ignored" { | ||
| 508 | 535 | ); |
| 509 | 536 | } |
| 510 | 537 | |
| 511 | test "haptic refuses rather than firing an ungated haptic" { | |
| 512 | // The refusal is the migration's honest answer while `config.enableHaptics` | |
| 513 | // has no Zig representation. If this test starts failing because someone | |
| 514 | // implemented the action, the flag has to have been plumbed through first — | |
| 515 | // that is the conversation this assertion exists to force. | |
| 516 | var bridge = HapticsBridge.init(testing.allocator); | |
| 517 | defer bridge.deinit(); | |
| 518 | ||
| 519 | try testing.expectError( | |
| 520 | error.HapticsGateNotVisibleToZig, | |
| 521 | bridge.handleMessage(A.haptic, "{\"style\":\"medium\"}"), | |
| 522 | ); | |
| 538 | test "haptic reads the style the way the spec's cast reads it" { | |
| 539 | // What replaced the refusal. The old test asserted | |
| 540 | // `error.HapticsGateNotVisibleToZig` and existed to force a conversation if | |
| 541 | // anyone implemented the action; the flag is readable now | |
| 542 | // (`ios_config.gateFor("haptic")`), the conversation happened, and this is | |
| 543 | // what the action actually decides. | |
| 544 | const alloc = testing.allocator; | |
| 545 | ||
| 546 | try testing.expectEqual(HapticType.impact_heavy, try hapticTypeIn(alloc, "{\"style\":\"heavy\"}")); | |
| 547 | try testing.expectEqual(HapticType.selection, try hapticTypeIn(alloc, "{\"style\":\"selection\"}")); | |
| 548 | ||
| 549 | // `as? String` fails for each of these, and `?? "medium"` catches them all. | |
| 550 | // A page posting `{style: 2}` gets a medium impact, not an error — matching | |
| 551 | // the spec, which cannot report one either: `case "haptic"` has no reply. | |
| 552 | try testing.expectEqual(HapticType.impact_medium, try hapticTypeIn(alloc, "{}")); | |
| 553 | try testing.expectEqual(HapticType.impact_medium, try hapticTypeIn(alloc, "{\"style\":null}")); | |
| 554 | try testing.expectEqual(HapticType.impact_medium, try hapticTypeIn(alloc, "{\"style\":2}")); | |
| 555 | try testing.expectEqual(HapticType.impact_medium, try hapticTypeIn(alloc, "{\"style\":true}")); | |
| 556 | try testing.expectEqual(HapticType.impact_medium, try hapticTypeIn(alloc, "{\"style\":[\"heavy\"]}")); | |
| 557 | try testing.expectEqual(HapticType.impact_medium, try hapticTypeIn(alloc, "{\"style\":{\"v\":\"heavy\"}}")); | |
| 558 | ||
| 559 | // And an unrecognised string takes the spec's `default:` arm rather than | |
| 560 | // failing — `craft.haptics.selection()` posts 'soft'. | |
| 561 | try testing.expectEqual(HapticType.impact_medium, try hapticTypeIn(alloc, "{\"style\":\"soft\"}")); | |
| 562 | ||
| 563 | // Malformed input is the one thing that is not a style: there is no | |
| 564 | // dictionary for Swift's subscript to read, so this cannot take the | |
| 565 | // default and call itself faithful. | |
| 566 | try testing.expectError(bridge_error.BridgeError.InvalidJSON, hapticTypeIn(alloc, "not json")); | |
| 567 | try testing.expectError(bridge_error.BridgeError.InvalidJSON, hapticTypeIn(alloc, "[1,2]")); | |
| 523 | 568 | } |
| 524 | 569 | |
| 525 | 570 | test "every style the shipped surface posts maps the way the spec's switch does" { |