The worker's status claims success only on its first frame (improvise-i0v9)
The worker transport wrote "in-page worker · <doc>" when it spawned the worker, before anything had answered, so a tab whose worker could not start showed a success-looking status over a blank page. It now writes "starting the worker · <doc>…", and createShell's arm() takes an `answer` that sawFrame writes on the first frame, but only while the watchdog is still armed. A ReadOnly or SeedMissing notice lands BEFORE the Snapshot, and fail() has already disarmed, so the claim cannot overwrite the precise reason. The two-tab spec now logs every #status and denies both the premature claim and "did not answer" before the page clock is walked. A new spec pins the exact status sequence on the writable path. Reasoning: context/code/clients.md, "The watchdog". Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
@@ -1700,6 +1700,31 @@ watchdog must be DISARMED and not merely pre-empted**: it fires later and would
|
||||
overwrite the better message with "did not answer" (PR #13 review). The watchdog
|
||||
is for silence only, and `setShellFailure` clears the timer.
|
||||
|
||||
**Success is claimed by the first frame, and only if nothing failed first**
|
||||
(improvise-i0v9). The worker transport used to write `in-page worker · <doc>`
|
||||
the moment it spawned the worker — 13ms after navigation, before anything had
|
||||
answered — so a worker that could not start showed a success-looking status
|
||||
over a blank page. It now writes `starting the worker · <doc>…`, and
|
||||
`createShell().arm(message, { answer })` holds the claim until `sawFrame`. The
|
||||
rule the claim obeys is **the watchdog's own state**: it is written only if the
|
||||
watchdog is still armed when the first frame lands. That is not a convenience.
|
||||
`worker.js` posts a `ReadOnly` (or `SeedMissing`) notice straight after
|
||||
`start()` and **before** the Snapshot that answers the handshake, so an
|
||||
unconditional "first frame ⇒ say we are up" would overwrite the precise reason
|
||||
the notice just wrote. `fail` already disarms the watchdog, so "armed" answers
|
||||
"has anything been said since arming" without a second flag to keep in step.
|
||||
`fail` disarms **directly** rather than through `sawFrame`, or it would
|
||||
briefly flash the success text on its way to the failure. The starting text
|
||||
deliberately does not contain `in-page worker`, so the older specs asserting
|
||||
that substring now need the first frame to pass. The socket path passes no
|
||||
`answer`: `open` is already a real fact, and its status was never premature.
|
||||
|
||||
One consequence changes what the watchdog guards. In the read-only case a
|
||||
Snapshot always follows the notice, and `sawFrame` disarms the watchdog either
|
||||
way, so a notice that forgot to disarm would **not** be overwritten eight
|
||||
seconds later. It is overwritten **at once**, by the claim. The eight-second
|
||||
overwrite survives only where no frame follows: `BootFailed`.
|
||||
|
||||
**Did this socket ever open?** The page has to record that itself, because
|
||||
nothing on the close event carries it: `readyState` is already CLOSED by the time
|
||||
a close listener runs, whichever happened, and a refused connection and a
|
||||
|
||||
@@ -3054,6 +3054,23 @@ The third is the gotcha 48 property, and it needs `timesArmed() > 0` first or it
|
||||
denies an unreachable event: the precise reason must still be on screen after
|
||||
the 8s window, not replaced by the watchdog. `page.clock` walks past it.
|
||||
|
||||
**Strengthened by improvise-i0v9's second pass.** Every assertion above read the
|
||||
*final* `#status`, so two failures could still pass it. A watchdog that fired
|
||||
first and was then replaced by a late notice passed. So did a success claim
|
||||
shown before the notice. The test now records every status with `logStatus` and
|
||||
asserts, **before** the page clock is walked, that the log ends in `read-only`
|
||||
and never contained `did not answer` or the exact `in-page worker · <doc>`. A
|
||||
reason that arrived after the watchdog would sit in that log behind "did not
|
||||
answer"; that is the "well before 8s" property, stated without a wall-clock
|
||||
bound. Measured failing against `gh/main` at `da2e5ab`, whose log was
|
||||
`["in-page worker · two-tab-….improv", "read-only — …"]`: the premature claim
|
||||
the ticket measured at 13ms is still there once 1wft's notice exists. The
|
||||
brief's rival, `onNotice` calling `setShell` instead of `fail` (the watchdog not
|
||||
disarmed), fails it at the first `toContainText("read-only")`. The Snapshot's
|
||||
claim replaced the reason at once, which is why the eight-second form of this
|
||||
rival is now exercised by the invalid-`?doc=` spec instead (see
|
||||
`context/code/clients.md`, "The watchdog").
|
||||
|
||||
It runs on Chromium and Firefox — the two engines with OPFS — rather than
|
||||
Chromium alone, because the exclusivity semantics are the engine's and the two
|
||||
word the refusal completely differently.
|
||||
@@ -4405,3 +4422,35 @@ assertion. The tail yanks off the `Region` and `+` columns and requires the two
|
||||
registers to agree; a TUI-only blank filter (`Text("")` to `None`) fails it on
|
||||
`+`, which is why that filter was removed rather than kept. The evidence entries
|
||||
for the host-side tests are in `command.md`.
|
||||
|
||||
### the worker is not called up until its first frame has arrived
|
||||
|
||||
improvise-i0v9. The worker transport wrote `in-page worker · <doc>` when it
|
||||
spawned the worker, before anything had answered. A worker that then failed
|
||||
therefore showed a success-looking status over a blank page. The spec records
|
||||
every `#status` with `logStatus` and requires the whole log to be exactly
|
||||
`["starting the worker · <doc>…", "in-page worker · <doc>"]`. It pins the order
|
||||
as well as the final text, since a final read cannot see a claim made too early.
|
||||
Measured failing against `gh/main` at `da2e5ab`. The rival, restoring the
|
||||
immediate `setShell(\`in-page worker · ${DOC}\`)` in `workerTransport`, fails it
|
||||
on the log's first entry. The final read is `toHaveText`, not `toContainText`.
|
||||
The older specs' `toContainText("in-page worker")` could not tell the starting
|
||||
text from the claim if the one contained the other, and the starting text
|
||||
deliberately does not.
|
||||
|
||||
### only the first frame claims the peer answered, and not before it
|
||||
|
||||
improvise-i0v9, the unit half of the spec above. `arm(message, { answer })`
|
||||
must write nothing until `sawFrame`, then write `answer` once, however many
|
||||
frames follow. Fails against `gh/main`'s `createShell`, which has no `answer`
|
||||
and so never writes the claim at all.
|
||||
|
||||
### a reason given before the first frame is not replaced by the frame's claim
|
||||
|
||||
The order `worker.js` really produces on a locked document: the `ReadOnly`
|
||||
notice is posted after `start()` and **before** the Snapshot. So `fail` then
|
||||
`sawFrame` must leave the notice's text as the only thing ever shown. The rival
|
||||
is the obvious fix, `sawFrame` writing `answer` unconditionally. It fails both
|
||||
this test and the one above: the first on its exact log, which is then written
|
||||
twice. This test passes against `gh/main` by construction, since no claim exists
|
||||
there. It is not evidence of the bug; it guards against the naive fix.
|
||||
|
||||
@@ -187,6 +187,21 @@ test.describe("the standalone deployment", () => {
|
||||
await expect(page.locator("#grid td.editable").first()).toBeVisible();
|
||||
});
|
||||
|
||||
// improvise-i0v9: "in-page worker · <doc>" was written at 13ms, before the
|
||||
// worker had answered anything, and read as success over a blank page.
|
||||
test("the worker is not called up until its first frame has arrived", async ({ page }) => {
|
||||
const doc = `first-frame-${Date.now()}.improv`;
|
||||
await page.addInitScript(logStatus);
|
||||
await page.goto(`/app/index.html?doc=${doc}`);
|
||||
await expect(page.locator("#grid table")).toBeVisible({ timeout: 20_000 });
|
||||
await expect(status(page)).toHaveText(`in-page worker · ${doc}`);
|
||||
const log = await page.evaluate(() => window.__statusLog);
|
||||
expect(log, `#status said: ${json(log)}`).toEqual([
|
||||
`starting the worker · ${doc}…`,
|
||||
`in-page worker · ${doc}`,
|
||||
]);
|
||||
});
|
||||
|
||||
// improvise-9cma: a `?doc=` this backend cannot hold used to reach the
|
||||
// generic watchdog 8s later. The REASON now crosses the port (improvise-1wft).
|
||||
test("an invalid ?doc= is named on screen, and not blamed on the browser", async ({
|
||||
@@ -235,6 +250,7 @@ test.describe("the standalone deployment", () => {
|
||||
|
||||
const second = await context.newPage();
|
||||
await second.clock.install();
|
||||
await second.addInitScript(logStatus);
|
||||
await second.goto(`/app/index.html?doc=${doc}`);
|
||||
|
||||
// It PAINTS. That is the whole point: a locked document is read-only, not
|
||||
@@ -242,6 +258,15 @@ test.describe("the standalone deployment", () => {
|
||||
await expect(second.locator("#grid table")).toBeVisible({ timeout: 20_000 });
|
||||
await expect(second.locator("#status")).toContainText("read-only");
|
||||
await expect(second.locator("#status")).toContainText("another tab");
|
||||
// The page's clock has not been walked, so a reason that arrived AFTER the
|
||||
// watchdog would have been logged behind "did not answer". Nor was success
|
||||
// ever claimed: the Snapshot that follows the notice must not overwrite it.
|
||||
const early = await second.evaluate(() => window.__statusLog);
|
||||
expect(early.at(-1), `#status said: ${json(early)}`).toContain("read-only");
|
||||
expect(
|
||||
early.filter((t) => t.includes("did not answer") || t === `in-page worker · ${doc}`),
|
||||
`#status said: ${json(early)}`,
|
||||
).toEqual([]);
|
||||
|
||||
// And the reason STANDS: it is not replaced 8s later by a watchdog
|
||||
// blaming the browser for something the code knew precisely.
|
||||
@@ -252,6 +277,10 @@ test.describe("the standalone deployment", () => {
|
||||
await second.clock.runFor(HANDSHAKE_TIMEOUT_MS + 2_000);
|
||||
await expect(second.locator("#status")).toContainText("read-only");
|
||||
await expect(second.locator("#status")).not.toContainText("did not answer");
|
||||
const late = await second.evaluate(() => window.__statusLog);
|
||||
expect(late.some((t) => t.includes("did not answer")), `#status said: ${json(late)}`).toBe(
|
||||
false,
|
||||
);
|
||||
|
||||
await second.close();
|
||||
await first.close();
|
||||
|
||||
+3
-1
@@ -181,7 +181,8 @@ function workerTransport() {
|
||||
shell.fail(`worker failed to start: ${event.message ?? "see console"}`);
|
||||
console.error("improvise: worker error", event);
|
||||
});
|
||||
shell.setShell(`in-page worker · ${DOC}`);
|
||||
// Not yet a claim that it works: the first frame makes that (improvise-i0v9).
|
||||
shell.setShell(`starting the worker · ${DOC}…`);
|
||||
// Frames posted before the worker has finished `start()` are held by
|
||||
// worker.js in arrival order, so the handshake can go out immediately.
|
||||
return { send: (text) => worker.postMessage(text), ready: Promise.resolve() };
|
||||
@@ -797,6 +798,7 @@ shell.arm(
|
||||
WS_URL
|
||||
? `${WS_URL} never answered — is improvise-ws-server running, and did it send a document?`
|
||||
: "the in-page worker did not answer (see console)",
|
||||
{ answer: WS_URL ? null : `in-page worker · ${DOC}` },
|
||||
);
|
||||
await transport.ready;
|
||||
sendViewport(); // the handshake: the snapshot is built at this extent
|
||||
|
||||
+13
-7
@@ -339,7 +339,12 @@ export function createShell({
|
||||
clearTimer = clearTimeout,
|
||||
}) {
|
||||
let watchdog = null;
|
||||
let answered = null;
|
||||
let arms = 0;
|
||||
const disarm = () => {
|
||||
clearTimer(watchdog);
|
||||
watchdog = null;
|
||||
};
|
||||
|
||||
return {
|
||||
setShell(text) {
|
||||
@@ -350,24 +355,25 @@ export function createShell({
|
||||
},
|
||||
/** A peer that never answers the handshake. Deliberately about the *first
|
||||
* frame*, not either peer's own failure modes — clients.md. */
|
||||
arm(message) {
|
||||
arm(message, { answer = null } = {}) {
|
||||
arms += 1;
|
||||
clearTimer(watchdog);
|
||||
answered = answer;
|
||||
disarm();
|
||||
watchdog = setTimer(() => {
|
||||
watchdog = null;
|
||||
this.setShell(message);
|
||||
}, timeoutMs);
|
||||
},
|
||||
/** A frame arrived, so the peer answered: the watchdog has no question
|
||||
* left to ask. */
|
||||
/** A frame arrived, so the peer answered. Only the FIRST, and only if no
|
||||
* failure spoke first, may say so (improvise-i0v9, clients.md). */
|
||||
sawFrame() {
|
||||
clearTimer(watchdog);
|
||||
watchdog = null;
|
||||
if (watchdog !== null && answered !== null) this.setShell(answered);
|
||||
disarm();
|
||||
},
|
||||
/** A peer that failed *loudly* has already said something more precise, so
|
||||
* the watchdog must be DISARMED and not merely pre-empted (PR #13). */
|
||||
fail(text) {
|
||||
this.sawFrame();
|
||||
disarm();
|
||||
this.setShell(text);
|
||||
},
|
||||
armed: () => watchdog !== null,
|
||||
|
||||
@@ -351,6 +351,43 @@ test("a notice DISARMS the watchdog rather than being replaced by it", () => {
|
||||
);
|
||||
});
|
||||
|
||||
// improvise-i0v9: "in-page worker · <doc>" was written before the worker had
|
||||
// answered anything, and read as success over a blank page for eight seconds.
|
||||
test("only the first frame claims the peer answered, and not before it", () => {
|
||||
const clock = fakeClock();
|
||||
const seen = [];
|
||||
const shell = createShell({
|
||||
onShell: (t) => seen.push(t),
|
||||
timeoutMs: 8000,
|
||||
setTimer: clock.setTimer,
|
||||
clearTimer: clock.clearTimer,
|
||||
});
|
||||
shell.arm("did not answer", { answer: "in-page worker · a.improv" });
|
||||
assert.deepEqual(seen, [], "the claim was made before any frame arrived");
|
||||
shell.sawFrame();
|
||||
shell.sawFrame();
|
||||
assert.deepEqual(seen, ["in-page worker · a.improv"]);
|
||||
});
|
||||
|
||||
// The order worker.js really produces on a locked document: the ReadOnly
|
||||
// notice is posted after `start()` and BEFORE the Snapshot answers the handshake.
|
||||
test("a reason given before the first frame is not replaced by the frame's claim", () => {
|
||||
const clock = fakeClock();
|
||||
const seen = [];
|
||||
const shell = createShell({
|
||||
onShell: (t) => seen.push(t),
|
||||
timeoutMs: 8000,
|
||||
setTimer: clock.setTimer,
|
||||
clearTimer: clock.clearTimer,
|
||||
});
|
||||
shell.arm("did not answer", { answer: "in-page worker · a.improv" });
|
||||
shell.fail(noticeText({ kind: "ReadOnly", document: "a.improv", detail: "" }));
|
||||
shell.sawFrame();
|
||||
clock.fire();
|
||||
assert.equal(seen.length, 1, `#status said: ${JSON.stringify(seen)}`);
|
||||
assert.match(seen[0], /read-only/);
|
||||
});
|
||||
|
||||
// The notice's remembered state. The value crosses `localStorage`, which is
|
||||
// allowed to forget and allowed to throw, so the interesting inputs are the
|
||||
// ones that are NOT either spelling.
|
||||
|
||||
Reference in New Issue
Block a user