diff --git a/e2e/playwright.config.ts b/e2e/playwright.config.ts index a6908a4..e2ec53a 100644 --- a/e2e/playwright.config.ts +++ b/e2e/playwright.config.ts @@ -2,8 +2,8 @@ import { defineConfig, devices } from '@playwright/test'; export default defineConfig({ testDir: './tests', - timeout: 60_000, - expect: { timeout: 30_000 }, + timeout: 10_000, + expect: { timeout: 10_000 }, reporter: 'list', use: { baseURL: 'http://localhost:8080', diff --git a/e2e/tests/ticket-flow.spec.ts b/e2e/tests/ticket-flow.spec.ts index 659f0ee..0297b4b 100644 --- a/e2e/tests/ticket-flow.spec.ts +++ b/e2e/tests/ticket-flow.spec.ts @@ -11,7 +11,7 @@ function ticketStatus(ticketItem: Locator): Locator { } async function waitForStatus(ticketItem: Locator, status: string): Promise { - await expect(ticketStatus(ticketItem)).toContainText(status, { timeout: 30_000 }); + await expect(ticketStatus(ticketItem)).toContainText(status, { timeout: 10_000 }); } test('full ticket lifecycle: add → investigate → implement → done → cleanup', async ({ page }) => { @@ -46,7 +46,7 @@ test('full ticket lifecycle: add → investigate → implement → done → clea await page.getByRole('button', { name: 'Approve' }).click(); // ── 5. Wait for implementation state (Accept button enabled) ──────────── - await expect(page.getByRole('button', { name: 'Accept' })).toBeEnabled({ timeout: 30_000 }); + await expect(page.getByRole('button', { name: 'Accept' })).toBeEnabled({ timeout: 10_000 }); // ── 6. Accept → transition to done ────────────────────────────────────── await page.getByRole('button', { name: 'Accept' }).click(); diff --git a/web/src/App.tsx b/web/src/App.tsx index c4178c1..20407e9 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -33,7 +33,16 @@ import { stateRunsFromDetails, ticketKey } from "./tickets"; -import type { AcceptedJob, DiscoveredTicket, ExecutionLog, Job, OptimisticTransition, ServerEvent, TicketDetails, TicketSummary } from "./types"; +import type { + AcceptedJob, + DiscoveredTicket, + ExecutionLog, + Job, + OptimisticTransition, + ServerEvent, + TicketDetails, + TicketSummary +} from "./types"; export function App() { const [tickets, setTickets] = useState([]); @@ -45,8 +54,8 @@ export function App() { const [questionAnswers, setQuestionAnswers] = useState>({}); const [generalFeedback, setGeneralFeedback] = useState(""); const [optimisticTransition, setOptimisticTransition] = useState(null); - const [activeJobId, setActiveJobId] = useState(""); - const [activeJob, setActiveJob] = useState(null); + const [pendingTicketKeys, setPendingTicketKeys] = useState([]); + const [jobFailuresByTicket, setJobFailuresByTicket] = useState>({}); const [loading, setLoading] = useState(false); const [artifactLoading, setArtifactLoading] = useState(false); const [error, setError] = useState(""); @@ -84,41 +93,205 @@ export function App() { () => (selectedSummary ? projectedTicketStatusLabel(selectedSummary, optimisticTransition) : ""), [optimisticTransition, selectedSummary] ); + const selectedTicketPending = selectedSummary ? pendingTicketKeys.includes(ticketKey(selectedSummary)) : false; + const selectedTicketDisabled = Boolean(selectedSummary && (selectedSummary.busy || selectedTicketPending)); + const selectedTicketFailure = selectedSummary ? jobFailuresByTicket[ticketKey(selectedSummary)] ?? null : null; const selectedSummaryRef = useRef(null); const selectedRunIdRef = useRef(""); - const activeJobIdRef = useRef(""); const showLogsModalRef = useRef(false); const fullRefreshScheduledRef = useRef(false); const prevLastRunIdRef = useRef(""); const handleServerEventRef = useRef<(evt: ServerEvent) => Promise>(async () => {}); const reconnectErrorMessage = "event stream connection lost; reconnecting"; + function setTicketPending(key: string, pending: boolean) { + setPendingTicketKeys((current) => { + if (pending) { + return current.includes(key) ? current : [...current, key]; + } + return current.filter((candidate) => candidate !== key); + }); + } + + function clearTicketFailure(key: string) { + setJobFailuresByTicket((current) => { + if (!(key in current)) { + return current; + } + const next = { ...current }; + delete next[key]; + return next; + }); + } + + function setTicketFailure(key: string, job: Job) { + setJobFailuresByTicket((current) => ({ ...current, [key]: job })); + } + + function optimisticTicketJob(accepted: AcceptedJob) { + if (!accepted.ticket_number) { + return; + } + const key = `${accepted.repo_id}::${accepted.ticket_number}`; + const createdAt = new Date().toISOString(); + setTicketPending(key, true); + clearTicketFailure(key); + setTickets((current) => current.map((ticket) => { + if (ticketKey(ticket) !== key) { + return ticket; + } + // SSE events may arrive before the 202 response — don't downgrade an already-progressed job back to "queued". + const existingJob = (ticket.jobs ?? []).find((job) => job.id === accepted.job_id); + const optimisticJob = existingJob ?? { + id: accepted.job_id, + action: accepted.action, + repo_id: accepted.repo_id, + repo_path: accepted.repo_path, + ticket_number: accepted.ticket_number, + status: "queued" as Job["status"], + created_at: createdAt + }; + const nextJobs = [ + optimisticJob, + ...(ticket.jobs ?? []).filter((job) => job.id !== accepted.job_id) + ]; + const isBusy = nextJobs.some((job) => job.status === "queued" || job.status === "running"); + return { + ...ticket, + busy: isBusy, + jobs: nextJobs + }; + })); + } + + async function fetchTicketFailure(key: string, evt: ServerEvent) { + if (!evt.job_id || !evt.repo_id || !evt.repo_path || !evt.ticket_number) { + return; + } + try { + const job = await getJob(evt.job_id); + setTicketFailure(key, job); + } catch { + setTicketFailure(key, { + id: evt.job_id, + action: evt.action ?? "", + repo_id: evt.repo_id, + repo_path: evt.repo_path, + ticket_number: evt.ticket_number, + status: "failed", + error: evt.error, + created_at: new Date().toISOString() + }); + } + } + + function makeSyntheticRunId(): string { + return `optimistic-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; + } + + function applyOptimisticTransition(next: Omit) { + const transition = { + ...next, + synthetic_run_id: makeSyntheticRunId() + }; + setOptimisticTransition(transition); + setSelectedRunId(transition.synthetic_run_id); + } + + function createMoveTransition(accepted: AcceptedJob, targetStateName: string): OptimisticTransition | null { + if (!selectedSummary || !details) { + return null; + } + return { + ticket_key: ticketKey(selectedSummary), + repo_path: selectedSummary.repo_path, + ticket_number: selectedSummary.ticket_number, + job_id: accepted.job_id, + target_state_name: targetStateName, + target_state_display_name: resolveStateDisplayName(details.workflow_states ?? [], targetStateName), + previous_selected_run_id: selectedRunIdRef.current, + previous_current_run_id: details.state.current_run_id ?? "", + kind: "move_to_state", + synthetic_run_id: "" + }; + } + + function createRerunTransition(accepted: AcceptedJob): OptimisticTransition | null { + if (!selectedSummary || !details) { + return null; + } + const currentStateName = details.state.current_state; + const fallbackDisplayName = currentRun?.state_display_name || currentStateName; + return { + ticket_key: ticketKey(selectedSummary), + repo_path: selectedSummary.repo_path, + ticket_number: selectedSummary.ticket_number, + job_id: accepted.job_id, + target_state_name: currentStateName, + target_state_display_name: resolveStateDisplayName(details.workflow_states ?? [], currentStateName, fallbackDisplayName), + previous_selected_run_id: selectedRunIdRef.current, + previous_current_run_id: details.state.current_run_id ?? "", + kind: "rerun", + synthetic_run_id: "" + }; + } + + function rollbackOptimisticTransition(jobId?: string) { + setOptimisticTransition((current) => { + if (!current || (jobId && current.job_id !== jobId)) { + return current; + } + if (selectedRunIdRef.current === current.synthetic_run_id) { + setSelectedRunId(current.previous_selected_run_id); + } + return null; + }); + } + + function clearOptimisticTransitionIfConfirmed(ticketDetails: TicketDetails) { + setOptimisticTransition((current) => { + if (!current) { + return current; + } + if (current.repo_path !== ticketDetails.repo_path || current.ticket_number !== ticketDetails.ticket_number) { + return current; + } + + const currentRunId = ticketDetails.state.current_run_id ?? ""; + const rerunConfirmed = current.kind === "rerun" && currentRunId !== "" && currentRunId !== current.previous_current_run_id; + const moveConfirmed = + current.kind === "move_to_state" && + ticketDetails.state.current_state === current.target_state_name && + currentRunId !== "" && + currentRunId !== current.previous_current_run_id; + const terminalConfirmed = + current.kind === "move_to_state" && + (ticketDetails.state.flow_status ?? "") === current.target_state_name; + + if (!rerunConfirmed && !moveConfirmed && !terminalConfirmed) { + return current; + } + + if (selectedRunIdRef.current === current.synthetic_run_id && moveConfirmed) { + setSelectedRunId(currentRunId); + } + + return null; + }); + } + async function handleServerEvent(evt: ServerEvent) { const selected = selectedSummaryRef.current; - const trackedJobID = activeJobIdRef.current; - if (evt.type === "job" && evt.job_id && trackedJobID && evt.job_id === trackedJobID) { + const evtTicketKey = evt.repo_id && evt.ticket_number ? `${evt.repo_id}::${evt.ticket_number}` : ""; + if (evt.type === "job" && evtTicketKey) { const status = evt.status ?? ""; + setTicketPending(evtTicketKey, false); if (status === "failed") { rollbackOptimisticTransition(evt.job_id); - try { - const job = await getJob(evt.job_id); - setActiveJob(job); - } catch (err) { - setError(err instanceof Error ? err.message : "failed to refresh job"); - } finally { - setActiveJobId(""); - } - } else if (status === "done") { - setActiveJob(null); - setActiveJobId(""); - } else if (status === "queued" || status === "running") { - setActiveJob((current) => { - if (!current) { - return current; - } - return { ...current, status }; - }); + await fetchTicketFailure(evtTicketKey, evt); + } else if (status === "queued" || status === "running" || status === "done") { + clearTicketFailure(evtTicketKey); } } @@ -134,16 +307,16 @@ export function App() { scheduleFullRefresh(); } + if (evt.type === "ticket_updated" && evtTicketKey && (evt.status ?? "") !== "running") { + setTicketPending(evtTicketKey, false); + } + if ( selected && evt.type === "ticket_updated" && evt.repo_path === selected.repo_path && evt.ticket_number === selected.ticket_number ) { - if ((evt.status ?? "") !== "running" && activeJobIdRef.current) { - activeJobIdRef.current = ""; - setActiveJobId(""); - } await refreshTicketDetails(selected.repo_path, selected.ticket_number, false); if (showLogsModalRef.current) { await refreshExecutionLogs(selected.repo_path, selected.ticket_number); @@ -155,10 +328,6 @@ export function App() { selectedSummaryRef.current = selectedSummary; }, [selectedSummary]); - useEffect(() => { - activeJobIdRef.current = activeJobId; - }, [activeJobId]); - useEffect(() => { selectedRunIdRef.current = selectedRunId; }, [selectedRunId]); @@ -198,16 +367,16 @@ export function App() { return; } const lastRunId = stateRuns[stateRuns.length - 1].id; - const currentRunId = details?.state.current_run_id; + const nextCurrentRunId = details?.state.current_run_id; const prevLastRunId = prevLastRunIdRef.current; prevLastRunIdRef.current = lastRunId; setSelectedRunId((current) => { if (!current || !stateRuns.some((run) => run.id === current)) { - return (currentRunId && stateRuns.some((run) => run.id === currentRunId)) ? currentRunId : lastRunId; + return (nextCurrentRunId && stateRuns.some((run) => run.id === nextCurrentRunId)) ? nextCurrentRunId : lastRunId; } if (current === prevLastRunId) { - return (currentRunId && stateRuns.some((run) => run.id === currentRunId)) ? currentRunId : lastRunId; + return (nextCurrentRunId && stateRuns.some((run) => run.id === nextCurrentRunId)) ? nextCurrentRunId : lastRunId; } return current; }); @@ -294,6 +463,13 @@ export function App() { setGeneralFeedback(""); }, [selectedSummary?.repo_path, selectedSummary?.ticket_number, currentRunId]); + useEffect(() => { + setPendingTicketKeys((current) => current.filter((key) => { + const ticket = tickets.find((candidate) => ticketKey(candidate) === key); + return ticket ? ticket.busy : false; + })); + }, [tickets]); + useEffect(() => { if (!showLogsModal || !selectedSummary) { return; @@ -391,8 +567,7 @@ export function App() { return; } void refreshTicketDetails(selectedSummary.repo_path, selectedSummary.ticket_number); - // `refreshTicketDetails` is a stable effect event; this effect should only - // react to selection changes. + // `refreshTicketDetails` is a stable effect event; this effect should only react to selection changes. // eslint-disable-next-line react-hooks/exhaustive-deps }, [selectedSummary]); @@ -411,9 +586,7 @@ export function App() { setError(""); try { const accepted = await fn(); - activeJobIdRef.current = accepted.job_id; - setActiveJobId(accepted.job_id); - setActiveJob(null); + optimisticTicketJob(accepted); return accepted; } catch (err) { setError(err instanceof Error ? err.message : "action failed"); @@ -491,101 +664,6 @@ export function App() { setAddTicketError(""); } - function makeSyntheticRunId(): string { - return `optimistic-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`; - } - - function applyOptimisticTransition(next: Omit) { - const transition = { - ...next, - synthetic_run_id: makeSyntheticRunId() - }; - setOptimisticTransition(transition); - setSelectedRunId(transition.synthetic_run_id); - } - - function createMoveTransition(accepted: AcceptedJob, targetStateName: string): OptimisticTransition | null { - if (!selectedSummary || !details) { - return null; - } - return { - ticket_key: ticketKey(selectedSummary), - repo_path: selectedSummary.repo_path, - ticket_number: selectedSummary.ticket_number, - job_id: accepted.job_id, - target_state_name: targetStateName, - target_state_display_name: resolveStateDisplayName(details.workflow_states ?? [], targetStateName), - previous_selected_run_id: selectedRunIdRef.current, - previous_current_run_id: details.state.current_run_id ?? "", - kind: "move_to_state", - synthetic_run_id: "" - }; - } - - function createRerunTransition(accepted: AcceptedJob): OptimisticTransition | null { - if (!selectedSummary || !details) { - return null; - } - const currentStateName = details.state.current_state; - const fallbackDisplayName = currentRun?.state_display_name || currentStateName; - return { - ticket_key: ticketKey(selectedSummary), - repo_path: selectedSummary.repo_path, - ticket_number: selectedSummary.ticket_number, - job_id: accepted.job_id, - target_state_name: currentStateName, - target_state_display_name: resolveStateDisplayName(details.workflow_states ?? [], currentStateName, fallbackDisplayName), - previous_selected_run_id: selectedRunIdRef.current, - previous_current_run_id: details.state.current_run_id ?? "", - kind: "rerun", - synthetic_run_id: "" - }; - } - - function rollbackOptimisticTransition(jobId?: string) { - setOptimisticTransition((current) => { - if (!current || (jobId && current.job_id !== jobId)) { - return current; - } - if (selectedRunIdRef.current === current.synthetic_run_id) { - setSelectedRunId(current.previous_selected_run_id); - } - return null; - }); - } - - function clearOptimisticTransitionIfConfirmed(ticketDetails: TicketDetails) { - setOptimisticTransition((current) => { - if (!current) { - return current; - } - if (current.repo_path !== ticketDetails.repo_path || current.ticket_number !== ticketDetails.ticket_number) { - return current; - } - - const currentRunId = ticketDetails.state.current_run_id ?? ""; - const rerunConfirmed = current.kind === "rerun" && currentRunId !== "" && currentRunId !== current.previous_current_run_id; - const moveConfirmed = - current.kind === "move_to_state" && - ticketDetails.state.current_state === current.target_state_name && - currentRunId !== "" && - currentRunId !== current.previous_current_run_id; - const terminalConfirmed = - current.kind === "move_to_state" && - (ticketDetails.state.flow_status ?? "") === current.target_state_name; - - if (!rerunConfirmed && !moveConfirmed && !terminalConfirmed) { - return current; - } - - if (selectedRunIdRef.current === current.synthetic_run_id && moveConfirmed) { - setSelectedRunId(currentRunId); - } - - return null; - }); - } - function submitFeedback() { if (!selectedSummary || !feedbackAction) { return; @@ -727,12 +805,6 @@ export function App() { {error ?
{error}
: null} - {activeJob ? ( -
- Job `{activeJob.id}`: {activeJob.action} ({activeJob.status}) - {activeJob.error ? ` - ${activeJob.error}` : ""} -
- ) : null}
; generalFeedback: string; - isRunning: boolean; + actionsDisabled: boolean; + feedbackDisabled: boolean; + cleanupDisabled: boolean; + moveDisabled: boolean; + rerunDisabled: boolean; + jobFailure: Job | null; onSelectRun: (runId: string) => void; onQuestionAnswerChange: (index: number, value: string) => void; onGeneralFeedbackChange: (value: string) => void; @@ -40,7 +45,12 @@ export function TicketDetailPanel({ openQuestions, questionAnswers, generalFeedback, - isRunning, + actionsDisabled, + feedbackDisabled, + cleanupDisabled, + moveDisabled, + rerunDisabled, + jobFailure, onSelectRun, onQuestionAnswerChange, onGeneralFeedbackChange, @@ -82,7 +92,7 @@ export function TicketDetailPanel({
{actionButtons.map((action) => ( - ))} @@ -100,13 +110,18 @@ export function TicketDetailPanel({ workflowStates={details?.workflow_states ?? []} currentStateName={details?.state.current_state} rerunLabel={selectedSummary.status === "failed" ? "Retry" : "Rerun"} - rerunDisabled={isRunning || selectedSummary.busy} - cleanupDisabled={selectedSummary.busy} - moveDisabled={isRunning || selectedSummary.busy} + rerunDisabled={rerunDisabled} + cleanupDisabled={cleanupDisabled} + moveDisabled={moveDisabled} />
+ {jobFailure?.error ? ( +
+ Job `{jobFailure.id}`: {jobFailure.action} ({jobFailure.status}) - {jobFailure.error} +
+ ) : null} {details?.state.last_error ? (
{details.state.last_error}
) : null} @@ -186,7 +201,7 @@ export function TicketDetailPanel({ /> )}
- +
) : null} diff --git a/web/src/__tests__/App.test.tsx b/web/src/__tests__/App.test.tsx index 30edb18..39609ec 100644 --- a/web/src/__tests__/App.test.tsx +++ b/web/src/__tests__/App.test.tsx @@ -24,19 +24,41 @@ vi.mock("../api", () => apiMocks); let eventHandler: ((evt: Record) => void) | null = null; -function makeDetails(overrides: Record = {}) { +function makeSummary(overrides: Record = {}) { return { repo_id: "repo1", repo_path: "/repo1", ticket_number: "GH-5", - workflow_states: [{ name: "implementation", display_name: "Implementation" }], + title: "Structured feedback", + status: "waiting", + busy: false, + approved: false, + updated_at: "2024-01-01T00:00:00Z", + ...overrides + }; +} + +function makeDetails(ticketNumber = "GH-5", title = "Structured feedback", overrides: Record = {}) { + return { + repo_id: "repo1", + repo_path: "/repo1", + ticket_number: ticketNumber, + ticket: { + title, + url: `https://github.com/Neokil/AutoPR/issues/${ticketNumber.replace("GH-", "")}` + }, + workflow_states: [ + { name: "implementation", display_name: "Implementation" }, + { name: "cancelled", display_name: "Cancelled" }, + { name: "review", display_name: "Review" } + ], available_actions: [ { label: "Provide Feedback", type: "provide_feedback" }, { label: "Approve", type: "move_to_state", target: "implementation" }, { label: "Cancel", type: "move_to_state", target: "cancelled" } ], state: { - ticket_number: "GH-5", + ticket_number: ticketNumber, current_state: "investigation", current_run_id: "run-2", flow_status: "waiting", @@ -72,22 +94,13 @@ beforeEach(() => { eventHandler = onEvent; return { close: vi.fn() }; }); - apiMocks.listTickets.mockResolvedValue([ - { - repo_id: "repo1", - repo_path: "/repo1", - ticket_number: "GH-5", - title: "Structured feedback", - status: "waiting", - busy: false, - approved: false, - updated_at: "2024-01-01T00:00:00Z" - } - ]); + apiMocks.listTickets.mockResolvedValue([makeSummary()]); apiMocks.listRepositories.mockResolvedValue(["/repo1"]); apiMocks.getHealth.mockResolvedValue({ discover_tickets_configured: false }); apiMocks.getExecutionLogs.mockResolvedValue([]); - apiMocks.getTicket.mockResolvedValue(makeDetails()); + apiMocks.getTicket.mockImplementation(async (_repoPath: string, ticketNumber: string) => + makeDetails(ticketNumber, ticketNumber === "GH-6" ? "Second ticket" : "Structured feedback") + ); apiMocks.getArtifact.mockImplementation(async (_repoPath: string, _ticket: string, artifactRef: string) => { if (artifactRef.includes("run-1")) { return `## Open Questions @@ -103,7 +116,22 @@ beforeEach(() => { apiMocks.applyAction.mockResolvedValue({ status: "accepted", job_id: "job-1", - action: "apply_action", + action: "action", + repo_id: "repo1", + repo_path: "/repo1", + ticket_number: "GH-5" + }); + apiMocks.cleanupDone.mockResolvedValue({ + status: "accepted", + job_id: "job-cleanup-done", + action: "cleanup_done", + repo_id: "repo1", + repo_path: "/repo1" + }); + apiMocks.cleanupAll.mockResolvedValue({ + status: "accepted", + job_id: "job-cleanup-all", + action: "cleanup_all", repo_id: "repo1", repo_path: "/repo1" }); @@ -112,17 +140,26 @@ beforeEach(() => { job_id: "job-2", action: "move_to_state", repo_id: "repo1", - repo_path: "/repo1" + repo_path: "/repo1", + ticket_number: "GH-5" + }); + apiMocks.runTicket.mockResolvedValue({ + status: "accepted", + job_id: "job-3", + action: "run", + repo_id: "repo1", + repo_path: "/repo1", + ticket_number: "GH-5" }); apiMocks.getJob.mockResolvedValue({ id: "job-1", - action: "apply_action", + action: "action", repo_id: "repo1", repo_path: "/repo1", ticket_number: "GH-5", status: "failed", error: "job failed", - created_at: "2024-01-01T00:00:00Z" + created_at: "2024-01-02T00:00:00Z" }); }); @@ -179,7 +216,6 @@ describe("App", () => { }); expect(screen.getByRole("button", { name: "Implementation" })).toBeInTheDocument(); expect(screen.getByText("GH-5 - Structured feedback (Implementation)")).toBeInTheDocument(); - expect(screen.getByRole("button", { name: /GH-5 Structured feedback .*Implementation .*\/repo1/ })).toBeInTheDocument(); }); it("shows an optimistic rerun immediately after structured feedback submit", async () => { @@ -224,13 +260,99 @@ describe("App", () => { ticket_number: "GH-5", job_id: "job-1", status: "failed", - action: "apply_action" + action: "action", + error: "job failed" }); await waitFor(() => { expect(screen.queryByText("Running Implementation")).not.toBeInTheDocument(); }); expect(screen.getAllByText("Current question?").length).toBeGreaterThan(0); - expect(screen.getByText(/Job `job-1`: apply_action \(failed\)/)).toBeInTheDocument(); + expect(screen.getByText(/Job `job-1`: action \(failed\) - job failed/)).toBeInTheDocument(); + }); + + it("keeps another ticket actionable while one ticket has an optimistic pending job", async () => { + apiMocks.listTickets.mockResolvedValue([ + makeSummary(), + makeSummary({ ticket_number: "GH-6", title: "Second ticket" }) + ]); + + render(); + + expect(await screen.findByRole("button", { name: "Approve" })).toBeEnabled(); + + fireEvent.click(screen.getByRole("button", { name: "Approve" })); + + await waitFor(() => { + expect(apiMocks.applyAction).toHaveBeenCalledWith("/repo1", "GH-5", "Approve"); + }); + expect(screen.getAllByLabelText("Worker running")).toHaveLength(1); + + fireEvent.click(screen.getByRole("button", { name: /GH-6/i })); + + await waitFor(() => { + expect(screen.getByRole("button", { name: "Approve" })).toBeEnabled(); + }); + }); + + it("disables same-ticket controls immediately after queueing an action", async () => { + render(); + + expect(await screen.findByRole("button", { name: "Approve" })).toBeEnabled(); + fireEvent.click(screen.getByRole("button", { name: "Approve" })); + + await waitFor(() => { + expect(apiMocks.applyAction).toHaveBeenCalledWith("/repo1", "GH-5", "Approve"); + }); + expect(screen.getByRole("button", { name: "Approve" })).toBeDisabled(); + expect(screen.getAllByLabelText("Worker running")).toHaveLength(1); + }); + + it("keeps repo-wide cleanup actions available while a ticket job is pending", async () => { + render(); + + expect(await screen.findByRole("button", { name: "Approve" })).toBeEnabled(); + fireEvent.click(screen.getByRole("button", { name: "Approve" })); + + await waitFor(() => { + expect(apiMocks.applyAction).toHaveBeenCalledWith("/repo1", "GH-5", "Approve"); + }); + expect(screen.getByRole("button", { name: "Cleanup Done" })).toBeEnabled(); + expect(screen.getByRole("button", { name: "Cleanup All" })).toBeEnabled(); + }); + + it("shows failed jobs only for the affected ticket", async () => { + apiMocks.listTickets.mockResolvedValue([ + makeSummary(), + makeSummary({ ticket_number: "GH-6", title: "Second ticket" }) + ]); + + render(); + + expect(await screen.findByRole("button", { name: "Approve" })).toBeEnabled(); + fireEvent.click(screen.getByRole("button", { name: "Approve" })); + + await waitFor(() => { + expect(apiMocks.applyAction).toHaveBeenCalledWith("/repo1", "GH-5", "Approve"); + }); + + eventHandler?.({ + type: "job", + repo_id: "repo1", + repo_path: "/repo1", + ticket_number: "GH-5", + job_id: "job-1", + action: "action", + status: "failed", + error: "job failed" + }); + + expect(await screen.findByText(/Job `job-1`: action \(failed\) - job failed/)).toBeInTheDocument(); + + fireEvent.click(screen.getByRole("button", { name: /GH-6/i })); + + await waitFor(() => { + expect(screen.queryByText(/Job `job-1`: action \(failed\) - job failed/)).not.toBeInTheDocument(); + }); }); }); diff --git a/web/src/__tests__/TicketDetailPanel.test.tsx b/web/src/__tests__/TicketDetailPanel.test.tsx index fb64a30..4754707 100644 --- a/web/src/__tests__/TicketDetailPanel.test.tsx +++ b/web/src/__tests__/TicketDetailPanel.test.tsx @@ -1,7 +1,7 @@ import { fireEvent, render, screen } from "@testing-library/react"; import { describe, expect, it, vi } from "vitest"; import { TicketDetailPanel } from "../TicketDetailPanel"; -import type { TicketDetails, TicketSummary } from "../types"; +import type { DisplayStateRun, Job, TicketDetails, TicketSummary } from "../types"; function makeSummary(partial: Partial = {}): TicketSummary { return { @@ -41,6 +41,20 @@ function makeDetails(): TicketDetails { }; } +function makeJob(partial: Partial = {}): Job { + return { + id: "job-1", + action: "run", + repo_id: "repo1", + repo_path: "/repo1", + ticket_number: "GH-5", + status: "failed", + error: "job failed", + created_at: "2024-01-01T00:00:00Z", + ...partial + }; +} + describe("TicketDetailPanel", () => { it("renders one textarea per open question plus a general feedback textarea", () => { render( @@ -56,7 +70,12 @@ describe("TicketDetailPanel", () => { openQuestions={["What should happen first?", "What should happen second?"]} questionAnswers={{ "0": "First answer" }} generalFeedback="General feedback" - isRunning={false} + actionsDisabled={false} + feedbackDisabled={false} + cleanupDisabled={false} + moveDisabled={false} + rerunDisabled={false} + jobFailure={null} onSelectRun={vi.fn()} onQuestionAnswerChange={vi.fn()} onGeneralFeedbackChange={vi.fn()} @@ -92,7 +111,12 @@ describe("TicketDetailPanel", () => { openQuestions={[]} questionAnswers={{}} generalFeedback="" - isRunning={false} + actionsDisabled={false} + feedbackDisabled={false} + cleanupDisabled={false} + moveDisabled={false} + rerunDisabled={false} + jobFailure={null} onSelectRun={vi.fn()} onQuestionAnswerChange={vi.fn()} onGeneralFeedbackChange={onGeneralFeedbackChange} @@ -112,29 +136,35 @@ describe("TicketDetailPanel", () => { }); it("renders a running placeholder for a synthetic optimistic run", () => { + const runs: DisplayStateRun[] = [ + ...(makeDetails().state.state_history ?? []), + { + id: "optimistic-run", + state_name: "implementation", + state_display_name: "Implementation", + started_at: "2024-01-03T00:00:00Z", + synthetic: true + } + ]; + render( { expect(screen.getByText("Waiting for server confirmation.")).toBeInTheDocument(); expect(screen.queryByText("No artifact path available.")).not.toBeInTheDocument(); }); + + it("shows a ticket-scoped job failure banner", () => { + render( + + ); + + expect(screen.getByText(/Job `job-1`: run \(failed\) - job failed/)).toBeInTheDocument(); + }); });