fix(automation)!: getSuspendedScreen 读取持久化存储，而不只是内存热缓存 (#4515)

.changeset/durable-suspended-screen-refetch.md

@@ -0,0 +1,25 @@
+---
+'@objectstack/spec': minor
+'@objectstack/service-automation': minor
+'@objectstack/runtime': minor
+---
+
+**BREAKING**: `IAutomationService.getSuspendedScreen(runId)` is now **async** — it returns `Promise<ScreenSpec | null>` instead of `ScreenSpec | null` (#4515).
+
+FROM → TO for anyone calling or implementing it:
+
+```ts
+// caller
+- const screen = automationService.getSuspendedScreen(runId);
++ const screen = await automationService.getSuspendedScreen(runId);
+
+// implementer
+- getSuspendedScreen(runId: string): ScreenSpec | null
++ async getSuspendedScreen(runId: string): Promise<ScreenSpec | null>
+```
+
+One-line fix: `await` the call (the enclosing function is almost certainly already `async`), and make any test double resolve rather than return (`mockResolvedValue`, not `mockReturnValue`).
+
+Why it had to change: the method could only ever read the engine's in-memory hot cache, because a synchronous signature cannot consult the durable suspended-run store. `SuspendedRun.screen` *is* persisted (`sys_automation_run.screen_json`) and `resume()` cold-reads it back, so after a process restart a still-suspended screen run could be resumed (`POST …/runs/:runId/resume` → 200) while `GET …/runs/:runId/screen` returned 404 “No pending screen for run” — the refresh-safe re-fetch failing in exactly the situation it exists for (page refresh, another device), and the rendering half of ADR-0019's durable-suspend promise missing while the resuming half shipped.
+
+`AutomationEngine.getSuspendedScreen` now takes the hot cache as its fast path and falls through to the store via the same loader `resume()` rehydrates from. A run that does not exist, is no longer suspended, or paused at a non-screen node still resolves to `null`, so `GET …/runs/:runId/screen` keeps returning 404 for genuinely absent runs. No sync variant of the method remains on the contract.

docs/design/screen-flow-runtime.md

@@ -26,7 +26,7 @@ interface ScreenSpec { nodeId: string; title?: string; description?: string; fie
 - **screen executor**: suspend when `waitForInput === true` **or** (`config.fields` non-empty **and** `waitForInput !== false`). When suspending, return `{ success:true, suspend:true, screen: { nodeId, title, description, fields } }` built from `node.config`.
 - **suspend plumbing**: `NodeExecutionResult.screen` → `FlowSuspendSignal.screen` → `SuspendedRun.screen` → paused `AutomationResult.screen`.
 - **resume**: apply `signal.variables` as **bare** variables (`variables.set(name, value)`) in addition to the existing `signal.output` (`${nodeId}.key`). If the continuation suspends at another screen, return that screen (multi-screen wizards).
-- `getSuspendedScreen(runId)` getter so HTTP can re-fetch the current screen.
+- `async getSuspendedScreen(runId)` getter so HTTP can re-fetch the current screen. Durable (#4515): hot cache first, then the `SuspendedRunStore` via the same loader `resume` rehydrates from, so a screen run that survived a restart renders as well as it resumes.
 
 ### HTTP (`runtime/http-dispatcher.ts` `handleAutomation`)
 - **Launch**: existing `POST /api/v1/automation/:name/trigger` — when the run pauses at a screen, the response includes `{ status:'paused', runId, screen }`.

packages/qa/dogfood/test/flow-durable-suspend.dogfood.test.ts

@@ -291,6 +291,42 @@ describe('objectstack verify FLOW: a suspended run survives a real cold boot (#4
  expect(JSON.parse(String(rec.variables_json)).noteId).toBe(noteId);
  });
 
+ it('the cold kernel RE-FETCHES the screen — refresh-safe rendering, not just resuming (#4515)', async () => {
+ // The rendering half of the same promise, and the half that was missing.
+ // `GET …/runs/:runId/screen` exists so a user who refreshes the page — or
+ // picks the flow up on another device — gets the form back. It was backed
+ // by `AutomationEngine.getSuspendedScreen`, which was SYNCHRONOUS and so
+ // structurally could only read the in-memory hot cache: after this cold
+ // boot the run was resumable (the test below) yet its screen 404'd, i.e.
+ // the route failed in exactly the situation it was built for. Fixed by
+ // making the contract method async and falling through to the same
+ // suspended-run store `resume` rehydrates from.
+ const res = await cold!.apiAs(coldToken, 'GET', `/automation/flow_durable_suspend/runs/${runId}/screen`);
+ expect(res.status, `screen re-fetch after cold boot: ${await res.clone().text()}`).toBe(200);
+ const body = (await res.json()) as any;
+ const payload = body.data ?? body;
+ expect(payload.runId).toBe(runId);
+
+ // The screen served by a kernel that never rendered it must be the screen
+ // the flow declared — read back out of `screen_json`, field contract intact
+ // (that contract is what the resume below is validated against, #4477).
+ const screen = payload.screen;
+ expect(screen.nodeId).toBe('ask');
+ expect(screen.fields.map((f: any) => f.name)).toContain('resolution');
+ expect(screen.fields.find((f: any) => f.name === 'resolution').required).toBe(true);
+
+ // Read-only: re-fetching must not consume the pause, or a refresh would
+ // destroy the very run it is trying to display.
+ const again = await cold!.apiAs(coldToken, 'GET', `/automation/flow_durable_suspend/runs/${runId}/screen`);
+ expect(again.status).toBe(200);
+ const row = await cold!.apiAs(coldToken, 'GET', `/data/sys_automation_run/${runId}`);
+ expect(row.status).toBe(200);
+
+ // A genuinely absent run still 404s — durable ≠ credulous.
+ const missing = await cold!.apiAs(coldToken, 'GET', '/automation/flow_durable_suspend/runs/run_nope/screen');
+ expect(missing.status).toBe(404);
+ });
+
  it('the cold kernel RESUMES the run and takes the right branch', async () => {
  // The one assertion #4470 was written for. The second kernel never
  // executed a node of this run: it has to rebuild the continuation from
@@ -313,6 +349,12 @@ describe('objectstack verify FLOW: a suspended run survives a real cold boot (#4
  const history = await cold!.apiAs(coldToken, 'GET', `/data/sys_automation_run/run_${runId}`);
  expect(history.status).toBe(200);
  expect((((await history.json()) as any).record ?? {}).status).toBe('completed');
+
+ // …and with the suspension consumed there is no screen left to render:
+ // the durable fallback answers for runs that are SUSPENDED, not for every
+ // id that was ever suspended (#4515).
+ const screen = await cold!.apiAs(coldToken, 'GET', `/automation/flow_durable_suspend/runs/${runId}/screen`);
+ expect(screen.status).toBe(404);
  });
 
  it('the resumed result is itself durable — a THIRD boot still reads it', async () => {

packages/runtime/src/domains/automation.ts

@@ -370,7 +370,7 @@ export async function handleAutomationRequest(deps: DomainHandlerDeps, path: str
  // (refresh-safe re-fetch for the UI flow-runner).
  if (parts[1] === 'runs' && parts[2] && parts[3] === 'screen' && m === 'GET') {
  if (typeof automationService.getSuspendedScreen === 'function') {
- const screen = automationService.getSuspendedScreen(parts[2]);
+ const screen = await automationService.getSuspendedScreen(parts[2]);
  if (!screen) return { handled: true, response: deps.error('No pending screen for run', 404) };
  return { handled: true, response: deps.success({ runId: parts[2], screen }) };
  }

packages/runtime/src/http-dispatcher.test.ts

@@ -188,8 +188,11 @@ describe('HttpDispatcher', () => {
  listRuns: vi.fn().mockResolvedValue([{ id: 'run_1', status: 'completed' }]),
  getRun: vi.fn().mockResolvedValue({ id: 'run_1', status: 'completed' }),
  resume: vi.fn().mockResolvedValue({ success: true, output: {}, durationMs: 7 }),
- // Sync per IAutomationService — `ScreenSpec | null`, not a promise.
- getSuspendedScreen: vi.fn().mockReturnValue({ nodeId: 'collect', fields: [] }),
+ // ASYNC per IAutomationService (#4515) — `Promise<ScreenSpec | null>`.
+ // It has to be: a screen re-fetch answers for any genuinely
+ // suspended run, which after a restart means reading the
+ // durable suspended-run store, not just the hot cache.
+ getSuspendedScreen: vi.fn().mockResolvedValue({ nodeId: 'collect', fields: [] }),
  getActionDescriptors: vi.fn().mockReturnValue([
  { type: 'decision', name: 'Decision', category: 'logic', paradigms: ['flow'], source: 'builtin' },
  { type: 'http_request', name: 'HTTP Request', category: 'io', paradigms: ['flow', 'approval'], source: 'builtin' },
@@ -444,7 +447,7 @@ describe('HttpDispatcher', () => {
  });
 
  it('should return 404 when the run is not awaiting a screen', async () => {
- mockAutomationService.getSuspendedScreen.mockReturnValue(null);
+ mockAutomationService.getSuspendedScreen.mockResolvedValue(null);
  const result = await dispatcher.handleAutomation('flow_a/runs/run_1/screen', 'GET', {}, { request: {} });
  expect(result.handled).toBe(true);
  expect(result.response?.status).toBe(404);

packages/services/service-automation/src/builtin/screen-resume-validation.test.ts

@@ -140,7 +140,7 @@ describe('screen resume validation (#4477)', () => {
  const bad = await engine.resume(runId, { variables: {} });
  expect(bad.success).toBe(false);
  // The screen is still fetchable…
- expect(engine.getSuspendedScreen(runId)?.nodeId).toBe('ask');
+ expect((await engine.getSuspendedScreen(runId))?.nodeId).toBe('ask');
  // …and the legitimate submission still lands.
  const good = await engine.resume(runId, { variables: { kind: 'normal' } });
  expect(good.success).toBe(true);

packages/services/service-automation/src/builtin/subflow-node.test.ts

@@ -245,7 +245,7 @@ describe('subflow node executor', () => {
  expect(r2.status).toBe('paused');
  expect(r2.runId).toBe(parentRunId); // UI keeps one stable run id
  expect(r2.screen).toEqual(s2); // next wizard screen
- expect(engine.getSuspendedScreen(parentRunId)).toEqual(s2); // refresh-safe re-fetch
+ expect(await engine.getSuspendedScreen(parentRunId)).toEqual(s2); // refresh-safe re-fetch
 
  const r3 = await engine.resume(parentRunId, { variables: { other: 'x' } });
  expect(r3.success).toBe(true);

packages/services/service-automation/src/engine.test.ts

@@ -581,13 +581,13 @@ describe('AutomationEngine', () => {
  expect(paused.screen!.fields[0]).toMatchObject({ name: 'new_assignee', required: true, type: 'text' });
  expect(captured).toBe('UNSET'); // downstream not run yet
  // Re-fetchable for a refreshed client.
- expect(engine.getSuspendedScreen(paused.runId!)).toMatchObject({ nodeId: 'collect' });
+ expect(await engine.getSuspendedScreen(paused.runId!)).toMatchObject({ nodeId: 'collect' });
 
  const done = await engine.resume(paused.runId!, { variables: { new_assignee: 'ada@example.com' } });
  expect(done.success).toBe(true);
  expect(done.status).toBeUndefined();
  expect(captured).toBe('ada@example.com'); // bare var set on resume → downstream read it
- expect(engine.getSuspendedScreen(paused.runId!)).toBeNull();
+ expect(await engine.getSuspendedScreen(paused.runId!)).toBeNull();
  });
 
  it('passes a field-less screen straight through (no pause)', async () => {

packages/services/service-automation/src/engine.ts

@@ -3186,9 +3186,20 @@ export class AutomationEngine implements IAutomationService {
  * The screen a paused run is currently waiting on (screen-flow runtime), or
  * `null` if the run isn't suspended / didn't pause at a screen node. Lets a
  * UI flow-runner re-fetch the form after a refresh.
+ *
+ * Durable (#4515): the hot cache is the fast path, and a miss falls through
+ * to the {@link SuspendedRunStore} via the same {@link loadSuspendedRun}
+ * that {@link resume} rehydrates from — one loader, two callers. Without
+ * that fallback a screen run that survived a restart could be *resumed* but
+ * not *rendered*, which is precisely when a refresh-safe re-fetch matters.
+ *
+ * Best-effort by design: a store outage reads as "no such run" (`null`),
+ * matching the 404 this backs. A caller that must distinguish "gone" from
+ * "unknown" before writing anything wants {@link hasSuspendedRun}, which
+ * throws instead.
  */
- getSuspendedScreen(runId: string): ScreenSpec | null {
- return this.suspendedRuns.get(runId)?.screen ?? null;
+async getSuspendedScreen(runId: string): Promise<ScreenSpec | null> {
+ return (await this.loadSuspendedRun(runId))?.screen ?? null;
  }
 
  // ── DAG Traversal Core ──────────────────────────────────