feat(workflows): execution debug UI, live progress, re-run, and fixes - #6504

The re-run button no longer actually re-runs anything. handleRerun(run.input) (line 644) only pre-fills paramValues/runInput from the previous run's stored input and scrolls to the run section — it never calls rerunWorkflowRun or the useRerunWorkflowRun mutation. useRerunWorkflowRun is still exported from src/lib/mutations/workflows.ts:64 (with correct onSuccess/invalidateQueries wiring per the dashboard-mutations rule in CLAUDE.md) but this page no longer imports or calls it anywhere — grep confirms its only other reference is the stale mock in WorkflowsPage.test.tsx. So the mutation hook this PR's own commit history added for "add missing rerunWorkflowRun API function" is dead code, and the previously-working one-click re-run (which actually queued a new run server-side) is gone, silently replaced by a "prefill + scroll, then the user must click Run again" flow. If that behavior change is intentional it should be reflected in the PR description and the button's tooltip/telemetry, and the orphaned hook/API function should be removed or wired up.

WorkflowsPage.test.tsx (untouched by this PR, so not diff-commentable directly) has an existing test — "surfaces run parameters + error inline and re-runs with the same params (#6292)" at lines 273-304 — that now fails against this branch. I verified by running pnpm install && npx vitest run src/pages/WorkflowsPage.test.tsx in this worktree: 1 failed / 18 passed, failing at the assertions in that test.

Causes, all traceable to this PR's changes:

1. Test line 294 expects text /sector: fintech/ (old formatRunParamsPreview format), but the new inline inputPreview logic here now renders sector=fintech (=, not : ).
2. Test line 295 expects the run-history row to show the run-level error inline; that block was intentionally removed in this PR.
3. Test line 298 does getByLabelText("Re-run with same parameters") — the new re-run button (~line 1357) only has a title ("Re-run with these parameters"), no aria-label, so this query no longer matches.
4. Test lines 299-303 assert mutations.rerun.mutateAsync was called with {runId, workflowId} — handleRerun here never calls the rerun mutation (see separate comment on the button), so this assertion fails outright.

This test wasn't updated alongside the behavior/markup changes and is red on this branch — worth fixing before merge per the repo's testing expectations.

This new block (2430-2444) stamps sender_chat_id into manifest.metadata, with the comment "Mirrors the identical block in send_message_full_inner so the streaming and non-streaming paths stay in sync." But send_message_full_inner isn't the relevant comparison — this code is inside send_message_streaming_with_sender_and_opts itself (spans lines 2007-3503), and that same function already stamps sender_chat_id from the same sender_context.chat_id about 300 lines later, at line 2736-2742 (part of the pre-existing block starting at line 2708 that also stamps sender_user_id, sender_channel, and — for the #6443 cross-tenant channel_send guard — SENDER_ACCOUNT_ID_METADATA_KEY). Nothing between the two blocks reads manifest.metadata["sender_chat_id"], so this new insertion is redundant dead code within the same function it claims to be syncing with.

More importantly, this new block only stamps sender_chat_id and not the sibling sender_channel/sender_account_id keys. If a future cleanup treats this new block as "the" sender-context stamping site and removes/refactors the block at line 2708 (since this one looks like it already does the job), the streaming path would silently stop populating sender_channel/sender_account_id — which run_streaming.rs reads back for the #6443 cross-tenant channel_send guard. Worth removing this duplicate block (or, if there's a reason sender_chat_id genuinely needs to be set earlier than line 2708 for something between the two — e.g. async task registration — that reason isn't evidenced in this diff and should be called out in the comment).

current_step_index and total_steps are hardcoded to None/0 here instead of being reconstructed from persisted data. row_to_workflow_run is invoked from load_runs_from_sqlite (line ~1667) at boot to hydrate every persisted run directly into the live self.runs DashMap — so after any daemon restart, every historical WorkflowRun, including completed ones, permanently reports total_steps: 0 for the rest of that process's lifetime (nothing re-derives it later for a terminal run).

This contradicts the field's own doc comment on the struct: "Total number of steps in the workflow (copied at creation so the UI can show 'Step 2/4' without loading the definition separately)" — that promise only holds for runs created during the current process, not ones reloaded from storage. Cross-checking WorkflowRunRow in crates/librefang-memory/src/workflow_store.rs:21 confirms there's no total_steps/current_step_index column at all, so this isn't a wiring bug so much as the persistence layer never having been extended for these new fields.

The dashboard's totalSteps = rd.total_steps || allSteps.length fallback (WorkflowsPage.tsx) partially masks this for a fully-completed run (step_results.length happens to equal the real total), but understates the plan for a run that failed partway through and is then viewed after a restart — it'll show e.g. "Step 3/3" instead of "Step 3/5", implying full completion when the run actually failed midway.

This PR only needs ~6 new keys for the workflow debug UI (rerun_hint, step_executing, starting, variables, step_output, step_pending), but the uk.json diff touches 94 lines — far beyond that, and several of the unrelated edits are outright typos introduced by this PR, regressing previously-correct Ukrainian text:

• Line 1394: показує → показувє (misspelled — not a valid Ukrainian word).
• Line 1043: обов'язкові → обов'язковикими (nonsense word).
• Line 1754: обов'язковим → обов'язковій (wrong grammatical case).
• Line 2342: середовища → середоваща (misspelled).
• Line 1637 (pending_review_other): plural form запусків → singular запуску, which is grammatically wrong for the "other" plural category (compare the correct pending_review_many on the line above, which still says запусків).
• "nominal": Норма → Номінальний, and "tools": Інструменти → Тули (regressing a proper Ukrainian noun to an anglicism), both unrelated to workflows.

This is scope creep per CLAUDE.md's "one PR ↔ one issue" rule, and it's actively making the localization worse, not just touching unrelated lines. Looks like it may have come from an automated whole-file relocalization pass rather than a targeted key addition — worth reverting to just the new keys.

Same scope-creep pattern as uk.json (see that file's comment): this PR only needs a handful of new workflow keys, but zh.json has 83 unrelated lines changed, several of which regress already-correct Chinese translations back toward raw English or change meaning:

• Line 247: "profile": "配置" → "profile": "Profile" (reverts a translated term to English).
• Line 297: "from_form": "从表单" (literally "from form") → "可视化" ("visualization") — changes the meaning, not just wording.
• Line 340: "temperature": "温度" → "temperature": "Temperature" (reverts to English).
• Line ~958-1009 area: "通道" ("channel", already-translated) swapped for the raw loanword "channel" in several strings (all_configured, connect_first, empty_title, picker_title), inconsistent with the rest of the file which still uses 通道.
• remove_confirm: full-width ？ changed to half-width ? mid-sentence, inconsistent with Chinese punctuation conventions used elsewhere in the same file.

None of this is related to the workflow execution debug UI this PR is about, and it net-regresses existing translations rather than just being unrelated churn.

async_mode now defaults to true, but the agent_send tool's own input_schema description (crates/librefang-runtime/src/tool_runner/definitions.rs ~line 318-330, unchanged by this PR) still tells the model: "By default this BLOCKS until the agent replies and returns their response" and the async param description says "Defaults to false (blocking)." That text reaches the LLM prompt directly. After this change an agent that omits async (trusting the tool's own docs) will get an immediate task_id back instead of a blocking reply, which can silently break any skill/workflow logic that expects the response inline this turn. Please update the tool description/schema text to match the new default (or keep the default false and instead raise tool_timeout_secs / fix the timeout at the call site the commit message describes).

This button's tooltip is t("workflows.rerun_hint", { defaultValue: "Re-run with these parameters" }) and it looks like a one-click re-run, but handleRerun (lines 644-664) no longer calls the backend re-run mutation — it only pre-fills paramValues/runInput from the stored run and scrolls to the Run section, requiring a second manual click on "Run" to actually execute anything. As a result useRerunWorkflowRun (lib/mutations/workflows.ts) and rerunWorkflowRun (api.ts) are now dead code — grepping the dashboard, they're referenced only in api.ts / lib/http/client.ts / lib/mutations/workflows.ts, never imported into this page anymore (the useRerunWorkflowRun import was removed from this file). Per the dashboard data-layer rule this orphaned mutation should either be wired back in (if a true one-click backend re-run is still wanted) or deleted along with its API function, and the button copy should say something like "Load parameters" if the new pre-fill-only behavior is intentional — right now the label and the behavior disagree.

Ordering bug: this new "resolve default provider/model" block mutates a clone (manifest, created at line 2391) — but resolve_driver(&entry.manifest) (line 2374, unchanged) and the ctx_window catalog lookup (cat.find_model_for_manifest(&entry.manifest.model.provider, &entry.manifest.model.model), line 2381) both already ran before this block, against the un-mutated entry.manifest. For exactly the agents this fix targets (post-boot provider="default"/model="default", or the auto-spawned "assistant"), the catalog will never find a row literally named "default", so ctx_window silently falls back to None (caller default) on the streaming path instead of the real model's context window — undermining compaction-size accuracy for the very population this fix is meant to help. Contrast with the later model_supports_tools lookup a few dozen lines down, which correctly reads the mutated manifest because it happens to sit after this block. Suggest moving this default-resolution block above resolve_driver/ctx_window, or recomputing ctx_window from manifest after this block runs.

This PR redesigns re-run entirely — handleRerun now only pre-fills local form state (paramValues/runInput) and scrolls to #workflow-run-section, instead of calling the useRerunWorkflowRun mutation (which is no longer imported/used anywhere in this component). But crates/librefang-api/dashboard/src/pages/WorkflowsPage.test.tsx is untouched by this diff and still has a test built entirely around the old behavior (surfaces run parameters + error inline and re-runs with the same params (#6292), lines ~292-301):

• screen.getByLabelText("Re-run with same parameters") — no element has that accessible name anymore; the new button only has a title ("Re-run with these parameters"), no aria-label. This lookup should throw.
• expect(mutations.rerun.mutateAsync).toHaveBeenCalledWith({runId, workflowId}) — handleRerun here never calls the mutation, so this can't pass.
• screen.getByText(/sector: fintech/) — the new inputPreview builder in this file formats params as k=v (e.g. sector=fintech), not k: v.
• screen.getByText("step 'analyze' failed: provider 500") — the per-row inline failure text block this asserted on was deleted by this same PR.

Since this test isn't in the diff, it's easy to miss that it now targets behavior that no longer exists. Worth confirming it still passes (I'd expect it to fail outright at the getByLabelText call), and updating/replacing it to match the new pre-fill-and-scroll flow.

Separately: with no caller left in the dashboard, useRerunWorkflowRun (lib/mutations/workflows.ts) and rerunWorkflowRun (api.ts) look like dead code now — intentional (kept for a future direct-rerun surface) or should they come out with this test?

The same literal message — "You are on the LibreFang web interface. Files, images, and media you generate are shown to the user automatically in your response" — is emitted for all three system channels matched at line 1027 ("webui" | "cron" | "autonomous"), but that statement is only true for webui. For cron and autonomous fires there is no web interface and typically no live viewer watching a response stream at all — telling the agent it's "on the LibreFang web interface" during a scheduled/autonomous run is factually wrong and could mislead the agent's behavior (e.g. it may reason a human is watching the page right now).

This file already treats cron and autonomous as distinct from webui elsewhere — see agent_loop/mod.rs (Some("cron") => Some("[Scheduled trigger]\n"), Some("autonomous") => Some("[Autonomous trigger]\n")), which gives each its own framing. This block collapses all three into one webui-flavored message instead of following that precedent. Consider three distinct messages (or at least a channel-neutral one for cron/autonomous, e.g. "Files/media you generate are not deliverable via channel_send here — do not use it"), so the fix doesn't trade one confusing prompt for another.

row_to_workflow_run hardcodes total_steps: 0 for every run reloaded from persisted storage (SQLite), rather than deriving it from the workflow definition or a persisted value. load_runs_from_sqlite calls this for every row at daemon boot to repopulate the in-memory DashMap that get_run/get_workflow_run read from — so after any daemon restart, every run that existed before the restart (not just currently-running ones) reports total_steps: 0 forever, since nothing ever corrects it post-load.

This directly undermines two things this same PR adds:

1. The dashboard's "Step X/Y" live-progress display (WorkflowsPage.tsx: const totalSteps = rd.total_steps || allSteps.length;) — for a reloaded run this falls back to allSteps.length, which happens to be numerically right for completed runs but wrong for anything still "pending" at the instant of reload.
2. The "N steps defined, 0 executed" empty-state added in 6db34156 for failed runs — for a reloaded failed run with 0 executed steps, rd.total_steps (0) and allSteps.length (0) are both 0, so the UI loses the "N steps defined" info this feature exists to show, exactly for the runs (old/persisted) where a human is most likely to be looking it up after the fact.

The doc comment on the total_steps field says it's "copied at creation so the UI can show 'Step 2/4' without loading the definition separately" — that promise is broken the moment the daemon restarts. Consider looking up workflow.steps.len() via workflow_id (if the definition is still available) or persisting total_steps itself in WorkflowRunRow and reading it back here instead of hardcoding 0.

This hunk removes source?: string; (and its "Provenance hint…" comment) from ModelItem, unrelated to anything else in this PR — looks like a stray revert picked up during a rebase/merge rather than an intentional change.

ModelsPage.tsx still reads it: const isCustom = m.tier === "custom" && m.source !== "cli_config"; (line 189), used to decide whether a live-detected CLI model gets a delete control. With source gone from the ModelItem type, that access no longer type-checks against the interface, which should break the dashboard TS build for a page this PR never intended to touch. Please restore the source?: string; field (and ideally the removed context comment) unless there's a reason it needs to go that isn't visible from this diff.

rerunWorkflowRun already existed on main (with LONG_RUNNING_TIMEOUT_MS and a detailed doc comment explaining the backend re-reads the stored run's input) — this "add missing API function" commit re-adds it here with DEFAULT_POST_TIMEOUT_MS (60s) instead, then deletes the original definition later in the file. That's a functional regression: the deleted comment explicitly said LONG_RUNNING_TIMEOUT_MS was needed because this call "queues a multi-step LLM run." A rerun that takes longer than 60s to queue/respond will now hit the client timeout where it previously had 5 minutes.

Separately: WorkflowsPage.tsx's handleRerun no longer calls useRerunWorkflowRun/this function at all (see the handleRerun rewrite around line ~628 of the page diff) — it now just pre-fills the run form and scrolls to the Run button, leaving useRerunWorkflowRun (in lib/mutations/workflows.ts) with zero callers in the dashboard. Worth confirming whether keeping this endpoint/hook around unused is intentional (e.g. for future API consumers) or leftover from an earlier iteration of this PR — and if kept, the timeout regression above still needs fixing.

Generated by Claude Code

Flipping the default here to non-blocking makes the LLM-facing tool schema stale: crates/librefang-runtime/src/tool_runner/definitions.rs:332 still describes the async parameter as "Defaults to false (blocking)". That description is part of the ToolDefinition sent to the model, so the agent is now told the opposite of the actual default behavior for its own tool call. Since this description reaches the LLM prompt (deterministic-prompt-ordering territory per CLAUDE.md's #3298 note, though this is a content-correctness issue rather than an ordering one), please update the schema text in definitions.rs alongside this default change.

Generated by Claude Code

t("workflows.console", ...) has no matching key in any locale file — en.json/uk.json/zh.json only have a nav.console key (pre-existing, unrelated Terminal-nav label), not workflows.console. This silently falls back to the English defaultValue for every locale, unlike the other new strings this PR added (rerun_hint, step_executing, starting, variables, step_output, step_pending), which were correctly added under workflows.* in all three locale files. Please add workflows.console to en.json/uk.json/zh.json so the log-console header actually localizes.

Generated by Claude Code

New response fields (current_step_index, total_steps here and in list_workflow_runs, plus variables on step results) don't have a #[tokio::test] asserting their presence/shape. Per CLAUDE.md's MANDATORY Integration Testing section this is exactly the kind of route-shape change that should get a TestServer test — crates/librefang-api/tests/workflows_routes_integration.rs (the file with the existing workflow-run route tests) wasn't touched by this PR. A test that runs a workflow and asserts current_step_index advances / clears and that variables shows up on a step result would also validate the new live-progress polling UI end-to-end (right now nothing exercises the row_to_workflow_run fallback of total_steps: 0 either).

Generated by Claude Code

The comment above this block says it "Mirrors the resolution in execute_llm_agent", but it doesn't: execute_llm_agent's equivalent block (crates/librefang-kernel/src/kernel/agent_execution.rs:610-614) only overrides when is_default_provider && is_default_model. This streaming-path copy adds || is_auto_spawned, which is a real behavioral divergence, not just a mirror.

Effect: for any agent literally named "assistant" whose manifest.description starts with "General-purpose assistant" (the boot-installed default from kernel/boot.rs:2531), this condition is true regardless of is_default_provider/is_default_model. So even after a user explicitly reconfigures their default assistant to a specific provider/model (a very common action — arguably the single most likely agent to get a deliberate model choice), every streaming turn silently overwrites manifest.model.provider/.model back to cfg.default_model (or the hot-reloadable override). The non-streaming path (execute_llm_agent) does not do this, so the two paths now disagree on which model an explicitly-configured "assistant" agent actually uses depending on whether the turn came in via streaming or not.

Suggest dropping the || is_auto_spawned clause to actually match execute_llm_agent, or if there's a real reason the auto-spawned assistant needs unconditional override even with an explicit model set, that reasoning belongs in the comment (and should probably also be added to execute_llm_agent for consistency, since right now only one of the two "mirrored" paths has it).

Generated by Claude Code

This rewrite of channels.schema_unavailable_hint ("This channel runs as an out-of-process sidecar…" → "This channel now runs as an out-of-process sidecar… Install the sidecar SDK, then reload channels…") is unrelated to the workflow execution debug UI this PR is about — it's channels-sidecar setup copy, not touched by anything else in this diff (ChannelsPage.tsx isn't in the changed-files list). Same category of issue already flagged elsewhere in this PR for uk.json/zh.json (unrelated locale churn violating CLAUDE.md's "one PR ↔ one issue" rule), just on the English source file and via a copy rewrite rather than a mistranslation. Since en.json is the source of truth other locales get translated from, this also means uk.json/zh.json/ko.json now have a stale translation of the old English text with no indication it needs updating. Worth reverting to just the new workflow keys, or splitting into a separate PR if the channels-copy fix is intentional and wanted.

Generated by Claude Code

This doc comment promises current_step_index is "cleared on terminal state." That's true for exactly two of the Failed transitions this diff touches — the daemon-restart interruption path and the DAG-pause-on-Failed path both get an explicit run.current_step_index = None; alongside state = Failed. It is not true for the by-far-most-common failure path: an actual step error (Err(e) while executing a step — see the handler around workflow.rs:4053-4062), which is repeated at ~20 other sites in this file (grep r.state = WorkflowRunState::Failed, e.g. lines 3978, 4100, 4168, 4183, 4328, 4429, 4478, 4639, 4724, 4794, 4868, 4934, 4949, 4997, 5314, 5389, 5551). None of those clear current_step_index.

So for a run that fails mid-step (the normal way workflows fail), current_step_index is left pointing at the step that was executing when it failed, for the rest of that process's lifetime — still returned as-is by get_workflow_run/list_workflow_runs alongside state: "failed". The dashboard UI added in this PR happens not to read current_step_index outside an isActive (running/pending) gate, so this doesn't visibly break anything shipped here, but it violates this doc comment's own contract for any other/future consumer of the field. Cheap fix: clear current_step_index at every site that transitions a run to a terminal state, or centralize it in a small helper (e.g. mark_run_terminal(state, error)) instead of repeating state = Failed; error = …; completed_at = …; by hand at 20+ call sites.

Generated by Claude Code

CLAUDE.md's prose-wrapping rule requires one sentence per line, breaking only at sentence boundaries — never a hard column wrap.
This new comment block hard-wraps mid-sentence: "Mirrors the resolution in execute_llm_agent so the streaming path (WebUI, Telegram, forks) and the non-streaming path stay in sync." is split across lines 2420-2422, and the following sentence starting on line 2422 wraps again onto line 2423.
The same pattern repeats a few lines down at 2457-2461 ("Stamp sender_chat_id into the manifest so tool dispatch (agent_send, defer, approval-resume) can thread the conversation context through async task registration." spans three lines, and the following "Mirrors the identical block..." sentence spans two more).
Please reflow both new blocks to one sentence per line while this code is already being touched.

Generated by Claude Code

Same prose-wrapping issue as the one flagged in messaging.rs on this PR: CLAUDE.md requires one sentence per line for doc/prose comments, breaking only at sentence boundaries, not at a fixed column.
This new block (lines 1021-1026) hard-wraps three sentences across six lines instead — e.g. "Tell the agent it can send rich media via channel_send when the tool is available AND the channel is a real messaging adapter (not a kernel system channel like "webui" / "cron" / "autonomous")." spans lines 1021-1023.
The same pattern also appears elsewhere in this PR's new code: crates/librefang-kernel/src/workflow.rs:3890-3891 ("Update the run's current_step_index so pollers (dashboard) can show live progress as each step begins executing.") and crates/librefang-api/dashboard/src/pages/WorkflowsPage.tsx:377-378 ("Start polling immediately when a run is selected; disable once we see a terminal state (completed/failed/cancelled/paused).").
Worth reflowing all of these to one sentence per line since this PR is already touching each of them.

Generated by Claude Code

This PR's diff against origin/main removes the source?: string; field from ModelItem (it existed at the merge-base, so this branch drops it). That breaks the dashboard typecheck right now: ModelsPage.tsx:189 reads m.source !== "cli_config" to decide whether to show the delete control on a CLI-detected model, and ModelsPage.test.tsx:328 constructs a fixture with source: "cli_config". Confirmed locally: npm run typecheck fails with TS2339: Property 'source' does not exist on type 'ModelItem' (ModelsPage.tsx:189) and TS2353: Object literal may only specify known properties, and 'source' does not exist (ModelsPage.test.tsx:328) on this branch. Restoring the field fixes both:

  aliases?: string[];
  available?: boolean;
  // Provenance hint. "cli_config" marks a row synthesized from a CLI tool's own
  // live config (codex/claude-code/gemini/qwen) rather than a catalog entry — it
  // is not a user-added custom model, so it must not show a delete control.
  source?: string;
}

This looks unrelated to the PR's stated purpose (workflow execution UX) — likely dropped by accident during a rebase/merge. I can't push directly since this PR's head is on a fork (DaBlitzStein/librefang), so flagging here instead.

Generated by Claude Code

sender_context is Option<&SenderContext> (already a reference), so Some(ref ctx) binds ctx: &&SenderContext — a reference to a reference. This fails cargo clippy -p librefang-kernel --lib -- -D warnings with needless_borrow (confirmed locally on this branch). The mirrored block in send_message_full_inner (further down this file, "Approval-flow group-chat support") correctly uses if let Some(ctx) = sender_context, matching CLAUDE.md's zero-clippy-warnings requirement — this new block should match it:

        if let Some(ctx) = sender_context {

Generated by Claude Code

The rerun/run-history redesign here breaks an existing test, unrelated to typecheck. Running npx vitest run src/pages/WorkflowsPage.test.tsx on this branch: 1 failed / 18 passed. The failing test ("surfaces run parameters + error inline and re-runs with the same params (#6292)") asserts behavior this PR intentionally changed:

• screen.getByText(/sector: fintech/) — the old formatRunParamsPreview output format (k: v); this PR's new inline preview renders k=v instead, so the text no longer matches.
• The inline per-row failure-reason paragraph (state === "failed" && run.error) was removed from the run-history row entirely.
• screen.getByLabelText("Re-run with same parameters") — the new re-run button (line 1358-1365) has a title but no aria-label, and the new handleRerun no longer calls rerunMutation.mutateAsync at all (it now just pre-fills the form locally), so mutations.rerun.mutateAsync is never invoked.

None of this is a typecheck-catchable issue (mocks still typecheck fine), so it wasn't caught by the PR's stated "Dashboard typecheck: green" verification — it needs an actual test run. Since the new UX is a deliberate redesign, updating WorkflowsPage.test.tsx requires knowing the intended final behavior (should the re-run control keep an accessible name? should failures still surface inline anywhere in the list?), so I'm flagging rather than guessing at a rewrite.

Generated by Claude Code

This flips the default for every existing agent_send call that doesn't pass async explicitly, from synchronous (wait for the callee's reply inline, per the pre-existing default) to fire-and-forget. That's a behavioral/API-contract change for any agent, skill, or hand already relying on the old default to get the delegated reply back in the same turn — worth confirming this is intentional and not just a fix for the workflow-execution path this PR otherwise touches. If the goal is narrower (e.g. only workflow step delegation should be non-blocking), consider gating it there instead of flipping the tool-wide default. No CHANGELOG entry documents this default change either, unlike the sibling PRs in this queue.

Generated by Claude Code

This flips the default of an already-shipped, widely-used parameter from blocking to non-blocking, but the tool's own advertised contract wasn't updated to match, so it's now self-contradictory.

crates/librefang-runtime/src/tool_runner/definitions.rs (AGENT_SEND tool, lines ~317-333) still tells every LLM tool-caller:

• description: "By default this BLOCKS until the agent replies and returns their response"
• async param doc: "Defaults to false (blocking)."

With this change, an agent that omits async (relying on the documented default) will now get back {"task_id": ..., "status": "delegated", ...} instead of the actual reply text it was told to expect — a different response shape entirely, not just a timing change. Any existing skill/workflow-step/system-prompt written against the documented blocking default will silently start receiving task-id JSON instead of the delegate's answer.

Given this is a behavior-changing default flip for a tool that's part of the standing LLM contract (and unrelated to the rest of this PR's workflow-UX scope), please either:

• keep unwrap_or(false) and make async strictly opt-in (matching the current docs), or
• if the default really should change, update the AGENT_SEND description/param doc in definitions.rs to say so, and call it out as a breaking change (CHANGELOG entry) since callers that depended on the blocking default will need to adapt.

No existing test (tool_runner/tests/mod.rs) exercises the omitted-async case, so this slipped through unguarded.

Generated by Claude Code

This re-implements the "is this a kernel-internal system channel" check as a fresh, case-sensitive 3-string match, instead of reusing the existing shared helper this same crate already depends on.

librefang-runtime already has a librefang-channels dependency (Cargo.toml line 20), and librefang-channels::types already solves exactly this cross-crate duplication problem:

// crates/librefang-channels/src/types.rs
pub const RESERVED_SYSTEM_CHANNEL_NAMES: &[&str] = &["cron", "autonomous", "webui"];
pub fn is_reserved_system_channel(name: &str) -> bool {
    let lower = name.trim().to_ascii_lowercase();
    RESERVED_SYSTEM_CHANNEL_NAMES.iter().any(|r| *r == lower)
}

That helper is documented as mirroring librefang-kernel::kernel::{SYSTEM_CHANNEL_CRON, SYSTEM_CHANNEL_AUTONOMOUS, SYSTEM_CHANNEL_WEBUI} (duplicated only because the dependency can't go the other way), and it does case-insensitive matching specifically because of a fixed prior bug ("cron-channel-name-not-reserved" — a custom channel adapter passing channel = "Cron"/"CRON" case-insensitively used to collide with the internal cron session).

matches!(channel, "webui" | "cron" | "autonomous") here is now a third, case-sensitive copy of the same 3 strings, sitting alongside the kernel's constants and the channels crate's helper. If channel ever arrives with different casing (the exact scenario the channels-crate audit called out), this guard silently fails to recognize it as a system channel and the agent gets told to channel_send to e.g. "Webui", which has no adapter. Recommend calling librefang_channels::is_reserved_system_channel(channel) here instead of the literal match.

Generated by Claude Code

This block's comment says it "mirrors the resolution in execute_llm_agent so the streaming path ... stay[s] in sync", but it actually diverges from that function in two ways:

1. Missing the extra_params merge. The non-streaming version (agent_execution.rs lines ~625-634) does:

   for (key, value) in &dm.extra_params {
       manifest.model.extra_params.entry(key.clone()).or_insert(value.clone());
   }

   This streaming-path copy has no equivalent — any provider-specific extra_params set on the global default-model override (e.g. special headers/params some providers need) will silently not apply on the streaming path (WebUI/Telegram/forks) even though they apply on the non-streaming path for the exact same agent/config.

2. Extra is_auto_spawned special case with no non-streaming counterpart. entry.name == "assistant" && manifest.description.starts_with("General-purpose assistant") forces the default-model override even when the agent's provider/model is not "default"/"default" — i.e., a user who explicitly customized the model on their "assistant" agent (while leaving the boot-seeded description untouched) will have that customization silently overridden back to the global default, but only on the streaming path. The non-streaming execute_llm_agent has no such carve-out, so the same agent gets different effective models depending on which path served the turn.

If the auto-spawned carve-out is intentional, it needs to exist in agent_execution.rs too (and the "mirrors" comment updated to explain why they now differ); if it's not intentional, the is_auto_spawned condition and the missing extra_params loop both look like bugs relative to the stated goal of keeping the two paths in sync.

Generated by Claude Code

The PR description says this adds a "missing rerunWorkflowRun API function", but rerunWorkflowRun already exists on main (added in #6324) — this diff removes that version (further down in the same file) and re-adds a near-duplicate here with a regressed timeout and a thinner doc comment:

• Old (main, being deleted by this PR): LONG_RUNNING_TIMEOUT_MS (300s), with an explicit comment: // queues a multi-step LLM run.
• New (this PR): DEFAULT_POST_TIMEOUT_MS (60s, per the constant definitions at the top of this file).

runWorkflow itself (a few lines above, unchanged) still uses LONG_RUNNING_TIMEOUT_MS with the comment "workflows run multiple LLM steps". Re-running a workflow queues the same kind of multi-step LLM run, so cutting the client timeout to 1/5 for the rerun path only will surface a client-side timeout error on any rerun that legitimately takes more than 60s, even though the backend is still working. This looks like a stale-base artifact (the branch's own earlier rerunWorkflowRun re-landing on top of the version that already merged into main) rather than an intentional change — worth rebasing onto current main to drop this duplicate/regression.

Separately: as of this PR, WorkflowsPage.tsx no longer imports useRerunWorkflowRun at all (see the handleRerun comment on that file), so this function — and the useRerunWorkflowRun mutation hook in lib/mutations/workflows.ts that wraps it — appear to have no remaining caller in the dashboard.

Generated by Claude Code

Confirmed by actually running the suite (npx vitest run src/pages/WorkflowsPage.test.tsx after npm install): this PR breaks an existing, untouched test — "surfaces run parameters + error inline and re-runs with the same params (#6292)" in WorkflowsPage.test.tsx fails (1 failed / 18 passed).

Root cause: handleRerun here no longer calls the rerun mutation at all — it only pre-fills paramValues/runInput from the stored run input and scrolls to the run form (document.getElementById("workflow-run-section")?.scrollIntoView(...)). The user has to then separately click "Run", which goes through runMutation (a fresh runWorkflow call), not rerunWorkflowRun. That's a real product behavior change from the PR description's "one-click re-run button" — it's now two clicks, and it no longer re-executes via the dedicated rerun endpoint (the backend endpoint that reads params off the stored run) at all.

Concretely, the existing test at WorkflowsPage.test.tsx:272-301 (not modified by this PR) still expects the old contract and fails for three independent reasons against the new code:

1. getByText(/sector: fintech/) — the params-preview format changed from "sector: fintech" to "sector=fintech" (new inputPreview logic a few hundred lines up).
2. getByText("step 'analyze' failed: provider 500") — the run-level error-in-list-row block was deleted outright (no replacement in the list row).
3. getByLabelText("Re-run with same parameters") — the re-run button no longer has an aria-label, only a title; and even if found, clicking it no longer calls mutations.rerun.mutateAsync (asserted via toHaveBeenCalledWith({ runId: "run-1", workflowId: "wf-1" })), since rerunMutation was removed from this component entirely.

Please update or replace this test to match the new UX, and confirm whether the two-click prefill-then-run flow (vs. the previous one-click immediate rerun) is the intended design — it's a meaningful UX regression if not.

Generated by Claude Code

uk.json (94 lines changed) and zh.json (83 lines changed) carry a large amount of churn unrelated to this PR's workflow-UX scope, and it isn't just noise — it reverts previously-correct translations to worse ones, including outright typos:

• Line 1044 here: "обов'язковими" (correct, "mandatory") → "обов'язковикими" (not a real Ukrainian word — typo introduced by this diff).
• Line 1395: "показує" → "показувє" (wrong verb form, also a typo introduced by this diff).
• Line 1638: "pending_review_other" changed from "{{count}} запусків воркфлоу очікують..." (correct genitive plural) to "{{count}} запуску воркфлоу очікують..." (singular genitive + plural verb — grammatically broken).
• Several correctly-localized terms get replaced with raw English mid-sentence: "Пауза оператора недоступна" → "Operator pause недоступна", "Завантаження перегляду оператором…" → "Завантаження operator review…", and "tools" → "Тули" (a non-word transliteration replacing the previously-correct "Інструменти").
• zh.json shows the same pattern: "配置" (Profile) → "Profile", "温度" (Temperature) → "Temperature", "已配置全部通道" → "已配置全部 channel", etc.

None of this is required by the workflow-UX or runtime fixes this PR describes (the only locale keys this PR's own scope actually needs are the rerun_hint/step_executing/starting/variables/step_output/step_pending additions and the rerun/rerun_started/rerun_failed removals, which are fine). The rest looks like the branch carrying stale/reverted translation state from before other locale-improvement PRs landed on main — worth rebasing onto current main to drop this unrelated, quality-regressing diff rather than shipping it alongside the workflow feature.

Generated by Claude Code

ModelItem.source was dropped from this interface by the "Merge branch 'main' into feat/workflow-ux-v2" commit (this field is unrelated to the workflow-debug-UI scope of this PR, so it looks like a stale conflict resolution rather than an intentional removal).

ModelsPage.tsx:189 still reads it: const isCustom = m.tier === "custom" && m.source !== "cli_config"; with m: ModelItem. Without the field on the type, tsc --noEmit fails with "Property 'source' does not exist on type 'ModelItem'" — I confirmed this by running npm run typecheck in the dashboard after temporarily reverting this hunk.

Suggested fix (restores the field verbatim from origin/main):

  aliases?: string[];
  available?: boolean;
  // Provenance hint. "cli_config" marks a row synthesized from a CLI tool's own
  // live config (codex/claude-code/gemini/qwen) rather than a catalog entry — it
  // is not a user-added custom model, so it must not show a delete control.
  source?: string;
}

I made this exact fix locally and pushed it to feat/workflow-ux-v2, but the push landed on librefang/librefang — I don't have write access to the fork (DaBlitzStein/librefang) this PR's head branch actually lives on (403), so it never reached this PR. Leaving it here as a comment instead.

Generated by Claude Code

Same root cause as the ModelItem.source comment above: this function already existed on origin/main using LONG_RUNNING_TIMEOUT_MS (300s) with a docstring explaining why ("queues a multi-step LLM run"). This PR's merge silently replaced it with a duplicate definition using DEFAULT_POST_TIMEOUT_MS (60s) and dropped the docstring.

A workflow re-run queues the same multi-step LLM execution as the original run — 60s is exactly the kind of timeout the original fix was written to avoid. This is a functional regression, not a style nit: a legitimately long-running re-run will now hit the client-side timeout and surface as a spurious failure.

Suggested fix (restores origin/main's version):

/**
 * Re-run a previous run with its original parameters.
 *
 * The backend reads the workflow + input off the stored run (not caller-supplied
 * params), so this is a faithful, non-destructive repeat of what executed. The
 * original run is left untouched; a fresh run is queued and `{ run_id }` of the
 * new run is returned.
 */
export async function rerunWorkflowRun(runId: string): Promise<ApiActionResponse> {
  return post<ApiActionResponse>(
    `/api/workflows/runs/${encodeURIComponent(runId)}/rerun`,
    {},
    LONG_RUNNING_TIMEOUT_MS, // queues a multi-step LLM run
  );
}

Same push-access caveat as above — I have this fix committed locally but couldn't land it on the actual PR branch.

Generated by Claude Code

This diff removes "pending_review_one" (visible in the raw diff as a deleted line right above this one) while keeping "pending_review_other". en.json and uk.json both still have the _one form, so zh.json is now the only locale with an asymmetric plural pair for workflows.operator.pending_review_*.

node scripts/i18n-parity.mjs confirms this is the only drift in zh.json — everything else in the file is at parity with en.json.

Fix (restores the deleted line):

      "pending_review_one": "1 个工作流运行正在等待操作员审查",
      "pending_review_other": "{{count}} 个工作流运行正在等待操作员审查",

I made this fix locally and confirmed i18n-parity.mjs reports zh.json clean afterward, but — same as the two api.ts comments above — couldn't push it to the actual PR branch (no write access to the fork).

Generated by Claude Code

Flipping the default from false to true makes every existing agent_send call that omits "async" non-blocking by default — a system-wide behavior change for any agent, workflow step, or skill that calls agent_send today expecting a synchronous reply. That may well be the intent ("agent_send async: non-blocking by default" is in the PR title), but it breaks tests that assert the previous default in crates/librefang-runtime/src/tool_runner/tests/mod.rs:

• agent_send_no_key_no_caller_routes_to_send_to_agent (line ~1069) — builds input with no "async" key and no caller, and asserts the result is Ok("no-key-no-parent") via the synchronous send_to_agent path. With the default flipped, this same input now takes the async_mode = true branch, which (per agent_send_async_without_caller_is_invalid_parameter a few tests down) requires a caller — so this call would now return Err(InvalidParameter) instead.
• agent_send_no_key_with_caller_routes_to_send_to_agent_as (~1084), agent_send_same_key_routes_to_as_with_key_both_calls (~1102), and agent_send_distinct_keys_produce_isolated_dispatch (~1135) all similarly omit "async" and assert routing through the blocking send_to_agent_as / send_to_agent_as_with_key paths.

I didn't run these locally (no working native cargo toolchain in this environment — sysinfo@0.39.6 requires rustc 1.95, this box has 1.94.1 — and the "cargo test only via the sanctioned Docker image" path wasn't feasible in the time available for this pass), but reading tool_agent_send's dispatch (if async_mode { … } else { … } around line 110) against these tests' assertions, I'm confident at least the four listed above will fail as written.

Given this is a deliberate default-behavior change per the PR description, I'm leaving this as a comment rather than touching it myself: is the intent really "async by default everywhere," and if so, should the tests above be updated to reflect the new default (and should this be called out more prominently as a breaking change for existing agent manifests), or was the flip meant to be scoped more narrowly (e.g. only for a specific caller path) and unwrap_or(true) here is itself the bug?

Generated by Claude Code

This handleRerun is a different design than what WorkflowsPage.test.tsx still tests. The new version only pre-fills paramValues/runInput from run.input and scrolls to the run form — it never calls the rerunWorkflowRun API (the useRerunWorkflowRun import/mutation was removed from this file). The mutation hook itself is still exported from lib/mutations/workflows.ts and is still imported/mocked by the test file, so nothing fails to compile, but the test's behavioral assertions are now stale:

WorkflowsPage.test.tsx (the case titled "surfaces run parameters + error inline and re-runs with the same params (#6292)") does screen.getByLabelText("Re-run with same parameters") (the old button, which had that exact aria-label — the new button only has a title, which getByLabelText does not match) and then asserts mutations.rerun.mutateAsync was called with { runId, workflowId }. I ran this test file with npx vitest run in the dashboard workspace: 1 of 19 tests fails, exactly this one, with the getByLabelText lookup throwing first.