From 8b9e7e6d59efebcf960cc47eba2587e26bb25564 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Sun, 3 May 2026 16:24:43 -0700 Subject: [PATCH 1/2] ci: port DELETE-verify pattern to remaining staging e2e workflows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to #2648 — same `>/dev/null || true` swallow-on-error pattern existed in: e2e-staging-canvas.yml (single-slug) e2e-staging-saas.yml (loop) e2e-staging-sanity.yml (loop) e2e-staging-external.yml (loop, was `>/dev/null 2>&1` variant) All four now capture the HTTP code, log a "[teardown] deleted $slug (HTTP $code)" line on success, and emit a workflow warning naming the slug + body excerpt on non-2xx. Loop bodies also tally + summarise total leaks at the end. Exit semantics unchanged: a single cleanup miss still doesn't fail-flag the test (sweep-stale-e2e-orgs is the safety net within ~45 min). The behavior change is purely surfacing — failures that were silent are now visible on the workflow run page. Pairs with #2648's tightened sweeper. Together: per-run cleanup failures are visible AND the safety net catches them quickly. Closes the per-workflow port noted as out-of-scope in #2648. See molecule-controlplane#420. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/e2e-staging-canvas.yml | 19 +++++++++++++++++-- .github/workflows/e2e-staging-external.yml | 20 ++++++++++++++++++-- .github/workflows/e2e-staging-saas.yml | 20 ++++++++++++++++++-- .github/workflows/e2e-staging-sanity.yml | 19 +++++++++++++++++-- 4 files changed, 70 insertions(+), 8 deletions(-) diff --git a/.github/workflows/e2e-staging-canvas.yml b/.github/workflows/e2e-staging-canvas.yml index c1620a20..6c59e72a 100644 --- a/.github/workflows/e2e-staging-canvas.yml +++ b/.github/workflows/e2e-staging-canvas.yml @@ -184,8 +184,23 @@ jobs: exit 0 fi echo "Deleting orphan tenant: $slug" - curl -sS -X DELETE "$MOLECULE_CP_URL/cp/admin/tenants/$slug" \ + # Verify HTTP 2xx instead of `>/dev/null || true` swallowing + # failures. A 5xx or timeout previously looked identical to + # success, leaving the tenant alive for up to ~45 min until + # sweep-stale-e2e-orgs caught it. Surface failures as + # workflow warnings naming the slug. Don't `exit 1` — a single + # cleanup miss shouldn't fail-flag the canvas test when the + # actual smoke check passed; the sweeper is the safety net. + # See molecule-controlplane#420. + code=$(curl -sS -o /tmp/canvas-cleanup.out -w "%{http_code}" \ + -X DELETE "$MOLECULE_CP_URL/cp/admin/tenants/$slug" \ -H "Authorization: Bearer $ADMIN_TOKEN" \ -H "Content-Type: application/json" \ - -d "{\"confirm\":\"$slug\"}" >/dev/null || true + -d "{\"confirm\":\"$slug\"}" \ + || echo "000") + if [ "$code" = "200" ] || [ "$code" = "204" ]; then + echo "[teardown] deleted $slug (HTTP $code)" + else + echo "::warning::canvas teardown for $slug returned HTTP $code — sweep-stale-e2e-orgs will catch it within ~45 min. Body: $(head -c 300 /tmp/canvas-cleanup.out 2>/dev/null)" + fi exit 0 diff --git a/.github/workflows/e2e-staging-external.yml b/.github/workflows/e2e-staging-external.yml index d1d8def7..12ac4577 100644 --- a/.github/workflows/e2e-staging-external.yml +++ b/.github/workflows/e2e-staging-external.yml @@ -153,12 +153,28 @@ jobs: if [ -n "$orgs" ]; then echo "Safety-net sweep: deleting leftover orgs:" echo "$orgs" + # Per-slug verified DELETE — see molecule-controlplane#420. + # `>/dev/null 2>&1` previously hid every failure; surface + # non-2xx as workflow warnings so the run page names what + # leaked. Sweeper catches the rest within ~45 min. + leaks=() for slug in $orgs; do - curl -sS -X DELETE "$MOLECULE_CP_URL/cp/admin/tenants/$slug" \ + code=$(curl -sS -o /tmp/external-cleanup.out -w "%{http_code}" \ + -X DELETE "$MOLECULE_CP_URL/cp/admin/tenants/$slug" \ -H "Authorization: Bearer $ADMIN_TOKEN" \ -H "Content-Type: application/json" \ - -d "{\"confirm\":\"$slug\"}" >/dev/null 2>&1 + -d "{\"confirm\":\"$slug\"}" \ + || echo "000") + if [ "$code" = "200" ] || [ "$code" = "204" ]; then + echo "[teardown] deleted $slug (HTTP $code)" + else + echo "::warning::external teardown for $slug returned HTTP $code — sweep-stale-e2e-orgs will catch it within ~45 min. Body: $(head -c 300 /tmp/external-cleanup.out 2>/dev/null)" + leaks+=("$slug") + fi done + if [ ${#leaks[@]} -gt 0 ]; then + echo "::warning::external teardown left ${#leaks[@]} leak(s): ${leaks[*]}" + fi else echo "Safety-net sweep: no leftover orgs to clean." fi diff --git a/.github/workflows/e2e-staging-saas.yml b/.github/workflows/e2e-staging-saas.yml index f055c491..2a7efe16 100644 --- a/.github/workflows/e2e-staging-saas.yml +++ b/.github/workflows/e2e-staging-saas.yml @@ -164,11 +164,27 @@ jobs: and o.get('instance_status') not in ('purged',)] print('\n'.join(candidates)) " 2>/dev/null) + # Per-slug verified DELETE (was `>/dev/null || true` — see + # molecule-controlplane#420). Surface non-2xx as a workflow + # warning naming the leaked slug; don't exit 1 (sweeper is + # the safety net within ~45 min). + leaks=() for slug in $orgs; do echo "Safety-net teardown: $slug" - curl -sS -X DELETE "$MOLECULE_CP_URL/cp/admin/tenants/$slug" \ + code=$(curl -sS -o /tmp/saas-cleanup.out -w "%{http_code}" \ + -X DELETE "$MOLECULE_CP_URL/cp/admin/tenants/$slug" \ -H "Authorization: Bearer $ADMIN_TOKEN" \ -H "Content-Type: application/json" \ - -d "{\"confirm\":\"$slug\"}" >/dev/null || true + -d "{\"confirm\":\"$slug\"}" \ + || echo "000") + if [ "$code" = "200" ] || [ "$code" = "204" ]; then + echo "[teardown] deleted $slug (HTTP $code)" + else + echo "::warning::saas teardown for $slug returned HTTP $code — sweep-stale-e2e-orgs will catch it within ~45 min. Body: $(head -c 300 /tmp/saas-cleanup.out 2>/dev/null)" + leaks+=("$slug") + fi done + if [ ${#leaks[@]} -gt 0 ]; then + echo "::warning::saas teardown left ${#leaks[@]} leak(s): ${leaks[*]}" + fi exit 0 diff --git a/.github/workflows/e2e-staging-sanity.yml b/.github/workflows/e2e-staging-sanity.yml index edfa5359..e98b38fe 100644 --- a/.github/workflows/e2e-staging-sanity.yml +++ b/.github/workflows/e2e-staging-sanity.yml @@ -143,10 +143,25 @@ jobs: and o.get('status') not in ('purged',)] print('\n'.join(candidates)) " 2>/dev/null) + # Per-slug verified DELETE — see molecule-controlplane#420. + # Failures surface as workflow warnings; the sweeper is the + # safety net within ~45 min. + leaks=() for slug in $orgs; do - curl -sS -X DELETE "$MOLECULE_CP_URL/cp/admin/tenants/$slug" \ + code=$(curl -sS -o /tmp/sanity-cleanup.out -w "%{http_code}" \ + -X DELETE "$MOLECULE_CP_URL/cp/admin/tenants/$slug" \ -H "Authorization: Bearer $ADMIN_TOKEN" \ -H "Content-Type: application/json" \ - -d "{\"confirm\":\"$slug\"}" >/dev/null || true + -d "{\"confirm\":\"$slug\"}" \ + || echo "000") + if [ "$code" = "200" ] || [ "$code" = "204" ]; then + echo "[teardown] deleted $slug (HTTP $code)" + else + echo "::warning::sanity teardown for $slug returned HTTP $code — sweep-stale-e2e-orgs will catch it within ~45 min. Body: $(head -c 300 /tmp/sanity-cleanup.out 2>/dev/null)" + leaks+=("$slug") + fi done + if [ ${#leaks[@]} -gt 0 ]; then + echo "::warning::sanity teardown left ${#leaks[@]} leak(s): ${leaks[*]}" + fi exit 0 From b1a1c8e4a9199cfaa9300c7c7c34036d763a4a71 Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Sun, 3 May 2026 16:31:23 -0700 Subject: [PATCH 2/2] canvas/BatchActionBar: wire Esc to clear selection (matches button title) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two small fixes for the batch-action toolbar: 1. The deselect button's title says "Clear selection (Escape)" — but pressing Escape did NOTHING. The title has been lying since the bar shipped. Now wired: window keydown handler calls clearSelection when Esc fires. Skipped while the confirm dialog is open (`pending !== null`) so the dialog's own Esc-cancels takes precedence, and skipped during a busy in-flight action so the user can't strand a partial-failure mid-flight. 2. focus-visible:ring-zinc-500/70 → focus-visible:ring-accent/50 on the deselect button. The hardcoded zinc broke the semantic- token pattern used by the other action buttons. Tests: two new vitest cases — Esc clears with selection, Esc no-op when empty (the bar isn't mounted at count===0 so the listener never registers). Full suite: 1222/1222. Co-Authored-By: Claude Opus 4.7 (1M context) --- canvas/src/components/BatchActionBar.tsx | 20 ++++++++++++++++++- .../__tests__/BatchActionBar.test.tsx | 20 +++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/canvas/src/components/BatchActionBar.tsx b/canvas/src/components/BatchActionBar.tsx index 004b3205..2a293631 100644 --- a/canvas/src/components/BatchActionBar.tsx +++ b/canvas/src/components/BatchActionBar.tsx @@ -30,6 +30,24 @@ export function BatchActionBar() { if (count === 0 && hasFailedBatch) setHasFailedBatch(false); }, [count, hasFailedBatch]); + // Esc clears selection — the deselect button title has been promising + // "(Escape)" since the bar shipped, but no handler was wired. Skip when + // the confirm dialog is open (`pending !== null`) so the dialog's own + // Esc-cancels takes precedence and we don't double-handle the keystroke. + // Also skip during a busy in-flight action so the user can't accidentally + // strand a partial-failure mid-flight. + useEffect(() => { + if (count === 0 || pending !== null || busy) return; + const onKey = (e: KeyboardEvent) => { + if (e.key === "Escape") { + e.stopPropagation(); + clearSelection(); + } + }; + window.addEventListener("keydown", onKey); + return () => window.removeEventListener("keydown", onKey); + }, [count, pending, busy, clearSelection]); + // Hide when nothing is selected. Hide for single-node selection UNLESS a // partial-failure left a survivor awaiting retry. if (count === 0) return null; @@ -129,7 +147,7 @@ export function BatchActionBar() { onClick={clearSelection} aria-label="Clear selection" title="Clear selection (Escape)" - className="p-1.5 rounded-lg text-[12px] text-ink-mid hover:text-ink hover:bg-surface-card/50 transition-colors disabled:opacity-50 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-zinc-500/70" + className="p-1.5 rounded-lg text-[12px] text-ink-mid hover:text-ink hover:bg-surface-card/50 transition-colors disabled:opacity-50 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-accent/50" > ✕ diff --git a/canvas/src/components/__tests__/BatchActionBar.test.tsx b/canvas/src/components/__tests__/BatchActionBar.test.tsx index 4dd9fef6..f72b7570 100644 --- a/canvas/src/components/__tests__/BatchActionBar.test.tsx +++ b/canvas/src/components/__tests__/BatchActionBar.test.tsx @@ -130,6 +130,26 @@ describe("BatchActionBar", () => { const toolbar = screen.getByRole("toolbar"); expect(toolbar.getAttribute("aria-label")).toBe("Batch workspace actions"); }); + + it("Esc clears the selection — matches the deselect button title", () => { + // The deselect button has been promising "Clear selection (Escape)" + // since the bar shipped, but no handler was wired. This pins the + // contract. + mockSelectedNodeIds = new Set(["ws-1", "ws-2"]); + render(); + fireEvent.keyDown(window, { key: "Escape" }); + expect(mockClearSelection).toHaveBeenCalled(); + }); + + it("Esc is a no-op when nothing is selected", () => { + mockSelectedNodeIds = new Set(); + render(); + fireEvent.keyDown(window, { key: "Escape" }); + // The early-return at count===0 prevents the bar from mounting at all, + // so the keydown listener never registers. clearSelection must NOT be + // called. + expect(mockClearSelection).not.toHaveBeenCalled(); + }); }); /**