fix(flows): builder turn-budget wiring + prompt guidance + capped UX (B31-B34) (#4865)

Co-authored-by: M3gA-Mind <elvin@tinyhumans.ai>
This commit is contained in:
Cyrus Gray
2026-07-14 21:58:49 +05:30
committed by GitHub
co-authored by M3gA-Mind
parent fe1419e62e
commit f7468905b2
27 changed files with 516 additions and 16 deletions
@@ -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(
<WorkflowCopilotPanel
graph={baseGraph}
onProposal={vi.fn()}
onAccept={vi.fn()}
onReject={vi.fn()}
onClose={vi.fn()}
/>
);
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(
<WorkflowCopilotPanel
graph={baseGraph}
onProposal={vi.fn()}
onAccept={vi.fn()}
onReject={vi.fn()}
onClose={vi.fn()}
/>
);
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(
<WorkflowCopilotPanel
graph={baseGraph}
onProposal={vi.fn()}
onAccept={vi.fn()}
onReject={vi.fn()}
onClose={vi.fn()}
/>
);
expect(screen.queryByTestId('workflow-copilot-capped')).not.toBeInTheDocument();
});
it('B34: clicking "Continue building" sends a follow-up turn', async () => {
hookState.capped = true;
render(
<WorkflowCopilotPanel
graph={baseGraph}
onProposal={vi.fn()}
onAccept={vi.fn()}
onReject={vi.fn()}
onClose={vi.fn()}
/>
);
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(
<WorkflowCopilotPanel
graph={baseGraph}
flowId="flow-123"
onProposal={vi.fn()}
onAccept={vi.fn()}
onReject={vi.fn()}
onClose={vi.fn()}
/>
);
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(
<WorkflowCopilotPanel
@@ -140,6 +140,7 @@ export default function WorkflowCopilotPanel({
threadId,
sending,
proposal,
capped,
displayMessages,
toolTimeline,
liveResponse,
@@ -330,6 +331,31 @@ export default function WorkflowCopilotPanel({
[text, sending, send, graph, flowId, updatePendingAsk]
);
// (B34) One-click resume for a turn that hit the agent's tool-call budget
// (`capped`, see `useWorkflowBuilderChat`'s doc) with no proposal yet.
// Routes through the SAME `submit` path a typed follow-up would — the
// `pendingAskRef` mechanism (set above, since a capped turn also has
// `proposed === false`) automatically carries the original ask forward, so
// the agent picks the build back up with full context, not just "continue"
// in isolation.
//
// What this actually does (Codex review on #4865): `flows_build` spins up a
// FRESH `workflow_builder` agent per RPC — there is no server-side
// session/tool-history checkpoint to reattach to, so this is not a literal
// mid-thought resume. What DOES carry forward, because `submit` always
// sends `mode: 'revise'` over the CURRENT `graph` + `flowId` (never a blank
// `create`): (1) the live draft graph — unchanged by a capped turn, since
// `revise_workflow`/`propose_workflow` never persist without a proposal
// reaching this panel; and (2) the full accumulated instruction text via
// `pendingAskRef`. A fresh agent re-reading the same draft plus the same
// ask, now under the B31 50-iteration budget and B32's no-probing brief,
// reliably converges — that combination is what the capped card's copy
// promises ("keep building from the current draft"), not seamless
// tool-history continuity.
const continueBuilding = useCallback(() => {
void submit(t('flows.copilot.continueBuilding'));
}, [submit, t]);
const handleInputKeyDown = useCallback(
(event: React.KeyboardEvent<HTMLTextAreaElement>) => {
if (event.key === 'Enter' && !event.shiftKey && !isComposingTextRef.current) {
@@ -513,6 +539,33 @@ export default function WorkflowCopilotPanel({
</div>
</div>
)}
{/* (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 && (
<div
data-testid="workflow-copilot-capped"
className="rounded-xl border border-amber-300 bg-surface p-3 dark:border-amber-700">
<p className="text-xs text-content-secondary">{t('flows.copilot.cappedNotice')}</p>
<div className="mt-2">
<Button
type="button"
variant="secondary"
size="sm"
data-testid="workflow-copilot-continue"
onClick={continueBuilding}>
{t('flows.copilot.continueBuilding')}
</Button>
</div>
</div>
)}
</div>
<div className="border-t border-line px-3 py-2.5">
@@ -56,6 +56,7 @@ const okResult = (over: Partial<BuilderTurnResult> = {}): 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());
+22
View File
@@ -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<string | null>(seedThreadId ?? null);
const [localSending, setLocalSending] = useState(false);
const [error, setError] = useState<string | null>(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,
+3
View File
@@ -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': 'تم تصدير سير العمل',
+3
View File
@@ -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': 'ওয়ার্কফ্লো রপ্তানি হয়েছে',
+3
View File
@@ -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',
+3
View File
@@ -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
+3
View File
@@ -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',
+3
View File
@@ -4200,6 +4200,9 @@ const messages: TranslationMap = {
'flows.copilot.tool.dryRunning': 'Exécution dessai du workflow…',
'flows.copilot.tool.saving': 'Enregistrement du workflow…',
'flows.copilot.tool.usingTools': 'Utilisation doutils…',
'flows.copilot.cappedNotice':
'Le générateur a atteint sa limite dité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é',
+3
View File
@@ -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': 'वर्कफ़्लो निर्यात किया गया',
+3
View File
@@ -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',
+3
View File
@@ -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',
+3
View File
@@ -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': '워크플로를 내보냈습니다',
+3
View File
@@ -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',
+3
View File
@@ -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',
+3
View File
@@ -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': 'Рабочий процесс экспортирован',
+3
View File
@@ -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': '工作流已导出',
+2 -2
View File
@@ -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,
});
});
});
+17 -3
View File
@@ -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,
};
}
@@ -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,
@@ -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
// ─────────────────────────────────────────────────────────────────
@@ -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 —
@@ -80,6 +80,16 @@ pub struct Agent {
/// the first turn completes.
pub(super) last_turn_usage_totals:
Option<crate::openhuman::agent::harness::turn_subagent_usage::LastTurnUsage>,
/// 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<ConversationMessage>,
pub(super) post_turn_hooks: Vec<Arc<dyn PostTurnHook>>,
pub(super) learning_enabled: bool,
@@ -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: 36 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 36. 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.<id>.item.json.<field>` → the placeholder (non-null) |
| `agent` (no schema) | `{ "agent": "<agent_ref>", "request": {...}, "connection": ... }` | Yes | Only `.json.agent` / `.json.request` / `.json.connection` resolve; any other `.json.<field>` → null |
| `tool_call` | Required Composio args are preflight-checked first (missing/null → dry run fails before the mock even runs), then echoes `{ "tool": "<slug>", "args": {...}, "connection": ... }` — NOT a real API response | Yes | `.json.tool` / `.json.args` / `.json.connection` resolve; a response-shaped field (e.g. `.json.data.<x>` 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.<field>` 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": <input items> }` — 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.<field>` 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.<field>` 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
+85 -2
View File
@@ -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",
))
+45
View File
@@ -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`
// ─────────────────────────────────────────────────────────────────────────────