ReviewOS

also looking at this

craft-native/craft

feat(bridge): let the page ask what native actually serves

#60
Merged glennmichael123 wants to merge feat/capabilities into main
20 files +1546 -37
11 days ago

Closes #49.

The injected bridge script is one blob, @embedFiled into the binary and injected whole into every window — the same JavaScript whatever the native side behind it implements. So typeof craft.tray.destroy === 'function' has never been evidence that anything is behind it. Three shipped bugs came from assuming otherwise, and none of them were catchable, because nothing compared the declared surface against the dispatched one.

The audit

I ran the full cross-reference first: all 279 JS→native action pairs and all 42 _evt subscriptions, against the live dispatch chain and all 55 bridge_*.zig files. 32 candidate gaps; the 6 most consequential went to independent agents told to refute them. All 6 held, none were refuted.

I spot-checked the issue's own claims by hand too. All confirmed — with one correction worth carrying: the issue writes the power/network channels as power.onSleep / network.onChange, but the actual event names are craft:powerSleep, craft:powerWake, craft:networkChange. Which turns out to matter for the design:

Event names follow no convention. craft:menu:action is colon-separated; craft:powerSleep is camelCase. A generated map has to enumerate channels, not derive them from namespace and method.

The finding is not "12 bugs". It is that there was no mechanism by which a shipped surface could be known to be wired, so the count is whatever the last manual audit says. The deliverable is the mechanism; the gaps are triage.

How it cannot drift

A table nobody dispatches on drifts on the first PR that adds an action and forgets it — and that is worse than no capabilities API, because it turns "the page can't tell" into "the page is told something false". So:

An action name appears as a string literal exactly once, in its bridge's A block, and the dispatch chain compares against that constant.

test/capabilities_test.zig enforces it in both directions, which is the part that matters — checking only declarations-against-dispatch catches a deleted arm; only the reverse catches an arm added and never declared, which is the drift that actually happens. On top of those, a source lint bans literal comparisons outright in declared bridges: a new action cannot be written without a table entry, because there is nowhere else to put the name.

Control-checked, each independently:

I brokeit caught
added an undeclared dispatch arma declared bridge compares against no action literals
deleted a dispatch arm, left the declarationevery declared action is actually dispatched on
deleted a table row, left the armevery dispatched action is declared
routed a namespace with no registry rowthe registry covers exactly the namespaces the dispatcher routes
marked something unavailable with no reasonan unavailable surface always says why
claimed an unrouted namespace was routedan unrouted namespace really is unrouted
un-namespaced the JS action3 contract tests
deleted the unavailable branch in supports()craft.supports answers from the manifest

Two of those controls found bugs in my own tests, which is why they are listed:

  • "every declared action appears in its source" searched for the name string — which survives deleting the dispatch arm, since it is still in the A block. It caught nothing. Rewritten to follow the constant, whose only two homes are the table and the chain.
  • The supports() control passed, i.e. the line I deleted did nothing for any case I had tested. It turned out to be load-bearing only for unrouted, which I had no coverage of — so marketplace is now declared unrouted and tested, and the line is genuinely exercised.

Emitters

Channel liveness cannot come from an action table — an emitter is separate code that may or may not have been written. registerEmitter() returns the only value that permits a dispatch, so "this channel is live" is earned by code that ran, not asserted. On a build where the observer never installs, the page is told the channel is dead instead of subscribing to something that will never fire.

Staging, honestly

Declared: clipboard, tray, app, screen, capabilities. Everything else routed answers undeclared — a real answer meaning craft claims nothing either way, not a placeholder. Declaring all 47 bridges beside a new public API would be a ~2500-line diff reviewable only by trusting the compiler.

undeclared ships under a ratchet that fails the build if the count rises, and if it is beaten by more than 2 without lowering the ceiling — so it cannot become where surfaces go to avoid being audited.

Declared, not fixed

Deliberately, because fixing them is an API decision and this is an introspection change:

  • tray.destroy — reachable, dispatched nowhere, and _send resolves anyway. Whether "destroy" should remove the status item or is just hide under another name is a design question.
  • updater — every call reaches a null SUUpdater; build.zig links no Sparkle, there is no dlopen. Linking, vendoring and signing a third-party framework is its own PR.
  • marketplace — implements 11 actions, no dispatcher arm reaches it. Listed unrouted precisely so it stops being invisible.

supports() fails open: true before capabilities are fetched, true for undeclared. A feature-detection mechanism that breaks working code when it cannot see itself would be worse than the gaps it describes.

Scope

macOS only — iOS/Android build window.craft from separate, smaller Zig string literals. Noted rather than silently implied.

I did not delete craft-app.js / craft-tray.js / craft-window.js, which the audit flagged as dead. I checked: they have zero @embedFile references, so they cannot affect the runtime surface — only a source-reading audit. 669 lines of deletion belongs in its own PR.

zig build test clean, test-js 11 pass, Windows cross-build clean, tsc --noEmit clean, pickier 38 warnings — identical to main.

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

Sign in to comment on this pull request.