Repository navigation
fix(runtime,kernel): error classification, system channel detection, and model sentinel tests - #7995
Conversation
houko
left a comment
There was a problem hiding this comment.
Three unrelated changes are riding in one PR, and the one with user-visible behaviour is the one with no test.
The prompt change is untested.
build_channel_section now emits a different instruction for webui / cron / autonomous, telling the agent not to call channel_send at all.
That is a real change to what reaches the model, and nothing in this diff asserts it.
The tests added here are in llm_drivers.rs and cover resolve_fallback_target, which is a different concern entirely.
A prompt_builder test asserting that a system channel gets the "do NOT use channel_send" line and a real adapter channel still gets the channel=… / recipient=… instruction would pin the behaviour this PR exists to change.
The sentinel list is now a third copy with no pointer back.
crates/librefang-runtime/src/agent_loop/mod.rs:380 carries the same matches!(channel, "webui" | "cron" | "autonomous") with a comment explaining that runtime cannot import librefang_kernel::SYSTEM_CHANNEL_{CRON,AUTONOMOUS,WEBUI} because of the dependency direction, so a grep-pointer is the best available.
The new copy in prompt_builder.rs has no such note, so the next person to add a system channel has two chances to find one site and miss the other.
Either lift a fn is_system_channel(channel: &str) -> bool that both call, or repeat the pointer comment verbatim.
Two additions have no caller in this PR.
UsageStore::recent_events + UsageEventRow are new public API that nothing in the diff invokes, and lookup_provider_url is widened from private to pub(crate) with no new call site.
Both look like they belong to #7976 (feat(dashboard,tui): show agent token footprint and recent calls).
Landing them here means neither is exercised by any test on this branch and the visibility change reads as unexplained.
Either move them into the PR that uses them, or say in the body which PR consumes them so a reviewer is not left guessing.
The retry-logging change itself is fine and the two resolve_fallback_target tests are worth having — splitting them out would make all three parts easier to land.
8f74cd0 to
2f63d2b
Compare
…ection, and model sentinel resolution - Unsupported-parameter rejections no longer count against the circuit breaker, preventing false provider lockouts after reasoning_effort mismatches. - System channels (webui, cron, autonomous) get a channel_send suppression instruction so agents stop trying to route media through nonexistent adapters. - lookup_provider_url visibility widened to pub(crate) for reuse. - Regression tests for model="default" sentinel resolution that caused "Invalid model name" failures.
Adds UsageEventRow and UsageStore::recent_events() to surface individual LLM calls (model, tokens, cost) for the agent token footprint UI. Backed by the existing idx_usage_agent_time index.
2f63d2b to
1f4e5b9
Compare
houko
left a comment
There was a problem hiding this comment.
Five unrelated changes across four crates, two of which have no caller. The prompt-builder fix is real and worth landing; the rest should be split or dropped.
Blocking
1. The system-channel list already exists in this crate, one file over.
crates/librefang-runtime/src/agent_loop/mod.rs:392:
if matches!(channel, "webui" | "cron" | "autonomous") {and now prompt_builder.rs has a byte-identical copy. Two definitions of "which channels are kernel-internal" in one crate is exactly the drift this PR's own title ("system channel detection") implies it is fixing. Extract fn is_system_channel(channel: &str) -> bool (or a const SYSTEM_CHANNELS: &[&str]) somewhere both can reach, and call it from both sites. Adding a fourth system channel later must not be a two-file edit that one reviewer catches.
2. UsageStore::recent_events / UsageEventRow have no caller and no test.
A new pub method plus a Serialize-deriving row type, and nothing in the diff reads either. This is the backend half of #7976 ("show agent token footprint and recent calls"). Landing it here means unexercised public API on librefang-memory that nothing proves works — the SELECT column order, the i64 → u64 clamping, and the ORDER BY timestamp DESC on a text column are all untested. Move it into #7976 where its consumer and test live, or add both here.
Note the ordering caveat while it moves: ORDER BY timestamp DESC on a text column with no tiebreaker means calls recorded inside the same second come back in arbitrary order. Every other store in this repo that hit this pairs it with , id DESC.
3. lookup_provider_url is widened to pub(crate) with no new caller.
Nothing in the diff uses it from outside its module. Either the consumer is missing, or this is a leftover from another branch — please drop it if so; a visibility bump is an API decision and should arrive with the code that needs it.
4. No test for the only behavioural fix in the PR.
The channel_send prompt branch changes what reaches the LLM, which is the one thing here a user can observe. prompt_builder.rs already has tests. Assert that a webui channel with channel_send granted produces the "do NOT use channel_send" text and not the recipient="{id}" instruction, and that a telegram channel still produces the latter. Prompt text is also under the #3298 determinism rules, so a test at this boundary is cheap insurance.
Non-blocking
- Per
CLAUDE.md, one PR ↔ one issue. Retry log wording, tworesolve_fallback_targettests, a usage-store method, a visibility bump and a prompt fix are five separable things; the title having to name three of them is the tell. The prompt fix in particular deserves to be reviewable on its own. - The retry change duplicates the whole
build_user_facing_llm_error+record_retry_failureshape in two functions with only the string differing. A singlelet label = if counts_against_breaker { … } else { … };is already what it does — worth hoisting the pair into a small helper socall_with_retryandstream_with_retrycannot drift the classification apart. - No changelog fragment. The
channel_send-on-webui fix is user-visible (agents were being pushed to fall back to Telegram) and belongs in the release notes:changelog.d/fixed/7995-....md.
|
Status check while I worked through the review queue — this one is still waiting on you, and I would rather say so plainly than leave it looking like my turn. No commits of your own since my review on 2026-09-01T13:47:43Z; the only thing that moved the branch tip is a merge from Still open from that review:
Full reasoning and the file/line evidence for each is in that review. I re-read it before writing this and have not changed my mind on any of them. No conflict with Not asking you to prioritise this over anything else. |
|
A suggestion to trim this PR, found while merging every open branch into one integration tree: the Both suppress the
A cron run is not on the web interface, and nothing it generates is shown to a user automatically. So on two of the three channels this branch guards, the guidance it substitutes is factually wrong — and it is the kind of wrong an agent will act on, since the whole point of the hint is to steer behaviour. The hardcoded list is also a second source of truth for something the channels crate already owns; a future reserved channel would be guarded in one place and not the other. Nothing else in this PR overlaps — this is only about that one hunk. Dropping it here and letting #8149 carry the fix leaves both branches strictly better, and removes a conflict that would otherwise have to be resolved at merge time by whoever lands second. For what it is worth in the integration tree, #8149's version was kept and this hunk dropped; the merge is clean that way. |
Address the review by removing scope rather than adding it. Dropped, because nothing in the diff called either: - `UsageStore::recent_events` and `UsageEventRow` — the backend half of librefang#7976, whose consumer and test belong with it. When they move, pair `ORDER BY timestamp DESC` with `, id DESC`: the column is text and without a tiebreaker calls recorded inside the same second come back in arbitrary order, which is how every other store in this repo handles it. - The `pub(crate)` widening of `LibreFangKernel::lookup_provider_url`. A visibility bump is an API decision and should arrive with the code that needs it. Kept and finished: - The system-channel list is now `channel_registry::is_system_channel`, called from both `build_sender_prefix` and `build_channel_section`, replacing the byte-identical `matches!` copy the fix had introduced one file from the original. Adding a fourth sentinel is a one-line edit instead of a two-file edit a reviewer has to catch. - Two tests over the one behaviour a user can observe: a `webui` / `cron` / `autonomous` channel with `channel_send` granted is told not to use it and is never handed a recipient, while `telegram` keeps the recipient instruction. Verified red first — with the branch disabled the `webui` prompt reads `recipient="127.0.0.1"` for an adapter that does not exist. - The terminal-failure path of `call_with_retry` and `stream_with_retry` folded into `classify_terminal_llm_error`, so the two cannot drift apart on which failures count against the circuit breaker.
|
Addressed at 1 — one definition of "system channel". 2 — 3 — the 4 — the behavioural fix has a test. Non-blocking: the duplicated Ready for another look. |
| .any(|t| t == "channel_send" || t == "*"); | ||
| if has_channel_send { | ||
| if let Some(id) = sender_id { | ||
| if is_system_channel { |
There was a problem hiding this comment.
This branch is taken for cron and autonomous too, and for those two the text is both factually wrong and a capability regression.
- "You are on the LibreFang web interface" is false on a cron firing (
cron_tick.rs:219stampschannel: SYSTEM_CHANNEL_CRON) and on an autonomous heartbeat / goal tick (goal_lifecycle.rs:54,background_lifecycle.rs:1251). - "media you generate are shown to the user automatically in your response" is false there as well. Cron output goes through
CronDeliveryEngine, whose only channel primitive isCronChannelSender::send_channel_message(channel_type, recipient, message)(crates/librefang-kernel/src/cron_delivery.rs:80) — text only, no attachment parameter. Autonomous turns have no reader on the other end of a response stream at all. channel_sendis the only outbound media path those turns have, and it is explicitly designed to be usable from them: the bug(security/channels): channel_send can dispatch to a different recipient on the same channel (cross-chat message leak) #6117 cross-chat guard intool_channel_sendfires only whenturn_channel.eq_ignore_ascii_case(&channel), so a cron turn (turn_channel = "cron") targetingchannel = "slack"passes, and the parameter docs on that function say so ("Nonefor out-of-band callers (cron, triggers, external MCP) — the guard no-ops").
Concrete failure: a cron job "render the daily chart and post it to #ops" previously called channel_send with channel="slack", recipient="#ops". With this change its system prompt says the image is delivered automatically and to not use channel_send, so the chart is generated and never delivered — the plain-text fan-out carries only the reply body.
The narrow fix for #7995 is to stop handing a recipient for a sentinel channel, not to forbid the tool. Suggest keeping the "do NOT use channel_send" wording for webui only, and for cron / autonomous telling the agent it must name a real channel and recipient explicitly — or omitting the paragraph for those two, which restores the pre-PR behaviour there.
There was a problem hiding this comment.
Agreed, and fixed narrower than the suggestion: the suppression now checks channel == "webui" specifically rather than is_system_channel, so cron and autonomous fall through to a new branch that keeps channel_send available but tells the agent this turn has no default channel or recipient and to name a real one explicitly. Added cron_and_autonomous_keep_channel_send_but_require_an_explicit_target and renamed the old system_channels_suppress_... test to webui_suppresses_... (scoped to webui only) — verified red by reverting the webui-only check back to is_system_channel, which makes the new cron/autonomous test fail exactly as described.
| if is_system_channel { | ||
| section.push_str( | ||
| "\n\nYou are on the LibreFang web interface. Files, images, and media you \ | ||
| generate are shown to the user automatically in your response — do NOT use \ |
There was a problem hiding this comment.
"shown to the user automatically in your response" does not hold for webui either, so the instruction replaces one wrong belief with another.
Nothing attaches agent-generated media to an assistant message. GET /api/agents/{id}/session populates msg["images"] only from ContentBlock::Image blocks present in the stored message (crates/librefang-api/src/routes/agents/sessions.rs:302), and an LLM never emits an image block. On the dashboard side ChatPage's images field is documented as "sourced from session history (AgentSessionMessage.images) or the user's pending uploads at send" (ChatPage.tsx:99-101) — i.e. inbound attachments. An assistant turn is rendered through MarkdownContent only.
The one path that actually reaches the user is the agent embedding the URL itself: tool_image_generate returns image_urls from save_media_images_to_uploads (tool_runner/media.rs:648 and :772), and markdown renders it.
Concrete failure: an agent on webui calls image_generate, reads "media you generate are shown to the user automatically", and replies "Here is the chart you asked for." The returned image_urls never make it into the reply text, so the user sees a bare sentence and no image — the same dead end as the Telegram fallback, one step later.
Suggest stating what is true: media is delivered by including the returned URL or path in the reply as markdown, not by channel_send.
There was a problem hiding this comment.
Fixed — the webui instruction now says media is delivered by including the returned URL or file path in the reply as markdown, and explicitly says there is no channel for channel_send to deliver to, rather than claiming media is attached automatically. Added webui_states_the_true_media_delivery_mechanism asserting the new wording is present and the old "shown to the user automatically" claim is gone.
| /// Keep these literals in sync with the kernel-side constants: | ||
| /// `librefang_kernel::SYSTEM_CHANNEL_{CRON,AUTONOMOUS,WEBUI}`. | ||
| /// Runtime can't import them directly (circular dep — runtime is below kernel), | ||
| /// so a grep-pointer is the best we can do; api / cli / kernel sites reference | ||
| /// the kernel constants by name and stay in lock-step. | ||
| /// | ||
| /// Every runtime site that needs the distinction calls this, so adding a fourth | ||
| /// sentinel is a one-line edit rather than a hunt for `matches!` copies. | ||
| pub fn is_system_channel(channel: &str) -> bool { | ||
| matches!(channel, "webui" | "cron" | "autonomous") | ||
| } |
There was a problem hiding this comment.
The circular-dep premise is not accurate, and following it costs the drift guard that already exists for this list.
librefang-runtime already depends on librefang-channels (crates/librefang-runtime/Cargo.toml:22; agent_loop/retry.rs uses librefang_channels::message_journal::RATE_LIMIT_DEFER_MARKER), and that crate publishes exactly this list for exactly this reason — crates/librefang-channels/src/types.rs:15 defines RESERVED_SYSTEM_CHANNEL_NAMES = &["cron", "autonomous", "webui"] plus is_reserved_system_channel, with the doc comment "duplicated here because librefang-channels cannot depend on librefang-kernel". crates/librefang-kernel/tests/audit_cron_channel_name_test.rs pins that list against the kernel constants.
So this is a third copy of the literals, and the only one no test observes. Concrete failure: a fourth sentinel is added — kernel constant, RESERVED_SYSTEM_CHANNEL_NAMES, api/cli sites — audit_cron_channel_name_test stays green, and channel_registry::is_system_channel keeps returning false for it. build_sender_prefix then stamps a sender prefix from a placeholder identity again and build_channel_section hands out recipient="<sentinel>" again, which is the bug this PR is fixing.
Delegating gives the doc comment's claim real teeth, and picks up the trim plus case-insensitive matching is_reserved_system_channel already does:
| /// Keep these literals in sync with the kernel-side constants: | |
| /// `librefang_kernel::SYSTEM_CHANNEL_{CRON,AUTONOMOUS,WEBUI}`. | |
| /// Runtime can't import them directly (circular dep — runtime is below kernel), | |
| /// so a grep-pointer is the best we can do; api / cli / kernel sites reference | |
| /// the kernel constants by name and stay in lock-step. | |
| /// | |
| /// Every runtime site that needs the distinction calls this, so adding a fourth | |
| /// sentinel is a one-line edit rather than a hunt for `matches!` copies. | |
| pub fn is_system_channel(channel: &str) -> bool { | |
| matches!(channel, "webui" | "cron" | "autonomous") | |
| } | |
| /// Keep this aligned with the kernel-side constants | |
| /// `librefang_kernel::SYSTEM_CHANNEL_{CRON,AUTONOMOUS,WEBUI}`. Runtime can't | |
| /// import those directly (circular dep — runtime is below kernel), but | |
| /// `librefang-channels` mirrors the same list for the same reason and is | |
| /// drift-guarded against the kernel constants by | |
| /// `crates/librefang-kernel/tests/audit_cron_channel_name_test.rs`, so defer to | |
| /// it rather than keeping a third copy of the literals here. | |
| /// | |
| /// Every runtime site that needs the distinction calls this, so adding a fourth | |
| /// sentinel is a one-line edit rather than a hunt for `matches!` copies. | |
| pub fn is_system_channel(channel: &str) -> bool { | |
| librefang_channels::types::is_reserved_system_channel(channel) | |
| } |
There was a problem hiding this comment.
Fixed exactly as suggested — is_system_channel now delegates to librefang_channels::types::is_reserved_system_channel, with the doc comment pointing at the drift guard (audit_cron_channel_name_test.rs) instead of repeating the circular-dependency premise that no longer holds. Verified librefang-runtime already declares the librefang-channels path dependency (Cargo.toml:22).
…nel list Fixes review findings on librefang#7995: the system-channel channel_send suppression covered cron and autonomous turns too, which regressed a capability those turns already had (channel_send with an explicit real channel and recipient), and its wording claimed media is attached automatically even for webui, which nothing in the runtime does. channel_registry::is_system_channel also kept a third hand-copied sentinel list behind a circular-dependency excuse that no longer held; it now delegates to librefang-channels, which is drift-guarded against the kernel constants.
|
Every one of the 3 review threads on this PR now has an inline reply, and the branch is at Each reply states what changed and where, the commit it landed in, and the literal failure from reverting the production block and running the test against it — a regression test that stays green with the fix removed guards nothing, so that cycle was run rather than assumed. Where a finding was not acted on, the reply says so and gives the reason instead of leaving the thread unanswered. Flagging it here because the review is still recorded as requesting changes, and a push does not retract that on its own. Ready for another look whenever it suits you. |
houko
left a comment
There was a problem hiding this comment.
The three findings from the previous round are addressed in the code: cron / autonomous no longer get told they are on the web interface, the webui text no longer claims automatic attachment, and is_system_channel now delegates to librefang_channels::types::is_reserved_system_channel instead of keeping a third copy of the literals. The retry.rs extraction is also a genuine improvement and behaviour-preserving on the user-facing path — classify_terminal_llm_error's added suffix reaches only the warn! in build_user_facing_llm_error, never user_msg, and the suffix's wording is accurate because should_count_against_circuit_breaker returns false for exactly one case (Api { status: 400 } + is_unsupported_parameter_error, retry.rs:145-152).
Three things, the first of which should block.
1. changelog.d/fixed/7995-webui-channel-send-instruction.md is stale and contradicts both the code and the PR's own other fragment. It still says the instruction applies to "webui, cron and autonomous" and closes with "Those channels now say the opposite: media generated during the turn is shown to the user automatically" — which is the precise claim the previous round asked you to remove and which 7995-system-channel-prompt-review-fixes.md now says was removed. Both files carry (#7995), so cargo xtask collect-fragments folds both into ### Fixed verbatim and the release body ships the retracted statement next to its own retraction. Delete the stale one and keep a single fragment describing the shipped behaviour.
2. channel == "webui" is exact while is_system_channel(channel) three lines below trims and lowercases. A "WebUI" or " webui" therefore misses the first branch and lands in the second, telling a live browser session "This turn has no default channel or recipient" — a live user's turn described as a background one, which is the bug class this PR exists to close. In practice the kernel always stamps the lowercase constant (ws.rs:1236,1316 use SYSTEM_CHANNEL_WEBUI), so this is latent rather than live, but the two comparisons sitting three lines apart with different normalisation is the part that will not survive the next edit. channel.trim().eq_ignore_ascii_case("webui") matches the predicate you are already deferring to.
3. This PR and #8149 rewrite the same block of prompt_builder.rs, with different code and opposite rationales. Whichever merges second will conflict, and the conflict is not mechanical — it is a real disagreement that should be settled before either lands:
| #7995 (this PR) | #8149 | |
|---|---|---|
| system-channel predicate | is_reserved_system_channel via channel_registry::is_system_channel |
hardcoded cron / autonomous / webui literals |
| stated reason | the circular-dep excuse "no longer held" — runtime already depends on librefang-channels |
the reserved list belongs to sanitize_channel_name's SessionId-collision concern and may gain an interactive surface |
| cron / autonomous text | channel_send still works — name a real channel and recipient |
"channel_send cannot reach this system channel — do NOT use it here" |
| case handling | exact for webui, insensitive for the rest |
insensitive throughout, with a test |
Both of those rationales came out of my own review rounds, and they pull in opposite directions, so this is mine to resolve rather than yours: the dependency argument in this PR is correct and the semantic argument in #8149 is also correct. The version that satisfies both is one set of literals living low in the stack and a separate, named predicate for the prompt's question. Concretely: keep is_reserved_system_channel as the SessionId-collision answer it already is, and give the prompt builder its own is_non_interactive_turn(channel) defined over the same constants — so adding a future interactive surface to the reserved list does not silently change what the prompt tells a live user, and there is still only one copy of the strings.
Say which PR you would rather carry that, and I will close the loop on the other one.
Review follow-up on librefang#7995. The webui arm compared exactly while the system-channel arm three lines below it trimmed and lowercased, so a "WebUI" fell past the first into the second and told a live browser session its turn had no default channel or recipient. The kernel only ever stamps the lowercase constant today, so this was latent, but two comparisons that close disagreeing about normalisation do not survive the next edit. The second arm also asked the wrong question. is_reserved_system_channel answers which names would collide with a kernel SessionId, and webui is in that set precisely because it derives one — yet a live user is waiting on a webui reply. Deriving the prompt's question from that list means a future interactive surface added for SessionId reasons silently starts telling a real user their turn is a background one. The prompt builder now has its own is_non_interactive_turn over named constants, so the strings are still declared once in librefang-channels while the two questions stay separate. Deletes changelog.d/fixed/7995-webui-channel-send-instruction.md, which still described the cron/autonomous suppression this PR removed and closed with the 'shown to the user automatically' claim the same PR retracted. Both fragments carried (librefang#7995), so collect-fragments would have folded the retracted statement into the release body next to its own retraction. Verified the new test fails against the exact comparison ("WebUI" must take the webui arm) and passes with it.
|
Pushed 1. Stale changelog fragment — deleted. You were right on both counts: 2. The asymmetric comparison — fixed. 3. I would rather #7995 carried it, and it now does. Your framing is the one I would not have reached on my own: the dependency argument and the semantic argument are both correct because they are answers to two different questions. So
To keep one copy of the strings I gave That makes #8149 redundant rather than conflicting — it is yours to close whenever you like, and I have not touched it. Verification
|
…told Both PRs rewrite the same branch of build_channel_section, which origin/main has neither of, and they had landed on opposite instructions: this one offered channel_send with an explicit target, librefang#8149 forbade it and then explained how to use it in the next sentence. The offer is the true one. channel_send reads channel and recipient from its own arguments and dispatches to that adapter, so the sentinel channel the turn runs under never enters into it. What is impossible is replying into cron or autonomous, which have no adapter, and that is what the prompt says now. The sentence is byte-identical to the one librefang#8149 writes, and a test on each side asserts it by name, so whichever merges second the resolution is textual rather than a choice between two instructions. The webui arm also names the /api/uploads URL the generating tool returns, which is the only thing that puts a file in front of the browser.
|
Añadido en Los dos PRs se contradecían en el mismo bloqueEste PR y #8149 reescriben cada uno la misma rama de Es la oferta. Las dos ramas emiten ya la misma frase byte a byte, comprobada con Lo que NO apliqué, y por quéEl rescate traía además la normalización de
|
…told The hint contradicted itself inside its own paragraph: it said do NOT use channel_send here and then, in the next sentence, explained how to use it. channel_send reads channel and recipient from its own arguments and dispatches to that adapter, so the sentinel channel the turn runs under never enters into it, and a background turn reaches a person on a real channel exactly like any other turn. What is impossible is replying into cron or autonomous, which have no adapter. librefang#7995 rewrites this same branch from a different pre-image and had landed on the opposite instruction. Both now emit the same sentence byte for byte, and a test on each side asserts it by name, so whichever merges second the resolution is textual.
|
Findings from carrying this PR and #8149 together in an integration branch. Splitting them because the two halves of The behavioural half is already satisfied by #8149's landed version. Its review finding — that the But the merged wording already preserves it. The structural half is still open. One caveat if you do land it, because the two call sites want opposite things: Why the commits do not cherry-pick cleanly: the test assertions were written against this PR's own wording. Measured, not assumed: No action requested — I did not resolve any of this in the integration branch, because choosing between two reviewed wordings is not mine to make. Flagging it so the rebase is done knowing which half is already in. |
houko
left a comment
There was a problem hiding this comment.
Re-reviewed at the current head. All four blocking findings are addressed, and the first one better than I asked.
1 — the duplicated system-channel list. I asked for one predicate both call sites reach. channel_registry::is_system_channel is that, and it delegates to librefang_channels::types::is_reserved_system_channel rather than holding its own copy of the literals — so this is the second definition folding into the existing one, not a third being created. agent_loop/mod.rs:387 calls it, and the drift guard against the kernel constants (audit_cron_channel_name_test.rs) covers the remaining copy.
2 and 3 — UsageStore::recent_events and the lookup_provider_url visibility bump. Both gone; 1dd118ce narrows the PR to the prompt fix. The diff is now eight files in one area rather than five unrelated changes across four crates, which also settles the non-blocking one-PR-one-issue point.
4 — a test for the behavioural fix. Six, including the exact pair I asked for: webui_suppresses_the_channel_send_recipient_instruction and real_channels_keep_the_channel_send_recipient_instruction.
…told The hint contradicted itself inside its own paragraph: it said do NOT use channel_send here and then, in the next sentence, explained how to use it. channel_send reads channel and recipient from its own arguments and dispatches to that adapter, so the sentinel channel the turn runs under never enters into it, and a background turn reaches a person on a real channel exactly like any other turn. What is impossible is replying into cron or autonomous, which have no adapter. the opposite instruction. Both now emit the same sentence byte for byte, and a test on each side asserts it by name, so whichever merges second the resolution is textual.
The rebase onto main dropped this branch's `prompt_builder.rs` changes, because librefang#7995 had already landed the same block — and landed it better, matching `SYSTEM_CHANNEL_WEBUI` and `is_non_interactive_turn()` rather than the three string literals written here. What is left is the seven regression tests and the changelog, and the two fragments described work that is no longer in the diff. One of them claimed the three channels are "matched by name rather than through the shared reserved-channel-name list", which is not how main does it. The other claimed the wording "is now byte-identical to the one librefang#7995 writes into the same block", a sentence that cannot be true of a branch that no longer writes into that block at all. Fragments reach the GitHub release body verbatim, so both are replaced by one that says what actually merges: the behaviour shipped with librefang#7995, and these tests are what stop it drifting back.
…pped (#8149) * fix(runtime): stop suggesting channel_send to kernel-internal system channels The channel_send media hint was emitted unconditionally whenever the tool was granted, including on the kernel-internal system channels (webui, cron, autonomous) where no messaging adapter exists. On those channels the suggestion only pushed the agent to attempt a send that cannot reach anyone and to improvise a fallback channel on its own. The hint is now suppressed there: webui is told media flows through the normal response stream, and background runs are told to target a real messaging channel explicitly (or notify_owner) instead. Recovered from the closed workflow-ux-v2 PR (#6504), which the maintainer called 'a real fix for a real problem' outside that PR's scope; the suppression now reuses the existing is_reserved_system_channel helper and adds tests for all three system channels. * docs(changelog): reference PR 8149 in the channel-send guard fragment * fix(runtime): scope the webui channel_send guidance to replying in place Review of #8149 pointed out the webui branch was broader than the bug it fixes. `channel_send` takes an explicit `channel` argument and reaches email, telegram and slack, so a blanket "do NOT use `channel_send`" told an agent on the dashboard not to email a report or post to a Slack channel — both ordinary, supported actions. The guidance now forbids only replying through it, and points at what it is still for, matching the shape the cron/autonomous branch already had. Also match the inner `webui` test to the outer guard's case sensitivity. `is_reserved_system_channel` trims and lowercases, so `"WebUI"` entered the block and then failed `channel == "webui"`, dropping a live web session into the background-run branch and telling it there is no live user watching. Not reachable today because the kernel passes `SYSTEM_CHANNEL_WEBUI`, but the two comparisons should not disagree; `test_channel_send_hint_webui_match_is_case_insensitive` pins it. And add the trailing newline the changelog fragment was missing. * docs(changelog): reference the PR in the channel_send guard fragment The bullet closed with the issue number alone, so the release flow had no PR number to match and would have emitted the generated line as well, listing the change twice in the release body. * fix(runtime): stop asserting false things in the channel-send system guard The `channel_send` system-channel guard told a cron run nobody was watching its response, even though a configured delivery target hands that response straight to a real person (kernel/cron_bridge.rs). It recommended `notify_owner` as a fallback there, even though neither the cron nor the autonomous path ever reads the notice it queues. It told webui that generated media is shown to the user automatically, even though the browser only ever sees what the reply text embeds. The background-run and webui branches now match the literal cron / autonomous / webui channel names instead of deriving from the shared reserved-channel-name list, so a future addition to that list no longer inherits background-run guidance by accident. * docs(changelog): fold the 8149 wording fragment into the guard's own Two fragments both carried (#8149) and both would have been folded into ### Fixed verbatim, so a release reader got one bullet saying the guard was added and a second saying three assertions in its wording were wrong. The first wording never shipped, so the second bullet described fixing something no user ever saw. One fragment now describes the shipped behaviour: which channels stop being offered channel_send, and what each of them is told instead. * fix(runtime): agree with #7995 on what a background turn is told The hint contradicted itself inside its own paragraph: it said do NOT use channel_send here and then, in the next sentence, explained how to use it. channel_send reads channel and recipient from its own arguments and dispatches to that adapter, so the sentinel channel the turn runs under never enters into it, and a background turn reaches a person on a real channel exactly like any other turn. What is impossible is replying into cron or autonomous, which have no adapter. the opposite instruction. Both now emit the same sentence byte for byte, and a test on each side asserts it by name, so whichever merges second the resolution is textual. * docs(changelog): describe what this branch still contains The rebase onto main dropped this branch's `prompt_builder.rs` changes, because #7995 had already landed the same block — and landed it better, matching `SYSTEM_CHANNEL_WEBUI` and `is_non_interactive_turn()` rather than the three string literals written here. What is left is the seven regression tests and the changelog, and the two fragments described work that is no longer in the diff. One of them claimed the three channels are "matched by name rather than through the shared reserved-channel-name list", which is not how main does it. The other claimed the wording "is now byte-identical to the one #7995 writes into the same block", a sentence that cannot be true of a branch that no longer writes into that block at all. Fragments reach the GitHub release body verbatim, so both are replaced by one that says what actually merges: the behaviour shipped with #7995, and these tests are what stop it drifting back. --------- Co-authored-by: Evan <suzukaze.haduki@gmail.com>
Summary
lookup_provider_urlvisibility widened topub(crate)for internal reuse.model="default"sentinel resolution that caused "Invalid model name" failures on fallback slots.Verification
cargo check -p librefang-runtime --lib— cleancargo check -p librefang-kernel --tests— cleancargo clippy -p librefang-runtime -p librefang-kernel -- -D warnings— zero warningsIssue references
reasoning_effortfails every turn and then opens the provider circuit breaker #7769: Partial coverage: exclude unsupported-parameter errors from provider cooldown accounting.