mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 17:45:53 +01:00
fix(ui): prevent duplicate recovery retries
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
7f679f2850
commit
c3f5b73ac6
4 files changed
+76
-27
No files matched your search
@@ -455,7 +455,7 @@ interface IssueChatThreadProps {
|
||||
successfulRunHandoff?: SuccessfulRunHandoffState | null;
|
||||
scheduledRetry?: IssueScheduledRetry | null;
|
||||
recoveryAction?: IssueRecoveryAction | null;
|
||||
onResolveRecoveryAction?: (outcome: RecoveryResolveOutcome) => void;
|
||||
onResolveRecoveryAction?: (outcome: RecoveryResolveOutcome) => Promise<void> | void;
|
||||
recoveryActionPending?: boolean;
|
||||
onReissueIsolatedRecoveryAction?: (request: RecoveryReissueRequest) => void;
|
||||
reissueIsolatedRecoveryActionPending?: boolean;
|
||||
|
||||
@@ -196,17 +196,13 @@ describe("TaskChatThread recovery actions", () => {
|
||||
expect(disclosure.getAttribute("aria-expanded")).toBe("false");
|
||||
expect(retry.textContent).toBe("Try again");
|
||||
|
||||
flushSync(() => retry.click());
|
||||
expect(onResolveRecoveryAction).toHaveBeenLastCalledWith("todo");
|
||||
expect(disclosure.getAttribute("aria-expanded")).toBe("false");
|
||||
|
||||
flushSync(() => disclosure.click());
|
||||
expect(disclosure.getAttribute("aria-expanded")).toBe("true");
|
||||
expect(container.querySelector('[data-testid="task-chat-system-notice-details"]')).not.toBeNull();
|
||||
expect(container.querySelector('[data-testid="task-chat-recovery-try-again"]')).not.toBeNull();
|
||||
|
||||
flushSync(() => retry.click());
|
||||
expect(onResolveRecoveryAction).toHaveBeenCalledTimes(2);
|
||||
expect(onResolveRecoveryAction).toHaveBeenCalledTimes(1);
|
||||
expect(onResolveRecoveryAction).toHaveBeenLastCalledWith("todo");
|
||||
});
|
||||
|
||||
@@ -256,6 +252,31 @@ describe("TaskChatThread recovery actions", () => {
|
||||
flushSync(() => retry.click());
|
||||
expect(onResolveRecoveryAction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("submits only once across rapid clicks before pending state renders", () => {
|
||||
const onResolveRecoveryAction = vi.fn(() => new Promise<void>(() => {}));
|
||||
render(
|
||||
<TaskChatThread
|
||||
comments={[recoveryComment]}
|
||||
onAdd={async () => {}}
|
||||
issueId="issue-1"
|
||||
issueStatus="blocked"
|
||||
recoveryAction={recoveryAction}
|
||||
onResolveRecoveryAction={onResolveRecoveryAction}
|
||||
/>,
|
||||
);
|
||||
|
||||
const retry = container.querySelector<HTMLButtonElement>(
|
||||
'[data-testid="task-chat-recovery-try-again"]',
|
||||
)!;
|
||||
flushSync(() => {
|
||||
retry.click();
|
||||
retry.click();
|
||||
});
|
||||
|
||||
expect(onResolveRecoveryAction).toHaveBeenCalledTimes(1);
|
||||
expect(retry.disabled).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("TaskChatThread composer alignment (PAP-498)", () => {
|
||||
|
||||
@@ -101,6 +101,8 @@ export type TaskChatThreadProps = ComponentProps<typeof IssueChatThread>;
|
||||
* folded row. flag-OFF remains byte-for-byte IssueChatThread.
|
||||
*/
|
||||
export function TaskChatThread(props: TaskChatThreadProps) {
|
||||
const recoveryRetryInFlightRef = useRef(false);
|
||||
const [recoveryRetryInFlight, setRecoveryRetryInFlight] = useState(false);
|
||||
const {
|
||||
comments,
|
||||
interactions,
|
||||
@@ -626,6 +628,22 @@ export function TaskChatThread(props: TaskChatThreadProps) {
|
||||
[interruptingQueuedRunId, onInterruptQueued],
|
||||
);
|
||||
|
||||
const handleRecoveryTryAgain = useCallback(() => {
|
||||
if (!onResolveRecoveryAction || recoveryRetryInFlightRef.current) return;
|
||||
recoveryRetryInFlightRef.current = true;
|
||||
setRecoveryRetryInFlight(true);
|
||||
void (async () => {
|
||||
try {
|
||||
await onResolveRecoveryAction("todo");
|
||||
} catch {
|
||||
// The mutation owns user-visible error reporting.
|
||||
} finally {
|
||||
recoveryRetryInFlightRef.current = false;
|
||||
setRecoveryRetryInFlight(false);
|
||||
}
|
||||
})();
|
||||
}, [onResolveRecoveryAction]);
|
||||
|
||||
const renderSystemNoticeAction = useCallback(
|
||||
(item: TaskChatMessageItem) => {
|
||||
if (
|
||||
@@ -645,14 +663,20 @@ export function TaskChatThread(props: TaskChatThreadProps) {
|
||||
variant="outline"
|
||||
size="xs"
|
||||
data-testid="task-chat-recovery-try-again"
|
||||
disabled={recoveryActionPending}
|
||||
onClick={() => onResolveRecoveryAction("todo")}
|
||||
disabled={recoveryActionPending || recoveryRetryInFlight}
|
||||
onClick={handleRecoveryTryAgain}
|
||||
>
|
||||
Try again
|
||||
</Button>
|
||||
);
|
||||
},
|
||||
[onResolveRecoveryAction, recoveryAction, recoveryActionPending],
|
||||
[
|
||||
handleRecoveryTryAgain,
|
||||
onResolveRecoveryAction,
|
||||
recoveryAction,
|
||||
recoveryActionPending,
|
||||
recoveryRetryInFlight,
|
||||
],
|
||||
);
|
||||
|
||||
const renderInteraction = useCallback(
|
||||
|
||||
@@ -960,7 +960,7 @@ type IssueDetailChatTabProps = {
|
||||
successfulRunHandoff: Issue["successfulRunHandoff"] | null;
|
||||
scheduledRetry: Issue["scheduledRetry"] | null;
|
||||
recoveryAction: Issue["activeRecoveryAction"];
|
||||
onResolveRecoveryAction?: (outcome: import("../components/IssueRecoveryActionCard").RecoveryResolveOutcome) => void;
|
||||
onResolveRecoveryAction?: (outcome: import("../components/IssueRecoveryActionCard").RecoveryResolveOutcome) => Promise<void> | void;
|
||||
recoveryActionPending?: boolean;
|
||||
onReissueIsolatedRecoveryAction?: (request: import("../components/IssueRecoveryActionCard").RecoveryReissueRequest) => void;
|
||||
reissueIsolatedRecoveryActionPending?: boolean;
|
||||
@@ -4068,25 +4068,29 @@ export function IssueDetail() {
|
||||
}, [updateIssue.mutateAsync]);
|
||||
const activeRecoveryActionId = issue?.activeRecoveryAction?.id;
|
||||
const handleResolveRecoveryAction = useCallback(
|
||||
(outcome: import("../components/IssueRecoveryActionCard").RecoveryResolveOutcome) => {
|
||||
async (outcome: import("../components/IssueRecoveryActionCard").RecoveryResolveOutcome) => {
|
||||
const actionId = activeRecoveryActionId;
|
||||
if (!actionId) return;
|
||||
switch (outcome) {
|
||||
case "todo":
|
||||
void resolveRecoveryAction.mutateAsync({ actionId, outcome: "restored", sourceIssueStatus: "todo" });
|
||||
return;
|
||||
case "done":
|
||||
void resolveRecoveryAction.mutateAsync({ actionId, outcome: "restored", sourceIssueStatus: "done" });
|
||||
return;
|
||||
case "in_review":
|
||||
void resolveRecoveryAction.mutateAsync({ actionId, outcome: "restored", sourceIssueStatus: "in_review" });
|
||||
return;
|
||||
case "false_positive_done":
|
||||
void resolveRecoveryAction.mutateAsync({ actionId, outcome: "false_positive", sourceIssueStatus: "done" });
|
||||
return;
|
||||
case "false_positive_in_review":
|
||||
void resolveRecoveryAction.mutateAsync({ actionId, outcome: "false_positive", sourceIssueStatus: "in_review" });
|
||||
return;
|
||||
try {
|
||||
switch (outcome) {
|
||||
case "todo":
|
||||
await resolveRecoveryAction.mutateAsync({ actionId, outcome: "restored", sourceIssueStatus: "todo" });
|
||||
return;
|
||||
case "done":
|
||||
await resolveRecoveryAction.mutateAsync({ actionId, outcome: "restored", sourceIssueStatus: "done" });
|
||||
return;
|
||||
case "in_review":
|
||||
await resolveRecoveryAction.mutateAsync({ actionId, outcome: "restored", sourceIssueStatus: "in_review" });
|
||||
return;
|
||||
case "false_positive_done":
|
||||
await resolveRecoveryAction.mutateAsync({ actionId, outcome: "false_positive", sourceIssueStatus: "done" });
|
||||
return;
|
||||
case "false_positive_in_review":
|
||||
await resolveRecoveryAction.mutateAsync({ actionId, outcome: "false_positive", sourceIssueStatus: "in_review" });
|
||||
return;
|
||||
}
|
||||
} catch {
|
||||
// The mutation's onError handler surfaces the failure.
|
||||
}
|
||||
},
|
||||
[activeRecoveryActionId, resolveRecoveryAction.mutateAsync],
|
||||
|
||||
Reference in new issue
Block a user