ReviewOS

also looking at this

craft-native/craft

fix(capabilities): stop the channel manifest claiming knowledge it does not have

#68
Merged chris wants to merge fix/capability-channels into main
15 files +227 -67
11 days ago

I shipped this in #60 and it was wrong in two ways at once. Found while surveying #62–#67; reporting it before building anything else on top.

It told pages that working features were dead

Only one call site ever registered an emitter (bridge_screen.zig:158), so craft.capabilities().channels reported 21 of 22 channels as false — including craft:menu:action, craft:theme, craft:deepLink, craft:fileDrop and the whole craft:window:* family. Every one of those works.

A page feature-detecting honestly — the entire point of #49 — would have read that and disabled working features.

And the enum listed 22 of 44

Its doc comment claimed to enumerate every craft:* the JS surface subscribes to. It enumerated half, and nothing checked the claim.

Both are the bug class #49 exists to end, committed by the fix for #49.

false was never knowledge

It only ever meant "no emitter has registered". Craft cannot establish absence — I wrote a source scan to find dead channels and it called all five craft:window:* dead, because those names are composed in JavaScript from __craftDeliverWindowEvent('focus') and no .zig file contains the literal. The emitter is right there.

So liveness is two-valued now, live or unknown, with deliberately no third value to regress to:

caps.channels['craft:menu:action']  // 'live'
caps.channels['craft:fs:change']    // 'unknown' — subscribe and see

'unknown' means craft cannot prove it either way, not it will not fire.

Emitters register at install, not first fire

"This build contains an emitter" is a property of the binary. Registering when a subsystem first emits means a lazily-initialised one reports unknown merely because nobody has used it yet — which is the same false-negative in slower motion.

The durable part

A test that every _evt('craft:…') in craft-bridge.js has a Channel entry, checked against the embedded script. That is what was missing; it would have caught 22-vs-44 on day one.

It also carries a floor assertion (checked >= 40), so a regex that silently stops matching fails instead of passing vacuously. Control-checked both ways:

  • removing an unreferenced channel from the enum → craft-bridge.js subscribes to 'craft:bonjour:found' but it is not in the Channel enum
  • breaking the search string → the floor assertion fires

My first attempt at the first control removed menu_action, which is referenced elsewhere and didn't compile, so it proved nothing. Redone with an unreferenced channel.

Channel enum 22 → 44. Emitter registrations 1 → 13.

zig build test clean, 27 JS contract tests, Windows cross-build clean, zig fmt clean, tsc clean, pickier 38 warnings — same as main.

This is a breaking change to channels' value type, one day after it shipped. Worth taking now rather than leaving a manifest that lies.

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

Sign in to comment on this pull request.