ReviewOS

also looking at this

stacks/drone

test: observe native execution callback verification cost

#86
Merged chris wants to merge codex/observe-execution-verification into main
8 files +363 -18

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+2-0
Changes to .github/workflows/ci.yml
@@ -511,6 +511,8 @@ jobs:
511511 ${{ runner.temp }}/execution-browser-evidence/browser-launch.json
512512 ${{ runner.temp }}/execution-browser-evidence/failure-diagnostics.json
513513 ${{ runner.temp }}/execution-browser-evidence/admission-renewal.json
514 ${{ runner.temp }}/execution-browser-evidence/pre-admission-renewal.json
515 ${{ runner.temp }}/execution-browser-evidence/completion-read-renewal.json
514516 ${{ runner.temp }}/execution-browser-evidence/chrome-startup-stderr.log
515517 purge-namespace:
516518 runs-on: ubuntu-latest
tests/fixtures/public-safety-access/execution-browser.tsmodified+51-11
Changes to tests/fixtures/public-safety-access/execution-browser.ts
@@ -23,6 +23,15 @@ let delayStart = true, releaseStart: (() => void) | undefined
2323let streamStarts = 0, startRequests = 0, admissionRequests = 0, closeStream: (() => Promise<void>) | undefined
2424let closeWorkspaceStream: (() => Promise<void>) | undefined, releaseWorkspace: (() => void) | undefined
2525let holdWorkspace = false, workspaceHeld = false
26let preAdmissionRenewalArmed = false, preAdmissionRenewed = false, preAdmissionHoldExpired = false
27const preAdmissionReads = new Set<() => void>()
28let preAdmissionTimeout: ReturnType<typeof setTimeout> | undefined
29function releasePreAdmissionReads() {
30 preAdmissionRenewalArmed = false
31 clearTimeout(preAdmissionTimeout)
32 for (const release of preAdmissionReads) release()
33 preAdmissionReads.clear()
34}
2635
2736// Diagnostic state belongs only to this disposable fixture, never the execution controller.
2837type RouteClass = 'execution-start' | 'execution-close' | 'execution-admission' | 'execution-events' | 'execution-status' | 'workspace-read'
@@ -53,7 +62,7 @@ function diagnosticSnapshot() {
5362 }
5463 return {
5564 available: true, pendingChecks: scalar(owner.pendingChecks),
56 fixture: { startRequests, admissionRequests, streamStarts, holdArmed, held, holdUsed, holdWorkspace, workspaceHeld }, lastStart, lastAdmission, responses: responses.slice(),
65 fixture: { startRequests, admissionRequests, streamStarts, holdArmed, held, holdUsed, holdWorkspace, workspaceHeld, preAdmissionRenewalArmed, preAdmissionRenewed, preAdmissionHeldReads: preAdmissionReads.size, preAdmissionHoldExpired }, lastStart, lastAdmission, responses: responses.slice(),
5766 slot: slot === null ? null : {
5867 ordinal: ordinal(slot),
5968 phase: choice(field(slot, 'phase'), ['waiting-for-consumer', 'running', 'stopping', 'completed', 'canceled', 'failed', 'discarded'] satisfies ExecutionView['phase'][]),
@@ -102,10 +111,18 @@ assert(!html.includes('<IncidentWorkspace'))
102111assert(!html.includes(fixture.senderToken))
103112const secret = randomUUID()
104113const sources = () => fixture.database.db.primary.selectFrom('public_safety_sources').selectAll().execute()
114async function holdWorkspaceRenewal() {
115 holdWorkspace = true
116 assert(closeWorkspaceStream, 'The real workspace stream must exist before renewal')
117 await closeWorkspaceStream()
118 const until = Date.now() + 5000
119 while (!workspaceHeld && Date.now() < until) await Bun.sleep(5)
120 assert(workspaceHeld, 'Native workspace renewal must request a fresh authorized snapshot')
121}
105122const control = async (request: Request) => {
106123 if (request.headers.get('x-fixture-control') !== secret) return new Response(null, { status: 404 })
107124 const action = new URL(request.url).pathname.slice('/__fixture/'.length)
108 if (action === 'status') return Response.json({ held, streamStarts, startRequests, startResponseReady: !!releaseStart })
125 if (action === 'status') return Response.json({ held, streamStarts, startRequests, startResponseReady: !!releaseStart, workspaceHeld, preAdmissionRenewed, preAdmissionHeldReads: preAdmissionReads.size, preAdmissionHoldExpired, admissionRequests })
109126 if (action === 'diagnostics') {
110127 try {
111128 const text = `${JSON.stringify(diagnosticSnapshot())}\n`
@@ -134,21 +151,34 @@ const control = async (request: Request) => {
134151 }
135152 if (action === 'release-and-renew') { releaseCredit?.(); await closeStream?.(); return Response.json({ ok: true }) }
136153 if (action === 'before-admission') { assert.equal((await sources()).length, 1, 'Completion does not write a durable source'); return Response.json({ ok: true }) }
154 if (action === 'hold-completion-read-renewal') {
155 assert.equal(field(executionOwner().slot, 'phase'), 'completed')
156 assert.equal(admissionRequests, 0)
157 await holdWorkspaceRenewal()
158 return Response.json({ ok: true })
159 }
160 if (action === 'arm-pre-admission-renewal') {
161 assert(!preAdmissionRenewalArmed && !preAdmissionRenewed && admissionRequests === 0 && closeWorkspaceStream, 'Arm one renewal before any admission POST')
162 preAdmissionRenewalArmed = true
163 return Response.json({ ok: true })
164 }
165 if (action === 'renew-pre-admission-held-read') {
166 assert(preAdmissionRenewalArmed && !preAdmissionRenewed && !preAdmissionHoldExpired && preAdmissionReads.size > 0 && admissionRequests === 0, 'Renew only while an explicit decision is waiting on held native reads')
167 assert(closeWorkspaceStream, 'The real workspace stream must exist before admission')
168 preAdmissionRenewed = true
169 await closeWorkspaceStream()
170 return Response.json({ ok: true })
171 }
137172 if (action === 'verify-admitted') {
138173 const rows = await sources()
139174 assert.equal(rows.length, 2)
140175 assert(rows.some(row => row.digest !== seeded.source.digest))
141176 const committed = await fixture.snapshot(seeded.key)
142177 assert.equal(committed.audits.filter(audit => audit.action === 'source-registered').length, 2)
143 holdWorkspace = true
144 assert(closeWorkspaceStream, 'The real workspace stream must exist before renewal')
145 await closeWorkspaceStream()
146 const until = Date.now() + 5000
147 while (!workspaceHeld && Date.now() < until) await Bun.sleep(5)
148 assert(workspaceHeld, 'Native workspace renewal must request a fresh authorized snapshot')
178 await holdWorkspaceRenewal()
149179 return Response.json({ ok: true })
150180 }
151 if (action === 'release-workspace') { assert(workspaceHeld, 'Release only the held native renewal'); releaseWorkspace?.(); return Response.json({ ok: true }) }
181 if (action === 'release-workspace') { assert(workspaceHeld || preAdmissionReads.size > 0, 'Release only held native reads'); releaseWorkspace?.(); releasePreAdmissionReads(); return Response.json({ ok: true }) }
152182 if (action === 'read-only') return Response.json({ token: await fixture.credential(fixture.sender.id, ['public-safety:read']) })
153183 if (action === 'revoke') { await fixture.tokens.revokeToken(fixture.senderToken); return Response.json({ ok: true }) }
154184 return new Response(null, { status: 404 })
@@ -162,6 +192,16 @@ route.use(async (request: Request, next: () => Promise<Response | null> | Respon
162192 if (admissionOrdinal !== null) lastAdmission = { ordinal: admissionOrdinal, settled: false, status: null, elapsedMs: null }
163193 let status: number | null = null
164194 try {
195 if (kind === 'workspace-read' && preAdmissionRenewalArmed) {
196 // Ordinary polls may arrive first. Keep authorization visible until the driver
197 // observes a real pending admission decision, then explicitly renew the stream.
198 assert(preAdmissionReads.size < 8, 'The bounded native read barrier must not overflow')
199 await new Promise<void>(resolve => {
200 const release = () => { preAdmissionReads.delete(release); resolve() }
201 preAdmissionReads.add(release)
202 if (!preAdmissionTimeout) preAdmissionTimeout = setTimeout(() => { preAdmissionHoldExpired = true; releasePreAdmissionReads() }, 5000)
203 })
204 }
165205 if (kind === 'workspace-read' && holdWorkspace) {
166206 holdWorkspace = false; workspaceHeld = true
167207 await new Promise<void>(resolve => {
@@ -207,7 +247,7 @@ route.use(async (request: Request, next: () => Promise<Response | null> | Respon
207247})
208248const page = () => new Response(html, { headers: { 'content-type': 'text/html', 'cache-control': 'no-store' } })
209249route.get('/incidents', page)
210for (const action of ['status', 'diagnostics', 'release-start', 'arm-before-return', 'before-return', 'release-and-renew', 'before-admission', 'verify-admitted', 'release-workspace', 'read-only', 'revoke']) route.get(`/__fixture/${action}`, control)
250for (const action of ['status', 'diagnostics', 'release-start', 'arm-before-return', 'before-return', 'release-and-renew', 'before-admission', 'hold-completion-read-renewal', 'arm-pre-admission-renewal', 'renew-pre-admission-held-read', 'verify-admitted', 'release-workspace', 'read-only', 'revoke']) route.get(`/__fixture/${action}`, control)
211251const server = await route.serve({ hostname: '127.0.0.1', port: 0, nativeRoutes: false })
212252if (process.env.EXECUTION_BROWSER_EVIDENCE) { await mkdir(process.env.EXECUTION_BROWSER_EVIDENCE, { recursive: true }); await writeFile(join(process.env.EXECUTION_BROWSER_EVIDENCE, 'compiled.html'), html) }
213253await writeFile(join(root, 'execution-credentials.json'), JSON.stringify({ url: `http://127.0.0.1:${server.port}`, key: seeded.key, sourceDigest: seeded.source.digest, token: fixture.senderToken, control: secret }), { mode: 0o600 })
@@ -216,7 +256,7 @@ try {
216256 assert.equal(await child.exited, 0, 'Isolated real execution browser driver must pass')
217257}
218258finally {
219 releaseStart?.(); releaseCredit?.(); releaseWorkspace?.()
259 releaseStart?.(); releaseCredit?.(); releaseWorkspace?.(); releasePreAdmissionReads()
220260 await closeStream?.(); await closeWorkspaceStream?.(); await server.stop(true)
221261 const slot = executionOwner().slot as { execution: string, binding: { scope: string }, producer: string, worker: Worker | null } | null
222262 if (slot?.producer === 'pending') {
tests/helpers/execution-browser-driver.tsmodified+42-4
Changes to tests/helpers/execution-browser-driver.ts
@@ -121,11 +121,25 @@ try {
121121 await first.fill('#execution-scenario', 'nominal'); await first.fill('#execution-seconds', '100')
122122 await first.click('Start synthetic run')
123123 mark('real nominal100 completion')
124 await first.wait("document.querySelector('[data-execution-phase]')?.textContent.trim() === 'completed' && !!document.querySelector('[data-execution-completion]')", 'Actual Worker recording must complete', 40000)
124 // Consume the completed fields from the same authorized DOM frame that satisfies readiness.
125 // A later evaluation could run after native renewal has correctly hidden private content.
126 const completedView = `(() => {
127 const status = document.querySelector('[data-workspace-status]')?.textContent.trim();
128 const phase = document.querySelector('[data-execution-phase]')?.textContent.trim();
129 const digest = document.querySelector('[data-execution-digest]')?.textContent.trim();
130 const execution = document.querySelector('[data-execution-id]')?.textContent.trim();
131 const originalSelected = document.querySelector('[data-workspace-source]')?.textContent.trim() === ${JSON.stringify(data.sourceDigest)};
132 return status === 'current' && phase === 'completed' && !!document.querySelector('[data-execution-completion]') && /^[a-f0-9]{64}$/.test(digest || '') && !!execution && originalSelected ? { digest, execution } : false;
133 })()`
134 const completed = await first.wait<{ digest: string, execution: string }>(completedView, 'Actual Worker recording must complete', 40000)
125135 await control('before-admission')
126 const digest = await first.evaluate<string>("document.querySelector('[data-execution-digest]')?.textContent.trim()")
127 assert(/^[a-f0-9]{64}$/.test(digest))
128 assert.equal(await first.evaluate("document.querySelector('[data-workspace-source]')?.textContent.trim()"), data.sourceDigest, 'Completion cannot change selected imagery')
136 await control('hold-completion-read-renewal')
137 await first.wait("!document.querySelector('[data-execution-completion]')", 'Native completion-read renewal clears private content')
138 await control('release-workspace')
139 const restored = await first.wait<typeof completed>(completedView, 'Fresh authorization must restore the same completed result')
140 assert.deepEqual(restored, completed)
141 const { digest } = completed
142 await writeFile(join(evidence, 'completion-read-renewal.json'), `${JSON.stringify({ privateContentCleared: true, sameCompletedResultRestored: true, readinessFieldsReadAtomically: true })}\n`, { mode: 0o600, flag: 'wx' })
129143 await first.evaluate("document.querySelector('[data-execution-panel]').scrollIntoView()")
130144 await first.screenshot(join(evidence, 'execution-completed-desktop.png'))
131145 await first.send('Emulation.setDeviceMetricsOverride', { width: 375, height: 900, deviceScaleFactor: 1, mobile: true })
@@ -146,8 +160,32 @@ try {
146160 observer.observe(document.body, { childList: true, characterData: true, subtree: true });
147161 sample(); globalThis.executionSourceProbe = { probe, stop: () => observer.disconnect() };
148162 })()`)
163 await control('arm-pre-admission-renewal')
164 // Force the ordinary refresh ordering that can also occur from the native poll.
165 // The held read must leave the admission action available until this explicit click.
166 await first.click('Refresh current records')
167 let preAdmissionHeld = false
168 for (let attempt = 0; attempt < 100 && !preAdmissionHeld; attempt++) { preAdmissionHeld = (await control('status')).preAdmissionHeldReads > 0; if (!preAdmissionHeld) await Bun.sleep(10) }
169 assert(preAdmissionHeld, 'An ordinary native refresh must reach the read barrier before admission')
170 assert.equal((await control('status')).preAdmissionRenewed, false, 'Holding an ordinary refresh never conceals authorization')
171 await first.click('Admit completed source')
172 await first.wait("document.querySelector('[data-workspace-status]')?.textContent.trim() === 'current' && document.querySelector('[data-execution-phase]')?.textContent.trim() === 'completed' && document.querySelector('[data-execution-admit]')?.disabled === true", 'The explicit admission decision must be pending while native reads are held')
173 await control('renew-pre-admission-held-read')
174 await first.wait("!document.querySelector('[data-workspace-source]') && !document.querySelector('[data-execution-completion]')", 'Native renewal clears private content during the admission read')
175 await control('release-workspace')
176 assert.deepEqual(await first.wait<typeof completed>(completedView, 'Fresh authorization restores completed execution after the canceled admission'), completed)
177 const canceledDecision = await control('status')
178 assert.equal(canceledDecision.preAdmissionRenewed, true)
179 assert.equal(canceledDecision.preAdmissionHoldExpired, false, 'The controlled native renewal must complete within the original read bound')
180 assert.equal(canceledDecision.admissionRequests, 0, 'Lost availability cancels the earlier decision before any admission POST')
181 assert.equal(await first.evaluate("!!document.querySelector('[data-execution-source-exists]')"), false)
182 await control('before-admission')
183 mark('explicit source admission after fresh authorization')
149184 await first.click('Admit completed source')
150185 await first.wait("!!document.querySelector('[data-execution-source-exists]')", 'Explicit admission must refresh actual source catalog', 15000)
186 const acceptedDecision = await control('status')
187 assert.equal(acceptedDecision.admissionRequests, 1, 'Only the new explicit decision submits one admission POST')
188 await writeFile(join(evidence, 'pre-admission-renewal.json'), `${JSON.stringify({ ordinaryRefreshHeldBeforeClick: preAdmissionHeld, pendingDecisionObservedBeforeRenewal: true, nativeRenewalObserved: canceledDecision.preAdmissionRenewed, postsAfterCanceledDecision: canceledDecision.admissionRequests, postsAfterFreshExplicitDecision: acceptedDecision.admissionRequests, originalSelectionRestored: true })}\n`, { mode: 0o600, flag: 'wx' })
151189 await control('verify-admitted')
152190 mark('fresh workspace after admission renewal')
153191 assert.equal(await first.evaluate("!!document.querySelector('[data-workspace-source]') || !!document.querySelector('[data-execution-completion]')"), false, 'Native renewal must clear private panes before fresh authorization')
tests/helpers/workspace-browser.tsmodified+2-2
Changes to tests/helpers/workspace-browser.ts
@@ -79,10 +79,10 @@ export async function startWorkspaceBrowser(root: string) {
7979 }
8080 const page = {
8181 errors, requests, send, evaluate,
82 async wait(expression: string, message: string, timeout = 10000) {
82 async wait<T = boolean>(expression: string, message: string, timeout = 10000): Promise<T> {
8383 phase = message
8484 const end = Date.now() + timeout
85 while (Date.now() < end) { if (await evaluate(expression)) return; await Bun.sleep(50) }
85 while (Date.now() < end) { const value = await evaluate<T | false>(expression); if (value) return value; await Bun.sleep(50) }
8686 throw new Error(message)
8787 },
8888 async checkLiveness() { phase = 'post-renewal liveness'; assert.equal(await evaluate('1'), 1) },
tests/unit/public-safety-execution-client.test.tsmodified+34-0
Changes to tests/unit/public-safety-execution-client.test.ts
@@ -197,3 +197,37 @@ test('uncertain admission checks its digest beyond the currently paged catalog w
197197 }
198198 finally { model.close() }
199199})
200
201test('renewal during an admission authorization read cancels that decision before POST and requires a new explicit decision', async () => {
202 let intent = '', admissions = 0, refreshes = 0
203 const model = openIncidentExecution({ endpoint: () => '/execution', request: async (path, init) => {
204 if (init?.method !== 'POST') return Response.json(intent ? completed(intent) : null)
205 if (path.endsWith('/admission')) { admissions++; return Response.json({ admitted: true }) }
206 intent = JSON.parse(String(init.body)).intent
207 return Response.json(completed(intent))
208 }, stream: () => ({ close() {} }), refreshWorkspace: async () => {
209 refreshes++
210 if (refreshes === 1) {
211 model.sync({ ...context, available: false })
212 expect(model.get().execution).toBeNull()
213 expect(model.get().sourceExists).toBe(false)
214 model.sync(context)
215 await model.refresh()
216 }
217 else model.sync({ ...context, version: 19, catalogDigests: admissions ? ['b'.repeat(64)] : [] })
218 } })
219 try {
220 model.sync(context)
221 expect(await model.start('nominal', 100)).toBe(true)
222 expect(await model.admit()).toBe(false)
223 expect(admissions).toBe(0)
224 expect(model.get().execution?.phase).toBe('completed')
225 expect(model.get().execution?.admission).toBe('unsubmitted')
226 expect(model.get().busy).toBe(false)
227 expect(model.get().sourceExists).toBe(false)
228 expect(await model.admit()).toBe(true)
229 expect(admissions).toBe(1)
230 expect(model.get().sourceExists).toBe(true)
231 }
232 finally { model.close() }
233})