Skip to content

Commit 072330d

Browse files
os-zhuangzhuangjianguoclaude
authored
fix(console): make a paused screen flow completable, and stop the runner from tearing down its host (framework#3528) (#2830)
Two defects on the screen-flow path, both ending in "Submit never resumes". **Flow Runs test runner had no second half.** Developer → Flow Runs triggers a flow and renders the result. For a screen flow that result is not a result — it is `{ status: 'paused', runId, screen }`, and the run sits suspended until something posts to its resume endpoint. The panel dumped the envelope as JSON and stopped: no screen, no Submit, no resume call, and an orphaned `paused` row in Recent Runs for every test run. It now hands the pause to the same `FlowRunner` the record and list surfaces use, so the screen renders for real (flat fields, multi-step wizards, and `object-form` steps with their master-detail grids). Dismissing the runner no longer strands the run: the suspension is durable, so a "Continue run" affordance reopens the pending screen. `paused` also gets its own status badge instead of falling through to the unknown-status style. **FlowRunner tore down its host.** An `object-form` step mounts ObjectForm, whose field widgets are lazy. That suspension unwound to the *host's* nearest `<Suspense>`; where that is the route-level boundary, React swapped the whole page for the fallback and remounted it, destroying the host's state and this dialog with it. The screen vanished before it could be filled in and the run stayed paused with no resume call — the reported symptom exactly. The runner now owns a boundary around its screen body, so the lazy load is local and no host can be torn down by it. Verified in a browser against a live server: the two-step Convert Lead wizard now creates the account, resumes, and renders step 2. Also: a screen payload without `fields` no longer throws. `fields` is optional on the wire (a message-only screen, or an executor that omits it) but was read unguarded as the dialog mounted; reads go through a `screenFields()` helper and the design-time builder keeps its exhaustive shape. `FlowRunner` and its types are exported from the package so surfaces outside `views/` can mount the one runner instead of reimplementing it. Claude-Session: https://claude.ai/code/session_01LX9ut3MK3KykE11S9bJmv5 Co-authored-by: Claude <zhuangjianguo@steedos.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 697cda4 commit 072330d

10 files changed

Lines changed: 325 additions & 28 deletions

File tree

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
"@object-ui/console": patch
3+
"@object-ui/app-shell": patch
4+
---
5+
6+
fix(console): let a screen flow be completed from the developer Flow Runs page (framework#3528)
7+
8+
Developer → Flow Runs triggers a flow and renders the result. For a **screen**
9+
flow that result is not a result — it is `{ status: 'paused', runId, screen }`,
10+
and the run sits suspended until something posts to its resume endpoint. The
11+
panel dumped that envelope as JSON and stopped: no screen, no Submit, no resume
12+
call. Every test run of a screen flow left an orphaned `paused` row in Recent
13+
Runs, and there was no way to drive one to completion from this surface.
14+
15+
- **console** — a paused test run now opens the same `FlowRunner` the record and
16+
list surfaces use, so the screen renders for real (flat fields, multi-step
17+
wizards, and `object-form` steps with their master-detail grids) and Submit
18+
posts to `/automation/:flow/runs/:runId/resume`. Dismissing the runner no
19+
longer strands the run: the pause is durable, so the panel keeps a "Continue
20+
run" affordance to reopen the pending screen. `paused` also gets its own
21+
status badge instead of falling through to the unknown-status style.
22+
- **app-shell**`FlowRunner` (and its `ScreenFlowState` / `ScreenSpec` types)
23+
is now exported from the package so surfaces outside `views/` can mount the
24+
one screen-flow runner rather than reimplementing it.
25+
- **app-shell**`FlowRunner` now wraps the screen body in its own `<Suspense>`
26+
boundary. An `object-form` step mounts `ObjectForm`, whose field widgets are
27+
lazy; that suspension used to unwind to the *host's* nearest boundary, and on
28+
a surface whose nearest boundary is the route-level one, React swapped the
29+
whole page for the fallback and remounted it — destroying the host's state
30+
along with this dialog. The screen vanished before it could be filled in and
31+
the run stayed paused with no resume call, which is exactly the "Submit does
32+
nothing" shape. Reproduced on the Flow Runs page and fixed at the source, so
33+
every host that mounts the runner is covered.
34+
- **app-shell** — a screen payload without `fields` no longer throws. `fields`
35+
is optional on the wire (a message-only screen, or an `object-form` step from
36+
a node executor that omits it), but `FlowRunner`/`ScreenView` read it
37+
unguarded and blew up as the dialog mounted. Reads now go through a
38+
`screenFields()` helper; the design-time builder keeps its exhaustive shape.
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
/**
2+
* Flow Runs — a screen flow triggered from the Test Run panel must be
3+
* completable, not left suspended.
4+
*
5+
* Regression for framework#3528: the panel triggered the flow, dumped the
6+
* `{ status: 'paused', runId, screen }` envelope as JSON and stopped — the run
7+
* stayed paused forever and the resume endpoint was never called from any
8+
* developer surface. The panel now hands the pause to the shared FlowRunner.
9+
*
10+
* jsdom integration test — no backend; the automation client and the
11+
* authenticated fetch the runner resumes through are both stubbed.
12+
*/
13+
14+
import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest';
15+
import { render, screen, cleanup, waitFor } from '@testing-library/react';
16+
import userEvent from '@testing-library/user-event';
17+
18+
// Hoisted so the vi.mock factories below can close over them, and so the
19+
// adapter is a STABLE singleton (a fresh object per render loops the page's
20+
// fetch effects).
21+
const { execute, ADAPTER, authFetch } = vi.hoisted(() => {
22+
const flow = {
23+
name: 'reassign_wizard',
24+
label: 'Reassign',
25+
variables: [{ name: 'recordId', type: 'text', isInput: true }],
26+
};
27+
// The trigger response for a screen flow: suspended at its `screen` node.
28+
const execute = vi.fn(async () => ({
29+
success: true,
30+
status: 'paused',
31+
runId: 'run_1',
32+
durationMs: 3,
33+
screen: {
34+
nodeId: 'collect',
35+
title: 'New Assignee',
36+
fields: [{ name: 'new_assignee', label: 'New Assignee', type: 'text', required: true }],
37+
},
38+
}));
39+
const ADAPTER = {
40+
getClient: () => ({
41+
meta: { getItems: async (type: string) => (type === 'flow' ? [{ spec: flow }] : []) },
42+
automation: { execute, listRuns: async () => ({ runs: [] }) },
43+
}),
44+
};
45+
const authFetch = vi.fn(async () =>
46+
new Response(JSON.stringify({ success: true, data: { success: true, durationMs: 9 } }), {
47+
status: 200,
48+
headers: { 'Content-Type': 'application/json' },
49+
}),
50+
);
51+
return { execute, ADAPTER, authFetch };
52+
});
53+
54+
vi.mock('@object-ui/app-shell', async () => {
55+
// The runner itself is the real component — this test asserts it is mounted
56+
// AND that it resumes, so stubbing it would defeat the purpose.
57+
const { FlowRunner } = await import('../../../../../packages/app-shell/src/views/FlowRunner');
58+
return {
59+
useAdapter: () => ADAPTER,
60+
useMetadata: () => ({ objects: [] }),
61+
FlowRunner,
62+
};
63+
});
64+
65+
vi.mock('@object-ui/auth', () => ({ createAuthenticatedFetch: () => authFetch }));
66+
67+
// Imported AFTER the mocks so the page picks them up.
68+
import { FlowRunsPage } from './FlowRunsPage';
69+
70+
beforeEach(() => {
71+
execute.mockClear();
72+
authFetch.mockClear();
73+
});
74+
afterEach(cleanup);
75+
76+
describe('Flow Runs — screen flows are completable from the Test Run panel', () => {
77+
it('renders the paused screen and resumes the run on Submit', async () => {
78+
const user = userEvent.setup();
79+
render(<FlowRunsPage />);
80+
81+
await screen.findByRole('button', { name: /Run Flow/i });
82+
await user.click(screen.getByRole('button', { name: /Run Flow/i }));
83+
84+
// The pause opens the shared screen-flow runner instead of dead-ending.
85+
await waitFor(() => expect(execute).toHaveBeenCalledWith('reassign_wizard', expect.any(Object)));
86+
const dialog = await screen.findByRole('dialog');
87+
expect(dialog).toHaveTextContent('New Assignee');
88+
89+
// The dialog's field is the last textbox on the page (the panel's own
90+
// input-variable boxes render above it).
91+
const textboxes = screen.getAllByRole('textbox');
92+
await user.type(textboxes[textboxes.length - 1], 'ada@example.com');
93+
await user.click(screen.getByRole('button', { name: 'Submit' }));
94+
95+
await waitFor(() =>
96+
expect(authFetch).toHaveBeenCalledWith(
97+
'/api/v1/automation/reassign_wizard/runs/run_1/resume',
98+
expect.objectContaining({
99+
method: 'POST',
100+
body: JSON.stringify({ inputs: { new_assignee: 'ada@example.com' } }),
101+
}),
102+
),
103+
);
104+
});
105+
106+
it('offers a "Continue run" affordance after the runner is dismissed', async () => {
107+
const user = userEvent.setup();
108+
render(<FlowRunsPage />);
109+
110+
await screen.findByRole('button', { name: /Run Flow/i });
111+
await user.click(screen.getByRole('button', { name: /Run Flow/i }));
112+
await screen.findByRole('dialog');
113+
114+
await user.click(screen.getByRole('button', { name: 'Cancel' }));
115+
await waitFor(() => expect(screen.queryByRole('dialog')).not.toBeInTheDocument());
116+
117+
// The suspension is durable — the tester can reopen the pending screen.
118+
const resume = await screen.findByRole('button', { name: /Continue run/i });
119+
await user.click(resume);
120+
expect(await screen.findByRole('dialog')).toHaveTextContent('New Assignee');
121+
});
122+
});

apps/console/src/pages/developer/FlowRunsPage.tsx

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,20 @@
55
* Console is not project-scoped: there is no useScopedClient and no projectId
66
* param. The client comes from `useAdapter().getClient()` (which exposes
77
* `.automation` and `.meta`). Run output renders as a plain `<pre>` block.
8+
*
9+
* A *screen* flow pauses instead of completing: the trigger returns
10+
* `{ status: 'paused', runId, screen }`. This page used to dump that envelope
11+
* as JSON and stop — the run stayed suspended forever with no way to finish it
12+
* from the UI, and every test run of a screen flow orphaned a `paused` row in
13+
* the history (framework#3528). It now hands the pause to the SAME
14+
* {@link FlowRunner} the record/list surfaces use, so a screen flow can be
15+
* driven to completion (multi-step wizards and `object-form` steps included)
16+
* from here too.
817
*/
918

1019
import { useEffect, useMemo, useState, useCallback } from 'react';
11-
import { useAdapter } from '@object-ui/app-shell';
20+
import { useAdapter, useMetadata, FlowRunner, type ScreenFlowState } from '@object-ui/app-shell';
21+
import { createAuthenticatedFetch } from '@object-ui/auth';
1222
import {
1323
Card,
1424
CardContent,
@@ -71,6 +81,9 @@ function StatusBadge({ status }: { status: string }) {
7181
error: { icon: XCircle, cls: 'text-red-600 border-red-300' },
7282
running: { icon: Loader2, cls: 'text-blue-600 border-blue-300 animate-pulse' },
7383
pending: { icon: Clock, cls: 'text-amber-600 border-amber-300' },
84+
// A screen/approval node suspended the run — it is waiting for input, not
85+
// stalled (ADR-0019 durable pause).
86+
paused: { icon: Clock, cls: 'text-amber-600 border-amber-300' },
7487
skipped: { icon: AlertCircle, cls: 'text-muted-foreground' },
7588
};
7689
const cfg = map[status] ?? { icon: AlertCircle, cls: 'text-muted-foreground' };
@@ -247,15 +260,23 @@ function FlowTestRunner({
247260
[flow],
248261
);
249262

263+
const adapter = useAdapter();
264+
const { objects } = useMetadata();
265+
const authFetch = useMemo(() => createAuthenticatedFetch(), []);
266+
250267
const [values, setValues] = useState<Record<string, string>>({});
251268
const [running, setRunning] = useState(false);
252269
const [result, setResult] = useState<any>(null);
253270
const [error, setError] = useState<string | null>(null);
271+
// The paused screen a test run stopped at — driven to completion by
272+
// <FlowRunner>, exactly as a `type: 'flow'` action would.
273+
const [screenFlow, setScreenFlow] = useState<ScreenFlowState | null>(null);
254274

255275
useEffect(() => {
256276
setValues({});
257277
setResult(null);
258278
setError(null);
279+
setScreenFlow(null);
259280
}, [flow?.name]);
260281

261282
const setVal = (k: string, v: string) => setValues(s => ({ ...s, [k]: v }));
@@ -268,6 +289,7 @@ function FlowTestRunner({
268289
setRunning(true);
269290
setError(null);
270291
setResult(null);
292+
setScreenFlow(null);
271293
try {
272294
const params: Record<string, unknown> = {};
273295
for (const v of inputVars) {
@@ -277,6 +299,11 @@ function FlowTestRunner({
277299
}
278300
const res = await client.automation.execute(flow.name, { params });
279301
setResult(res);
302+
// Screen flow: the run is suspended at a `screen` node. Open the runner
303+
// so the tester can fill it in and the run can actually finish.
304+
if (res?.status === 'paused' && res?.screen && res?.runId) {
305+
setScreenFlow({ flowName: flow.name, runId: res.runId, screen: res.screen });
306+
}
280307
onExecuted?.();
281308
} catch (e: any) {
282309
setError(e?.message ?? String(e));
@@ -342,6 +369,8 @@ function FlowTestRunner({
342369
<div className="flex items-center gap-2 text-sm font-medium">
343370
{error || result?.success === false ? (
344371
<><XCircle className="h-4 w-4 text-red-500" /> Failed</>
372+
) : result?.status === 'paused' ? (
373+
<><Clock className="h-4 w-4 text-amber-500" /> Waiting for input</>
345374
) : (
346375
<><CheckCircle2 className="h-4 w-4 text-emerald-500" /> Result</>
347376
)}
@@ -352,10 +381,38 @@ function FlowTestRunner({
352381
)}
353382
</div>
354383
{error && <p className="text-sm text-red-500 font-mono break-all">{error}</p>}
384+
{/* A run left paused (the runner was dismissed) can be reopened —
385+
the suspension is durable, so the screen is still waiting. */}
386+
{result?.status === 'paused' && result?.screen && result?.runId && !screenFlow && (
387+
<div className="flex flex-wrap items-center gap-2">
388+
<p className="text-sm text-muted-foreground">
389+
This run is paused at screen <span className="font-mono">{result.screen.nodeId}</span>.
390+
</p>
391+
<Button
392+
size="sm"
393+
variant="outline"
394+
onClick={() => setScreenFlow({ flowName: flow.name, runId: result.runId, screen: result.screen })}
395+
>
396+
Continue run
397+
</Button>
398+
</div>
399+
)}
355400
{result && <JsonBlock data={result} />}
356401
</div>
357402
)}
358403
</CardContent>
404+
405+
{/* The shared screen-flow runner — renders the paused screen and POSTs
406+
the collected values to the run's resume endpoint. */}
407+
<FlowRunner
408+
state={screenFlow}
409+
authFetch={authFetch}
410+
baseUrl={import.meta.env.VITE_SERVER_URL || ''}
411+
dataSource={adapter}
412+
objects={objects}
413+
onClose={() => { setScreenFlow(null); onExecuted?.(); }}
414+
onComplete={() => { setScreenFlow(null); onExecuted?.(); }}
415+
/>
359416
</Card>
360417
);
361418
}

packages/app-shell/src/index.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,16 @@ export {
134134
SearchResultsPage,
135135
ViewConfigPanel,
136136
DeclaredActionsBar,
137+
FlowRunner,
138+
} from './views';
139+
export type {
140+
RecordFormPageProps,
141+
DeclaredActionsBarProps,
142+
FlowRunnerProps,
143+
ScreenFlowState,
144+
ScreenSpec,
145+
ScreenFieldSpec,
137146
} from './views';
138-
export type { RecordFormPageProps, DeclaredActionsBarProps } from './views';
139147

140148
// Hooks
141149
export {

packages/app-shell/src/views/FlowRunner.tsx

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* {@link ScreenView} — the same renderer the Studio design preview reuses, so
1515
* the two can never drift (cf. #1927).
1616
*/
17-
import { useEffect, useState } from 'react';
17+
import { Suspense, useEffect, useState } from 'react';
1818
import {
1919
Dialog,
2020
DialogContent,
@@ -25,7 +25,7 @@ import {
2525
Button,
2626
} from '@object-ui/components';
2727
import { toast } from 'sonner';
28-
import { ScreenView, isObjectFormScreen, initialScreenValues, type ScreenSpec } from './ScreenView';
28+
import { ScreenView, isObjectFormScreen, initialScreenValues, screenFields, type ScreenSpec } from './ScreenView';
2929

3030
export type { ScreenSpec, ScreenFieldSpec } from './ScreenView';
3131

@@ -120,7 +120,7 @@ export function FlowRunner({ state, authFetch, baseUrl, onClose, onComplete, dat
120120
};
121121

122122
const submit = async () => {
123-
const missing = screen.fields.filter(
123+
const missing = screenFields(screen).filter(
124124
(f) => f.required && (values[f.name] === undefined || values[f.name] === '' || values[f.name] === null),
125125
);
126126
if (missing.length) {
@@ -164,21 +164,29 @@ export function FlowRunner({ state, authFetch, baseUrl, onClose, onComplete, dat
164164
{screen.description && <DialogDescription>{screen.description}</DialogDescription>}
165165
</DialogHeader>
166166

167-
<ScreenView
168-
screen={screen}
169-
values={values}
170-
onValueChange={setVal}
171-
dataSource={dataSource}
172-
objects={objects}
173-
objectForm={{
174-
onSuccess: onObjectFormSaved,
175-
onCancel: onClose,
176-
showSubmit: true,
177-
showCancel: true,
178-
submitText: 'Save & Continue',
179-
cancelText: 'Cancel',
180-
}}
181-
/>
167+
{/* The screen body pulls in lazily-loaded chunks (an `object-form` step
168+
mounts ObjectForm, whose field widgets are lazy). Without a boundary
169+
HERE, that suspension unwinds to the host's nearest <Suspense> — a
170+
route-level one on some surfaces — which swaps the whole page for a
171+
fallback and destroys the host's state, taking this dialog (and the
172+
run it is driving) with it. */}
173+
<Suspense fallback={<div className="py-6 text-sm text-muted-foreground">Loading…</div>}>
174+
<ScreenView
175+
screen={screen}
176+
values={values}
177+
onValueChange={setVal}
178+
dataSource={dataSource}
179+
objects={objects}
180+
objectForm={{
181+
onSuccess: onObjectFormSaved,
182+
onCancel: onClose,
183+
showSubmit: true,
184+
showCancel: true,
185+
submitText: 'Save & Continue',
186+
cancelText: 'Cancel',
187+
}}
188+
/>
189+
</Suspense>
182190

183191
{!isObjectForm && (
184192
<DialogFooter>

0 commit comments

Comments
 (0)