ReviewOS
Reload

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
docs/BRIDGE_API.mdmodified+9-5
@@ -559,11 +559,15 @@ holds a ceiling on how many may remain that only ever goes down.
559559
560560### `channels`
561561
562Every `craft:*` event, and whether anything native emits on it. This cannot be
563derived from the action tables — an emitter is a separate piece of code that may
564or may not have been written — so a channel is reported live only because the
565code that emits on it took out a permit at startup. Sixteen channels the JS
566surface subscribes to currently have no emitter at all.
562Every `craft:*` event, and what craft can say about it: `'live'` when something
563in this build took out a permit to emit on it, `'unknown'` when craft cannot
564prove it either way.
565
566There is no `'dead'`, deliberately. Craft cannot establish absence — the
567`craft:window:*` names are composed in JavaScript from
568`__craftDeliverWindowEvent('focus')`, so no source scan finds the literal even
569though the emitter is right there. Treat `'unknown'` as "subscribe and see", not
570as "this will never fire".
567571
568572### `supports()` fails open
569573
packages/typescript/src/types.tsmodified+10-5
@@ -1247,13 +1247,18 @@ export interface CapabilityNamespace {
12471247export interface CraftCapabilities {
12481248 namespaces: Record<string, CapabilityNamespace>
12491249 /**
1250 * Every `craft:*` event channel, and whether anything native emits on it.
1250 * Every `craft:*` event channel, and what craft can say about it.
12511251 *
1252 * A `false` here means subscribing would work and never fire — which cannot
1253 * be derived from the action tables, so it is tracked separately by the
1254 * emitters themselves.
1252 * `'live'` means something in this build took out a permit to emit on it.
1253 * `'unknown'` means craft cannot prove it either way — subscribe and see, and
1254 * do **not** disable a feature over it.
1255 *
1256 * There is deliberately no `'dead'`. Craft cannot establish absence: the
1257 * `craft:window:*` names are composed in JavaScript from
1258 * `__craftDeliverWindowEvent('focus')`, so no source scan finds the literal
1259 * even though the emitter is right there.
12551260 */
1256 channels: Record<string, boolean>
1261 channels: Record<string, 'live' | 'unknown'>
12571262}
12581263
12591264
packages/zig/build.zigmodified+1-0
@@ -666,6 +666,7 @@ pub fn build(b: *std.Build) void {
666666 "src/bridge_screen.zig",
667667 "src/bridge_capabilities.zig",
668668 "src/bridge_capabilities_actions.zig",
669 "src/js/craft-bridge.js",
669670 }) |source_path| {
670671 capability_conformance_tests.root_module.addAnonymousImport(source_path, .{
671672 .root_source_file = b.path(source_path),
packages/zig/src/bridge_menu.zigmodified+2-0
@@ -4,6 +4,7 @@ const bridge_error = ;
44const icons = ;
55const menu_roles = ;
66const logging = ;
7const capabilities = ;
78
89const BridgeError = bridge_error.BridgeError;
910const log = logging.menu;
@@ -617,6 +618,7 @@ fn menuActionCallback(_: ?*anyopaque, _: ?*anyopaque, sender: ?*anyopaque) callc
617618}
618619
619620fn ensureMenuTarget() ?*anyopaque {
621 _ = capabilities.registerEmitter(.menu_action);
620622 if (comptime builtin.os.tag != .macos) return null;
621623 if (menu_target != null) return menu_target;
622624
packages/zig/src/bridge_shortcuts.zigmodified+3-0
@@ -29,6 +29,7 @@ const bridge_error = ;
2929const logging = ;
3030const registry_mod = ;
3131const accel = ;
32const capabilities = ;
3233
3334const log = logging.shortcuts;
3435
@@ -149,6 +150,8 @@ pub const ShortcutsBridge = struct {
149150 /// idempotent, so it runs before every registration rather than depending
150151 /// on an initialisation order.
151152 fn ensureDelivery(self: *Self) void {
153 _ = capabilities.registerEmitter(.shortcut);
154 _ = capabilities.registerEmitter(.shortcut_error);
152155 const global_state = ;
153156 global_state.instance.setShortcutsBridge(self);
154157 if (comptime builtin.os.tag == .macos) {
packages/zig/src/capabilities.zigmodified+127-45
@@ -91,54 +91,103 @@ pub const NamespaceDecl = struct {
9191/// deriving from: `craft:menu:action` is colon-separated and `craft:powerSleep`
9292/// is camel-cased, and both are load-bearing spellings a page already listens
9393/// for. Renaming them to be consistent would break every app.
94///
95/// `test/capabilities_test.zig` checks this list against every `_evt('craft:…')`
96/// in `craft-bridge.js`. It shipped with 22 of the 44 and claimed to be
97/// complete, which is the same overclaiming this whole mechanism exists to
98/// stop — so now the claim is enforced.
9499pub const Channel = enum {
100 bluetooth_deviceconnected,
101 bluetooth_devicedisconnected,
102 bluetooth_devicefound,
103 bonjour_found,
104 bonjour_lost,
105 deeplink,
106 filedrop,
107 fs_change,
108 handoff_incoming,
109 iap_failed,
110 iap_productsloaded,
111 iap_purchased,
112 iap_restored,
113 localserver_request,
114 location_authchanged,
115 location_error,
116 location_update,
95117 menu_action,
96 theme,
97 settings_open,
98 window_focus,
99 window_blur,
100 window_resize,
101 window_move,
118 menubar_statechange,
119 midi_message,
120 networkchange,
121 powersleep,
122 powerwake,
102123 screen_change,
103 deep_link,
104 file_drop,
124 screensharing_change,
125 serial_data,
126 servicemenu_invoked,
127 settings_open,
105128 shortcut,
106 fs_change,
107 power_sleep,
108 power_wake,
109 network_change,
110 bluetooth_device_found,
111 bluetooth_device_connected,
112 bluetooth_device_disconnected,
129 shortcut_error,
130 speechrecognition_final,
131 speechrecognition_partial,
132 theme,
113133 touchbar_action,
114 serial_data,
115 speech_partial,
116 speech_final,
134 tray_menuaction,
135 updateavailable,
136 updatedownloaded,
137 window_blur,
138 window_close,
139 window_focus,
140 window_minimize,
141 window_move,
142 window_resize,
143 window_restore,
117144
118145 pub fn eventName(self: Channel) []const u8 {
119146 return switch (self) {
147 .bluetooth_deviceconnected => "craft:bluetooth:deviceConnected",
148 .bluetooth_devicedisconnected => "craft:bluetooth:deviceDisconnected",
149 .bluetooth_devicefound => "craft:bluetooth:deviceFound",
150 .bonjour_found => "craft:bonjour:found",
151 .bonjour_lost => "craft:bonjour:lost",
152 .deeplink => "craft:deepLink",
153 .filedrop => "craft:fileDrop",
154 .fs_change => "craft:fs:change",
155 .handoff_incoming => "craft:handoff:incoming",
156 .iap_failed => "craft:iap:failed",
157 .iap_productsloaded => "craft:iap:productsLoaded",
158 .iap_purchased => "craft:iap:purchased",
159 .iap_restored => "craft:iap:restored",
160 .localserver_request => "craft:localServer:request",
161 .location_authchanged => "craft:location:authChanged",
162 .location_error => "craft:location:error",
163 .location_update => "craft:location:update",
120164 .menu_action => "craft:menu:action",
121 .theme => "craft:theme",
122 .settings_open => "craft:settings:open",
123 .window_focus => "craft:window:focus",
124 .window_blur => "craft:window:blur",
125 .window_resize => "craft:window:resize",
126 .window_move => "craft:window:move",
165 .menubar_statechange => "craft:menubar:stateChange",
166 .midi_message => "craft:midi:message",
167 .networkchange => "craft:networkChange",
168 .powersleep => "craft:powerSleep",
169 .powerwake => "craft:powerWake",
127170 .screen_change => "craft:screen:change",
128 .deep_link => "craft:deepLink",
129 .file_drop => "craft:fileDrop",
171 .screensharing_change => "craft:screenSharing:change",
172 .serial_data => "craft:serial:data",
173 .servicemenu_invoked => "craft:serviceMenu:invoked",
174 .settings_open => "craft:settings:open",
130175 .shortcut => "craft:shortcut",
131 .fs_change => "craft:fs:change",
132 .power_sleep => "craft:powerSleep",
133 .power_wake => "craft:powerWake",
134 .network_change => "craft:networkChange",
135 .bluetooth_device_found => "craft:bluetooth:deviceFound",
136 .bluetooth_device_connected => "craft:bluetooth:deviceConnected",
137 .bluetooth_device_disconnected => "craft:bluetooth:deviceDisconnected",
176 .shortcut_error => "craft:shortcut:error",
177 .speechrecognition_final => "craft:speechRecognition:final",
178 .speechrecognition_partial => "craft:speechRecognition:partial",
179 .theme => "craft:theme",
138180 .touchbar_action => "craft:touchbar:action",
139 .serial_data => "craft:serial:data",
140 .speech_partial => "craft:speechRecognition:partial",
141 .speech_final => "craft:speechRecognition:final",
181 .tray_menuaction => "craft:tray:menuAction",
182 .updateavailable => "craft:updateAvailable",
183 .updatedownloaded => "craft:updateDownloaded",
184 .window_blur => "craft:window:blur",
185 .window_close => "craft:window:close",
186 .window_focus => "craft:window:focus",
187 .window_minimize => "craft:window:minimize",
188 .window_move => "craft:window:move",
189 .window_resize => "craft:window:resize",
190 .window_restore => "craft:window:restore",
142191 };
143192 }
144193};
@@ -173,6 +222,33 @@ pub fn registerEmitter(channel: Channel) Emitter {
173222 return .{ .channel = channel };
174223}
175224
225/// What craft can honestly say about a channel.
226///
227/// There is no `dead`. A source scan cannot establish absence: `craft:window:*`
228/// names are composed in JavaScript from `__craftDeliverWindowEvent('focus')`,
229/// so no Zig file contains the literal even though the emitter is right there.
230/// The shipped version reported a bare `false` for anything unregistered, which
231/// a page reasonably reads as "this will never fire" — and 21 of 22 channels
232/// said it, including `craft:menu:action` and `craft:theme`, both of which work.
233pub const Liveness = enum {
234 /// Something in this build took out a permit to emit on it.
235 live,
236 /// Craft cannot prove it either way. Subscribe and see; do not disable a
237 /// feature over it.
238 unknown,
239
240 pub fn text(self: Liveness) []const u8 {
241 return switch (self) {
242 .live => "live",
243 .unknown => "unknown",
244 };
245 }
246};
247
248pub fn liveness(channel: Channel) Liveness {
249 return if (live_channels[@backingInt(channel)]) .live else .unknown;
250}
251
176252pub fn isLive(channel: Channel) bool {
177253 return live_channels[@backingInt(channel)];
178254}
@@ -257,8 +333,9 @@ pub fn buildManifest(gpa: std.mem.Allocator, registry: []const NamespaceDecl) ![
257333 if (index > 0) try out.append(gpa, ',');
258334 try out.append(gpa, '"');
259335 try appendEscaped(gpa, &out, channel.eventName());
260 try out.appendSlice(gpa, "\":");
261 try out.appendSlice(gpa, if (isLive(channel)) "true" else "false");
336 try out.appendSlice(gpa, "\":\"");
337 try out.appendSlice(gpa, liveness(channel).text());
338 try out.append(gpa, '"');
262339 }
263340 try out.appendSlice(gpa, "}}");
264341
@@ -281,17 +358,20 @@ test "a channel has no emitter until something registers one" {
281358 try testing.expectEqualStrings("craft:fs:change", emitter.eventName());
282359
283360 // Registering one channel says nothing about any other.
284 try testing.expect(!isLive(.power_sleep));
361 try testing.expect(!isLive(.powersleep));
285362}
286363
287364test "every channel has a distinct event name" {
288365 // A duplicate would make one of the two silently unreportable: the manifest
289366 // is keyed by event name, so the second would overwrite the first.
367 //
368 // Runtime rather than `inline for`: at 44 channels the nested comptime loop
369 // exceeds Zig's backwards-branch budget.
290370 const all = std.enums.values(Channel);
291 inline for (all, 0..) |left, i| {
292 inline for (all, 0..) |right, j| {
293 if (i < j) try testing.expect(!std.mem.eql(u8, left.eventName(), right.eventName()));
294 }
371 var names: [all.len][]const u8 = undefined;
372 for (all, 0..) |ch, i| names[i] = ch.eventName();
373 for (names, 0..) |a, i| {
374 for (names[i + 1 ..]) |b| try testing.expect(!std.mem.eql(u8, a, b));
295375 }
296376}
297377
@@ -388,8 +468,10 @@ test "the manifest reports every channel, live or not" {
388468 // Every channel is present — a dead one has to be reported as dead, not
389469 // omitted, or an app cannot tell "no emitter" from "channel I misspelled".
390470 try testing.expectEqual(@as(usize, channel_count), channels.count());
391 try testing.expect(channels.get("craft:menu:action").?.bool);
392 try testing.expect(!channels.get("craft:fs:change").?.bool);
471 try testing.expectEqualStrings("live", channels.get("craft:menu:action").?.string);
472 // Not "dead" — craft cannot prove a channel has no emitter, and saying so
473 // is what made the shipped manifest tell pages to disable working features.
474 try testing.expectEqualStrings("unknown", channels.get("craft:fs:change").?.string);
393475}
394476
395477test "a reason cannot break the manifest out of its JSON" {
packages/zig/src/js/craft-bridge.jsmodified+4-2
@@ -748,11 +748,13 @@
748748 // const caps = await craft.capabilities()
749749 // caps.namespaces.updater.status // 'unavailable'
750750 // caps.namespaces.updater.reason // 'the Sparkle framework is not linked…'
751 // caps.channels['craft:fs:change'] // false — subscribing would never fire
751 // caps.channels['craft:fs:change'] // 'live' | 'unknown'
752752 //
753753 // A namespace craft has not audited reports `undeclared`, which means exactly
754754 // that: craft will not claim anything either way. Treat it as "try it and
755 // handle failure", not as "missing".
755 // handle failure", not as "missing". Channels answer 'live' or 'unknown' for
756 // the same reason — there is no 'dead', because craft cannot prove a channel
757 // has no emitter.
756758 window.craft.capabilities = function () {
757759 return _req('capabilities', 'capabilities:get', '{}', 5000).then(function (manifest) {
758760 window.__craftCapabilities = manifest
packages/zig/src/macos_deep_link.zigmodified+2-0
@@ -1,6 +1,7 @@
11const std = ;
22const builtin = ;
33const macos = ;
4const capabilities = ;
45
56const objc = macos.objc;
67
@@ -77,6 +78,7 @@ pub fn install() void {
7778 );
7879
7980 installed = true;
81 _ = capabilities.registerEmitter(.deeplink);
8082
8183 if (comptime builtin.mode == .debug) {
8284 std.debug.print("[DeepLink] Installed AppleEvent handler for kAEGetURL\n", .{});
packages/zig/src/macos_file_drop.zigmodified+2-0
@@ -1,6 +1,7 @@
11const std = ;
22const builtin = ;
33const macos = ;
4const capabilities = ;
45
56const objc = macos.objc;
67
@@ -32,6 +33,7 @@ const PerformDragFn = *const fn (objc.id, objc.SEL, objc.id) callconv(.c) c_char
3233///
3334/// Idempotent — safe to call from every window create. macOS-only.
3435pub fn install() void {
36 _ = capabilities.registerEmitter(.filedrop);
3537 if (swizzled) return;
3638 if (builtin.target.os.tag != .macos) return;
3739
packages/zig/src/macos_theme.zigmodified+2-0
@@ -1,6 +1,7 @@
11const std = ;
22const builtin = ;
33const macos = ;
4const capabilities = ;
45
56const objc = macos.objc;
67
@@ -59,6 +60,7 @@ pub fn install() void {
5960 }
6061
6162 installed = true;
63 _ = capabilities.registerEmitter(.theme);
6264
6365 if (comptime builtin.mode == .debug) {
6466 std.debug.print("[Theme] Installed NSAppearance observer\n", .{});
packages/zig/src/macos_window_events.zigmodified+6-0
@@ -1,6 +1,7 @@
11const std = ;
22const builtin = ;
33const macos = ;
4const capabilities = ;
45
56const objc = macos.objc;
67
@@ -50,6 +51,11 @@ pub fn install(window: objc.id) void {
5051
5152 delegate_instance = macos.msgSend0(macos.msgSend0(cls, "alloc"), "init");
5253 installed = true;
54 // Five channels, one delegate. The names are composed in JS from
55 // `__craftDeliverWindowEvent`, so no source scan could ever find them.
56 inline for ([_]capabilities.Channel{ .window_focus, .window_blur, .window_resize, .window_move, .window_close }) |ch| {
57 _ = capabilities.registerEmitter(ch);
58 }
5359
5460 if (comptime builtin.mode == .debug) {
5561 std.debug.print("[WindowEvents] Installed NSWindowDelegate\n", .{});
packages/zig/test/capabilities_test.zigmodified+46-0
@@ -56,6 +56,52 @@ const dispatcher_source = @embedFile("src/macos.zig");
5656/// lowered, and lowering it is what "declaring a namespace" costs.
5757const max_undeclared: usize = 45;
5858
59/// The injected bridge script, so the channel list can be checked against the
60/// events the page actually subscribes to.
61const bridge_js = @embedFile("src/js/craft-bridge.js");
62
63test "every event the page subscribes to is a declared channel" {
64 // The check that was missing. `Channel` shipped with 22 entries and a doc
65 // comment claiming it listed every `craft:*` the JS surface subscribes to;
66 // there were 44. A manifest that omits a channel answers nothing about it,
67 // which is the same overclaiming this mechanism exists to stop — so the
68 // claim is now enforced rather than asserted.
69 var search: usize = 0;
70 var checked: usize = 0;
71 while (std.mem.indexOfPos(u8, bridge_js, search, "_evt('craft:")) |at| {
72 const name_start = at + "_evt('".len;
73 const name_end = std.mem.indexOfScalarPos(u8, bridge_js, name_start, '\'') orelse break;
74 const name = bridge_js[name_start..name_end];
75 search = name_end;
76
77 var known = false;
78 for (std.enums.values(capabilities.Channel)) |ch| {
79 if (std.mem.eql(u8, ch.eventName(), name)) known = true;
80 }
81 if (!known) {
82 std.debug.print(
83 "craft-bridge.js subscribes to '{s}' but it is not in the Channel enum,\n" ++
84 " so craft.capabilities() says nothing about it at all.\n",
85 .{name},
86 );
87 return error.SubscribedChannelNotDeclared;
88 }
89 checked += 1;
90 }
91 // A regex that silently matched nothing would make this test vacuous.
92 try testing.expect(checked >= 40);
93}
94
95test "no channel is reported as definitely dead" {
96 // `false` was never knowledge — it only ever meant "no emitter has
97 // registered", and a page reasonably read it as "this will never fire".
98 // Absence of proof is reported as `unknown` now, and there is deliberately
99 // no third value to regress to.
100 try testing.expectEqualStrings("live", capabilities.Liveness.live.text());
101 try testing.expectEqualStrings("unknown", capabilities.Liveness.unknown.text());
102 try testing.expectEqual(@as(usize, 2), std.enums.values(capabilities.Liveness).len);
103}
104
59105test "nothing is marked declared without being enforced" {
60106 // Closes the obvious loophole: marking a namespace `.declared` in the
61107 // registry buys the app's trust, and it must cost a `declared_sources` row,
packages/zig/test/cli_test.zigmodifiedformatting only+2-2
@@ -297,8 +297,8 @@ test "parseArgs - --frame-autosave leaves the explicit geometry alone" {
297297 // The two are not alternatives: the geometry flags are what the window
298298 // opens at the first time, before anything has been saved to restore.
299299 var options = try parse(&.{
300 "craft", "--frame-autosave", "main", "--width", "900",
301 "--height", "700", "--x", "40", "--y",
300 "craft", "--frame-autosave", "main", "--width", "900",
301 "--height", "700", "--x", "40", "--y",
302302 "60",
303303 });
304304 defer freeOptions(&options);
packages/zig/test/injected_js_test.zigmodified+5-2
formatting only · 8 hidden lines
@@ -614,7 +614,7 @@ test "every wire action the prefs facade posts is namespaced" { formatting only
614614 try fx.text("String(posted.every(function (m) { return m.a.indexOf('prefs:') === 0 }))"),
615615 );
616616 for ([_][]const u8{
617 prefs_actions.get, prefs_actions.set, prefs_actions.delete,
617 prefs_actions.get, prefs_actions.set, prefs_actions.delete,
618618 prefs_actions.clear, prefs_actions.keys, prefs_actions.info,
619619 }, 0..) |want, index| {
620620 var buf: [64]u8 = undefined;
@@ -886,7 +886,10 @@ test "the manifest native builds is the shape the facade reads" {
886886 // The reason is the difference between an app hunting a bug in its own code
887887 // and an app knowing the answer.
888888 try testing.expect(std.mem.indexOf(u8, try fx.text("caps.namespaces.updater.reason"), "Sparkle") != null);
889 try testing.expectEqualStrings("false", try fx.text("String(caps.channels['craft:fs:change'])"));
889 // Not "false" — craft cannot prove a channel has no emitter, and saying it
890 // could is what made the shipped manifest tell pages to disable working
891 // features. The only two answers are "live" and "unknown".
892 try testing.expectEqualStrings("unknown", try fx.text("caps.channels['craft:fs:change']"));
890893}
891894
892895test "craft.supports answers from the manifest, and fails open without one" {
packages/zig/test/logging_test.zigmodified+6-6
@@ -3,12 +3,12 @@ const testing = std.testing;
33const logging = ;
44
55test "LogLevel ordering" {
6 try testing.expect(@intFromEnum(logging.LogLevel.trace) < @intFromEnum(logging.LogLevel.debug));
7 try testing.expect(@intFromEnum(logging.LogLevel.debug) < @intFromEnum(logging.LogLevel.info));
8 try testing.expect(@intFromEnum(logging.LogLevel.info) < @intFromEnum(logging.LogLevel.warn));
9 try testing.expect(@intFromEnum(logging.LogLevel.warn) < @intFromEnum(logging.LogLevel.err));
10 try testing.expect(@intFromEnum(logging.LogLevel.err) < @intFromEnum(logging.LogLevel.fatal));
11 try testing.expect(@intFromEnum(logging.LogLevel.fatal) < @intFromEnum(logging.LogLevel.off));
6 try testing.expect(@backingInt(logging.LogLevel.trace) < @backingInt(logging.LogLevel.debug));
7 try testing.expect(@backingInt(logging.LogLevel.debug) < @backingInt(logging.LogLevel.info));
8 try testing.expect(@backingInt(logging.LogLevel.info) < @backingInt(logging.LogLevel.warn));
9 try testing.expect(@backingInt(logging.LogLevel.warn) < @backingInt(logging.LogLevel.err));
10 try testing.expect(@backingInt(logging.LogLevel.err) < @backingInt(logging.LogLevel.fatal));
11 try testing.expect(@backingInt(logging.LogLevel.fatal) < @backingInt(logging.LogLevel.off));
1212}
1313
1414test "LogLevel asText" {