ReviewOS

also looking at this

craft-native/craft

feat(ios): scanNFC — the first migration with no blocker to expire, ratchet 25 → 24

#121
Merged glennmichael123 wants to merge feat/ios-scan-nfc into main
5 files +539 -1
9 days ago

Of the 25 actions still on the shim, 21 carry a recorded reason and #119's deferral table now checks them. Four carry none — registerPush, scanNFC, scanQRCode, signInWithApple were never blocked, just never reached. scanNFC is the smallest: one framework, one guard, one delegate with two methods.

The chain

readingAvailable, then a session with invalidateAfterFirstRead:YES — which is what lets a single pending slot serve this action without a queue, since one scan answers one call. Every fallible step runs before ios_async.acquire, so no error path sits between leasing a slot and handing the session its delegate.

The reply

Records flattened across messages, as the spec's nested loop does:

[{"typeNameFormat":1,"type":"T","identifier":"","payload":"hello"}]

type and identifier fall back to "" when the bytes are not UTF-8; payload falls back to base64 instead. That asymmetry is the spec's and it is right — the payload is the point of the record, and dropping it silently is worse than handing back an encoding the caller has to notice.

One divergence, and it is a hang

readerSession:didInvalidateWithError: in the spec rejects only when the error code is not 200 — and 200 is user-cancel. So a cancelled scan clears pendingCallbackId and settles nothing. This answers CANCELLED, which is what the image, document and contact pickers all answer for the same gesture. (Same family as #116, which this file is otherwise clear of now.)

Two mistakes worth recording

  • bridge_error.appendJsonEscaped escapes contents and adds no quotes. The first version emitted bare tokens; two of its own tests caught it. Every field now goes through a wrapping helper so a call site cannot forget.
  • std.fmt.format does not exist on this toolchain — the tree's idiom is bufPrint into a stack buffer.

On the class lookup

A null answer is expected, not a failure. A generated app autolinks CoreNFC because CraftApp.swift:17 imports it and Swift autolinks what it imports; the zig-slice fixture links twelve frameworks by hand and CoreNFC is not among them, so there the lookup misses and this answers PLATFORM_NOT_SUPPORTED. Both are honest answers to "can this device scan a tag".

Verification

Cold zig build test: 150/150 steps, 2494/2506 passed, 0 failed. zig fmt --check clean. build-ios-all -Doptimize=ReleaseSafe builds all three archives. The NDEF walk itself cannot run on a host or a simulator, so what is pinned is everything that decides what the page sees — routing, the reply bytes, the escaping, the empty-array case, and the two constants taken from the spec.

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

Sign in to comment on this pull request.