From 4cc670a8c71bfe6e12806358610de06dc63bc459 Mon Sep 17 00:00:00 2001 From: Xan Torres Date: Wed, 19 Aug 2026 12:39:58 +0800 Subject: [PATCH 01/13] test: guard frontend behaviour a successful build does not demonstrate Two things the server and the app depend on are invisible to the build. Both fail silently, so a green build and a working dev server actively disguise them. The server parses the script and stylesheet paths out of the built index.html and reuses them on every server-rendered page. A build that emits no script or stylesheet tag at all still succeeds, the dev server still works, the binary still compiles, and the pages simply render with no scripts and no stylesheet. TestGetStyleResolvesBuiltAssets asserts the parse still finds them. check-built-assets.sh builds the frontend and runs that test, and its --self-check rewrites the built index.html into shapes the parser cannot use (a stylesheet link but no script with a src, a script but no stylesheet link, an inline-only script) and confirms the check fails on each, so a check that quietly stopped asserting anything is distinguishable from a passing one. Languages other than the default one are loaded with a template-literal dynamic import through an alias that points outside the frontend root. A toolchain that cannot enumerate that pattern still builds and still serves a working app; the resources never arrive, and only for non-default languages, so a smoke test in the default language misses it. check-locale-resolution.js drives the project's dev server directly and asserts two separate properties, because only one of them is about resolution: 1. The module graph resolves the alias and parses the file. Covered by importing the probe through the server's module runner. 2. A browser is allowed to fetch it. The languages live outside the frontend root, so they are reachable only if the dev server's filesystem allow-list covers their directory. Checking only the first passes while every language 403s in a browser. Order is load-bearing and is commented as such: once a module is in the graph the dev server answers from the transform pipeline instead of reading the file, and the request stops passing through the allow-list. Measured: 403 for a cold request, 200 for the same request after the module has been loaded. So the reachability check resolves the path without loading it and asks before anything else touches a language. Observed failing on each of: alias removed, yaml plugin removed, allow-list narrowed to the frontend root, and the application's import shape changed out from under the probe. Both run through make check-ui. (cherry picked from commit 71cd9246fedbf6430c6a7373759fdf0be4029e4a, without its Go test file, which landed separately; check-locale-resolution.js carries the content of commit 919d3603bcdcc0ce8ab6d94c0c36c354065d3926 and the config-path line of commit 1b59fa7be74f8a2767556afebebcc0871100789f; check-built-assets.sh carries the self-check fixtures of commit eab6f9d80c084d8345a4c82821a64aaef512cce5) --- Makefile | 14 +- script/check-built-assets.sh | 94 ++++++++++++ ui/package.json | 1 + ui/scripts/check-locale-resolution.js | 210 ++++++++++++++++++++++++++ ui/scripts/locale-probe.js | 32 ++++ 5 files changed, 350 insertions(+), 1 deletion(-) create mode 100755 script/check-built-assets.sh create mode 100644 ui/scripts/check-locale-resolution.js create mode 100644 ui/scripts/locale-probe.js diff --git a/Makefile b/Makefile index 74e51886b..cf1854046 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: build clean ui +.PHONY: build clean ui check-ui check-ui-assets check-ui-locales VERSION=2.0.3 BIN=answer @@ -47,6 +47,18 @@ check: test: @$(GO) test ./internal/repo/repo_test +# Frontend checks for behaviour a successful build does not demonstrate. +# Both guard runtime failures that leave every build step reporting success. +check-ui: check-ui-assets check-ui-locales + +# The server reads the built asset paths out of index.html. +check-ui-assets: + @./script/check-built-assets.sh + +# The app loads languages other than the default one through a dynamic import. +check-ui-locales: + @cd ui && pnpm check-locales + # clean all build result clean: @$(GO) clean ./... diff --git a/script/check-built-assets.sh b/script/check-built-assets.sh new file mode 100755 index 000000000..e7672baf4 --- /dev/null +++ b/script/check-built-assets.sh @@ -0,0 +1,94 @@ +#!/bin/bash +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# Builds the frontend and asserts the server can still find the built assets +# inside index.html. See internal/controller/template_controller_test.go for +# why that is not implied by a successful build. +# +# --skip-build reuse an existing ui/build, do not rebuild +# --self-check additionally rewrite ui/build/index.html with asset tag +# shapes the server cannot parse and confirm the check fails +# on them. Restores the real build output afterwards. + +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +INDEX_HTML="$REPO_ROOT/ui/build/index.html" +TEST_PACKAGE="./internal/controller/" +TEST_NAME="TestGetStyleResolvesBuiltAssets" + +skip_build=0 +self_check=0 +for arg in "$@"; do + case "$arg" in + --skip-build) skip_build=1 ;; + --self-check) self_check=1 ;; + *) echo "unknown option: $arg" >&2; exit 2 ;; + esac +done + +run_check() { + (cd "$REPO_ROOT" && go test -count=1 "$TEST_PACKAGE" -run "$TEST_NAME" "$@") +} + +if [ "$skip_build" -eq 0 ]; then + echo "==> building frontend" + (cd "$REPO_ROOT/ui" && pnpm build) +fi + +if [ ! -f "$INDEX_HTML" ]; then + echo "no built index.html at $INDEX_HTML; run without --skip-build" >&2 + exit 1 +fi + +echo "==> checking the server can parse the built asset tags" +run_check -v + +if [ "$self_check" -eq 0 ]; then + exit 0 +fi + +# Confirm the check actually fails when the asset tags change shape. Without +# this, a check that silently stopped asserting anything would look identical +# to a passing one. +backup="$(mktemp)" +cp "$INDEX_HTML" "$backup" +trap 'cp "$backup" "$INDEX_HTML"; rm -f "$backup"' EXIT + +expect_failure() { + local label="$1" + local html="$2" + printf '%s' "$html" > "$INDEX_HTML" + echo "==> self-check: expecting failure on $label" + if run_check >/dev/null 2>&1; then + echo "SELF-CHECK FAILED: the check passed on $label, so it is not guarding anything" >&2 + exit 1 + fi + echo " check failed as expected" +} + +expect_failure "stylesheet link but no script src" \ + '
' + +expect_failure "script src but no stylesheet link" \ + '
' + +expect_failure "only an inline script, no src" \ + '
' + +echo "==> self-check passed" diff --git a/ui/package.json b/ui/package.json index 07e5293f3..525ac5788 100644 --- a/ui/package.json +++ b/ui/package.json @@ -12,6 +12,7 @@ "build:packages": "pnpm -r --filter=./src/plugins/* run build", "clean": "rm -rf node_modules && rm -rf src/plugins/**/node_modules", "analyze": "source-map-explorer 'build/static/js/*.js'", + "check-locales": "node ./scripts/check-locale-resolution.js", "setup-lint": "node scripts/setup-eslint.js && cd .. && husky install", "lint": "eslint . --cache --fix --ext .ts,.tsx", "prettier": "prettier --write \"src/**/*.{ts,tsx,css,scss,md}\"", diff --git a/ui/scripts/check-locale-resolution.js b/ui/scripts/check-locale-resolution.js new file mode 100644 index 000000000..d78c4820a --- /dev/null +++ b/ui/scripts/check-locale-resolution.js @@ -0,0 +1,210 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/* + * src/utils/localize.ts loads a language with a template-literal dynamic + * import through the @i18n alias, which points outside the frontend root. A + * build tool that cannot resolve that shape still produces a clean build and + * still serves a working app; the resources simply never arrive. The failure + * is confined to runtime and to languages other than the default one, so + * neither a green build nor a smoke test in the default language sees it. + * + * So: start the project's own dev server, load that same import through it, + * and require two different languages to come back as different, real + * translated content. + * + * Two separate things have to hold, and only one of them is about resolution: + * + * 1. The module graph resolves the alias and parses the file. Covered by + * importing the probe through the server's module runner. + * 2. A browser is actually allowed to fetch it. The languages live outside + * the frontend root, so they are reachable only if the dev server's + * filesystem allow-list includes their directory. The module runner does + * not go through that allow-list. A real request does, so this makes one. + * + * Checking only the first would pass while every language 403s in a browser. + */ + +const fs = require('fs'); +const path = require('path'); +const util = require('util'); + +const UI_DIR = path.resolve(__dirname, '..'); +const PROBE = path.resolve(__dirname, 'locale-probe.js'); +const LOCALIZE_SOURCE = path.resolve(UI_DIR, 'src/utils/localize.ts'); +const DEV_SERVER_CONFIG = path.resolve(UI_DIR, 'vite.config.mts'); + +// A language written in a script the default language does not use, so +// "resolved the language that was asked for" cannot be mistaken for "fell +// back to the default language". +const TARGET_LANG = 'zh_CN'; +const DEFAULT_LANG = 'en_US'; +const TARGET_SCRIPT = /[一-鿿]/; + +const rel = (file) => path.relative(UI_DIR, file); + +function fail(message) { + console.error(`FAIL: ${message}`); + process.exit(1); +} + +// The probe carries a copy of the application's import expression, so it is +// only evidence for as long as the two agree. +function assertProbeStillMatchesApplication() { + const source = fs.readFileSync(LOCALIZE_SOURCE, 'utf8'); + if (!/import\(\s*`@i18n\/\$\{[^}]+\}\.yaml`\s*\)/.test(source)) { + fail( + `${rel(LOCALIZE_SOURCE)} no longer loads languages with a template-literal import ` + + `through @i18n, so ${rel(PROBE)} is exercising a shape the application does not use. ` + + `Update the probe to match the application, then re-run.`, + ); + } +} + +async function openProbe() { + if (!fs.existsSync(DEV_SERVER_CONFIG)) { + fail( + `no dev server configuration this check knows how to drive was found at ` + + `${rel(DEV_SERVER_CONFIG)}; teach it how to load ${rel(PROBE)} with the current ` + + `tooling before relying on it again`, + ); + } + + const { createServer, createServerModuleRunner } = await import('vite'); + const server = await createServer({ + root: UI_DIR, + configFile: DEV_SERVER_CONFIG, + logLevel: 'warn', + }); + await server.listen(); + + const ssr = server.environments.ssr; + const baseUrl = (server.resolvedUrls.local[0] || '').replace(/\/$/, ''); + + // Loaded on demand rather than up front, because the reachability check + // below is only meaningful before anything pulls a language into the module + // graph. See the comment on assertBrowserCanFetch. + let probe = null; + const importProbe = async () => { + if (!probe) { + probe = await createServerModuleRunner(ssr).import(PROBE); + } + return probe; + }; + + return { + load: async (langName) => (await importProbe()).loadLocaleResource(langName), + + // Ask for the language file the way the browser will: over HTTP, at the + // path the dev server assigns to a file outside the frontend root. + // + // ORDER MATTERS. Run this before any language is loaded through the module + // graph. Once a module is in the graph the dev server answers from the + // transform pipeline instead of reading the file, and the request stops + // passing through the filesystem allow-list. Checking afterwards returns + // 200 even when the allow-list would give a browser a 403, which is to say + // it checks nothing. Resolve the path without loading it, then ask. + async assertBrowserCanFetch(langName) { + const specifier = `@i18n/${langName}.yaml`; + const resolved = await ssr.pluginContainer.resolveId(specifier, PROBE); + + if (!resolved || !resolved.id) { + fail(`${specifier} does not resolve at all, so there is nothing for a browser to request`); + } + + const url = `${baseUrl}/@fs${resolved.id.split('?')[0]}`; + let response; + try { + response = await fetch(url); + } catch (err) { + fail(`requesting ${langName} at ${url} failed outright: ${err.message}`); + } + + if (!response.ok) { + fail( + `the dev server answered ${response.status} for ${langName} at ${url}; the language ` + + `files sit outside the frontend root, so a browser would get this too and every ` + + `language other than the default would fail to load`, + ); + } + + const body = await response.text(); + if (!TARGET_SCRIPT.test(body)) { + fail( + `the dev server served ${langName} at ${url} but the response carries none of its ` + + `script; a browser would receive something that is not the translated file`, + ); + } + }, + + close: () => server.close(), + }; +} + +function resourcesOf(resConf, langName) { + // The application reads .ui off the loaded file and registers it with i18next. + const resources = resConf && resConf.ui; + if ( + !resources || + typeof resources !== 'object' || + Object.keys(resources).length === 0 + ) { + fail( + `${langName} resolved to ${util.inspect(resConf, { depth: 1 })}, which carries no ui ` + + `section; the application would register no translations for it`, + ); + } + return resources; +} + +async function main() { + assertProbeStillMatchesApplication(); + + const probe = await openProbe(); + try { + // Before any load, while the filesystem allow-list still governs the request. + await probe.assertBrowserCanFetch(TARGET_LANG); + + const target = resourcesOf(await probe.load(TARGET_LANG), TARGET_LANG); + const fallback = resourcesOf(await probe.load(DEFAULT_LANG), DEFAULT_LANG); + + if (JSON.stringify(target) === JSON.stringify(fallback)) { + fail( + `${TARGET_LANG} and ${DEFAULT_LANG} resolved to identical resources; the language ` + + `name is not selecting a file, so every language would render as ${DEFAULT_LANG}`, + ); + } + + if (!TARGET_SCRIPT.test(JSON.stringify(target))) { + fail( + `${TARGET_LANG} resolved without a single character of its own script; the content ` + + `is not the translated file`, + ); + } + + console.log( + `OK: ${TARGET_LANG} and ${DEFAULT_LANG} resolve at runtime to distinct translated ` + + `resources, and ${TARGET_LANG} is fetchable over the dev server`, + ); + } finally { + await probe.close(); + } +} + +main().catch((err) => fail(err.stack || String(err))); diff --git a/ui/scripts/locale-probe.js b/ui/scripts/locale-probe.js new file mode 100644 index 000000000..ac5eadf72 --- /dev/null +++ b/ui/scripts/locale-probe.js @@ -0,0 +1,32 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/* + * Mirrors how src/utils/localize.ts loads a language at runtime. The import + * specifier is a template literal resolved through an alias that points + * outside the frontend root, so whether it resolves is a property of the + * bundler rather than of this file. + * + * Keep this expression identical to the one in the application. + * check-locale-resolution.js asserts that it still matches. + */ +export const loadLocaleResource = async (langName) => { + const { default: resConf } = await import(`@i18n/${langName}.yaml`); + return resConf; +}; From 0c9c3504073b561f641d329e1b5ff687ec41ba0a Mon Sep 17 00:00:00 2001 From: Xan Torres Date: Wed, 19 Aug 2026 12:40:40 +0800 Subject: [PATCH 02/13] test: guard plugin translation registration against evaluation order Plugin i18n modules register their translations while they are being evaluated, and i18next only attaches its resource-store methods during init. Which of the two happens first is decided by how the bundler groups and orders chunks, so it has to hold in both orders. The check loads the plugin helper without touching the application's i18n bootstrap, so i18next is guaranteed uninitialised, registers a bundle, and then initialises. Two assertions, because either one alone can pass while the feature is broken: registering must not throw, and the translations must actually be present afterwards. A fix that swallowed the error would satisfy the first and leave every plugin string untranslated. The check also asserts that exactly one i18next module is loaded. Without that, the helper and the check can each resolve their own copy and every later assertion silently measures an object nothing under test wrote to. Observed failing on the unguarded call with the same error the browser reports: addResourceBundle is not a function. (cherry picked from commit c27c204289d11c35fb984d2873bb28030632a76d; the config path points at vite.config.mts, the name it carries on this branch, per commit 1b59fa7be74f8a2767556afebebcc0871100789f) --- Makefile | 10 +- ui/package.json | 1 + ui/scripts/check-plugin-i18n-order.js | 148 ++++++++++++++++++++++++++ 3 files changed, 156 insertions(+), 3 deletions(-) create mode 100644 ui/scripts/check-plugin-i18n-order.js diff --git a/Makefile b/Makefile index cf1854046..af1fe3188 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: build clean ui check-ui check-ui-assets check-ui-locales +.PHONY: build clean ui check-ui check-ui-assets check-ui-locales check-ui-plugin-i18n VERSION=2.0.3 BIN=answer @@ -48,8 +48,8 @@ test: @$(GO) test ./internal/repo/repo_test # Frontend checks for behaviour a successful build does not demonstrate. -# Both guard runtime failures that leave every build step reporting success. -check-ui: check-ui-assets check-ui-locales +# Each guards a runtime failure that leaves every build step reporting success. +check-ui: check-ui-assets check-ui-locales check-ui-plugin-i18n # The server reads the built asset paths out of index.html. check-ui-assets: @@ -59,6 +59,10 @@ check-ui-assets: check-ui-locales: @cd ui && pnpm check-locales +# Plugin translations register while modules evaluate, in bundler-decided order. +check-ui-plugin-i18n: + @cd ui && pnpm check-plugin-i18n + # clean all build result clean: @$(GO) clean ./... diff --git a/ui/package.json b/ui/package.json index 525ac5788..9a252a6b9 100644 --- a/ui/package.json +++ b/ui/package.json @@ -13,6 +13,7 @@ "clean": "rm -rf node_modules && rm -rf src/plugins/**/node_modules", "analyze": "source-map-explorer 'build/static/js/*.js'", "check-locales": "node ./scripts/check-locale-resolution.js", + "check-plugin-i18n": "node ./scripts/check-plugin-i18n-order.js", "setup-lint": "node scripts/setup-eslint.js && cd .. && husky install", "lint": "eslint . --cache --fix --ext .ts,.tsx", "prettier": "prettier --write \"src/**/*.{ts,tsx,css,scss,md}\"", diff --git a/ui/scripts/check-plugin-i18n-order.js b/ui/scripts/check-plugin-i18n-order.js new file mode 100644 index 000000000..61b6f0dc5 --- /dev/null +++ b/ui/scripts/check-plugin-i18n-order.js @@ -0,0 +1,148 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/* + * Plugin i18n modules call initI18nResource while they are being evaluated, + * and i18next only attaches its resource-store methods to the instance while + * init runs. So whether registering a plugin's translations works depends on + * whether that module evaluated before or after init, which is decided by how + * the bundler groups and orders chunks. Nothing in the application controls + * it, and when it goes wrong the entry module throws while still evaluating: + * the build succeeds, the server returns a page, the console stays empty, and + * the application never mounts. + * + * Registering BEFORE init is the order that breaks, so do exactly that. Then + * initialise, then require the translations to actually be present. That last + * step is the point: a fix that merely swallowed the error would leave every + * plugin string untranslated and would otherwise look identical to a working + * one. + */ + +const fs = require('fs'); +const path = require('path'); +const util = require('util'); + +const UI_DIR = path.resolve(__dirname, '..'); +const PLUGIN_UTILS = path.resolve(UI_DIR, 'src/utils/pluginKit/utils.ts'); +const DEV_SERVER_CONFIG = path.resolve(UI_DIR, 'vite.config.mts'); + +const LANG = 'en_US'; +const PLUGIN_NS = 'plugin'; +const SLUG = 'check_only_plugin'; +const SENTINEL = 'registered before init'; + +const rel = (file) => path.relative(UI_DIR, file); + +function fail(message) { + console.error(`FAIL: ${message}`); + process.exit(1); +} + +async function main() { + if (!fs.existsSync(DEV_SERVER_CONFIG)) { + fail( + `no dev server configuration this check knows how to drive was found at ` + + `${rel(DEV_SERVER_CONFIG)}; teach it how to load ${rel(PLUGIN_UTILS)} with the ` + + `current tooling before relying on it again`, + ); + } + + const { createServer, createServerModuleRunner } = await import('vite'); + const server = await createServer({ + root: UI_DIR, + configFile: DEV_SERVER_CONFIG, + logLevel: 'warn', + // Without this the helper under test and this check each resolve their own + // copy of i18next, and the check ends up inspecting an instance nobody + // registered anything into. It reads as a failure with a confusing message + // rather than as a broken harness, so the single-instance assertion below + // guards it too. + ssr: { noExternal: ['i18next'] }, + }); + await server.listen(); + + try { + const runner = createServerModuleRunner(server.environments.ssr); + + // Deliberately load the plugin helper first and never touch the app's own + // i18n bootstrap, so i18next is guaranteed to be uninitialised here. This + // is the ordering the bundler is free to produce. + const pluginUtils = await runner.import(PLUGIN_UTILS); + const i18next = (await runner.import('i18next')).default; + + // If the helper and this check hold different copies, every assertion + // below is measuring an object nothing under test ever touched. + const loaded = [...server.environments.ssr.moduleGraph.idToModuleMap.keys()].filter( + (id) => /[\\/]i18next[\\/]/.test(id) && !/[\\/]\.vite[\\/]/.test(id), + ); + if (loaded.length !== 1) { + fail( + `expected exactly one i18next module to be loaded, found ${loaded.length}: ` + + `${util.inspect(loaded)}. This check can only observe what the helper registers ` + + `if both resolve the same copy.`, + ); + } + + if (i18next.isInitialized) { + fail( + `i18next was already initialised before this check registered anything, so the ` + + `ordering the check exists to exercise was never exercised; the check is not ` + + `proving what it claims`, + ); + } + + const resource = { + [LANG]: { plugin: { [SLUG]: { ui: { title: SENTINEL } } } }, + }; + + try { + pluginUtils.initI18nResource(resource); + } catch (err) { + fail( + `registering a plugin's translations before i18next.init threw: ${err.message}\n` + + ` This is the ordering a bundler is free to produce. When it happens the entry ` + + `module throws while evaluating and the application never mounts, with no console ` + + `output and a successful build.`, + ); + } + + await i18next.init({ lng: LANG, fallbackLng: LANG, resources: {} }); + + const bundle = i18next.getResourceBundle(LANG, PLUGIN_NS); + const title = bundle && bundle[SLUG] && bundle[SLUG].ui && bundle[SLUG].ui.title; + + if (title !== SENTINEL) { + fail( + `registering before init did not throw, but the translations never arrived: ` + + `expected ${util.inspect(SENTINEL)} at ${PLUGIN_NS}.${SLUG}.ui.title, got ` + + `${util.inspect(bundle)}.\n` + + ` Surviving the call is not enough. Plugin strings have to actually be ` + + `registered once i18next is up, or every plugin renders untranslated.`, + ); + } + + console.log( + `OK: plugin translations registered before i18next.init survive and are present afterwards`, + ); + } finally { + await server.close(); + } +} + +main().catch((err) => fail(err.stack || String(err))); From 48d2fce77c945f98413806764fe3544b78b9a4cf Mon Sep 17 00:00:00 2001 From: Xan Torres Date: Sat, 1 Aug 2026 20:37:56 +0800 Subject: [PATCH 03/13] style: apply the project formatter to the check scripts (cherry picked from commit 6ef81d3e1fcee2f78729a6341353af06ec34ac22) --- ui/scripts/check-locale-resolution.js | 11 ++++++++--- ui/scripts/check-plugin-i18n-order.js | 7 +++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/ui/scripts/check-locale-resolution.js b/ui/scripts/check-locale-resolution.js index d78c4820a..64ad275ff 100644 --- a/ui/scripts/check-locale-resolution.js +++ b/ui/scripts/check-locale-resolution.js @@ -109,7 +109,8 @@ async function openProbe() { }; return { - load: async (langName) => (await importProbe()).loadLocaleResource(langName), + load: async (langName) => + (await importProbe()).loadLocaleResource(langName), // Ask for the language file the way the browser will: over HTTP, at the // path the dev server assigns to a file outside the frontend root. @@ -125,7 +126,9 @@ async function openProbe() { const resolved = await ssr.pluginContainer.resolveId(specifier, PROBE); if (!resolved || !resolved.id) { - fail(`${specifier} does not resolve at all, so there is nothing for a browser to request`); + fail( + `${specifier} does not resolve at all, so there is nothing for a browser to request`, + ); } const url = `${baseUrl}/@fs${resolved.id.split('?')[0]}`; @@ -133,7 +136,9 @@ async function openProbe() { try { response = await fetch(url); } catch (err) { - fail(`requesting ${langName} at ${url} failed outright: ${err.message}`); + fail( + `requesting ${langName} at ${url} failed outright: ${err.message}`, + ); } if (!response.ok) { diff --git a/ui/scripts/check-plugin-i18n-order.js b/ui/scripts/check-plugin-i18n-order.js index 61b6f0dc5..54d846a69 100644 --- a/ui/scripts/check-plugin-i18n-order.js +++ b/ui/scripts/check-plugin-i18n-order.js @@ -88,7 +88,9 @@ async function main() { // If the helper and this check hold different copies, every assertion // below is measuring an object nothing under test ever touched. - const loaded = [...server.environments.ssr.moduleGraph.idToModuleMap.keys()].filter( + const loaded = [ + ...server.environments.ssr.moduleGraph.idToModuleMap.keys(), + ].filter( (id) => /[\\/]i18next[\\/]/.test(id) && !/[\\/]\.vite[\\/]/.test(id), ); if (loaded.length !== 1) { @@ -125,7 +127,8 @@ async function main() { await i18next.init({ lng: LANG, fallbackLng: LANG, resources: {} }); const bundle = i18next.getResourceBundle(LANG, PLUGIN_NS); - const title = bundle && bundle[SLUG] && bundle[SLUG].ui && bundle[SLUG].ui.title; + const title = + bundle && bundle[SLUG] && bundle[SLUG].ui && bundle[SLUG].ui.title; if (title !== SENTINEL) { fail( From 629d47f09b4df250df771d4be70a0da15fb18c36 Mon Sep 17 00:00:00 2001 From: Xan Torres Date: Mon, 3 Aug 2026 00:22:32 +0800 Subject: [PATCH 04/13] style: declare properties before nested rules in comment styles border-bottom was declared after the &:hover nested rule. Current Sass hoists trailing declarations above nested rules, so output is unaffected today, but a future Sass release will stop doing that and emit border-bottom last, changing rule order. Move it above the nested rule so source order already matches CSS order. Verified the compiled css is byte-for-byte identical before and after (same filenames, same content, same sha256 across every emitted css file). (cherry picked from commit a181e304a482a517fc4471d5b5c9b546586c2b59) --- ui/src/components/Comment/index.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/components/Comment/index.scss b/ui/src/components/Comment/index.scss index 7264c66c4..415fd25e9 100644 --- a/ui/src/components/Comment/index.scss +++ b/ui/src/components/Comment/index.scss @@ -23,6 +23,7 @@ .comments-wrap { .comment-item { + border-bottom: 1px solid var(--an-comment-item-border-bottom); &:hover { @include media-breakpoint-up(md) { .control-area { @@ -30,7 +31,6 @@ } } } - border-bottom: 1px solid var(--an-comment-item-border-bottom); } .fmt { display: inline; From d1e5f4b6c6b2b7339b20c4e7659227f892cd3782 Mon Sep 17 00:00:00 2001 From: Xan Torres Date: Mon, 3 Aug 2026 00:54:27 +0800 Subject: [PATCH 05/13] fix: point the 403 route at its existing page module The 403 route pointed at pages/403, one directory above the module that actually implements it. The real page lives at pages/404/403/index.tsx, a nested sibling of the generic 404 page, matching the same depth-two pattern already used by routes such as Admin/Mcp and Legal/Tos. Because the referenced module never existed, the route always fell through to the router's generic error boundary instead of rendering the real 403 content. Before the migration to Vite, a lookup miss like this failed silently and produced a blank or default fallback. The new router surfaces the mismatch as a visible error boundary, so the same wrong path now breaks loudly instead of quietly. Pointing it at the module that actually exists fixes both behaviors. (cherry picked from commit e50aa010c8b00f355da5746d3f760cab4b604ded) --- ui/src/router/routes.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/router/routes.ts b/ui/src/router/routes.ts index 8423fb7a3..e0b48c809 100644 --- a/ui/src/router/routes.ts +++ b/ui/src/router/routes.ts @@ -537,7 +537,7 @@ const routes: RouteNode[] = [ }, { path: '403', - page: 'pages/403', + page: 'pages/404/403', }, ], }, From 8e92cd7bc5cdbbbe54ebe75d8348705e421b7fd8 Mon Sep 17 00:00:00 2001 From: Xan Torres Date: Mon, 3 Aug 2026 01:01:57 +0800 Subject: [PATCH 06/13] test: fail the asset check when its Go test no longer exists go test -run with a pattern matching zero tests exits 0, not an error. If TEST_NAME in this script ever went stale, because the underlying test got renamed or deleted, the check would keep reporting success while testing nothing, forever, silently. Add a guard that lists the package for the exact test name before the first real check runs. If the name is not found, the script now fails loudly and explains why, instead of quietly turning into a no-op. (cherry picked from commit caf88df820a3462405635cd1e11f60e13bf75705) --- script/check-built-assets.sh | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/script/check-built-assets.sh b/script/check-built-assets.sh index e7672baf4..d7362d4db 100755 --- a/script/check-built-assets.sh +++ b/script/check-built-assets.sh @@ -32,6 +32,12 @@ INDEX_HTML="$REPO_ROOT/ui/build/index.html" TEST_PACKAGE="./internal/controller/" TEST_NAME="TestGetStyleResolvesBuiltAssets" +list_output="$(cd "$REPO_ROOT" && go test "$TEST_PACKAGE" -list "^${TEST_NAME}$" 2>&1)" +if ! grep -qx "$TEST_NAME" <<<"$list_output"; then + echo "no test named $TEST_NAME in $TEST_PACKAGE; go test -run with a stale/renamed test name matches nothing and still exits 0, which would turn this check into a silent no-op" >&2 + exit 1 +fi + skip_build=0 self_check=0 for arg in "$@"; do From 5bf72094905918d0a1930bd31af027de26a6609d Mon Sep 17 00:00:00 2001 From: Xan Torres Date: Mon, 3 Aug 2026 01:22:42 +0800 Subject: [PATCH 07/13] test: fail the frontend checks closed instead of exiting mid-cleanup fail() in check-locale-resolution.js and check-plugin-i18n-order.js called process.exit(1) synchronously on an assertion failure. That skips any pending finally block, so the dev server each script starts was never closed once fail() ran. It only looked fine because killing the process tears down the listening socket anyway, but it left no real cleanup path, and it would have silently broken the moment anything meaningful was added after the finally in the future. fail() now throws instead, and is caught once at the top level, so the finally that closes the dev server always runs before the process exits. The top-level catch sets process.exitCode instead of calling process.exit, so the event loop can drain naturally rather than being killed mid-async-work. Both scripts also bound their dev-server-start and module-runner-import calls with a 30 second timeout, so a hang in either one fails the check instead of blocking it forever. Confirmed the fix by perturbing a working copy to force a failure: the new code prints the failure and exits 1, but only after the finally block's cleanup step actually runs; forcing the same failure through the old process.exit(1) shape prints the failure and exits 1 without ever reaching that cleanup step. (cherry picked from commit 98804a0c35d3d5d6abb05c2f65d87b775b2fcbf2) --- ui/scripts/check-locale-resolution.js | 28 +++++++++++++++++---- ui/scripts/check-plugin-i18n-order.js | 35 ++++++++++++++++++++++----- 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/ui/scripts/check-locale-resolution.js b/ui/scripts/check-locale-resolution.js index 64ad275ff..83d4306a7 100644 --- a/ui/scripts/check-locale-resolution.js +++ b/ui/scripts/check-locale-resolution.js @@ -60,8 +60,15 @@ const TARGET_SCRIPT = /[一-鿿]/; const rel = (file) => path.relative(UI_DIR, file); function fail(message) { - console.error(`FAIL: ${message}`); - process.exit(1); + throw new Error(`FAIL: ${message}`); +} + +function withTimeout(promise, ms, message) { + let timer; + const timeout = new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(`FAIL: ${message}`)), ms); + }); + return Promise.race([promise, timeout]).finally(() => clearTimeout(timer)); } // The probe carries a copy of the application's import expression, so it is @@ -92,7 +99,11 @@ async function openProbe() { configFile: DEV_SERVER_CONFIG, logLevel: 'warn', }); - await server.listen(); + await withTimeout( + server.listen(), + 30000, + 'dev server did not start within 30s', + ); const ssr = server.environments.ssr; const baseUrl = (server.resolvedUrls.local[0] || '').replace(/\/$/, ''); @@ -103,7 +114,11 @@ async function openProbe() { let probe = null; const importProbe = async () => { if (!probe) { - probe = await createServerModuleRunner(ssr).import(PROBE); + probe = await withTimeout( + createServerModuleRunner(ssr).import(PROBE), + 30000, + 'module runner did not import the probe within 30s', + ); } return probe; }; @@ -212,4 +227,7 @@ async function main() { } } -main().catch((err) => fail(err.stack || String(err))); +main().catch((err) => { + console.error(err.message || err.stack || String(err)); + process.exitCode = 1; +}); diff --git a/ui/scripts/check-plugin-i18n-order.js b/ui/scripts/check-plugin-i18n-order.js index 54d846a69..b8051c1fa 100644 --- a/ui/scripts/check-plugin-i18n-order.js +++ b/ui/scripts/check-plugin-i18n-order.js @@ -50,8 +50,15 @@ const SENTINEL = 'registered before init'; const rel = (file) => path.relative(UI_DIR, file); function fail(message) { - console.error(`FAIL: ${message}`); - process.exit(1); + throw new Error(`FAIL: ${message}`); +} + +function withTimeout(promise, ms, message) { + let timer; + const timeout = new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(`FAIL: ${message}`)), ms); + }); + return Promise.race([promise, timeout]).finally(() => clearTimeout(timer)); } async function main() { @@ -75,7 +82,11 @@ async function main() { // guards it too. ssr: { noExternal: ['i18next'] }, }); - await server.listen(); + await withTimeout( + server.listen(), + 30000, + 'dev server did not start within 30s', + ); try { const runner = createServerModuleRunner(server.environments.ssr); @@ -83,8 +94,17 @@ async function main() { // Deliberately load the plugin helper first and never touch the app's own // i18n bootstrap, so i18next is guaranteed to be uninitialised here. This // is the ordering the bundler is free to produce. - const pluginUtils = await runner.import(PLUGIN_UTILS); - const i18next = (await runner.import('i18next')).default; + const pluginUtils = await withTimeout( + runner.import(PLUGIN_UTILS), + 30000, + 'module runner did not import the plugin utils within 30s', + ); + const i18nextModule = await withTimeout( + runner.import('i18next'), + 30000, + 'module runner did not import i18next within 30s', + ); + const i18next = i18nextModule.default; // If the helper and this check hold different copies, every assertion // below is measuring an object nothing under test ever touched. @@ -148,4 +168,7 @@ async function main() { } } -main().catch((err) => fail(err.stack || String(err))); +main().catch((err) => { + console.error(err.message || err.stack || String(err)); + process.exitCode = 1; +}); From eb1519b4bbb821ed69cde7379245ee437a4e3ca5 Mon Sep 17 00:00:00 2001 From: Xan Torres Date: Mon, 3 Aug 2026 01:35:44 +0800 Subject: [PATCH 08/13] test: cover immediate plugin translation registration too The check only ever registered a plugin's translations before i18next.init ran, which exercises the deferred branch in initI18nResource: the listener registered for the 'initialized' event. The immediate branch, which fires synchronously when i18next is already initialised, had no coverage at all. Add a second registration right after init and read the resource bundle back with no i18next event and no other statement in between. The 'initialized' event has already fired for the first scenario at that point, so the new translations only land if the immediate branch actually runs. Without it, this second registration would never appear in the bundle, and every plugin whose module evaluates after i18next.init would render untranslated. (cherry picked from commit 90d1c25a8877761ecda006b77e61a6116f42d7dc) --- ui/scripts/check-plugin-i18n-order.js | 34 ++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/ui/scripts/check-plugin-i18n-order.js b/ui/scripts/check-plugin-i18n-order.js index b8051c1fa..9ad8b335a 100644 --- a/ui/scripts/check-plugin-i18n-order.js +++ b/ui/scripts/check-plugin-i18n-order.js @@ -160,8 +160,40 @@ async function main() { ); } + // i18next.init already ran above, and the 'initialized' event it fires + // has already been consumed by the first scenario's listener. Registering + // a second plugin now only works if initI18nResource also takes the + // immediate branch, which had no coverage until this scenario existed. + const slugAfterInit = 'check_only_plugin_after_init'; + const sentinelAfterInit = 'registered after init'; + + pluginUtils.initI18nResource({ + [LANG]: { + plugin: { [slugAfterInit]: { ui: { title: sentinelAfterInit } } }, + }, + }); + const bundleAfterInit = i18next.getResourceBundle(LANG, PLUGIN_NS); + const titleAfterInit = + bundleAfterInit && + bundleAfterInit[slugAfterInit] && + bundleAfterInit[slugAfterInit].ui && + bundleAfterInit[slugAfterInit].ui.title; + + if (titleAfterInit !== sentinelAfterInit) { + fail( + `registering a plugin's translations after i18next.init did not land ` + + `synchronously: expected ${util.inspect(sentinelAfterInit)} at ` + + `${PLUGIN_NS}.${slugAfterInit}.ui.title, got ${util.inspect(bundleAfterInit)}.\n` + + ` The 'initialized' event already fired for the first scenario, so this ` + + `only passes if initI18nResource also registers immediately when i18next ` + + `is already initialised. Missing that branch means every plugin loaded ` + + `after i18next.init renders untranslated.`, + ); + } + console.log( - `OK: plugin translations registered before i18next.init survive and are present afterwards`, + `OK: plugin translations registered before i18next.init (deferred) and ` + + `after i18next.init (immediate) both survive and are present`, ); } finally { await server.close(); From 8eae8b8b71876ddce115b4c3d48bd61d5a201314 Mon Sep 17 00:00:00 2001 From: Xan Torres Date: Mon, 3 Aug 2026 01:41:41 +0800 Subject: [PATCH 09/13] test: assert a non-stylesheet link never counts as a stylesheet The three existing self-check fixtures all under-count: one drops the script, one drops the stylesheet, and one has a script tag with no src. None of them would catch a parser that over-matches, counting any link tag as a stylesheet regardless of its rel attribute. Add a fixture with a real script (has a src) and a single non-stylesheet link (rel="manifest") and nothing else. Only a parser that keys off rel="stylesheet" specifically, rather than just the presence of a link tag, correctly rejects this page. (cherry picked from commit 5b89c4bdf36be752abd4f028946085c1ed9443b1) --- script/check-built-assets.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/script/check-built-assets.sh b/script/check-built-assets.sh index d7362d4db..3a1fcf19b 100755 --- a/script/check-built-assets.sh +++ b/script/check-built-assets.sh @@ -97,4 +97,7 @@ expect_failure "script src but no stylesheet link" \ expect_failure "only an inline script, no src" \ '
' +expect_failure "manifest link but no stylesheet link" \ + '
' + echo "==> self-check passed" From 3d49bb8cf8ce197f0ecb7a11ea599ea89804229a Mon Sep 17 00:00:00 2001 From: Xan Torres Date: Mon, 3 Aug 2026 03:17:07 +0800 Subject: [PATCH 10/13] test: close the dev server when its startup times out withTimeout() wrapped server.listen() but ran before either script could reach its own cleanup path. In check-locale-resolution.js a listen failure threw out of openProbe() before it returned the closable probe, so main()'s try/finally never started and the server openProbe() had already created was never closed. In check-plugin-i18n-order.js the same call sat above the try/finally entirely, so a listen failure skipped past the close() in finally. Vite creates the watcher and websocket server before listen() runs, so either shape leaves the process alive forever on a timeout: the event loop never drains and nothing closes the sockets. Moved the listen call inside a try that always reaches the close: an inline try/catch around openProbe()'s listen for the first file, and the existing try/finally extended to cover listen for the second. A scratch repro mirroring both shapes with a listen that never settles confirms the process now exits on its own once the timeout fires, instead of hanging until something else kills it. (cherry picked from commit 5fcb134ca09a9250b171b62c2c5c0110bbaeb711) --- ui/scripts/check-locale-resolution.js | 19 ++++++++++++++----- ui/scripts/check-plugin-i18n-order.js | 12 ++++++------ 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/ui/scripts/check-locale-resolution.js b/ui/scripts/check-locale-resolution.js index 83d4306a7..819f8b1c6 100644 --- a/ui/scripts/check-locale-resolution.js +++ b/ui/scripts/check-locale-resolution.js @@ -99,11 +99,20 @@ async function openProbe() { configFile: DEV_SERVER_CONFIG, logLevel: 'warn', }); - await withTimeout( - server.listen(), - 30000, - 'dev server did not start within 30s', - ); + try { + await withTimeout( + server.listen(), + 30000, + 'dev server did not start within 30s', + ); + } catch (err) { + // listen() failed or timed out, but the server (and the watcher and + // websocket server it created before listen() ever ran) already exists. + // Nothing else will hold a reference to it once this throws, so close it + // here or it keeps the event loop alive forever. + await server.close().catch(() => {}); + throw err; + } const ssr = server.environments.ssr; const baseUrl = (server.resolvedUrls.local[0] || '').replace(/\/$/, ''); diff --git a/ui/scripts/check-plugin-i18n-order.js b/ui/scripts/check-plugin-i18n-order.js index 9ad8b335a..41a89403f 100644 --- a/ui/scripts/check-plugin-i18n-order.js +++ b/ui/scripts/check-plugin-i18n-order.js @@ -82,13 +82,13 @@ async function main() { // guards it too. ssr: { noExternal: ['i18next'] }, }); - await withTimeout( - server.listen(), - 30000, - 'dev server did not start within 30s', - ); - try { + await withTimeout( + server.listen(), + 30000, + 'dev server did not start within 30s', + ); + const runner = createServerModuleRunner(server.environments.ssr); // Deliberately load the plugin helper first and never touch the app's own From 88a595865971bba2ec83d9a4c6b895c6d2a00349 Mon Sep 17 00:00:00 2001 From: Xan Torres Date: Mon, 3 Aug 2026 04:16:16 +0800 Subject: [PATCH 11/13] test: bound server creation and guarantee check termination creating the dev server can hang before listen is ever called, and a hang there leaves live handles that keep the process alive even after the timeout error is reported; bound creation with the same timeout, exit hard once the final catch has reported (all cleanup has run by then), and stop a failing close from replacing the error that actually caused the failure. close() was still unbounded: it awaits plugin buildEnd and closeBundle hooks with no timeout of its own, so a hung close blocked the failure path's own exit, and on the success path could hang the process after the OK line had already printed. Bound every close() call the same way, and exit explicitly on both success and failure, since a leaked handle from any bounded step, close included, must not keep a finished check alive. (cherry picked from commit 62955d7469e238b24838e7c37c2ae03a20697599) --- ui/scripts/check-locale-resolution.js | 48 ++++++++++++++++++++------- ui/scripts/check-plugin-i18n-order.js | 47 +++++++++++++++++--------- 2 files changed, 67 insertions(+), 28 deletions(-) diff --git a/ui/scripts/check-locale-resolution.js b/ui/scripts/check-locale-resolution.js index 819f8b1c6..026dec5e6 100644 --- a/ui/scripts/check-locale-resolution.js +++ b/ui/scripts/check-locale-resolution.js @@ -94,11 +94,15 @@ async function openProbe() { } const { createServer, createServerModuleRunner } = await import('vite'); - const server = await createServer({ - root: UI_DIR, - configFile: DEV_SERVER_CONFIG, - logLevel: 'warn', - }); + const server = await withTimeout( + createServer({ + root: UI_DIR, + configFile: DEV_SERVER_CONFIG, + logLevel: 'warn', + }), + 30000, + 'dev server creation did not complete within 30s', + ); try { await withTimeout( server.listen(), @@ -110,7 +114,11 @@ async function openProbe() { // websocket server it created before listen() ever ran) already exists. // Nothing else will hold a reference to it once this throws, so close it // here or it keeps the event loop alive forever. - await server.close().catch(() => {}); + await withTimeout( + server.close(), + 10000, + 'dev server close did not complete within 10s', + ).catch(() => {}); throw err; } @@ -182,7 +190,12 @@ async function openProbe() { } }, - close: () => server.close(), + close: () => + withTimeout( + server.close(), + 10000, + 'dev server close did not complete within 10s', + ).catch(() => {}), }; } @@ -232,11 +245,22 @@ async function main() { `resources, and ${TARGET_LANG} is fetchable over the dev server`, ); } finally { - await probe.close(); + await withTimeout( + probe.close(), + 10000, + 'probe close did not complete within 10s', + ).catch(() => {}); } } -main().catch((err) => { - console.error(err.message || err.stack || String(err)); - process.exitCode = 1; -}); +// Every finally above has already run by the time either callback below +// fires, so a hard exit here cannot skip cleanup; it only guarantees +// termination on both outcomes, including when a bounded-but-hung close +// would otherwise keep a finished check alive after success. +main().then( + () => process.exit(0), + (err) => { + console.error(err.message || err.stack || String(err)); + process.exit(1); + }, +); diff --git a/ui/scripts/check-plugin-i18n-order.js b/ui/scripts/check-plugin-i18n-order.js index 41a89403f..be83eff72 100644 --- a/ui/scripts/check-plugin-i18n-order.js +++ b/ui/scripts/check-plugin-i18n-order.js @@ -71,17 +71,21 @@ async function main() { } const { createServer, createServerModuleRunner } = await import('vite'); - const server = await createServer({ - root: UI_DIR, - configFile: DEV_SERVER_CONFIG, - logLevel: 'warn', - // Without this the helper under test and this check each resolve their own - // copy of i18next, and the check ends up inspecting an instance nobody - // registered anything into. It reads as a failure with a confusing message - // rather than as a broken harness, so the single-instance assertion below - // guards it too. - ssr: { noExternal: ['i18next'] }, - }); + const server = await withTimeout( + createServer({ + root: UI_DIR, + configFile: DEV_SERVER_CONFIG, + logLevel: 'warn', + // Without this the helper under test and this check each resolve + // their own copy of i18next, and the check ends up inspecting an + // instance nobody registered anything into. It reads as a failure + // with a confusing message rather than as a broken harness, so the + // single-instance assertion below guards it too. + ssr: { noExternal: ['i18next'] }, + }), + 30000, + 'dev server creation did not complete within 30s', + ); try { await withTimeout( server.listen(), @@ -196,11 +200,22 @@ async function main() { `after i18next.init (immediate) both survive and are present`, ); } finally { - await server.close(); + await withTimeout( + server.close(), + 10000, + 'dev server close did not complete within 10s', + ).catch(() => {}); } } -main().catch((err) => { - console.error(err.message || err.stack || String(err)); - process.exitCode = 1; -}); +// Every finally above has already run by the time either callback below +// fires, so a hard exit here cannot skip cleanup; it only guarantees +// termination on both outcomes, including when a bounded-but-hung close +// would otherwise keep a finished check alive after success. +main().then( + () => process.exit(0), + (err) => { + console.error(err.message || err.stack || String(err)); + process.exit(1); + }, +); From 0ad872b039f94c131943600400691e086e42d7fe Mon Sep 17 00:00:00 2001 From: Xan Torres Date: Mon, 3 Aug 2026 05:20:08 +0800 Subject: [PATCH 12/13] test: fail the checks by default until they prove success the earlier commits bound every known-hangable step; any await this or a future change leaves unbounded could still hang the run, or worse, drain the event loop and exit 0 silently. Preset the exit code to failure, add an unreferenced whole-run watchdog that forces a verdict through any held-alive hang, and keep both explicit exits, so every termination mode ends with the code that matches what was proven. (cherry picked from commit 61e41ce144a940b85c3614e496b08638dce778f7) --- ui/scripts/check-locale-resolution.js | 15 +++++++++++++++ ui/scripts/check-plugin-i18n-order.js | 15 +++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/ui/scripts/check-locale-resolution.js b/ui/scripts/check-locale-resolution.js index 026dec5e6..abdc89b09 100644 --- a/ui/scripts/check-locale-resolution.js +++ b/ui/scripts/check-locale-resolution.js @@ -57,6 +57,21 @@ const TARGET_LANG = 'zh_CN'; const DEFAULT_LANG = 'en_US'; const TARGET_SCRIPT = /[一-鿿]/; +// The check is a failure until it proves otherwise, so an unexpected +// event-loop drain (a pending promise with no live handles) cannot exit 0. +process.exitCode = 1; + +// unref lets a finished run exit on time; a run held alive by any leaked +// or hung handle still gets terminated with a verdict. +// Must exceed the sum of every bounded step below (worst case 100s), or a +// run whose steps all succeed slowly gets a false FAIL from its own backstop. +const WATCHDOG_MS = 180000; +const watchdog = setTimeout(() => { + console.error(`FAIL: check did not complete within ${WATCHDOG_MS / 1000}s`); + process.exit(1); +}, WATCHDOG_MS); +watchdog.unref(); + const rel = (file) => path.relative(UI_DIR, file); function fail(message) { diff --git a/ui/scripts/check-plugin-i18n-order.js b/ui/scripts/check-plugin-i18n-order.js index be83eff72..5c87895ec 100644 --- a/ui/scripts/check-plugin-i18n-order.js +++ b/ui/scripts/check-plugin-i18n-order.js @@ -47,6 +47,21 @@ const PLUGIN_NS = 'plugin'; const SLUG = 'check_only_plugin'; const SENTINEL = 'registered before init'; +// The check is a failure until it proves otherwise, so an unexpected +// event-loop drain (a pending promise with no live handles) cannot exit 0. +process.exitCode = 1; + +// unref lets a finished run exit on time; a run held alive by any leaked +// or hung handle still gets terminated with a verdict. +// Must exceed the sum of every bounded step below (worst case 130s), or a +// run whose steps all succeed slowly gets a false FAIL from its own backstop. +const WATCHDOG_MS = 180000; +const watchdog = setTimeout(() => { + console.error(`FAIL: check did not complete within ${WATCHDOG_MS / 1000}s`); + process.exit(1); +}, WATCHDOG_MS); +watchdog.unref(); + const rel = (file) => path.relative(UI_DIR, file); function fail(message) { From 339ddd711499457f812bf0246c5591a159cf8dae Mon Sep 17 00:00:00 2001 From: Xan Torres Date: Wed, 19 Aug 2026 12:42:29 +0800 Subject: [PATCH 13/13] chore: align asset-check comments with the DOM-tolerant parser The GetStyle comment framed the DOM walk around attribute order, attribute set, and quoting, the same properties the old regex parser depended on, without stating that the new parser ignores all of them. check-built-assets.sh described its self-check as failing when asset tags change shape, but the fixtures test a missing script or stylesheet tag, and the parser accepts any shape as long as the tag is present. Comments now state the actual constraint: tag shape does not affect parsing, and the guarding check fails when a build is present but a required script or stylesheet tag is missing from it. (cherry picked from commit 3bc12e80a659a08dbe72c840a60b25c8c0f90963, script/check-built-assets.sh only) --- script/check-built-assets.sh | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/script/check-built-assets.sh b/script/check-built-assets.sh index 3a1fcf19b..6e8d2cc8b 100755 --- a/script/check-built-assets.sh +++ b/script/check-built-assets.sh @@ -21,9 +21,10 @@ # why that is not implied by a successful build. # # --skip-build reuse an existing ui/build, do not rebuild -# --self-check additionally rewrite ui/build/index.html with asset tag -# shapes the server cannot parse and confirm the check fails -# on them. Restores the real build output afterwards. +# --self-check additionally rewrite ui/build/index.html so a script +# or stylesheet tag is missing, and confirm the check +# fails on the missing asset. Restores the real build +# output afterwards. set -euo pipefail @@ -69,9 +70,9 @@ if [ "$self_check" -eq 0 ]; then exit 0 fi -# Confirm the check actually fails when the asset tags change shape. Without -# this, a check that silently stopped asserting anything would look identical -# to a passing one. +# Confirm the check actually fails when a required asset is missing from +# the build output. Without this, a check that silently stopped asserting +# anything would look identical to a passing one. backup="$(mktemp)" cp "$INDEX_HTML" "$backup" trap 'cp "$backup" "$INDEX_HTML"; rm -f "$backup"' EXIT