fix(flows): live-run reliability — reconcile orphaned/cancelled runs + detach agent-initiated run_flow (B41, B42) - #5135
Merged
graycyrus merged 5 commits intoJul 23, 2026
Conversation
…+ detach agent-initiated run_flow (B41, B42)
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe PR adds ChangesInterrupted status contracts and UI
Run execution and recovery
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Action performedReview finished.
|
coderabbitai
Bot
added
bug
rust-core
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/lib/i18n/id.ts`:
- Line 4149: Replace the Indonesian interrupted-run translation with the
distinct lifecycle term “Terhenti” for both flowRuns.status.interrupted and
flows.allRuns.status.interrupted in app/src/lib/i18n/id.ts at lines 4149-4149
and 4176-4176; update both sites consistently and leave the
channels.status.disconnected translation unchanged.
In `@app/src/services/api/flowsApi.ts`:
- Around line 59-64: Update the FlowRun field documentation in
app/src/services/api/flowsApi.ts at lines 59-64 so error and settlement metadata
explicitly include interrupted runs. Also update the finished_at and error field
documentation in src/openhuman/flows/types.rs at lines 331-340 to include
interrupted alongside the existing terminal statuses; no behavioral changes are
needed.
In `@src/openhuman/flows/ops.rs`:
- Around line 3937-3960: Update RunRowFinalizer::drop to call the synchronous
store::record_run after reconciling the orphaned run as "interrupted", matching
the other interrupted and terminal paths. Pass the same flow, thread, status,
and interruption details used by finish_flow_run_row so
flow_definitions.last_status and last_run_at are updated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 84f9775d-4c27-449f-9e96-0aaf11d1a320
📒 Files selected for processing (25)
app/src/components/flows/FlowRunInspectorDrawer.tsxapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.tsapp/src/pages/WorkflowRunsPage.tsxapp/src/services/api/flowsApi.tssrc/core/runtime/services.rssrc/openhuman/flows/ops.rssrc/openhuman/flows/ops_tests.rssrc/openhuman/flows/run_registry.rssrc/openhuman/flows/store.rssrc/openhuman/flows/store_tests.rssrc/openhuman/flows/tools.rssrc/openhuman/flows/types.rs
…top (B42 review) The new `interrupted` terminal status (bug B42) was missed in three terminal-status consumers, none caught by TypeScript (two are `Set`s, not exhaustive `Record`s): - `flows_cancel_run`'s "already terminal" guard omitted `interrupted`, so cancelling an already-interrupted run fell through to the not-in-flight path and clobbered the row (and flow summary) to `cancelled`, discarding the interruption reason. Added to the match + a regression test. - `useFlowRunPoller`'s `TERMINAL` set omitted `interrupted`, so the inspector drawer would poll a settled interrupted run forever at 3s. Added + test. - `useFlowRunsLiveRefresh`'s `TERMINAL_STATUSES` omitted `interrupted`, so the runs-list active-run backstop poll never quiesced. Added.
graycyrus
commented
Jul 22, 2026
graycyrus
left a comment
graycyrus
left a comment
Contributor
Author
There was a problem hiding this comment.
Fresh CodeRabbit-style review (self-generated, DRAFT PR). I applied the clear/safe terminal-status gaps directly (commit f18a2e9 — see PR conversation summary); the three items below are concurrency/design observations I am flagging rather than fixing unilaterally.
…rop-guard summary, docs CodeRabbit + self-review follow-ups on the B41/B42 live-run reliability work. - Boot-sweep TOCTOU: the sweep distinguished a live run from an orphan solely via `run_registry::is_in_flight`, but both entry points insert the `running` row before `run_flow_body` registers — so a row is briefly running-but-not- in-flight. An external-trigger run firing in that boot window could be flipped to `interrupted` AND have its durable checkpoint dropped mid-run (the live run's own terminal write fixes the status but cannot restore the checkpoint). Adds `PROCESS_RUN_FLOOR`, anchored by `start_flow_run_row` before it stamps the row it inserts, and narrows `list_running_run_ids` to `started_at < floor`. Rows this process created are now provably out of scope regardless of registration timing; `is_in_flight` stays as defence in depth. - Drop guard now calls `store::record_run` after reconciling, matching the boot sweep and every other terminal arm — otherwise the runs list kept advertising the previous run's `last_status`/`last_run_at`. - Boot reconciliation moved out of `spawn_cron_service` into a dedicated `spawn_flows_boot_reconcile`, selected by the flows *domain* flag rather than by cron's presence in the `ServiceSet`. Runs can start without cron, so their orphans must be reconcilable without it; the new floor removes the ordering requirement that motivated the cron placement. - `id.ts`: interrupted runs read `Terhenti`, not `Terputus` (already the Indonesian for a *disconnected channel* — it made an interrupted run look like a connectivity failure). - `FlowRun.error` / `finished_at` docs (both `types.rs` and `flowsApi.ts`) now state that they cover `interrupted` too. Tests: boot_sweep_skips_a_run_started_after_the_process_floor (pins the TOCTOU window directly), list_running_run_ids_excludes_rows_started_at_or_after_the_floor (exclusive floor), and the drop-guard test now asserts the flow summary.
graycyrus
marked this pull request as ready for review
July 23, 2026 11:30
coderabbitai
Bot
removed
the
rust-core
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/runtime/services.rs`:
- Around line 122-140: Update spawn_flows_boot_reconcile to add grep-friendly
logs when scheduling the boot reconciliation task, after successful config
loading, and after sweep_orphaned_running_runs_on_boot completes, including the
no-op case where swept is zero. Preserve the existing positive-sweep info log
and config-load warning, and ensure the messages distinguish scheduling, config
readiness, and completion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 86a1e43d-54c1-41c9-a412-98b5a07abfb1
📒 Files selected for processing (12)
app/src/hooks/__tests__/useFlowRunPoller.test.tsapp/src/hooks/useFlowRunPoller.tsapp/src/hooks/useFlowRunsLiveRefresh.tsapp/src/lib/i18n/id.tsapp/src/services/api/flowsApi.tssrc/core/runtime/builder.rssrc/core/runtime/services.rssrc/openhuman/flows/ops.rssrc/openhuman/flows/ops_tests.rssrc/openhuman/flows/store.rssrc/openhuman/flows/store_tests.rssrc/openhuman/flows/types.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- src/openhuman/flows/types.rs
- app/src/lib/i18n/id.ts
- src/openhuman/flows/store.rs
- app/src/services/api/flowsApi.ts
- src/openhuman/flows/ops_tests.rs
- src/openhuman/flows/ops.rs
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6494445ee4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
coderabbitai
Bot
added
the
rust-core
|
| Filename | Overview |
|---|---|
| src/openhuman/flows/ops.rs | Core changes: flows_run_detached, RunRowFinalizer drop-guard, PROCESS_RUN_FLOOR, sweep_orphaned_running_runs_on_boot, and refactored run_flow_body. The finalizer covers the awaiting region correctly; the 'not in flight' cancel path uses an unconditional finish_flow_run_row that could be clobbered in a narrow race. |
| src/openhuman/flows/store.rs | Adds list_running_run_ids (with started_before floor filter) and mark_run_interrupted (with AND status='running' predicate guard) to support the boot sweep; both are well-guarded and tested. |
| src/openhuman/flows/tools.rs | Switches RunFlowTool::execute from flows_run to flows_run_detached; tool result shape changed to workflow_run_started with polling note; error case updated. |
| src/openhuman/flows/run_registry.rs | Adds is_in_flight query function used by the boot sweep to protect live runs from reconciliation; clean and simple addition. |
| src/openhuman/flows/types.rs | Extends FlowRun.status doc to include interrupted; updates finished_at and error field docs accordingly. |
| src/core/runtime/services.rs | Adds spawn_flows_boot_reconcile with correct feature-flag gating; loads config then runs the sweep in a background task. |
| src/core/runtime/builder.rs | Wires spawn_flows_boot_reconcile into startup, gated on the flows domain flag rather than the cron service flag — correct placement. |
| src/openhuman/flows/ops_tests.rs | Good coverage: drop-guard, disarm, boot sweep, PROCESS_RUN_FLOOR window, detached registration race; the detached test does not assert the background task eventually settles the row (noted in prior review). |
| src/openhuman/flows/store_tests.rs | New store-layer tests for list_running_run_ids (status filter, floor exclusion) and mark_run_interrupted (happy path + idempotent no-op on terminal row). |
| app/src/hooks/useFlowRunPoller.ts | Adds interrupted to the TERMINAL set; poller correctly stops on interrupted runs. |
| app/src/hooks/useFlowRunsLiveRefresh.ts | Adds interrupted to TERMINAL_STATUSES so the backstop poll quiesces correctly. |
| app/src/components/flows/FlowRunInspectorDrawer.tsx | Adds interrupted to FLOW_RUN_STATUS_ACCENT, FLOW_RUN_STATUS_DOT, and FLOW_RUN_STATUS_KEY with amber settled styling; previously-noted 'Fix with agent' gap remains. |
| app/src/services/api/flowsApi.ts | Adds interrupted to FlowRunStatus union; extends FlowRun.finished_at and FlowRun.error field docs with B42 semantics. |
| app/src/pages/WorkflowRunsPage.tsx | Adds interrupted to STATUS_CLASS with amber styling matching pending_approval. |
Sequence Diagram
sequenceDiagram
participant Agent as Agent / Harness
participant Tool as RunFlowTool
participant Ops as flows::ops
participant Reg as run_registry
participant DB as SQLite (flow_runs)
participant BG as Background Task
Note over Agent,BG: B41 — Detached agent-initiated run
Agent->>Tool: run_flow(flow_id, input)
Tool->>Ops: flows_run_detached(...)
Ops->>Ops: prepare_flow_run() — validate + compile-check
Ops->>Reg: register(thread_id) → (cancel_token, run_guard)
Ops->>DB: "start_flow_run_row → status=running"
Ops->>BG: tokio::spawn(run_flow_body(...))
Ops-->>Tool: "{ run_id, status:running, detached:true }"
Tool-->>Agent: workflow_run_started + polling note
Note over BG,DB: B42 — Drop-guard guards the await
BG->>BG: "RunRowFinalizer::new (armed=true)"
BG->>BG: "tokio::select! { cancel_token | timed_engine }"
alt Run completes / fails / times out / is cancelled
BG->>DB: finish_flow_run_row(terminal_status)
BG->>BG: finalizer.disarm()
else Future dropped mid-await (harness abort, shutdown)
BG--xBG: future dropped
BG->>DB: RunRowFinalizer::drop → finish_flow_run_row(interrupted)
BG->>Reg: RunGuard::drop → deregister
end
Agent->>Ops: get_flow_run(run_id)
Ops->>DB: SELECT status
DB-->>Ops: terminal status (completed/failed/interrupted/…)
Ops-->>Agent: "FlowRun { status }"
Note over Ops,DB: Boot sweep (B42b) — handles hard crash / SIGKILL
Ops->>DB: "list_running_run_ids(started_before=PROCESS_RUN_FLOOR)"
loop each orphaned row
Ops->>Reg: is_in_flight(run_id)?
alt Not in-flight
Ops->>DB: "mark_run_interrupted (AND status=running guard)"
end
end
Reviews (2): Last reviewed commit: "fix(flows): register detached runs befor..." | Re-trigger Greptile
Codex P2 on PR tinyhumansai#5135, plus CodeRabbit's boot-reconcile logging note. `flows_run_detached` returned the `run_id` to the agent while registration still happened inside the spawned `run_flow_body`. A `flows_cancel_run` landing in that gap — an ordinary user action, since the agent hands the run_id straight to the UI — saw `is_in_flight == false`, took the "parked/stale" branch, wrote a terminal `cancelled` row and dropped the checkpoint. The background task then started anyway, executed the flow's real side effects, and overwrote `cancelled` with its own terminal status. A cancelled run that still runs, and reports as something else afterwards. Registration moves to both entry points, ahead of `start_flow_run_row`, and the token + guard are passed into `run_flow_body`. A cancel on a returned run_id now always takes the signalled branch and the run's own cancellation arm unwinds it. Pinned by flows_run_detached_registers_the_run_before_returning_its_id, which fails if registration moves back inside the task. This also closes the sweep's insert-before-register window at the source. PROCESS_RUN_FLOOR stays — it holds regardless of what callers do with that ordering, and prior-process rows are what the sweep is actually for. Both doc blocks that described the old ordering are updated rather than left stale. Also: unconditional entry/config/completion debug logs in spawn_flows_boot_reconcile, so a no-op sweep is distinguishable from a task that never ran (per the project's debug-logging rule).
coderabbitai
Bot
removed
bug
rust-core
This was referenced Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
run_flowno longer orphans theflow_runsrow. A newRunRowFinalizerdrop-guard reconciles arunningrow to a terminalinterrupted(with a human reason) if the run future is dropped mid-await; a boot-time sweep reconciles crash/restart orphans. The run-details sidebar shows "Run interrupted…" instead of a perpetual blank spinner.run_flownow detaches instead of blocking past the tinyagents harness's 120s per-tool-call cap. It validates + compile-checks synchronously, then returns{ run_id, status: "running" }immediately and runs the flow on a background task; the copilot pollsget_flow_run(run_id)(which it already does).interruptedthreaded through the run type, store, and the run-details / runs-list UI (+ i18n for all 14 locales).run_flow_body; the synchronous RPC "Run" and trigger-bus paths keep their existing behavior, now also covered by the drop-guard.Problem
Ref
my_docs/flows_workflow_bugs.mdB41 and B42.flows_runbracketed the long engine.awaitbetweenstart_flow_run_row(insertsrunning) andfinish_flow_run_row(writes terminal status). When the future was dropped mid-await (harness 120s abort, chat turn end, panic, or a hard crash),finish_flow_run_rownever ran — the row stayedstatus=running,error=NULL,steps=[]forever. The sidebar read that as a perpetually-running, blank run; the real failure lived only in the copilot's agent trace.RunFlowTool::executefully.awaitedflows_runwith no internal budget. The 120s cap is the tinyagents harness's default per-tool-call timeout, so any flow whose first real node is a live-research agent node (web_search+web_fetch+parallel_research) exceeded it deterministically — agent-initiated test runs of realistic flows could never succeed, and each 120s abort produced a B42 orphan.Solution — architect decision for B41: DETACH (preferred option)
I chose the strongly-preferred detach approach over bumping a timeout, after reading the code:
FLOW_RUN_TIMEOUT_SECS) is already 600s — the 120s that killed runs is the harness per-tool-call timeout, not ours. Bumping it is impossible (it's tinyagents' cap) and blocking the agent's turn for up to 10 min would starve every other tool call in that turn.RunFlowToolnow calls a newflows_run_detached, which:runningrow + publishesFlowRunStarted, then spawnsrun_flow_bodyon a background task and returns{ run_id, status: "running", detached: true }in well under 120s.get_flow_run(run_id)and not assume success — which the copilot already does (verified in the live traces cited in the bug doc).flows::bus::spawn_run) already fire runs fire-and-forget. The synchronousflows_run(RPC "Run" button + trigger dispatch) is preserved unchanged in behavior.Why detach is safe here specifically: it composes with B42. A detached run is no longer cancelled at 120s (the tool returned already), so it reaches its natural terminal write; and if the process dies mid-run, the drop-guard (soft shutdown) or the boot sweep (hard crash) always settles the row. Detach without B42 would have moved the orphan window, not closed it — so both ship together.
B42 implementation
RunRowFinalizer) — held across the engine await insiderun_flow_body. OnDropwithout a priordisarm(), it writesstatus="interrupted"+ a human reason and preserves any live-observed steps. Every real terminal path (success / failure / timeout / cancel / pause)disarm()s it after its own write. Uses a single-taskCellflag so the type staysSendfortokio::spawn.sweep_orphaned_running_runs_on_boot) — wired into core startup (services.rs, before the cron gate so it runs even on cron-disabled/slim configs). Reconciles everyrunningrow that no live in-process run owns (checked viarun_registry::is_in_flight, so a run this process already started is never swept out from under itself) tointerrupted, updates the flow summary, emitsFlowRunFinished, and drops the stale checkpoint.interruptedstatus carries theerrorreason; the run-details sidebar renders the error banner and a settled (non-spinner) state; runs-list + inspector get an amber "worth a look" pill and i18n label in all 14 locales.record_runfailure can no longer leave the row wedged atrunning.Impact
interruptedis a new value in the already free-formstatuscolumn.interruptedgated correctly in theflows-off slim build (--no-default-features), which compiles clean.run_flowreturnsstatus:"running"+run_idinstead of a completed result; the model is instructed to poll. Cancelled/timed-out/crashed runs now settle tointerruptedinstead of hanging atrunning.Submission Checklist
diff-cover) meet the gate enforced by.github/workflows/ci-lite.yml. Runpnpm test:coverageandpnpm test:rustlocally; PRs below 80% on changed lines will not merge.docs/TEST-COVERAGE-MATRIX.mdreflect this change (orN/A: behaviour-only change) —N/A: reliability fix to an existing feature (Flows run lifecycle), no new feature row## Relateddocs/RELEASE-MANUAL-SMOKE.md) —N/A: no change to release-cut smoke surfacesCloses #NNNin the## Relatedsection —N/A: tracked in my_docs/flows_workflow_bugs.md (B41/B42), no GitHub issueRelated
my_docs/flows_workflow_bugs.md→ B41, B42flows::ops::flows_run/run_flowagent tool / run-details sidebar)AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/flows-live-run-reliabilityValidation Run
pnpm --filter openhuman-app format:check— Rustcargo fmt --checkclean; Prettier N/A (no manual reformatting)pnpm typecheckcargo test -p openhuman --lib flows::(614 pass, incl. 8 new); flows run-status Vitest specs (24 pass)cargo fmt --check+cargo check(default +--no-default-featuresslim) cleanapp/src-taurichangesValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
run_flowdetaches and returnsrunning+run_id; cancelled/timed-out/crashed flow runs reconcile to a terminalinterruptedwith a reason.Summary by CodeRabbit
New Features
Bug Fixes