fix(bridge): correlate replies by request id, not by action name

packages/typescript/src/__tests__/bridge-request-id.test.tsadded+287-0

Changes to packages/typescript/src/__tests__/bridge-request-id.test.ts

		@@ -0,0 +1,287 @@
	1	+import { describe, expect, it } from 'bun:test'
	2	+
	3	+/**
	4	+ * The reply-correlation contract, exercised against the real
	5	+ * `packages/zig/src/js/craft-bridge.js` — the file that ships inside the
	6	+ * binary — rather than a reimplementation of it.
	7	+ *
	8	+ * This is behavioural, not a source scan. Every test here drives the actual
	9	+ * promise machinery: call a facade, watch what goes out over
	10	+ * `webkit.messageHandlers.craft`, answer it the way native would, and check
	11	+ * which promise settles with what. A source scan cannot tell whether the
	12	+ * right caller was resolved, and that is the entire bug.
	13	+ */
	14	+
	15	+const BRIDGE_SRC = await Bun.file(
	16	+ new URL('../../../zig/src/js/craft-bridge.js', import.meta.url),
	17	+).text()
	18	+
	19	+interface Envelope { t: string, a: string, d?: string, i?: number }
	20	+
	21	+interface Harness {
	22	+ craft: any
	23	+ /** Every message the page posted to native, in order. */
	24	+ sent: Envelope[]
	25	+ /** Native answering a call, the way `sendResultToJS` does. */
	26	+ reply: (action: string, payload: unknown, id: number | null) => void
	27	+ /** Native failing a call, the way `sendErrorToJS` does. */
	28	+ fail: (err: { action?: string, code?: string, message?: string, id?: number }) => void
	29	+ win: any
	30	+ errors: unknown[][]
	31	+}
	32	+
	33	+/**
	34	+ * Run craft-bridge.js in a synthetic window.
	35	+ *
	36	+ * The script is an IIFE that reads bare `window`, `document` and `console`, so
	37	+ * they can simply be passed in as parameters — no DOM, no jsdom, and nothing
	38	+ * shared between tests.
	39	+ */
	40	+function loadBridge(): Harness {
	41	+ const sent: Envelope[] = []
	42	+ const errors: unknown[][] = []
	43	+ const win: any = {
	44	+ webkit: {
	45	+ messageHandlers: {
	46	+ craft: { postMessage: (m: Envelope) => { sent.push(m) } },
	47	+ },
	48	+ },
	49	+ addEventListener: () => {},
	50	+ removeEventListener: () => {},
	51	+ // The script fires `craft:ready` on load.
	52	+ dispatchEvent: () => true,
	53	+ }
	54	+ const doc: any = { readyState: 'complete', addEventListener: () => {} }
	55	+ const console_: any = { ...console, error: (...a: unknown[]) => { errors.push(a) }, warn: () => {} }
	56	+
	57	+ // eslint-disable-next-line no-new-func
	58	+ const run = new Function('window', 'document', 'console', 'setTimeout', 'clearTimeout', 'setInterval', BRIDGE_SRC)
	59	+ run(win, doc, console_, setTimeout, clearTimeout, () => 0)
	60	+
	61	+ return {
	62	+ craft: win.craft,
	63	+ sent,
	64	+ win,
	65	+ errors,
	66	+ reply: (action, payload, id) => {
	67	+ // Mirrors formatResultJS: three arguments, `null` when there is no id.
	68	+ win.__craftBridgeResult(action, payload, id)
	69	+ },
	70	+ fail: err => win.__craftBridgeError(err),
	71	+ }
	72	+}
	73	+
	74	+/** Let queued promise callbacks run. */
	75	+const settle = () => new Promise(r => setTimeout(r, 0))
	76	+
	77	+/**
	78	+ * Capture a promise's outcome now, so several promises can reject on the same
	79	+ * tick without any of them being momentarily unhandled. Resolves to the
	80	+ * rejection reason, or to `'resolved'` if it settled the other way.
	81	+ */
	82	+function settled(p: Promise<unknown>): Promise<unknown> {
	83	+ return p.then(() => 'resolved', (e: unknown) => e)
	84	+}
	85	+
	86	+describe('bridge reply correlation', () => {
	87	+ it('gives each caller its own answer when two bridges share an action name', async () => {
	88	+ // `get` is served by both keychain and tags. Before request ids, both
	89	+ // callers queued under the string "get" and were matched by arrival order,
	90	+ // so answering out of order swapped the payloads.
	91	+ const h = loadBridge()
	92	+
	93	+ const keychain = h.craft.keychain.get('svc', 'acct')
	94	+ const tags = h.craft.tags.get('/some/file')
	95	+
	96	+ expect(h.sent).toHaveLength(2)
	97	+ const [keychainMsg, tagsMsg] = h.sent
	98	+ expect(keychainMsg.t).toBe('keychain')
	99	+ expect(tagsMsg.t).toBe('tags')
	100	+ expect(keychainMsg.i).toBeDefined()
	101	+ expect(tagsMsg.i).not.toBe(keychainMsg.i)
	102	+
	103	+ // Native answers the second call first — the case that used to swap.
	104	+ h.reply('get', { tags: ['red'] }, tagsMsg.i!)
	105	+ h.reply('get', { value: 'hunter2' }, keychainMsg.i!)
	106	+
	107	+ expect(await keychain).toBe('hunter2')
	108	+ expect(await tags).toEqual(['red'])
	109	+ })
	110	+
	111	+ it('would have swapped those payloads without the ids', async () => {
	112	+ // The control. Same two calls, answered in the same order, but as a
	113	+ // pre-id binary would answer them: no id, so the action-name queue picks
	114	+ // the head. keychain asked first, so keychain is handed the tags payload
	115	+ // and reads `undefined` off it. This is the bug, reproduced.
	116	+ const h = loadBridge()
	117	+
	118	+ const keychain = h.craft.keychain.get('svc', 'acct')
	119	+ const tags = h.craft.tags.get('/some/file')
	120	+
	121	+ h.reply('get', { tags: ['red'] }, null)
	122	+ h.reply('get', { value: 'hunter2' }, null)
	123	+
	124	+ expect(await keychain).toBeUndefined()
	125	+ expect(await tags).toEqual([])
	126	+ })
	127	+
	128	+ it('resolves the right caller among several calls to one action on one bridge', async () => {
	129	+ const h = loadBridge()
	130	+ const a = h.craft.tags.get('/a')
	131	+ const b = h.craft.tags.get('/b')
	132	+ const c = h.craft.tags.get('/c')
	133	+
	134	+ const ids = h.sent.map(m => m.i!)
	135	+ expect(new Set(ids).size).toBe(3)
	136	+
	137	+ // Answer middle, last, first.
	138	+ h.reply('get', { tags: ['B'] }, ids[1])
	139	+ h.reply('get', { tags: ['C'] }, ids[2])
	140	+ h.reply('get', { tags: ['A'] }, ids[0])
	141	+
	142	+ expect(await a).toEqual(['A'])
	143	+ expect(await b).toEqual(['B'])
	144	+ expect(await c).toEqual(['C'])
	145	+ })
	146	+
	147	+ it('rejects only the call that failed, not everything sharing its name', async () => {
	148	+ // `isEnabled` is served by autoLaunch, bluetooth and crashReporter.
	149	+ // Draining by action name rejected callers of all three.
	150	+ const h = loadBridge()
	151	+
	152	+ const autoLaunch = h.craft.autoLaunch.isEnabled()
	153	+ const bluetooth = h.craft.bluetooth.isEnabled()
	154	+ const bluetoothId = h.sent[1].i!
	155	+
	156	+ h.fail({ action: 'isEnabled', code: 'NATIVE_CALL_FAILED', message: 'no bluetooth', id: bluetoothId })
	157	+
	158	+ await expect(bluetooth).rejects.toMatchObject({ message: 'no bluetooth' })
	159	+
	160	+ // autoLaunch never heard about it and still answers normally.
	161	+ h.reply('isEnabled', { value: true }, h.sent[0].i!)
	162	+ expect(await autoLaunch).toBe(true)
	163	+ })
	164	+
	165	+ it('would have rejected the innocent caller without the id', async () => {
	166	+ // The control for the test above.
	167	+ const h = loadBridge()
	168	+ // Outcomes are captured synchronously: both reject on the same tick, and
	169	+ // awaiting them one after the other would leave the second unhandled.
	170	+ const autoLaunch = settled(h.craft.autoLaunch.isEnabled())
	171	+ const bluetooth = settled(h.craft.bluetooth.isEnabled())
	172	+
	173	+ h.fail({ action: 'isEnabled', code: 'NATIVE_CALL_FAILED', message: 'no bluetooth' })
	174	+
	175	+ expect(await bluetooth).toMatchObject({ message: 'no bluetooth' })
	176	+ expect(await autoLaunch).toMatchObject({ message: 'no bluetooth' })
	177	+ })
	178	+
	179	+ it('drops a reply for an id it does not know instead of giving it to someone else', async () => {
	180	+ const h = loadBridge()
	181	+ const pending = h.craft.tags.get('/a')
	182	+ const id = h.sent[0].i!
	183	+
	184	+ // A late or duplicated reply for a call that already settled. The old code
	185	+ // had no way to tell, and would have handed this to the waiting caller.
	186	+ h.reply('get', { tags: ['GHOST'] }, id + 999)
	187	+ await settle()
	188	+
	189	+ h.reply('get', { tags: ['MINE'] }, id)
	190	+ expect(await pending).toEqual(['MINE'])
	191	+ })
	192	+
	193	+ it('answers a call once — a duplicate reply changes nothing', async () => {
	194	+ const h = loadBridge()
	195	+ const p = h.craft.tags.get('/a')
	196	+ const id = h.sent[0].i!
	197	+
	198	+ h.reply('get', { tags: ['first'] }, id)
	199	+ h.reply('get', { tags: ['second'] }, id)
	200	+
	201	+ expect(await p).toEqual(['first'])
	202	+ })
	203	+
	204	+ it('still resolves a reply that arrives without an id', async () => {
	205	+ // Not version skew — this file is embedded in the binary, so the two
	206	+ // always match. It is for replies raised outside any dispatch, where
	207	+ // native has no request to name.
	208	+ const h = loadBridge()
	209	+ const p = h.craft.tags.get('/a')
	210	+ h.reply('get', { tags: ['ok'] }, null)
	211	+ expect(await p).toEqual(['ok'])
	212	+ })
	213	+
	214	+ it('puts a distinct id on every message, fire-and-forget included', async () => {
	215	+ const h = loadBridge()
	216	+ h.craft.tags.get('/a')
	217	+ h.craft.keychain.set('svc', 'acct', 'pw') // _send, no reply expected
	218	+ h.craft.window.show?.()
	219	+ h.craft.autoLaunch.isEnabled()
	220	+
	221	+ const ids = h.sent.map(m => m.i)
	222	+ expect(ids.every(i => typeof i === 'number' && i > 0)).toBe(true)
	223	+ expect(new Set(ids).size).toBe(ids.length)
	224	+ })
	225	+
	226	+ it('reports a fire-and-forget failure rather than rejecting a live call', async () => {
	227	+ const h = loadBridge()
	228	+ const live = h.craft.tags.get('/a')
	229	+ const liveId = h.sent[0].i!
	230	+
	231	+ h.craft.keychain.set('svc', 'acct', 'pw')
	232	+ const sendId = h.sent[1].i!
	233	+ expect(sendId).not.toBe(liveId)
	234	+
	235	+ // `_send` registered no pending entry, so there is nobody to reject.
	236	+ h.fail({ action: 'set', code: 'NATIVE_CALL_FAILED', message: 'keychain locked', id: sendId })
	237	+ expect(h.errors).toHaveLength(1)
	238	+
	239	+ h.reply('get', { tags: ['fine'] }, liveId)
	240	+ expect(await live).toEqual(['fine'])
	241	+ })
	242	+
	243	+ it('forgets a timed-out call in both tables, and drops its late reply', async () => {
	244	+ const h = loadBridge()
	245	+ h.win.__craftBridgeRequestTimeoutMs = 1
	246	+ const p = h.craft.tags.get('/a')
	247	+ const id = h.sent[0].i!
	248	+
	249	+ await expect(p).rejects.toThrow(/timed out/)
	250	+
	251	+ expect(h.win.__craftBridgeById[id]).toBeUndefined()
	252	+ expect(h.win.__craftBridgePending.get).toBeUndefined()
	253	+
	254	+ // The late reply lands on an empty table and goes nowhere. Nothing to
	255	+ // assert but the absence of a throw — and that the next call is unaffected.
	256	+ h.reply('get', { tags: ['late'] }, id)
	257	+
	258	+ h.win.__craftBridgeRequestTimeoutMs = 30000
	259	+ const next = h.craft.tags.get('/b')
	260	+ h.reply('get', { tags: ['next'] }, h.sent[1].i!)
	261	+ expect(await next).toEqual(['next'])
	262	+ })
	263	+
	264	+ it('leaves no entry behind after a call settles', async () => {
	265	+ const h = loadBridge()
	266	+ const p = h.craft.tags.get('/a')
	267	+ h.reply('get', { tags: [] }, h.sent[0].i!)
	268	+ await p
	269	+
	270	+ expect(Object.keys(h.win.__craftBridgeById)).toHaveLength(0)
	271	+ expect(Object.keys(h.win.__craftBridgePending)).toHaveLength(0)
	272	+ })
	273	+
	274	+ it('still drops every pending call when native reports an unattributable failure', async () => {
	275	+ // An error with neither id nor action means the bridge itself is in
	276	+ // trouble; nobody should be left hanging.
	277	+ const h = loadBridge()
	278	+ const a = settled(h.craft.tags.get('/a'))
	279	+ const b = settled(h.craft.autoLaunch.isEnabled())
	280	+
	281	+ h.fail({ code: 'NATIVE_CALL_FAILED', message: 'bridge down' })
	282	+
	283	+ expect(await a).toMatchObject({ message: 'bridge down' })
	284	+ expect(await b).toMatchObject({ message: 'bridge down' })
	285	+ expect(Object.keys(h.win.__craftBridgeById)).toHaveLength(0)
	286	+ })
	287	+})

packages/zig/build.zigmodified+12-0

Changes to packages/zig/build.zig

		⋯660 linesShow the 660 lines above this hunk@@ -661,6 +661,16 @@ pub fn build(b: *std.Build) void {
661	661	}),
662	662	});
663	663
	664	+ // Which bridge call a reply belongs to. A thread-local stack, so nesting
	665	+ // (a modal run loop re-entering dispatch) is part of what gets tested.
	666	+ const request_context_tests = b.addTest(.{
	667	+ .root_module = b.createModule(.{
	668	+ .root_source_file = b.path("src/request_context.zig"),
	669	+ .target = target,
	670	+ .optimize = optimize,
	671	+ }),
	672	+ });
	673	+
664	674	// The conformance test: what craft declares, against what it dispatches.
665	675	const capability_conformance_tests = b.addTest(.{
666	676	.root_module = b.createModule(.{
		⋯367 linesShow the 367 lines above this hunk@@ -1034,6 +1044,7 @@ pub fn build(b: *std.Build) void {
1034	1044	const run_capabilities_tests = b.addRunArtifact(capabilities_tests);
1035	1045	const run_external_link_tests = b.addRunArtifact(external_link_tests);
1036	1046	const run_webview_recovery_tests = b.addRunArtifact(webview_recovery_tests);
	1047	+ const run_request_context_tests = b.addRunArtifact(request_context_tests);
1037	1048	const run_prefs_tests = b.addRunArtifact(prefs_tests);
1038	1049	const run_prefs_macos_tests = b.addRunArtifact(prefs_macos_tests);
1039	1050	const run_key_codes_tests = b.addRunArtifact(key_codes_tests);
		⋯109 linesShow the 109 lines above this hunk@@ -1149,6 +1160,7 @@ pub fn build(b: *std.Build) void {
1149	1160	test_step.dependOn(&run_capabilities_tests.step);
1150	1161	test_step.dependOn(&run_external_link_tests.step);
1151	1162	test_step.dependOn(&run_webview_recovery_tests.step);
	1163	+ test_step.dependOn(&run_request_context_tests.step);
1152	1164	// macOS only: the registry describes the dispatch chain in macos.zig, and
1153	1165	// compiling it elsewhere drags the whole native graph into a test binary
1154	1166	// for a platform it does not describe.

packages/zig/src/bridge_contracts.zigmodified+4-0

Changes to packages/zig/src/bridge_contracts.zig

		⋯12 linesShow the 12 lines above this hunk@@ -13,6 +13,10 @@
13	13	//! its own directory, so this has to live here beside the code it re-exports
14	14	//! rather than in `test/`.
15	15
	16	+/// The reply formatter. `test/injected_js_test.zig` answers the page with the
	17	+/// very string `sendResultToJS` evaluates, rather than a hand-written copy of
	18	+/// it that cannot notice when the two drift apart.
	19	+pub const errors = ;
16	20	pub const menu = ;
17	21	pub const shortcuts = ;
18	22	pub const prefs = ;

packages/zig/src/bridge_error.zigmodified+178-14

Changes to packages/zig/src/bridge_error.zig

		@@ -1,4 +1,5 @@
1	1	const std = ;
	2	+const request_context = ;
2	3	const builtin = ;
3	4
4	5	/// Common error types for all bridge operations
		⋯62 linesShow the 62 lines above this hunk@@ -67,6 +68,13 @@ pub const ErrorContext = struct {
67	68	err: BridgeError,
68	69	action: []const u8,
69	70	message: []const u8,
	71	+ /// The page's id for the call that failed, when the failure happened while
	72	+ /// serving one. Lets `craft-bridge.js` reject exactly that caller instead
	73	+ /// of every caller queued under the same action name — which matters most
	74	+ /// here, because six action names are served by more than one bridge, so
	75	+ /// draining by name rejects unrelated calls with an error from a bridge
	76	+ /// they never touched.
	77	+ request_id: ?u64 = null,
70	78
71	79	pub fn init(err: BridgeError, action: []const u8, message: []const u8) ErrorContext {
72	80	return .{
		⋯17 linesShow the 17 lines above this hunk@@ -90,7 +98,12 @@ pub const ErrorContext = struct {
90	98	try appendJsonEscaped(allocator, &out, self.action);
91	99	try out.appendSlice(allocator, "\",\"message\":\"");
92	100	try appendJsonEscaped(allocator, &out, self.message);
93		- try out.appendSlice(allocator, "\"}");
	101	+ try out.append(allocator, '"');
	102	+ if (self.request_id) |id| {
	103	+ try out.appendSlice(allocator, ",\"id\":");
	104	+ try out.print(allocator, "{d}", .{id});
	105	+ }
	106	+ try out.append(allocator, '}');
94	107
95	108	return out.toOwnedSlice(allocator);
96	109	}
		⋯68 linesShow the 68 lines above this hunk@@ -165,7 +178,8 @@ pub fn errorMessage(err: BridgeError) []const u8 {
165	178
166	179	/// Send error to JavaScript via eval
167	180	pub fn sendErrorToJS(allocator: std.mem.Allocator, action: []const u8, err: BridgeError) void {
168		- const ctx = ErrorContext.init(err, action, errorMessage(err));
	181	+ var ctx = ErrorContext.init(err, action, errorMessage(err));
	182	+ ctx.request_id = request_context.current();
169	183	const json = ctx.toJSON(allocator) catch {
170	184	if (comptime builtin.mode == .debug)
171	185	std.debug.print("[BridgeError] Failed to serialize error\n", .{});
		⋯26 linesShow the 26 lines above this hunk@@ -198,26 +212,48 @@ pub fn sendErrorToJS(allocator: std.mem.Allocator, action: []const u8, err: Brid
198	212	std.debug.print("[BridgeError] {s}: {s} - {s}\n", .{ action, errorCodeString(err), errorMessage(err) });
199	213	}
200	214
	215	+/// Render the JS call that hands `result_json` back to the page as the answer
	216	+/// to `request_id`.
	217	+///
	218	+/// Split out from `sendResultToJS` so the exact string that reaches the page
	219	+/// can be asserted in a test. It could not be before, and the shape of this
	220	+/// string is the whole reply contract.
	221	+pub fn formatResultJS(
	222	+ allocator: std.mem.Allocator,
	223	+ action: []const u8,
	224	+ result_json: []const u8,
	225	+ request_id: ?u64,
	226	+) ![]u8 {
	227	+ // Null, not 0 or -1, for "no id": JSON has a real absent value and zero is
	228	+ // a perfectly good id. A sentinel here is how the page ends up treating
	229	+ // some real call as unidentified.
	230	+ var id_buf: [24]u8 = undefined;
	231	+ const id_text: []const u8 = if (request_id) |id|
	232	+ try std.fmt.bufPrint(&id_buf, "{d}", .{id})
	233	+ else
	234	+ "null";
	235	+
	236	+ return std.fmt.allocPrint(
	237	+ allocator,
	238	+ "if(window.__craftBridgeResult)window.__craftBridgeResult('{s}',{s},{s});",
	239	+ .{ action, result_json, id_text },
	240	+ );
	241	+}
	242	+
201	243	/// Send success result to JavaScript
202	244	pub fn sendResultToJS(allocator: std.mem.Allocator, action: []const u8, result_json: []const u8) void {
203	245	const bridge = ;
204	246
205		- // Build JavaScript to call result handler
206		- const js_template = "if(window.__craftBridgeResult)window.__craftBridgeResult('{s}',{s});";
207		- const js_len = js_template.len + action.len + result_json.len;
208		-
209		- const js_buf = allocator.alloc(u8, js_len) catch {
210		- if (comptime builtin.mode == .debug)
211		- std.debug.print("[BridgeResult] Failed to allocate JS buffer\n", .{});
212		- return;
213		- };
214		- defer allocator.free(js_buf);
215		-
216		- const js = std.fmt.bufPrint(js_buf, js_template, .{ action, result_json }) catch {
	247	+ // Which call this answers. Null when there is no call to name: a reply
	248	+ // raised outside any dispatch, or a message the page sent without an `i`
	249	+ // (the tray and menubar polling `_post`s, which nobody awaits). The page
	250	+ // then matches by action name exactly as it did before ids.
	251	+ const js = formatResultJS(allocator, action, result_json, request_context.current()) catch {
217	252	if (comptime builtin.mode == .debug)
218	253	std.debug.print("[BridgeResult] Failed to format JS\n", .{});
219	254	return;
220	255	};
	256	+ defer allocator.free(js);
221	257
222	258	bridge.evalJS(js) catch |err| {
223	259	// Was misleadingly logged as "failed to send error to JS" — this
		⋯180 linesShow the 180 lines above this hunk@@ -404,3 +440,131 @@ test "ErrorContext.toJSON handles long messages without overflow" {
404	440
405	441	try testing.expect(json.len > 4096);
406	442	}
	443	+
	444	+test "a reply names the call it answers" {
	445	+ const allocator = std.testing.allocator;
	446	+ const js = try formatResultJS(allocator, "getDisplays", "{\"displays\":[]}", 7);
	447	+ defer allocator.free(js);
	448	+ try std.testing.expectEqualStrings(
	449	+ "if(window.__craftBridgeResult)window.__craftBridgeResult('getDisplays',{\"displays\":[]},7);",
	450	+ js,
	451	+ );
	452	+}
	453	+
	454	+test "a reply with no call to name says null, not a sentinel" {
	455	+ // An old page, or a reply raised outside dispatch. The page falls back to
	456	+ // matching by action name; it must not read this as "call number 0".
	457	+ const allocator = std.testing.allocator;
	458	+ const js = try formatResultJS(allocator, "getPrimary", "{}", null);
	459	+ defer allocator.free(js);
	460	+ try std.testing.expectEqualStrings(
	461	+ "if(window.__craftBridgeResult)window.__craftBridgeResult('getPrimary',{},null);",
	462	+ js,
	463	+ );
	464	+}
	465	+
	466	+test "call zero is answered as zero" {
	467	+ const allocator = std.testing.allocator;
	468	+ const js = try formatResultJS(allocator, "a", "{}", 0);
	469	+ defer allocator.free(js);
	470	+ try std.testing.expect(std.mem.endsWith(u8, js, "('a',{},0);"));
	471	+}
	472	+
	473	+test "the largest id JavaScript can hold survives the round trip" {
	474	+ // Number.MAX_SAFE_INTEGER. The page counts up from 1 and will never reach
	475	+ // it, but a buffer sized for a small id is the kind of thing that only
	476	+ // fails in the field.
	477	+ const allocator = std.testing.allocator;
	478	+ const js = try formatResultJS(allocator, "a", "{}", 9007199254740991);
	479	+ defer allocator.free(js);
	480	+ try std.testing.expect(std.mem.endsWith(u8, js, "('a',{},9007199254740991);"));
	481	+}
	482	+
	483	+test "a failure names the call that failed" {
	484	+ const allocator = std.testing.allocator;
	485	+ var ctx = ErrorContext.init(BridgeError.NotFound, "get", "Resource not found");
	486	+ ctx.request_id = 42;
	487	+ const json = try ctx.toJSON(allocator);
	488	+ defer allocator.free(json);
	489	+ try std.testing.expectEqualStrings(
	490	+ "{\"error\":true,\"code\":\"NOT_FOUND\",\"action\":\"get\",\"message\":\"Resource not found\",\"id\":42}",
	491	+ json,
	492	+ );
	493	+
	494	+ // Still valid JSON with the field appended, not just string-equal.
	495	+ const parsed = try std.json.parseFromSlice(std.json.Value, allocator, json, .{});
	496	+ defer parsed.deinit();
	497	+ try std.testing.expectEqual(@as(i64, 42), parsed.value.object.get("id").?.integer);
	498	+}
	499	+
	500	+test "a failure outside any call carries no id at all" {
	501	+ // Not `"id":null` — absent. The page tells the two apart: an id it does not
	502	+ // recognise is a call that already settled and the error is dropped, while
	503	+ // no id at all is the old whole-action drain.
	504	+ const allocator = std.testing.allocator;
	505	+ const ctx = ErrorContext.init(BridgeError.NotFound, "get", "Resource not found");
	506	+ const json = try ctx.toJSON(allocator);
	507	+ defer allocator.free(json);
	508	+ try std.testing.expect(std.mem.indexOf(u8, json, "id") == null);
	509	+}
	510	+
	511	+test "the id survives an action name that needs escaping" {
	512	+ const allocator = std.testing.allocator;
	513	+ var ctx = ErrorContext.init(BridgeError.InvalidJSON, "we\"ird\\", "boom\nboom");
	514	+ ctx.request_id = 5;
	515	+ const json = try ctx.toJSON(allocator);
	516	+ defer allocator.free(json);
	517	+ const parsed = try std.json.parseFromSlice(std.json.Value, allocator, json, .{});
	518	+ defer parsed.deinit();
	519	+ try std.testing.expectEqual(@as(i64, 5), parsed.value.object.get("id").?.integer);
	520	+ try std.testing.expectEqualStrings("we\"ird\\", parsed.value.object.get("action").?.string);
	521	+}
	522	+
	523	+test "the reply to a dispatched message names that message's call" {
	524	+ // The seam this whole change turns on, walked end to end: the id is read
	525	+ // out of the envelope the way `handleBridgeMessageJSON` reads it, recorded
	526	+ // the way the dispatcher records it, and read back the way
	527	+ // `sendResultToJS` reads it — without any of the 267 reply sites in
	528	+ // between being told about it.
	529	+ const allocator = std.testing.allocator;
	530	+ request_context.resetForTesting();
	531	+
	532	+ const parsed = try std.json.parseFromSlice(
	533	+ std.json.Value,
	534	+ allocator,
	535	+ \\{"t":"tags","a":"get","d":"{\"path\":\"/a\"}","i":12}
	536	+ ,
	537	+ .{},
	538	+ );
	539	+ defer parsed.deinit();
	540	+
	541	+ request_context.push(request_context.fromEnvelope(parsed.value.object));
	542	+ defer request_context.pop();
	543	+
	544	+ const js = try formatResultJS(allocator, "get", "{\"tags\":[]}", request_context.current());
	545	+ defer allocator.free(js);
	546	+ try std.testing.expect(std.mem.endsWith(u8, js, ",12);"));
	547	+}
	548	+
	549	+test "a dialog's reply is not stamped with the call it dispatched while it waited" {
	550	+ // `runModal` spins a nested run loop that keeps delivering bridge
	551	+ // messages, so a second call is served in the middle of the first. The
	552	+ // dialog's own reply, sent after the inner one, still belongs to the outer
	553	+ // call — this is why the dispatcher keeps a stack.
	554	+ const allocator = std.testing.allocator;
	555	+ request_context.resetForTesting();
	556	+
	557	+ request_context.push(4); // showOpenDialog, now blocked in runModal
	558	+ defer request_context.pop();
	559	+ {
	560	+ request_context.push(5); // a getState the nested run loop delivered
	561	+ defer request_context.pop();
	562	+ const inner = try formatResultJS(allocator, "getState", "{}", request_context.current());
	563	+ defer allocator.free(inner);
	564	+ try std.testing.expect(std.mem.endsWith(u8, inner, ",5);"));
	565	+ }
	566	+
	567	+ const outer = try formatResultJS(allocator, "showOpenDialog", "{}", request_context.current());
	568	+ defer allocator.free(outer);
	569	+ try std.testing.expect(std.mem.endsWith(u8, outer, ",4);"));
	570	+}

packages/zig/src/bridge_menubar.zigmodified+18-15

Changes to packages/zig/src/bridge_menubar.zig

		⋯16 linesShow the 16 lines above this hunk@@ -17,8 +17,6 @@ pub const MenubarCollapseBridge = struct {
17	17	}
18	18
19	19	pub fn handleMessage(self: *Self, action: []const u8, data: []const u8) !void {
20		- _ = self;
21		-
22	20	if (std.mem.eql(u8, action, "init")) {
23	21	menubar_collapse.init();
24	22	} else if (std.mem.eql(u8, action, "collapse")) {
		⋯3 linesShow the 3 lines above this hunk@@ -28,20 +26,25 @@ pub const MenubarCollapseBridge = struct {
28	26	} else if (std.mem.eql(u8, action, "toggle")) {
29	27	menubar_collapse.toggle();
30	28	} else if (std.mem.eql(u8, action, "getState")) {
31		- // Return current state via JS callback
32		- const collapsed = menubar_collapse.isCollapsed();
33		- const initialized = menubar_collapse.isInitialized();
34		- const sep_hidden = menubar_collapse.isSeparatorHidden();
	29	+ // Answered through `sendResultToJS` like every other request, so it
	30	+ // is stamped with the id of the call it answers. It used to hand
	31	+ // the page a bespoke reply key — `menubarCollapse:getState` — which
	32	+ // the JS side had to queue under by hand: its own pending entry,
	33	+ // its own failure path, and no timeout, so a call native never
	34	+ // answered held its resolver for the life of the page. The id makes
	35	+ // the key irrelevant, so the bespoke queue is gone and this is an
	36	+ // ordinary `_req`.
35	37	var buf: [384]u8 = undefined;
36		- const js = std.fmt.bufPrint(&buf, "window.__craftBridgeResult('menubarCollapse:getState',);", .{
37		- if (collapsed) "true" else "false",
38		- if (initialized) "true" else "false",
39		- if (sep_hidden) "true" else "false",
40		- }) catch return;
41		- const bridge = ;
42		- bridge.evalJS(js) catch |err| {
43		- std.log.debug("JS eval failed for menubar status callback: {}", .{err});
44		- };
	38	+ const json = std.fmt.bufPrint(
	39	+ &buf,
	40	+ "",
	41	+ .{
	42	+ if (menubar_collapse.isCollapsed()) "true" else "false",
	43	+ if (menubar_collapse.isInitialized()) "true" else "false",
	44	+ if (menubar_collapse.isSeparatorHidden()) "true" else "false",
	45	+ },
	46	+ ) catch return;
	47	+ .sendResultToJS(self.allocator, "getState", json);
45	48	} else if (std.mem.eql(u8, action, "setAutoCollapse")) {
46	49	// Parse delay from data (seconds as string)
47	50	if (data.len > 0) {

packages/zig/src/js/craft-bridge.jsmodified+136-62

Changes to packages/zig/src/js/craft-bridge.js

		⋯11 linesShow the 11 lines above this hunk@@ -12,9 +12,10 @@
12	12	// when served outside a Craft window). Don't throw at module load.
13	13	//
14	14	// The native side dispatches messages by `t` (type) and answers async ones
15		-// via `window.__craftBridgeResult(action, payload)`. Promises are resolved
16		-// per-action — callers that need different per-call replies should pass an
17		-// `id` field through the `data` payload and correlate themselves.
	15	+// via `window.__craftBridgeResult(action, payload, id)`, where `id` is the
	16	+// `i` this file put on the outgoing message. Replies are matched by that id,
	17	+// so concurrent calls to the same action — and to the same action name on two
	18	+// different bridges — resolve their own callers and nobody else's.
18	19
19	20	;(function () {
20	21	if (window.craft && window.craft.__craft_bridge_loaded) return
		⋯4 linesShow the 4 lines above this hunk@@ -25,63 +26,143 @@
25	26	// Core: pending-call queue, result/error delivery, send helpers.
26	27	// -------------------------------------------------------------------------
27	28
	29	+ // Calls in flight, queued per action name.
	30	+ //
	31	+ // Kept as the fallback for replies that arrive without an id. This file is
	32	+ // `@embedFile`d into the binary and injected at document-start, so the page's
	33	+ // bridge and the native side always ship together and cannot disagree about
	34	+ // the protocol — the fallback is not for version skew. It is for replies
	35	+ // raised outside any dispatch, where there is no request to name: a handler
	36	+ // that answers from a callback after its message returned, or an error the
	37	+ // bridge raises on its own behalf. Matching by name in order is the best
	38	+ // guess available then, and it is exactly what this file did before ids.
28	39	window.__craftBridgePending = window.__craftBridgePending || {}
29	40
30		- // Native calls this with the same `action` string the JS facade enqueued
31		- // under, so the queue is keyed by action name. Action collisions across
32		- // bridges would confuse the queue — keep names unique on the Zig side.
33		- window.__craftBridgeResult = function (action, payload) {
34		- const q = window.__craftBridgePending[action] || []
35		- if (q.length > 0) {
36		- const e = q.shift()
37		- if (e && e.resolve) e.resolve(payload || {})
	41	+ // Calls in flight, indexed by the id native echoes back. This is the
	42	+ // correlation that actually holds, and the one used whenever native gives us
	43	+ // an id to use.
	44	+ window.__craftBridgeById = window.__craftBridgeById || {}
	45	+
	46	+ // Monotonic, starting at 1, so `if (id)` is never true for a real call and
	47	+ // zero is free to mean nothing at all.
	48	+ window.__craftBridgeSeq = window.__craftBridgeSeq || 0
	49	+ function _nextId() {
	50	+ window.__craftBridgeSeq += 1
	51	+ return window.__craftBridgeSeq
	52	+ }
	53	+
	54	+ // Drop an entry from both tables. Both, always: an entry left in `byId`
	55	+ // holds its resolver forever, and one left in the action queue shifts every
	56	+ // later fallback reply onto the wrong caller.
	57	+ function _forget(entry) {
	58	+ if (!entry) return
	59	+ if (entry.id) delete window.__craftBridgeById[entry.id]
	60	+ const q = window.__craftBridgePending[entry.action]
	61	+ if (Array.isArray(q)) {
	62	+ const i = q.indexOf(entry)
	63	+ if (i !== -1) q.splice(i, 1)
	64	+ if (q.length === 0) delete window.__craftBridgePending[entry.action]
38	65	}
39	66	}
40	67
41		- // Catastrophic native-side failure: drop every pending promise with the
42		- // same error so callers don't hang forever.
43		- // Native calls this when an action fails. The payload names the action that
44		- // failed, so only that action's callers are rejected — this used to empty the
45		- // whole pending table, so one bridge failing took down every unrelated call
46		- // in flight at the time.
	68	+ // Native's answer to one call. `id` is the `i` we sent, echoed back; a reply
	69	+ // raised outside any dispatch has none and falls back to the action queue.
	70	+ window.__craftBridgeResult = function (action, payload, id) {
	71	+ if (id !== undefined && id !== null) {
	72	+ const e = window.__craftBridgeById[id]
	73	+ // An id we don't recognise is a call that has already settled — it timed
	74	+ // out, or native answered it twice. Drop it. The old code had no id to
	75	+ // check, so it shifted the action queue and handed this payload to
	76	+ // whoever was at the head: another call's answer, delivered as that
	77	+ // call's own, with nothing to indicate anything had gone wrong.
	78	+ if (!e) return
	79	+ _forget(e)
	80	+ if (e.resolve) e.resolve(payload || {})
	81	+ return
	82	+ }
	83	+
	84	+ // No id to match on. Match by action name, in order, exactly as this file
	85	+ // did before ids existed.
	86	+ const q = window.__craftBridgePending[action]
	87	+ if (!q || q.length === 0) return
	88	+ const e = q[0]
	89	+ _forget(e)
	90	+ if (e.resolve) e.resolve(payload || {})
	91	+ }
	92	+
	93	+ // Native calls this when an action fails.
47	94	//
48		- // Fire-and-forget calls (`_send`) never register a pending entry: native
49		- // acknowledges nothing on success, so there is nothing to wait on and the
50		- // promise has already resolved by the time a failure comes back. Those have
51		- // no caller left to reject, so they are reported to the console instead —
	95	+ // `err.id` names the exact call that failed, so exactly that caller is
	96	+ // rejected. That matters more than it sounds: six action names are served by
	97	+ // two or three bridges each (`get` by keychain and tags, `isEnabled` by
	98	+ // autoLaunch, bluetooth and crashReporter, and four more), and rejecting by
	99	+ // action name hands one bridge's failure to a caller waiting on a different
	100	+ // bridge — `craft.bluetooth.isEnabled()` failing would reject an in-flight
	101	+ // `craft.autoLaunch.isEnabled()` with bluetooth's error.
	102	+ //
	103	+ // Without an id, the old behaviour stands: drain that action's queue, or the
	104	+ // whole table if the error does not even name an action, since an error that
	105	+ // cannot be attributed is most likely the bridge itself being in trouble.
	106	+ //
	107	+ // Fire-and-forget calls (`_send`) carry an id but register no pending entry:
	108	+ // native acknowledges nothing on success, so there is nothing to wait on and
	109	+ // the promise resolved the moment the message was posted. Those have no
	110	+ // caller left to reject, so they are reported to the console instead —
52	111	// otherwise the only trace is in the native log, which is not where anyone
53	112	// writing a web page is looking.
54	113	window.__craftBridgeError = function (err) {
55	114	const pending = window.__craftBridgePending || {}
56	115	const action = err && err.action
	116	+ const id = err && err.id
57	117	let rejected = 0
58	118
	119	+ const report = function () {
	120	+ if (typeof console !== 'undefined' && console.error) {
	121	+ console.error(
	122	+ '[craft] bridge call failed: ' + (action || '(unnamed action)'),
	123	+ (err && err.code) || '', (err && err.message) || '',
	124	+ )
	125	+ }
	126	+ }
	127	+
	128	+ if (id !== undefined && id !== null) {
	129	+ const e = window.__craftBridgeById[id]
	130	+ // No entry means the call already settled, or it was a `_send` that
	131	+ // never registered one. Nobody to tell — say so, rather than rejecting
	132	+ // some other caller who is still waiting and would have succeeded.
	133	+ if (!e) { report(); return }
	134	+ _forget(e)
	135	+ if (e.reject) e.reject(err)
	136	+ return
	137	+ }
	138	+
59	139	const drain = function (key) {
60	140	const q = pending[key]
61	141	if (!Array.isArray(q)) return
62	142	while (q.length > 0) {
63	143	const e = q.shift()
64		- if (e && e.reject) { e.reject(err); rejected++ }
	144	+ if (e) {
	145	+ if (e.id) delete window.__craftBridgeById[e.id]
	146	+ if (e.reject) { e.reject(err); rejected++ }
	147	+ }
65	148	}
66	149	delete pending[key]
67	150	}
68	151
69	152	if (action) drain(action)
70		- // An error that does not name its action cannot be targeted; the
71		- // conservative reading is that the bridge itself is in trouble.
72	153	else Object.keys(pending).forEach(drain)
73	154
74		- if (rejected === 0 && typeof console !== 'undefined' && console.error) {
75		- console.error(
76		- '[craft] bridge call failed: ' + (action || '(unnamed action)'),
77		- (err && err.code) || '', (err && err.message) || '',
78		- )
79		- }
	155	+ if (rejected === 0) report()
80	156	}
81	157
82		- function _post(t, a, d) {
	158	+ // `i` is the call id. Every message carries one, fire-and-forget included:
	159	+ // a failure has to be able to name the exact call it came from, and native
	160	+ // reads it back out of the envelope in `handleBridgeMessageJSON`.
	161	+ function _post(t, a, d, i) {
83	162	try {
84		- window.webkit.messageHandlers.craft.postMessage({ t: t, a: a, d: d || '' })
	163	+ const msg = { t: t, a: a, d: d || '' }
	164	+ if (i) msg.i = i
	165	+ window.webkit.messageHandlers.craft.postMessage(msg)
85	166	return true
86	167	}
87	168	catch (e) {
		⋯15 linesShow the 15 lines above this hunk@@ -103,19 +184,18 @@
103	184	// can still `await craft.window.show()` without surprises.
104	185	function _send(t, a, d) {
105	186	return new Promise(function (ok, no) {
106		- if (_post(t, a, d)) ok()
	187	+ if (_post(t, a, d, _nextId())) ok()
107	188	else no(new Error('craft bridge unavailable'))
108	189	})
109	190	}
110	191
111	192	// Request-with-response. Native must call sendResultToJS(action, json)
112		- // exactly once for each in-flight call. The action name must match.
	193	+ // exactly once for each in-flight call; the dispatcher stamps the reply with
	194	+ // the `i` this call sent, and that id is what resolves the promise.
113	195	//
114		- // Note: keyed by action only (not type+action) for compat with the
115		- // existing `__craftBridgeResult(action, payload)` contract bridges
116		- // have been using. Action collisions across bridges (e.g. two
117		- // bridges both having `cancel`) would mix replies; in practice the
118		- // action namespace is small and unique. Documented limitation.
	196	+ // The call is registered in both tables: by id, which is exact, and on the
	197	+ // action-name queue, which catches a reply that reaches us without one.
	198	+ // Whichever settles it, `_forget` clears both.
119	199	//
120	200	// Each call is reaped after `__craftBridgeRequestTimeoutMs` (default
121	201	// 30s) so a misbehaving native side can't strand callers forever.
		⋯2 linesShow the 2 lines above this hunk@@ -124,21 +204,22 @@
124	204	const DEFAULT_TIMEOUT_MS = 30000
125	205	function _req(t, a, d, timeoutMs) {
126	206	return new Promise(function (ok, no) {
	207	+ const id = _nextId()
127	208	const q = (window.__craftBridgePending[a] = window.__craftBridgePending[a] || [])
128		- const entry = { resolve: ok, reject: no }
	209	+ const entry = { id: id, action: a, resolve: ok, reject: no }
129	210	q.push(entry)
	211	+ window.__craftBridgeById[id] = entry
130	212
131	213	const timeout = (typeof window.__craftBridgeRequestTimeoutMs === 'number')
132	214	? window.__craftBridgeRequestTimeoutMs
133	215	: (typeof timeoutMs === 'number' ? timeoutMs : DEFAULT_TIMEOUT_MS)
134	216	const timer = (timeout > 0)
135	217	? setTimeout(function () {
136		- // Remove our entry (may be at any position since other
137		- // calls might have arrived in the meantime) and reject.
138		- // We only target our own entry, so concurrent calls for
139		- // the same action stay healthy.
140		- const idx = q.indexOf(entry)
141		- if (idx !== -1) q.splice(idx, 1)
	218	+ // Only ever our own entry, from both tables. A late reply then
	219	+ // finds no entry under this id and is dropped, rather than being
	220	+ // handed to whichever caller has since taken our place at the head
	221	+ // of the action queue.
	222	+ _forget(entry)
142	223	no(new Error('craft bridge timed out for ' + t + '/' + a))
143	224	}, timeout)
144	225	: null
		⋯2 linesShow the 2 lines above this hunk@@ -147,9 +228,8 @@
147	228	entry.resolve = function (v) { if (timer) clearTimeout(timer); ok(v) }
148	229	entry.reject = function (e) { if (timer) clearTimeout(timer); no(e) }
149	230
150		- if (!_post(t, a, d)) {
151		- const pi = q.indexOf(entry)
152		- if (pi !== -1) q.splice(pi, 1)
	231	+ if (!_post(t, a, d, id)) {
	232	+ _forget(entry)
153	233	if (timer) clearTimeout(timer)
154	234	no(new Error('craft bridge unavailable'))
155	235	}
		⋯1,246 linesShow the 1,246 lines above this hunk@@ -1402,19 +1482,13 @@
1402	1482	collapse: function () { return _send('menubarCollapse', 'collapse') },
1403	1483	expand: function () { return _send('menubarCollapse', 'expand') },
1404	1484	toggle: function () { return _send('menubarCollapse', 'toggle') },
1405		- getState: function () {
1406		- // Native menubar uses a fully-qualified result key, so we have to
1407		- // queue under the same key to receive the response.
1408		- return new Promise(function (ok, no) {
1409		- const key = 'menubarCollapse:getState'
1410		- const q = (window.__craftBridgePending[key] = window.__craftBridgePending[key] || [])
1411		- q.push({ resolve: ok, reject: no })
1412		- if (!_post('menubarCollapse', 'getState', '')) {
1413		- q.pop()
1414		- no(new Error('craft bridge unavailable'))
1415		- }
1416		- })
1417		- },
	1485	+ // Was a hand-rolled pending entry queued under the reply key native used
	1486	+ // to invent for it, `menubarCollapse:getState` — with no timeout, so a
	1487	+ // call native never answered held its resolver until the page went away.
	1488	+ // Native answers by request id now, so the key carries no meaning and this
	1489	+ // is an ordinary request. `getState` is also served by screenSharing; the
	1490	+ // id is what keeps the two apart.
	1491	+ getState: function () { return _req('menubarCollapse', 'getState') },
1418	1492	// Seconds to wait before tidying up again, or 0 to leave the bar alone.
1419	1493	// The native side parses this as a number: coercing it to a boolean here
1420	1494	// sent "true", which parsed as 0 and quietly switched the feature off.

packages/zig/src/macos.zigmodified+12-0

Changes to packages/zig/src/macos.zig

		⋯5 linesShow the 5 lines above this hunk@@ -6,6 +6,7 @@ const local_tls = ;
6	6	const menu_roles = ;
7	7	const external_link = ;
8	8	const webview_recovery = ;
	9	+const request_context = ;
9	10
10	11	// Objective-C runtime types and functions (manual declarations to avoid @cImport issues)
11	12	pub const objc = struct {
		⋯4,573 linesShow the 4,573 lines above this hunk@@ -4585,6 +4586,17 @@ pub fn handleBridgeMessageJSON(json_str: []const u8) !void {
4585	4586	else => return error.InvalidBridgeMessage,
4586	4587	};
4587	4588
	4589	+ // The call this message is, so the reply can name it instead of leaving
	4590	+ // `craft-bridge.js` to guess by action name. Recorded here, once, rather
	4591	+ // than passed down through 267 reply sites that would each have to
	4592	+ // remember to forward it. See `request_context.zig`.
	4593	+ //
	4594	+ // Pushed unconditionally — a message with no `i` pushes null, so it
	4595	+ // shadows an enclosing request instead of inheriting an id that is not
	4596	+ // its own. The pop is deferred because dispatch below can fail anywhere.
	4597	+ request_context.push(request_context.fromEnvelope(root));
	4598	+ defer request_context.pop();
	4599	+
4588	4600	// Extract data if present - could be string, object, array, or missing
4589	4601	var data_json_str: []const u8 = "";
4590	4602	var data_needs_free: bool = false;

packages/zig/src/request_context.zigadded+248-0

Changes to packages/zig/src/request_context.zig

		@@ -0,0 +1,248 @@
	1	+//! Which bridge call the current reply belongs to.
	2	+//!
	3	+//! The page can have several calls of the same action in flight at once, and
	4	+//! six action names are served by more than one bridge (`get` by keychain and
	5	+//! tags, `isEnabled` by autoLaunch, bluetooth and crashReporter, and four
	6	+//! more). Until now a reply named only its action, so `craft-bridge.js` had to
	7	+//! guess which caller it answered by shifting a per-action FIFO — a guess that
	8	+//! is wrong the moment two calls settle out of the order they were made.
	9	+//!
	10	+//! Rather than thread an id parameter through 267 `sendResultToJS` call sites
	11	+//! in 39 files — five different `handleMessage` signatures, most of them
	12	+//! reached through helpers — the dispatcher records the id of the message it
	13	+//! is serving here, and `bridge_error.zig` reads it back when it formats the
	14	+//! reply. Handlers are untouched and cannot forget to pass it along.
	15	+//!
	16	+//! ## Why a stack and not one slot
	17	+//!
	18	+//! Bridge dispatch nests. `bridge_dialog.zig` runs `NSOpenPanel` and friends
	19	+//! through `runModal`, which spins a nested run loop, and that run loop keeps
	20	+//! delivering `WKScriptMessage`s — so a second bridge call is dispatched, and
	21	+//! completes, in the middle of the first one. A single slot would be
	22	+//! overwritten by the inner call and the outer dialog's reply would go out
	23	+//! wearing the inner call's id, which is worse than no id at all: the page
	24	+//! would resolve a stranger's promise and be certain it was right.
	25	+//!
	26	+//! ## Why every frame is pushed, even without an id
	27	+//!
	28	+//! `push` takes an optional. A message that carries no `i` — an older page, or
	29	+//! the description-format fallback path — still pushes a frame, holding null.
	30	+//! If id-less messages skipped the push instead, one dispatched inside a call
	31	+//! that *does* have an id would inherit it, and the same misattribution
	32	+//! follows. An absent id has to shadow the outer one.
	33	+//!
	34	+//! ## Threading
	35	+//!
	36	+//! Thread-local, and deliberately so. Every bridge message is dispatched on the
	37	+//! main thread, and every one of the 267 reply sites is reached synchronously
	38	+//! from that dispatch (verified: none sits inside a `callconv(.c)` callback).
	39	+//! A reply that did somehow originate on a worker thread reads an empty stack,
	40	+//! reports no id, and the page falls back to the action FIFO — exactly the
	41	+//! behaviour it has today. Degrading to the old guess is acceptable; handing
	42	+//! out an id belonging to whatever the main thread happens to be serving is
	43	+//! not.
	44	+
	45	+const std = ;
	46	+
	47	+/// Deep enough for any real nesting: frames are only added by a modal run loop
	48	+/// re-entering dispatch, and those nest by user action — a file panel opened
	49	+/// from a file panel. Sixteen is far past anything a person can drive.
	50	+pub const max_depth = 16;
	51	+
	52	+threadlocal var frames: [max_depth]?u64 = undefined;
	53	+threadlocal var depth: usize = 0;
	54	+
	55	+/// Begin serving `id`. Pass null when the message carried no id, so it shadows
	56	+/// any enclosing request rather than inheriting it. Always pair with `pop`.
	57	+pub fn push(id: ?u64) void {
	58	+ if (depth < max_depth) frames[depth] = id;
	59	+ // Counted past the array on purpose: `pop` stays balanced with `push`, and
	60	+ // `current` reports the overflowed frames as unknown rather than reading a
	61	+ // stale id from the top of the array.
	62	+ depth += 1;
	63	+}
	64	+
	65	+/// Finish serving the innermost request.
	66	+pub fn pop() void {
	67	+ if (depth == 0) return;
	68	+ depth -= 1;
	69	+}
	70	+
	71	+/// The id of the request being served on this thread, or null if there is
	72	+/// none, if it carried none, or if nesting ran past `max_depth`. Null means
	73	+/// "cannot say" — the page keeps its old FIFO behaviour — never "no such
	74	+/// call".
	75	+pub fn current() ?u64 {
	76	+ if (depth == 0 or depth > max_depth) return null;
	77	+ return frames[depth - 1];
	78	+}
	79	+
	80	+/// How deep dispatch is nested right now. For tests and diagnostics.
	81	+pub fn currentDepth() usize {
	82	+ return depth;
	83	+}
	84	+
	85	+/// Drop every frame. Tests only — production balances push/pop with `defer`.
	86	+pub fn resetForTesting() void {
	87	+ depth = 0;
	88	+}
	89	+
	90	+/// The `i` field of a bridge envelope: the page's id for this call.
	91	+///
	92	+/// Absent for the tray and menubar polling `_post`s, which nobody awaits, and
	93	+/// for the description-format fallback path — so a missing or unusable value
	94	+/// is normal and means "no id", not a malformed message.
	95	+/// Accepts a JSON number or a decimal string: JavaScript integers arrive as
	96	+/// numbers, but an id that has been through a `JSON.stringify` round trip on
	97	+/// the page's side could arrive as either.
	98	+pub fn fromEnvelope(root: std.json.ObjectMap) ?u64 {
	99	+ const v = root.get("i") orelse return null;
	100	+ return switch (v) {
	101	+ .integer => |n| if (n >= 0) @intCast(n) else null,
	102	+ .string => |s| std.fmt.parseInt(u64, s, 10) catch null,
	103	+ else => null,
	104	+ };
	105	+}
	106	+
	107	+const testing = std.testing;
	108	+
	109	+test "no request in flight reports no id" {
	110	+ resetForTesting();
	111	+ try testing.expectEqual(@as(?u64, null), current());
	112	+}
	113	+
	114	+test "the id of the request being served is the one reported" {
	115	+ resetForTesting();
	116	+ push(7);
	117	+ defer pop();
	118	+ try testing.expectEqual(@as(?u64, 7), current());
	119	+}
	120	+
	121	+test "a nested request does not leak its id to the enclosing reply" {
	122	+ // This is the dialog case: runModal spins a nested run loop, another
	123	+ // bridge message is dispatched and answered inside it, and only then does
	124	+ // the dialog reply. The outer reply must still be id 1.
	125	+ resetForTesting();
	126	+ push(1);
	127	+ {
	128	+ push(2);
	129	+ try testing.expectEqual(@as(?u64, 2), current());
	130	+ pop();
	131	+ }
	132	+ try testing.expectEqual(@as(?u64, 1), current());
	133	+ pop();
	134	+ try testing.expectEqual(@as(?u64, null), current());
	135	+}
	136	+
	137	+test "a nested message without an id shadows the enclosing one" {
	138	+ // The failure this prevents: an older page, or the description-format
	139	+ // fallback, dispatched inside a call that does have an id. Inheriting 1
	140	+ // here would send that page's reply to a promise it never made.
	141	+ resetForTesting();
	142	+ push(1);
	143	+ push(null);
	144	+ try testing.expectEqual(@as(?u64, null), current());
	145	+ pop();
	146	+ try testing.expectEqual(@as(?u64, 1), current());
	147	+ pop();
	148	+}
	149	+
	150	+test "nesting past max_depth reports unknown rather than a stale id" {
	151	+ resetForTesting();
	152	+ var i: usize = 0;
	153	+ while (i < max_depth) : (i += 1) push(@intCast(i));
	154	+ try testing.expectEqual(@as(?u64, max_depth - 1), current());
	155	+
	156	+ push(999);
	157	+ // 999 had nowhere to go. The answer is "cannot say", not frames[15].
	158	+ try testing.expectEqual(@as(?u64, null), current());
	159	+ pop();
	160	+
	161	+ try testing.expectEqual(@as(?u64, max_depth - 1), current());
	162	+ i = 0;
	163	+ while (i < max_depth) : (i += 1) pop();
	164	+ try testing.expectEqual(@as(usize, 0), currentDepth());
	165	+}
	166	+
	167	+test "push and pop stay balanced across overflow" {
	168	+ resetForTesting();
	169	+ const over = max_depth + 5;
	170	+ var i: usize = 0;
	171	+ while (i < over) : (i += 1) push(1);
	172	+ try testing.expectEqual(@as(usize, over), currentDepth());
	173	+ i = 0;
	174	+ while (i < over) : (i += 1) pop();
	175	+ try testing.expectEqual(@as(usize, 0), currentDepth());
	176	+ try testing.expectEqual(@as(?u64, null), current());
	177	+}
	178	+
	179	+test "pop on an empty stack is not an underflow" {
	180	+ resetForTesting();
	181	+ pop();
	182	+ pop();
	183	+ try testing.expectEqual(@as(usize, 0), currentDepth());
	184	+ push(3);
	185	+ try testing.expectEqual(@as(?u64, 3), current());
	186	+ pop();
	187	+}
	188	+
	189	+test "id zero is a real id, distinct from absent" {
	190	+ // JS ids start at 1 so this should not arise, but the type must not quietly
	191	+ // conflate the two — a `0 == none` sentinel is how this bug class returns.
	192	+ resetForTesting();
	193	+ push(0);
	194	+ try testing.expectEqual(@as(?u64, 0), current());
	195	+ pop();
	196	+ try testing.expectEqual(@as(?u64, null), current());
	197	+}
	198	+
	199	+fn envelopeId(json: []const u8) !?u64 {
	200	+ const parsed = try std.json.parseFromSlice(std.json.Value, testing.allocator, json, .{});
	201	+ defer parsed.deinit();
	202	+ return fromEnvelope(parsed.value.object);
	203	+}
	204	+
	205	+test "the envelope's id is read as the call id" {
	206	+ try testing.expectEqual(@as(?u64, 12), try envelopeId(
	207	+ \\{"t":"tags","a":"get","d":"","i":12}
	208	+ ));
	209	+}
	210	+
	211	+test "a message with no id is not an error, it is a message nobody awaits" {
	212	+ try testing.expectEqual(@as(?u64, null), try envelopeId(
	213	+ \\{"t":"tray","a":"pollActions","d":""}
	214	+ ));
	215	+}
	216	+
	217	+test "an id that arrived as a string is still an id" {
	218	+ try testing.expectEqual(@as(?u64, 900719925474099), try envelopeId(
	219	+ \\{"t":"tags","a":"get","i":"900719925474099"}
	220	+ ));
	221	+}
	222	+
	223	+test "an unusable id is no id rather than a rejected message" {
	224	+ // The message still has to be served. Refusing to dispatch it would turn a
	225	+ // page that sends something odd into a page where nothing works at all.
	226	+ for ([_][]const u8{
	227	+ \\{"a":"get","i":-1}
	228	+ ,
	229	+ \\{"a":"get","i":null}
	230	+ ,
	231	+ \\{"a":"get","i":"abc"}
	232	+ ,
	233	+ \\{"a":"get","i":{"nested":1}}
	234	+ ,
	235	+ \\{"a":"get","i":[1]}
	236	+ ,
	237	+ \\{"a":"get","i":true}
	238	+ ,
	239	+ }) |json| {
	240	+ try testing.expectEqual(@as(?u64, null), try envelopeId(json));
	241	+ }
	242	+}
	243	+
	244	+test "the largest id JavaScript can count to round-trips" {
	245	+ try testing.expectEqual(@as(?u64, 9007199254740991), try envelopeId(
	246	+ \\{"a":"get","i":9007199254740991}
	247	+ ));
	248	+}

packages/zig/test/injected_js_test.zigmodified+191-0

Changes to packages/zig/test/injected_js_test.zig

		⋯22 linesShow the 22 lines above this hunk@@ -23,6 +23,7 @@ const capability_actions = contracts.capabilities_actions;
23	23	const prefs = contracts.prefs;
24	24	const prefs_actions = contracts.prefs_actions;
25	25	const shortcut_registry = contracts.shortcuts;
	26	+const bridge_error = contracts.errors;
26	27
27	28	// Supplied by build.zig as named imports — a test module cannot embed
28	29	// files outside its own package path.
		⋯906 linesShow the 906 lines above this hunk@@ -935,3 +936,193 @@ test "craft.supports answers from the manifest, and fails open without one" {
935	936	try testing.expectEqualStrings("false", try fx.text("String(window.craft.supports('marketplace'))"));
936	937	try testing.expectEqualStrings("false", try fx.text("String(window.craft.supports('marketplace.list'))"));
937	938	}
	939	+
	940	+// =============================================================================
	941	+// Reply correlation (#66)
	942	+// =============================================================================
	943	+
	944	+/// Answer a call the way native does: the exact string `sendResultToJS`
	945	+/// evaluates in the page, built by the same function, id and all.
	946	+fn answer(fx: *Fixture, action: []const u8, payload: []const u8, id: ?u64) !void {
	947	+ const script = try bridge_error.formatResultJS(testing.allocator, action, payload, id);
	948	+ defer testing.allocator.free(script);
	949	+ _ = try fx.ctx.evaluate(script);
	950	+}
	951	+
	952	+test "a call carries an id, and the reply that names it resolves that call" {
	953	+ var fx = try Fixture.init();
	954	+ defer fx.deinit();
	955	+ _ = try fx.ctx.evaluate(WEBVIEW_HOST);
	956	+ _ = try fx.ctx.evaluate(BRIDGE);
	957	+
	958	+ _ = try fx.ctx.evaluate(
	959	+ \\var got = null;
	960	+ \\window.craft.tags.get('/a').then(function (t) { got = t.join(',') });
	961	+ );
	962	+
	963	+ // The page put its own id on the message; native reads it back out of the
	964	+ // envelope in `handleBridgeMessageJSON` and hands it to `formatResultJS`.
	965	+ const id_text = try fx.text("String(posted[0].i)");
	966	+ const id = try std.fmt.parseInt(u64, id_text, 10);
	967	+ try testing.expect(id > 0);
	968	+
	969	+ try answer(&fx, "get", "{\"tags\":[\"red\"]}", id);
	970	+ try testing.expectEqualStrings("red", try fx.text("String(got)"));
	971	+}
	972	+
	973	+test "two bridges sharing an action name do not swap replies" {
	974	+ // `get` is served by keychain and tags both. Answered out of order, which
	975	+ // is what a modal run loop or any slower handler produces, the action-name
	976	+ // queue alone hands each caller the other's payload.
	977	+ var fx = try Fixture.init();
	978	+ defer fx.deinit();
	979	+ _ = try fx.ctx.evaluate(WEBVIEW_HOST);
	980	+ _ = try fx.ctx.evaluate(BRIDGE);
	981	+
	982	+ _ = try fx.ctx.evaluate(
	983	+ \\var secret = 'unset';
	984	+ \\var labels = 'unset';
	985	+ \\window.craft.keychain.get('svc', 'acct').then(function (v) { secret = String(v) });
	986	+ \\window.craft.tags.get('/a').then(function (t) { labels = t.join(',') });
	987	+ );
	988	+
	989	+ try testing.expectEqualStrings("keychain", try fx.text("posted[0].t"));
	990	+ try testing.expectEqualStrings("tags", try fx.text("posted[1].t"));
	991	+
	992	+ const keychain_id = try std.fmt.parseInt(u64, try fx.text("String(posted[0].i)"), 10);
	993	+ const tags_id = try std.fmt.parseInt(u64, try fx.text("String(posted[1].i)"), 10);
	994	+ try testing.expect(keychain_id != tags_id);
	995	+
	996	+ // Tags answers first — the ordering that used to swap them.
	997	+ try answer(&fx, "get", "{\"tags\":[\"red\",\"blue\"]}", tags_id);
	998	+ try answer(&fx, "get", "{\"value\":\"hunter2\"}", keychain_id);
	999	+
	1000	+ try testing.expectEqualStrings("hunter2", try fx.text("secret"));
	1001	+ try testing.expectEqualStrings("red,blue", try fx.text("labels"));
	1002	+}
	1003	+
	1004	+test "a reply for an id nobody is waiting on resolves nobody" {
	1005	+ // A late reply, or a second reply to a call already answered. Without an id
	1006	+ // to check against, this payload went to whoever was at the head of the
	1007	+ // action queue — a different call's answer, delivered as correct.
	1008	+ var fx = try Fixture.init();
	1009	+ defer fx.deinit();
	1010	+ _ = try fx.ctx.evaluate(WEBVIEW_HOST);
	1011	+ _ = try fx.ctx.evaluate(BRIDGE);
	1012	+
	1013	+ _ = try fx.ctx.evaluate(
	1014	+ \\var got = 'unset';
	1015	+ \\window.craft.tags.get('/a').then(function (t) { got = t.join(',') });
	1016	+ );
	1017	+ const id = try std.fmt.parseInt(u64, try fx.text("String(posted[0].i)"), 10);
	1018	+
	1019	+ try answer(&fx, "get", "{\"tags\":[\"ghost\"]}", id + 1000);
	1020	+ try testing.expectEqualStrings("unset", try fx.text("got"));
	1021	+
	1022	+ try answer(&fx, "get", "{\"tags\":[\"mine\"]}", id);
	1023	+ try testing.expectEqualStrings("mine", try fx.text("got"));
	1024	+}
	1025	+
	1026	+test "a reply that arrives without an id still resolves its caller" {
	1027	+ // The fallback path, for a reply raised outside any dispatch — native has
	1028	+ // no request to name. `formatResultJS` renders `null` and the page matches
	1029	+ // by action name, exactly as it did before ids.
	1030	+ var fx = try Fixture.init();
	1031	+ defer fx.deinit();
	1032	+ _ = try fx.ctx.evaluate(WEBVIEW_HOST);
	1033	+ _ = try fx.ctx.evaluate(BRIDGE);
	1034	+
	1035	+ _ = try fx.ctx.evaluate(
	1036	+ \\var got = 'unset';
	1037	+ \\window.craft.tags.get('/a').then(function (t) { got = t.join(',') });
	1038	+ );
	1039	+ try answer(&fx, "get", "{\"tags\":[\"ok\"]}", null);
	1040	+ try testing.expectEqualStrings("ok", try fx.text("got"));
	1041	+}
	1042	+
	1043	+test "fire-and-forget messages carry an id too" {
	1044	+ // So a failure can name the exact send it came from instead of rejecting
	1045	+ // whatever request happens to share its action name.
	1046	+ var fx = try Fixture.init();
	1047	+ defer fx.deinit();
	1048	+ _ = try fx.ctx.evaluate(WEBVIEW_HOST);
	1049	+ _ = try fx.ctx.evaluate(BRIDGE);
	1050	+
	1051	+ _ = try fx.ctx.evaluate("window.craft.keychain.set('svc', 'acct', 'pw');");
	1052	+ try testing.expectEqualStrings("keychain", try fx.text("posted[0].t"));
	1053	+ try testing.expectEqualStrings("true", try fx.text("String(typeof posted[0].i === 'number' && posted[0].i > 0)"));
	1054	+}
	1055	+
	1056	+test "a failure rejects the call it names and leaves the others alone" {
	1057	+ // `isEnabled` is served by autoLaunch, bluetooth and crashReporter.
	1058	+ var fx = try Fixture.init();
	1059	+ defer fx.deinit();
	1060	+ _ = try fx.ctx.evaluate(WEBVIEW_HOST);
	1061	+ _ = try fx.ctx.evaluate(BRIDGE);
	1062	+
	1063	+ _ = try fx.ctx.evaluate(
	1064	+ \\var autoLaunch = 'pending';
	1065	+ \\var bluetooth = 'pending';
	1066	+ \\window.craft.autoLaunch.isEnabled().then(
	1067	+ \\ function (v) { autoLaunch = 'ok:' + v }, function (e) { autoLaunch = 'rejected' });
	1068	+ \\window.craft.bluetooth.isEnabled().then(
	1069	+ \\ function (v) { bluetooth = 'ok:' + v }, function (e) { bluetooth = 'rejected:' + e.message });
	1070	+ );
	1071	+
	1072	+ const bluetooth_id = try std.fmt.parseInt(u64, try fx.text("String(posted[1].i)"), 10);
	1073	+
	1074	+ var ctx = bridge_error.ErrorContext.init(
	1075	+ bridge_error.BridgeError.NativeCallFailed,
	1076	+ "isEnabled",
	1077	+ "no bluetooth",
	1078	+ );
	1079	+ ctx.request_id = bluetooth_id;
	1080	+ const json = try ctx.toJSON(testing.allocator);
	1081	+ defer testing.allocator.free(json);
	1082	+ const script = try std.fmt.allocPrint(testing.allocator, "window.__craftBridgeError({s});", .{json});
	1083	+ defer testing.allocator.free(script);
	1084	+ _ = try fx.ctx.evaluate(script);
	1085	+
	1086	+ try testing.expectEqualStrings("rejected:no bluetooth", try fx.text("bluetooth"));
	1087	+ // The other bridge never heard about it and still answers normally.
	1088	+ try testing.expectEqualStrings("pending", try fx.text("autoLaunch"));
	1089	+
	1090	+ const auto_id = try std.fmt.parseInt(u64, try fx.text("String(posted[0].i)"), 10);
	1091	+ try answer(&fx, "isEnabled", "{\"value\":true}", auto_id);
	1092	+ try testing.expectEqualStrings("ok:true", try fx.text("autoLaunch"));
	1093	+}
	1094	+
	1095	+test "the menubar state reply no longer needs a queue of its own" {
	1096	+ // `getState` used to be answered under an invented reply key,
	1097	+ // `menubarCollapse:getState`, with a hand-rolled pending entry on the JS
	1098	+ // side that had no timeout. It is an ordinary request now, and the reply
	1099	+ // is matched by id like every other — which also keeps it apart from
	1100	+ // screenSharing's `getState`, the seventh action name two bridges share.
	1101	+ var fx = try Fixture.init();
	1102	+ defer fx.deinit();
	1103	+ _ = try fx.ctx.evaluate(WEBVIEW_HOST);
	1104	+ _ = try fx.ctx.evaluate(BRIDGE);
	1105	+
	1106	+ _ = try fx.ctx.evaluate(
	1107	+ \\var menubar = 'pending';
	1108	+ \\var sharing = 'pending';
	1109	+ \\window.craft.menubar.getState().then(function (s) { menubar = String(s.collapsed) });
	1110	+ \\window.craft.screenSharing.getState().then(function (s) { sharing = String(s.sharing) });
	1111	+ );
	1112	+
	1113	+ try testing.expectEqualStrings("menubarCollapse", try fx.text("posted[0].t"));
	1114	+ try testing.expectEqualStrings("getState", try fx.text("posted[0].a"));
	1115	+ try testing.expectEqualStrings("screenSharing", try fx.text("posted[1].t"));
	1116	+
	1117	+ const menubar_id = try std.fmt.parseInt(u64, try fx.text("String(posted[0].i)"), 10);
	1118	+ const sharing_id = try std.fmt.parseInt(u64, try fx.text("String(posted[1].i)"), 10);
	1119	+
	1120	+ try answer(&fx, "getState", "{\"sharing\":true}", sharing_id);
	1121	+ try answer(&fx, "getState", "{\"collapsed\":true,\"initialized\":true,\"separatorHidden\":false}", menubar_id);
	1122	+
	1123	+ try testing.expectEqualStrings("true", try fx.text("menubar"));
	1124	+ try testing.expectEqualStrings("true", try fx.text("sharing"));
	1125	+
	1126	+ // And nothing is left queued under the key it used to invent.
	1127	+ try testing.expectEqualStrings("undefined", try fx.text("String(window.__craftBridgePending['menubarCollapse:getState'])"));
	1128	+}