fix(tinyflows): preserve explicit node timeouts, align discover timeout, log cap pins

app/src/components/flows/canvas/__tests__/EditableFlowCanvas.validation.test.tsx

@@ -1,18 +1,30 @@
 /**
  * EditableFlowCanvas — validation UX (Phase 3c) + draft/dirty state (Phase 3d).
  *
- * Drives the canvas through the public `FlowCanvas editable` entry point with a
- * mocked `flowsApi` so `validateFlow` is deterministic. Covers:
- *  - an invalid graph shows the inline error banner, rings the offending node,
- *    and blocks Save;
- *  - a valid-with-warnings graph surfaces warnings distinctly and allows Save;
- *  - dirty tracking gates Save/Discard, Discard resets to baseline, and a
- *    successful Save clears the dirty flag.
+ * A canvas-refactor moved the Save / Discard / dirty-badge *buttons* out of
+ * this component and up into `FlowCanvasPage`'s header — the canvas now only
+ * exposes them through the `EditableFlowCanvasHandle` ref (`save()`/
+ * `discard()`) and reports state up via `onSaveMetaChange`
+ * (`{ dirty, hasErrors, saving }`), same as `onDirtyChange`. See
+ * `FlowCanvasPage.test.tsx` for the header-button + confirm-dialog + RPC
+ * integration coverage (clicking `flow-editor-save`/`flow-editor-discard`).
+ *
+ * This file drives the canvas through the public `FlowCanvas editable` entry
+ * point with a mocked `flowsApi` so `validateFlow` is deterministic, and
+ * covers what `FlowCanvas`/`EditableFlowCanvas` itself still owns:
+ *  - an invalid graph shows the inline error banner, rings the offending
+ *    node, and the imperative `save()` handle refuses to fire `onSave`;
+ *  - a valid-with-warnings graph surfaces warnings distinctly and still
+ *    lets `save()` fire;
+ *  - dirty tracking gates `save()`/`discard()`, `discard()` resets to
+ *    baseline, and a successful `save()` clears the dirty flag.
  */
-import { fireEvent, render, screen, waitFor } from '@testing-library/react';
+import { act, fireEvent, render, screen, waitFor } from '@testing-library/react';
+import { createRef } from 'react';
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 
 import type { FlowNode } from '../../../../lib/flows/graphAdapter';
+import type { EditableFlowCanvasHandle, EditorSaveMeta } from '../EditableFlowCanvas';
 import FlowCanvas from '../FlowCanvas';
 
 const validateFlow = vi.hoisted(() => vi.fn());
@@ -38,16 +50,27 @@ function triggerNode(): FlowNode {
 const META = { schema_version: 1, id: 'wf_1', name: 'My flow' } as const;
 
 function renderCanvas(props: Partial<React.ComponentProps<typeof FlowCanvas>> = {}) {
-  return render(
+  const ref = createRef<EditableFlowCanvasHandle>();
+  const onSaveMetaChange = vi.fn<(meta: EditorSaveMeta) => void>();
+  const utils = render(
     <FlowCanvas
+      ref={ref}
       editable
       nodes={[triggerNode()]}
       edges={[]}
       meta={META}
       onSave={vi.fn().mockResolvedValue(undefined)}
+      onSaveMetaChange={onSaveMetaChange}
       {...props}
     />
   );
+  return { ...utils, ref, onSaveMetaChange };
+}
+
+/** Latest `{ dirty, hasErrors, saving }` the canvas reported to its host. */
+function lastSaveMeta(onSaveMetaChange: ReturnType<typeof vi.fn<(meta: EditorSaveMeta) => void>>) {
+  const calls = onSaveMetaChange.mock.calls;
+  return calls[calls.length - 1][0];
 }
 
 describe('EditableFlowCanvas — validation + dirty state', () => {
@@ -57,24 +80,29 @@ describe('EditableFlowCanvas — validation + dirty state', () => {
     listFlowConnections.mockResolvedValue([]);
   });
 
-  it('surfaces hard errors, rings the offending node, and blocks Save', async () => {
+  it('surfaces hard errors, rings the offending node, and blocks save()', async () => {
     validateFlow.mockResolvedValue({
       valid: false,
       errors: ['invalid config for node t: missing schedule'],
       warnings: [],
     });
-    const { container } = renderCanvas();
+    const onSave = vi.fn().mockResolvedValue(undefined);
+    const { container, ref, onSaveMetaChange } = renderCanvas({ onSave });
 
-    // Make an edit so the graph is dirty (Save is only ever enabled when dirty).
+    // Make an edit so the graph is dirty (Save is only ever attempted when
+    // dirty). Validation runs automatically on the debounce after the edit
+    // (the manual Validate button now lives on the selected node card).
     fireEvent.click(screen.getByTestId('flow-palette-item-agent'));
-    // Validation runs automatically on the debounce after the edit (the manual
-    // Validate button now lives on the selected node card).
 
     const errors = await screen.findByTestId('flow-editor-errors');
     expect(errors).toHaveTextContent('invalid config for node t: missing schedule');
 
-    // Save is blocked while there are hard errors, even though the graph is dirty.
-    expect(screen.getByTestId('flow-editor-save')).toBeDisabled();
+    // The host header reads `hasErrors` off `onSaveMetaChange` to disable its
+    // Save button; the canvas itself also refuses to fire `onSave` through
+    // the imperative handle while hard errors exist, even though dirty.
+    expect(lastSaveMeta(onSaveMetaChange)).toMatchObject({ dirty: true, hasErrors: true });
+    act(() => ref.current?.save());
+    expect(onSave).not.toHaveBeenCalled();
 
     // The named node ('t') is ringed with the error class on its RF wrapper.
     await waitFor(() =>
@@ -84,13 +112,14 @@ describe('EditableFlowCanvas — validation + dirty state', () => {
     );
   });
 
-  it('shows warnings distinctly from errors and allows Save', async () => {
+  it('shows warnings distinctly from errors and still lets save() fire', async () => {
     validateFlow.mockResolvedValue({
       valid: true,
       errors: [],
       warnings: ['this trigger kind does not fire automatically yet'],
     });
-    renderCanvas();
+    const onSave = vi.fn().mockResolvedValue(undefined);
+    const { ref, onSaveMetaChange } = renderCanvas({ onSave });
 
     fireEvent.click(screen.getByTestId('flow-palette-item-agent'));
     // Auto-validation (debounced) surfaces the warning.
@@ -99,71 +128,77 @@ describe('EditableFlowCanvas — validation + dirty state', () => {
     expect(warnings).toHaveTextContent('does not fire automatically');
     // A valid graph never renders the errors list…
     expect(screen.queryByTestId('flow-editor-errors')).not.toBeInTheDocument();
-    // …and Save is allowed (warnings don't block).
-    expect(screen.getByTestId('flow-editor-save')).not.toBeDisabled();
+    // …and warnings don't block Save.
+    expect(lastSaveMeta(onSaveMetaChange)).toMatchObject({ dirty: true, hasErrors: false });
+
+    act(() => ref.current?.save());
+    expect(onSave).toHaveBeenCalledTimes(1);
   });
 
-  it('tracks dirty state: Save/Discard gate on it, Discard resets, Save clears it', async () => {
+  it('tracks dirty state: save()/discard() gate on it, discard resets, save clears it', async () => {
     validateFlow.mockResolvedValue({ valid: true, errors: [], warnings: [] });
     const onSave = vi.fn().mockResolvedValue(undefined);
     const onDirtyChange = vi.fn();
-    renderCanvas({ onSave, onDirtyChange });
+    const { ref, onSaveMetaChange } = renderCanvas({ onSave, onDirtyChange });
 
-    // Pristine: no dirty badge, Save + Discard disabled.
-    expect(screen.queryByTestId('flow-editor-dirty')).not.toBeInTheDocument();
-    expect(screen.getByTestId('flow-editor-save')).toBeDisabled();
-    expect(screen.getByTestId('flow-editor-discard')).toBeDisabled();
+    // Pristine: not dirty; save()/discard() both no-op through the imperative
+    // handle (the host header renders both buttons disabled in this state).
+    expect(lastSaveMeta(onSaveMetaChange)).toMatchObject({ dirty: false });
+    act(() => ref.current?.discard());
+    act(() => ref.current?.save());
+    expect(onSave).not.toHaveBeenCalled();
 
     // Edit → dirty.
     fireEvent.click(screen.getByTestId('flow-palette-item-agent'));
-    expect(screen.getByTestId('flow-editor-dirty')).toBeInTheDocument();
-    expect(screen.getByTestId('flow-editor-save')).not.toBeDisabled();
-    expect(screen.getByTestId('flow-editor-discard')).not.toBeDisabled();
     expect(onDirtyChange).toHaveBeenLastCalledWith(true);
+    expect(lastSaveMeta(onSaveMetaChange)).toMatchObject({ dirty: true });
     expect(screen.getAllByTestId('flow-node')).toHaveLength(2);
 
     // Discard → back to the single trigger, no longer dirty.
-    fireEvent.click(screen.getByTestId('flow-editor-discard'));
+    act(() => ref.current?.discard());
     expect(screen.getAllByTestId('flow-node')).toHaveLength(1);
-    expect(screen.queryByTestId('flow-editor-dirty')).not.toBeInTheDocument();
     expect(onDirtyChange).toHaveBeenLastCalledWith(false);
+    expect(lastSaveMeta(onSaveMetaChange)).toMatchObject({ dirty: false });
 
-    // Edit again and Save → onSave called, dirty cleared once it resolves.
+    // Edit again and save() → onSave called, dirty cleared once it resolves.
     fireEvent.click(screen.getByTestId('flow-palette-item-agent'));
-    fireEvent.click(screen.getByTestId('flow-editor-save'));
+    act(() => ref.current?.save());
     await waitFor(() => expect(onSave).toHaveBeenCalledTimes(1));
     const graph = onSave.mock.calls[0][0];
     expect(graph.nodes.map((n: { kind: string }) => n.kind).sort()).toEqual(['agent', 'trigger']);
-    await waitFor(() => expect(screen.queryByTestId('flow-editor-dirty')).not.toBeInTheDocument());
-    expect(onDirtyChange).toHaveBeenLastCalledWith(false);
+    await waitFor(() => expect(onDirtyChange).toHaveBeenLastCalledWith(false));
+    await waitFor(() => expect(lastSaveMeta(onSaveMetaChange)).toMatchObject({ dirty: false }));
   });
 
-  it('starts dirty when the host passes initialDirty (a remount carrying unsaved content)', async () => {
+  it('starts dirty when the host passes initialDirty (a remount carrying unsaved content)', () => {
     validateFlow.mockResolvedValue({ valid: true, errors: [], warnings: [] });
+    const onSave = vi.fn().mockResolvedValue(undefined);
     const onDirtyChange = vi.fn();
     // Mirrors `FlowCanvasPage` remounting the canvas (`key={canvasVersion}`)
     // after accepting a copilot proposal: the incoming nodes/edges ARE the
     // component's "initial" graph, so without `initialDirty` the canvas would
     // seed its baseline from them and instantly read as clean even though
     // nothing was persisted (the P1 this regression test guards against).
-    renderCanvas({ onDirtyChange, initialDirty: true });
+    const { ref, onSaveMetaChange } = renderCanvas({ onSave, onDirtyChange, initialDirty: true });
 
-    expect(screen.getByTestId('flow-editor-dirty')).toBeInTheDocument();
-    expect(screen.getByTestId('flow-editor-save')).not.toBeDisabled();
     expect(onDirtyChange).toHaveBeenLastCalledWith(true);
+    expect(lastSaveMeta(onSaveMetaChange)).toMatchObject({ dirty: true });
+    act(() => ref.current?.save());
+    expect(onSave).toHaveBeenCalledTimes(1);
   });
 
   it('surfaces a Save failure inline and leaves the graph dirty', async () => {
     validateFlow.mockResolvedValue({ valid: true, errors: [], warnings: [] });
     const onSave = vi.fn().mockRejectedValue(new Error('core unreachable'));
-    renderCanvas({ onSave });
+    const onDirtyChange = vi.fn();
+    const { ref } = renderCanvas({ onSave, onDirtyChange });
 
     fireEvent.click(screen.getByTestId('flow-palette-item-agent'));
-    fireEvent.click(screen.getByTestId('flow-editor-save'));
+    act(() => ref.current?.save());
 
     const saveError = await screen.findByTestId('flow-editor-save-error');
     expect(saveError).toHaveTextContent('core unreachable');
     // Still dirty — nothing persisted.
-    expect(screen.getByTestId('flow-editor-dirty')).toBeInTheDocument();
+    expect(onDirtyChange).toHaveBeenLastCalledWith(true);
   });
 });

app/src/services/api/flowsApi.test.ts

@@ -340,7 +340,7 @@ describe('flowsApi', () => {
       expect(mockCallCoreRpc).toHaveBeenCalledWith({
         method: 'openhuman.flows_discover',
         params: {},
-        timeoutMs: 310_000,
+        timeoutMs: 610_000,
       });
       expect(result).toEqual([suggestion]);
     });
@@ -353,7 +353,7 @@ describe('flowsApi', () => {
       expect(mockCallCoreRpc).toHaveBeenCalledWith({
         method: 'openhuman.flows_discover',
         params: { thread_id: 'scout-thread-1' },
-        timeoutMs: 310_000,
+        timeoutMs: 610_000,
       });
     });
 

app/src/services/api/flowsApi.ts

@@ -799,12 +799,14 @@ export async function promoteDraft(id: string, requireApproval?: boolean): Promi
 
 /**
  * `openhuman.flows_discover` runs the read-only Flow Scout agent, which reasons
- * over the user's memory/threads/connections/flows and can take up to ~300s
- * server-side (`FLOW_DISCOVER_TIMEOUT_SECS` in `src/openhuman/flows/ops.rs`).
- * Give the client a matching budget so a slow discovery run doesn't time out
- * client-side while the agent is still thinking.
+ * over the user's memory/threads/connections/flows and can take up to ~600s
+ * server-side (`FLOW_DISCOVER_TIMEOUT_SECS` in `src/openhuman/flows/ops.rs`,
+ * raised to match `FLOW_BUILD_TIMEOUT_SECS` for the same iteration cap). Give
+ * the client a matching budget (mirrors {@link FLOW_BUILD_TIMEOUT_MS}) so a
+ * slow discovery run doesn't time out client-side while the agent is still
+ * thinking.
  */
-const FLOW_DISCOVER_TIMEOUT_MS = 310_000;
+const FLOW_DISCOVER_TIMEOUT_MS = 610_000;
 
 /**
  * Run the Flow Scout discovery agent via `openhuman.flows_discover` and return

src/openhuman/agent/task_dispatcher/executor.rs

@@ -157,6 +157,13 @@ pub(super) async fn run_autonomous(
     // post-construction override rather than a pre-set on `config` (which
     // the builder would otherwise clobber).
     agent.set_max_tool_iterations(TASK_RUN_MAX_ITERATIONS);
+    tracing::debug!(
+        agent_id = %executor.agent_id,
+        run_id = %run_id,
+        max_tool_iterations = TASK_RUN_MAX_ITERATIONS,
+        "[task_dispatcher] pinned autonomous task-run iteration budget post-construction \
+         (overrides the session builder's per-definition cap)"
+    );
     agent.set_event_context(run_id.to_string(), "task");
     agent.set_agent_definition_name(format!(
         "task-{}-{}",

src/openhuman/agent_registry/agents/loader.rs

@@ -1039,19 +1039,41 @@ mod tests {
         );
         match &def.tools {
             ToolScope::Named(names) => {
+                // Reconciled against `agent.toml`'s current `[tools].named`
+                // after the workflow-tools expansion PR widened the belt to
+                // agent-native editing/creation/run-control (`edit_workflow`,
+                // `validate_workflow`, `create_workflow`, `duplicate_flow`,
+                // `list_node_kinds`, `get_node_kind_contract`,
+                // `get_flow_history`, `list_flow_runs`, `resume_flow_run`,
+                // `cancel_flow_run`, `list_connectable_toolkits`) — these are
+                // the agent's own scoped tool surface, not the raw `flows_*`
+                // controller RPCs banned below, so the "no flow
+                // creation/enable via the raw controller" invariant still
+                // holds via the forbidden list.
                 let expected = [
                     "propose_workflow",
                     "revise_workflow",
+                    "edit_workflow",
+                    "validate_workflow",
                     "save_workflow",
                     "list_flows",
                     "get_flow",
+                    "get_flow_history",
                     "get_flow_run",
                     "list_flow_connections",
                     "search_tool_catalog",
                     "get_tool_contract",
                     "get_tool_output_sample",
                     "list_agent_profiles",
+                    "list_connectable_toolkits",
+                    "list_node_kinds",
+                    "get_node_kind_contract",
                     "dry_run_workflow",
+                    "list_flow_runs",
+                    "resume_flow_run",
+                    "cancel_flow_run",
+                    "create_workflow",
+                    "duplicate_flow",
                     "run_flow",
                     "composio_list_toolkits",
                     "composio_list_connections",
@@ -1070,8 +1092,9 @@ mod tests {
                     expected.len(),
                     "workflow_builder scope must be EXACTLY the propose-or-read belt (got {names:?})"
                 );
-                // Hard exclusions: nothing that creates/enables a flow,
-                // executes raw integration actions, or touches the host.
+                // Hard exclusions: nothing that reaches the raw flow
+                // controller directly, executes raw integration actions, or
+                // touches the host.
                 // (Persistence onto an EXISTING flow is the deliberate
                 // `save_workflow` carve-out above; raw `flows_update` — which
                 // could also rename/re-gate arbitrary flows — stays out.)

src/openhuman/flows/ops.rs

@@ -3297,7 +3297,14 @@ fn notify_pending_approval(flow: &Flow, thread_id: &str, pending_approvals: &[St
 /// reasons read-only over the user's data and ends by emitting
 /// `suggest_workflows`; its own `max_iterations` caps the loop, but a hung
 /// LLM/tool call must never let the RPC block indefinitely.
-const FLOW_DISCOVER_TIMEOUT_SECS: u64 = 300;
+///
+/// Matches [`FLOW_BUILD_TIMEOUT_SECS`] (600s): the session builder applies the
+/// `flow_discovery` definition's `effective_max_iterations()` (50, not the
+/// global default of 10) to this path (issue #4868), so a worst-case run at
+/// ~10s/iteration can take up to ~500s — the old 300s bound could clip a
+/// legitimate long discovery run before the iteration cap ever got a chance
+/// to (post-merge Codex P2 finding).
+const FLOW_DISCOVER_TIMEOUT_SECS: u64 = 600;
 
 /// The canned brief handed to the `flow_discovery` agent. The agent's own
 /// archetype prompt teaches the read → correlate → ground → emit loop; this is

src/openhuman/flows/schemas.rs

@@ -1694,6 +1694,7 @@ mod tests {
                 "resume",
                 "cancel_run",
                 "list_runs",
+                "list_all_runs",
                 "get_run",
                 "prune_runs",
                 "build",
@@ -1719,7 +1720,7 @@ mod tests {
     #[test]
     fn all_registered_controllers_has_handler_per_schema() {
         let controllers = all_registered_controllers();
-        assert_eq!(controllers.len(), 32);
+        assert_eq!(controllers.len(), 33);
         let names: Vec<_> = controllers.iter().map(|c| c.schema.function).collect();
         assert_eq!(
             names,
@@ -1738,6 +1739,7 @@ mod tests {
                 "resume",
                 "cancel_run",
                 "list_runs",
+                "list_all_runs",
                 "get_run",
                 "prune_runs",
                 "build",

src/openhuman/skill_runtime/run_machinery.rs

@@ -202,6 +202,12 @@ pub async fn spawn_workflow_run_background(
             // a post-construction override rather than a pre-set on `config`
             // (which the builder would otherwise clobber).
             agent.set_max_tool_iterations(WORKFLOW_RUN_MAX_ITERATIONS);
+            tracing::debug!(
+                run_id = %run_id,
+                max_tool_iterations = WORKFLOW_RUN_MAX_ITERATIONS,
+                "[skills] workflow_run: pinned iteration budget post-construction (overrides \
+                 the session builder's orchestrator definition cap)"
+            );
             agent.set_event_context(run_id.clone(), "skill");
             agent.set_agent_definition_name(format!(
                 "orchestrator-skill-{}",