diff --git a/app/src-tauri/src/telegram_scanner/mod.rs b/app/src-tauri/src/telegram_scanner/mod.rs
index e55e21be0..92a1ad9e3 100644
--- a/app/src-tauri/src/telegram_scanner/mod.rs
+++ b/app/src-tauri/src/telegram_scanner/mod.rs
@@ -694,7 +694,10 @@ mod tests {
assert_eq!(done.load(Ordering::SeqCst), 20, "every item must run");
let peak = max_seen.load(Ordering::SeqCst);
assert!(peak >= 2, "work should actually overlap (peak {peak})");
- assert!(peak <= 3, "concurrency must stay bounded to 3 (peak {peak})");
+ assert!(
+ peak <= 3,
+ "concurrency must stay bounded to 3 (peak {peak})"
+ );
assert_eq!(
inflight.load(Ordering::SeqCst),
0,
diff --git a/app/src/components/flows/WorkflowCopilotPanel.test.tsx b/app/src/components/flows/WorkflowCopilotPanel.test.tsx
index 6c9322c83..9a744770b 100644
--- a/app/src/components/flows/WorkflowCopilotPanel.test.tsx
+++ b/app/src/components/flows/WorkflowCopilotPanel.test.tsx
@@ -1,4 +1,4 @@
-import { act, fireEvent, render, screen } from '@testing-library/react';
+import { act, fireEvent, render, screen, waitFor } from '@testing-library/react';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import type { WorkflowGraph, WorkflowNode } from '../../lib/flows/types';
@@ -55,7 +55,7 @@ describe('WorkflowCopilotPanel', () => {
hookState.toolTimeline = [];
hookState.liveResponse = '';
hookState.error = null;
- hookState.send = vi.fn().mockResolvedValue({ proposed: false });
+ hookState.send = vi.fn().mockResolvedValue({ outcome: 'dispatched', proposed: false });
hookState.clearProposal = vi.fn();
});
@@ -358,13 +358,140 @@ describe('WorkflowCopilotPanel', () => {
expect(hookState.send).toHaveBeenCalledTimes(1);
});
+ it('reports the build seed as consumed once the turn actually dispatched', async () => {
+ const onBuildSeedConsumed = vi.fn();
+ render(
+
+ );
+ expect(hookState.send).toHaveBeenCalledTimes(1);
+ // Fires exactly once, after the dispatched build turn resolves, so the
+ // route seed can be stripped.
+ await waitFor(() => expect(onBuildSeedConsumed).toHaveBeenCalledTimes(1));
+ });
+
+ it('does not consume the seed when send no-ops (socket not connected) (#4597)', async () => {
+ // `send` resolves `outcome: 'skipped'` when the socket isn't connected —
+ // the turn never dispatched, so the seed must be preserved (not cleared) so
+ // the build can still fire once the socket connects.
+ hookState.send = vi.fn().mockResolvedValue({ outcome: 'skipped', proposed: false });
+ const onBuildSeedConsumed = vi.fn();
+ render(
+
+ );
+ expect(hookState.send).toHaveBeenCalledTimes(1);
+ // Flush the actual send() promise so the effect's `.then` runs, then assert
+ // the no-op path never consumed the seed.
+ await hookState.send.mock.results[0]?.value;
+ await waitFor(() => expect(onBuildSeedConsumed).not.toHaveBeenCalled());
+ });
+
+ it('does not consume or resend the seed when send fails (#4597)', async () => {
+ // A dispatch error resolves 'failed' (not 'skipped'): the seed is NOT
+ // consumed, and — crucially — the guard stays set so the effect does not
+ // auto-resend the turn (which would duplicate the user message and hammer
+ // the backend). The error surfaces separately for the user to retry.
+ hookState.send = vi.fn().mockResolvedValue({ outcome: 'failed', proposed: false });
+ const onBuildSeedConsumed = vi.fn();
+ const { rerender } = render(
+
+ );
+ expect(hookState.send).toHaveBeenCalledTimes(1);
+ await hookState.send.mock.results[0]?.value;
+ expect(onBuildSeedConsumed).not.toHaveBeenCalled();
+
+ // A re-render with a fresh `send` identity (as happens on any state change)
+ // must NOT resend — the guard remains set for a failed dispatch.
+ hookState.send = vi.fn().mockResolvedValue({ outcome: 'failed', proposed: false });
+ rerender(
+
+ );
+ expect(hookState.send).not.toHaveBeenCalled();
+ expect(onBuildSeedConsumed).not.toHaveBeenCalled();
+ });
+
+ it('does not re-fire the build turn when remounted after the seed is cleared (#4597)', async () => {
+ const onBuildSeedConsumed = vi.fn();
+ // First mount with the seed present (as the prompt-bar route lands).
+ const { unmount } = render(
+
+ );
+ expect(hookState.send).toHaveBeenCalledTimes(1);
+ await waitFor(() => expect(onBuildSeedConsumed).toHaveBeenCalledTimes(1));
+
+ // The host clears the route seed (buildSeed -> null) in response. Closing
+ // and reopening the copilot fully remounts the panel — the per-mount
+ // `buildSentRef` resets to false — but with no seed there is nothing to
+ // re-fire, so the build turn must NOT be sent a second time.
+ unmount();
+ render(
+
+ );
+ expect(hookState.send).toHaveBeenCalledTimes(1);
+ expect(onBuildSeedConsumed).toHaveBeenCalledTimes(1);
+ });
+
it('carries the build seed description forward when the auto-sent build turn asks a clarifying question instead of proposing', async () => {
hookState.send = vi
.fn()
- // The auto-sent build turn asks a question rather than proposing.
- .mockResolvedValueOnce({ proposed: false })
+ // The auto-sent build turn dispatches but asks a question rather than
+ // proposing.
+ .mockResolvedValueOnce({ outcome: 'dispatched', proposed: false })
// The user's free-text answer then resolves it.
- .mockResolvedValueOnce({ proposed: true });
+ .mockResolvedValueOnce({ outcome: 'dispatched', proposed: true });
render(
void;
/**
* The workflow's persisted copilot thread id (from the per-flow cache), so
* reopening the panel resumes the same conversation instead of starting fresh.
@@ -98,6 +106,7 @@ export default function WorkflowCopilotPanel({
onClose,
repairSeed = null,
buildSeed = null,
+ onBuildSeedConsumed,
seedThreadId = null,
onThreadIdChange,
}: Props) {
@@ -195,17 +204,41 @@ export default function WorkflowCopilotPanel({
const buildSentRef = useRef(false);
useEffect(() => {
if (!buildSeed || buildSentRef.current) return;
+ // Optimistically guard re-entry while the async dispatch is in flight.
buildSentRef.current = true;
send({
displayText: buildSeed.description,
request: flowId
? { mode: 'build', instruction: buildSeed.description, graph, flowId }
: { mode: 'revise', instruction: buildSeed.description, graph, flowId },
- }).then(({ proposed }) => {
- // Not proposed => the seed turn asked a clarifying question instead of
- // building. Carry the original description forward so the user's
- // free-text answer (via `submit` below) doesn't strand the agent with
- // no idea what it was asked to build.
+ }).then(({ outcome, proposed }) => {
+ if (outcome === 'dispatched') {
+ // Clear the ephemeral route seed only once the turn actually
+ // dispatched, so closing and reopening the panel (which remounts it
+ // and resets `buildSentRef`) can't re-fire the same build turn
+ // (issue #4597).
+ onBuildSeedConsumed?.();
+ } else if (outcome === 'skipped') {
+ // Retryable no-op (socket not connected yet, or a turn already in
+ // flight): keep the seed and release the guard so the effect retries
+ // once `send` changes identity on reconnect — otherwise the prompt is
+ // lost and the blank flow never auto-builds.
+ buildSentRef.current = false;
+ }
+ // `failed`: the dispatch was attempted but errored (surfaced via
+ // `error`). Leave the guard set so THIS mount doesn't auto-resend and
+ // duplicate the turn — the user retries from the input instead. Note the
+ // route seed is deliberately NOT consumed on failure, so a later close +
+ // reopen remounts the panel with a fresh `buildSentRef` and WILL re-fire
+ // the build: a reopen is thus an intentional retry, not a manual-only one.
+ // That's safe/desired — a failed turn persisted nothing (`mode:'build'`
+ // never saves; issue #4596), so there's no partial state to clean up.
+ //
+ // Regardless of outcome, record whether the turn proposed: when it didn't
+ // (a clarifying question, or a no-op/failed turn that never built), carry
+ // the original description forward so the user's free-text answer (via
+ // `submit` below) doesn't strand the agent with no idea what it was asked
+ // to build. A later turn that proposes clears it.
updatePendingAsk(proposed, buildSeed.description);
});
// `graph`/`flowId` are read once for the seed turn — later edits must not
diff --git a/app/src/hooks/useWorkflowBuilderChat.test.ts b/app/src/hooks/useWorkflowBuilderChat.test.ts
index e0be52332..4b8e0c5c0 100644
--- a/app/src/hooks/useWorkflowBuilderChat.test.ts
+++ b/app/src/hooks/useWorkflowBuilderChat.test.ts
@@ -4,14 +4,16 @@ import { beforeEach, describe, expect, it, vi } from 'vitest';
import type { BuilderTurnResult } from '../services/api/flowsApi';
import type { WorkflowProposal } from '../store/chatRuntimeSlice';
import type { ThreadMessage } from '../types/thread';
-import { useWorkflowBuilderChat } from './useWorkflowBuilderChat';
+import { useWorkflowBuilderChat, type WorkflowBuilderSendResult } from './useWorkflowBuilderChat';
// The hook now runs the builder server-side via `openhuman.flows_build`.
const buildWorkflow = vi.hoisted(() => vi.fn());
vi.mock('../services/api/flowsApi', () => ({ buildWorkflow }));
-// Socket is always "connected" for these tests.
-vi.mock('../store/socketSelectors', () => ({ selectSocketStatus: () => 'connected' }));
+// Socket status is configurable per test (reset to 'connected' in beforeEach)
+// so the no-op/`skipped` path (socket not connected) can be exercised.
+const socketStatus = vi.hoisted(() => ({ current: 'connected' as string }));
+vi.mock('../store/socketSelectors', () => ({ selectSocketStatus: () => socketStatus.current }));
const dispatch = vi.hoisted(() => vi.fn());
const selectorState = vi.hoisted(() => ({
@@ -69,6 +71,7 @@ const okResult = (over: Partial = {}): BuilderTurnResult => (
describe('useWorkflowBuilderChat', () => {
beforeEach(() => {
buildWorkflow.mockReset().mockResolvedValue(okResult());
+ socketStatus.current = 'connected';
selectorState.proposals = {};
selectorState.messagesByThreadId = {};
selectorState.toolTimelineByThread = {};
@@ -280,6 +283,60 @@ describe('useWorkflowBuilderChat', () => {
await waitFor(() => expect(result.current.error).toBe('run failed'));
});
+ // The `send()` outcome contract (`dispatched` | `skipped` | `failed`) is the
+ // heart of the build-seed fix (PR #4628): a caller retries a seeded auto-send
+ // only on `skipped`, never on `failed` (which would resend and duplicate the
+ // turn). Assert it here at its SOURCE — the panel tests mock this hook, so
+ // without these a `failed` misreported as `skipped` would pass every test.
+ describe('send() outcome contract', () => {
+ it("returns 'dispatched' when the turn actually runs", async () => {
+ const { result } = renderHook(() => useWorkflowBuilderChat());
+ let outcome: WorkflowBuilderSendResult | undefined;
+ await act(async () => {
+ outcome = await result.current.send({
+ displayText: 'hi',
+ request: { mode: 'create', instruction: 'x' },
+ });
+ });
+ expect(outcome?.outcome).toBe('dispatched');
+ expect(buildWorkflow).toHaveBeenCalledTimes(1);
+ });
+
+ it("returns 'failed' (not 'skipped') when the dispatch throws", async () => {
+ // A thrown dispatch is a real error, distinct from the retryable
+ // `skipped` no-op — misreporting it as `skipped` would re-arm the seeded
+ // build effect and loop duplicate turns (see WorkflowCopilotPanel's
+ // `buildSentRef`). The error is also surfaced via `error`.
+ buildWorkflow.mockRejectedValueOnce(new Error('rpc boom'));
+ const { result } = renderHook(() => useWorkflowBuilderChat());
+ let outcome: WorkflowBuilderSendResult | undefined;
+ await act(async () => {
+ outcome = await result.current.send({
+ displayText: 'hi',
+ request: { mode: 'create', instruction: 'x' },
+ });
+ });
+ expect(outcome?.outcome).toBe('failed');
+ expect(result.current.error).toBe('rpc boom');
+ });
+
+ it("returns 'skipped' without dispatching when the socket is not connected", async () => {
+ socketStatus.current = 'connecting';
+ const { result } = renderHook(() => useWorkflowBuilderChat());
+ let outcome: WorkflowBuilderSendResult | undefined;
+ await act(async () => {
+ outcome = await result.current.send({
+ displayText: 'hi',
+ request: { mode: 'create', instruction: 'x' },
+ });
+ });
+ expect(outcome?.outcome).toBe('skipped');
+ // A retryable no-op: nothing was sent and no thread was created.
+ expect(buildWorkflow).not.toHaveBeenCalled();
+ await waitFor(() => expect(result.current.error).toBe('offline'));
+ });
+ });
+
describe('displayMessages', () => {
it('excludes isInterim agent messages but keeps user + terminal agent messages (incl. a clarifying question)', () => {
selectorState.messagesByThreadId = {
diff --git a/app/src/hooks/useWorkflowBuilderChat.ts b/app/src/hooks/useWorkflowBuilderChat.ts
index 1a290b8ea..4e0c4e86b 100644
--- a/app/src/hooks/useWorkflowBuilderChat.ts
+++ b/app/src/hooks/useWorkflowBuilderChat.ts
@@ -59,6 +59,35 @@ export interface WorkflowBuilderSendParams {
request: BuilderTurnRequest;
}
+/**
+ * Outcome of a {@link UseWorkflowBuilderChat.send} call:
+ * - `dispatched` — the turn actually ran (thread created + `flows_build` sent).
+ * - `skipped` — a retryable no-op: the socket wasn't connected, or a turn was
+ * already in flight. Nothing was sent; a caller may retry later.
+ * - `failed` — the dispatch was attempted but threw (thread create / RPC
+ * error). The error is surfaced via `error`; a caller must NOT auto-retry, or
+ * it would resend and duplicate the turn.
+ */
+export type WorkflowBuilderSendOutcome = 'dispatched' | 'skipped' | 'failed';
+
+/**
+ * Result of a {@link UseWorkflowBuilderChat.send} call. Carries two orthogonal
+ * signals a caller may need:
+ * - `outcome` — whether/why the turn dispatched (see {@link
+ * WorkflowBuilderSendOutcome}). Drives a seeded auto-send's retry decision: it
+ * retries only on `skipped`, never on `failed` (which would duplicate the
+ * turn) or `dispatched`.
+ * - `proposed` — `true` iff this turn's `flows_build` call returned a proposal;
+ * `false` for a clarifying question, an error, or a call that never ran.
+ * Callers looping a conversation (the copilot's free-text follow-ups) use this
+ * to know whether the turn's instruction is still "unresolved" and must be
+ * carried into the next turn — see `WorkflowCopilotPanel`'s `pendingAskRef`.
+ */
+export interface WorkflowBuilderSendResult {
+ outcome: WorkflowBuilderSendOutcome;
+ proposed: boolean;
+}
+
export interface UseWorkflowBuilderChat {
/** The dedicated thread id, or `null` before the first send creates it. */
threadId: string | null;
@@ -101,14 +130,15 @@ export interface UseWorkflowBuilderChat {
error: string | null;
/**
* Send a builder turn, creating the dedicated thread on first use. Resolves
- * with `proposed: true` iff this turn's `flows_build` call returned a
- * proposal — `false` for a clarifying question, an error, or a call that
- * never ran (already sending / offline). Callers that loop a conversation
- * (the copilot's free-text follow-ups) use this to know whether the turn's
- * instruction is still "unresolved" and must be carried into the next turn
- * — see `WorkflowCopilotPanel`'s `pendingAskRef`.
+ * to a {@link WorkflowBuilderSendResult}: `outcome` lets callers tell a
+ * retryable no-op (`skipped`) apart from a real dispatch (`dispatched`) or an
+ * error (`failed`) — a seeded auto-send retries only on `skipped`, never on
+ * `failed`, so a dispatch error can't loop into duplicate turns — while
+ * `proposed` reports whether this turn's `flows_build` call returned a
+ * proposal (vs. a clarifying question / error / no-op), so a looping caller
+ * knows whether the instruction is still unresolved.
*/
- send: (params: WorkflowBuilderSendParams) => Promise<{ proposed: boolean }>;
+ send: (params: WorkflowBuilderSendParams) => Promise;
/** Clear the current proposal (e.g. after Accept/Reject) without persisting. */
clearProposal: () => void;
}
@@ -239,15 +269,18 @@ export function useWorkflowBuilderChat(seedThreadId?: string | null): UseWorkflo
const sending = localSending;
const send = useCallback(
- async ({ displayText, request }: WorkflowBuilderSendParams) => {
+ async ({
+ displayText,
+ request,
+ }: WorkflowBuilderSendParams): Promise => {
if (localSending) {
log('send: ignored — a turn is already dispatching');
- return { proposed: false };
+ return { outcome: 'skipped', proposed: false };
}
if (socketStatus !== 'connected') {
log('send: blocked — socket not connected (%s)', socketStatus);
setError('offline');
- return { proposed: false };
+ return { outcome: 'skipped', proposed: false };
}
setLocalSending(true);
setError(null);
@@ -333,6 +366,7 @@ export function useWorkflowBuilderChat(seedThreadId?: string | null): UseWorkflo
dispatch(addMessageLocal({ threadId: targetThreadId, message: assistantMessage }));
}
}
+ return { outcome: 'dispatched', proposed };
} catch (err) {
const msg = err instanceof Error ? err.message : String(err);
log('send: failed err=%o', err);
@@ -351,10 +385,12 @@ export function useWorkflowBuilderChat(seedThreadId?: string | null): UseWorkflo
);
setThreadId(null);
}
+ // A dispatch error must NOT auto-retry (it would resend and duplicate
+ // the turn) — `failed` is distinct from the retryable `skipped`.
+ return { outcome: 'failed', proposed };
} finally {
setLocalSending(false);
}
- return { proposed };
},
[dispatch, localSending, socketStatus, threadId]
);
diff --git a/app/src/pages/FlowCanvasPage.tsx b/app/src/pages/FlowCanvasPage.tsx
index a95d25651..0752378ef 100644
--- a/app/src/pages/FlowCanvasPage.tsx
+++ b/app/src/pages/FlowCanvasPage.tsx
@@ -137,11 +137,14 @@ function FlowEditor({
editorFlow,
initialCopilotSeed = null,
initialBuildSeed = null,
+ onBuildSeedConsumed,
}: {
editorFlow: EditorFlow;
initialCopilotSeed?: CopilotRepairSeed | null;
/** Prompt-bar instant-create seed: open the copilot already building this. */
initialBuildSeed?: CopilotBuildSeed | null;
+ /** Clear the route's build seed once the copilot has dispatched it (#4597). */
+ onBuildSeedConsumed?: () => void;
}) {
const { t } = useT();
const navigate = useNavigate();
@@ -539,6 +542,7 @@ function FlowEditor({
onClose={() => setCopilotOpen(false)}
repairSeed={copilotRepairSeed}
buildSeed={initialBuildSeed}
+ onBuildSeedConsumed={onBuildSeedConsumed}
seedThreadId={copilotThreadId}
onThreadIdChange={handleCopilotThreadId}
/>
@@ -561,6 +565,28 @@ export default function FlowCanvasPage() {
// seed so the copilot opens already building the described workflow.
const buildSeed = useMemo(() => asCopilotBuildSeed(location.state), [location.state]);
+ // Strip the ephemeral build seed from `location.state` once the copilot has
+ // dispatched it. The panel's own `buildSentRef` guard is per-mount, so
+ // closing + reopening the copilot remounts it and would otherwise re-fire the
+ // same `build` turn against the still-present route seed (issue #4597).
+ // Preserve any other state fields (e.g. a repair seed) — only drop
+ // `copilotBuild`.
+ const clearBuildSeed = useCallback(() => {
+ // Named `routeState` (not `state`) to avoid shadowing the component-level
+ // `state` from `useState` that drives this file's render switch.
+ const routeState = location.state;
+ if (!routeState || typeof routeState !== 'object' || !('copilotBuild' in routeState)) return;
+ const next = { ...(routeState as Record) };
+ delete next.copilotBuild;
+ log('build seed consumed — clearing route state: id=%s', id);
+ // Navigate with an object (not a bare pathname) so the current search and
+ // hash are preserved — a string target would drop them.
+ navigate(
+ { pathname: location.pathname, search: location.search, hash: location.hash },
+ { replace: true, state: next }
+ );
+ }, [id, location.state, location.pathname, location.search, location.hash, navigate]);
+
useEffect(() => {
// Guards a stale response from clobbering newer state: this effect
// re-runs on every `:id` change without the component remounting (same
@@ -620,6 +646,7 @@ export default function FlowCanvasPage() {
}}
initialCopilotSeed={copilotSeed}
initialBuildSeed={buildSeed}
+ onBuildSeedConsumed={clearBuildSeed}
/>
);
}
diff --git a/app/src/pages/__tests__/FlowCanvasPage.test.tsx b/app/src/pages/__tests__/FlowCanvasPage.test.tsx
index 3b46338dc..bb157b286 100644
--- a/app/src/pages/__tests__/FlowCanvasPage.test.tsx
+++ b/app/src/pages/__tests__/FlowCanvasPage.test.tsx
@@ -5,7 +5,7 @@
* and the Phase 3d host wiring: Save persists via `flows_update`, and the
* unsaved-changes guard intercepts the Back button while dirty.
*/
-import { fireEvent, render, screen, waitFor } from '@testing-library/react';
+import { act, fireEvent, render, screen, waitFor } from '@testing-library/react';
import { createMemoryRouter, MemoryRouter, Route, RouterProvider, Routes } from 'react-router-dom';
import { beforeEach, describe, expect, it, vi } from 'vitest';
@@ -328,6 +328,45 @@ describe('FlowCanvasPage copilot build seed (prompt-bar instant create)', () =>
expect(copilotPanelProps.current?.flowId).toBe('test-id');
});
+ it('clears only the build seed on consume, preserving sibling route state (#4597)', async () => {
+ render(
+
+
+ } />
+
+
+ );
+
+ await waitFor(() =>
+ expect(copilotPanelProps.current?.buildSeed).toEqual({ description: 'digest it' })
+ );
+ // The sibling repair seed is present before consumption.
+ expect(copilotPanelProps.current?.repairSeed).toMatchObject({ runId: 'run-1' });
+
+ // The panel dispatched the build turn and reports it consumed; the host must
+ // strip `copilotBuild` from `location.state` so a later remount (close +
+ // reopen) has no seed left to re-fire.
+ act(() => {
+ (copilotPanelProps.current?.onBuildSeedConsumed as () => void)();
+ });
+
+ await waitFor(() => expect(copilotPanelProps.current?.buildSeed).toBeNull());
+ // ...but the sibling repair seed must NOT be nuked along with it.
+ expect(copilotPanelProps.current?.repairSeed).toMatchObject({ runId: 'run-1' });
+ });
+
it('keeps the copilot closed without a seed', async () => {
render(