ReviewOS

also looking at this

craft-native/craft

feat: add Linux and Windows named-window routing

#325
Merged glennmichael123 wants to merge codex/multi-window-linux-windows into main
29 files +2250 -200

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

.github/workflows/ci.ymlmodified+5-1
Changes to .github/workflows/ci.yml
@@ -51,7 +51,7 @@ jobs:
5151 if: runner.os == 'Linux'
5252 run: |
5353 sudo apt-get update
54 sudo apt-get install -y libgtk-3-dev libwebkit2gtk-4.1-dev
54 sudo apt-get install -y libgtk-3-dev libwebkit2gtk-4.1-dev xvfb xauth
5555
5656 - name: First-party Zig dependencies
5757 uses: ./.github/actions/first-party-zig-deps
@@ -90,6 +90,10 @@ jobs:
9090 zig build -Doptimize=ReleaseSafe -Dversion="$VERSION" -Dtarget=x86_64-windows
9191 test -f zig-out/bin/craft.exe
9292
93 - name: Smoke Linux multi-window bridge
94 if: runner.os == 'Linux'
95 run: bun scripts/verify-linux-multi-window.ts packages/zig/zig-out/bin/craft
96
9397 typescript-sdk:
9498 runs-on: ubuntu-latest
9599
docs/features/window-management.mdmodified+10-8
Changes to docs/features/window-management.md
@@ -29,8 +29,9 @@ await show(html, {
2929### Runtime Window Creation
3030
3131The `craft-native` SDK returns a typed `Window` handle. Runtime-created
32windows are currently implemented by the macOS host and must provide either
33`html` or `url` content.
32windows are available on macOS, Linux, and Windows and must provide either
33`html` or `url` content. The common create, close, title, geometry, and event
34routes are cross-platform; some advanced window controls remain macOS-only.
3435
3536```typescript
3637import { createWindow } from 'craft-native'
@@ -477,12 +478,13 @@ that created it sees the same transitions through the `inspector` handle.
477478Other open windows do not receive them. Call the unsubscribe function returned
478479by `on`, such as `stopWatching()` above, when the listener is no longer needed.
479480
480Closing a window keeps its native page alive so the macOS reopen lifecycle can
481restore its DOM and JavaScript state. Call `destroy()` on a runtime-created
482handle to permanently release its native window, webview, recovery state and
483event subscriptions; opening the same ID afterwards creates a fresh native
484page behind the same stable SDK handle. The unnamed primary window cannot be
485force-destroyed through this API.
481On macOS, closing a window keeps its native page alive so reopening can
482restore its DOM and JavaScript state. On Linux and Windows, closing releases
483the native window and webview; reopening the same ID creates a fresh page
484behind the same stable SDK handle. Call `destroy()` to permanently release a
485runtime-created window and its event subscriptions. The unnamed primary window
486cannot be force-destroyed through this API. CI exercises the Linux
487create/close/reopen path under Xvfb.
486488Modal/parent semantics are not part of the runtime-created-window contract yet.
487489See [Multi-window ownership](../architecture/multi-window-ownership.md) for the
488490implemented routing guarantees, application-level event sinks and remaining
scripts/verify-linux-multi-window.tsadded+146-0
Changes to scripts/verify-linux-multi-window.ts
@@ -0,0 +1,146 @@
1/** Exercise the shipped Linux binary's page-to-native multi-window bridge. */
2import { spawn } from 'node:child_process'
3import { createServer } from 'node:http'
4import type { AddressInfo } from 'node:net'
5
6const binary = process.argv[2]
7if (!binary) throw new Error('usage: bun scripts/verify-linux-multi-window.ts <craft-binary>')
8
9const mainPage = `<!doctype html><script>
10(async () => {
11 const report = (step) => fetch('/report?step=' + encodeURIComponent(step), { method: 'POST' })
12 const waitFor = async (step) => {
13 for (let attempt = 0; attempt < 200; attempt++) {
14 const status = await (await fetch('/status')).json()
15 if (status.steps.includes(step)) return
16 await new Promise(resolve => setTimeout(resolve, 100))
17 }
18 throw new Error('timed out waiting for ' + step)
19 }
20 const child = (cycle) => window.craft.window.open({
21 name: 'settings', title: 'Child ' + cycle,
22 url: location.origin + '/child?cycle=' + cycle,
23 })
24 try {
25 if (!window.craft || !window.craft.window || !window.craft.window.open)
26 throw new Error('document-start Craft bridge missing')
27 await report('main')
28 const first = await child(1)
29 if (first.name !== 'settings') throw new Error('wrong child handle')
30 await waitFor('child-1')
31 const bounds = await window.craft.window._call('getBounds', {}, 'settings')
32 if (!(bounds.width > 0 && bounds.height > 0)) throw new Error('child bounds not routed to creator')
33 const title = await window.craft.window._call('getTitle', {}, 'settings')
34 if (title !== 'Child 1') throw new Error('child title not routed to creator: ' + title)
35
36 const closed = new Promise((resolve, reject) => {
37 const timeout = setTimeout(() => reject(new Error('child close event missing')), 20000)
38 window.addEventListener('craft:window:close', (event) => {
39 if (event.detail.windowId !== 'settings') return
40 clearTimeout(timeout)
41 resolve()
42 })
43 })
44 await window.craft.window._call('close', {}, 'settings')
45 await closed
46 await report('closed')
47 let staleRejected = false
48 try { await window.craft.window._call('getBounds', {}, 'settings') }
49 catch (_) { staleRejected = true }
50 if (!staleRejected) throw new Error('destroyed child still answered a read')
51
52 const second = await child(2)
53 if (second.name !== 'settings') throw new Error('reopened child has wrong handle')
54 await waitFor('child-2')
55 const reopenedTitle = await window.craft.window._call('getTitle', {}, 'settings')
56 if (reopenedTitle !== 'Child 2') throw new Error('reopened child retained stale title')
57 await report('done')
58 }
59 catch (error) {
60 await fetch('/report?error=' + encodeURIComponent(String(error)), { method: 'POST' })
61 }
62})()
63</script>`
64
65const childPage = `<!doctype html><script>
66(async () => {
67 try {
68 const cycle = new URLSearchParams(location.search).get('cycle')
69 const bounds = await window.craft.window._call('getBounds', {}, 'main')
70 const title = await window.craft.window._call('getTitle', {}, 'main')
71 if (!(bounds.width > 0 && bounds.height > 0) || title !== 'Child ' + cycle)
72 throw new Error('child page read targeted another webview')
73 await fetch('/report?step=child-' + cycle, { method: 'POST' })
74 }
75 catch (error) {
76 await fetch('/report?error=' + encodeURIComponent(String(error)), { method: 'POST' })
77 }
78})()
79</script>`
80
81const steps = new Set<string>()
82let resolveDone!: () => void
83let rejectDone!: (error: Error) => void
84const done = new Promise<void>((resolve, reject) => {
85 resolveDone = resolve
86 rejectDone = reject
87})
88const server = createServer((request, response) => {
89 const url = new URL(request.url ?? '/', 'http://127.0.0.1')
90 if (url.pathname === '/main' || url.pathname === '/child') {
91 response.setHeader('content-type', 'text/html; charset=utf-8')
92 response.end(url.pathname === '/main' ? mainPage : childPage)
93 }
94 else if (url.pathname === '/status') {
95 response.setHeader('content-type', 'application/json')
96 response.end(JSON.stringify({ steps: [...steps] }))
97 }
98 else if (url.pathname === '/report') {
99 const error = url.searchParams.get('error')
100 const step = url.searchParams.get('step')
101 if (error) rejectDone(new Error(error))
102 if (step) steps.add(step)
103 response.end('ok')
104 if (step === 'done') {
105 const required = ['main', 'child-1', 'closed', 'child-2', 'done']
106 if (required.every(name => steps.has(name))) resolveDone()
107 else rejectDone(new Error(`incomplete smoke steps: ${[...steps].join(', ')}`))
108 }
109 }
110 else {
111 response.writeHead(404).end()
112 }
113})
114
115await new Promise<void>((resolve, reject) => {
116 server.once('error', reject)
117 server.listen(0, '127.0.0.1', resolve)
118})
119const port = (server.address() as AddressInfo).port
120const child = spawn('timeout', ['110s', 'xvfb-run', '-a', binary, '--url', `http://127.0.0.1:${port}/main`], {
121 detached: true,
122 stdio: ['ignore', 'pipe', 'pipe'],
123})
124let output = ''
125for (const stream of [child.stdout, child.stderr]) {
126 stream.on('data', chunk => { output = (output + chunk.toString()).slice(-16_000) })
127}
128child.once('error', error => rejectDone(error))
129child.once('exit', (code, signal) => rejectDone(new Error(`Craft exited before smoke completed (${code ?? signal})`)))
130const timer = setTimeout(() => rejectDone(new Error('Linux multi-window smoke timed out')), 100_000)
131
132try {
133 await done
134 console.log(`Linux multi-window smoke passed: ${[...steps].join(', ')}`)
135}
136catch (error) {
137 throw new Error(`${String(error)}\nCraft output:\n${output}`)
138}
139finally {
140 clearTimeout(timer)
141 if (child.pid) {
142 try { process.kill(-child.pid, 'SIGTERM') }
143 catch { /* The process group may have already exited. */ }
144 }
145 await new Promise<void>(resolve => server.close(() => resolve()))
146}