From eebe54262c075d85da740ef364731bc7623011e3 Mon Sep 17 00:00:00 2001 From: Simon Schulte Date: Tue, 9 Jun 2026 11:26:51 +0200 Subject: [PATCH] Show optimistic workflow state transitions --- internal/api/generated.go | 5 +- internal/server/http.go | 5 +- internal/server/strict_api.go | 5 +- internal/server/transport.go | 5 +- openapi/openapi.yaml | 1 + web/src/App.tsx | 225 ++++++++++++++++--- web/src/StateTimeline.tsx | 4 +- web/src/TicketDetailPanel.tsx | 43 ++-- web/src/TicketList.tsx | 46 ++-- web/src/__tests__/App.test.tsx | 148 +++++++++--- web/src/__tests__/TicketDetailPanel.test.tsx | 43 ++++ web/src/__tests__/tickets.test.ts | 71 +++++- web/src/generated/api.ts | 1 + web/src/styles.css | 11 + web/src/tickets.ts | 66 +++++- web/src/types.ts | 18 ++ 16 files changed, 587 insertions(+), 110 deletions(-) diff --git a/internal/api/generated.go b/internal/api/generated.go index 5921bd9..c98a9da 100644 --- a/internal/api/generated.go +++ b/internal/api/generated.go @@ -100,8 +100,9 @@ type ActionAcceptedResponse struct { // ActionInfo defines model for ActionInfo. type ActionInfo struct { - Label string `json:"label"` - Type string `json:"type"` + Label string `json:"label"` + Target *string `json:"target,omitempty"` + Type string `json:"type"` } // ActionRequest defines model for ActionRequest. diff --git a/internal/server/http.go b/internal/server/http.go index 36b88fd..66188cc 100644 --- a/internal/server/http.go +++ b/internal/server/http.go @@ -15,8 +15,9 @@ import ( ) type actionInfo struct { - Label string `json:"label"` - Type string `json:"type"` + Label string `json:"label"` + Type string `json:"type"` + Target string `json:"target,omitempty"` } type workflowStateInfo struct { diff --git a/internal/server/strict_api.go b/internal/server/strict_api.go index fba379a..a29add0 100644 --- a/internal/server/strict_api.go +++ b/internal/server/strict_api.go @@ -183,8 +183,9 @@ func (s *server) GetTicket(ctx context.Context, request api.GetTicketRequestObje if stateCfg, ok := wflow.StateByName(ticketState.CurrentState); ok { for _, a := range stateCfg.Actions { availableActions = append(availableActions, actionInfo{ - Label: a.Label, - Type: string(a.Type), + Label: a.Label, + Type: string(a.Type), + Target: a.Target, }) } } diff --git a/internal/server/transport.go b/internal/server/transport.go index d310466..ab608d5 100644 --- a/internal/server/transport.go +++ b/internal/server/transport.go @@ -121,8 +121,9 @@ func toActionResponses(actions []actionInfo) []api.ActionInfo { out := make([]api.ActionInfo, 0, len(actions)) for _, action := range actions { out = append(out, api.ActionInfo{ - Label: action.Label, - Type: action.Type, + Label: action.Label, + Type: action.Type, + Target: stringPtr(action.Target), }) } diff --git a/openapi/openapi.yaml b/openapi/openapi.yaml index 1dc7220..e1e10af 100644 --- a/openapi/openapi.yaml +++ b/openapi/openapi.yaml @@ -386,6 +386,7 @@ components: properties: label: { type: string } type: { type: string } + target: { type: string } WorkflowStateInfo: type: object required: [name] diff --git a/web/src/App.tsx b/web/src/App.tsx index 8d1c30a..c4178c1 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo, useRef, useState } from "react"; +import { useEffect, useEffectEvent, useMemo, useRef, useState } from "react"; import { applyAction, cleanupAll, @@ -27,11 +27,13 @@ import { getFeedbackAction, knownRepoPaths, pendingTicketKey, + projectedTicketStatusLabel, + resolveStateDisplayName, selectTicketKey, stateRunsFromDetails, ticketKey } from "./tickets"; -import type { DiscoveredTicket, ExecutionLog, Job, ServerEvent, TicketDetails, TicketSummary } from "./types"; +import type { AcceptedJob, DiscoveredTicket, ExecutionLog, Job, OptimisticTransition, ServerEvent, TicketDetails, TicketSummary } from "./types"; export function App() { const [tickets, setTickets] = useState([]); @@ -42,6 +44,7 @@ export function App() { const [currentFeedbackArtifactContent, setCurrentFeedbackArtifactContent] = useState(""); const [questionAnswers, setQuestionAnswers] = useState>({}); const [generalFeedback, setGeneralFeedback] = useState(""); + const [optimisticTransition, setOptimisticTransition] = useState(null); const [activeJobId, setActiveJobId] = useState(""); const [activeJob, setActiveJob] = useState(null); const [loading, setLoading] = useState(false); @@ -66,7 +69,7 @@ export function App() { const selectedSummary = useMemo(() => tickets.find((ticket) => ticketKey(ticket) === selectedKey) ?? null, [tickets, selectedKey]); const availableRepoPaths = useMemo(() => knownRepoPaths(repositoryOptions, tickets), [repositoryOptions, tickets]); - const stateRuns = useMemo(() => stateRunsFromDetails(details), [details]); + const stateRuns = useMemo(() => stateRunsFromDetails(details, optimisticTransition), [details, optimisticTransition]); const feedbackAction = useMemo(() => getFeedbackAction(details, selectedSummary), [details, selectedSummary]); const currentRunId = details?.state.current_run_id ?? ""; const currentRun = useMemo( @@ -77,8 +80,13 @@ export function App() { () => extractOpenQuestions(currentFeedbackArtifactContent), [currentFeedbackArtifactContent] ); + const selectedStatusLabel = useMemo( + () => (selectedSummary ? projectedTicketStatusLabel(selectedSummary, optimisticTransition) : ""), + [optimisticTransition, selectedSummary] + ); const selectedSummaryRef = useRef(null); + const selectedRunIdRef = useRef(""); const activeJobIdRef = useRef(""); const showLogsModalRef = useRef(false); const fullRefreshScheduledRef = useRef(false); @@ -92,6 +100,7 @@ export function App() { if (evt.type === "job" && evt.job_id && trackedJobID && evt.job_id === trackedJobID) { const status = evt.status ?? ""; if (status === "failed") { + rollbackOptimisticTransition(evt.job_id); try { const job = await getJob(evt.job_id); setActiveJob(job); @@ -150,6 +159,10 @@ export function App() { activeJobIdRef.current = activeJobId; }, [activeJobId]); + useEffect(() => { + selectedRunIdRef.current = selectedRunId; + }, [selectedRunId]); + useEffect(() => { showLogsModalRef.current = showLogsModal; }, [showLogsModal]); @@ -178,23 +191,6 @@ export function App() { return () => stream.close(); }, []); - useEffect(() => { - if (!selectedSummary) { - setDetails(null); - setSelectedRunId(""); - setSelectedArtifactContent(""); - setCurrentFeedbackArtifactContent(""); - setQuestionAnswers({}); - setGeneralFeedback(""); - setArtifactLoading(false); - setShowLogsModal(false); - setExecutionLogs([]); - setExecutionLogsLoading(false); - return; - } - void refreshTicketDetails(selectedSummary.repo_path, selectedSummary.ticket_number); - }, [selectedSummary]); - useEffect(() => { if (stateRuns.length === 0) { prevLastRunIdRef.current = ""; @@ -228,6 +224,11 @@ export function App() { setArtifactLoading(false); return; } + if (selectedRun.synthetic) { + setSelectedArtifactContent(""); + setArtifactLoading(false); + return; + } const artifactRef = selectedRun.artifact_ref || selectedRun.log_ref; if (!artifactRef) { setSelectedArtifactContent(""); @@ -346,7 +347,7 @@ export function App() { } } - async function refreshTicketDetails(repoPath: string, ticket: string, showLoader = true) { + const refreshTicketDetails = useEffectEvent(async (repoPath: string, ticket: string, showLoader = true) => { if (showLoader) { setLoading(true); } @@ -354,6 +355,7 @@ export function App() { try { const ticketDetails = await getTicket(repoPath, ticket); setDetails(ticketDetails); + clearOptimisticTransitionIfConfirmed(ticketDetails); } catch (err) { setError(err instanceof Error ? err.message : "failed to load ticket details"); setDetails(null); @@ -363,16 +365,36 @@ export function App() { setLoading(false); } } - } + }); - async function refreshExecutionLogs(repoPath: string, ticketNumber: string) { + const refreshExecutionLogs = useEffectEvent(async (repoPath: string, ticketNumber: string) => { try { const logs = await getExecutionLogs(repoPath, ticketNumber); setExecutionLogs(logs); } catch (err) { setError(err instanceof Error ? err.message : "failed to refresh execution logs"); } - } + }); + + useEffect(() => { + if (!selectedSummary) { + setDetails(null); + setSelectedRunId(""); + setSelectedArtifactContent(""); + setCurrentFeedbackArtifactContent(""); + setQuestionAnswers({}); + setGeneralFeedback(""); + setArtifactLoading(false); + setShowLogsModal(false); + setExecutionLogs([]); + setExecutionLogsLoading(false); + return; + } + void refreshTicketDetails(selectedSummary.repo_path, selectedSummary.ticket_number); + // `refreshTicketDetails` is a stable effect event; this effect should only + // react to selection changes. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [selectedSummary]); function scheduleFullRefresh() { if (fullRefreshScheduledRef.current) { @@ -385,17 +407,17 @@ export function App() { }, 250); } - async function queueAction(fn: () => Promise<{ job_id: string }>): Promise { + async function queueAction(fn: () => Promise): Promise { setError(""); try { const accepted = await fn(); activeJobIdRef.current = accepted.job_id; setActiveJobId(accepted.job_id); setActiveJob(null); - return true; + return accepted; } catch (err) { setError(err instanceof Error ? err.message : "action failed"); - return false; + return null; } } @@ -428,9 +450,9 @@ export function App() { } setPendingAddedTickets((current) => [...current, pendingKey]); - const ok = await queueAction(() => runTicket(repoPath, ticketNumber, baseBranch)); + const accepted = await queueAction(() => runTicket(repoPath, ticketNumber, baseBranch)); setPendingAddedTickets((current) => current.filter((key) => key !== pendingKey)); - if (ok) { + if (accepted) { closeAddTicketDialog(); scheduleFullRefresh(); } @@ -469,6 +491,101 @@ 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; @@ -484,8 +601,12 @@ export function App() { feedbackAction.label, message ) - ).then((ok) => { - if (ok) { + ).then((accepted) => { + if (accepted) { + const transition = createRerunTransition(accepted); + if (transition) { + applyOptimisticTransition(transition); + } setQuestionAnswers({}); setGeneralFeedback(""); } @@ -497,17 +618,34 @@ export function App() { } function applyNamedAction(label: string) { - if (!selectedSummary) { + if (!selectedSummary || !details) { return; } - void queueAction(() => applyAction(selectedSummary.repo_path, selectedSummary.ticket_number, label)); + const action = details.available_actions.find((candidate) => candidate.label === label); + void queueAction(() => applyAction(selectedSummary.repo_path, selectedSummary.ticket_number, label)).then((accepted) => { + if (!accepted || !action?.target) { + return; + } + const transition = createMoveTransition(accepted, action.target); + if (transition) { + applyOptimisticTransition(transition); + } + }); } function rerunSelectedTicket() { if (!selectedSummary) { return; } - void queueAction(() => runTicket(selectedSummary.repo_path, selectedSummary.ticket_number)); + void queueAction(() => runTicket(selectedSummary.repo_path, selectedSummary.ticket_number)).then((accepted) => { + if (!accepted) { + return; + } + const transition = createRerunTransition(accepted); + if (transition) { + applyOptimisticTransition(transition); + } + }); } function cleanupSelectedTicket() { @@ -521,7 +659,15 @@ export function App() { if (!selectedSummary) { return; } - void queueAction(() => moveToState(selectedSummary.repo_path, selectedSummary.ticket_number, target)); + void queueAction(() => moveToState(selectedSummary.repo_path, selectedSummary.ticket_number, target)).then((accepted) => { + if (!accepted) { + return; + } + const transition = createMoveTransition(accepted, target); + if (transition) { + applyOptimisticTransition(transition); + } + }); } function openDiscoverModal() { @@ -589,7 +735,13 @@ export function App() { ) : null}
- + void; }; diff --git a/web/src/TicketDetailPanel.tsx b/web/src/TicketDetailPanel.tsx index 13842e6..8f4e756 100644 --- a/web/src/TicketDetailPanel.tsx +++ b/web/src/TicketDetailPanel.tsx @@ -2,15 +2,16 @@ import { MarkdownView } from "./MarkdownView"; import { StateTimeline } from "./StateTimeline"; import { TicketMenu } from "./TicketMenu"; import { getNonFeedbackActions, runDisplayLabel, ticketTitle, ticketURL } from "./tickets"; -import type { ActionInfo, StateRun, TicketDetails, TicketSummary } from "./types"; +import type { ActionInfo, DisplayStateRun, TicketDetails, TicketSummary } from "./types"; type TicketDetailPanelProps = { selectedSummary: TicketSummary | null; details: TicketDetails | null; - stateRuns: StateRun[]; + stateRuns: DisplayStateRun[]; selectedRunId: string; selectedArtifactContent: string; artifactLoading: boolean; + statusLabel: string; feedbackAction?: ActionInfo; openQuestions: string[]; questionAnswers: Record; @@ -34,6 +35,7 @@ export function TicketDetailPanel({ selectedRunId, selectedArtifactContent, artifactLoading, + statusLabel, feedbackAction, openQuestions, questionAnswers, @@ -61,6 +63,7 @@ export function TicketDetailPanel({ const actionButtons = getNonFeedbackActions(details, selectedSummary); const url = ticketURL(details); const title = ticketTitle(details, selectedSummary); + const selectedRunLabel = selectedRun ? runDisplayLabel(selectedRun, stateRuns) : ""; return (
@@ -68,11 +71,11 @@ export function TicketDetailPanel({

{url ? ( - {selectedSummary.ticket_number} - {title} ({selectedSummary.status}) + {selectedSummary.ticket_number} - {title} ({statusLabel}) ) : ( <> - {selectedSummary.ticket_number} - {title} ({selectedSummary.status}) + {selectedSummary.ticket_number} - {title} ({statusLabel}) )}

@@ -114,18 +117,30 @@ export function TicketDetailPanel({ {selectedRun ? (
-

{runDisplayLabel(selectedRun, stateRuns)}

+

{selectedRunLabel}

{new Date(selectedRun.started_at).toLocaleString()}
-

{selectedRun.artifact_ref || selectedRun.log_ref || "No artifact path available."}

- {artifactLoading ?

Loading artifact...

: null} - + {selectedRun.synthetic ? ( +
+
+
+

Waiting for server confirmation.

+
+ ) : ( + <> +

{selectedRun.artifact_ref || selectedRun.log_ref || "No artifact path available."}

+ {artifactLoading ?

Loading artifact...

: null} + + + )}
) : (

No workflow runs available yet.

diff --git a/web/src/TicketList.tsx b/web/src/TicketList.tsx index aa5a8ec..a6d0307 100644 --- a/web/src/TicketList.tsx +++ b/web/src/TicketList.tsx @@ -1,5 +1,5 @@ -import type { Job, TicketSummary } from "./types"; -import { ticketKey } from "./tickets"; +import type { Job, OptimisticTransition, TicketSummary } from "./types"; +import { projectedTicketBusy, projectedTicketStatusLabel, ticketKey } from "./tickets"; function summarizeJobAction(action: string): string { switch (action) { @@ -18,6 +18,7 @@ function summarizeJobAction(action: string): string { type TicketListProps = { tickets: TicketSummary[]; + optimisticTransition: OptimisticTransition | null; selectedKey: string; onSelectTicket: (key: string) => void; onAddTicket: () => void; @@ -31,30 +32,35 @@ function renderJobChip(job: Job) { ); } -export function TicketList({ tickets, selectedKey, onSelectTicket, onAddTicket }: TicketListProps) { +export function TicketList({ tickets, optimisticTransition, selectedKey, onSelectTicket, onAddTicket }: TicketListProps) { return (

Tickets (All Repos)

    - {tickets.map((ticket) => ( -
  • - -
    • - ))} + {tickets.map((ticket) => { + const busy = projectedTicketBusy(ticket, optimisticTransition); + const statusLabel = projectedTicketStatusLabel(ticket, optimisticTransition); + + return ( +
  • + +
    • + ); + })}
diff --git a/web/src/__tests__/App.test.tsx b/web/src/__tests__/App.test.tsx index 1d260dd..30edb18 100644 --- a/web/src/__tests__/App.test.tsx +++ b/web/src/__tests__/App.test.tsx @@ -22,30 +22,19 @@ const apiMocks = vi.hoisted(() => ({ vi.mock("../api", () => apiMocks); -beforeEach(() => { - vi.clearAllMocks(); - apiMocks.connectEvents.mockReturnValue({ 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.listRepositories.mockResolvedValue(["/repo1"]); - apiMocks.getHealth.mockResolvedValue({ discover_tickets_configured: false }); - apiMocks.getExecutionLogs.mockResolvedValue([]); - apiMocks.getTicket.mockResolvedValue({ +let eventHandler: ((evt: Record) => void) | null = null; + +function makeDetails(overrides: Record = {}) { + return { repo_id: "repo1", repo_path: "/repo1", ticket_number: "GH-5", - workflow_states: [], - available_actions: [{ label: "Provide Feedback", type: "provide_feedback" }], + workflow_states: [{ name: "implementation", display_name: "Implementation" }], + 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", current_state: "investigation", @@ -71,8 +60,34 @@ beforeEach(() => { started_at: "2024-01-02T00:00:00Z" } ] - } + }, + ...overrides + }; +} + +beforeEach(() => { + vi.clearAllMocks(); + eventHandler = null; + apiMocks.connectEvents.mockImplementation((onEvent: (evt: Record) => void) => { + 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.listRepositories.mockResolvedValue(["/repo1"]); + apiMocks.getHealth.mockResolvedValue({ discover_tickets_configured: false }); + apiMocks.getExecutionLogs.mockResolvedValue([]); + apiMocks.getTicket.mockResolvedValue(makeDetails()); apiMocks.getArtifact.mockImplementation(async (_repoPath: string, _ticket: string, artifactRef: string) => { if (artifactRef.includes("run-1")) { return `## Open Questions @@ -92,19 +107,36 @@ beforeEach(() => { repo_id: "repo1", repo_path: "/repo1" }); + apiMocks.moveToState.mockResolvedValue({ + status: "accepted", + job_id: "job-2", + action: "move_to_state", + repo_id: "repo1", + repo_path: "/repo1" + }); + apiMocks.getJob.mockResolvedValue({ + id: "job-1", + action: "apply_action", + repo_id: "repo1", + repo_path: "/repo1", + ticket_number: "GH-5", + status: "failed", + error: "job failed", + created_at: "2024-01-01T00:00:00Z" + }); }); -describe("App structured feedback", () => { +describe("App", () => { it("submits structured feedback from the current run even when an older run is selected", async () => { render(); expect(await screen.findByPlaceholderText("Answer open question 1")).toBeInTheDocument(); - expect(screen.getByText("Current question?")).toBeInTheDocument(); + expect(screen.getAllByText("Current question?").length).toBeGreaterThan(0); fireEvent.click(screen.getByRole("button", { name: "Investigation 1" })); await waitFor(() => { - expect(screen.getByText("Current question?")).toBeInTheDocument(); + expect(screen.getAllByText("Current question?").length).toBeGreaterThan(0); }); const answerField = screen.getByPlaceholderText("Answer open question 1"); @@ -135,4 +167,70 @@ describe("App structured feedback", () => { expect.stringContaining("## Additional Feedback") ); }); + + it("shows an optimistic upcoming state immediately after approve", async () => { + render(); + + expect(await screen.findByRole("button", { name: "Approve" })).toBeInTheDocument(); + fireEvent.click(screen.getByRole("button", { name: "Approve" })); + + await waitFor(() => { + expect(screen.getByText("Running Implementation")).toBeInTheDocument(); + }); + 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 () => { + render(); + + expect(await screen.findByPlaceholderText("Answer open question 1")).toBeInTheDocument(); + fireEvent.change(screen.getByPlaceholderText("Answer open question 1"), { target: { value: "Answer" } }); + fireEvent.click(screen.getByRole("button", { name: "Provide Feedback" })); + + await waitFor(() => { + expect(screen.getByText("Running Investigation 3")).toBeInTheDocument(); + }); + }); + + it("applies the same optimistic behavior to overflow move-to-state", async () => { + render(); + + expect(await screen.findByRole("button", { name: "☰" })).toBeInTheDocument(); + fireEvent.click(screen.getByRole("button", { name: "☰" })); + fireEvent.click(screen.getByRole("menuitem", { name: "Implementation" })); + + await waitFor(() => { + expect(apiMocks.moveToState).toHaveBeenCalledWith("/repo1", "GH-5", "implementation"); + }); + expect(screen.getByText("Running Implementation")).toBeInTheDocument(); + }); + + it("rolls back the optimistic run when the tracked job fails", async () => { + render(); + + expect(await screen.findByRole("button", { name: "Approve" })).toBeInTheDocument(); + fireEvent.click(screen.getByRole("button", { name: "Approve" })); + + await waitFor(() => { + expect(screen.getByText("Running Implementation")).toBeInTheDocument(); + }); + + eventHandler?.({ + type: "job", + repo_id: "repo1", + repo_path: "/repo1", + ticket_number: "GH-5", + job_id: "job-1", + status: "failed", + action: "apply_action" + }); + + 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(); + }); }); diff --git a/web/src/__tests__/TicketDetailPanel.test.tsx b/web/src/__tests__/TicketDetailPanel.test.tsx index 0d0a020..fb64a30 100644 --- a/web/src/__tests__/TicketDetailPanel.test.tsx +++ b/web/src/__tests__/TicketDetailPanel.test.tsx @@ -51,6 +51,7 @@ describe("TicketDetailPanel", () => { selectedRunId="run-2" selectedArtifactContent="artifact" artifactLoading={false} + statusLabel="waiting" feedbackAction={{ label: "Provide Feedback", type: "provide_feedback" }} openQuestions={["What should happen first?", "What should happen second?"]} questionAnswers={{ "0": "First answer" }} @@ -86,6 +87,7 @@ describe("TicketDetailPanel", () => { selectedRunId="run-2" selectedArtifactContent="artifact" artifactLoading={false} + statusLabel="waiting" feedbackAction={{ label: "Provide Feedback", type: "provide_feedback" }} openQuestions={[]} questionAnswers={{}} @@ -108,4 +110,45 @@ describe("TicketDetailPanel", () => { expect(onGeneralFeedbackChange).toHaveBeenCalledWith("General note"); expect(screen.getAllByRole("textbox")).toHaveLength(1); }); + + it("renders a running placeholder for a synthetic optimistic run", () => { + render( + + ); + + expect(screen.getByText("Running Implementation")).toBeInTheDocument(); + expect(screen.getByText("Waiting for server confirmation.")).toBeInTheDocument(); + expect(screen.queryByText("No artifact path available.")).not.toBeInTheDocument(); + }); }); diff --git a/web/src/__tests__/tickets.test.ts b/web/src/__tests__/tickets.test.ts index 094d1b7..1cd77e6 100644 --- a/web/src/__tests__/tickets.test.ts +++ b/web/src/__tests__/tickets.test.ts @@ -1,11 +1,15 @@ import { describe, it, expect } from "vitest"; -import type { TicketDetails, TicketSummary, StateRun, ServerEvent } from "../types"; +import type { OptimisticTransition, TicketDetails, TicketSummary, StateRun, ServerEvent } from "../types"; import { runDisplayLabel, selectTicketKey, applyTicketEvent, getFeedbackAction, getNonFeedbackActions, + projectedTicketBusy, + projectedTicketStatusLabel, + resolveStateDisplayName, + stateRunsFromDetails, } from "../tickets"; function makeSummary(partial: Partial = {}): TicketSummary { @@ -35,7 +39,7 @@ function makeDetails(actions: TicketDetails["available_actions"]): TicketDetails repo_id: "repo1", repo_path: "/repo1", ticket_number: "1", - workflow_states: [], + workflow_states: [{ name: "implementation", display_name: "Implementation" }], available_actions: actions, state: { ticket_number: "1", @@ -49,6 +53,22 @@ function makeDetails(actions: TicketDetails["available_actions"]): TicketDetails }; } +function makeOptimistic(partial: Partial = {}): OptimisticTransition { + return { + ticket_key: "repo1::1", + repo_path: "/repo1", + ticket_number: "1", + job_id: "job-1", + synthetic_run_id: "optimistic-run", + target_state_name: "implementation", + target_state_display_name: "Implementation", + previous_selected_run_id: "run-1", + previous_current_run_id: "run-1", + kind: "move_to_state", + ...partial + }; +} + // ── runDisplayLabel ──────────────────────────────────────────────────────── // The duplicate-indexing behaviour is non-obvious: when the same state is // visited more than once, pills get a 1-based suffix so the user can tell them @@ -119,6 +139,53 @@ describe("getNonFeedbackActions", () => { }); }); +describe("stateRunsFromDetails", () => { + it("appends an optimistic move-to-state run", () => { + const details = makeDetails([{ label: "Approve", type: "move_to_state", target: "implementation" }]); + details.state.state_history = [makeRun({ id: "run-1", state_display_name: "Investigation" })]; + + const runs = stateRunsFromDetails(details, makeOptimistic()); + + expect(runs).toHaveLength(2); + expect(runs[1]).toMatchObject({ + id: "optimistic-run", + state_name: "implementation", + state_display_name: "Implementation", + synthetic: true + }); + }); + + it("does not append an optimistic rerun once the server confirms a new current run", () => { + const details = makeDetails([{ label: "Provide Feedback", type: "provide_feedback" }]); + details.state.current_run_id = "run-2"; + details.state.state_history = [ + makeRun({ id: "run-1", state_display_name: "Investigation" }), + makeRun({ id: "run-2", state_display_name: "Investigation" }) + ]; + + const runs = stateRunsFromDetails(details, makeOptimistic({ + kind: "rerun", + target_state_name: "investigate", + target_state_display_name: "Investigation" + })); + + expect(runs).toHaveLength(2); + expect(runs.some((run) => run.id === "optimistic-run")).toBe(false); + }); +}); + +describe("projected ticket helpers", () => { + it("replaces the displayed status label for the optimistic ticket and marks it busy", () => { + const ticket = makeSummary(); + expect(projectedTicketStatusLabel(ticket, makeOptimistic())).toBe("Implementation"); + expect(projectedTicketBusy(ticket, makeOptimistic())).toBe(true); + }); + + it("falls back to terminal display labels", () => { + expect(resolveStateDisplayName([], "done")).toBe("Done"); + }); +}); + // ── applyTicketEvent ─────────────────────────────────────────────────────── // This is the heart of the real-time UI: SSE events mutate the ticket list // without a full server round-trip. diff --git a/web/src/generated/api.ts b/web/src/generated/api.ts index 1e106d4..deaed66 100644 --- a/web/src/generated/api.ts +++ b/web/src/generated/api.ts @@ -294,6 +294,7 @@ export interface components { ActionInfo: { label: string; type: string; + target?: string; }; WorkflowStateInfo: { name: string; diff --git a/web/src/styles.css b/web/src/styles.css index 30e8691..c620018 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -346,6 +346,17 @@ button:disabled { padding: 12px; } +.timeline-running-placeholder { + display: grid; + gap: 8px; +} + +.timeline-running-header { + display: inline-flex; + align-items: center; + gap: 8px; +} + .timeline-content-header { display: flex; justify-content: space-between; diff --git a/web/src/tickets.ts b/web/src/tickets.ts index b9ddee6..6f82105 100644 --- a/web/src/tickets.ts +++ b/web/src/tickets.ts @@ -1,4 +1,4 @@ -import type { ActionInfo, Job, ServerEvent, StateRun, TicketDetails, TicketSummary } from "./types"; +import type { ActionInfo, DisplayStateRun, Job, OptimisticTransition, ServerEvent, StateRun, TicketDetails, TicketSummary, WorkflowStateInfo } from "./types"; function asRecord(value: unknown): Record | null { if (!value || typeof value !== "object" || Array.isArray(value)) { @@ -35,8 +35,54 @@ export function knownRepoPaths(repositoryOptions: string[], tickets: TicketSumma return paths; } -export function stateRunsFromDetails(details: TicketDetails | null): StateRun[] { - return details?.state.state_history ?? []; +const terminalStateLabels: Record = { + done: "Done", + cancelled: "Cancelled", + failed: "Failed" +}; + +export function resolveStateDisplayName(workflowStates: WorkflowStateInfo[], stateName: string, fallback = ""): string { + const workflowState = workflowStates.find((state) => state.name === stateName); + if (workflowState?.display_name) { + return workflowState.display_name; + } + if (terminalStateLabels[stateName]) { + return terminalStateLabels[stateName]; + } + return fallback || stateName; +} + +export function stateRunsFromDetails(details: TicketDetails | null, optimistic: OptimisticTransition | null = null): DisplayStateRun[] { + const runs = details?.state.state_history ?? []; + if (!details || !optimistic) { + return runs; + } + if (details.repo_path !== optimistic.repo_path || details.ticket_number !== optimistic.ticket_number) { + return runs; + } + if (optimistic.kind === "rerun" && details.state.current_run_id && details.state.current_run_id !== optimistic.previous_current_run_id) { + return runs; + } + if ( + optimistic.kind === "move_to_state" && + details.state.current_run_id && + details.state.current_run_id !== optimistic.previous_current_run_id && + details.state.current_state === optimistic.target_state_name + ) { + return runs; + } + + return [ + ...runs, + { + id: optimistic.synthetic_run_id, + state_name: optimistic.target_state_name, + state_display_name: optimistic.target_state_display_name, + started_at: new Date().toISOString(), + synthetic: true, + synthetic_status: "running" + } + ]; } export function getFeedbackAction(details: TicketDetails | null, selectedSummary: TicketSummary | null): ActionInfo | undefined { @@ -72,6 +118,20 @@ export function runDisplayLabel(run: StateRun, runs: StateRun[]): string { return `${base} ${index + 1}`; } +export function projectedTicketStatusLabel(ticket: TicketSummary, optimistic: OptimisticTransition | null): string { + if (optimistic && optimistic.ticket_key === ticketKey(ticket)) { + return optimistic.target_state_display_name; + } + return ticket.status; +} + +export function projectedTicketBusy(ticket: TicketSummary, optimistic: OptimisticTransition | null): boolean { + if (optimistic && optimistic.ticket_key === ticketKey(ticket)) { + return true; + } + return ticket.busy; +} + export function selectTicketKey(current: string, tickets: TicketSummary[]): string { if (tickets.length === 0) { return ""; diff --git a/web/src/types.ts b/web/src/types.ts index 2845a1b..042c083 100644 --- a/web/src/types.ts +++ b/web/src/types.ts @@ -15,3 +15,21 @@ export type ServerEvent = components["schemas"]["ServerEvent"]; export type RepositoryListResponse = components["schemas"]["RepositoryListResponse"]; export type DiscoveredTicket = components["schemas"]["DiscoveredTicket"]; export type HealthResponse = components["schemas"]["HealthResponse"]; + +export type DisplayStateRun = StateRun & { + synthetic?: boolean; + synthetic_status?: "queued" | "running"; +}; + +export type OptimisticTransition = { + ticket_key: string; + repo_path: string; + ticket_number: string; + job_id: string; + synthetic_run_id: string; + target_state_name: string; + target_state_display_name: string; + previous_selected_run_id: string; + previous_current_run_id: string; + kind: "move_to_state" | "rerun"; +};