From cdaeddd2fe69c005d3dfcf0a21aa17363f099ac0 Mon Sep 17 00:00:00 2001 From: Renn F Date: Sun, 28 Jun 2026 18:28:49 +0200 Subject: [PATCH] [F084] scope per-control disable to the in-flight mutation, not all MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FeatureFlagsCard disabled every switch while any one flag toggle was pending, and PlaybookReviewQueue disabled every row's Approve while any one approve was pending — so the operator couldn't act on an independent control during a slow round-trip. Gate the disable on the in-flight mutation's variables (matching key / id) so only the control being mutated locks; the others stay usable. The same-flag double-tap protection is preserved. --- .../__tests__/playbook-review-queue.test.tsx | 96 +++++++++++++++++++ .../dashboard/playbook-review-queue.tsx | 5 +- .../__tests__/feature-flags-card.test.tsx | 81 ++++++++++++++++ .../settings/feature-flags-card.tsx | 5 +- 4 files changed, 185 insertions(+), 2 deletions(-) create mode 100644 panel/src/components/dashboard/__tests__/playbook-review-queue.test.tsx create mode 100644 panel/src/components/settings/__tests__/feature-flags-card.test.tsx diff --git a/panel/src/components/dashboard/__tests__/playbook-review-queue.test.tsx b/panel/src/components/dashboard/__tests__/playbook-review-queue.test.tsx new file mode 100644 index 00000000..6cadec87 --- /dev/null +++ b/panel/src/components/dashboard/__tests__/playbook-review-queue.test.tsx @@ -0,0 +1,96 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import type { ReactNode } from "react"; +import type { Playbook } from "@/lib/api/playbooks"; + +const { resolveApproveRef } = vi.hoisted(() => ({ + resolveApproveRef: { current: null as null | ((v: unknown) => void) }, +})); + +const { listDrafts, approve, reject } = vi.hoisted(() => ({ + listDrafts: vi.fn( + async () => + [ + { + id: "pb-1", + title: "Recover a stuck claim lock", + slug: "recover-claim-lock", + problem: "an agent's claim TOCTOU wedges the task", + procedure: "1. ...", + tags: ["backend"], + team: "backend", + scope: "cell", + status: "draft", + }, + { + id: "pb-2", + title: "Rebase a behind-base branch", + slug: "rebase-behind-base", + problem: "the dev's branch fell behind master", + procedure: "1. ...", + tags: ["git"], + team: "backend", + scope: "cell", + status: "draft", + }, + ] as Playbook[], + ), + // Deferred so the test can freeze the approve mid-flight. + approve: vi.fn( + () => + new Promise((r) => { + resolveApproveRef.current = r as (v: unknown) => void; + }), + ), + reject: vi.fn(async () => ({})), +})); + +vi.mock("@/lib/api", () => ({ playbooksApi: { listDrafts, approve, reject } })); + +import { PlaybookReviewQueue } from "../playbook-review-queue"; + +function withQueryClient(ui: ReactNode) { + const client = new QueryClient({ + defaultOptions: { queries: { retry: false }, mutations: { retry: false } }, + }); + return {ui}; +} + +describe("PlaybookReviewQueue — per-row disable during an approve (F084)", () => { + beforeEach(() => { + listDrafts.mockClear(); + approve.mockClear(); + reject.mockClear(); + resolveApproveRef.current = null; + }); + afterEach(() => { + vi.clearAllMocks(); + }); + + it("disables only the playbook being approved, not every row's Approve", async () => { + render(withQueryClient()); + + const approveButtons = await screen.findAllByRole("button", { + name: "Approve", + }); + expect(approveButtons).toHaveLength(2); + expect(approveButtons[0]).not.toBeDisabled(); + expect(approveButtons[1]).not.toBeDisabled(); + + // Approve the first playbook — the mutation stays pending (deferred fn). + fireEvent.click(approveButtons[0]); + await waitFor(() => expect(approve).toHaveBeenCalledWith("pb-1")); + + // Row 1's Approve locks while its approve is in flight; row 2's Approve + // stays usable so the reviewer can act on an independent playbook at the + // same time. Before the fix every row shared `disabled={approveMutation.isPending}`. + await waitFor(() => expect(approveButtons[0]).toBeDisabled()); + expect(approveButtons[1]).not.toBeDisabled(); + + // Mutation resolves → row 1's Approve unlocks again. + resolveApproveRef.current?.(undefined); + await waitFor(() => expect(approveButtons[0]).not.toBeDisabled()); + expect(approveButtons[1]).not.toBeDisabled(); + }); +}); diff --git a/panel/src/components/dashboard/playbook-review-queue.tsx b/panel/src/components/dashboard/playbook-review-queue.tsx index bd7d4570..1ad87ec2 100644 --- a/panel/src/components/dashboard/playbook-review-queue.tsx +++ b/panel/src/components/dashboard/playbook-review-queue.tsx @@ -132,7 +132,10 @@ export function PlaybookReviewQueue({ className }: { className?: string }) {