fix(workflow): address ready review findings

2026-08-10 20:56:31 +08:00
parent fff7dfac8e
commit a7ddded2ef
11 changed files with 80 additions and 62 deletions
--- a/packages/client/ui-workflow-run/src/client/WorkflowRunPanel.tsx
+++ b/packages/client/ui-workflow-run/src/client/WorkflowRunPanel.tsx
@@ -1,9 +1,9 @@
-import { useMemo, useState } from 'react'
+import { useState } from 'react'
 import {
  DisclosureRow, IconChevronRightOutline14, StateDot, type StateDotState,
 } from '@deepseek-ai/dsh-client-ui-primitives'
 import type { PropsLocale, PropsRuntime } from '@deepseek-ai/dsh-client-ui-slots'
-import type { SessionId } from '@deepseek-ai/dsh-client-runtime/client'
+import { shallowEqual, type SessionId, type SessionListState } from '@deepseek-ai/dsh-client-runtime/client'
 import type { WorkflowRunKey } from './locales.ts'
 import type {
  WorkflowRunMemberData, WorkflowRunPhaseData, WorkflowRunStatus,
@@ -58,6 +58,10 @@ function statusCount(
  return t(`statusCount.${status}`, { count })
 }

+function memberCount(count: number, t: WorkflowRunPanelProps['t']): string {
+  return t(count === 1 ? 'run.members.one' : 'run.members.other', { count })
+}
+
 function phaseStatusSummary(members: readonly WorkflowRunMemberData[], t: WorkflowRunPanelProps['t']): string {
  const counts = new Map<WorkflowRunStatus, number>()
  for (const member of members) counts.set(member.status, (counts.get(member.status) ?? 0) + 1)
@@ -71,6 +75,28 @@ function phaseStatusSummary(members: readonly WorkflowRunMemberData[], t: Workfl
  return visible.map(status => statusCount(status, count(status), t)).join(' · ')
 }

+function navigableMembers(
+  sessions: SessionListState,
+  phases: readonly WorkflowRunPhaseData[],
+  parentId: SessionId,
+): readonly SessionId[] {
+  const ordinary = new Set(sessions.ids)
+  const result: SessionId[] = []
+  for (const phase of phases) {
+    for (const member of phase.members) {
+      const summary = sessions.byId[member.childId]
+      if (member.status === 'running'
+        && ordinary.has(member.childId)
+        && summary?.origin === 'subagent'
+        && summary.parentId === parentId
+        && summary.running) {
+        result.push(member.childId)
+      }
+    }
+  }
+  return result
+}
+
 function RunHeader({ count, name, onToggle, open, status, t }: {
  readonly count: number
  readonly name: string
@@ -95,7 +121,7 @@ function RunHeader({ count, name, onToggle, open, status, t }: {
      collapsedContent={(
        <>
          <span className={css.separator} aria-hidden />
-          <span className={css.runSummary}>{t('run.members', { count })}</span>
+          <span className={css.runSummary}>{memberCount(count, t)}</span>
          <span className={css.statusTail} data-status={status}>
            <StateDot state={dotState(status)} />
            <span>{t(STATUS_KEYS[status])}</span>
@@ -138,7 +164,7 @@ function MemberRow({ member, navigable, openSession, t }: {

 function PhaseSection({ phase, navigable, openSession, t }: {
  readonly phase: WorkflowRunPhaseData
-  readonly navigable: ReadonlySet<SessionId>
+  readonly navigable: readonly SessionId[]
  readonly openSession: WorkflowRunInjected['openSession']
  readonly t: WorkflowRunPanelProps['t']
 }) {
@@ -161,7 +187,7 @@ function PhaseSection({ phase, navigable, openSession, t }: {
      collapsedContent={(
        <>
          <span className={css.separator} aria-hidden />
-          <span className={css.phaseCount} data-phase-count>{t('run.members', { count: phase.members.length })}</span>
+          <span className={css.phaseCount} data-phase-count>{memberCount(phase.members.length, t)}</span>
          <span className={css.phaseStatus} data-phase-status-text>{phaseStatusSummary(phase.members, t)}</span>
        </>
      )}
@@ -171,7 +197,7 @@ function PhaseSection({ phase, navigable, openSession, t }: {
          <MemberRow
            key={member.seq}
            member={member}
-            navigable={navigable.has(member.childId)}
+            navigable={navigable.includes(member.childId)}
            openSession={openSession}
            t={t}
          />
@@ -184,25 +210,11 @@ function PhaseSection({ phase, navigable, openSession, t }: {
 /** Render one durable workflow run with independent run and phase disclosure. */
 export function WorkflowRunPanel({ node, sessionId, useSessions, openSession, t }: WorkflowRunPanelProps) {
  const [open, setOpen] = useState(() => node.data.status === 'running')
-  const sessions = useSessions(value => value)
  const memberCount = node.data.phases.reduce((count, phase) => count + phase.members.length, 0)
-  const navigable = useMemo(() => {
-    const ordinary = new Set(sessions.ids)
-    const result = new Set<SessionId>()
-    for (const phase of node.data.phases) {
-      for (const member of phase.members) {
-        const summary = sessions.byId[member.childId]
-        if (member.status === 'running'
-          && ordinary.has(member.childId)
-          && summary?.origin === 'subagent'
-          && summary.parentId === sessionId
-          && summary.running) {
-          result.add(member.childId)
-        }
-      }
-    }
-    return result
-  }, [node.data.phases, sessionId, sessions])
+  const navigable = useSessions(
+    sessions => navigableMembers(sessions, node.data.phases, sessionId),
+    shallowEqual,
+  )
  return (
    <section className={css.root} data-workflow-run data-run-status={node.data.status}>
      <RunHeader
--- a/packages/client/ui-workflow-run/src/client/locales.ts
+++ b/packages/client/ui-workflow-run/src/client/locales.ts
@@ -6,7 +6,8 @@ export const NS = 'workflowRun'
 /** Simplified Chinese dictionary (the key-set source of truth). */
 export const zh = {
  'run.title': '{name}',
-  'run.members': '{count} 个成员',
+  'run.members.one': '{count} 个成员',
+  'run.members.other': '{count} 个成员',
  'run.empty': '没有启动成员',
  'phase.unassigned': '未分阶段',
  'phase.empty': '空阶段名',
@@ -27,7 +28,8 @@ export const zh = {
 /** English dictionary (same key set). */
 export const en: Record<WorkflowRunKey, string> = {
  'run.title': '{name}',
-  'run.members': '{count} members',
+  'run.members.one': '{count} member',
+  'run.members.other': '{count} members',
  'run.empty': 'No members started',
  'phase.unassigned': 'Unphased',
  'phase.empty': 'Empty phase name',
--- a/packages/client/ui-workflow-run/src/client/workflow-definition.ts
+++ b/packages/client/ui-workflow-run/src/client/workflow-definition.ts
@@ -80,8 +80,7 @@ function statusFromOutcome(outcome: WorkflowAgentOutcome): WorkflowRunStatus {
  }
 }

-function locationClosed(location: ConversationLocation | undefined): boolean {
-  if (location === undefined) return false
+function locationClosed(location: ConversationLocation): boolean {
  if (location.kind === 'step') {
    return location.step.status === 'closed' || location.turn.status === 'closed'
  }
@@ -90,11 +89,11 @@ function locationClosed(location: ConversationLocation | undefined): boolean {

 function projectWorkflow(
  context: ConversationNodeContext<WorkflowState>,
-): WorkflowRunChatData | undefined {
-  const state = context.state
-  if (state === undefined) return undefined
+  location: ConversationLocation,
+): WorkflowRunChatData {
+  const state = context.state as WorkflowState
  const interrupted = state.stopReason === undefined
-    && locationClosed(context.start?.location ?? context.matches[0]?.location)
+    && locationClosed(location)
  const phases = new Map<string, { phase: string | null; members: WorkflowRunMemberData[] }>()
  for (const member of state.members) {
    const phase = member.phase === undefined ? null : member.phase
@@ -177,9 +176,8 @@ export const workflowRunDefinition: ConversationNodeDefinition<WorkflowState> =
    return context.state
  },
  buildViewNode: (context, target): ChatConversationViewNode | null => {
-    if (target !== 'chat') return null
-    const data = projectWorkflow(context)
-    if (data === undefined || context.start === undefined) return null
+    if (target !== 'chat' || context.start === undefined) return null
+    const data = projectWorkflow(context, context.start.location)
    return {
      key: context.key,
      kind: 'workflow-run',
--- a/packages/client/ui-workflow-run/tests/workflow-run.spec.tsx
+++ b/packages/client/ui-workflow-run/tests/workflow-run.spec.tsx
@@ -7,7 +7,7 @@ import {
 } from '@deepseek-ai/dsh-client-runtime/client'
 import type {
  ChatConversationViewNode, ConversationEventInput, ConversationMatch, ConversationNodeDefinition,
-  ConversationViewDefinition, ConversationViewNode, SessionId, SessionListState,
+  ConversationViewDefinition, SessionId, SessionListState,
 } from '@deepseek-ai/dsh-client-runtime/client'
 import { apply as applyLocale } from '@deepseek-ai/dsh-client-locale/client'
 import { makeTranslate } from '@deepseek-ai/dsh-client-test-runtime'
@@ -517,5 +517,3 @@ describe('plugin lifecycle', () => {
    expect(registered).toEqual(['@deepseek-ai/dsh-client-ui-workflow-run'])
  })
 })
-
-void ({} as ConversationViewNode)
--- a/packages/workflow/tool-workflow/src/index.ts
+++ b/packages/workflow/tool-workflow/src/index.ts
@@ -289,6 +289,8 @@ export function apply(ctx: Context, config: Config): void {
        signal: exec.signal,
      })
      const recordsRun = exec.parent === undefined
+      // The shipped worker-thread engine publishes member events from later
+      // worker messages, after start() returns and this run record is active.
      if (recordsRun) recorder.start(parent.session, run)

      // Bridge the tool's abort signal to the run: if the parent step is aborted while the
@@ -317,8 +319,11 @@ export function apply(ctx: Context, config: Config): void {
          // Keep member listeners alive through disposal: an engine may
          // synthesize cancelled member endings while reaching quiescence.
          await run.dispose()
-          /* v8 ignore next -- WorkflowRun.result never rejects by contract, so result is assigned before finally. */
-          if (recordsRun && result !== undefined) recorder.finish(run.id, result.stopReason)
+          if (recordsRun) {
+            /* v8 ignore next -- WorkflowRun.result never rejects by contract, so result is assigned before finally. */
+            if (result === undefined) throw new Error('workflow run settled without a result')
+            recorder.finish(run.id, result.stopReason)
+          }
        } finally {
          if (recordsRun) recorder.abandon(run.id)
        }
--- a/packages/workflow/tool-workflow/src/invariant.ts
+++ b/packages/workflow/tool-workflow/src/invariant.ts
@@ -128,11 +128,6 @@ function applyEvent(trace: WorkflowTrace, event: SessionEvent, fail: InvariantFa
  }
 }

-/** Apply one cold-load or live-append candidate through the package reporter. */
-function applyChecked(trace: WorkflowTrace, event: SessionEvent, fail: InvariantFailure): void {
-  applyEvent(trace, event, fail)
-}
-
 /** Install an independent incremental fold over every attached Session. */
 const install: InvariantInstaller = Object.assign((ctx: Context, fail: InvariantFailure) => {
  const traces = new WeakMap<Session, WorkflowTrace>()
@@ -140,7 +135,7 @@ const install: InvariantInstaller = Object.assign((ctx: Context, fail: Invariant

  const seed = (session: Session): WorkflowTrace => {
    const trace: WorkflowTrace = new Map()
-    for (const event of session.events.filter(isWorkflowRecordEvent)) applyChecked(trace, event, fail)
+    for (const event of session.events.filter(isWorkflowRecordEvent)) applyEvent(trace, event, fail)
    traces.set(session, trace)
    return trace
  }
@@ -152,7 +147,7 @@ const install: InvariantInstaller = Object.assign((ctx: Context, fail: Invariant
    if (!isWorkflowRecordEvent(event)) return
    // session/event dispatch follows list() or session/created seeding.
    const trace = cloneTraceForEvent(traces.get(session) as WorkflowTrace, event, fail)
-    applyChecked(trace, event, fail)
+    applyEvent(trace, event, fail)
    staged.set(event, { session, trace })
  }, { global: true })
  ctx.on('session/event', (session, event) => {