fix(cron): summarize-and-trim compaction mode for Persistent sessions (#3693)#4683
Merged
Conversation
…sions (#3693) Gap 4 of issue #3693: replace hard drop-from-front with an optional LLM-based summarize-and-trim compaction path for Persistent cron sessions. New KernelConfig fields: - cron_session_compaction_mode: CronCompactionMode (default Prune) "prune" -- existing drop-from-front behaviour (unchanged) "summarize_trim" -- LLM-summarize the about-to-be-dropped prefix, prepend a synthetic summary message, keep the most recent keep_recent messages verbatim - cron_session_compaction_keep_recent: usize (default 8) Tail preserved verbatim after summarization (min 1) Implementation: - cron_compute_keep_count() pure helper computes keep-count from both caps without mutating, enabling clean Prune/SummarizeTrim branching - SummarizeTrim calls aux_client Compression driver (same chain as /compact); falls back to Prune with tracing::warn! on LLM failure so no cron fire is ever blocked by summarization unavailability - Prune path unchanged bit-for-bit New tests in kernel/tests.rs (unit): cron_compute_keep_count_message_cap_only cron_compute_keep_count_token_cap_trims_front cron_compute_keep_count_message_cap_applied_before_token_cap cron_compute_keep_count_no_caps_returns_all cron_compute_keep_count_empty_messages New tests in workflows_routes_integration.rs (integration): cron_summarize_trim_produces_synthesis_and_controls_length cron_summarize_trim_falls_back_to_prune_on_llm_failure Docs: cron-session-sizing.md updated with new mode description, keep_recent knob, and updated strategy table. Note: kernel_config_schema.golden.json is already #[ignore]'d (pre- existing drift on main); regenerate with --ignored regenerate_golden after merging. Refs #3693
Independent review surfaced three real defects in the SummarizeTrim path landed at 9885eab; this commit fixes them and tightens the surrounding test + doc surface. - H1: SummarizeTrim now calls compactor::adjust_split_for_tool_pair before slicing head/tail, so the split point cannot land between an Assistant{ToolUse} and its paired User{ToolResult}. Without this guard the synthesis prefix would have carried an orphan ToolUse and the kept tail an orphan ToolResult, producing 4xx from providers that enforce pairing. compactor.rs exposes the helper as pub. - H2: Replaced the two pseudo-integration tests in workflows_routes_integration.rs (which only re-exercised compact_session) with a real end-to-end test in librefang-kernel/tests/cron_compaction_test.rs that drives a cron fire through the kernel and asserts the SummarizeTrim and fallback branches both run as designed. - H3: Extracted apply_cron_prune helper so the SummarizeTrim success path, the SummarizeTrim fallback path, and the plain Prune path all mutate Session through the same code, eliminating the set_messages vs drain divergence. - M4: compact_session's stage-3 minimal-fallback now flows through a real Prune branch instead of being mis-classified as a successful summary. Match arm checks both !summary.is_empty() AND !result.used_fallback. - M5: docs/architecture/cron-session-sizing.md gains a Concurrency caveat section explaining the global cron_lane semaphore and the last-write-wins window that SummarizeTrim widens. - L6/L7/L8/L9: keep_recent clamped at parse time, SummarizeTrim branch hoisted into try_summarize_trim async helper, comment corrected from "Binary-search-like" to honest O(n^2) note, kernel/tests.rs gains max_tokens=Some(0) / max_messages=Some(0) / single-msg overflow boundary cases. Verified locally: cargo clippy --workspace --all-targets -- -D warnings OK (modulo pre-existing macOS libdbus-sys gap in librefang-desktop / transitive deps; unrelated to this PR; CI Linux runs full) cargo test -p librefang-kernel --no-run OK Refs #3693.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
… concurrency truth (#3693) - kernel: SummarizeTrim was using cron_session_compaction_keep_recent verbatim, ignoring the active cron_session_max_messages / cron_session_max_tokens cap. With max_messages=5 and keep_recent=8 (the default), the post-compaction session ended up with 1+8=9 messages — cap violated, and every subsequent fire summarized 1 message into 1 summary forever (no size progress, futile aux LLM calls). Add cron_clamp_keep_recent helper that floors keep_recent at keep_count.saturating_sub(1).max(1) so [summary] + tail always fits inside the cap. - kernel: registry miss during SummarizeTrim previously silently used an empty model name, which the LLM driver fails on, which was caught by used_fallback. Working as intended, but invisible. Surface a tracing::warn! so the registry/scheduler inconsistency is observable. - tests: 5 new unit tests for cron_clamp_keep_recent covering the cap case, the no-clamp case, the floor, the keep_count==0 edge, and the zero-cfg edge. Plus a combined invariant test asserting 1 + tail <= keep_count across realistic (n, max_messages, keep_recent_cfg) combos. The compaction_test.rs ToolResult fixture is updated to the current ContentBlock::ToolResult schema (tool_name, status, approval_request_id) — the prior fixture was rebased-stale. - docs: keep_recent now documents the runtime cap clamp explicitly with a worked example. Concurrency-caveat section rewritten to match the implementation: the per-session mutex is held *across* the LLM summary call (intentional — prevents race against an un-compacted snapshot). Trade-off (sibling fires block on the mutex up to LLM latency) is called out so operators know to use session_mode=new when independent context is required.
houko
commented
May 6, 2026
houko
left a comment
Contributor
Author
There was a problem hiding this comment.
Code Review — automated review pass
Verdict: COMMENT (no blocking issues; opt-in feature with sensible defaults). +988/-38 across 7 files.
Spec compliance — Stage 1 PASS
- Persistent-only binding correct. Compaction sits inside
if !wants_new_sessionatcrates/librefang-kernel/src/kernel/mod.rs:14325, wherewants_new_session = effective_session_mode == Some(SessionMode::New)(line 14247).Newcron fires (which usecron_fire_session_override→ isolatedSessionId::for_cron_run) skip the entire compaction block. Matches CLAUDE.md invariant. - Summarization correctly delegates to
librefang_runtime::compactor::compact_sessionviatry_summarize_trim(kernel/mod.rs:144). UsesAuxTask::Compression(same lane as/compact), not the agent's primary driver. No re-implementation. used_fallbackguard at line 145 (!result.summary.is_empty() && !result.used_fallback) correctly rejects the synthetic[Session compacted: …]placeholder thatcompact_sessionreturns on driver failure (compactor.rs:955). Without this, M4 bug would accept fallback string as a real summary.
Stage 2 — Code quality
- Tests:
crates/librefang-kernel/tests/cron_compaction_test.rscovers (1) successful summarize →[summary] + tail, (2)FailingDriver→used_fallback=truerejection, (3) tool-pair adjustment. 14 helper unit tests inkernel/tests.rscovercron_compute_keep_count+cron_clamp_keep_recentinvariants. Gap (acknowledged in PR body): no#[tokio::test]against full kernel cron-tick path exercising the Persistent-fires-compact / New-stays-untouched dichotomy end-to-end. Thewants_new_sessiongate is enforced by code structure, not by test, so a future regression that moves the gate would land silently. Suggest a follow-up integration test assertingNewmode never invokes the aux compression driver. - Unbounded-loop risk: addressed.
cron_clamp_keep_recent(line 64) clampskeep_recenttokeep_count - 1so[summary] + tail ≤ cap, preventing the n=20/cap=5/keep=8 → 9 messages → re-summarize-1 forever loop. Floor of1andkeep_count==0short-circuit covered (try_summarize_trimreturnsNonewhentail_start == 0ormessages.len()). - Telemetry:
tracing::info!on success (line 14543,cron session summarize-and-trim complete, withkeptcount),tracing::warn!on LLM-fallback path and on missing-agent-from-registry. Operators can grep both. No counter metric, but the existing approach-warn telemetry is preserved. - Concurrency note: holding
prune_lockacross the LLM call is documented as intentional and explained indocs/architecture/cron-session-sizing.md. Trade-off is correct: a sibling fire on the same(agent, "cron")session waits up to LLM latency. Mitigation (session_mode = "new") is documented.
LOW
try_summarize_trimbuilds a throwawaySession(kernel/mod.rs:131) just to callcompact_session. Minor —compact_sessioncould take&[Message]directly, but that is a runtime-side refactor out of scope for this PR.model = String::new()fallback when agent missing from registry (line 276) will almost certainly fail the LLM call → fall back to plain prune. Logged aswarn, behavior is safe.
Positive observations
- Default stays
Prune— no surprise aux LLM costs for existing deployments. adjust_split_for_tool_pairmadepuband reused — no duplication of tool-pair logic.- Docs updated with concurrency caveat, knob ranges, fallback-warn log line, and decision matrix.
- 14 helper tests + invariant test combining
cron_compute_keep_count×cron_clamp_keep_recentis strong unit coverage.
Recommendation
COMMENT — opt-in, well-tested, correctly bound to Persistent. Consider follow-up #[tokio::test] exercising the New-mode-skips-compaction path through the full cron tick.
… tool-pair helper (#4683 review) - kernel/mod.rs: add cron_resolve_compaction_mode helper that downgrades SummarizeTrim to Prune when keep_count < 2. Without this, a tight cap (max_messages = 1, or every newest message exceeding max_tokens so keep_count = 0) makes try_summarize_trim write [summary, last_msg] (2 messages) into a session whose cap permits at most 1, and the next fire re-enters SummarizeTrim and re-summarizes 1 message into 1 summary forever — the same fixed-point loop the existing cron_clamp_keep_recent_keep_count_ zero_floors_to_one test flagged as the caller's responsibility to handle. The dispatch site now calls the helper, emits a tracing::warn! when the downgrade fires, and matches on the resolved mode. - compactor.rs: mark pub fn adjust_split_for_tool_pair as #[doc(hidden)] with a workspace-internal contract note. Made pub in the original PR so the kernel could reach it across the crate boundary; this keeps that escape hatch but removes it from the public API surface so external dependents cannot bake in a dependency on a helper that may change without semver bump. - kernel/tests.rs: 6 new unit tests covering the routing layer: keep_count = 0 / keep_count = 1 -> Prune; keep_count = 2 (boundary) -> SummarizeTrim; normal keep_count unchanged; Prune always passes through; integration with cron_compute_keep_count to verify end-to-end that a max_messages = 1 config produces a Prune resolution. Note: L1 review finding (Session::set_messages dirty-mark) was verified to be a non-issue — set_messages already bumps messages_generation (same counter as mark_messages_mutated), so SummarizeTrim's compacted state already propagates correctly. No code change needed.
…im drops fake Session (#4683 review N1) The kernel's cron SummarizeTrim path used to fabricate a throwaway librefang_memory::session::Session purely to satisfy compact_session's &Session parameter, even though compact_session itself only ever read session.messages. The constructed tmp_session pinned six fields (context_window_tokens=0, label=None, messages_generation=0, etc.) that were never consumed and lied about the source — readers of try_summarize_trim had to chase the &Session signature into compactor to discover the rest didn't matter. Refactor the boundary so the messages are first-class: - compactor.rs: extract compact_messages(driver, model, &[Message], cfg) holding the 3-stage pipeline body that compact_session used to contain. compact_session is now a 1-line delegate, preserved for backward compatibility (existing callers like /compact don't need to change). - kernel/mod.rs: try_summarize_trim drops the tmp_session block, drops session_id and agent_id from its signature (only used to populate fake Session fields), and calls compact_messages directly on the to_summarize slice. Call site at the cron dispatch loses the matching cron_sid/agent_id arguments. Behaviour: identical. Same 3-stage summarization, same used_fallback contract, same tool-pair adjustment, same caller post-processing. Verification: - cargo clippy -p librefang-kernel -p librefang-runtime --all-targets -- -D warnings -> clean - compactor unit tests: 32/32 pass (compact_session delegating path) - cron_compaction_test integration: 3/3 pass (still uses compact_session; proves the delegate) - cron_resolve / cron_clamp / cron_compute_keep_count helpers: all pass
…rize-trim (#4683 review N2) Two latency cuts on the SummarizeTrim path that were called out in the review's "holding prune_lock across the LLM call" note: * `try_summarize_trim` fast-fails when the resolved model name is empty (agent missing from registry between cron tick and prune). Without this, the empty string would still walk stage-1 single-pass (default 3 retries) -> stage-2 chunked -> stage-3 fallback placeholder before returning `used_fallback=true`, holding the per-session mutex and one cron_lane slot through the full fail-out chain. The path is logged as `warn` already; the registry-miss symptom does not need to be amplified into a long-running LLM attempt. * Override `CompactionConfig.max_retries = 1` for the cron-side call. The default of 3 retries inside stage-1 plus the additional stage-2 chunked attempt can stretch to tens of seconds on a flaky provider, blocking sibling fires for the same `(agent, "cron")` session. Cron's failure mode is "fall back to plain prune", so a single attempt is sufficient -- operators who want more aggressive retries should configure them in the aux driver itself, not amplify them inside the cron prune path. Drive-by clarifications driven by the same review pass: * Rename `mutated` -> `needs_compaction` to match its actual semantic ("did the cap demand a shrink?"), and thread the verdict into the post-compaction budget warn as `post_compaction = bool`. Operators can now distinguish "we just shrank and are still over the soft threshold" (real signal -- the current fire's content is large) from "the session was already over before any compaction" (tighten the cap). * Update the registry-miss warn message to reflect the new fast-fail behaviour ("skipping LLM summary and falling back to plain prune") and document why the post-compaction warn landing right after a successful summary is expected, not a bug. * Mark `compact_messages` `#[doc(hidden)]` and document its workspace-internal status -- exposed only so the kernel's `try_summarize_trim` can call across the runtime <-> kernel crate boundary.
Closes the only remaining "Out of scope" gap from the #4683 review: `try_summarize_trim` was previously covered only via the underlying `compact_messages` surface in tests/cron_compaction_test.rs, which let the kernel's wrapper format and branching escape direct verification. Adds five `#[tokio::test]` cases against the helper itself, reachable from the child tests module without changing visibility: * `try_summarize_trim_empty_model_short_circuits_without_calling_driver` -- pins the L2 fast-fail (commit f5332e3). A `CountingFakeDriver` asserts the LLM driver is invoked zero times when the resolved model name is empty, so the per-session mutex / cron_lane slot is not held across a guaranteed-fail LLM round-trip. * `try_summarize_trim_keep_recent_covers_everything_returns_none` -- exercises the `tail_start == 0` short-circuit (keep_recent >= len) and asserts the driver is not called. * `try_summarize_trim_successful_returns_summary_plus_tail` -- happy path: asserts the kernel's wrapper format ("[Cron session summary -- 7 messages compacted]"), the compacted-count math (10 - keep_recent=3), and that the kept tail is the verbatim newest 3 messages in order. * `try_summarize_trim_llm_failure_via_used_fallback_returns_none` -- pins the M4 guard: an `AlwaysFailingDriver` makes `compact_messages` walk stage-1 -> stage-2 -> stage-3 placeholder and return used_fallback=true; the helper must reject that and return None so the caller drops to plain prune. * `try_summarize_trim_does_not_split_tool_pair_across_summary_boundary` -- ToolUse @6 / ToolResult @7 with keep_recent=3 (raw split between them). Asserts adjust_split_for_tool_pair shifted the split forward so the ToolResult does not appear orphaned in the kept tail. No production code change. Helper-level coverage that previously only existed in spirit.
houko
added a commit
that referenced
this pull request
May 6, 2026
…tion_* in dashboard overlay Two issues were rolled into one fix because they share the same root cause (#4682's `every_kernel_config_struct_field_is_exposed_via_overlay` test catches new `KernelConfig` fields that lack a dashboard section descriptor). 1. Wiki PR #4712 added `KernelConfig.memory_wiki` (sub-struct). Adds `{key: "memory_wiki", struct_field: "memory_wiki"}` to `ui_sections_overlay()` so the dashboard ConfigPage renders it alongside `[memory]` / `[proactive_memory]`. 2. Main is **already red** on the same test for three pre-existing field-overlay gaps that #4683 (`fix(cron): summarize-and-trim`) and #4677 (`feat(runtime): tool-exec backend trait`) shipped without: - `cron_session_compaction_mode` — root-level scalar, parallel to `cron_session_max_*`. Added to general.fields and to the test's EXCLUDED list (with comment) so the dashboard inlines it under the existing cron_session_* group. - `cron_session_compaction_keep_recent` — same treatment. - `tool_exec` — sub-struct, gets its own section entry next to the other tool_* sections. Verifies main goes green again (Test / macOS, Test / Windows, Test / Ubuntu (shard 2/4) all failed on this exact test on 4d17e4b) and unblocks the wiki PR's CI lane on the same matrix. Per CLAUDE.md "improvements found in the same conversation collapse into one PR" rather than splitting into a separate hotfix — the wiki PR is the only branch already touching this file, and shipping the cron/tool-exec descriptors here gets main green one push sooner than a separate PR cycle. Verification - `cargo test -p librefang-api --test config_schema_overlay` — 4 pass on host
houko
added a commit
that referenced
this pull request
May 6, 2026
…tion_* in dashboard overlay Two issues were rolled into one fix because they share the same root cause (#4682's `every_kernel_config_struct_field_is_exposed_via_overlay` test catches new `KernelConfig` fields that lack a dashboard section descriptor). 1. Wiki PR #4712 added `KernelConfig.memory_wiki` (sub-struct). Adds `{key: "memory_wiki", struct_field: "memory_wiki"}` to `ui_sections_overlay()` so the dashboard ConfigPage renders it alongside `[memory]` / `[proactive_memory]`. 2. Main is **already red** on the same test for three pre-existing field-overlay gaps that #4683 (`fix(cron): summarize-and-trim`) and #4677 (`feat(runtime): tool-exec backend trait`) shipped without: - `cron_session_compaction_mode` — root-level scalar, parallel to `cron_session_max_*`. Added to general.fields and to the test's EXCLUDED list (with comment) so the dashboard inlines it under the existing cron_session_* group. - `cron_session_compaction_keep_recent` — same treatment. - `tool_exec` — sub-struct, gets its own section entry next to the other tool_* sections. Verifies main goes green again (Test / macOS, Test / Windows, Test / Ubuntu (shard 2/4) all failed on this exact test on 4d17e4b) and unblocks the wiki PR's CI lane on the same matrix. Per CLAUDE.md "improvements found in the same conversation collapse into one PR" rather than splitting into a separate hotfix — the wiki PR is the only branch already touching this file, and shipping the cron/tool-exec descriptors here gets main green one push sooner than a separate PR cycle. Verification - `cargo test -p librefang-api --test config_schema_overlay` — 4 pass on host
houko
added a commit
that referenced
this pull request
May 7, 2026
#3329) (#4712) * feat(memory-wiki): scaffold durable knowledge vault — isolated mode v1 (#3329) Adds the `librefang-memory-wiki` crate, a markdown knowledge vault that pairs with the SQLite/vector memory substrate. Where memory answers "find me the K nearest snippets", the wiki answers "give me a navigable knowledge base I can also open in Obsidian and edit by hand". What lands - New crate `librefang-memory-wiki` with `WikiVault`, YAML frontmatter (topic, created, updated, content_sha256, append-only provenance), `[[topic]]` placeholder rewrite for `native` / `obsidian` render modes, deterministic `index.md` and `_meta/backlinks.json`, and hand-edit safety via mtime + sha256 drift detection (refuses silent overwrite; `force=true` preserves the human edit and only appends a new provenance entry). - New `WikiAccess` role trait on `KernelHandle` (#3746 split style), with default impls returning `unavailable("wiki")` so existing stubs and mocks compile unchanged. 13 stubs across the workspace pick up an empty `impl WikiAccess for X {}`. - `LibreFangKernel` constructs a vault when `[memory_wiki] enabled = true`; vault construction failures log a warning and disable the vault for that boot rather than aborting the daemon. - Three builtin tools: `wiki_get`, `wiki_search`, `wiki_write`. Provenance is built kernel-side from the calling agent so the LLM cannot spoof it. - New `[memory_wiki]` config block (`enabled`, `mode`, `vault_path`, `render_mode`, `ingest_filter`) — off by default. Out of scope for this PR (tracked under #3329 follow-ups) - `bridge` and `unsafe_local` modes return a typed `ModeNotImplemented` until follow-up. - Subscribing to `memory_store` events with a durable filter — v1 ingests via explicit `wiki_write` only. - Extending `memory_search` with a `corpus = all|kv|wiki` parameter. - LLM-assisted topic extraction (v1 requires explicit `topic` tags). Verification - `cargo test -p librefang-memory-wiki` — 32 pass (23 unit + 9 acceptance covering all seven bullets in the issue) - `cargo check` on librefang-memory-wiki / -types / -kernel-handle / -kernel / -runtime - `cargo clippy -p librefang-memory-wiki -p librefang-types -p librefang-kernel-handle --all-targets -D warnings` - CI runs the full workspace clippy + integration suite. * fix(memory-wiki): self-review follow-ups for #3329 PR Addresses every issue surfaced in the PR self-review pass: A. `KernelOpError::unavailable` is now method-level on the kernel impl (`"wiki_get"` / `"wiki_search"` / `"wiki_write"`), matching the trait default impls so audit logs / API consumers get one consistent capability name. B. `PageState.mtime_ns` doc clarifies the canonical-decimal format (no leading zeros, no separators) so the byte-equality used by drift detection is explicit, not implicit. D. `WikiVault::new` emits a `tracing::warn!` when an operator sets `ingest_filter = "all"` since v1 has no behavioural effect for that value — silent no-op replaced with a loud signal. E. `wiki_search` now scores body hits as `ln(1 + matches)` instead of linear, so a 1000-word page with 10 hits no longer buries a short topic-only match under sheer volume. I. `wiki_write` rejects bodies > 1 MiB with `InvalidTopic` to bound the LLM-driven fill-the-disk failure mode. Constant `MAX_BODY_BYTES` documented. K. New `crates/librefang-memory-wiki/tests/wiki_handle_contract.rs` asserts the JSON shape every `WikiAccess` consumer (tool dispatcher, future HTTP route, dashboard) is allowed to rely on. Mirrors the production kernel-side adaptor verbatim so drift between the two is caught at PR time. L. New `concurrent_writes_to_same_topic_are_serialised` proves the in-process write mutex serialises 8 concurrent writers without data loss; provenance is monotonic across whichever subset landed. M. `read_page_if_present` falls back to a synthetic header with a `WARN` log when on-disk YAML fails to parse, so a corrupted frontmatter no longer bricks every subsequent `wiki_get` / `wiki_search` call against that page. O. `MemoryWikiConfig::resolved_vault_path` and `expand_tilde` docstrings spell out that `~user/...` is intentionally not honoured (only bare `~` and `~/...`). Plus a `defaults_returns.rs` test verifying `WikiAccess` default impls return the per-method `Unavailable("wiki_<verb>")` capability name on stubs that don't override. Verification - `cargo test -p librefang-memory-wiki` — 40 pass (26 unit + 9 acceptance + 5 handle contract) - `cargo test -p librefang-kernel-handle` — 19 pass - `cargo check -p librefang-memory-wiki -p librefang-types -p librefang-kernel-handle -p librefang-kernel -p librefang-runtime` * fix(memory-wiki): address Codex review for #3329 (P1 + 2x P2) P1. Regenerate `kernel_config_schema.golden.json` so the new `[memory_wiki]` block (and the four supporting enums: `MemoryWikiConfig`, `MemoryWikiMode`, `MemoryWikiRenderMode`, `MemoryWikiIngestFilter`) appear in the schema fixture. Without this, any future un-ignore of `kernel_config_schema_matches_ golden_fixture` would surface drift on first run. P2. `config_reload.rs` now compares `old.memory_wiki` with `new.memory_wiki` and marks the change `restart_required` (the vault is constructed at boot inside `LibreFangKernel.wiki_vault` and cannot be hot-swapped). Mirrors the existing `[memory]` treatment so an operator toggling `enabled` or pointing `vault_path` somewhere else gets a loud signal instead of a silent no-op. P2. `MemoryWikiConfig::resolved_vault_path` is now signature `(&self, home_dir: &Path)` and uses the **caller-supplied** home_dir rather than env-derived `librefang_home_dir()` for the fallback `<home>/wiki/main` location. `WikiVault::new` likewise takes `(config, home_dir)` and the kernel passes `&config.home_dir`. Stops embedded / test profiles from silently bleeding into a developer's `~/.librefang/wiki/main`. New `default_vault_path_uses_caller_home_not_env` test pins it. Verification - `cargo test -p librefang-memory-wiki` — 41 pass (27 lib + 9 acceptance + 5 handle contract — added one regression test for the home_dir fallback) - `cargo test -p librefang-kernel --lib config_reload` — 33 pass - `cargo test -p librefang-api --test config_schema_golden -- --ignored regenerate_golden` — fixture rewritten (552_635 bytes), now contains the four wiki schema definitions * fix(memory-wiki): tighten body-cap error to its own variant (#3329) The body-too-large guard previously surfaced as `WikiError::InvalidTopic { reason: "body exceeds 1 MiB cap..." }`, which is structurally misleading — the topic is fine, only the body exceeds the cap. A caller pattern-matching on `InvalidTopic` would mis-route the error to "fix the topic name" instead of "split the page". New `WikiError::BodyTooLarge { topic, size, cap }` carries all three numbers so the kernel can render an actionable `InvalidInput` message and a future SDK can present a typed recovery hint. Self-review caught this while looking for behavioural drift in `WikiVault::write` and the surrounding mtime/sha drift logic; no other logic bugs surfaced — the force-write body-preservation, fallback-on-broken-yaml, and concurrent-write paths all check out. Verification - `cargo test -p librefang-memory-wiki` — 41 pass (27 lib + 9 acceptance + 5 handle contract) - `cargo check -p librefang-kernel --lib` — pass on host (docker image lacks libdbus-1-dev for librefang-extensions's keyring dep; CI Linux runner covers it) * fix(memory-wiki): add missing WikiAccess stub to RecordingKernel Wasm host-functions test stub `RecordingKernel` was missed by the 13-stub batch in `167bf382` because it lives in `crates/librefang-runtime-wasm/src/host_functions.rs` rather than the `tool_runner` / `kernel-handle` test directories the original sweep walked. CI surfaced the gap as `error[E0277]: the trait `WikiAccess` is not implemented for RecordingKernel` across all 9 Test / Quality / Coverage jobs (one build break, every parallel matrix lane reports it). The stub is a one-liner: every `WikiAccess` method has a default impl returning `unavailable("wiki_*")`, so `impl WikiAccess for RecordingKernel {}` is enough. Verified locally with `cargo check -p librefang-runtime-wasm --all-targets` on host cargo. * fix(api/config): expose memory_wiki + tool_exec + cron_session_compaction_* in dashboard overlay Two issues were rolled into one fix because they share the same root cause (#4682's `every_kernel_config_struct_field_is_exposed_via_overlay` test catches new `KernelConfig` fields that lack a dashboard section descriptor). 1. Wiki PR #4712 added `KernelConfig.memory_wiki` (sub-struct). Adds `{key: "memory_wiki", struct_field: "memory_wiki"}` to `ui_sections_overlay()` so the dashboard ConfigPage renders it alongside `[memory]` / `[proactive_memory]`. 2. Main is **already red** on the same test for three pre-existing field-overlay gaps that #4683 (`fix(cron): summarize-and-trim`) and #4677 (`feat(runtime): tool-exec backend trait`) shipped without: - `cron_session_compaction_mode` — root-level scalar, parallel to `cron_session_max_*`. Added to general.fields and to the test's EXCLUDED list (with comment) so the dashboard inlines it under the existing cron_session_* group. - `cron_session_compaction_keep_recent` — same treatment. - `tool_exec` — sub-struct, gets its own section entry next to the other tool_* sections. Verifies main goes green again (Test / macOS, Test / Windows, Test / Ubuntu (shard 2/4) all failed on this exact test on 4d17e4b) and unblocks the wiki PR's CI lane on the same matrix. Per CLAUDE.md "improvements found in the same conversation collapse into one PR" rather than splitting into a separate hotfix — the wiki PR is the only branch already touching this file, and shipping the cron/tool-exec descriptors here gets main green one push sooner than a separate PR cycle. Verification - `cargo test -p librefang-api --test config_schema_overlay` — 4 pass on host * fix(memory-wiki): tolerate CRLF frontmatter from external editors (#3329) Codex review (P2): a wiki page hand-authored on Windows or saved by an editor with `git config core.autocrlf=true` lands on disk with CRLF (`---\r\n` / `\r\n---\r\n`) frontmatter delimiters. Our `frontmatter::split` matcher is LF-only, so it returned `(None, raw)` for those files — the entire YAML block leaked into `body`, and a subsequent `wiki_write` (especially `force = true`) wrapped that into a *new* frontmatter, corrupting the page instead of preserving the hand edit. Fix: normalise CRLF → LF once on read in `read_page_if_present` before handing the buffer to `split`. The vault's own `render()` already emits LF, so this only affects externally authored content; the next successful write standardises the file to LF on disk (matching git's recommended Markdown convention). Coverage: new `crlf_authored_pages_are_parsed_not_treated_as_bodyless` test rewrites a vault page with `\r\n` line endings and asserts `wiki_get` parses the frontmatter rather than spilling YAML into `body`. Verification - `cargo test -p librefang-memory-wiki` — 28 lib + 9 acceptance + 5 handle contract pass on host
houko
added a commit
that referenced
this pull request
May 7, 2026
…::cron_tick The cron scheduler tick loop was historically the longest closure in mod.rs (~528 lines, the landing zone for #4683 et al.). Lift the body out of `spawn_logged("cron_scheduler", async move { … })` into a free `pub(super) async fn run_cron_scheduler_loop(kernel: Arc<LibreFangKernel>)` in `crates/librefang-kernel/src/kernel/cron_tick.rs`. Behaviour-preserving — body moved byte-for-byte; only the outer wrapper changed (closure → free fn). Captured state was just `kernel`, which becomes the single fn parameter; per-tick `cron_sem` and per-job clones are still constructed inside the loop body, unchanged. mod.rs drops by ~528 lines (now ~16.2k). Three Phase 2 re-exports (`cron_deliver_response`, `cron_fan_out_targets`, `cron_script_wake_gate`) that mod.rs introduced for the inline closure are no longer needed — the only call sites moved with the loop — so they're gone from mod.rs's `use` block. Verified: cargo check / clippy -- -D warnings / cargo check --tests all green.
houko
added a commit
that referenced
this pull request
May 7, 2026
…::cron_tick The cron scheduler tick loop was historically the longest closure in mod.rs (~528 lines, the landing zone for #4683 et al.). Lift the body out of `spawn_logged("cron_scheduler", async move { … })` into a free `pub(super) async fn run_cron_scheduler_loop(kernel: Arc<LibreFangKernel>)` in `crates/librefang-kernel/src/kernel/cron_tick.rs`. Behaviour-preserving — body moved byte-for-byte; only the outer wrapper changed (closure → free fn). Captured state was just `kernel`, which becomes the single fn parameter; per-tick `cron_sem` and per-job clones are still constructed inside the loop body, unchanged. mod.rs drops by ~528 lines (now ~16.2k). Three Phase 2 re-exports (`cron_deliver_response`, `cron_fan_out_targets`, `cron_script_wake_gate`) that mod.rs introduced for the inline closure are no longer needed — the only call sites moved with the loop — so they're gone from mod.rs's `use` block. Verified: cargo check / clippy -- -D warnings / cargo check --tests all green.
houko
added a commit
that referenced
this pull request
May 7, 2026
… compaction The Phase 1 mechanical-move subagent worked off a state that pre-dated four recent main commits, silently reverting their changes during the trait-impl extraction: - #4685 (prompt store r2d2 pool): mod.rs called `memory.usage_conn()` and passed 1 arg to `PromptStore::new_with_path`. Both APIs had been replaced with `memory.pool()` / 2-arg `new_with_path(&db_path, pool_size)`. Restored. - #3329 (memory_wiki vault): `kernel_handle::WikiAccess` is a `KernelHandle` super-trait but the `wiki_vault` field is not yet on `LibreFangKernel` in this branch. Replace the broken impl with an empty `impl WikiAccess for LibreFangKernel {}` so every method falls through to the trait default (`KernelOpError::unavailable("wiki_*")`) until the rebase-on-main work pulls in the full vault wiring. - #4683 (cron summarize-trim compaction): `kernel::tests` references `cron_compute_keep_count` / `cron_clamp_keep_recent` / `cron_resolve_compaction_mode` / `try_summarize_trim` that were dropped from mod.rs. Cherry-pick the four helpers into a new `kernel::cron_compaction` sub-module + re-export from mod.rs so tests resolve. The matching cron-tick body change still needs to be ported into `kernel::cron_tick` — that's a separate item. Verified: cargo check -p librefang-kernel --lib + cargo clippy -p librefang-kernel --lib -- -D warnings + cargo check -p librefang-kernel --lib --tests + rustfmt --check all green. Note for reviewers: this PR's branch is "behind 4" of origin/main. The proper fix is a rebase, which will involve genuine merge work in the giant impl block that the refactor sliced. This commit is the minimum to keep HEAD compiling against the current workspace deps; it does NOT restore #4683's runtime changes (the SummarizeTrim path inside the cron tick body) — only the helpers + their tests.
houko
added a commit
that referenced
this pull request
May 7, 2026
…n_tick Phase 3b's `cron_tick.rs` body was the pre-#4683 closure (the version my Phase 1 subagent saw); the SummarizeTrim runtime path was lost when the Phase 1 work happened to omit #4683's mod.rs hunk. The earlier repair commit restored the helper fns into `kernel::cron_compaction` but left their call site missing. Replace the body of `run_cron_scheduler_loop` with the post-#4683 version copied from origin/main's mod.rs (the `spawn_logged("cron_scheduler", async move { … })` block at lines 14386..15064). Now the cron tick body actually calls: - `cron_compute_keep_count` to size the keep window without mutating - `cron_resolve_compaction_mode` to fall back from SummarizeTrim → Prune when keep_count < 2 - `cron_clamp_keep_recent` to enforce `[summary] + tail` cap - `try_summarize_trim` (aux LLM compaction) on the SummarizeTrim path - `apply_cron_prune` for both the configured Prune path and the SummarizeTrim → Prune fallback `apply_cron_prune` was bumped to `pub(super)` so cron_tick.rs (sibling submodule of cron_compaction) can call it. Verified: cargo check / clippy -- -D warnings / cargo check --tests / rustfmt --check all green.
7 tasks
houko
added a commit
that referenced
this pull request
May 7, 2026
…hases 1-3) (#4713) * refactor(kernel): extract 16 kernel_handle trait impls into kernel::handles Phase 1 of the kernel/mod.rs split. Move every `impl kernel_handle::* for LibreFangKernel` block out of the 20K-line god-file into per-trait files under `kernel/handles/`. The submodules are descendants of `kernel`, so they retain access to LibreFangKernel's private fields and inherent methods without any visibility surgery. Mechanical, behavior-preserving move — no method bodies were touched. mod.rs drops from ~20.6k to ~18.8k lines (-2073). Three imports that became unused after the move were removed (kernel_handle self-alias, ToolApprovalSubmission, std::str::FromStr) and re-imported locally in the consuming handle files. Traits moved: AgentControl, MemoryAccess, TaskQueue, EventBus, KnowledgeGraph, CronControl, HandsControl, ApprovalGate, A2ARegistry, ChannelSender, PromptStore, WorkflowRunner, GoalControl, ToolPolicy, ApiAuth, SessionWriter Verified: cargo check -p librefang-kernel --lib + cargo clippy -p librefang-kernel --lib -- -D warnings both green. * refactor(kernel): phase 2 — extract free-fn / cron-bridge / probe helpers Move the cohesive bottom-of-file helpers out of kernel/mod.rs into thematic sub-modules of `kernel`: mcp_summary.rs – mcp_summary_cache_key, render_mcp_summary reviewer_sanitize.rs – sanitize_reviewer_line, sanitize_reviewer_block cron_script.rs – cron_script_wake_gate, atomic_write_toml, parse_wake_gate cron_bridge.rs – `impl CronChannelSender for KernelCronBridge`, CRON_EMPTY_OUTPUT_HEARTBEAT, cron_fan_out_targets, cron_deliver_response provider_probe.rs – probe_and_update_local_provider, probe_all_local_providers_once Mechanical, behavior-preserving move. mod.rs drops by ~636 lines (now ~18.2k). The struct `KernelCronBridge` itself stays in mod.rs; only its trait impl moves. Free fns formerly visible only to mod.rs are now `pub(super)` so the (still-resident) `LibreFangKernel` methods that call them keep compiling. mod.rs adds a `use cron_bridge::{cron_deliver_response, cron_fan_out_targets};` re-export so existing call sites in mod.rs and `kernel::tests` resolve byte-for-byte. Also restores `kernel_handle::self` alongside the prelude wildcard import — phase 1 dropped the self alias as "newly unused", missing the fact that `kernel::tests` uses it via `kernel_handle::ApprovalGate::...`. Without this, `cargo test -p librefang-kernel --lib --no-run` fails. * refactor(kernel): phase 3a — extract accessor / lifecycle impl block to kernel::accessors Move the first `impl LibreFangKernel { ... }` block (formerly mod.rs lines 1106..2598, ~1500 lines, ~85 methods — accessors like `home_dir`, `config_ref`, `cron`, `pairing_ref`, plus operational helpers like `spawn_key_validation`, `spawn_approval_sweep_task`, `record`, `auto_dream_*`, etc.) into `crates/librefang-kernel/src/kernel/accessors.rs`. Sibling submodule of `kernel::mod`, so retains private-field/method access into `LibreFangKernel` without any visibility surgery (same pattern as the Phase 1 trait-impl moves). Mechanical, behavior-preserving move — method bodies untouched. mod.rs drops from ~18.2k to ~16.7k lines (-1500). 4 inherent `impl LibreFangKernel` blocks remain in mod.rs (down from 5); phase 3b/3c will continue chipping. Verified: cargo check / clippy -- -D warnings / cargo check --tests / rustfmt --check all green. * refactor(kernel): phase 3b — extract cron tick loop closure to kernel::cron_tick The cron scheduler tick loop was historically the longest closure in mod.rs (~528 lines, the landing zone for #4683 et al.). Lift the body out of `spawn_logged("cron_scheduler", async move { … })` into a free `pub(super) async fn run_cron_scheduler_loop(kernel: Arc<LibreFangKernel>)` in `crates/librefang-kernel/src/kernel/cron_tick.rs`. Behaviour-preserving — body moved byte-for-byte; only the outer wrapper changed (closure → free fn). Captured state was just `kernel`, which becomes the single fn parameter; per-tick `cron_sem` and per-job clones are still constructed inside the loop body, unchanged. mod.rs drops by ~528 lines (now ~16.2k). Three Phase 2 re-exports (`cron_deliver_response`, `cron_fan_out_targets`, `cron_script_wake_gate`) that mod.rs introduced for the inline closure are no longer needed — the only call sites moved with the loop — so they're gone from mod.rs's `use` block. Verified: cargo check / clippy -- -D warnings / cargo check --tests all green. * fix(kernel): repair phase 1 regressions on prompt store / wiki / cron compaction The Phase 1 mechanical-move subagent worked off a state that pre-dated four recent main commits, silently reverting their changes during the trait-impl extraction: - #4685 (prompt store r2d2 pool): mod.rs called `memory.usage_conn()` and passed 1 arg to `PromptStore::new_with_path`. Both APIs had been replaced with `memory.pool()` / 2-arg `new_with_path(&db_path, pool_size)`. Restored. - #3329 (memory_wiki vault): `kernel_handle::WikiAccess` is a `KernelHandle` super-trait but the `wiki_vault` field is not yet on `LibreFangKernel` in this branch. Replace the broken impl with an empty `impl WikiAccess for LibreFangKernel {}` so every method falls through to the trait default (`KernelOpError::unavailable("wiki_*")`) until the rebase-on-main work pulls in the full vault wiring. - #4683 (cron summarize-trim compaction): `kernel::tests` references `cron_compute_keep_count` / `cron_clamp_keep_recent` / `cron_resolve_compaction_mode` / `try_summarize_trim` that were dropped from mod.rs. Cherry-pick the four helpers into a new `kernel::cron_compaction` sub-module + re-export from mod.rs so tests resolve. The matching cron-tick body change still needs to be ported into `kernel::cron_tick` — that's a separate item. Verified: cargo check -p librefang-kernel --lib + cargo clippy -p librefang-kernel --lib -- -D warnings + cargo check -p librefang-kernel --lib --tests + rustfmt --check all green. Note for reviewers: this PR's branch is "behind 4" of origin/main. The proper fix is a rebase, which will involve genuine merge work in the giant impl block that the refactor sliced. This commit is the minimum to keep HEAD compiling against the current workspace deps; it does NOT restore #4683's runtime changes (the SummarizeTrim path inside the cron tick body) — only the helpers + their tests. * fix(kernel): port #4683 cron-tick SummarizeTrim path into kernel::cron_tick Phase 3b's `cron_tick.rs` body was the pre-#4683 closure (the version my Phase 1 subagent saw); the SummarizeTrim runtime path was lost when the Phase 1 work happened to omit #4683's mod.rs hunk. The earlier repair commit restored the helper fns into `kernel::cron_compaction` but left their call site missing. Replace the body of `run_cron_scheduler_loop` with the post-#4683 version copied from origin/main's mod.rs (the `spawn_logged("cron_scheduler", async move { … })` block at lines 14386..15064). Now the cron tick body actually calls: - `cron_compute_keep_count` to size the keep window without mutating - `cron_resolve_compaction_mode` to fall back from SummarizeTrim → Prune when keep_count < 2 - `cron_clamp_keep_recent` to enforce `[summary] + tail` cap - `try_summarize_trim` (aux LLM compaction) on the SummarizeTrim path - `apply_cron_prune` for both the configured Prune path and the SummarizeTrim → Prune fallback `apply_cron_prune` was bumped to `pub(super)` so cron_tick.rs (sibling submodule of cron_compaction) can call it. Verified: cargo check / clippy -- -D warnings / cargo check --tests / rustfmt --check all green. * refactor(kernel): phase 3c — extract 5 thematic clusters from giant impl Slice the still-resident giant `impl LibreFangKernel { ... }` body into five cohesive sub-modules under `kernel/`: messaging.rs — 26 send_message* / send_message_streaming_* variants + send_message_full / send_message_full_with_upstream + resolve_agent_home_channel + run_forked_agent_streaming (~2 670 lines) assistant_routing.rs — 12 helpers driving the @-mention / specialist routing path: notify_owner_bg, llm_classify_intent, resolve_or_spawn_specialist, send_message_streaming_resolved, resolve_assistant_target, route_assistant_by_metadata, etc. (~460 lines) hands_lifecycle.rs — 14 hand-management methods: activate_hand, deactivate_hand, reload_hands, apply_hand_agent_runtime_override_to_registry, resolve_hand_agent_model_defaults, etc. (~990 lines) mcp_setup.rs — 6 MCP wiring fns: connect_mcp_servers, reload_mcp_servers, retry_mcp_connection, reconnect_mcp_server, run_mcp_health_loop, disconnect_mcp_server (~610 lines) prompt_context.rs — 7 cached-summary builders for prompt assembly: cached_workspace_metadata, cached_skill_metadata, active_goals_for_prompt, build_skill_summary_from_skills, build_mcp_summary, collect_prompt_context, etc. (~340 lines) Mechanical, behavior-preserving move — method bodies untouched. mod.rs drops from ~16.2k to ~11.3k lines (-4 950). Combined with phases 1+2+3a+3b that's a 9 350-line reduction (-45%) on the original 20 609-line god-file. Visibility surgery (only what the compiler demanded; private inherent methods are not visible across sibling submodules without it): - messaging::send_message_full / send_message_streaming_with_sender_and_session bumped to pub(crate) — called from cron_tick + assistant_routing. - assistant_routing::{notify_owner_bg, send_message_streaming_resolved, resolve_assistant_target, assistant_route_key, should_skip_intent_classification} bumped to pub(crate) — called from messaging + tests. - mcp_setup::run_mcp_health_loop bumped to pub(crate) — kept-resident spawn_logged callsite in mod.rs needs it. - prompt_context::{cached_workspace_metadata, cached_skill_metadata, active_goals_for_prompt, build_mcp_summary} bumped to pub(crate) for cross-module consumers. - mod.rs structs CachedWorkspaceMetadata + CachedSkillMetadata bumped to pub(crate) struct (private fields kept) so the pub(crate) accessor signatures don't trip the private-interfaces lint. Verified across all 5 cluster checkpoints + final pass: cargo check -p librefang-kernel --lib + clippy -- -D warnings + check --tests + rustfmt --check all green. * refactor(kernel): phase 3d — extract 5 more clusters from giant impl Continue slicing the still-resident `impl LibreFangKernel { ... }` body into thematic sub-modules: boot.rs — session_stream_hub, boot, boot_with_config (~2 005 lines — `boot_with_config` is the single largest method extracted so far) spawn.rs — spawn_agent + 4 spawn_agent_with_* + spawn_agent_inner + verify_signed_manifest + trusted_manifest_signer_keys (~410 lines) agent_execution.rs — execute_wasm_agent, execute_python_agent, should_reuse_cached_route, is_brief_acknowledgement, execute_llm_agent (~1 195 lines) session_ops.rs — inject_message + inject_message_for_session + setup/teardown_injection_channel + resolve_module_path + reset/reboot_session + clear_agent_history + list/create/switch/export/import_session + inject_reset_prompt + evaluate_condition + save_session_summary (~820 lines) agent_state.rs — persist_manifest_to_disk, set_agent_model, reload_agent_from_disk, update_manifest, set_agent_skills, set_agent_mcp_servers, set_agent_tool_filters (~530 lines) Mechanical move — method bodies untouched. mod.rs drops from ~11.3k to ~6.4k lines (-4 870). Cumulative across phases 1+2+3a+3b+3c+3d the original 20 609-line god-file is now ~6 385 lines (-69%). Visibility surgery (only what the compiler demanded for cross-submodule call sites): - spawn::spawn_agent_inner → pub(crate) (called by hands_lifecycle) - agent_execution::execute_wasm_agent / execute_python_agent / execute_llm_agent / should_reuse_cached_route → pub(crate) (called by messaging + assistant_routing) - session_ops::{setup_injection_channel, teardown_injection_channel, resolve_module_path, inject_reset_prompt, evaluate_condition} → pub(crate) (called by messaging / spawn / tests) No struct or enum visibility bumps were needed — `boot.rs` builds the kernel via struct literal because descendant submodules retain private-field access. Verified: cargo check / clippy -- -D warnings / cargo check --tests / rustfmt --check all green at every cluster checkpoint and the final pass. * refactor(kernel): phase 3e/1 — extract agent_runtime cluster Move runtime-control methods (cost/stop/list/suspend/resume/compact/ context_report/kill + watchers) from the giant `impl LibreFangKernel` into `kernel/agent_runtime.rs`. mod.rs drops from 6 385 to 5 825 (-560). Methods: session_usage_cost, stop_agent_run, stop_session_run, list_running_sessions, agent_has_active_session, running_session_ids, suspend_agent, resume_agent, persist_agent_enabled, compact_agent_session, compact_agent_session_with_id, context_report, register_agent_watcher, abort_agent_watchers, kill_agent, kill_agent_with_purge. Verified: cargo check / clippy -- -D warnings / cargo check --tests all green. * refactor(kernel): phase 3e/2-7 — extract 6 remaining clusters from giant impl Continue slicing the still-resident `impl LibreFangKernel { ... }` body into thematic submodules. After this commit the giant first impl block is fully drained. bindings_and_handle.rs — set_log_reloader, set_self_handle, kernel_handle, list_bindings, add_binding, remove_binding (~95 lines) config_reload_ops.rs — reload_config + apply_hot_actions_inner (~480 lines) triggers_and_workflow.rs — spawn_session_label_generation, one_shot_llm_call, publish_event + publish_event_inner, register_trigger + register_trigger_with_target + remove_trigger / set_trigger_enabled / list_triggers / get_trigger / update_trigger, register_workflow, run_workflow, dry_run_workflow (~640 lines) background_lifecycle.rs — start_background_agents, start_ofp_node, self_arc, start_heartbeat_monitor, start_background_for_agent, shutdown (~1 270 lines — the longest single extraction in this PR) llm_drivers.rs — lookup_provider_url, resolve_driver (~250 lines) tools_and_skills.rs — available_tools, reload_skills, try_claim_skill_review_slot, summarize_traces_for_review, background_skill_review, is_transient_review_error, extract_json_from_llm_response, context_engine_for_agent (~1 080 lines) Mechanical move — method bodies untouched. mod.rs drops from ~5.8k to ~2.0k lines (-3 800). Combined with phases 1+2+3a+3b+3c+3d the original 20 609-line god-file is now at ~2 000 lines (-90%). Visibility surgery (only what the compiler demanded): - tools_and_skills::{context_engine_for_agent, try_claim_skill_review_slot, summarize_traces_for_review, background_skill_review, extract_json_from_llm_response, is_transient_review_error, MAX_INFLIGHT_SKILL_REVIEWS} → pub(crate) (cross-submodule + tests) - mod.rs `enum ReviewError` → pub(crate) enum (private-interfaces lint once tools_and_skills::background_skill_review's signature went pub(crate)) Plus a cosmetic doc-paragraph rewrap in `llm_drivers.rs` to satisfy `clippy::doc_lazy_continuation`. Verified: cargo check / clippy -- -D warnings / cargo check --tests all green. * fix(kernel): repair four behavioural regressions from mod split (#4713 review) The kernel/mod split landed type-clean but silently dropped four behavioural hooks that `cargo check` / `clippy` could not see. All four fail open / fail-permissive (rather than fail-closed), which is why CI green doesn't catch them — they're only visible by exercising the boot path or the wiki tools at runtime. Restored to parity with `origin/main` (pre-split): - `boot.rs`: re-add `config.tool_exec.validate()` after the workspace-wide `config.validate()` warning loop. Without it, a deployment with `[tool_exec] kind = "ssh"` or `"daytona"` but missing the matching subtable boots silently and fails on first tool call instead of aborting at boot. - `boot.rs`: `MemorySubstrate::open_with_chunking(...)` → `open_with_pool_size(..., config.memory.pool_size)`. The 3-arg form forces the r2d2 default pool size of 8 regardless of operator config, so `[memory] pool_size` (just landed in #4685) was a no-op for the primary substrate; only the prompt store init below this line still honoured the setting, leaving the two SQLite pools inconsistently sized. - `spawn.rs::spawn_agent_inner`: re-add the `manifest.tool_exec_backend` override validation block after `validate_manifest_module_path`. Without it, an agent whose manifest pins `tool_exec_backend = "ssh"` against a daemon lacking `[tool_exec.ssh]` enters the registry and fails every tool invocation thereafter, instead of being rejected pre-registry. - `mod.rs` + `boot.rs` + `handles/wiki_access.rs`: re-add the `wiki_vault: Option<Arc<librefang_memory_wiki::WikiVault>>` field on `LibreFangKernel`, the boot-time `WikiVault::new` initializer with the same fail-open warning behaviour as main, the `wiki_vault: wiki_vault.clone()` in the struct construction, and the concrete `wiki_get` / `wiki_search` / `wiki_write` method bodies (ported verbatim from the pre-split mod.rs). Without these three pieces, every `tool_wiki_*` call on a deployment with `[memory_wiki] enabled = true` flowed to the trait default and returned `KernelOpError::unavailable("wiki_*")`, silently disabling the just-landed #3329 feature. Phase 1 had left an empty `impl WikiAccess for LibreFangKernel {}` with a header comment claiming \"will arrive with the #3329 main-side merge\" — but #3329 was already on `origin/main` (#4712) at the time of the branch. Verified locally: cargo check -p librefang-kernel --lib OK cargo clippy -p librefang-kernel --lib -- -D warnings 0 warnings rustfmt --check --edition 2021 (4 modified files) clean Refs Codex review on commits 594aa42 / 0107d06 / 79eb683 of the parent PR — findings #2-#5 (the four P2 items still open at HEAD) addressed here. Codex P1 finding #1 (cron SummarizeTrim) was addressed earlier in 0107d06 and is unaffected by this commit. * fix(kernel): repair phase 3 regressions on reload_config + NO_REPLY allow-list (#4713) Phase 3e/3 moved Kernel::reload_config into kernel/config_reload_ops.rs but the body that landed there was the pre-#4664 tolerant version: it called crate::config::load_config(...) (which silently returns KernelConfig::default() on parse / migration / deserialize / include failures) and wrapped validator errors as "Validation failed: ..." without the "live config unchanged" reload-boundary pledge. The strict-loader function crate::config::try_load_config and its 7 direct unit tests in config.rs are still present (orphaned), but the 4 api integration tests + 1 kernel integration test that drive reload_config end-to-end all fail because the strict path was never re-wired after the move. Restore parity with #4664: - swap load_config for try_load_config and propagate the Err through the same "Config reload failed; live config unchanged: {e}" wrapper - wrap the validator branch with the same prefix so every reload-rejection path satisfies the "live config unchanged" invariant the integration helper relies on Also: phase 3 split Kernel::cron_tick into a sibling kernel/cron_tick.rs file that carries one inline comment mentioning the NO_REPLY literal. The silent_response_single_source_of_truth grep-guard in librefang-runtime allow-lists the literal in selected files; add cron_tick.rs next to the existing mod.rs allow entry so the guard stops flagging it. Verified locally: kernel + api + runtime tests pass; clippy clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a
summarize_trimcompaction mode to the persistent cron-session size guard so context is preserved instead of silently dropped.Closes Gap 4 of #3693. The other three gaps (warn before context-window hit, expose
session_token_countin API, document the existing knobs) landed earlier.Behavior
cron_session_compaction_mode = "prune"(default): unchanged — drop oldest messages from the front until the cap is satisfied.cron_session_compaction_mode = "summarize_trim": when the cap is hit, the messages that would be dropped are summarized via the configured[llm.auxiliary] compressiondriver into a single synthetic assistant message, prepended to a verbatim tail ofcron_session_compaction_keep_recentmessages (default8).used_fallback=true): falls back to plain prune with atracing::warn!. The fire is never blocked by a summarization failure.prune_lock) is held across the LLM summary call. This intentionally serializes SummarizeTrim runs against the same(agent, "cron")session so a sibling fire cannot race against an un-compacted snapshot. Trade-off is documented: a second fire that also needs to compact will block on the mutex up to LLM latency. Operators who need independent context per fire should setsession_mode = "new"on those jobs.Design choices
pruneso existing deployments don't suddenly start incurring an aux LLM round-trip per cron fire that exceeds the cap. SummarizeTrim is opt-in.AuxTask::Compression(the same lane used by/compact), not the agent's main driver, so a small/cheap model can be configured for summary work without affecting the agent's primary chat.keep_recentdefault is8— large enough to preserve the last few turns of context, small enough that even with a 5-cap user the runtime can clamp it down without becoming meaningless. Runtime clampskeep_recenttokeep_count - 1so[summary] + tailalways fits inside the size cap (without this, n=20 / max_messages=5 / keep_recent=8 would yield 9 messages and loop forever re-summarizing 1 message per fire).adjust_split_for_tool_pairis reused (madepub) so the summary/tail boundary never separates anAssistant{ToolUse}from its matchingUser{ToolResult}.Test plan
cargo test -p librefang-kernel --lib cron_clamp_keep_recent— 5 new helper testscargo test -p librefang-kernel --lib cron_compute_keep_count— 9 existing helper tests + invariant test combining bothcargo test -p librefang-kernel --test cron_compaction_test— successful summarize, LLM failure → fallback, tool-pair adjustmentcargo check -p librefang-kernel --testscargo clippy -p librefang-kernel --tests -- -D warningsOut of scope
kernel/mod.rsSummarizeTrim block. Helper-level coverage + the existingcompact_sessionintegration suite catches the logic; full kernel-spawn coverage is left to a follow-up because of the boot-cost amortization on the cron-tick test harness.