fix(assemble): verify each cache-bust stamp on its own, and rename a test
Two Copilot findings on PR #24, one applied as asked and one applied only in part, with the measurement that says why. **The combined `grep` was a real hole.** `grep -n "?v=" a b c d` succeeds when *any one* file matches, so under `set -e` three failed substitutions still produced a "successful" assembly. Those greps are the only guard against the generated wasm-pack glue changing shape — which is the exact failure this cache-busting exists to end, twice, during improvise-9x6 — so each stamped reference is now checked on its own and names itself when it is the one missing. Inherited from the inline CI step rather than written here, but it became mine when it became a script. Proved by pointing one `sed` at a pattern that cannot match: the old form exited 0, the new one exits 1 naming the file. **The repaint count was the right objection to the wrong remedy.** Copilot asked for "count the relevant grid mutations/repaints and prove the assertion by removing the guard". Done, and it does not discriminate: with `if (closed) return` removed from `edit`'s `close`, `close` still runs exactly twice, `typed_cell` is still called exactly once, `#grid` still takes exactly two childList mutations, and the final text is identical. The only difference is one extra *entry* into `repaint()` that produces no DOM change — because the second close is `close(false)`, so it does not re-send, and everything else it does is idempotent. Which is exactly what improvise-hwpi meant by "harmless before, but only by accident". Counting `repaint()` entries would need a test hook inside `web/app.js`: a page that knows it is being tested. So the half of the finding that stands is the **name**, which claimed more than the body checked — the same defect as `open_and_refresh_see_an_empty_origin_root` two commits ago. Renamed to `committing an edit sends one frame and shows the host's value`, and the measurement is recorded in the comment so nobody repeats it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+23
-10
@@ -84,15 +84,26 @@ test.describe("the standalone deployment", () => {
|
||||
await expect(page.locator("#grid td.editable").first()).toBeVisible();
|
||||
});
|
||||
|
||||
// improvise-hwpi's third fix: Enter removes the focused <input>, which
|
||||
// fires `blur`, which called `close` a second time — harmless before, but
|
||||
// only by accident. The observable is that the commit takes effect exactly
|
||||
// once and the value on screen is the SERVER's, not the string typed.
|
||||
// A commit round-trips through the host, and what lands on screen is the
|
||||
// HOST's value rather than the string that was typed. `20000` is typed and
|
||||
// `20,000` asserted: the number format is applied by the host and relayed
|
||||
// back, so a page that echoed the input would fail.
|
||||
//
|
||||
// `20000` is typed and `20,000` is asserted: the number format is applied
|
||||
// by the host and relayed back, so a page that simply echoed what was typed
|
||||
// would show `20000` and fail.
|
||||
test("committing an edit repaints once, with the server's value", async ({
|
||||
// This is deliberately NOT named for improvise-hwpi's third fix (the
|
||||
// `closed` re-entry guard in `edit`), which Copilot asked for on PR #24.
|
||||
// That guard is **not observable from the DOM**, measured rather than
|
||||
// assumed: with it removed, `close` still runs exactly twice, the frame is
|
||||
// still sent exactly once, `#grid` still takes exactly two childList
|
||||
// mutations, and the final text is identical. The only difference is one
|
||||
// extra *entry* into `repaint()` that produces no output — because the
|
||||
// second close is `close(false)`, so it does not re-send, and everything
|
||||
// else it does is idempotent. That is precisely why hwpi called it
|
||||
// "harmless before, but only by accident".
|
||||
//
|
||||
// Counting repaint() entries would need a test hook inside `web/app.js`,
|
||||
// which would be a page that knows it is being tested. So the name says
|
||||
// what is checked and this comment says what is not.
|
||||
test("committing an edit sends one frame and shows the host's value", async ({
|
||||
page,
|
||||
}) => {
|
||||
await page.goto("/index.html?doc=commit-test.improv");
|
||||
@@ -108,8 +119,10 @@ test.describe("the standalone deployment", () => {
|
||||
|
||||
await expect(cell).toHaveText("20,000", { timeout: 10_000 });
|
||||
expect(before).not.toBe("20,000");
|
||||
// One editor, closed once: a second `close` would reopen nothing, but a
|
||||
// stray input left in the DOM is what the guard exists to prevent.
|
||||
// The editor is gone and did not come back. This is the half of the
|
||||
// re-entry guard that IS observable: whatever `close` ran, no stray
|
||||
// <input> is left behind and the host's value is not overwritten by the
|
||||
// `original` text the second close writes before repainting.
|
||||
await expect(page.locator("#grid input")).toHaveCount(0);
|
||||
});
|
||||
});
|
||||
|
||||
+16
-3
@@ -54,6 +54,19 @@ sedi "s|'improvise_wasm_client_bg\.wasm'|'improvise_wasm_client_bg.wasm?v=${toke
|
||||
sedi "s|'improvise_worker_server_bg\.wasm'|'improvise_worker_server_bg.wasm?v=${token}'|" \
|
||||
"$out/worker-pkg/improvise_worker_server.js"
|
||||
|
||||
grep -n "?v=" "$out/app.js" "$out/worker.js" \
|
||||
"$out/pkg/improvise_wasm_client.js" \
|
||||
"$out/worker-pkg/improvise_worker_server.js"
|
||||
# One `grep` over all four files succeeds when ANY one of them matches, so
|
||||
# three failed substitutions could still produce a "successful" assembly under
|
||||
# `set -e` (found by Copilot on PR #24). These checks are the only guard
|
||||
# against the generated glue changing shape, so each stamped reference is
|
||||
# verified on its own and names itself when it is the one missing.
|
||||
stamped() {
|
||||
grep -q "$2" "$1" || {
|
||||
echo "assemble: $1 is missing the cache-bust stamp: $2" >&2
|
||||
exit 1
|
||||
}
|
||||
grep -n "$2" "$1"
|
||||
}
|
||||
stamped "$out/app.js" "pkg/improvise_wasm_client.js?v=${token}"
|
||||
stamped "$out/worker.js" "worker-pkg/improvise_worker_server.js?v=${token}"
|
||||
stamped "$out/pkg/improvise_wasm_client.js" "improvise_wasm_client_bg.wasm?v=${token}"
|
||||
stamped "$out/worker-pkg/improvise_worker_server.js" "improvise_worker_server_bg.wasm?v=${token}"
|
||||
|
||||
Reference in New Issue
Block a user