From f7468905b2efed243588e4a21535a2fad741ff14 Mon Sep 17 00:00:00 2001 From: Cyrus Gray <144336577+graycyrus@users.noreply.github.com> Date: Tue, 14 Jul 2026 21:58:49 +0530 Subject: [PATCH] fix(flows): builder turn-budget wiring + prompt guidance + capped UX (B31-B34) (#4865) Co-authored-by: M3gA-Mind --- .../flows/WorkflowCopilotPanel.test.tsx | 94 +++++++++++++++++++ .../components/flows/WorkflowCopilotPanel.tsx | 53 +++++++++++ app/src/hooks/useWorkflowBuilderChat.test.ts | 57 +++++++++++ app/src/hooks/useWorkflowBuilderChat.ts | 22 +++++ app/src/lib/i18n/ar.ts | 3 + app/src/lib/i18n/bn.ts | 3 + app/src/lib/i18n/de.ts | 3 + app/src/lib/i18n/en.ts | 3 + app/src/lib/i18n/es.ts | 3 + app/src/lib/i18n/fr.ts | 3 + app/src/lib/i18n/hi.ts | 3 + app/src/lib/i18n/id.ts | 3 + app/src/lib/i18n/it.ts | 3 + app/src/lib/i18n/ko.ts | 3 + app/src/lib/i18n/pl.ts | 3 + app/src/lib/i18n/pt.ts | 3 + app/src/lib/i18n/ru.ts | 3 + app/src/lib/i18n/zh-CN.ts | 3 + app/src/services/api/flowsApi.test.ts | 4 +- app/src/services/api/flowsApi.ts | 20 +++- .../agent/harness/session/builder/setters.rs | 1 + .../agent/harness/session/runtime.rs | 9 ++ .../agent/harness/session/turn/core.rs | 6 ++ src/openhuman/agent/harness/session/types.rs | 10 ++ .../flows/agents/workflow_builder/prompt.md | 82 ++++++++++++++-- src/openhuman/flows/ops.rs | 87 ++++++++++++++++- src/openhuman/flows/ops_tests.rs | 45 +++++++++ 27 files changed, 516 insertions(+), 16 deletions(-) diff --git a/app/src/components/flows/WorkflowCopilotPanel.test.tsx b/app/src/components/flows/WorkflowCopilotPanel.test.tsx index 705f342c5..0fe3fdea7 100644 --- a/app/src/components/flows/WorkflowCopilotPanel.test.tsx +++ b/app/src/components/flows/WorkflowCopilotPanel.test.tsx @@ -17,6 +17,7 @@ interface MockMessage { const hookState = vi.hoisted(() => ({ sending: false, proposal: null as WorkflowProposal | null, + capped: false, // Panel renders `displayMessages` (already interim-filtered upstream by // `useWorkflowBuilderChat`) — kept separate from `messages` in these tests // so a mismatch between the two proves the panel is reading the right field. @@ -51,6 +52,7 @@ describe('WorkflowCopilotPanel', () => { beforeEach(() => { hookState.sending = false; hookState.proposal = null; + hookState.capped = false; hookState.displayMessages = []; hookState.toolTimeline = []; hookState.liveResponse = ''; @@ -335,6 +337,98 @@ describe('WorkflowCopilotPanel', () => { expect(hookState.clearProposal).toHaveBeenCalledTimes(1); }); + it('B34: renders a "Continue building" card when the turn hit the iteration cap', () => { + hookState.capped = true; + render( + + ); + expect(screen.getByTestId('workflow-copilot-capped')).toBeInTheDocument(); + expect(screen.getByTestId('workflow-copilot-continue')).toBeInTheDocument(); + }); + + it('B34: does NOT render the capped card for a normal (non-capped) turn', () => { + hookState.capped = false; + render( + + ); + expect(screen.queryByTestId('workflow-copilot-capped')).not.toBeInTheDocument(); + }); + + it('B34: does not render the capped card while a proposal is pending, even if capped is stale-true', () => { + // Defense-in-depth: the server already scopes `capped` to `proposal === + // null`, but the panel re-checks `!proposal` itself too (see the JSX + // condition) in case a stale `capped=true` from a prior turn outlives a + // later turn's proposal. + hookState.capped = true; + hookState.proposal = proposalWith(['a', 'c']); + render( + + ); + expect(screen.queryByTestId('workflow-copilot-capped')).not.toBeInTheDocument(); + }); + + it('B34: clicking "Continue building" sends a follow-up turn', async () => { + hookState.capped = true; + render( + + ); + fireEvent.click(screen.getByTestId('workflow-copilot-continue')); + await waitFor(() => expect(hookState.send).toHaveBeenCalledTimes(1)); + const arg = hookState.send.mock.calls[0][0]; + expect(arg.request.mode).toBe('revise'); + expect(arg.request.graph).toEqual(baseGraph); + }); + + // Codex review on #4865: "Continue building" must resume ON the current + // draft — a `revise` turn over the EXISTING `flowId`, never a blank/`create` + // restart — since `flows_build` spins up a fresh `workflow_builder` agent + // per RPC with no server-side session/checkpoint to resume. Carrying the + // live `graph` + `flowId` is what makes "Continue" a correct, working + // continuation instead of an empty restart. + it('B34: "Continue building" carries the current flowId, not a blank restart', async () => { + hookState.capped = true; + render( + + ); + fireEvent.click(screen.getByTestId('workflow-copilot-continue')); + await waitFor(() => expect(hookState.send).toHaveBeenCalledTimes(1)); + const arg = hookState.send.mock.calls[0][0]; + expect(arg.request.mode).toBe('revise'); + expect(arg.request.flowId).toBe('flow-123'); + expect(arg.request.graph).toEqual(baseGraph); + }); + it('auto-sends a repair turn once when opened with a repair seed', () => { render( { + void submit(t('flows.copilot.continueBuilding')); + }, [submit, t]); + const handleInputKeyDown = useCallback( (event: React.KeyboardEvent) => { if (event.key === 'Enter' && !event.shiftKey && !isComposingTextRef.current) { @@ -513,6 +539,33 @@ export default function WorkflowCopilotPanel({ )} + + {/* (B34) The turn hit the agent's iteration limit with no proposal + yet — distinguish this from a voluntary clarifying question (which + renders as a plain agent bubble above, no card) with an explicit + "reached its iteration limit" signal and a one-click resume that + continues building from the current draft (see `continueBuilding` + above for why this is accurate rather than a seamless resume). + Never shown alongside `sending` (a fresh turn already cleared + `capped`) or a proposal (mutually exclusive server-side — see + `ops.rs`). */} + {capped && !sending && !proposal && ( +
+

{t('flows.copilot.cappedNotice')}

+
+ +
+
+ )}
diff --git a/app/src/hooks/useWorkflowBuilderChat.test.ts b/app/src/hooks/useWorkflowBuilderChat.test.ts index b865b1d98..20cf47c07 100644 --- a/app/src/hooks/useWorkflowBuilderChat.test.ts +++ b/app/src/hooks/useWorkflowBuilderChat.test.ts @@ -56,6 +56,7 @@ const okResult = (over: Partial = {}): BuilderTurnResult => ( proposal: null, assistantText: 'done', error: null, + capped: false, ...over, }); @@ -129,6 +130,62 @@ describe('useWorkflowBuilderChat', () => { ); }); + it('B34: surfaces `capped` when the turn hit the iteration cap with no proposal', async () => { + buildWorkflow.mockResolvedValue(okResult({ capped: true, proposal: null })); + const { result } = renderHook(() => useWorkflowBuilderChat()); + expect(result.current.capped).toBe(false); + await act(async () => { + await result.current.send({ + displayText: 'build me something complex', + request: { mode: 'create', instruction: 'x' }, + }); + }); + expect(result.current.capped).toBe(true); + }); + + it('B34: does not surface `capped` for a normal turn (not capped)', async () => { + buildWorkflow.mockResolvedValue(okResult({ capped: false })); + const { result } = renderHook(() => useWorkflowBuilderChat()); + await act(async () => { + await result.current.send({ + displayText: 'hi', + request: { mode: 'create', instruction: 'x' }, + }); + }); + expect(result.current.capped).toBe(false); + }); + + it('B34: resets a stale `capped=true` at the start of a new turn even if the server sends capped again with a proposal', async () => { + // First turn: capped, no proposal. + buildWorkflow.mockResolvedValueOnce(okResult({ capped: true, proposal: null })); + const { result } = renderHook(() => useWorkflowBuilderChat()); + await act(async () => { + await result.current.send({ + displayText: 'build me something complex', + request: { mode: 'create', instruction: 'x' }, + }); + }); + expect(result.current.capped).toBe(true); + + // Second turn resolves with a proposal — `capped` must clear even if the + // (malformed/stale) server payload still says `capped: true`, since the + // hook re-checks `!result.proposal` itself, not just at the top of send(). + const proposal: WorkflowProposal = { + name: 'Digest', + graph: { nodes: [], edges: [] }, + requireApproval: true, + summary: { trigger: 'schedule', steps: [] }, + }; + buildWorkflow.mockResolvedValueOnce(okResult({ capped: true, proposal })); + await act(async () => { + await result.current.send({ + displayText: 'continue', + request: { mode: 'revise', instruction: 'continue' }, + }); + }); + expect(result.current.capped).toBe(false); + }); + it('appends the user turn locally but never the agent reply — onDone is the single authoritative path (B26)', async () => { buildWorkflow.mockResolvedValue(okResult({ assistantText: 'Here is your workflow.' })); const { result } = renderHook(() => useWorkflowBuilderChat()); diff --git a/app/src/hooks/useWorkflowBuilderChat.ts b/app/src/hooks/useWorkflowBuilderChat.ts index 8a7a4af3d..fa2710243 100644 --- a/app/src/hooks/useWorkflowBuilderChat.ts +++ b/app/src/hooks/useWorkflowBuilderChat.ts @@ -96,6 +96,16 @@ export interface UseWorkflowBuilderChat { sending: boolean; /** The latest proposal the agent returned on this thread, or `null`. */ proposal: WorkflowProposal | null; + /** + * `true` when the most recently settled turn paused because it hit the + * agent's tool-call budget with no proposal yet (B34) — the caller should + * render a "Continue building" affordance instead of treating + * `displayMessages`' latest agent bubble (the raw "Done so far / Next + * steps" checkpoint) as a normal reply or a clarifying question. Reset to + * `false` at the start of every new `send()` call, so it only ever + * reflects the most recent turn. + */ + capped: boolean; /** * The dedicated thread's FULL transcript (user + agent turns, including * between-tool narration bubbles), so a caller that needs the complete @@ -164,6 +174,7 @@ export function useWorkflowBuilderChat(seedThreadId?: string | null): UseWorkflo const [threadId, setThreadId] = useState(seedThreadId ?? null); const [localSending, setLocalSending] = useState(false); const [error, setError] = useState(null); + const [capped, setCapped] = useState(false); // Tracks a thread id this hook created itself via `send()`'s `createNewThread` // call — as opposed to one that arrived from `seedThreadId` because a caller // (e.g. `WorkflowCopilotPanel`) reports every `threadId` change back up via @@ -295,6 +306,10 @@ export function useWorkflowBuilderChat(seedThreadId?: string | null): UseWorkflo } setLocalSending(true); setError(null); + // A fresh turn supersedes any prior cap-hit signal, same as the + // proposal-clearing dispatch below — `capped` must only ever reflect + // this turn, not a stale one. + setCapped(false); let targetThreadId = threadId; let proposed = false; try { @@ -344,6 +359,12 @@ export function useWorkflowBuilderChat(seedThreadId?: string | null): UseWorkflo } else if (result.error) { setError(result.error); } + // (B34) Surface the cap-hit signal so the panel can render a + // "Continue building" card instead of the raw checkpoint text as a + // normal reply. Scoped to `!result.proposal` server-side already + // (`ops.rs`'s `capped` field), but re-checked here too — a proposal + // means there's nothing left to "continue". + setCapped(result.capped && !result.proposal); // Note: no local fallback append for `result.assistantText` here (B26). // `ChatRuntimeProvider.onDone` is the SINGLE authoritative path that // persists the assistant's reply on the turn's `chat_done` event — the @@ -392,6 +413,7 @@ export function useWorkflowBuilderChat(seedThreadId?: string | null): UseWorkflo threadId, sending, proposal, + capped, messages, displayMessages, toolTimeline, diff --git a/app/src/lib/i18n/ar.ts b/app/src/lib/i18n/ar.ts index 5d7bafd3b..eafa4d199 100644 --- a/app/src/lib/i18n/ar.ts +++ b/app/src/lib/i18n/ar.ts @@ -4002,6 +4002,9 @@ const messages: TranslationMap = { 'flows.copilot.tool.dryRunning': 'تجربة تشغيل سير العمل…', 'flows.copilot.tool.saving': 'حفظ سير العمل…', 'flows.copilot.tool.usingTools': 'استخدام الأدوات…', + 'flows.copilot.cappedNotice': + 'بلغ المُنشئ الحد الأقصى لعدد التكرارات قبل الانتهاء من سير العمل هذا. عند المتابعة، سيستمر البناء انطلاقًا من المسودة الحالية.', + 'flows.copilot.continueBuilding': 'متابعة الإنشاء', 'flows.list.view': 'عرض سير العمل', 'flows.list.export': 'تصدير', 'flows.list.exported': 'تم تصدير سير العمل', diff --git a/app/src/lib/i18n/bn.ts b/app/src/lib/i18n/bn.ts index 75f87bd5d..dddbcfc08 100644 --- a/app/src/lib/i18n/bn.ts +++ b/app/src/lib/i18n/bn.ts @@ -4100,6 +4100,9 @@ const messages: TranslationMap = { 'flows.copilot.tool.dryRunning': 'ওয়ার্কফ্লো ড্রাই-রান করা হচ্ছে…', 'flows.copilot.tool.saving': 'ওয়ার্কফ্লো সংরক্ষণ করা হচ্ছে…', 'flows.copilot.tool.usingTools': 'টুল ব্যবহার করা হচ্ছে…', + 'flows.copilot.cappedNotice': + 'এই ওয়ার্কফ্লো শেষ করার আগেই বিল্ডার তার পুনরাবৃত্তির সীমায় পৌঁছে গেছে। চালিয়ে গেলে এটি বর্তমান খসড়া থেকে নির্মাণ চালিয়ে যাবে।', + 'flows.copilot.continueBuilding': 'নির্মাণ চালিয়ে যান', 'flows.list.view': 'ওয়ার্কফ্লো দেখুন', 'flows.list.export': 'রপ্তানি', 'flows.list.exported': 'ওয়ার্কফ্লো রপ্তানি হয়েছে', diff --git a/app/src/lib/i18n/de.ts b/app/src/lib/i18n/de.ts index 886236acd..f63dec255 100644 --- a/app/src/lib/i18n/de.ts +++ b/app/src/lib/i18n/de.ts @@ -4218,6 +4218,9 @@ const messages: TranslationMap = { 'flows.copilot.tool.dryRunning': 'Workflow wird testweise ausgeführt…', 'flows.copilot.tool.saving': 'Workflow wird gespeichert…', 'flows.copilot.tool.usingTools': 'Werkzeuge werden verwendet…', + 'flows.copilot.cappedNotice': + 'Der Builder hat sein Iterationslimit erreicht, bevor dieser Workflow fertiggestellt wurde. Beim Fortsetzen wird auf Basis des aktuellen Entwurfs weitergebaut.', + 'flows.copilot.continueBuilding': 'Weiter erstellen', 'flows.list.view': 'Workflow anzeigen', 'flows.list.export': 'Exportieren', 'flows.list.exported': 'Workflow exportiert', diff --git a/app/src/lib/i18n/en.ts b/app/src/lib/i18n/en.ts index 9b3a66d41..398d224f3 100644 --- a/app/src/lib/i18n/en.ts +++ b/app/src/lib/i18n/en.ts @@ -4787,6 +4787,9 @@ const en: TranslationMap = { 'flows.copilot.tool.dryRunning': 'Dry-running workflow…', 'flows.copilot.tool.saving': 'Saving workflow…', 'flows.copilot.tool.usingTools': 'Using tools…', + 'flows.copilot.cappedNotice': + 'The builder reached its iteration limit before finishing this workflow. Continuing will keep building from the current draft.', + 'flows.copilot.continueBuilding': 'Continue building', // ── Workflow Canvas (issue B5b.1) — the read-only graph view of a saved // flow at /flows/:id. `flows.nodeKind.*` labels the 12 tinyflows node diff --git a/app/src/lib/i18n/es.ts b/app/src/lib/i18n/es.ts index d9e981ff3..61cc25687 100644 --- a/app/src/lib/i18n/es.ts +++ b/app/src/lib/i18n/es.ts @@ -4172,6 +4172,9 @@ const messages: TranslationMap = { 'flows.copilot.tool.dryRunning': 'Ejecutando prueba del flujo de trabajo…', 'flows.copilot.tool.saving': 'Guardando flujo de trabajo…', 'flows.copilot.tool.usingTools': 'Usando herramientas…', + 'flows.copilot.cappedNotice': + 'El generador alcanzó su límite de iteraciones antes de terminar este flujo de trabajo. Al continuar, seguirá construyendo a partir del borrador actual.', + 'flows.copilot.continueBuilding': 'Continuar creando', 'flows.list.view': 'Ver flujo de trabajo', 'flows.list.export': 'Exportar', 'flows.list.exported': 'Flujo de trabajo exportado', diff --git a/app/src/lib/i18n/fr.ts b/app/src/lib/i18n/fr.ts index 3daf29542..7d555c866 100644 --- a/app/src/lib/i18n/fr.ts +++ b/app/src/lib/i18n/fr.ts @@ -4200,6 +4200,9 @@ const messages: TranslationMap = { 'flows.copilot.tool.dryRunning': 'Exécution d’essai du workflow…', 'flows.copilot.tool.saving': 'Enregistrement du workflow…', 'flows.copilot.tool.usingTools': 'Utilisation d’outils…', + 'flows.copilot.cappedNotice': + 'Le générateur a atteint sa limite d’itérations avant de terminer ce workflow. En continuant, la construction reprendra à partir du brouillon actuel.', + 'flows.copilot.continueBuilding': 'Continuer la création', 'flows.list.view': 'Voir le workflow', 'flows.list.export': 'Exporter', 'flows.list.exported': 'Workflow exporté', diff --git a/app/src/lib/i18n/hi.ts b/app/src/lib/i18n/hi.ts index dd7fe5d90..eee3940a1 100644 --- a/app/src/lib/i18n/hi.ts +++ b/app/src/lib/i18n/hi.ts @@ -4098,6 +4098,9 @@ const messages: TranslationMap = { 'flows.copilot.tool.dryRunning': 'वर्कफ़्लो का ड्राई-रन किया जा रहा है…', 'flows.copilot.tool.saving': 'वर्कफ़्लो सहेजा जा रहा है…', 'flows.copilot.tool.usingTools': 'टूल का उपयोग किया जा रहा है…', + 'flows.copilot.cappedNotice': + 'यह वर्कफ़्लो पूरा होने से पहले बिल्डर अपनी पुनरावृत्ति सीमा तक पहुँच गया। जारी रखने पर यह मौजूदा ड्राफ़्ट से आगे निर्माण करेगा।', + 'flows.copilot.continueBuilding': 'निर्माण जारी रखें', 'flows.list.view': 'वर्कफ़्लो देखें', 'flows.list.export': 'निर्यात', 'flows.list.exported': 'वर्कफ़्लो निर्यात किया गया', diff --git a/app/src/lib/i18n/id.ts b/app/src/lib/i18n/id.ts index 4052a84d2..335f81623 100644 --- a/app/src/lib/i18n/id.ts +++ b/app/src/lib/i18n/id.ts @@ -4115,6 +4115,9 @@ const messages: TranslationMap = { 'flows.copilot.tool.dryRunning': 'Menjalankan uji coba alur kerja…', 'flows.copilot.tool.saving': 'Menyimpan alur kerja…', 'flows.copilot.tool.usingTools': 'Menggunakan alat…', + 'flows.copilot.cappedNotice': + 'Builder mencapai batas iterasinya sebelum menyelesaikan alur kerja ini. Jika dilanjutkan, builder akan terus membangun dari draf saat ini.', + 'flows.copilot.continueBuilding': 'Lanjutkan membangun', 'flows.list.view': 'Lihat alur kerja', 'flows.list.export': 'Ekspor', 'flows.list.exported': 'Alur kerja diekspor', diff --git a/app/src/lib/i18n/it.ts b/app/src/lib/i18n/it.ts index 06828119a..4c339bea3 100644 --- a/app/src/lib/i18n/it.ts +++ b/app/src/lib/i18n/it.ts @@ -4169,6 +4169,9 @@ const messages: TranslationMap = { 'flows.copilot.tool.dryRunning': 'Esecuzione di prova del flusso di lavoro…', 'flows.copilot.tool.saving': 'Salvataggio del flusso di lavoro…', 'flows.copilot.tool.usingTools': 'Utilizzo degli strumenti…', + 'flows.copilot.cappedNotice': + 'Il builder ha raggiunto il limite di iterazioni prima di completare questo flusso di lavoro. Continuando, la costruzione riprenderà dalla bozza attuale.', + 'flows.copilot.continueBuilding': 'Continua a costruire', 'flows.list.view': 'Visualizza flusso di lavoro', 'flows.list.export': 'Esporta', 'flows.list.exported': 'Flusso di lavoro esportato', diff --git a/app/src/lib/i18n/ko.ts b/app/src/lib/i18n/ko.ts index fed614336..bc2c5e49e 100644 --- a/app/src/lib/i18n/ko.ts +++ b/app/src/lib/i18n/ko.ts @@ -4055,6 +4055,9 @@ const messages: TranslationMap = { 'flows.copilot.tool.dryRunning': '워크플로 시험 실행 중…', 'flows.copilot.tool.saving': '워크플로 저장 중…', 'flows.copilot.tool.usingTools': '도구 사용 중…', + 'flows.copilot.cappedNotice': + '빌더가 이 워크플로를 완료하기 전에 반복 한도에 도달했습니다. 계속하면 현재 초안을 기반으로 빌드를 이어갑니다.', + 'flows.copilot.continueBuilding': '계속 빌드하기', 'flows.list.view': '워크플로 보기', 'flows.list.export': '내보내기', 'flows.list.exported': '워크플로를 내보냈습니다', diff --git a/app/src/lib/i18n/pl.ts b/app/src/lib/i18n/pl.ts index ce5718280..680ec3ae6 100644 --- a/app/src/lib/i18n/pl.ts +++ b/app/src/lib/i18n/pl.ts @@ -4153,6 +4153,9 @@ const messages: TranslationMap = { 'flows.copilot.tool.dryRunning': 'Testowe uruchamianie przepływu pracy…', 'flows.copilot.tool.saving': 'Zapisywanie przepływu pracy…', 'flows.copilot.tool.usingTools': 'Używanie narzędzi…', + 'flows.copilot.cappedNotice': + 'Kreator osiągnął limit iteracji przed ukończeniem tego przepływu pracy. Kontynuowanie oznacza dalsze budowanie na podstawie bieżącego szkicu.', + 'flows.copilot.continueBuilding': 'Kontynuuj tworzenie', 'flows.list.view': 'Wyświetl przepływ pracy', 'flows.list.export': 'Eksportuj', 'flows.list.exported': 'Wyeksportowano przepływ pracy', diff --git a/app/src/lib/i18n/pt.ts b/app/src/lib/i18n/pt.ts index 3d89a2254..e1a9eb668 100644 --- a/app/src/lib/i18n/pt.ts +++ b/app/src/lib/i18n/pt.ts @@ -4164,6 +4164,9 @@ const messages: TranslationMap = { 'flows.copilot.tool.dryRunning': 'Executando teste do fluxo de trabalho…', 'flows.copilot.tool.saving': 'Salvando fluxo de trabalho…', 'flows.copilot.tool.usingTools': 'Usando ferramentas…', + 'flows.copilot.cappedNotice': + 'O construtor atingiu o limite de iterações antes de concluir este fluxo de trabalho. Ao continuar, a construção prosseguirá a partir do rascunho atual.', + 'flows.copilot.continueBuilding': 'Continuar a construir', 'flows.list.view': 'Ver fluxo de trabalho', 'flows.list.export': 'Exportar', 'flows.list.exported': 'Fluxo de trabalho exportado', diff --git a/app/src/lib/i18n/ru.ts b/app/src/lib/i18n/ru.ts index 992715873..8ffe8da2c 100644 --- a/app/src/lib/i18n/ru.ts +++ b/app/src/lib/i18n/ru.ts @@ -4136,6 +4136,9 @@ const messages: TranslationMap = { 'flows.copilot.tool.dryRunning': 'Выполняется пробный запуск рабочего процесса…', 'flows.copilot.tool.saving': 'Сохранение рабочего процесса…', 'flows.copilot.tool.usingTools': 'Использование инструментов…', + 'flows.copilot.cappedNotice': + 'Конструктор достиг предела итераций до завершения этого рабочего процесса. При продолжении сборка возобновится на основе текущего черновика.', + 'flows.copilot.continueBuilding': 'Продолжить создание', 'flows.list.view': 'Просмотреть рабочий процесс', 'flows.list.export': 'Экспорт', 'flows.list.exported': 'Рабочий процесс экспортирован', diff --git a/app/src/lib/i18n/zh-CN.ts b/app/src/lib/i18n/zh-CN.ts index 5345a471c..054dd0cb6 100644 --- a/app/src/lib/i18n/zh-CN.ts +++ b/app/src/lib/i18n/zh-CN.ts @@ -3884,6 +3884,9 @@ const messages: TranslationMap = { 'flows.copilot.tool.dryRunning': '正在试运行工作流…', 'flows.copilot.tool.saving': '正在保存工作流…', 'flows.copilot.tool.usingTools': '正在使用工具…', + 'flows.copilot.cappedNotice': + '构建器在完成此工作流之前达到了迭代次数上限。继续将基于当前草稿继续构建。', + 'flows.copilot.continueBuilding': '继续构建', 'flows.list.view': '查看工作流', 'flows.list.export': '导出', 'flows.list.exported': '工作流已导出', diff --git a/app/src/services/api/flowsApi.test.ts b/app/src/services/api/flowsApi.test.ts index 72c674fb8..d35b13154 100644 --- a/app/src/services/api/flowsApi.test.ts +++ b/app/src/services/api/flowsApi.test.ts @@ -393,7 +393,7 @@ describe('flowsApi', () => { error: null, failing_node_ids: [], }, - timeoutMs: 310_000, + timeoutMs: 610_000, }); expect(result.assistantText).toBe('here you go'); expect(result.proposal?.name).toBe('Digest'); @@ -419,7 +419,7 @@ describe('flowsApi', () => { failing_node_ids: [], thread_id: 'builder-thread-9', }, - timeoutMs: 310_000, + timeoutMs: 610_000, }); }); }); diff --git a/app/src/services/api/flowsApi.ts b/app/src/services/api/flowsApi.ts index 27620f159..31bc2e8d0 100644 --- a/app/src/services/api/flowsApi.ts +++ b/app/src/services/api/flowsApi.ts @@ -619,14 +619,22 @@ export interface BuilderTurnResult { assistantText: string; /** A run error, if the turn failed but a prior proposal was still captured. */ error: string | null; + /** + * `true` when the turn paused because it hit the agent's tool-call budget + * (`max_tool_iterations`) with no proposal yet — as opposed to the agent + * voluntarily asking a clarifying question or finishing. `assistantText` is + * a "Done so far / Next steps" checkpoint in this case; the UI should offer + * a "Continue building" action rather than rendering it as a normal reply. + */ + capped: boolean; } /** - * The `workflow_builder` agent can take up to ~300s server-side + * The `workflow_builder` agent can take up to ~600s server-side * (`FLOW_BUILD_TIMEOUT_SECS` in `src/openhuman/flows/ops.rs`); match it so a slow * authoring turn doesn't time out client-side while the agent is still working. */ -const FLOW_BUILD_TIMEOUT_MS = 310_000; +const FLOW_BUILD_TIMEOUT_MS = 610_000; /** * Map a raw `{ type: 'workflow_proposal', … }` payload (from the agent's @@ -702,12 +710,18 @@ export async function buildWorkflow( proposal: unknown; assistant_text: string; error: string | null; + capped?: boolean; }>(response); - log('buildWorkflow: response hasProposal=%s', result.proposal != null); + log( + 'buildWorkflow: response hasProposal=%s capped=%s', + result.proposal != null, + result.capped ?? false + ); return { proposal: mapWorkflowProposal(result.proposal), assistantText: result.assistant_text ?? '', error: result.error ?? null, + capped: result.capped ?? false, }; } diff --git a/src/openhuman/agent/harness/session/builder/setters.rs b/src/openhuman/agent/harness/session/builder/setters.rs index 81bd46b31..b3cea8dc0 100644 --- a/src/openhuman/agent/harness/session/builder/setters.rs +++ b/src/openhuman/agent/harness/session/builder/setters.rs @@ -496,6 +496,7 @@ impl AgentBuilder { last_memory_context: None, last_turn_citations: Vec::new(), last_turn_usage_totals: None, + last_turn_hit_cap: false, history: Vec::new(), post_turn_hooks: self.post_turn_hooks, learning_enabled: self.learning_enabled, diff --git a/src/openhuman/agent/harness/session/runtime.rs b/src/openhuman/agent/harness/session/runtime.rs index 9284414fe..069780494 100644 --- a/src/openhuman/agent/harness/session/runtime.rs +++ b/src/openhuman/agent/harness/session/runtime.rs @@ -387,6 +387,15 @@ impl Agent { self.last_turn_usage_totals.take() } + /// Whether the most recently completed [`Self::turn`] / [`Self::run_single`] + /// paused because it hit `max_tool_iterations`, rather than finishing + /// naturally (see the field doc on `last_turn_hit_cap`). `false` before + /// any turn has run. Not draining — unlike the usage totals above, a + /// caller may reasonably check this more than once per turn. + pub fn last_turn_hit_cap(&self) -> bool { + self.last_turn_hit_cap + } + // ───────────────────────────────────────────────────────────────── // Static helpers for turn parsing + telemetry // ───────────────────────────────────────────────────────────────── diff --git a/src/openhuman/agent/harness/session/turn/core.rs b/src/openhuman/agent/harness/session/turn/core.rs index f83ab8e68..ecb1dca6b 100644 --- a/src/openhuman/agent/harness/session/turn/core.rs +++ b/src/openhuman/agent/harness/session/turn/core.rs @@ -973,6 +973,12 @@ impl Agent { .await; let outcome = outcome?; + // Record whether this turn paused at the tool-call cap (vs. finishing + // naturally) BEFORE anything below can early-return, so a caller + // inspecting `last_turn_hit_cap()` after `run_single` always reflects + // this turn, never a stale value from a prior one. + self.last_turn_hit_cap = outcome.hit_cap; + // The stamped user turn is already in `self.history` (pushed by `turn()`), // so append only the structured messages this turn produced — assistant // tool calls + tool results + (for a clean finish) the final assistant — diff --git a/src/openhuman/agent/harness/session/types.rs b/src/openhuman/agent/harness/session/types.rs index f0c58dc8e..10bac4625 100644 --- a/src/openhuman/agent/harness/session/types.rs +++ b/src/openhuman/agent/harness/session/types.rs @@ -80,6 +80,16 @@ pub struct Agent { /// the first turn completes. pub(super) last_turn_usage_totals: Option, + /// Whether the most recent turn's tinyagents loop paused because it hit + /// `max_tool_iterations` (`TinyagentsTurnOutcome::hit_cap`), rather than + /// finishing naturally. `false` until the first turn completes, and reset + /// on every subsequent turn — so it only ever reflects the LAST turn, not + /// "any turn ever". Consumed by callers that run a single headless turn + /// via [`run_single`](super::runtime) (e.g. `flows_build`) and need to + /// distinguish "the agent paused mid-work" from "the agent asked a + /// question" or "the agent finished" — `run_single` only returns the + /// checkpoint/final text, with no other signal for which case occurred. + pub(super) last_turn_hit_cap: bool, pub(super) history: Vec, pub(super) post_turn_hooks: Vec>, pub(super) learning_enabled: bool, diff --git a/src/openhuman/flows/agents/workflow_builder/prompt.md b/src/openhuman/flows/agents/workflow_builder/prompt.md index 3e2367d9a..4b9e3711d 100644 --- a/src/openhuman/flows/agents/workflow_builder/prompt.md +++ b/src/openhuman/flows/agents/workflow_builder/prompt.md @@ -452,6 +452,38 @@ Any acting node may carry: Prefer `retry` + `on_error: "route"` for flaky network/tool steps, and `requires_approval` for anything the user would not want to happen unattended. +### Graph complexity — prefer the minimal viable graph + +Build the **smallest graph that fulfills the request**. Every node you add +is a binding to get right, a dry-run cycle to verify, and a point of +failure at runtime. Rules of thumb: + +- **An `agent` node can format its own output.** If the only purpose of a + downstream `code` or `transform` node is to reshape/format/template the + agent's structured output before passing it to a `tool_call`, fold that + formatting into the agent's `prompt` instruction and `output_parser.schema` + instead. The agent is a full LLM — it can produce markdown, HTML, or any + text shape you need. A separate formatting node is only warranted when the + formatting is purely mechanical (date math, string concatenation with no + judgment) and the agent's token cost would be wasted on it. + +- **Avoid split/merge for single-item flows.** `split_out` + downstream + processing + `merge` is for fan-out over a LIST (e.g. "for each issue, + do X"). If the flow processes one item end-to-end (a single calendar + brief, a single email reply), there is no list to fan out — skip the + split/merge entirely. + +- **One agent node can do multiple reasoning steps.** Don't chain two + `agent` nodes when one could handle both tasks in its prompt (e.g. + "extract the key fields AND compose a brief" in one node, rather than + "extract" → "compose" as two nodes). Chain agents only when they need + genuinely different models, schemas, or `agent_ref` profiles. + +- **Target: 3–6 nodes for a simple automation.** A schedule-trigger → + source-tool → agent-summarize → destination-tool flow is 4 nodes. + Most "when X happens, do Y" requests fit in 3–6. If your draft exceeds + 8 nodes, re-examine whether any node can be folded into its neighbor. + ## Style Be concise. Your posture is **clarify genuinely-ambiguous inputs, verify before @@ -508,7 +540,8 @@ nothing. ### Interpreting dry-run results honestly `dry_run_workflow` runs against **mock** capabilities — no real LLM call, -no real tool execution. Two classes of null or placeholder values appear: +no real tool execution, no real HTTP. Two classes of null or placeholder +values appear: 1. **Mock-LLM-output placeholders** — an `agent` node with a correct `output_parser.schema` produces synthetic placeholder values (empty @@ -528,17 +561,48 @@ no real tool execution. Two classes of null or placeholder values appear: entry and the dry run returns `ok: false`. **These are real bugs — never dismiss them.** Fix every one before proposing. +#### Sandbox mock behavior per node type (authoritative — do NOT probe) + +| Node kind | Sandbox output | Enveloped? | What resolves downstream | +|-----------|----------------|------------|---------------------------| +| `trigger` | Passthrough — echoes the `input` value (default `{}`) | No | Whatever was passed as `input` | +| `agent` (with `output_parser.schema`) | Typed placeholder per schema property (`string`→`""`, `number`/`integer`→`0`, `boolean`→`false`, `object`→`{}`, `array`→`[]`, `enum`→its first listed value) | Yes | `=nodes..item.json.` → the placeholder (non-null) | +| `agent` (no schema) | `{ "agent": "", "request": {...}, "connection": ... }` | Yes | Only `.json.agent` / `.json.request` / `.json.connection` resolve; any other `.json.` → null | +| `tool_call` | Required Composio args are preflight-checked first (missing/null → dry run fails before the mock even runs), then echoes `{ "tool": "", "args": {...}, "connection": ... }` — NOT a real API response | Yes | `.json.tool` / `.json.args` / `.json.connection` resolve; a response-shaped field (e.g. `.json.data.` for a real Composio call — see "the envelope" above) does **not**, because the mock echo carries no `data` wrapper. That is a mock-shape gap, not a wiring bug — don't "fix" a correctly-wired `.json.data.` binding just because the dry run can't resolve it | +| `http_request` | `{ "status": 200, "request": {...}, "connection": ... }` | Yes | `.json.status` → `200`; response-body fields → null | +| `code` | `{ "result": }` — the real `source` is NOT executed | **No** | `.item.result` resolves directly (no `.json.` — `code` does not envelope) | +| `transform` | **REAL** execution — evaluates `config.set` expressions against scope | No | Real resolved values | +| `condition` | **REAL** execution — evaluates truthiness on the actual (mock) input data | No | Routes to the real "true"/"false" port | +| `switch` | **REAL** execution — evaluates the routing expression/field | No | Routes to the real matching port | +| `split_out` | **REAL** execution — fans out the array at `config.path` | No | Real fan-out of the mock data | +| `merge` | **REAL** execution — concatenates input items | No | Passthrough | + +**NEVER run isolated probe graphs** (e.g. a throwaway `[trigger, agent, +tool_call]` subgraph) to test whether a node type's output resolves in the +sandbox. The table above is authoritative. If an `agent` node has a correct +`output_parser.schema`, its `.json.` bindings WILL resolve to typed +placeholders — you do not need to verify this experimentally. Run +`dry_run_workflow` on the REAL graph you're building to check your actual +bindings; a probe graph burns tool calls re-discovering what's already +documented above. + **Never say "known sandbox limitation" or "at runtime this works perfectly" to dismiss a dry-run finding.** If the dry run returns `ok: false`, the -graph has a real problem. If it returns `ok: true` with -`routing_divergence_warnings`, say what was unverified and why (the mock -trigger payload routed differently than a real one would), so the user -knows which branches are untested — do not assert they "work perfectly." +graph has a real problem (with the sole documented exception of the +`tool_call` `.json.data.` mock-shape gap above). If it returns +`ok: true` with `routing_divergence_warnings`, say what was unverified and +why (the mock trigger payload routed differently than a real one would), so +the user knows which branches are untested — do not assert they "work +perfectly." -The only thing the sandbox genuinely cannot test is the CONTENT of an LLM's -reply or a real tool's response shape. Everything else — expression paths, -schema declarations, edge wiring, port labels, required args — is fully -testable in the sandbox, and a failure there is a real failure. +The only things the sandbox genuinely cannot test are: +- The CONTENT of an LLM's reply (placeholders only) +- The SHAPE of a real tool/HTTP response (echoes only) +- Real code execution output (echoes input under `result`, does not run + `source`) +Everything else — expression paths, schema declarations, edge wiring, port +labels, required args, condition/switch routing — is fully testable in the +sandbox, and a failure there is a real failure. ### Say what you inferred diff --git a/src/openhuman/flows/ops.rs b/src/openhuman/flows/ops.rs index 4975d75d1..61ec81c24 100644 --- a/src/openhuman/flows/ops.rs +++ b/src/openhuman/flows/ops.rs @@ -3258,7 +3258,13 @@ pub async fn flows_discover( /// Overall safety bound on one `flows_build` run. The `workflow_builder` agent's /// own `max_iterations` caps its loop, but a hung LLM/tool call must never let /// the RPC block indefinitely. -const FLOW_BUILD_TIMEOUT_SECS: u64 = 300; +/// +/// Matches [`FLOW_RUN_TIMEOUT_SECS`] (600s): since the (B31) fix below actually +/// applies the builder's `effective_max_iterations()` (50, not the global +/// default of 10) to this path, a worst-case run at ~10s/iteration can take up +/// to ~500s — the old 300s bound would have clipped a legitimate long build +/// before the iteration cap ever got a chance to. +const FLOW_BUILD_TIMEOUT_SECS: u64 = 600; /// Tools stripped from the `workflow_builder` belt on the direct `flows_build` /// RPC path (issue #4593). @@ -3301,6 +3307,61 @@ fn restrict_builder_toolset(agent: &mut crate::openhuman::agent::Agent) { agent.hide_tools(FLOWS_BUILD_HIDDEN_TOOLS); } +/// (B31) Returns a clone of `config` with `agent.max_tool_iterations` +/// overridden to the `workflow_builder` [`AgentDefinition`](crate::openhuman::agent::harness::definition::AgentDefinition)'s +/// [`effective_max_iterations()`](crate::openhuman::agent::harness::definition::AgentDefinition::effective_max_iterations), +/// for use by [`Agent::from_config_for_agent`](crate::openhuman::agent::Agent::from_config_for_agent) +/// in [`flows_build`]. +/// +/// Two independent iteration-cap systems exist in the harness: the per-agent +/// `AgentDefinition.max_iterations`/`iteration_policy` (`workflow_builder`'s +/// `agent.toml` declares `iteration_policy = "extended"`, so its effective cap +/// is `max(max_iterations, EXTENDED_MAX_TOOL_ITERATIONS)` = 50), and the +/// GLOBAL `Config.agent.max_tool_iterations` (default 10). Only the +/// **sub-agent runner** path (`spawn_subagent`, used when `workflow_builder` +/// is invoked as a chat delegate) applies the definition's effective cap. +/// `flows_build` calls `Agent::from_config_for_agent` directly — the session +/// builder stamps `config.agent.clone()` onto the session unconditionally, so +/// without this override the builder silently got the global default (10) +/// instead of the 50 its own `agent.toml` intends, burning its entire budget +/// on a handful of dry-run cycles for anything past a trivial graph. +/// +/// A missing registry or missing `workflow_builder` definition (registry not +/// yet initialised, a stripped-down test registry) falls back to `config` +/// unchanged — same behavior as before this fix, never a hard failure. +fn apply_builder_iteration_cap(config: &Config) -> Config { + let mut build_config = config.clone(); + let Some(reg) = crate::openhuman::agent::harness::AgentDefinitionRegistry::global() else { + tracing::warn!( + target: "flows", + "[flows] flows_build: agent registry not initialised; falling back to global \ + max_tool_iterations={}", + config.agent.max_tool_iterations + ); + return build_config; + }; + let Some(def) = reg.get("workflow_builder") else { + tracing::warn!( + target: "flows", + "[flows] flows_build: workflow_builder definition not found in registry; falling \ + back to global max_tool_iterations={}", + config.agent.max_tool_iterations + ); + return build_config; + }; + let effective = def.effective_max_iterations(); + tracing::debug!( + target: "flows", + toml_max_iterations = def.max_iterations, + effective_max_iterations = effective, + global_default = config.agent.max_tool_iterations, + "[flows] flows_build: overriding max_tool_iterations from the workflow_builder agent \ + definition (the session path does not apply effective_max_iterations() by default)" + ); + build_config.agent.max_tool_iterations = effective; + build_config +} + /// Runs the `workflow_builder` agent for one authoring turn and returns its /// proposal, invoking it as a first-class backend agent (exactly like the Flow /// Scout `flows_discover`) rather than routing a hand-crafted delegate prompt @@ -3343,7 +3404,13 @@ pub async fn flows_build( crate::openhuman::agent::harness::AgentDefinitionRegistry::init_global(&config.workspace_dir) .map_err(|e| format!("failed to initialise agent registry: {e}"))?; - let mut agent = Agent::from_config_for_agent(config, "workflow_builder") + // (B31) See `apply_builder_iteration_cap`'s doc: the session path below + // otherwise stamps the GLOBAL `config.agent.max_tool_iterations` (10) + // instead of the `workflow_builder` definition's intended + // `effective_max_iterations()` (50). + let build_config = apply_builder_iteration_cap(config); + + let mut agent = Agent::from_config_for_agent(&build_config, "workflow_builder") .map_err(|e| format!("failed to build workflow_builder agent: {e:#}"))?; agent.set_agent_definition_name("workflow_builder".to_string()); @@ -3427,9 +3494,24 @@ pub async fn flows_build( } } + // (B34) Whether this turn paused because it hit `max_tool_iterations` + // rather than finishing naturally (asking a question, or proposing). A + // capped turn with no proposal renders a raw checkpoint ("Done so far / + // Next steps") that's indistinguishable, in the response shape alone, + // from the agent voluntarily asking a clarifying question — `capped` + // gives the frontend the explicit signal to render a "Continue building" + // card instead. Scoped to `proposal.is_none()`: a turn that hit the cap + // but still squeezed out a proposal (the checkpoint fires before the + // final `propose_workflow` call in that ordering) has nothing left to + // continue. + let hit_cap = agent.last_turn_hit_cap(); + let capped = hit_cap && proposal.is_none(); + tracing::info!( target: "flows", has_proposal = proposal.is_some(), + hit_cap, + capped, "[flows] flows_build: workflow_builder turn complete" ); Ok(RpcOutcome::single_log( @@ -3437,6 +3519,7 @@ pub async fn flows_build( "proposal": proposal, "assistant_text": assistant_text, "error": run_error, + "capped": capped, }), "workflow builder turn complete", )) diff --git a/src/openhuman/flows/ops_tests.rs b/src/openhuman/flows/ops_tests.rs index c9eb7b40b..95f993989 100644 --- a/src/openhuman/flows/ops_tests.rs +++ b/src/openhuman/flows/ops_tests.rs @@ -3156,6 +3156,51 @@ async fn flows_build_hides_the_live_run_tool_from_the_builder_belt() { } } +/// Regression for B31: `flows_build` must apply the `workflow_builder` +/// `AgentDefinition`'s `effective_max_iterations()` (50, from `agent.toml`'s +/// `iteration_policy = "extended"`), not the global `Config::default()` +/// `agent.max_tool_iterations` (10) — see `apply_builder_iteration_cap`'s doc. +#[tokio::test] +async fn flows_build_applies_the_builder_definitions_effective_iteration_cap() { + let tmp = TempDir::new().unwrap(); + let config = test_config(&tmp); + + // Precondition: the global default really is lower than the definition's + // effective cap, otherwise this test can't distinguish the two. + assert_eq!(config.agent.max_tool_iterations, 10); + + crate::openhuman::agent::harness::AgentDefinitionRegistry::init_global(&config.workspace_dir) + .expect("agent registry init"); + let def = crate::openhuman::agent::harness::AgentDefinitionRegistry::global() + .expect("registry initialised") + .get("workflow_builder") + .expect("workflow_builder definition registered") + .clone(); + let expected = def.effective_max_iterations(); + assert_eq!( + expected, 50, + "workflow_builder's agent.toml is expected to declare iteration_policy = \"extended\", \ + yielding an effective cap of EXTENDED_MAX_TOOL_ITERATIONS (50)" + ); + + let build_config = apply_builder_iteration_cap(&config); + assert_eq!( + build_config.agent.max_tool_iterations, expected, + "flows_build's build_config must carry the definition's effective cap, not the global \ + default" + ); + assert_ne!( + build_config.agent.max_tool_iterations, config.agent.max_tool_iterations, + "sanity: the override must actually differ from the unmodified global config" + ); + + // End-to-end: the agent actually built for this path carries the override. + let agent = + crate::openhuman::agent::Agent::from_config_for_agent(&build_config, "workflow_builder") + .expect("build workflow_builder agent"); + assert_eq!(agent.agent_config().max_tool_iterations, expected); +} + // ───────────────────────────────────────────────────────────────────────────── // B23/B24 — condition node branch label must be on `from_port`, not `to_port` // ─────────────────────────────────────────────────────────────────────────────