fix(desktop): distinguish workflow preparation failures from submission

Signed-off-by: Logan Johnson <loganj@squareup.com>
This commit is contained in:
Logan Johnson
2026-09-04 08:04:09 -04:00
parent 547a6ef1dc
commit 07e4fd21f6
4 changed files with 52 additions and 3 deletions

View File

@@ -58,6 +58,7 @@ test("production owner retains exact signed payload after committed response los
release();
await assert.rejects(first, /response lost/);
assert.equal(owner.state(key).status, "error");
assert.equal(owner.state(key).failurePhase, "submit");
const result = await owner.run("workflow", scope);
assert.equal(result.runId, "run-1");
assert.equal(prepared, 1);
@@ -119,6 +120,10 @@ test("preflight rejection retries preparation without inventing a submitted even
assert.fail("preflight failure must not publish"),
});
await assert.rejects(owner.run("workflow", scope), /stale revision/);
assert.equal(
owner.state(owner.key("workflow", scope)).failurePhase,
"prepare",
);
await assert.rejects(owner.run("workflow", scope), /stale revision/);
assert.equal(tries, 2);
});

View File

@@ -8,6 +8,7 @@ import type { RelayEvent, TriggerWorkflowResponse } from "@/shared/api/types";
export type TriggerState = {
status: "idle" | "pending" | "error" | "success";
error?: string;
failurePhase?: "prepare" | "submit";
result?: TriggerWorkflowResponse;
};
const idle: TriggerState = { status: "idle" };
@@ -61,7 +62,8 @@ export class WorkflowTriggerOperations {
let op = this.operations.get(key);
if (op?.inFlight) return op.inFlight;
// A settled success means the next explicit Trigger is a distinct run.
// An error means Retry of the same event unless the user explicitly opts out.
// Submission errors retry the same event unless the user explicitly opts out.
// Preparation errors have not published anything and may prepare again.
if (!op || newRun || op.state.status === "success") {
if (!op && this.operations.size >= 256) {
for (const [oldKey, old] of this.operations) {
@@ -99,6 +101,7 @@ export class WorkflowTriggerOperations {
operation.state = {
status: "error",
error: error instanceof Error ? error.message : String(error),
failurePhase: operation.event ? "submit" : "prepare",
};
throw error;
} finally {

View File

@@ -34,6 +34,19 @@ export function WorkflowTriggerFeedback({
Run created: {state.result?.runId}
</p>
);
if (state.failurePhase === "prepare")
return (
<div
role="alert"
className="pointer-events-auto space-y-2 text-xs text-destructive"
>
<p>Workflow run was not started. {state.error}</p>
<p>Retry prepares a new request before submitting it.</p>
<Button size="sm" variant="outline" onClick={onRetry}>
Retry trigger
</Button>
</div>
);
return (
<div
role="alert"

View File

@@ -45,7 +45,7 @@ after(() => dom.window.close());
// Actual card + detail hook + IPC adapters. Unlike the former invocation-count
// assertion, this peer signs and commits an event, loses the response, then
// verifies the exact retry bytes and returns the same persisted run identity.
test("card/detail share pending, accessible error/retry and confirmed-only success animation", async () => {
test("card/detail distinguish preparation from ambiguous submission and animate only confirmed success", async () => {
const keys = generateSecretKey();
const pubkey = getPublicKey(keys);
localStorage.setItem(
@@ -92,6 +92,7 @@ test("card/detail share pending, accessible error/retry and confirmed-only succe
if (command === "get_run_approvals") return { approvals: [] };
if (command === "prepare_workflow_trigger") {
prepares++;
if (prepares === 1) throw new Error("revision lookup failed");
assert.equal(args.expectedRelayUrl, "wss://a.example");
assert.equal(args.expectedSignerPubkey, pubkey);
return finalizeEvent(
@@ -196,6 +197,26 @@ test("card/detail share pending, accessible error/retry and confirmed-only succe
fireEvent.click(
rendered.getByRole("menuitem", { name: "Trigger", exact: true }),
);
await waitFor(() =>
assert.match(
card.getByRole("alert").textContent,
/revision lookup failed/,
),
);
for (const surface of [card, detail]) {
const text = surface.getByRole("alert").textContent;
assert.match(text, /Workflow run was not started/);
assert.doesNotMatch(text, /may already exist|same signed request/);
assert.equal(
surface.queryByRole("button", { name: "Start a distinct run…" }),
null,
);
}
assert.equal(posts.length, 0, "preparation failure cannot publish");
assert.equal(stack.firstChild, originalTile);
fireEvent.click(
card.getByRole("button", { name: "Retry trigger", exact: true }),
);
await waitFor(() => assert.equal(posts.length, 1));
assert.match(card.getByRole("status").textContent, /Triggering/);
assert.equal(
@@ -226,6 +247,13 @@ test("card/detail share pending, accessible error/retry and confirmed-only succe
),
);
assert.match(detail.getByRole("alert").textContent, /same signed request/);
for (const surface of [card, detail]) {
assert.doesNotMatch(
surface.getByRole("alert").textContent,
/was not started/,
);
assert.ok(surface.getByRole("button", { name: "Start a distinct run…" }));
}
assert.equal(
stack.firstChild,
originalTile,
@@ -236,7 +264,7 @@ test("card/detail share pending, accessible error/retry and confirmed-only succe
);
await waitFor(() => assert.equal(posts.length, 2));
assert.equal(posts[0], posts[1]);
assert.equal(prepares, 1);
assert.equal(prepares, 2);
assert.equal(runs.size, 1);
await act(async () => {
resolvePost();