fix(mobile): hand the started document to the mount in the turn that started it

Round 6's two LOW items, and the pins for the owner-checked release.

LOW 1. `started` was assigned in the `.then` after the build, a microtask later
than the start sequence and the resize listener it installs. A dispose in that
window found nothing started, skipped the teardown and released the page with the
document still running on it. The build now takes an `adopt` callback and calls it
as its last statement, inside the guarded region, so whoever has to undo the
start is holding it before that turn ends. Pinned by queuing the dispose behind
the document import the build awaits, which lands in exactly that window: without
the change the started document's resize listener survives the dispose, five red
runs out of five.

The owner-checked release, which landed in 8b37221b57 without a pin of its own.
The one path that reaches a mount's cleanup holding someone else's page is a
rejected import: everywhere else the build re-reads the claim after its await and
stops, but a rejection never gets that far. So the pin drives that — the chunk
fails for the first mount only, the mount is disposed while pending, a second one
is built into the same element as Reload does, and then the first rejection
arrives. Without the guard inside `release` it empties the live mount's host:
three red runs out of three, on the markup. It also disposes the abandoned handle
a second time afterwards and asserts nothing moves, which is LOW 2's missing pin
for round 5's F2.

That case is its own file because the import has to fail before the mount module
loads, and the mocking the failure needs is only permitted in `.test.ts` — the
anti-slop override does not cover `.test.mjs`, which is what refused the port
recording in the render fixture's case. It fails once, so the mount that replaces
it gets real modules and is a live document worth protecting; its own resize
listener is the witness that it started.

Two oracles were dropped. Vitest reports its own message when a mock factory
throws, not the one thrown, so which import failed is read from the factory's
counter instead. And a counter of successful factory calls read zero even though
the second mount got a working document, which measures vitest's caching rather
than this code; the live mount's listener replaced it.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
This commit is contained in:
Jinwoo-H
2026-09-20 17:57:36 -04:00
parent 8b37221b57
commit 63eb8a40ae
3 changed files with 150 additions and 22 deletions
@@ -0,0 +1,78 @@
// @vitest-environment happy-dom
import { describe, expect, it, vi } from 'vitest'
/**
* What a mount whose chunk never arrived is allowed to touch on its way out.
*
* The one path where a mount reaches its own cleanup holding a page that belongs to someone else.
* Everywhere else the build reads the claim again after its import and stops, but a rejected
* import never gets that far: the failure arrives at the mount's error handler directly, and by
* then the overlay's Reload may already have built a second document into the same element. A
* release that emptied the host anyway would blank the terminal on the screen and hand the page
* back while its document ran on.
*
* Its own file because making the import fail is the only way to reach this, and the mock has to
* be in place before the mount module is loaded. It fails once, so the second mount gets the real
* modules and can be a live document to protect.
*/
const { chunk } = vi.hoisted(() => ({ chunk: { failures: 0 } }))
vi.mock('./document/page-document-modules', async (importOriginal) => {
if (chunk.failures === 0) {
chunk.failures += 1
throw new Error('orca-document-chunk-failed')
}
return importOriginal()
})
const { mountTerminalWebDocument } = await import('./terminal-web-document-mount')
const HOST_CLASS = 'orca-terminal-document-host'
describe('a page mount whose document chunk failed', () => {
it('leaves the document that replaced it alone', async () => {
const host = document.createElement('div')
document.body.appendChild(host)
let resizeListeners = 0
const realAdd = window.addEventListener.bind(window)
const realRemove = window.removeEventListener.bind(window)
window.addEventListener = (type, listener, options) => {
resizeListeners += type === 'resize' ? 1 : 0
realAdd(type, listener, options)
}
window.removeEventListener = (type, listener, options) => {
resizeListeners -= type === 'resize' ? 1 : 0
realRemove(type, listener, options)
}
const abandoned = mountTerminalWebDocument(host, () => {})
abandoned.dispose()
// The same element, as React hands it back on the overlay's Reload.
const live = mountTerminalWebDocument(host, () => {})
// The message is the mocking layer's, not the one thrown, so the two counters are what say
// which import did what: the abandoned mount's failed, and the live mount's did not.
await expect(abandoned.ready).rejects.toThrow()
expect(chunk.failures, 'the abandoned mount is the one whose chunk failed').toBe(1)
expect(host.querySelector('#terminal-container')).not.toBe(null)
expect(host.classList.contains(HOST_CLASS)).toBe(true)
// Still claimed, so the release did not hand the page back either.
expect(() => mountTerminalWebDocument(host, () => {})).toThrow(
'the terminal document is already mounted on this page'
)
await live.ready
// The other half of the precondition: the mount that replaced it is a real started document,
// not a second casualty. Its resize listener is the one the start sequence adds.
expect(resizeListeners, 'the live mount started its document').toBe(1)
// And disposing the abandoned handle a second time changes nothing.
abandoned.dispose()
expect(host.querySelector('#terminal-container')).not.toBe(null)
expect(host.classList.contains(HOST_CLASS)).toBe(true)
live.dispose()
window.addEventListener = realAdd
window.removeEventListener = realRemove
expect(host.querySelector('#terminal-container')).toBe(null)
expect(resizeListeners, 'and it took its listener back on the way out').toBe(0)
})
})
@@ -169,23 +169,19 @@ export function mountTerminalWebDocument(
throw error throw error
} }
const ready = buildTerminalWebDocument(host, receive, token).then( // Adopted by the build itself, in the same turn as the start sequence and the listener it adds,
(built) => { // rather than when this promise settles. A `.then` runs a microtask later, and a dispose in
if (built === null) { // between would find nothing started, skip the teardown and hand the page back with the
// Disposed while the import was in flight, and the build stopped at the await without // document still running on it.
// touching anything. Nothing to keep and nothing to give back. const ready = buildTerminalWebDocument(host, receive, token, (built) => {
return started = built
} }).catch((error: unknown) => {
started = built // The import failed, so nothing was started and the page has to go back — the overlay's
}, // Reload is a second mount and it must be allowed to make one. A later mount may already
(error: unknown) => { // hold the page, which `release` answers for.
// The import failed, so nothing was started and the page has to go back — the overlay's release()
// Reload is a second mount and it must be allowed to make one. A later mount may already throw error
// hold the page, which `release` answers for. })
release()
throw error
}
)
return { return {
send: (command) => { send: (command) => {
@@ -230,7 +226,7 @@ function teardownStartedDocument({ modules, onWindowResize }: StartedDocument) {
} }
/** /**
* The document, built and started — or `null` if the page stopped being this mount's. * Builds and starts the document, and hands it to `adopt` — or returns having done neither.
* *
* The token is read again the instant the import lands, before anything below it runs. Every * The token is read again the instant the import lands, before anything below it runs. Every
* statement after this point writes shared state: the six seams are fields on a module-singleton * statement after this point writes shared state: the six seams are fields on a module-singleton
@@ -239,15 +235,19 @@ function teardownStartedDocument({ modules, onWindowResize }: StartedDocument) {
* running the body anyway would plant its elements' listeners into a page a later mount is using * running the body anyway would plant its elements' listeners into a page a later mount is using
* and reset that mount's scope out from under it. Checking only when this resolves is too late: * and reset that mount's scope out from under it. Checking only when this resolves is too late:
* by then the writes have happened and the caller can do nothing but discard the result. * by then the writes have happened and the caller can do nothing but discard the result.
*
* `adopt` rather than a return value for the same reason: what it hands over is what undoes all of
* that, and the caller has to be holding it before this function's turn ends.
*/ */
async function buildTerminalWebDocument( async function buildTerminalWebDocument(
host: HTMLElement, host: HTMLElement,
receive: (message: Record<string, unknown>) => void, receive: (message: Record<string, unknown>) => void,
token: symbol token: symbol,
): Promise<StartedDocument | null> { adopt: (built: StartedDocument) => void
): Promise<void> {
const documentModules = await import('./document/page-document-modules') const documentModules = await import('./document/page-document-modules')
if (liveDocument !== token) { if (liveDocument !== token) {
return null return
} }
const { scope } = documentModules const { scope } = documentModules
@@ -300,5 +300,5 @@ async function buildTerminalWebDocument(
} }
window.addEventListener('resize', onWindowResize) window.addEventListener('resize', onWindowResize)
return { modules: documentModules, onWindowResize } adopt({ modules: documentModules, onWindowResize })
} }
@@ -222,6 +222,56 @@ describe('the page terminal document', () => {
remounted.dispose() remounted.dispose()
}) })
it('tears down a document that started, however late the dispose is', async () => {
// The start sequence and the handle on what undoes it have to land in one turn. They did not:
// the build started the document, installed its listeners and returned, and the assignment
// that recorded it ran a microtask later — so a dispose in between found nothing started,
// skipped the teardown and handed the page back with the document still running on it. The
// dispose here is queued behind the document module import the build awaits, which puts it in
// that window rather than before or after it.
const host = document.createElement('div')
document.body.appendChild(host)
const listeners = new Set<unknown>()
const realAdd = window.addEventListener.bind(window)
const realRemove = window.removeEventListener.bind(window)
window.addEventListener = (type, listener, options) => {
if (type === 'resize') {
listeners.add(listener)
}
realAdd(type, listener, options)
}
window.removeEventListener = (type, listener, options) => {
if (type === 'resize') {
listeners.delete(listener)
}
realRemove(type, listener, options)
}
const mounted = mountTerminalWebDocument(host, () => {})
try {
await import('./document/page-document-modules')
mounted.dispose()
await mounted.ready
} finally {
window.addEventListener = realAdd
window.removeEventListener = realRemove
}
// The precondition: the document did start, so there was something to tear down. A build that
// bailed at the ownership check would add no listener and satisfy the emptiness below for the
// one reason this case exists to refuse.
expect(
listeners.size + Number(host.querySelector('#terminal-container') === null)
).toBeGreaterThan(0)
expect(listeners.size, 'the resize listener the started document added').toBe(0)
const { scope } = await import('./document/page-document-modules')
expect(scope.term).toBe(null)
// And the page is free, which a handle that lost track of what it started would not have left.
const remounted = mountTerminalWebDocument(host, () => {})
await remounted.ready
remounted.dispose()
})
it('gives the page back when the mount itself fails, so Reload can try again', async () => { it('gives the page back when the mount itself fails, so Reload can try again', async () => {
// The overlay's Reload path. A mount that threw holds nothing, and a flag left set would // The overlay's Reload path. A mount that threw holds nothing, and a flag left set would
// refuse every later attempt — the document's chunk failing to load is exactly that case. // refuse every later attempt — the document's chunk failing to load is exactly that case.