fix: final review pass — email collision, orphaned-run loop, null confidence guards

- changeEmail validates and refuses an address another account owns (users.email is UNIQUE; a collision was a 500 inside the settings form).
- /app/[id] no longer redirects a 'running' row to itself; it renders what exists and says the run was interrupted.
- ScoreBar / OptionRow / VerdictBlock survive null numbers from the model instead of throwing and losing the block (seen live: '[openui] runtime/render-error: Component ScoreBar render failed').
- drop the ad-hoc render-debug scripts now that Playwright covers the browser.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
ahmed mohammed
2026-09-22 09:10:29 +03:00
co-authored by Claude Opus 4.7
parent 07dd3d6bf8
commit 5160a00db5
12 changed files with 101 additions and 80 deletions
+1 -2
View File
@@ -19,7 +19,6 @@ next-env.d.ts
/test-results /test-results
/playwright-report /playwright-report
# local fixtures # e2e fixture input is committed; scratch output is not
/tests/__fixtures__
.DS_Store .DS_Store
.playwright-data .playwright-data
+6 -2
View File
@@ -1,4 +1,4 @@
import { notFound, redirect } from "next/navigation"; import { notFound } from "next/navigation";
import { requireUser } from "@/lib/auth"; import { requireUser } from "@/lib/auth";
import { getAnalysis } from "@/lib/db"; import { getAnalysis } from "@/lib/db";
import { ReportView } from "@/components/report-view"; import { ReportView } from "@/components/report-view";
@@ -9,9 +9,13 @@ export default async function Stored({ params }: { params: Promise<{ id: string
const u = await requireUser(); const u = await requireUser();
const a = getAnalysis((await params).id, u.id); const a = getAnalysis((await params).id, u.id);
if (!a) notFound(); if (!a) notFound();
if (a.status === "running") redirect(`/app/${a.id}`); // still streaming elsewhere; rare, bounce once // Never redirect here: a row left "running" by a crashed process would bounce this page to itself forever.
const interrupted = a.status === "running";
return <main style={{ display: "grid", gap: "var(--qai-space-xl)" }}> return <main style={{ display: "grid", gap: "var(--qai-space-xl)" }}>
<h1 style={{ fontFamily: "var(--qai-font-display)", fontSize: "var(--qai-text-2xl)", margin: 0 }}>{a.title}</h1> <h1 style={{ fontFamily: "var(--qai-font-display)", fontSize: "var(--qai-text-2xl)", margin: 0 }}>{a.title}</h1>
{interrupted && <p role="alert" style={{ color: "var(--qai-warn)" }}>
This run was interrupted before it finished — what is below is everything it managed to produce.
</p>}
{a.error && <p role="alert" style={{ color: "var(--qai-danger)" }}>{a.error}</p>} {a.error && <p role="alert" style={{ color: "var(--qai-danger)" }}>{a.error}</p>}
{a.verdict && <VerdictSummary verdict={JSON.parse(a.verdict)} />} {a.verdict && <VerdictSummary verdict={JSON.parse(a.verdict)} />}
{a.oul ? <><ExportButtons oul={a.oul} title={a.title} /><ReportView oul={a.oul} streaming={false} /></> {a.oul ? <><ExportButtons oul={a.oul} title={a.title} /><ReportView oul={a.oul} streaming={false} /></>
+6 -4
View File
@@ -2,7 +2,7 @@
import { redirect } from "next/navigation"; import { redirect } from "next/navigation";
import { revalidatePath } from "next/cache"; import { revalidatePath } from "next/cache";
import { userByEmail, getSettings, renameAnalysis, deleteAnalysis, db } from "@/lib/db"; import { userByEmail, getSettings, renameAnalysis, deleteAnalysis, db } from "@/lib/db";
import { signUpNewUser, verifyPassword, startSession, endSession, requireUser, emailOk, hashPassword } from "@/lib/auth"; import { signUpNewUser, verifyPassword, startSession, endSession, requireUser, changeEmail, emailOk, hashPassword } from "@/lib/auth";
import { saveSettingsForm } from "@/lib/settings"; import { saveSettingsForm } from "@/lib/settings";
export async function signUp(_: unknown, fd: FormData) { export async function signUp(_: unknown, fd: FormData) {
@@ -36,16 +36,18 @@ export async function remove(_: unknown, fd: FormData) {
export async function saveSettingsAction(_: unknown, fd: FormData) { export async function saveSettingsAction(_: unknown, fd: FormData) {
const u = await requireUser(); const u = await requireUser();
const email = String(fd.get("email") ?? "").trim().toLowerCase(); const email = String(fd.get("email") ?? "").trim();
const password = String(fd.get("password") ?? ""); const password = String(fd.get("password") ?? "");
if (email && !emailOk(email)) return { ok: false, error: "Enter a valid email address." };
if (password && password.length < 10) return { ok: false, error: "Use at least 10 characters." }; if (password && password.length < 10) return { ok: false, error: "Use at least 10 characters." };
saveSettingsForm(u.id, fd); saveSettingsForm(u.id, fd);
// The wizard sends `after`: a configured user must land on the readiness screen, and the setup page // The wizard sends `after`: a configured user must land on the readiness screen, and the setup page
// forwards configured users away from steps 1-2, so "Saved." would never be seen there. // forwards configured users away from steps 1-2, so "Saved." would never be seen there.
if (String(fd.get("after") ?? "").startsWith("/setup")) redirect("/setup?step=3"); if (String(fd.get("after") ?? "").startsWith("/setup")) redirect("/setup?step=3");
if (password) db.prepare("UPDATE users SET pass = ? WHERE id = ?").run(hashPassword(password), u.id); if (password) db.prepare("UPDATE users SET pass = ? WHERE id = ?").run(hashPassword(password), u.id);
if (email) db.prepare("UPDATE users SET email = ? WHERE id = ?").run(email, u.id); if (email) {
const r = changeEmail(u.id, email);
if (!r.ok) return r;
}
revalidatePath("/settings"); revalidatePath("/settings");
return { ok: true }; return { ok: true };
} }
+11 -1
View File
@@ -1,5 +1,5 @@
import { randomBytes, scryptSync, createHash, timingSafeEqual } from "node:crypto"; import { randomBytes, scryptSync, createHash, timingSafeEqual } from "node:crypto";
import { createUser, userByEmail, userEmail, putSession, sessionUserId, dropSession, getSettings } from "./db.ts"; import { createUser, userByEmail, userEmail, putSession, sessionUserId, dropSession, getSettings, db } from "./db.ts";
export const SESSION_COOKIE = "qai_s"; export const SESSION_COOKIE = "qai_s";
export const SESSION_DAYS = 30; export const SESSION_DAYS = 30;
@@ -34,6 +34,16 @@ export function signUpNewUser(email: string, password: string): { ok: true; id:
// request-scoped binding must not be touched at import time. // request-scoped binding must not be touched at import time.
const jar = async () => (await import("next/headers")).cookies(); const jar = async () => (await import("next/headers")).cookies();
export function changeEmail(userId: string, email: string): { ok: true } | { ok: false; error: string } {
const e = email.trim().toLowerCase();
if (!emailOk(e)) return { ok: false, error: "Enter a valid email address." };
const taken = userByEmail(e);
if (taken && taken.id !== userId) return { ok: false, error: "That email already has an account." };
// users.email is UNIQUE: without this check a collision is a 500 inside the settings form.
if (!taken) db.prepare("UPDATE users SET email = ? WHERE id = ?").run(e, userId);
return { ok: true };
}
export async function startSession(userId: string) { export async function startSession(userId: string) {
const token = randomBytes(32).toString("hex"); const token = randomBytes(32).toString("hex");
putSession(hashToken(token), userId, Date.now() + SESSION_DAYS * 864e5); putSession(hashToken(token), userId, Date.now() + SESSION_DAYS * 864e5);
+10 -5
View File
@@ -10,6 +10,11 @@ const muted = { color: "var(--qai-ink-muted)" };
const small = { fontSize: "var(--qai-text-xs)", color: "var(--qai-ink-muted)" }; const small = { fontSize: "var(--qai-text-xs)", color: "var(--qai-ink-muted)" };
const disp = { fontFamily: "var(--qai-font-display)" }; const disp = { fontFamily: "var(--qai-font-display)" };
// Model output is untrusted: the schema says a number is optional, models still send null, and one throw
// deletes the whole block from the report. 0 is the honest stand-in for "the model did not give a number".
const num = (v: unknown) => (typeof v === "number" && Number.isFinite(v) ? v : 0);
const conf = (v: unknown) => (typeof v === "number" ? ` ${v.toFixed(2)}` : " not reported");
const renderers: Record<string, (x: any) => any> = { const renderers: Record<string, (x: any) => any> = {
Headline: ({ props }) => el("header", null, Headline: ({ props }) => el("header", null,
el("p", { className: "label" }, props.severity === "critical" ? "Critical" : props.severity === "warning" ? "Warning" : "Note"), el("p", { className: "label" }, props.severity === "critical" ? "Critical" : props.severity === "warning" ? "Warning" : "Note"),
@@ -50,14 +55,14 @@ const renderers: Record<string, (x: any) => any> = {
props.caption ? el("p", { style: small }, props.caption) : null), props.caption ? el("p", { style: small }, props.caption) : null),
ScoreBar: ({ props }) => el("div", null, ScoreBar: ({ props }) => el("div", null,
el("p", { className: "label" }, props.label + (props.confidence !== undefined el("p", { className: "label" }, props.label + (props.confidence === undefined
? ` · confidence ${props.confidence.toFixed(2)}` : " · no confidence reported")), ? " · no confidence reported" : ` · confidence${conf(props.confidence)}`)),
el("div", { className: "bar", style: { width: `${Math.round(props.value * 100)}%` } }), el("div", { className: "bar", style: { width: `${Math.round(num(props.value) * 100)}%` } }),
props.note ? el("p", { style: small }, props.note) : null), props.note ? el("p", { style: small }, props.note) : null),
OptionRow: ({ props }) => el("tr", { "data-headline": props.headline || undefined }, OptionRow: ({ props }) => el("tr", { "data-headline": props.headline || undefined },
el("td", null, String(props.rank)), el("td", null, props.title), el("td", null, String(props.rank)), el("td", null, props.title),
el("td", { className: "sql" }, props.composite.toFixed(2)), el("td", { className: "sql" }, num(props.composite).toFixed(2)),
el("td", null, props.flags || "—")), el("td", null, props.flags || "—")),
OptionTable: ({ props, renderNode }: any) => el("section", { className: "card" }, OptionTable: ({ props, renderNode }: any) => el("section", { className: "card" },
@@ -67,7 +72,7 @@ const renderers: Record<string, (x: any) => any> = {
VerdictBlock: ({ props }) => el("section", { className: "card", "data-verdict": "" }, VerdictBlock: ({ props }) => el("section", { className: "card", "data-verdict": "" },
el("p", { className: "label" }, "First move · " + (props.source === "jev" el("p", { className: "label" }, "First move · " + (props.source === "jev"
? `Jev, confidence ${props.confidence.toFixed(2)}` : "composite order — Jev did not decide")), ? `Jev, confidence${conf(props.confidence)}` : "composite order — Jev did not decide")),
p({ ...disp, fontSize: "var(--qai-text-lg)" }, props.headline), p({ ...disp, fontSize: "var(--qai-text-lg)" }, props.headline),
p({}, props.why)), p({}, props.why)),
-24
View File
@@ -1,24 +0,0 @@
import { chromium } from "@playwright/test";
const b = await chromium.launch();
const page = await (await b.newContext()).newPage();
page.on("console", (m) => console.log("QAI console", m.type(), m.text().slice(0, 200)));
page.on("pageerror", (e) => console.log("QAI pageerror", String(e).slice(0, 300)));
await page.goto("http://localhost:3000/signup");
await page.fill("#email", `dbg4328+${Date.now()}@qai.test`);
await page.fill("#password", "playwright password");
await page.getByRole("button", { name: "Create account" }).click();
await page.waitForTimeout(3000);
console.log("QAI url after signup", page.url());
console.log("QAI body after signup", (await page.locator("body").innerText()).replace(/\n+/g, " | ").slice(0, 260));
console.log("QAI cookie", (await page.context().cookies()).map(c=>c.name+"="+c.value.slice(0,8)).join(","));
await page.getByRole("link", { name: "Next" }).click();
await page.waitForTimeout(2000);
await page.fill("#analyst_base_url", "http://192.168.88.131:8888/v1");
await page.fill("#analyst_key", "EMPTY");
await page.fill("#analyst_model", "qwen3.8-flash-next");
await page.fill("#jev_key", "ts-test-key");
await page.getByRole("button", { name: "Save" }).click();
await page.waitForTimeout(4000);
console.log("QAI url after save", page.url());
console.log("QAI body", (await page.locator("body").innerText()).replace(/\n+/g, " | ").slice(0, 320));
await b.close();
-27
View File
@@ -1,27 +0,0 @@
import { chromium } from "@playwright/test";
const ID = process.argv[2] ?? "660dd007-cc6f-4644-a57b-9c32a012e60a";;
const b = await chromium.launch();
const ctx = await b.newContext();
await ctx.addCookies([{ name: "qai_s", value: "tokAPI", url: "http://localhost:3117" }]);
const page = await ctx.newPage();
const errs = [];
page.on("pageerror", (e) => errs.push(String(e.message).slice(0, 140)));
await page.goto(`http://localhost:3117/app/${ID}`, { waitUntil: "domcontentloaded" });
await page.waitForSelector("#report", { timeout: 25000 });
const q = async (sel) => page.locator(sel).count();
console.log("QAI report", await q("#report"), "cards", await q("#report .card"), "sql blocks", await q("#report pre.sql"),
"copy buttons", await q("#report button"), "tables", await q("#report table"), "bars", await q("#report .bar"),
"verdict", await q("[data-verdict]"), "metrics", await q(".grid-metrics .card"));
console.log("QAI headline", (await page.locator("#report h2").first().textContent())?.slice(0, 90));
console.log("QAI console errors", errs.length, errs.slice(0, 2));
for (const w of [320, 375, 414, 768]) {
await page.setViewportSize({ width: w, height: 800 });
await page.waitForTimeout(250);
const over = await page.evaluate(() => document.documentElement.scrollWidth - document.documentElement.clientWidth);
console.log(`QAI ${w}px overflow ${over}`);
}
await page.setViewportSize({ width: 1280, height: 900 });
await page.emulateMedia({ media: "print" });
console.log("QAI print rail hidden", await page.locator(".rail").isHidden(), "toolbar hidden", await page.locator(".toolbar").first().isHidden(),
"body bg", await page.evaluate(() => getComputedStyle(document.body).backgroundColor));
await b.close();
-13
View File
@@ -1,13 +0,0 @@
import { chromium } from "@playwright/test";
const ID = process.argv[2];
const PORT = process.argv[3] ?? "3117";
const b = await chromium.launch();
const ctx = await b.newContext();
await ctx.addCookies([{ name: "qai_s", value: "tokAPI", url: `http://localhost:${PORT}` }]);
const page = await ctx.newPage();
page.on("console", (m) => { if (m.type() === "error") console.log("QAI console", m.text().slice(0, 400)); });
page.on("pageerror", (e) => console.log("QAI pageerror", String(e).slice(0, 500)));
await page.goto(`http://localhost:${PORT}/app/${ID}`, { waitUntil: "load" });
await page.waitForTimeout(6000);
console.log("QAI report count", await page.locator("#report").count());
await b.close();
+38
View File
@@ -0,0 +1,38 @@
import { test, expect } from "@playwright/test";
import { DatabaseSync } from "node:sqlite";
import { randomUUID } from "node:crypto";
test.use({ storageState: ".auth/user.json" });
const DB = ".playwright-data/qai.db";
// The app opens this file too (WAL), so writes here are just another connection.
function withDb<T>(fn: (d: DatabaseSync) => T): T {
const d = new DatabaseSync(DB);
try { return fn(d); } finally { d.close(); }
}
test("a row left running renders what exists instead of redirecting forever", async ({ page }) => {
const id = withDb((d) => {
const userId = d.prepare("SELECT id FROM users ORDER BY rowid DESC LIMIT 1").get() as { id: string };
const aid = randomUUID();
// Backdated so it never becomes the rail's first row: history.spec.ts depends on that row being a real run.
d.prepare("INSERT INTO analyses (id, user_id, title, xml, status, created_at, updated_at) VALUES (?,?,?,?,?,?,?)")
.run(aid, userId.id, "Orphaned run", "<ShowPlanXML/>", "running", Date.now() - 3_600_000, Date.now() - 3_600_000);
return aid;
});
await page.goto(`/app/${id}`);
await expect(page.getByText(/This run was interrupted before it finished/)).toBeVisible({ timeout: 15_000 });
expect(page.url()).toContain(`/app/${id}`); // the old redirect sent this to itself
await expect(page.getByRole("button", { name: "Analyse" })).toHaveCount(0);
});
test("an email another account already has is refused inline", async ({ page }) => {
withDb((d) => d.prepare("INSERT INTO users (id, email, pass, created_at) VALUES (?,?,?,?)")
.run(randomUUID(), "taken@qai.test", "salt:hash", Date.now()));
await page.goto("/settings");
await page.fill("#email", "taken@qai.test");
await page.getByRole("button", { name: "Save" }).click();
await expect(page.locator("form p[role=alert]")).toContainText("That email already has an account", { timeout: 15_000 });
const still = withDb((d) => (d.prepare("SELECT email FROM users WHERE email = ?").get("taken@qai.test") as { email: string }).email);
expect(still).toBe("taken@qai.test");
});
+3 -2
View File
@@ -19,7 +19,8 @@ test("every settings field has a visible label", async ({ page }) => {
await page.goto("/settings"); await page.goto("/settings");
// Scoped to the form: the rail's rename boxes are labelled for assistive tech with aria-label, not visibly. // Scoped to the form: the rail's rename boxes are labelled for assistive tech with aria-label, not visibly.
const orphans = await page.evaluate(() => [...document.querySelectorAll("form input")] const orphans = await page.evaluate(() => [...document.querySelectorAll("form input")]
.filter((i) => i.type !== "hidden" && (!i.id || !document.querySelector(`label[for="${i.id}"]`))) .filter((el) => { const i = el as HTMLInputElement;
.map((i) => i.name || i.type)); return i.type !== "hidden" && (!i.id || !document.querySelector(`label[for="${i.id}"]`)); })
.map((el) => (el as HTMLInputElement).name || (el as HTMLInputElement).type));
expect(orphans).toEqual([]); expect(orphans).toEqual([]);
}); });
+13
View File
@@ -18,3 +18,16 @@ test("token hash is stable, 64 hex", () => {
assert.equal(hashToken("abc"), hashToken("abc")); assert.equal(hashToken("abc"), hashToken("abc"));
assert.match(hashToken("abc"), /^[0-9a-f]{64}$/); assert.match(hashToken("abc"), /^[0-9a-f]{64}$/);
}); });
test("changeEmail refuses an address another account already has", async () => {
const { signUpNewUser, changeEmail } = await import("../lib/auth.ts");
const db = await import("../lib/db.ts");
const a = signUpNewUser("first@x.test", "0123456789ab");
const b = signUpNewUser("second@x.test", "0123456789ab");
assert.ok(a.ok && b.ok);
assert.deepEqual(changeEmail(b.id, "first@x.test"), { ok: false, error: "That email already has an account." });
assert.deepEqual(changeEmail(b.id, "not an email"), { ok: false, error: "Enter a valid email address." });
assert.deepEqual(changeEmail(b.id, " FIRST@X.TEST "), { ok: false, error: "That email already has an account." });
assert.equal(changeEmail(a.id, "first@x.test").ok, true, "own address is not a conflict");
assert.equal(changeEmail(b.id, "fresh@x.test").ok, true);
assert.equal(db.userByEmail("fresh@x.test")!.id, b.id, "stored lower-cased");
});
+13
View File
@@ -13,3 +13,16 @@ test("every spec has a renderer, so the model can never emit an unrenderable blo
const { library } = await import("../lib/report-ui.ts"); const { library } = await import("../lib/report-ui.ts");
for (const s of specs) assert.ok(library.components[s.name], `no renderer for ${s.name}`); for (const s of specs) assert.ok(library.components[s.name], `no renderer for ${s.name}`);
}); });
test("a null confidence from the model does not blank the block", async () => {
// The prompt says optional; models still send null. confidence != undefined let null through to .toFixed().
const { library } = await import("../lib/report-ui.ts");
const renderNode = () => null;
for (const [name, props] of [
["ScoreBar", { label: "fit", value: 0.5, confidence: null }],
["VerdictBlock", { headline: "h", why: "w", source: "jev", confidence: null }],
] as const) {
const el = library.components[name].component({ props: props as any, renderNode } as any);
assert.ok(el, `${name} returned nothing`);
}
});