diff --git a/.gitignore b/.gitignore index ea8c4bf..ec1ab35 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,3 @@ /target +tests/e2e/.host-ca-certificates/* +!tests/e2e/.host-ca-certificates/.gitkeep diff --git a/crates/giskard-server/static/app.css b/crates/giskard-server/static/app.css index ff69459..b0d3e75 100644 --- a/crates/giskard-server/static/app.css +++ b/crates/giskard-server/static/app.css @@ -661,6 +661,8 @@ .remove-project-dialog { width:min(520px,92vw); } .remove-project-dialog p { margin:0 0 10px; } .remove-project-path { padding:8px 10px; border:1px solid var(--border); border-radius:8px; background:var(--panel2); font-size:12px; word-break:break-all; } + .remove-thread-dialog { width:min(520px,92vw); } + .remove-thread-dialog p { margin:0 0 10px; } .code-dialog { width:min(1120px,94vw); height:min(860px,90vh); } .code-head { padding:14px 18px; border-bottom:1px solid var(--border); display:flex; gap:10px; align-items:flex-start; } .code-head-main { flex:1; min-width:0; } diff --git a/crates/giskard-server/static/app.js b/crates/giskard-server/static/app.js index a580620..e8d4159 100644 --- a/crates/giskard-server/static/app.js +++ b/crates/giskard-server/static/app.js @@ -67,6 +67,7 @@ const MAX_ATTACHMENT_BYTES = 25 * 1024 * 1024; const MAX_TOTAL_ATTACHMENT_BYTES = 25 * 1024 * 1024; const MAX_ATTACHMENT_NAME_BYTES = 255; const MAX_ATTACHMENT_MIME_BYTES = 127; +const THREAD_DELETE_TIMEOUT_MS = 30000; let state = { projectId:null, threadId:null, mode:"build", ws:null, wsStatus:"closed", wsConnectId:0, wsReconnectTimer:null, wsReconnectAttempt:0, wsStatusDetail:"WebSocket disconnected", @@ -95,7 +96,8 @@ let state = { subagentImports:new Map(), projectThreads:new Map(), threadIndex:new Map(), lastNotificationPromptNoticeAt:0, swRegistration:null, pendingAttachments:[], attachmentGeneration:0, pendingAttachmentOperations:new Map(), - collapsedProjects:new Set(loadCollapsedProjects()), pendingRemoveProject:null, projectDirs:{} + collapsedProjects:new Set(loadCollapsedProjects()), pendingRemoveProject:null, + pendingRemoveThread:null, removeThreadRequestSeq:0, projectDirs:{} }; let attachmentIngestQueue = Promise.resolve(); const activeAttachmentReaders = new Set(); @@ -114,17 +116,45 @@ const EFFORT_OPTIONS = [ ]; setInterval(updateRunningCommandDurations, 1000); -async function api(method, path, body) { +async function api(method, path, body, options) { const opts = { method, headers:{} }; + const timeoutMs = options && Number(options.timeoutMs) > 0 ? Number(options.timeoutMs) : 0; + let timeoutId = null; + let timedOut = false; + let timeoutPromise = null; if (body !== undefined) { opts.headers["Content-Type"]="application/json"; opts.body=JSON.stringify(body); } - const r = await fetch(path, opts); - if (!r.ok) { - const err = new Error((await r.text()) || `HTTP ${r.status}`); - err.status = r.status; - throw err; + if (timeoutMs && typeof AbortController === "function") { + const controller = new AbortController(); + opts.signal = controller.signal; + timeoutId = setTimeout(() => { + timedOut = true; + controller.abort(); + }, timeoutMs); + } else if (timeoutMs) { + timeoutPromise = new Promise((_, reject) => { + timeoutId = setTimeout(() => { + timedOut = true; + reject(new Error(`Request timed out after ${Math.round(timeoutMs / 1000)} seconds.`)); + }, timeoutMs); + }); + } + try { + const fetchPromise = fetch(path, opts); + if (timeoutPromise) fetchPromise.catch(() => {}); + const r = timeoutPromise ? await Promise.race([fetchPromise, timeoutPromise]) : await fetchPromise; + if (!r.ok) { + const err = new Error((await r.text()) || `HTTP ${r.status}`); + err.status = r.status; + throw err; + } + const ct = r.headers.get("content-type")||""; + return ct.includes("json") ? r.json() : r.text(); + } catch (e) { + if (timedOut) throw new Error(`Request timed out after ${Math.round(timeoutMs / 1000)} seconds.`); + throw e; + } finally { + if (timeoutId !== null) clearTimeout(timeoutId); } - const ct = r.headers.get("content-type")||""; - return ct.includes("json") ? r.json() : r.text(); } function apiFailureMessage(e) { const msg = e && e.message ? e.message : String(e); @@ -1386,9 +1416,87 @@ async function deleteThread(pid, tid, title) { const cascade = descendants.length ? `, its ${descendants.length} linked sub-agent thread${descendants.length === 1 ? "" : "s"}, and all corresponding Codex threads` : " and its corresponding Codex thread"; - if (!confirm(`Permanently delete thread "${title}"${cascade}? This cannot be undone.`)) return; + // Open the modal instead of a native confirm() so the user gets the same confirmation card + // style as project removal. The cascade description is mirrored into the dialog content. + openRemoveThreadModal(pid, tid, title, cascade); +} + +function openRemoveThreadModal(pid, tid, title, cascade) { + const opener = threadRowForId(tid)?.closest(".thread-row")?.querySelector(".thread-menu-btn") + || (document.activeElement instanceof HTMLElement ? document.activeElement : null); + closeDrawers(); + closeThreadMenus(); + state.pendingRemoveThread = { + pid, + tid, + title, + descendants: threadDescendantIds(pid, tid), + opener, + deleting:false, + requestSeq:0, + }; + setRemoveThreadDeleting(false); + $("removeThreadErr").textContent = ""; + $("removeThreadName").textContent = title || "this thread"; + $("removeThreadCascade").textContent = cascade || ""; + $("removeThreadModal").classList.add("open"); + $("removeThreadConfirm").focus(); +} + +function setRemoveThreadDeleting(deleting) { + const modal = $("removeThreadModal"); + $("removeThreadConfirm").disabled = deleting; + $("removeThreadCancel").disabled = deleting; + modal.setAttribute("aria-busy", deleting ? "true" : "false"); +} + +function removeThreadFallbackFocus(pending) { + const rowButton = pending && pending.tid + ? threadRowForId(pending.tid)?.closest(".thread-row")?.querySelector(".thread-menu-btn") + : null; + return rowButton + || (state.threadId && threadRowForId(state.threadId)?.closest(".thread-row")?.querySelector(".thread-menu-btn")) + || $("newProj") + || $("btnMenu"); +} + +function restoreRemoveThreadFocus(pending) { + const target = pending && pending.opener && pending.opener.isConnected + ? pending.opener + : removeThreadFallbackFocus(pending); + if (target && target.isConnected && typeof target.focus === "function") target.focus(); +} + +function closeRemoveThreadModal(options) { + const force = options && options.force; + const restoreFocus = !options || options.restoreFocus !== false; + const pending = state.pendingRemoveThread; + if (pending && pending.deleting && !force) return false; + $("removeThreadModal").classList.remove("open"); + setRemoveThreadDeleting(false); + state.pendingRemoveThread = null; + if (restoreFocus) restoreRemoveThreadFocus(pending); + return true; +} + +$("removeThreadCancel").onclick = closeRemoveThreadModal; +$("removeThreadModal").addEventListener("click", (e) => { + if (e.target === $("removeThreadModal")) closeRemoveThreadModal(); +}); + +$("removeThreadConfirm").onclick = async () => { + const pending = state.pendingRemoveThread; + if (!pending || !pending.pid || !pending.tid) return; + const { pid, tid, descendants } = pending; + pending.deleting = true; + pending.requestSeq = ++state.removeThreadRequestSeq; + const requestSeq = pending.requestSeq; + setRemoveThreadDeleting(true); + $("removeThreadErr").textContent = ""; try { - await api("DELETE", `/api/projects/${pid}/threads/${tid}`); + await api("DELETE", `/api/projects/${pid}/threads/${tid}`, undefined, { + timeoutMs: THREAD_DELETE_TIMEOUT_MS, + }); // The server cascades to descendants it discovered itself, which can include children this // client never listed. Decide from the refreshed authoritative list whether the active view // was deleted; keep the pre-request set as a fallback in case the sidebar reload fails @@ -1405,8 +1513,17 @@ async function deleteThread(pid, tid, title) { ) { clearThreadView(activeThread); } + pending.deleting = false; + closeRemoveThreadModal({ force:true }); } catch (e) { - notice("Delete thread failed: " + e.message, "error"); + if (state.pendingRemoveThread === pending && pending.requestSeq === requestSeq) { + $("removeThreadErr").textContent = "Delete thread failed: " + apiFailureMessage(e); + } + } finally { + if (state.pendingRemoveThread === pending && pending.requestSeq === requestSeq) { + pending.deleting = false; + setRemoveThreadDeleting(false); + } } } @@ -7455,7 +7572,8 @@ $("btnMenu").onclick = () => toggleDrawer("left"); $("backdrop").onclick = closeDrawers; document.addEventListener("keydown", (e) => { if (e.key!=="Escape") return; - if ($("codeOverlay").classList.contains("open")) closeCodeOverlay(); + if ($("removeThreadModal").classList.contains("open")) closeRemoveThreadModal(); + else if ($("codeOverlay").classList.contains("open")) closeCodeOverlay(); else { closeSettingsMenu(); closeModelPicker(); diff --git a/crates/giskard-server/static/index.html b/crates/giskard-server/static/index.html index 9463bf5..2a73e6a 100644 --- a/crates/giskard-server/static/index.html +++ b/crates/giskard-server/static/index.html @@ -226,9 +226,9 @@

New project

-
+ + + +
diff --git a/tests/e2e/.host-ca-certificates/.gitkeep b/tests/e2e/.host-ca-certificates/.gitkeep new file mode 100644 index 0000000..8b13789 --- /dev/null +++ b/tests/e2e/.host-ca-certificates/.gitkeep @@ -0,0 +1 @@ + diff --git a/tests/e2e/Dockerfile b/tests/e2e/Dockerfile index 22d6ff0..d51a73e 100644 --- a/tests/e2e/Dockerfile +++ b/tests/e2e/Dockerfile @@ -13,11 +13,15 @@ FROM rust:1-slim-bookworm AS builder WORKDIR /src # ring (via rustls) needs a C toolchain and perl to build. +COPY tests/e2e/.host-ca-certificates/ /usr/local/share/ca-certificates/giskard-host/ RUN apt-get update \ - && apt-get install -y --no-install-recommends build-essential pkg-config perl \ + && apt-get install -y --no-install-recommends build-essential ca-certificates pkg-config perl \ + && update-ca-certificates \ && rm -rf /var/lib/apt/lists/* COPY . . +ENV SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt +ENV CARGO_HTTP_CAINFO=/etc/ssl/certs/ca-certificates.crt RUN cargo build --release -p giskard-server --bin giskard-server-replay # ---- Stage 2: run Playwright ---- @@ -26,6 +30,11 @@ RUN cargo build --release -p giskard-server --bin giskard-server-replay FROM mcr.microsoft.com/playwright:v1.56.0-jammy WORKDIR /e2e +COPY tests/e2e/.host-ca-certificates/ /usr/local/share/ca-certificates/giskard-host/ +RUN update-ca-certificates +ENV SSL_CERT_FILE=/etc/ssl/certs/ca-certificates.crt +ENV NODE_EXTRA_CA_CERTS=/etc/ssl/certs/ca-certificates.crt + COPY --from=builder /src/target/release/giskard-server-replay /usr/local/bin/giskard-server-replay # Install JS deps first (better layer caching), then copy the tests. diff --git a/tests/e2e/README.md b/tests/e2e/README.md index 07e92f3..0e29dee 100644 --- a/tests/e2e/README.md +++ b/tests/e2e/README.md @@ -45,6 +45,11 @@ tests/e2e/run.sh --reporter=line The HTML report lands in `tests/e2e/playwright-report/` on your host. +If your local network intercepts TLS, put any additional host CA certificates as `.crt` files in +`tests/e2e/.host-ca-certificates/` before running the Docker-based tests. The directory is optional: +CI and ordinary local environments build with only the committed `.gitkeep`, while any local +certificates in that hidden directory are copied into the e2e image trust store and ignored by git. + ## Run it without Docker (optional, for UI development) If you already have Node and the matching Playwright browsers, you can iterate faster by pointing diff --git a/tests/e2e/tests/subagents.spec.ts b/tests/e2e/tests/subagents.spec.ts index c3fb0a4..0800229 100644 --- a/tests/e2e/tests/subagents.spec.ts +++ b/tests/e2e/tests/subagents.spec.ts @@ -99,14 +99,12 @@ test.describe("linked sub-agent threads", () => { const parentRowContainer = parentRow.locator("xpath=.."); await parentRowContainer.locator(".thread-menu-btn").click(); - const dialogPromise = page.waitForEvent("dialog"); - const deleteClick = parentRowContainer.locator(".thread-menu .danger").click(); - const dialog = await dialogPromise; - expect(dialog.message()).toContain("1 linked sub-agent thread"); - expect(dialog.message()).toContain("all corresponding Codex threads"); - expect(dialog.message()).toContain("cannot be undone"); - await dialog.accept(); - await deleteClick; + await parentRowContainer.locator(".thread-menu .danger").click(); + await expect(page.locator("#removeThreadModal")).toHaveClass(/open/); + await expect(page.locator("#removeThreadCascade")).toContainText("1 linked sub-agent thread"); + await expect(page.locator("#removeThreadCascade")).toContainText("all corresponding Codex threads"); + await expect(page.locator("#removeThreadModal")).toContainText("cannot be undone"); + await page.locator("#removeThreadConfirm").click(); await expect(parentRow).toHaveCount(0); const remainingIds = await page.evaluate(async (pid) => { @@ -314,11 +312,9 @@ test.describe("cross-project thread deletion", () => { // Delete project A's thread via its row menu while project B's thread is the active view. const otherRowContainer = otherRow.locator("xpath=.."); await otherRowContainer.locator(".thread-menu-btn").click(); - const dialogPromise = page.waitForEvent("dialog"); - const deleteClick = otherRowContainer.locator(".thread-menu .danger").click(); - const dialog = await dialogPromise; - await dialog.accept(); - await deleteClick; + await otherRowContainer.locator(".thread-menu .danger").click(); + await expect(page.locator("#removeThreadModal")).toHaveClass(/open/); + await page.locator("#removeThreadConfirm").click(); await expect(otherRow).toHaveCount(0); // The active view is unaffected: its selection persists (a non-scoped clear would null it @@ -332,3 +328,175 @@ test.describe("cross-project thread deletion", () => { } }); }); + +test.describe("delete-thread confirmation card", () => { + test.beforeEach(async ({ page }) => { + await login(page); + }); + + // Shared seeding helpers: create an isolated project + persisted thread server-side so the + // tests below drive only the modal without depending on the shared composer/WebSocket timing. + const createProject = (page: import("@playwright/test").Page, name: string): Promise => + page.evaluate(async (projectName) => { + const res = await fetch("/api/projects", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + name: projectName, + dir: "/tmp", + default_model: { provider: "replay", model: "replay-model" }, + }), + }); + if (!res.ok) { + throw new Error(`create project failed: ${res.status} ${await res.text()}`); + } + return (await res.json()).id as string; + }, name); + const startThread = (page: import("@playwright/test").Page, pid: string, text: string): Promise => + page.evaluate( + async ({ pid, text }) => { + const res = await fetch(`/api/projects/${pid}/threads/start`, { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ + text, + model_ref: { provider: "replay", model: "replay-model" }, + mode: "build", + permission_preset: "ask_first", + }), + }); + if (!res.ok) { + throw new Error(`start thread failed: ${res.status} ${await res.text()}`); + } + return (await res.json()).thread_id as string; + }, + { pid, text }, + ); + const deleteProject = (page: import("@playwright/test").Page, id: string): Promise => + page.evaluate(async (projectId) => { + await fetch(`/api/projects/${projectId}`, { method: "DELETE" }); + }, id); + // Open the delete-thread card for `tid` via its sidebar row menu. + async function openDeleteCard(page: import("@playwright/test").Page, tid: string) { + const row = page.locator(`.thread[data-tid="${tid}"]`); + const rowContainer = row.locator("xpath=.."); + const menuButton = rowContainer.locator(".thread-menu-btn"); + await menuButton.click(); + await rowContainer.locator(".thread-menu .danger").click(); + await expect(page.locator("#removeThreadModal")).toHaveClass(/open/); + await expect(page.locator("#removeThreadConfirm")).toBeFocused(); + return menuButton; + } + + test("Cancel dismisses the card without deleting", async ({ page }) => { + const projectId = await createProject(page, "Card cancel project"); + try { + const tid = await startThread(page, projectId, "survives cancel"); + await page.evaluate(() => + (window as unknown as { loadProjects: () => Promise }).loadProjects(), + ); + const row = page.locator(`.thread[data-tid="${tid}"]`); + await expect(row).toBeVisible(); + + const menuButton = await openDeleteCard(page, tid); + await page.locator("#removeThreadCancel").click(); + await expect(page.locator("#removeThreadModal")).not.toHaveClass(/open/); + await expect(menuButton).toBeFocused(); + // The thread is still listed: no DELETE was issued. + await expect(row).toBeVisible(); + const stillThere = await page.evaluate(async (pid) => { + const res = await fetch(`/api/projects/${pid}/threads`); + const body = await res.json(); + return body.threads.map((t: { id: string }) => t.id); + }, projectId); + expect(stillThere).toContain(tid); + } finally { + await deleteProject(page, projectId); + } + }); + + test("Escape and outside-click dismiss without deleting", async ({ page }) => { + const projectId = await createProject(page, "Card escape project"); + try { + const tid = await startThread(page, projectId, "survives escape"); + await page.evaluate(() => + (window as unknown as { loadProjects: () => Promise }).loadProjects(), + ); + const row = page.locator(`.thread[data-tid="${tid}"]`); + await expect(row).toBeVisible(); + + // Escape closes the card. + const escapeMenuButton = await openDeleteCard(page, tid); + await page.keyboard.press("Escape"); + await expect(page.locator("#removeThreadModal")).not.toHaveClass(/open/); + await expect(escapeMenuButton).toBeFocused(); + await expect(row).toBeVisible(); + + // Reopen and dismiss by clicking the overlay backdrop (outside the dialog). Click a corner + // of the full-screen overlay rather than its center, which would hit the dialog card. + const backdropMenuButton = await openDeleteCard(page, tid); + await page.locator("#removeThreadModal").click({ position: { x: 4, y: 4 } }); + await expect(page.locator("#removeThreadModal")).not.toHaveClass(/open/); + await expect(backdropMenuButton).toBeFocused(); + await expect(row).toBeVisible(); + } finally { + await deleteProject(page, projectId); + } + }); + + test("a failed DELETE surfaces an inline error and keeps the card open", async ({ page }) => { + const projectId = await createProject(page, "Card error project"); + try { + const tid = await startThread(page, projectId, "delete failure"); + await page.evaluate(() => + (window as unknown as { loadProjects: () => Promise }).loadProjects(), + ); + const row = page.locator(`.thread[data-tid="${tid}"]`); + await expect(row).toBeVisible(); + + let resolveDeleteStarted: (() => void) | null = null; + let releaseDelete: (() => void) | null = null; + const deleteStarted = new Promise((resolve) => { + resolveDeleteStarted = resolve; + }); + const deleteReleased = new Promise((resolve) => { + releaseDelete = resolve; + }); + + // Make the single DELETE for this thread fail with a 500 so the handler surfaces the inline + // error instead of closing the card. `times: 1` lets the route auto-unregister after it + // fires, so a later retry (or teardown) reaches the real endpoint. + await page.route(`**/api/projects/${projectId}/threads/${tid}`, async (route) => { + if (route.request().method() !== "DELETE") { + await route.continue(); + return; + } + resolveDeleteStarted?.(); + await deleteReleased; + await route.fulfill({ status: 500, body: "scripted delete failure" }); + }, { times: 1 }); + + await openDeleteCard(page, tid); + const confirm = page.locator("#removeThreadConfirm"); + const cancel = page.locator("#removeThreadCancel"); + await confirm.click(); + await deleteStarted; + await expect(confirm).toBeDisabled(); + await expect(cancel).toBeDisabled(); + await page.keyboard.press("Escape"); + await expect(page.locator("#removeThreadModal")).toHaveClass(/open/); + await page.locator("#removeThreadModal").click({ position: { x: 4, y: 4 } }); + await expect(page.locator("#removeThreadModal")).toHaveClass(/open/); + releaseDelete?.(); + // The card stays open with an inline error message, not a closed-and-toasted failure. + await expect(page.locator("#removeThreadErr")).toContainText("Delete thread failed"); + await expect(page.locator("#removeThreadModal")).toHaveClass(/open/); + // Confirm is re-enabled after the failure so the user can dismiss or retry. + await expect(confirm).not.toBeDisabled(); + // The DELETE was intercepted before reaching the backend, so the thread is still listed. + await expect(row).toBeVisible(); + } finally { + await deleteProject(page, projectId); + } + }); +});