feat(workflows): HITL operator-step dashboard surfaces (#4977)#5257
Conversation
) Adds `WorkflowEngine::list_pending_operator_runs` returning every run currently paused at a `StepMode::Operator` step, paired with its inspectable `OperatorPause` (artifact + allowed actions). Runs are sorted oldest-first so the dashboard worklist surfaces the longest-waiting review at the top. Backs the new `GET /api/workflows/operator/pending` route in a separate commit — the helper takes its own dashmap reads, collects paused-run ids up front, and only then awaits the per-run `inspect_operator_pause` to avoid holding an iterator across an await. Test: `list_pending_operator_runs_returns_all_operator_paused_runs_oldest_first` covers two operator-paused runs (paired correctly with their own pauses), and asserts a manually-paused (non-operator) run is excluded from the worklist.
Adds two read-only endpoints so the dashboard can surface HITL
operator-step pauses without scanning every run:
- GET /api/workflows/runs/{run_id}/operator
Returns `{run_id, workflow_id, workflow_name, step_name,
operator_step_index, artifact, actions, started_at, paused_at}`
for a single paused run. 404 if the run is unknown, 409 (with
`error: not_operator_pause`) when paused for a different reason.
- GET /api/workflows/operator/pending
Worklist of every currently-paused operator run, oldest first,
in the same row shape so the dashboard can reuse the renderer.
Both routes go through the normal auth middleware (NOT on the public
allowlist) — the authenticated operator is the security boundary, same
as the existing POST resolve endpoint. Neither is added to the curated
`openapi.rs` paths list, matching the convention used by
`operator_action_workflow_run` / `pause_workflow_run` /
`resume_workflow_run` / `cancel_workflow_run` / `get_workflow_run`,
so `openapi.json` does not need a regen.
`operator_pause_row_json` is centralised so the single-row inspector
and the worklist endpoint render byte-identical rows — the dashboard
caches by `run_id` and switching surfaces must never change the shape.
Integration tests (`workflow_operator_action_test.rs`, 12 total
including 4 new):
- operator_http_inspect_returns_artifact_and_actions
- operator_http_inspect_unknown_run_is_404
- operator_http_inspect_non_operator_pause_is_409
- operator_http_pending_lists_all_operator_paused_runs
All exercised via the real `workflows::router()` against `TestServer`.
Surfaces paused workflow runs awaiting operator review on the
Workflows page. Two new components, both backed by the new GET
endpoints landed in the previous commit:
- `PendingOperatorReviewsBanner` — sits above the tabs; lists every
operator-paused run across all workflows (oldest first) so the
human operator lands on the page and immediately sees "N runs
waiting." Clicking a row selects the workflow + run so the action
bar inside the run detail panel is on screen.
- `OperatorActionBar` — renders inside the run detail panel when the
run state is `paused`. Shows the artifact preview and one button
per authorised action (`OperatorAction`); payload-bearing verbs
(Edit / FreeformInput / ProvideInput) expand into an inline
textarea that submits via the existing `POST .../operator`
resolve endpoint.
Data layer follows the dashboard rule: all reads through
`useWorkflowOperatorPause` / `usePendingOperatorRuns`
(`lib/queries/workflows.ts`); the write is `useResolveOperatorStep`
(`lib/mutations/workflows.ts`). Query keys live in the factory
(`workflowKeys.operatorAll/operatorPause/pendingOperator`). The
resolve mutation invalidates the inspect cache + worklist + run detail
+ all workflow lists on success so every surface picks up the
Paused→Running transition without a manual refetch.
The worklist polls every 15s in the foreground only (#3393 pattern).
The inspect query treats 404/409 as a stable "not pending" state
(does not retry) so non-operator paused runs render nothing rather
than a spinner.
WorkflowsPage tests extended (mocks for the new hooks + two new cases
covering the empty worklist + populated banner state). All 16
existing tests still pass.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba135dbceb
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <PendingOperatorReviewsBanner | ||
| onSelectRun={(runId, workflowId) => { | ||
| setSelectedWorkflowId(workflowId); | ||
| setSelectedRunId(runId); |
There was a problem hiding this comment.
Render banner-selected runs outside the first page
When an operator clicks a pending-review row for a workflow that has more than 10 runs, this only sets selectedRunId; the detail panel is rendered exclusively inside runsQuery.data.slice(0, 10), so a selected pending run that is not in that truncated/arbitrary first ten never mounts OperatorActionBar. The row click then appears to do nothing and the dashboard still provides no way to resolve that review; include the selected run in the rendered history or render the selected run detail outside the sliced list.
Useful? React with 👍 / 👎.
| .map(|r| (r.id, r.started_at)) | ||
| .collect(); | ||
| // Oldest first — longest-waiting review surfaces at the top. | ||
| paused_ids.sort_by_key(|(_, started)| *started); |
There was a problem hiding this comment.
Sort pending reviews by pause timestamp
This worklist is documented and surfaced as "oldest pause first", but it sorts by the run's started_at; if an older workflow run reaches the operator step after a newer run has already been waiting, the banner will put the newly paused old run ahead of the truly longest-waiting review. Since each paused state already carries paused_at and the API returns it, sorting by that timestamp preserves the intended operator queue order.
Useful? React with 👍 / 👎.
…4977) The kernel's `WorkflowRunState::Paused {…}` is a struct variant that serialises through serde to an externally-tagged object `{paused: {resume_token_hash, reason, paused_at}}` — not the bare string `"paused"` that unit variants (Pending / Running / Completed / Failed / Cancelled) produce. The OperatorActionBar mount guard added with the worklist UI compared `runDetailQuery.data.state === "paused"`, which silently never matched for any real paused run: the bar never appeared, and the resolution surface the PR is supposed to add was unreachable from the run-detail panel. Replace the strict-equality check with an `isPausedRunState` helper that accepts both shapes (bare string, defensive; tagged object, the real wire form) — mirrors the workaround the kernel-handle layer already uses at `workflow_runner.rs:198`. Add a vitest regression that mounts the bar against the real `{paused: {…}}` wire shape and confirms `Operator review required` + the artifact appear; without the helper the assertion fails because `<OperatorActionBar>` never renders. Out-of-scope but observed: - `list_workflow_runs` / `get_workflow_run` (pre-existing) call `serde_json::to_value(&r.state)` so the run-list also ships the tagged object for paused rows. `lastRunBadge` / `isRunState` already fall through to "unknown" for the object form — no visible breakage, but a unified `state` shape would let the badges show "Paused" too. Left for a focused follow-up since it spans the API contract.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Deploying librefang-docs with
|
| Latest commit: |
58b2474
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b8d5d4e6.librefang-docs-web.pages.dev |
| Branch Preview URL: | https://feat-workflows-hitl-steps-49.librefang-docs-web.pages.dev |
…eep runs (#4977) Two Codex P2 follow-ups from the #4977 HITL operator-step PR (round-2 review). 1) Kernel: `list_pending_operator_runs` now sorts by `WorkflowRunState::Paused { paused_at, .. }` instead of `WorkflowRun.started_at`. The worklist is surfaced to operators as "oldest pause first", and a long-running workflow can reach its operator step AFTER a shorter, newer run is already waiting — sorting by `started_at` silently put the wrong row at the top of the review banner. Regression test `list_pending_operator_runs_sorts_by_paused_at_not_started_at` constructs the divergent shape (older `started_at`, newer `paused_at`) and fails under the old sort key. 2) Dashboard: banner-selected runs that live outside the most recent 10 in run history now mount `OperatorActionBar` correctly. Before this fix the detail panel was rendered exclusively inside `runsQuery.data.slice(0, 10).map(...)`, so clicking a banner row for a run at position 11+ silently dropped the resolution UI and the operator had no way to act on the review. `displayedRuns` always includes the selected run; a "Selected review" Card above the Run History also mounts the bar as a safety net while the runs query is still loading. Regression test asserts the bar mounts for a banner-click on a deep paused row. Sibling cleanup (same one-line type fix as round-1's `isPausedRunState`): the run-history row dot/pill used `isRunState(run.state)` which returns `false` for the externally-tagged `{paused: {…}}` shape, so paused runs in the list rendered with a neutral grey "unknown" badge. Replaced with a `runStateKind()` helper that normalises both shapes and added the `paused` branch (warning hue) to the dot + pill switches. Verification: - `cargo test -p librefang-kernel --lib` — 1079 passed (incl. new test). - `cargo test -p librefang-api --lib` — 684 passed. - `cargo test -p librefang-api --test workflow_operator_action_test` (12 integration tests through `TestServer`) — pass. - `cargo clippy --workspace --all-targets -- -D warnings` — clean. - Dashboard: `pnpm typecheck` + `pnpm build` + `pnpm test --run -t WorkflowsPage` — 18/18 WorkflowsPage tests pass (includes new "outside the first 10" regression).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
💡 Codex Review
librefang/crates/librefang-kernel/src/workflow.rs
Lines 3590 to 3594 in 709ff34
When an operator step authorizes ProvideInput { field: "revision_notes" } without an output_var, this resume path only updates paused_variables when output_var is present. The documented/default ProvideInput behavior is to expose the submitted payload as {{revision_notes}}, but downstream steps will only see it as {{input}}, so named-field operator workflows render unresolved placeholders after approval.
librefang/crates/librefang-kernel/src/workflow.rs
Lines 3423 to 3426 in 709ff34
When the allowed action is ProvideInput { field: "revision_notes" }, this discriminant-only check also accepts ProvideInput { field: "anything_else" }, because all ProvideInput values share the same discriminant. That bypasses the per-field action the workflow author exposed to the operator/API client; compare the field for ProvideInput instead of accepting any value of the variant.
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Deploying librefang with
|
| Latest commit: |
5e711e6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b34d9ca5.librefang.pages.dev |
| Branch Preview URL: | https://feat-workflows-hitl-steps-49.librefang.pages.dev |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 709ff342a0
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| onSelectRun={(runId, workflowId) => { | ||
| setSelectedWorkflowId(workflowId); | ||
| setSelectedRunId(runId); | ||
| // Banner only shows on the workflows tab — flip back if the | ||
| // operator was browsing templates. | ||
| setActiveTab("workflows"); |
There was a problem hiding this comment.
Clear active search when opening banner reviews
If the operator has a search term that filters the workflow list to zero results, clicking a pending-review row only sets the selected workflow/run and switches tabs; hasWorkflows is still computed from the filtered workflows, so the workflows panel renders the empty state instead of the right-hand run detail where OperatorActionBar is mounted. In that filtered-empty state the banner click appears to do nothing and the paused run still cannot be resolved from the dashboard; clear searchQuery here or render the selected run panel based on the unfiltered workflow set.
Useful? React with 👍 / 👎.
…shboard cache invalidation Three code-review follow-ups on PR #5257 (HITL operator-step dashboard): 1. **Operator identity in audit trail** — `operator_action_workflow_run` previously only logged on `Err`; a successful approve / reject / edit emitted no event, so the audit trail had no record of WHO resolved a paused operator step. Added an optional `axum::Extension<AuthenticatedApiUser>` extractor (Option so the handler still compiles under auth-disabled installs; in production the auth middleware rejects unauthenticated callers before this point), captured the operator name + the action verb upfront, and added a structured `tracing::info!` on success with both fields. The existing `tracing::warn!` on failure now carries the same fields so an audit ship pipeline (SIEM tail) sees one event per operator decision, success or fail. 2. **Race documentation in apply_operator_outcome** — Operator click and timeout-watchdog firing the same instant is a real race window, but the state flip Paused → Running happens inside the `self.runs.get_mut(...)` critical section: only the first caller sees `is_paused() == true` and applies its outcome; the second gets a clean `Err(NotPaused)` and exits. So the existing code is idempotent. Added a doc-comment block above `cancel_operator_timeout_watchdog` explaining the critical-section guarantee so a future reader doesn't think the ordering needs to change. 3. **Scoped cache invalidation on `useResolveOperatorStep`** — `onSuccess` was invalidating `workflowKeys.all`, which cascades a refetch of every workflow list / template / unrelated run on the page. Narrowed to `operatorPause(runId)` + `operatorAll()` + `runDetail(runId)` — the three surfaces that actually change. Per CLAUDE.md "Use `.all` only when the mutation truly dirties every sub-key." Also merged latest origin/main into the branch to pick up the opentelemetry-stack alignment (#5279) so this branch compiles under the default `telemetry` feature again. Tests: `cargo check -p librefang-api -p librefang-kernel --lib`: clean. `pnpm exec vitest run src/lib/mutations/workflows`: 9 passed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58b2474419
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| qc.invalidateQueries({ queryKey: workflowKeys.operatorAll() }), | ||
| // The run itself transitions Paused → Running (and shortly to | ||
| // Completed / Failed), so the detail panel needs a refresh. | ||
| qc.invalidateQueries({ | ||
| queryKey: workflowKeys.runDetail(variables.runId), |
There was a problem hiding this comment.
Invalidate run-list caches after operator resolution
After a successful resolveOperatorStep, this mutation only invalidates operatorPause, operatorAll, and runDetail, so the workflow run-history query (workflowKeys.runs(workflowId)) stays stale until its 30s poll. In practice, the row can keep showing paused right after an approve/reject/edit, which is inconsistent with the refreshed detail panel and can mislead operators into thinking the action did not apply.
Useful? React with 👍 / 👎.
| if let Some(run) = self.runs.get(&run_id).map(|r| r.value().clone()) { | ||
| out.push((run, pause)); |
There was a problem hiding this comment.
Re-check paused state before emitting pending rows
This method can emit a run that is no longer paused: inspect_operator_pause(run_id) is awaited first (capturing a paused snapshot), then the run is fetched again and pushed without verifying it is still WorkflowRunState::Paused. If the run resumes in between those two reads, /workflows/operator/pending can briefly include a stale row that is no longer actionable and will return 409 when opened.
Useful? React with 👍 / 👎.
Summary
Completes the dashboard surface for #4977's HITL operator-step feature.
The kernel runtime, types, channel-side notify hook, timeout watchdog,
and
POST /api/workflows/runs/{run_id}/operatorresolve endpointalready landed via #5108 / #5035 / #5191 / #5244. What was missing was
any way for a human operator to discover a paused operator step and
act on it from the dashboard — the existing UI showed runs as
generically "paused" with no action surface. This PR adds the two read
endpoints + the React UX that close the loop.
Wire format (recap from the issue)
Already supported on the wire; this PR just lets a human resolve it
from the dashboard instead of via a hand-rolled
curl.Touched crates
librefang-kernel — new
WorkflowEngine::list_pending_operator_runshelper returning every operator-paused run paired with its
inspectable
OperatorPause(oldest first). Backs the worklistendpoint without holding a dashmap iterator across an await.
librefang-api — two new read-only routes (
GET /api/workflows/runs/{run_id}/operatorfor the single-run inspector,GET /api/workflows/operator/pendingfor the cross-workflowworklist). Both go through normal auth (the operator is the security
boundary; not on the public allowlist). Row JSON is centralised in
operator_pause_row_jsonso the two surfaces stay byte-identical.librefang-api/dashboard —
PendingOperatorReviewsBannerabove theWorkflows page tabs and
OperatorActionBarinside the run detailpanel. New hooks
usePendingOperatorRuns/useWorkflowOperatorPause/useResolveOperatorStep; query-keyfactory extended. The resolve mutation invalidates worklist + inspect
onSuccessso the UI catches thePaused→Running transition without a manual refetch.
Integration tests added
In
crates/librefang-api/tests/workflow_operator_action_test.rs(12tests total, 4 new):
operator_http_inspect_returns_artifact_and_actionsoperator_http_inspect_unknown_run_is_404operator_http_inspect_non_operator_pause_is_409operator_http_pending_lists_all_operator_paused_runsIn
crates/librefang-kernel/src/workflow.rs:list_pending_operator_runs_returns_all_operator_paused_runs_oldest_firstIn
crates/librefang-api/dashboard/src/pages/WorkflowsPage.test.tsx(16 total, 2 new):
does not render the pending-operator banner when the worklist is emptyrenders the pending-operator banner with row counts when the worklist has entriesVerification
cargo check --workspace --lib— clean.cargo clippy -p librefang-kernel -p librefang-api --all-targets -- -D warnings— zero warnings.cargo test -p librefang-kernel --lib workflow::tests::list_pending_operator_runs_returns_all_operator_paused_runs_oldest_first— 1 passed.cargo test -p librefang-api --test workflow_operator_action_test— 12 passed (including 4 new GET-endpoint tests).pnpm vitest run pages/WorkflowsPage.test— 16 passed.pnpm build(dashboard) — clean.openapi.jsonunchanged: the new GET routes follow the existingprecedent (the entire workflows-runs-CRUD family is intentionally
absent from the curated
openapi.rspaths list).Out-of-scope follow-ups (intentionally split)
Per the issue body and @houko's sequencing plan, two channel-side
pieces remain. Both require sidecar-protocol additions and are better
landed independently to keep this PR focused on the dashboard surface:
Telegram inline-keyboard buttons on the existing operator notify
message. Today the kernel→channel hook (
KernelOperatorBridge→send_channel_message) delivers the artifact + anActions: approve, edittext footer. The dashboard UX in this PR is theprimary resolution surface; channel buttons are a UX upgrade for
operators who prefer not to leave Telegram. Will file as a follow-up
issue once this lands.
Inbound channel reply → operator resolve. Capturing "operator
replied APPROVE in the Telegram thread" and routing it to the same
resolve_operator_steppath as the dashboard POST. Spec'd in theissue body's "How it works" section step 4; depends on a normalised
OperatorReplyshape across channels (one of the open questions inthe issue thread). Same follow-up issue as (1).
Closes part of #4977 — the issue remains open until (1) and (2) above
ship, but this PR delivers the operator-discovery + dashboard
resolution surface, which is the gap that was blocking real use.