Bump rusqlite from 0.31.0 to 0.38.0#6
Closed
dependabot[bot] wants to merge 1 commit into
Closed
Conversation
Bumps [rusqlite](https://github.com/rusqlite/rusqlite) from 0.31.0 to 0.38.0. - [Release notes](https://github.com/rusqlite/rusqlite/releases) - [Changelog](https://github.com/rusqlite/rusqlite/blob/master/Changelog.md) - [Commits](rusqlite/rusqlite@v0.31.0...v0.38.0) --- updated-dependencies: - dependency-name: rusqlite dependency-version: 0.38.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
Contributor
Author
LabelsThe following labels could not be found: Please fix the above issues or remove invalid values from |
Contributor
Author
|
OK, I won't notify you again about this release, but will get in touch when a new version is available. If you'd rather skip all updates until the next major or minor version, let me know by commenting If you change your mind, just re-open this PR and I'll resolve any conflicts on it. |
This was referenced Mar 18, 2026
houko
added a commit
that referenced
this pull request
Apr 18, 2026
…ation Fifth Codex batch on #2704 — covers the skill-evolution subsystem (code the upstream PR #2694 shipped to main but that shows up in this PR's diff), plus the KV race on worker telemetry writes. librefang-skills (evolution.rs): - fuzzy_find_and_replace rejects empty old_string up front (#16). - update_skill / patch_skill re-verify skill_dir.exists() under the lock so concurrent delete_skill can't be resurrected (#3, #5). - delete_skill runs validate_name to reject path traversal (#12). - remove_supporting_file canonicalises + contains-checks the target (#6), matching write_supporting_file. - write_supporting_file scans content BEFORE writing (#7) so a rejected update doesn't destroy the pre-existing valid file. librefang-runtime (tool_runner.rs): - tool_skill_evolve_delete resolves the real installed skill's parent dir via registry.get() (#15) instead of unconditionally targeting the global skills_dir. librefang-types / librefang-kernel (approval policy): - Default require_approval list includes skill_evolve_* (#14). Updated serde `true`-shorthand + tests to match. librefang-kernel (kernel/mod.rs): - build_skill_summary sanitises the category key (#1) — tags are third-party data. - extract_json_from_llm_response tries every '{' instead of giving up after the first (#8). - Background skill review uses the agent's resolved driver and model for cost attribution (#13); falls back to defaults only when manifest resolution fails. worker (github-stats-worker/index.js): - Click counts sharded across 8 blobs, unioned on read (#39). - UI error reports sharded across 4 blobs plus legacy-key fallback for historical data (#41).
houko
added a commit
that referenced
this pull request
Apr 20, 2026
…oard parity, smoke script Six follow-ups bundled per request to keep the channel-progress effort in one PR. Done from #2 onward; #1 (per-OutputFormat backtick adaptation) was investigated and dropped — backtick renders correctly in TelegramHtml (<code>), SlackMrkdwn, Discord/Matrix Markdown (inline code), and the only adapter that strips backticks (PlainText for Mastodon) is already opted out of the progress path via suppress_error_responses. #2 buffered_text fallback test: new integration test test_bridge_streaming_adapter_kernel_and_transport_both_fail covers the V2 4th outcome (send_streaming Err + kernel Err). #3 surface context_warning PhaseChange so the user sees when the agent's context window was trimmed/overflowed. Other phases stay SSE-only. #4 collapse repeated tool calls within an iteration — replace last_progress_tool with iter_tools_seen HashSet cleared at every ContentComplete. Batch agents no longer spam "🔧 web_search" per parallel call; retries in a later iteration still get a fresh line. #5 i18n for the "failed" suffix — supports en/zh-CN/es/ja/de/fr (matches librefang_types::i18n). Language threaded from kernel.config_snapshot().language through start_stream_text_bridge. #6 prettify tool names — web_search → Web Search; MCP_call → MCP Call (preserves internal caps). Backticks dropped from progress lines since the prettified form is now the identity. #8 dashboard ToolCallCard parity — mirror prettifyToolName() in dashboard/src/lib/string.ts and apply it in the card header so chat reply and dashboard show the same rendering. #7 live-integration smoke script — scripts/tests/channel_progress_smoke.sh automates the daemon-side flow once the user supplies an LLM API key and a configured channel adapter. Documents the channel-delivery gap (needs an external webhook receiver). No driver injection hook added to the kernel — that would have been scope creep. Tests added/updated: - test_prettify_tool_name_snake_to_title / _kebab_and_dotted / _preserves_internal_caps - test_tr_progress_failed_languages - test_bridge_streaming_adapter_kernel_and_transport_both_fail - Updated existing assertions to expect prettified names
houko
added a commit
that referenced
this pull request
Apr 20, 2026
…m fix, show_progress, i18n, prettify, dashboard parity (#2793) * feat(channels): channel-progress v2 — Telegram fallback fix, show_progress config, integration test Three follow-ups to #2792 (channel progress markers): 1. Telegram (streaming-adapter) buffered_text fallback now uses the `_status` variant. The same regression class fixed for non-streaming adapters in V1 was still latent on the Telegram path: - send_streaming Ok + kernel Err → Done reaction + success=true - send_streaming Err + buffered → Done reaction + success=true Both now correctly emit Error reaction, record_delivery(false), and journal Failed when the kernel actually errored. The fallback path also honors `suppress_error_responses` when the buffered text is a sanitized error (defense-in-depth — Telegram is not in the opt-in list today, but the path is now uniform across all adapters). 2. New `agent.toml show_progress` field (default true) plumbed through the streaming bridge. When false, the bridge skips both the `🔧 tool_name` and `⚠️ tool_name failed` injections — useful for agents whose output is parsed downstream, or for pristine-output scenarios where status markers would leak into the response. The trait-impl side looks up the agent's manifest from the kernel registry once per dispatch and passes it through to `start_stream_text_bridge[_with_status]`. 3. New end-to-end integration test in `crates/librefang-channels/tests/bridge_integration_test.rs` that wires a real `BridgeManager` + `MockAdapter` (non-streaming) + `MockProgressHandle` (override of `send_message_streaming_with_sender_status` that synthesises a delta stream with progress markers). Verifies the V2 dispatch pipeline actually surfaces progress to non-streaming adapters end-to-end via the consolidated `send_response` call. Tests: - test_stream_bridge_show_progress_false_suppresses_all_markers: show_progress=false produces no 🔧/⚠️ markers but still flows the actual model prose through - test_bridge_non_streaming_adapter_sees_progress_markers: full BridgeManager → MockAdapter pipeline; verifies progress markers end up in the captured `send()` call Notes: - True "live daemon" integration test (per CLAUDE.md) requires LLM API keys + a configured channel adapter (Telegram bot token, etc.) which the daemon environment does not currently have. The in-process integration test exercises the full dispatch wiring using real tokio tasks/channels and a mock kernel handle. * fix(kernel/wizard): add show_progress to AgentManifest struct literal CI compile error: wizard.rs constructed AgentManifest with an explicit struct literal listing every field, so adding show_progress in V2 broke that one site (the wizard agent created via setup intent). All other AgentManifest constructors use ..Default::default() and pick up show_progress=true automatically; only this one was a full-literal. * feat(channels): channel-progress v3 — collapse, i18n, prettify, dashboard parity, smoke script Six follow-ups bundled per request to keep the channel-progress effort in one PR. Done from #2 onward; #1 (per-OutputFormat backtick adaptation) was investigated and dropped — backtick renders correctly in TelegramHtml (<code>), SlackMrkdwn, Discord/Matrix Markdown (inline code), and the only adapter that strips backticks (PlainText for Mastodon) is already opted out of the progress path via suppress_error_responses. #2 buffered_text fallback test: new integration test test_bridge_streaming_adapter_kernel_and_transport_both_fail covers the V2 4th outcome (send_streaming Err + kernel Err). #3 surface context_warning PhaseChange so the user sees when the agent's context window was trimmed/overflowed. Other phases stay SSE-only. #4 collapse repeated tool calls within an iteration — replace last_progress_tool with iter_tools_seen HashSet cleared at every ContentComplete. Batch agents no longer spam "🔧 web_search" per parallel call; retries in a later iteration still get a fresh line. #5 i18n for the "failed" suffix — supports en/zh-CN/es/ja/de/fr (matches librefang_types::i18n). Language threaded from kernel.config_snapshot().language through start_stream_text_bridge. #6 prettify tool names — web_search → Web Search; MCP_call → MCP Call (preserves internal caps). Backticks dropped from progress lines since the prettified form is now the identity. #8 dashboard ToolCallCard parity — mirror prettifyToolName() in dashboard/src/lib/string.ts and apply it in the card header so chat reply and dashboard show the same rendering. #7 live-integration smoke script — scripts/tests/channel_progress_smoke.sh automates the daemon-side flow once the user supplies an LLM API key and a configured channel adapter. Documents the channel-delivery gap (needs an external webhook receiver). No driver injection hook added to the kernel — that would have been scope creep. Tests added/updated: - test_prettify_tool_name_snake_to_title / _kebab_and_dotted / _preserves_internal_caps - test_tr_progress_failed_languages - test_bridge_streaming_adapter_kernel_and_transport_both_fail - Updated existing assertions to expect prettified names * fix(channels): correct success/err pairing + treat timeout as soft success + clippy collapsible_if Three review-driven fixes for the V3 PR: Bug 1 — Telegram path outcome 3 (send_streaming Err + kernel Ok) was recording delivery as success=true with err=Some(stream_error). The fallback send_response had already delivered the buffered text and the kernel succeeded, so the transport-side stream error is irrelevant to delivery accounting — keeping it in the err field produced a contradictory metric (success AND err). Now: err is Some only when kernel actually failed. Bug 2 — TIMEOUT_PARTIAL_OUTPUT_MARKER was being mapped to status=Err, which flipped the lifecycle reaction to Error and record_delivery to success=false. Pre-V2 the bridge had no status channel and treated these turns as Done because the model emitted useful partial output before the inactivity timer fired. Restore that semantics: status=Ok for timeouts. The user still sees the "[Task timed out…]" tail appended to their reply. Clippy collapsible_if — flatten the inner if show_progress && ... into match guards on the ToolExecutionResult and PhaseChange arms (CI clippy -D warnings caught this in run 24651228831). * chore(channels): drop dead 'let _ = e' annotation The variable 'e' (stream transport error) is already used at warn! line 2886 and the empty-buffer fallback at line 2953, so the explicit underscore-binding introduced by the Bug 1 fix is noise — clean it up. * test(channels): regression coverage for Bug 1 (success/err pairing) + Bug 2 (timeout-as-success) Both bugs were caught by review, not tests — add the missing coverage so they cannot silently regress. Bug 2 unit test: test_stream_bridge_timeout_partial_output_reports_ok_status Constructs a kernel handle that fails with a string containing TIMEOUT_PARTIAL_OUTPUT_MARKER. Asserts: - The user-facing text channel still receives the "[Task timed out…]" tail so the user knows the reply may be incomplete. - The status oneshot resolves to Ok(()) — NOT Err — so bridge.rs drives the lifecycle reaction to Done and record_delivery to success=true. Pre-V2 semantics preserved. Bug 1 integration test: test_bridge_streaming_adapter_kernel_ok_transport_fail_records_clean_success Adds MockKernelOkHandle (overrides send_message_streaming_with_sender_status to emit clean text + status=Ok, AND overrides record_delivery to capture every (success, err) pair). Combined with the existing MockFailingStreamingAdapter (always Err on send_streaming), the test exercises Telegram outcome 3: - Fallback send_response delivers the buffered text ✓ - record_delivery is called with success=true AND err=None — proving the transport-side stream error does NOT leak into the err field when the kernel itself succeeded. (Pre-fix this would have been success=true, err=Some(stream_e) — a contradictory metric.) * polish(channels): unify progress-line spacing, codepoint-safe dashboard prettify, smoke script auto-spawn Three minor follow-ups from review: #3 — Symmetric blank lines around progress markers. ToolUseStart used \n\n…\n; ToolExecutionResult and PhaseChange used \n…\n. Adjacent markers (e.g. 🔧 X right before⚠️ X failed) ended up on consecutive lines without a blank separator and many markdown renderers collapsed them into one paragraph. All three now use \n\n…\n\n so every renderer that respects markdown blank-line semantics shows them as separate blocks. #4 — prettifyToolName() in dashboard/src/lib/string.ts now iterates by Unicode codepoint (via spread) instead of UTF-16 unit. A tool name starting with a non-BMP character (e.g. emoji) no longer drops its surrogate half when uppercased. Mirrors the Rust-side prettifier in channel_bridge.rs which uses chars().next() (also codepoint-correct). Tool names are usually ASCII so this is forward-looking, not a fix. #5 — channel_progress_smoke.sh now auto-spawns a temporary agent when the daemon has none. Picks the first available LLM key (GROQ/OPENAI/ANTHROPIC/MINIMAX) for the provider, sends a minimal manifest_toml inline (the SpawnRequest schema requires either manifest_toml or template — name alone isn't valid), and despawns the agent on exit. Pre-existing agents are still reused untouched. * fix(channels/tests): extract DeliveryLog type alias to satisfy clippy::type_complexity CI clippy -D warnings rejected Arc<Mutex<Vec<(bool, Option<String>)>>> as too complex. Hoist into a named alias — same shape, named contract.
This was referenced Apr 26, 2026
houko
added a commit
that referenced
this pull request
Apr 26, 2026
…us callers (#3239) * fix(security): RBAC M3 follow-up — memory ACL fail-closed for anonymous callers Closes the fail-open in routes/memory.rs::guard_for_request — when the auth middleware doesn't attach an AuthenticatedApiUser (loopback dev or LIBREFANG_ALLOW_NO_AUTH=1), the previous fallback granted full read/write across every namespace plus pii_access, export_allowed, and delete_allowed. Any process at 127.0.0.1 or any deployment with the no-auth env opt-in could exfiltrate every memory fragment including PII and bulk-delete across agents. Other admin-gated RBAC endpoints (audit query, per-user budget, authz effective) already reject anonymous callers; memory ACL was the outlier. Replaced the fallback with a Viewer-equivalent ACL inlined as anonymous_fallback_acl(): read access scoped to the proactive namespace only; writes, exports, deletes, and PII access all denied. Loopback dashboard reads continue to work for the proactive surface; sensitive operations now fail closed at the namespace_acl layer (return AuthDenied → 403). Two new unit tests in #[cfg(test)] mod tests pin the contract: - anonymous_fallback_denies_pii_export_and_delete asserts each flag - anonymous_fallback_guard_gates_match_acl_intent walks the guard through every namespace gate to make sure the four leak channels (read kv:*/shared:*/kg, write proactive, export proactive, delete proactive) all return Deny Refs PR #3205 review item #6. * style: cargo fmt for memory.rs ACL helper
houko
added a commit
that referenced
this pull request
May 4, 2026
…esolution (#4534) * feat(api): trusted_proxies + trust_forwarded_for for real-client-IP Adds two top-level KernelConfig fields that let proxied deployments (Cloudflare Tunnel, nginx, Traefik, …) recover the real client IP from forwarding headers without being exploitable when no proxy is present. Closes the long-standing TODO referenced in the now-retired rate_limiter::resolve_client_ip doc comment. trusted_proxies = ["172.19.0.0/16", "127.0.0.1", "::1"] trust_forwarded_for = true When BOTH are set AND the TCP peer matches the allowlist, the daemon resolves the real client IP from forwarding headers (preference: CF-Connecting-IP → X-Real-IP → Forwarded (RFC 7239) → rightmost- untrusted hop in X-Forwarded-For) for: * the GCRA rate limiter (per-IP keying) * the auth-login rate limiter (per-IP keying) * the per-IP WebSocket connection cap + the WS connect log line Fail-closed by default — empty allowlist OR master-switch off OR peer not in allowlist OR malformed headers all collapse back to the TCP peer. A spoofed X-Forwarded-For from an untrusted internet client still hits the limiter on its real source, so the per-IP brute-force properties are preserved. Implementation * New crates/librefang-api/src/client_ip.rs with TrustedProxies (hand-rolled CIDR matcher, no new deps), resolve_real_client_ip, resolve_from_request. * GcraState + new AuthRateLimitState carry Arc<TrustedProxies> + trust_forwarded_for; server.rs compiles the allowlist once at boot and threads it through both middleware layers. * ws.rs::agent_ws uses the resolved IP for the WS slot key and the client_ip log field. * Old fail-closed-only rate_limiter::resolve_client_ip removed. * Auth-bypass loopback gates (no-auth-allow-loopback, LIBREFANG_ALLOW_NO_AUTH) intentionally still key on the TCP peer via addr.ip(), not the resolved IP — auth bypass must never follow a forwarded-header claim. Tests * 19 unit tests in client_ip: CIDR v4/v6, bare IPs, unmasked input, invalid-entry skip, all four header preferences, RFC 7239 bracketed v6 + obfuscated _token, multi-header XFF concatenation, v4:port suffix, malformed-fallback, all-trusted chain. * 3 integration tests in rate_limiter: - browsers behind a trusted proxy get independent buckets - rotating XFF from an untrusted peer does not bypass the limiter - master switch off ignores XFF even from a trusted peer * Existing 611 librefang-api lib + 271 integration tests still pass; workspace cargo clippy --all-targets -D warnings clean. * refactor(api): cache compiled TrustedProxies on AppState Threads `Arc<TrustedProxies>` + `trust_forwarded_for` through `AppState`, compiled once at boot in `server.rs`. The GCRA + auth-login middlewares now read from the cached instance instead of recompiling, and `ws::agent_ws` no longer re-parses the raw config strings (and re-emits the malformed-entry warning) on every WebSocket upgrade. Also adds explicit "Behaviour change" comments at the two `agent_ws` sites that propagate the resolved client IP into `SenderContext.user_id`, so a future reader / operator flipping the new flags on understands that any per-`user_id` kernel state (audit attribution, channel-sender keying, session continuity that keys on it) re-keys from proxy IP to real client IP at that moment. Pure documentation — no behaviour change vs the prior PR commit. Updates the test fixtures (`librefang-testing::test_app::build_state` and the three in-tree `AppState` init sites in `routes/{agents,memory,network}.rs`) to set the new fields to default (header trust off), matching the production default. Addresses review #2 (no per-upgrade recompilation), #5 (migration-notes comments), and #10 (drops the cosmetic `drop(proxy_cfg)` since the allowlist is now read straight off `AppState`). * fix(api): resolve real client IP in terminal_ws WS slot key `terminal_ws` was still keying its per-IP WS slot on `addr.ip()`, which is the same bug `agent_ws` was just fixed for: behind a trusted reverse proxy (cloudflared / nginx / Traefik), the TCP peer is the proxy and every terminal connection from every browser collapses onto a single shared slot — `max_ws_per_ip` then throttles the whole organisation the moment the second tab opens. Mirrors the `agent_ws` fix exactly: pulls the boot-compiled `Arc<TrustedProxies>` and `trust_forwarded_for` flag off `AppState` and calls `client_ip::resolve_real_client_ip` before `try_acquire_ws_slot`. Untrusted peers fall through to `addr.ip()` — a spoofed `X-Forwarded-For` from the open internet still hits the per-IP cap on its real source. `SenderContext.user_id` parity is N/A here: `terminal_ws` does not construct a `SenderContext` (the terminal is a PTY pipe, not a kernel agent message), so the only behavioural surface that needs to swap to the resolved IP is the WS slot key + the rejection log line. Addresses review #1. * fix(api): tighten client_ip parser correctness + safety Five header-parsing fixes lifted from review: - **XFF: bracketed IPv6 with port** — `[2001:db8::1]:1234` fell off the fallback parser because `parse::<IpAddr>` rejects the bracket form and the prior `rsplit_once(':')` left the brackets attached. The walker now strips a surrounding `[...]` (with or without `:port`) before retry, while still bailing on ambiguous unbracketed v6 + port rather than silently truncating. - **RFC 7239 `for=` parameter case-insensitive** — RFC 7230 §3.2.4 makes parameter names case-insensitive; we were only matching `for=` and `For=`. Now compares the key with `eq_ignore_ascii_case("for")`. - **RFC 7239 `unknown` token case-insensitive** — companion fix; now compares with `eq_ignore_ascii_case("unknown")` so `Unknown` / `UNKNOWN` short-circuit the same way as the lowercase form. - **Single-value headers reject the unspecified address** — a misconfigured proxy that injects `0.0.0.0` / `::` into `cf-connecting-ip` / `x-real-ip` no longer poisons the per-IP slot key with an address that can't be a real client. Loopback and RFC1918/ULA addresses are accepted by design — the trusted proxy may legitimately pass an internal-network client. Doc-comment spells out the policy. - **Documentation comments** for the asymmetries reviewers flagged: `single_ip_header` reads only the first instance of the named header (vs the XFF path which concatenates all); the `Forwarded` parser only inspects the first list element and obfuscated tokens short-circuit by design. Adds 8 new unit tests covering: bracketed-v6 with/without port, case-insensitive `for=` (4 variants), case-insensitive `unknown` (3 variants), unspecified-address rejection on both single-value headers (4 variants), loopback + private acceptance (5 variants), plus two WS-slot-key composition tests asserting the resolver collapses spoof attempts from an untrusted peer down to the real TCP source AND separates real clients behind the same trusted proxy into distinct slot keys. Addresses review #3, #4, #6, #7, #8, #9. --------- Co-authored-by: Evan <[email protected]>
houko
added a commit
that referenced
this pull request
May 4, 2026
Daemon-side fixes for the issues raised in code review of PR #4600: * CRITICAL #3 / MEDIUM #12 — install_from_registry was calling verify_archive_signature against {listing_url}.sig where listing_url is a GitHub Contents API URL. That .sig file never exists in the official registry layout, so the function silently returned Ok(()) on every install — meaning the entire Ed25519 archive lane was dead code that always passed. Replaced with index-membership check: refuse to install plugins not present in the signed plugins-index. The unused function is removed; the trust path now flows through the worker- signed flat index instead of a per-plugin .sig that never existed. * HIGH #5 / #16 — TOFU pubkey resolver was vulnerable to first-install MITM (cafe wifi, hostile DNS, subdomain takeover) silently pinning an attacker key forever. Added EMBEDDED_REGISTRY_PUBKEY compiled into the daemon binary as the primary trust root for the official registry. Resolution chain is now: env override > embedded > TOFU/HTTP (the latter two only consulted for self-hosted forks that opt in). * HIGH #6 — fetch_verified_index treated a missing or unreachable index.json.sig as a soft warning even when pubkey resolution was required. An attacker who could serve a doctored index but suppress the .sig (404 or network error) bypassed verification entirely. Now hard-fails for the official mirror; self-hosted forks keep the soft path so they can adopt signing incrementally. * MEDIUM #13 — TOFU cache file open was vulnerable to symlink attacks from compromised post-install hooks. Added O_NOFOLLOW + regular-file check on read, mode 0600 + O_NOFOLLOW on write (Unix). Windows path validates regular-file status and relies on NTFS ACLs. * MEDIUM #15 — added scripts/check-pubkey-lockstep.sh that fails when the pubkey constant drifts between the daemon's EMBEDDED_REGISTRY_PUBKEY and the three worker locations (registry-worker wrangler.toml, marketplace-worker wrangler.toml, _worker.js). Wire into CI before any cargo / wrangler build to catch silent rotation footguns. clippy + plugin_manager tests pass.
houko
added a commit
that referenced
this pull request
May 4, 2026
…4600) * feat(plugins): Ed25519 signing across workers + daemon TOFU resolver (#3805) Worker side - registry-worker: sign canonical index.json on cron refresh; new endpoints /api/registry/index.json, /api/registry/index.json.sig, /.well-known/registry-pubkey. Backward-compatible: when REGISTRY_PRIVATE_KEY is unset the cron skips signing and the new endpoints return 503. - marketplace-worker: sign per-version metadata on publish over the canonical string \`<slug>@<version>|<bundle_url>|<bundle_sha256>\`; new endpoints /v1/pubkey and /v1/download/<slug>/<version>/signature. Adds bundle_sig column to package_versions (NULL when the secret is unset). - web/workers/keygen.mjs: standalone Ed25519 keypair generator. Output format (PKCS#8 private + raw 32-byte public) verified compatible end-to-end with ed25519_dalek::VerifyingKey::from_bytes. Daemon side - Replace the all-zero OFFICIAL_REGISTRY_PUBKEY_B64 placeholder + hard-fail with resolve_registry_pubkey(): env LIBREFANG_REGISTRY_PUBKEY > TOFU cache ~/.librefang/registry.pub > HTTP fetch from LIBREFANG_REGISTRY_PUBKEY_URL (default https://librefang.ai/.well-known/registry-pubkey). - install_from_registry now downgrades Ed25519 verification to a warning when SHA-256 has already verified the manifest, hard-failing only when neither integrity check is available — fixes the case where every install attempt failed because no real registry pubkey was deployed yet. - fetch_verified_index keeps the hard-fail (no per-index checksum fallback). - Drops the three #3799 placeholder regression tests; replaces with three resolver tests covering the validator and cache path. Docs - web/workers/SIGNING.md: operator runbook (keygen, deploy, rotation). - docs/architecture/plugin-signing.md: trust model + layered defenses. Drive-by - Fix pre-existing clippy doc_lazy_continuation in librefang-types config blocking workspace clippy. * chore(deps): re-sync Cargo.lock with librefang-testing Cargo.toml cargo check refreshes the librefang-memory dep entry that an upstream commit left out of Cargo.lock. Trivial single-line drive-by while in this PR's worktree. * chore(workers): wire real REGISTRY_PUBLIC_KEY into wrangler.toml Generated via web/workers/keygen.mjs; private half deployed as Wrangler secret on both librefang-registry and librefang-marketplace workers. Public key is non-secret — committed in cleartext per design. * chore(workers): correct REGISTRY_PUBLIC_KEY to match deployed secret Earlier commit wired julGSb...; the live Cloudflare secret has since been rotated. Restore PR ↔ deployed-secret consistency. * chore(workers): finalise REGISTRY_PUBLIC_KEY (matches Bitwarden-backed secret) Replaces the prior placeholder/intermediate keys with the keypair backed up in Bitwarden and live as REGISTRY_PRIVATE_KEY on both workers. * feat(web): serve plugin registry pubkey at /.well-known/registry-pubkey The daemon's TOFU resolver (resolve_registry_pubkey) defaults to fetching https://librefang.ai/.well-known/registry-pubkey. Without this short-circuit the Pages SPA fallback returns index.html, which fails base64 validation, which makes plugin install hard-fail in fetch_verified_index. Pubkey is non-secret. Mirror of REGISTRY_PUBLIC_KEY in the two worker wrangler.toml files; rotation must update all three in lockstep. * feat(plugins): wire daemon to worker-signed registry index mirror The worker-side signing endpoints landed previously, but the daemon was still fetching the registry index from raw.githubusercontent.com — and the official registry repo has no committed `index.json` at all, so `install_plugin_with_deps` either 404'd or skipped verification entirely. Connect the chain end-to-end: * registry-worker now extracts `needs` and `version` from each plugin TOML during the cron refresh, and stores a daemon-shaped flat array (`[{name, version?, description?, needs?}]`, sorted by name for byte determinism) at `kv_store('plugins_index')`. The Ed25519 signature covers exactly those bytes and is stored at `plugins_index_sig`. `/api/registry/index.json[.sig]` now serve these — the dashboard's dict-shaped `/api/registry` is unchanged. The cron's signature-based short-circuit also bails out when the new KV row is missing so the first deploy after this change actually populates it. * daemon's `fetch_verified_index` defaults to the worker mirror (`https://stats.librefang.ai/api/registry/index.json`) when the registry is the official repo. Self-hosted forks keep the GitHub raw fallback. Both pairs accept `LIBREFANG_REGISTRY_INDEX_URL` / `LIBREFANG_REGISTRY_INDEX_SIG_URL` overrides for air-gapped deployments. URL selection is pulled into `registry_index_urls` with three new unit tests covering official / fork / env-override. Result: once the worker is redeployed and the cron has run once, `librefang plugin install <name>` for plugins with `[[needs]]` will verify the index against the published Ed25519 key — no longer relying on HTTPS + GitHub commits as the trust anchor. * fix(workers): lazy-build plugins_index from registry_data on first hit The signed-plugins-index endpoint was 503'ing on Workers Free because the new `kv_store('plugins_index')` row only gets populated by the cron path (02:00 UTC) — and forcing a refresh from a request handler overshoots the 50-subrequest budget for the GitHub manifest fetch. Pull the build path that's safe under the request budget into a small `ensurePluginsIndex` helper called from the two daemon-facing endpoints. It derives the flat `[{name, version?, description?, needs?}]` array from the existing `registry_data` KV row, sorts by name (matches the cron path's byte ordering so the signature contract stays consistent), signs it, and persists both the bytes and the signature. After that, the cron-path code remains the source of truth — `ensurePluginsIndex` is a one-shot bootstrap that no-ops once the row exists. Also drop the `?refresh=1`-bypasses-staleness shortcut from the previous commit: same 50-subrequest ceiling makes it useless from fetch handlers, and the cron already has the higher quota it needs. * fix(plugins): default OFFICIAL_PUBKEY_URL to the worker subdomain The Pages-worker pubkey route at librefang.ai/.well-known/registry-pubkey only goes live after the next main-branch deploy of web/public/_worker.js. Until then, requests fall through to the SPA index.html — which is_valid_registry_pubkey_b64 correctly rejects, but only after taking the network round trip and erroring out. Point the daemon directly at the worker's own subdomain (stats.librefang.ai/.well-known/registry-pubkey), which is the canonical source anyway and is updated in lockstep with the signed index served at the same host. The Pages-worker alias remains a stable fallback that operators can opt into via LIBREFANG_REGISTRY_PUBKEY_URL. * feat(workers): add /api/registry/refresh forced-rebuild endpoint Lets the librefang-registry GitHub Action push fresh content to the worker's D1 cache + signed plugins index immediately on every push to main, instead of waiting up to ~24h for the 02:00 UTC cron tick. Auth is a constant-time bearer-token check against the REGISTRY_REFRESH_TOKEN worker secret; until that secret is set the endpoint returns 503 and remains inert (no probing surface). Pre-emptively drops registry_data / plugins_index / plugins_index_sig before re-running refreshRegistryCache so a no-op commit (e.g. README typo) still forces a real rebuild — the existing signature-equality short-circuit would otherwise skip the new path entirely. Also purges the Cache-API row so dashboard reads see the new bytes immediately. * refactor(workers): forced refresh fetches in-repo plugins-index.json Pair with librefang/librefang-registry@f9821d5 — the daemon-shaped flat plugins index is now built inside the registry repo by scripts/build-plugins-index.mjs and committed as plugins-index.json at the repo root. handleForcedRefresh now does a single GitHub raw fetch for that one file, validates the JSON shape, and signs+stores it. Constant-cost refresh regardless of registry size keeps the path under the Workers Free 50-subrequest budget that the previous walking-Contents-API approach blew through. The dict-shaped /api/registry payload (dashboard) and its 02:00 UTC cron rebuild are unchanged — that lane stays on the slower path because dashboard tolerance for staleness is much higher. * feat(workers): forced refresh now ingests both daemon + dashboard indexes Pair with librefang/librefang-registry@ff6f3f2 — registry-index.json joins plugins-index.json as a checked-in artefact built by scripts/build-registry-index.mjs. handleForcedRefresh fetches both files in parallel (2 subrequests total, constant in registry size) and writes: plugins_index ← plugins-index.json (signed with Ed25519) registry_data ← registry-index.json (dict-shaped, dashboard) Plus purges the Cache-API entry that backs /api/registry so the next dashboard hit reads fresh bytes immediately instead of the 1h-cached previous payload. Result: dashboard updates land within seconds of a registry push, no longer waiting up to ~24h for the 02:00 UTC cron tick. The cron path is kept as a backstop for cases where the GitHub Action cannot reach the worker (network blips, CI outages). * fix(plugins): address PR review CRITICAL/HIGH security defects (1/3) Daemon-side fixes for the issues raised in code review of PR #4600: * CRITICAL #3 / MEDIUM #12 — install_from_registry was calling verify_archive_signature against {listing_url}.sig where listing_url is a GitHub Contents API URL. That .sig file never exists in the official registry layout, so the function silently returned Ok(()) on every install — meaning the entire Ed25519 archive lane was dead code that always passed. Replaced with index-membership check: refuse to install plugins not present in the signed plugins-index. The unused function is removed; the trust path now flows through the worker- signed flat index instead of a per-plugin .sig that never existed. * HIGH #5 / #16 — TOFU pubkey resolver was vulnerable to first-install MITM (cafe wifi, hostile DNS, subdomain takeover) silently pinning an attacker key forever. Added EMBEDDED_REGISTRY_PUBKEY compiled into the daemon binary as the primary trust root for the official registry. Resolution chain is now: env override > embedded > TOFU/HTTP (the latter two only consulted for self-hosted forks that opt in). * HIGH #6 — fetch_verified_index treated a missing or unreachable index.json.sig as a soft warning even when pubkey resolution was required. An attacker who could serve a doctored index but suppress the .sig (404 or network error) bypassed verification entirely. Now hard-fails for the official mirror; self-hosted forks keep the soft path so they can adopt signing incrementally. * MEDIUM #13 — TOFU cache file open was vulnerable to symlink attacks from compromised post-install hooks. Added O_NOFOLLOW + regular-file check on read, mode 0600 + O_NOFOLLOW on write (Unix). Windows path validates regular-file status and relies on NTFS ACLs. * MEDIUM #15 — added scripts/check-pubkey-lockstep.sh that fails when the pubkey constant drifts between the daemon's EMBEDDED_REGISTRY_PUBKEY and the three worker locations (registry-worker wrangler.toml, marketplace-worker wrangler.toml, _worker.js). Wire into CI before any cargo / wrangler build to catch silent rotation footguns. clippy + plugin_manager tests pass. * fix(plugins): address PR review CRITICAL/HIGH defects (2/3) Worker-side fixes paired with the daemon hardening in 1/3 and the in-repo signing in librefang-registry@74745f1. CRITICAL #1 — registry-worker is no longer a sign-anything oracle. handleForcedRefresh now fetches plugins-index.json + .sig from raw (committed by the registry repo's CI, which holds the private key) and stores both verbatim. The worker carries no private key and performs no signing — REGISTRY_REFRESH_TOKEN can no longer be used to coerce the worker into signing attacker-supplied bytes. CRITICAL #4 — ensurePluginsIndex deleted entirely. The lazy-build path produced bytes that were not byte-identical to what refreshRegistryCache produced (different empty-string handling), so any signature built by cron over bytes A and served as bytes B from lazy-build would fail daemon verification. refreshRegistryCache (~165-line Contents-API walker) deleted too — it had been silently dropping half the plugins under Workers Free's 50-subrequest budget. Cron now re-runs the same forced-refresh path (3 subrequests) as a daily backstop. signWithRegistryKey + b64FromBytes + bytesFromB64 deleted — no remaining caller. -169 net lines on the registry worker. CRITICAL #2 — marketplace-worker handlePublishVersion now refuses bundle_url not on an approved CDN prefix. Without this, an author could publish bundle_url=https://attacker.example/payload.tgz with a valid sha256 and get a registry-signed signature back. Also tightens bundle_sha256 to ^[0-9a-f]{64}$. MEDIUM #11 — constantTimeEqual padded to fixed length so timing leaks at most "they are not the same", not "yours is shorter than mine". Key rotation: new keypair generated as part of decoupling the worker from key custody. New pub: ClGa0Ucap8NdrKAy1rw9Tt6A9I8eg4zJ53+xIuKMuq0= in lockstep across daemon EMBEDDED_REGISTRY_PUBKEY + 3 worker locations. Private key now ONLY in librefang-registry's GitHub Actions store. Old worker-side keys are unused but kept (rotation one-way; deleting them is purely cleanup). Cloudflare routing: stats.librefang.ai routes only /api/* to the worker, so the daemon's HTTP rotation-probe path needed an /api/registry/pubkey alias (the /.well-known/ form only resolves on workers.dev). Daemon's OFFICIAL_PUBKEY_URL now points at the routed alias. 29 plugin_manager tests pass; clippy clean; both worker JS files node --check clean; end-to-end signature verifies against served pubkey for all 11 plugins. * fix(plugins): address PR re-review HIGH/MEDIUM/LOW defects Round-2 review of PR #4600 found 5 new HIGH defects introduced by the round-1 fixes plus 2 MEDIUM and 1 LOW. Code-only follow-ups here; the relocated CRITICAL oracle (workflow signing on push) is its own follow-up commit. HIGH-NEW-B — fetch_verified_index require_sig flag was keyed on registry slug == OFFICIAL_REGISTRY_REPO. An attacker who could change the configured registry (e.g. via a self-hosted-fork-style override) flipped require_sig false; the daemon then verified against the embedded official pubkey, fell through to the soft-warn branch when verification failed, and silently accepted unsigned bytes. Now require_sig is unconditional. Self-hosted forks must explicitly opt out via LIBREFANG_REGISTRY_VERIFY=0; no implicit downgrade path. HIGH-NEW-C — install_from_registry's index-membership check had an "Err(e) if checksum_verified => warn" arm that downgraded on any fetch error. Combined with the manifest-only SHA-256 check (which an attacker who controls the GitHub repo can forge), DoS of stats.librefang.ai bypassed verification entirely. Removed the checksum-rescue arm; index-fetch errors now hard-fail after a single 500ms-backoff retry to absorb transient blips. checksum_verified binding is gone (the SHA-256 step still runs and still hard-errors on mismatch, but doesn't gate any other check). HIGH-NEW-D — marketplace bundle_url allowlist matched on the raw input string post-URL.parse, so WHATWG normalization tricks (`https://github.com/../../attacker/x`) bypassed it. Reworked to match on (parsed.host, parsed.pathname) tuples and tightened the GitHub allowance to /releases/download/ only — the previous prefix also matched .../raw/main/... which is a mutable branch HEAD and defeats the immutability rationale. Also rejects URLs with hash fragments. HIGH-NEW-E — check-pubkey-lockstep.sh regex was loose enough to match a renamed LEGACY_REGISTRY_PUBLIC_KEY or a base64-shaped fragment in a comment. Anchored to start-of-line, length-limited the captured value to exactly 44 chars (Ed25519 raw 32-byte b64), forbade other quoted strings between the name and the value. MEDIUM-NEW-F — plugins-index.json.sig length check was `>= 86`, admitting arbitrarily long garbage. Tightened to exactly 86 or 88 chars and stricter regex. MEDIUM-NEW-G — Cache-API purge failure was silently swallowed by try/catch (_), so the worker reported success while the dashboard served stale bytes for up to 1h. Now surfaces cache_purged: bool (plus cache_purge_error when present) in the JSON response. LOW — EMBEDDED_REGISTRY_PUBKEY was a single &str, leaving no acceptance window for daemon ↔ worker rotations to overlap. Promoted to EMBEDDED_REGISTRY_PUBKEYS: &[&str]. Added verify_registry_index_multi which tries the resolved key first then falls back to other embedded slice entries (with a warn so ops sees they're on the prior key). The lockstep script extracts the active slot 0 only. 29 plugin_manager tests + clippy clean; both worker JS files node --check clean. * fix(plugins): expire prior embedded pubkeys per round-3 MEDIUM Round-3 PR re-review noted that EMBEDDED_REGISTRY_PUBKEYS as &[&str] left rotation-window keys accepted indefinitely — a future leak of a retired private key would still be exploitable against any daemon binary still carrying it. Promoted slice element to a struct: EmbeddedPubkey { b64, expires_at: Option<i64> } Slot 0 (active) keeps expires_at: None. Rotation procedure (per the updated doc-comment): when shipping a new active key, move the prior slot-0 entry to slot 1 with expires_at: Some(now + ~4 weeks). After the deprecation window passes, drop the prior entry in a follow-up release. verify_registry_index_multi now skips embedded entries whose expiry is in the past, with a debug log so ops can tell when daemons stop falling back. Active key path is unchanged. Lockstep script extended to handle the struct-field syntax (matches the first `b64: "..."` in the file, which is slot 0). Same 44-char length validation as before. 29 plugin_manager tests + clippy clean. * chore(codegen): auto-regenerate openapi.json + sdk + schema baselines [skip ci] * fix(plugins): close round-4 PR re-review follow-ups Round-4 reviewer flagged 4 hardening items (all MEDIUM/LOW; PR was already MERGEABLE per the verdict). Closing the three code-side ones here; the branch-protection MEDIUM is documented as an intentional trade-off for the single-maintainer + auto-publish flow. LOW round 4 — `EmbeddedPubkey.b64` field renamed to `pubkey_b64`. The old generic name made the lockstep regex's "find any 44-char base64 in a `b64: \"...\"` field" rule fragile against unrelated future fields with the same shape. The unique name is grep-anchored and disambiguates intent. Lockstep script updated in lockstep. LOW round 4 — added `embedded_pubkeys_slot0_has_no_expiry` test + `debug_assert!` in resolve_registry_pubkey. A maintainer who absent- mindedly sets `expires_at: Some(...)` on slot 0 during a rotation edit would silently break installs the moment that timestamp passed. The runtime assert catches it pre-ship; the test catches it in CI. MEDIUM round 4 — `verify_registry_index_multi` dedup now compares against `resolved_pubkey.trim()` rather than raw. All current call sites pre-trim, but a future code path that forgets would otherwise verify the same key twice (wasted CPU on one extra Ed25519 verify; not unsafe). Cheap to harden, defensive against future drift. MEDIUM round 4 — when slot-0 verification fails AND every prior embedded pubkey is past expiry, the returned error now appends "(N prior embedded pubkey(s) past expiry — this daemon binary is past its rotation window; upgrade librefang to restore plugin installs)". Gives ops an actionable next step instead of a bare "Signature verification failed" mystery. MEDIUM round 4 (branch protection) — kept as documented trade-off: `bypass_pull_request_allowances.users:["houko"]` allows the single registry maintainer to direct-push to main, defeating CODEOWNERS for that one identity. `enforce_admins:false` similarly allows admin direct-push. Removing the bypass would force the maintainer through a self-approval PR cycle for every plugin commit — and the CI bot (`github-actions[bot]`, identity `apps:[]` per the API since the github-actions app slug isn't bypass-eligible) would also be blocked from `git push`, breaking the auto-publish UX. Multi-maintainer project would justify the strict stance; single-maintainer doesn't. 30 plugin_manager tests + clippy clean. * style(plugins): apply cargo fmt to round-4 follow-up changes The round-4 follow-up commit (9aca54a) shipped without running `cargo fmt` locally — CI Quality job's `Check formatting` step caught it. Reformatted plugin_manager.rs against the workspace rustfmt config; no semantic change. --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
This was referenced May 9, 2026
houko
added a commit
that referenced
this pull request
May 11, 2026
… (#4909) Adds `workflow_id: Option<String>` to `Trigger` and `TriggerPatch`, letting event-driven triggers dispatch a workflow run instead of sending a prompt to an agent via `send_message_full`. Changes: - `Trigger.workflow_id` field (`#[serde(default)]` — old persisted triggers load cleanly as `None`; no migration needed) - `TriggerPatch.workflow_id: Option<Option<String>>` mirrors the `target_agent` patch semantics (`Some(None)` clears) - `TriggerMatch` gains `workflow_id` and `trigger_id` fields so the dispatch loop can fork without an extra registry lookup - `publish_event_inner` dispatch fork: when `workflow_id` is set, resolve by UUID then by name, call `run_workflow`, emit structured telemetry; log `warn!` on lookup failure without falling back to agent path - `KernelApi::register_trigger_with_target` and `TriggerEngine:: register_with_target` gain `workflow_id: Option<String>` param - `routes/workflows.rs`: `create_trigger` and `update_trigger` parse `workflow_id` with the same absent/null/string semantics as `target_agent_id`; `trigger_to_json` round-trips the field; validation rejects empty strings and strings > 256 chars (mirrors `MAX_WORKFLOW_ID_LEN` from cron `CronAction::Workflow`, which was introduced in #1102) - `crates/librefang-api/tests/trigger_workflow_test.rs`: 6 new tests covering end-to-end dispatch, regression, API round-trip, PATCH clear, length validation, and legacy JSON compatibility Cron already had this path via `CronAction::Workflow` since #1102. This PR closes gap #6 (trigger half) from #4844.
houko
added a commit
that referenced
this pull request
May 15, 2026
…rigger concurrency Two #5033 review blockers in one commit since both live in `task_registry.rs` and the fixes are tightly coupled: (3) `register_async_task` minted a fresh `TaskId` on every call. If `workflow_start` returned the same `run_id` for two registrations (engine-dependent behaviour), the second entry silently orphaned at completion time — only one would receive the `TaskCompletionEvent`. Idempotency-of-completion did not extend to idempotency-of-registration. Fix: scan the registry on insert for a matching live entry — same `run_id` for workflows, same `(agent_id, prompt_hash)` for delegations — and return the existing handle if present. Different prompt hashes for the same target still mint distinct ids. New integration tests pin the dedupe contract and the negative case. (4) `spawn_wake_idle_turn` did a bare `tokio::spawn` followed by `send_message_full`, bypassing both `Lane::Trigger` and the per-agent `max_concurrent_invocations` semaphore that the trigger dispatcher acquires. A batch agent with `max_concurrent_invocations = 1` that fired 50 `workflow_start` calls would, on completion, get 50 wake-idle turns running in parallel — silently violating the operator-set cap. Fix: acquire `Lane::Trigger` (global) and the per-agent semaphore (resolved via the existing `agent_concurrency_for` helper) before calling `send_message_full`. Same two acquisitions the trigger dispatcher already performs; wake-idle is no longer a fourth unnamed concurrency dimension. Nit #6: `KernelApi::injection_senders_ref` and `pending_async_task_count` are test introspection — marked `#[doc(hidden)]` so dashboard / route handlers cannot stumble onto them via rustdoc. Refs #4983. Refs #5033 review.
7 tasks
houko
added a commit
that referenced
this pull request
May 19, 2026
* fix(api): preserve operator-hand-edited non-schema env keys on sidecar save upsert_sidecar_block previously rewrote the entire [sidecar_channels.env] table on every save, silently dropping operator hand-edits such as PYTHONPATH=/custom, HTTP_PROXY, locale settings, and even legacy inline TELEGRAM_BOT_TOKEN entries. Make the form's ownership of the env table explicit: callers now pass managed_env_keys — the set of non-secret schema field keys the configure form is the source of truth for. Within env: - A managed key with a non-empty form value overwrites the existing entry. - A managed key absent / empty in the form is removed. - Every other key in the existing env table is preserved untouched. Secret-typed schema fields never appear in config.toml (they live in secrets.env), so they are intentionally excluded from the managed set. Add coverage in sidecar_toml_test: - preserves_non_schema_env_keys_on_replace - removes_schema_managed_env_keys_when_form_clears_them Total in this test file: 4 → 8. * fix(api): use kernel.home_dir() in all channel handlers (codex fix #1 generalize) The original codex fix #1 only converted configure_sidecar_channel. Five sibling handlers in the same file kept resolving via the local librefang_home() helper (LIBREFANG_HOME env + ~/.librefang fallback), bypassing the kernel's authoritative KernelConfig.home_dir in any case where the two diverge: - configure_channel - remove_channel - create_channel_instance - update_channel_instance_handler - delete_channel_instance Replace every site with state.kernel.home_dir().to_path_buf() so they honour the same single source of truth as the rest of the HTTP surface. Delete the now-unused private librefang_home() helper (the unrelated cross-crate helper in routes/system.rs is untouched and still serves agent_templates.rs). The five channel-instance integration tests that previously seeded a config.toml into DiskHomeGuard's standalone tempdir now seed into state.kernel.home_dir() — that's the authoritative path the handlers read. DiskHomeGuard is retained as an RAII guard that keeps LIBREFANG_HOME pointed at a benign tempdir so any code still consulting the env var doesn't escape into the developer's real ~/.librefang. write_discord_instances also pins config_version to types::CONFIG_VERSION so the kernel's load_config migration path doesn't clobber the minimal fixture on first read. * fix(api): kill_on_drop on skills subprocess (codex fix #3 generalize) skills.rs auto-install command spawned via tokio::process::Command inside a tokio::time::timeout(300s) had no kill_on_drop(true), so a timeout (or any dropped Future) would leave the install command running detached in the background. Same defect class as the sidecar --describe subprocess in codex review fix #3, which already added kill_on_drop there. Add .kill_on_drop(true) before .output() so the child is SIGKILLed when the timeout future is dropped. Sweep of all Command::new in crates/librefang-api/src/routes/ confirms this was the only remaining async tokio::process::Command paired with a timeout. The std::process::Command calls in config.rs (ps / tasklist / hostname) and system.rs (hostname) are synchronous, run unguarded with no timeout, and complete in milliseconds — outside the kill_on_drop defect class. * fix(channels,api): accept quoted values in secrets.env parser (consistency with dotenv) librefang_channels::sidecar::parse_secrets_env previously did a naive find('=') with no quote stripping, so an operator hand-editing secrets.env with the common dotenv convention KEY="value" got the literal quote characters propagated into the sidecar child's environment. Tighten the contract: surrounding whitespace is trimmed, and a matched pair of outer ASCII single or double quotes on the value is stripped (mirroring dotenv conventions). Mismatched / one-sided quotes are left verbatim so a value that legitimately contains a single quote isn't silently corrupted. Escape sequences (\n etc.) are explicitly out of scope — documented in the function doc-comment. Add unit coverage in librefang-channels: - parse_secrets_env_strips_double_quotes - parse_secrets_env_strips_single_quotes - parse_secrets_env_keeps_internal_quotes - parse_secrets_env_keeps_mismatched_outer_quotes - parse_secrets_env_trims_whitespace - parse_secrets_env_handles_empty_quoted_value - parse_secrets_env_skips_comments_and_blanks The inline secrets.env reader in routes/channels.rs::configure_sidecar_channel extracts KEYS only (set membership check against schema's secret field names) — quotes never appear inside dotenv KEYS, so no value-side stripping is needed there. Add a leading comment pointing future readers at the channels-crate helper in case the shadow-detection ever grows a value comparison. * test(api): cover include-shadow detection (positive + commented false-positive) configure_sidecar_refuses_when_include_owns_sidecars already covers the positive case (an include with a real [[sidecar_channels]] entry triggers 409). Add configure_sidecar_refuses_even_on_commented_sidecar_string to document the substring-match heuristic's conservative false-positive surface: an included file that only mentions [[sidecar_channels]] inside a TOML comment still triggers 409. The test pins the behaviour so a future refactor that switches to a full TOML parse of included files makes a visible decision rather than silently relaxing the check. Total channels_routes_test: 37 → 38. * style(api): drop redundant deref in upsert_sidecar_block (clippy) `env_table.remove(*key)` triggered clippy::explicit-auto-deref under `-D warnings` (Rust 1.95). `Table::remove` accepts `&str` and Rust autoderefs `&&str` for us — same effect, no clippy fight. Spotted in the workspace clippy pass after fix #6.
houko
pushed a commit
that referenced
this pull request
May 28, 2026
…eshold, LLM confidence, KV mirror retirement, instrumented spawn, fuzzy categories, configurable prompt cap, CHANGELOG, comment fix 10 follow-ups raised on the code review of the prior two commits in this PR. All within the same memory-system scope. * #1 root user_id is now a constant sentinel UUID (00000000-0000-0000-0000-72006f0074a0, exported as `ROOT_API_KEY_USER_ID`) rather than `UserId::from_name("root")`. The from_name UUIDv5 lives inside `LIBREFANG_USER_NAMESPACE`, so an operator-registered `[users] name = "root"` would have silently inherited the master credential's ACL + per-user budget cap. The sentinel falls outside that namespace; AuthManager returns None for it and the fail-open Owner-default ACL applies. Regression test `root_api_key_user_id_does_not_collide_with_any_named_user` pins the non-collision invariant against {root, admin, owner, system, operator, user}. * #3 `HotAction::UpdateProactiveMemory` now also calls `substrate.set_consolidation_duplicate_threshold(...)`, so when `POST /api/config/reload` swaps in a new `[proactive_memory] duplicate_threshold`, the periodic global consolidation sweep picks it up alongside the per-agent on-demand consolidate. Without this the per-agent path picked up the new value but the global sweep stayed on the old one — exactly the inconsistency H5 set out to remove. `ConsolidationEngine` switched to an `Arc<AtomicU32>` threshold (f32 bits) so the setter takes `&self`, which is required because the hot-reload code path holds only `Arc<MemorySubstrate>`. `docs/operations/config-reload.md` row updated to call out the new behaviour. * #4 `build_extraction_prompt` now asks the LLM to emit a per-memory `confidence` field with a brief calibration guide; the parser reads it, clamps to [0, 1], and stashes the value in `metadata["confidence"]` and `MemoryItem.confidence` so the C3 insert path actually lands a non-default value in the `confidence` column. Missing field still defaults to 1.0 (matches the rule-based extractor's prior behaviour — never silently drops a memory). Tests `parse_extraction_propagates_confidence_to_metadata` and `parse_extraction_clamps_confidence_to_unit_interval` pin the new behaviour. * #5 The KV `memory:*` mirror is gone — fully retired, not just "non-load-bearing". All `structured.set("memory:*", ...)` / `structured.delete("memory:*", ...)` / `list_kv` scans that walked the mirror have been deleted from `import_memories`, `add_with_decision`'s ADD + UPDATE branches, `add_with_level`, `delete`, `update`, `reset`, `clear_level`, `cleanup_expired_sessions`, the eviction loop, and the consolidation merge-loser path. The read path was already on semantic (C1); leaving the writes in place would have grown the mirror without bound and risked future divergence regressions. `test_delete_memory` rewritten to assert behaviour through `search()` (the trait-level contract) instead of probing the underlying KV store. Any legacy `memory:*` entries from older installs are silently ignored. * #6 The detached auto-consolidate `tokio::spawn` is wrapped in a `tracing::info_span!("auto_consolidate", task = "auto_consolidate", agent = ...)` via `.instrument(span)`, so a panic inside the consolidate future surfaces in tracing output (instead of disappearing silently the way bare-spawn panics do) and operators can grep `task = "auto_consolidate"` to find the detached work. * #7 Category allowlist match is now case-insensitive + tolerant of trailing `s` on either side. `"Preferences"` / `"PREFERENCE"` / `"preferences"` all snap to a configured `"preference"` and the canonical configured spelling lands in the column. Test `parse_extraction_fuzzy_matches_category_case_and_plural` pins the four variants. * #8 `format_context_max_chars` is now a field on `ProactiveMemoryConfig` (default 8000 chars / ~2000 tokens); the store's `format_context_with_query` / `format_context` read it from the live config and pass it to `format_memories_with_budget(memories, max_chars)`. The trait fallback (`DefaultMemoryExtractor::format_context`, `LlmMemoryExtractor::format_context`) keeps the const default for callers without config access. Operators on 200k+ context windows can now raise the cap via `config.toml` without recompiling. * #2 + #9 New `Fixed` entry in `[Unreleased]` documents the breaking audit-shape change (root-api_key requests now stamp a `user_id` where they previously stamped `None`) and the `add()` behaviour change (no raw-transcript fallback for extraction misses). Both were noted inline in commits but absent from the CHANGELOG. The entry also lists the full audit-sweep scope so an operator can size-up the upgrade impact from one place. * #10 `import_memories` comment rewritten to explain the 0.95 threshold without the confusing "stricter than extraction-time dedup" framing. Drive-by: collapsed three more `clippy::manual_option_zip` sites in `kernel/tests.rs` that the workspace-clippy gate would have caught on the next CI run. Auto-restamped `.secrets.baseline` line numbers shifted by the CHANGELOG insert. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-runtime -p librefang-types -p librefang-api -p librefang-kernel --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 269 passed * cargo test -p librefang-runtime --lib proactive_memory — 60 passed (5 new follow-up regression tests included) * cargo test -p librefang-api --lib --test memory_routes_integration --test agent_kv_authz_integration --test auth_public_allowlist — all green
houko
added a commit
that referenced
this pull request
May 29, 2026
…cay, dedup, prompt budget, async consolidate) (#5839) * fix(memory): split-brain reads, raw-transcript fallback, RBAC on writes, forget() leak, immortal decay 5 CRITICAL findings from a memory-system audit, all in the proactive memory layer: * C1 split-brain: list()/get() read from the KV mirror while search()/auto_retrieve read from the semantic store. The KV write was best-effort (warn-and-continue) so a failure left rows visible to search but invisible to list. retrieve_memory_items now reads from semantic (the authoritative source); KV writes stay as a non-load- bearing compatibility mirror. * C2 raw-transcript fallback: when the extractor returned no signal, add() stored the verbatim concatenated message content as a session-level memory with no category. This was the dominant source of `category=null` rows and duplicate transcripts on the dashboard. The fallback is removed; callers that want raw content use add_with_level explicitly. * C3 confidence hardcoded to 1.0 + immortal decay: remember_with_embedding_and_peer now honors metadata["confidence"] (clamped to 0..=1, defaulting to 1.0) so the LLM extractor's signal reaches the column. decay_confidence reworked: boost divides the rate instead of multiplying the result and clamping to 1.0. The old formula made any memory with >=2 accesses freeze at confidence 1.0 forever; the new formula keeps "popular memories decay slower" but strictly monotonic (boost capped at MAX_BOOST=4.0). * C4 RBAC on write endpoints: memory_add, memory_update, memory_delete, memory_bulk_delete, memory_reset_agent, memory_clear_level, memory_consolidate, memory_cleanup, memory_export_agent, memory_import_agent, memory_decay, memory_store_relations now route through the namespace guard. New ProactiveMemoryStore wrappers cover the previously-unguarded ops (reset, clear_level, export_all, import_memories, decay_confidence). The root api_key is attributed as an Owner- equivalent AuthenticatedApiUser in middleware so operators using only the master credential keep their POST/PUT/DELETE access. * C5 forget() never marked deleted_at: SemanticStore::forget* now stamps deleted_at alongside deleted=1 so the prune_soft_deleted_memories sweep (filter `deleted_at IS NOT NULL`) can actually hard-delete user-/API-initiated deletions. Without the stamp every soft-deleted row leaked its embedding BLOB forever. consolidation.rs's merge-loser delete now stamps it too. Drive-by: removed a clippy::manual_option_zip in kernel/background_lifecycle.rs flagged while clippy-gating the change. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-api --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 269 passed (5 pre-existing tests adapted to the new add()-no-fallback semantics; new regression tests for C1 list-from-semantic, C2 no-fallback, C3 monotonic-decay + extractor-confidence-roundtrip, C4 viewer-denied on every write wrapper, C5 forget* stamps deleted_at) * cargo test -p librefang-api --lib --test memory_routes_integration --test agent_kv_authz_integration --test auth_public_allowlist — all green * chore(codegen): auto-regenerate openapi.json + sdk + schema baselines [skip ci] * fix(memory): tighten dedup thresholds, validate LLM extraction, cap prompt budget, detach auto-consolidate, unify consolidation knob Follow-up sweep on the same memory-system audit as the prior commit — picks off the HIGH findings that were in scope for the same crate / file cluster. * H1 duplicate_threshold tightened. The configured default jumps from 0.5 → 0.85 (mem0's recommended near-duplicate cut-off); both metrics — cosine and Jaccard — agree that 0.5 means "topically related", which let opposite-meaning sentences sharing keywords silently merge. `DefaultMemoryExtractor::decide_action`'s hardcoded same-category 0.5 / cross-category 0.6 UPDATE thresholds rise to 0.7 / 0.8; the 0.95 NOOP gate stays. `import_memories`' hardcoded 0.9 dedup floor rises to 0.95 with a docstring explaining why bulk-import is stricter than extraction-time dedup. * H2 LLM-extraction validation. `parse_llm_extraction_response` now enforces a 4-char content floor (drops "ok" / "no" / single-letter junk that was trivially unique and survived dedup), validates the emitted `category` against the configured `extract_categories` allowlist (out-of-allowlist values downgrade to "general" instead of polluting the dashboard's facets), and caps a single extraction call at MAX_MEMORIES_PER_EXTRACTION=20 rows so a runaway model can't churn the eviction loop. * H4 prompt-injection budget. `format_context` now goes through a shared `format_memories_with_budget` helper capped at FORMAT_CONTEXT_MAX_CHARS=8000 (~2000 tokens). Pre-fix the formatter concatenated everything with no ceiling — 10 retrieved memories × 2000-char MAX_MEMORY_CONTENT_LENGTH could push 20 KB into every request. Excess rows are reported via a "[+N additional memories omitted to keep the prompt within budget]" footer so the truncation is observable in the rendered prompt rather than silent. * H6 auto-consolidation no longer blocks the agent. The every-10 trigger in `auto_memorize` now `tokio::spawn`s the consolidate call instead of awaiting it inline; the next agent turn doesn't pay for the O(n²) merge pass plus its SQLite transaction. The detached future borrows nothing from `self` thanks to `ProactiveMemoryStore`'s manual Clone over Arc'd inner state. * H5 single source of truth for the consolidation threshold. `ConsolidationEngine` gains a `duplicate_threshold` field (defaulting to 0.85 to match the new config default) with a `set_duplicate_threshold` setter; `MemorySubstrate` exposes a passthrough; kernel boot pushes `config.proactive_memory. duplicate_threshold` down to the engine. The periodic global consolidation sweep and the on-demand `ProactiveMemoryStore::consolidate` now agree on what counts as a near-duplicate. Keeps the existing 142 callers of `MemorySubstrate::open_in_memory(decay_rate)` source-compatible (no signature break). H7 / H8 from the same audit were already addressed by the C3 fix in the previous commit (popular-memory immortality + dead extraction_threshold). H3 (memory_store/recall vs auto_memorize/ retrieve being disconnected tool surfaces) is intentional product shape rather than a bug — left out. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-runtime -p librefang-types -p librefang-api --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 269 passed * cargo test -p librefang-runtime --lib (proactive_memory) — 57 passed, including the 5 new regressions (parse_extraction_drops_sub_minimum_content, parse_extraction_downgrades_unknown_category, parse_extraction_preserves_category_when_allowlist_empty, parse_extraction_caps_total_memories_per_call, format_context_caps_prompt_budget_with_truncation_marker). * cargo test -p librefang-api --test memory_routes_integration --test agent_kv_authz_integration --test auth_public_allowlist — all green. * fix(memory): review-followups — sentinel root user_id, hot-reload threshold, LLM confidence, KV mirror retirement, instrumented spawn, fuzzy categories, configurable prompt cap, CHANGELOG, comment fix 10 follow-ups raised on the code review of the prior two commits in this PR. All within the same memory-system scope. * #1 root user_id is now a constant sentinel UUID (00000000-0000-0000-0000-72006f0074a0, exported as `ROOT_API_KEY_USER_ID`) rather than `UserId::from_name("root")`. The from_name UUIDv5 lives inside `LIBREFANG_USER_NAMESPACE`, so an operator-registered `[users] name = "root"` would have silently inherited the master credential's ACL + per-user budget cap. The sentinel falls outside that namespace; AuthManager returns None for it and the fail-open Owner-default ACL applies. Regression test `root_api_key_user_id_does_not_collide_with_any_named_user` pins the non-collision invariant against {root, admin, owner, system, operator, user}. * #3 `HotAction::UpdateProactiveMemory` now also calls `substrate.set_consolidation_duplicate_threshold(...)`, so when `POST /api/config/reload` swaps in a new `[proactive_memory] duplicate_threshold`, the periodic global consolidation sweep picks it up alongside the per-agent on-demand consolidate. Without this the per-agent path picked up the new value but the global sweep stayed on the old one — exactly the inconsistency H5 set out to remove. `ConsolidationEngine` switched to an `Arc<AtomicU32>` threshold (f32 bits) so the setter takes `&self`, which is required because the hot-reload code path holds only `Arc<MemorySubstrate>`. `docs/operations/config-reload.md` row updated to call out the new behaviour. * #4 `build_extraction_prompt` now asks the LLM to emit a per-memory `confidence` field with a brief calibration guide; the parser reads it, clamps to [0, 1], and stashes the value in `metadata["confidence"]` and `MemoryItem.confidence` so the C3 insert path actually lands a non-default value in the `confidence` column. Missing field still defaults to 1.0 (matches the rule-based extractor's prior behaviour — never silently drops a memory). Tests `parse_extraction_propagates_confidence_to_metadata` and `parse_extraction_clamps_confidence_to_unit_interval` pin the new behaviour. * #5 The KV `memory:*` mirror is gone — fully retired, not just "non-load-bearing". All `structured.set("memory:*", ...)` / `structured.delete("memory:*", ...)` / `list_kv` scans that walked the mirror have been deleted from `import_memories`, `add_with_decision`'s ADD + UPDATE branches, `add_with_level`, `delete`, `update`, `reset`, `clear_level`, `cleanup_expired_sessions`, the eviction loop, and the consolidation merge-loser path. The read path was already on semantic (C1); leaving the writes in place would have grown the mirror without bound and risked future divergence regressions. `test_delete_memory` rewritten to assert behaviour through `search()` (the trait-level contract) instead of probing the underlying KV store. Any legacy `memory:*` entries from older installs are silently ignored. * #6 The detached auto-consolidate `tokio::spawn` is wrapped in a `tracing::info_span!("auto_consolidate", task = "auto_consolidate", agent = ...)` via `.instrument(span)`, so a panic inside the consolidate future surfaces in tracing output (instead of disappearing silently the way bare-spawn panics do) and operators can grep `task = "auto_consolidate"` to find the detached work. * #7 Category allowlist match is now case-insensitive + tolerant of trailing `s` on either side. `"Preferences"` / `"PREFERENCE"` / `"preferences"` all snap to a configured `"preference"` and the canonical configured spelling lands in the column. Test `parse_extraction_fuzzy_matches_category_case_and_plural` pins the four variants. * #8 `format_context_max_chars` is now a field on `ProactiveMemoryConfig` (default 8000 chars / ~2000 tokens); the store's `format_context_with_query` / `format_context` read it from the live config and pass it to `format_memories_with_budget(memories, max_chars)`. The trait fallback (`DefaultMemoryExtractor::format_context`, `LlmMemoryExtractor::format_context`) keeps the const default for callers without config access. Operators on 200k+ context windows can now raise the cap via `config.toml` without recompiling. * #2 + #9 New `Fixed` entry in `[Unreleased]` documents the breaking audit-shape change (root-api_key requests now stamp a `user_id` where they previously stamped `None`) and the `add()` behaviour change (no raw-transcript fallback for extraction misses). Both were noted inline in commits but absent from the CHANGELOG. The entry also lists the full audit-sweep scope so an operator can size-up the upgrade impact from one place. * #10 `import_memories` comment rewritten to explain the 0.95 threshold without the confusing "stricter than extraction-time dedup" framing. Drive-by: collapsed three more `clippy::manual_option_zip` sites in `kernel/tests.rs` that the workspace-clippy gate would have caught on the next CI run. Auto-restamped `.secrets.baseline` line numbers shifted by the CHANGELOG insert. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-runtime -p librefang-types -p librefang-api -p librefang-kernel --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 269 passed * cargo test -p librefang-runtime --lib proactive_memory — 60 passed (5 new follow-up regression tests included) * cargo test -p librefang-api --lib --test memory_routes_integration --test agent_kv_authz_integration --test auth_public_allowlist — all green * fix(api): attribute Owner on no-auth loopback so memory writes aren't 403 The RBAC gating added 12 memory-write ACL checks, but the default `librefang start` (no api_key, loopback bind) takes the no-auth bypass which returned next.run() WITHOUT attaching an AuthenticatedApiUser. Memory write handlers then saw None -> anonymous Viewer fallback -> 403 on every POST/PUT/DELETE /api/memory*, breaking the documented default workflow. No-auth + trusted origin (loopback / LIBREFANG_ALLOW_NO_AUTH) is the same trust level as the root master credential, so attribute the same Owner-equivalent user (ROOT_API_KEY_USER_ID). Non-loopback still fails closed. Add integration tests for both: loopback write != 403, non-loopback no-auth still 401. * fix(memory): read-only recall for listing paths; correct decay doc MEDIUM (#5839): list/get/export/list_all read paths called recall(), which unconditionally bumps access_count + accessed_at. A dashboard polling the memory list would perpetually reset accessed_at = now and inflate access_count — the exact signals the C3 decay logic keys idle/popularity off — so polled listings could keep memories from ever decaying (and turned a GET into a 10k-row write). Add recall_readonly() (shared impl, no bump) and route the four listing/export reads through it; genuine semantic recalls still track access. Regression test asserts recall_readonly leaves access_count untouched while recall() bumps by 1. Also correct the decay_confidence doc: the once-per-hour cadence is enforced by the periodic maintenance scheduler, not an internal throttle (a direct call decays immediately). --------- Co-authored-by: Evan <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
houko
pushed a commit
that referenced
this pull request
May 29, 2026
…loor, partial-status, dedup-strip, test tightening 8 follow-ups from the code review of the prior commit. All in scope for the same proactive-memory layer. * #1 misattributed doc-comment in `proactive.rs` near `AUTO_CONSOLIDATE_EVERY` / `NEGATION_WORDS`. The `/// Negation/contradiction words …` line was orphaned above `AUTO_CONSOLIDATE_EVERY` when the new const got inserted; both consts now carry their own intended docstring. * #2 lowered `STALE_COUNTER_FLOOR` from `AUTO_CONSOLIDATE_EVERY / 2` (5) to `/ 4` (2). The /2 floor cleaned up "stuck at 1..4" agents but also reset slow-burn agents (single auto_memorize per maintenance tick) before they could climb to 10, so a steady 1-call/hour stream effectively never consolidated. /4 keeps the cold-slot eviction directional while letting low-frequency agents still accumulate to the trigger. * #3 `PATCH /api/memory/config` response shape is now explicit about partial success. `body.status` is `"applied"` when the reload succeeded and `"partial"` when the disk write landed but the live reload failed (e.g. operator hand-edited an unrelated section into an invalid shape between PATCH writes). Clients MUST inspect `status`; the HTTP code stays 200 for both branches since the disk write itself succeeded. 207 / 500 were considered and rejected — 500 misrepresents that the request was rejected (it wasn't) and 207 forces every existing client to re-classify success. Mirrors the `import_agent_memory` partial-success pattern. * #4 documented the M13 cost tradeoff. Up to 4 SQLite roundtrips per insertion on the no-embedding fallback path (versus the pre-fix 1), but the embedding-driver path is unaffected and the loop short-circuits as soon as the union hits fetch_limit, so the common case ("first keyword filled the slate") still runs a single query. Operators on the no-embedding path pay the cost on writes only, against an already-unindexed `content LIKE` scan. * #5 defensively strip the M14 `_update_threshold_*` keys from the enriched item right after `decide_action` returns + assert callers don't pre-populate them. Both insertion branches today build their stored metadata from the original `item.metadata` (not `enriched_item.metadata`), so the threshold keys never reach the column — but stripping + asserting is cheap insurance for the next refactorer who repoints either branch at the enriched copy. * #6 relaxed the `extract_search_keywords_returns_multiple_ordered_longest_first` test: no more exact `kws.len() == 4` — the cap and the "longest distinctive word survives" invariants are what we care about, not the exact count. Future additions to `STOP_WORDS` no longer silently break the test. * #7 dropped the unused `_update_threshold_cross_cat` set in `decide_action_honors_config_update_thresholds`. Both candidate memories share the "preference" category, so the cross-cat threshold branch was unreachable. * #8 moved `STALE_COUNTER_FLOOR` from a `fn`-scope `const` to module scope, alongside `AUTO_CONSOLIDATE_EVERY` from which it derives. Matches the repo convention and lets future readers see the floor / trigger relationship at one site. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-runtime -p librefang-types -p librefang-api --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 273 passed * cargo test -p librefang-api --test memory_routes_integration — 14 passed
houko
pushed a commit
that referenced
this pull request
May 29, 2026
…epts partial, M14 strips _embedding too, prune rate-limit, chrono::Duration const portability 6 followups from the third review pass on #5850. * #1 M12 test pinned the contract properly. `serde_json::Value` indexed by a missing key returns `Value::Null`, so the prior `assert_eq!(body["reload_error"], Null)` silently passed even if the field had been removed. Now asserts `body.as_object() .contains_key(...)` for `status`, `restart_required`, `reload_error` first, then asserts their values. * #2 strip + test for the `_embedding` private-stash key. `add_with_decision` now also calls `enriched_item.metadata.remove("_embedding")` after `decide_action` returns, so all three private stash keys (`_update_threshold_*` + `_embedding`) get the same defensive treatment. Test renamed to `add_with_decision_does_not_leak_private_stash_keys_to_stored_metadata` and attaches a tiny mock `EmbeddingFn` so the `_embedding` stash path actually fires — the prior assertion against `_embedding` was decorative because the test had no embedding driver configured. * #3 rate-limited the counter prune via a new `last_counter_prune: Arc<Mutex<Option<DateTime<Utc>>>>` and a `maybe_prune_counters` helper that mirrors the `maybe_decay_confidence` / `maybe_cleanup_expired` once-per-hour pattern. Prior to this the prune ran on every `maybe_run_maintenance` call, reachable from `search` / `auto_retrieve` / `consolidate` at potentially many Hz. The retain itself was microseconds, so this is a wash today, but it brings the three maintenance sub-tasks under the same scheduling budget for future scaling. * #4 docstring on `STALE_COUNTER_IDLE_WINDOW_HOURS` re-phrased to describe the slot-keeping guarantee in terms of the maintenance rate-limit window, not "consecutive prune passes" — the old phrasing happened to be true only because the prune wasn't rate-limited yet (now rectified by #3). * #5 M12 test now accepts either `"applied"` or `"partial"` as the body status — both are valid post-fix outcomes; the pre-fix contract had no `status` field at all. The status field's presence (asserted via the #1 fix) is the actual contract we're pinning. Made the test robust to future `KernelConfig::default()` changes that might cause the seeded toml to fail reload validation. * #6 swapped `const STALE_COUNTER_IDLE_WINDOW: chrono::Duration = chrono::Duration::hours(2)` for `const STALE_COUNTER_IDLE_WINDOW_HOURS: i64 = 2` plus `chrono::Duration::hours(STALE_COUNTER_IDLE_WINDOW_HOURS)` at the call site. `chrono::Duration::hours` is a `const fn` at the currently pinned `chrono` minor but the const-ness isn't a stable contract across `0.4.x` versions, so a lockfile bump could silently break the build. The integer-hours + runtime conversion stays valid regardless. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-api --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 274 passed (incl. `add_with_decision_does_not_leak_private_stash_keys_to_stored_metadata` with embedding-driver coverage) * cargo test -p librefang-api --test memory_routes_integration — 15 passed (incl. tighter `patch_memory_config_hot_reloads_and_reports_applied` contract assertions)
houko
added a commit
that referenced
this pull request
May 29, 2026
…CH, multi-keyword search, configurable UPDATE thresholds (#5850) * fix(memory): MEDIUM follow-ups — counter map sweep, hot-reload on PATCH, multi-keyword search, configurable UPDATE thresholds Continuation of the audit sweep on the proactive-memory subsystem. 4 MEDIUM findings from the same audit, all in scope for this PR. * M11 `consolidation_counters` HashMap is now actively pruned every maintenance tick. Pre-fix it only swept when the map crossed 1000 entries (truncate-to-500 by count DESC), which delayed cleanup until the leak was observable AND deleted the highest-count entries — exactly the agents about to fire a real consolidate. The new sweep drops every counter that hasn't passed the halfway mark (`< AUTO_CONSOLIDATE_EVERY / 2 = 5`) on each maintenance tick: HashMap::retain in-place, evicts cold entries first, never touches an entry that's about to fire. Added named constant `AUTO_CONSOLIDATE_EVERY = 10` so the trigger + the prune floor stay in lockstep. * M12 `PATCH /api/memory/config` now calls `kernel.reload_config()` after writing `config.toml`, so dashboard saves take effect on the running kernel instead of staying disk-only until restart. Pre-fix the response always reported `restart_required: true`, which confused operators who could see GET return the new values while live behaviour (ProactiveMemoryStore::config, decay engine, etc.) stayed on the boot snapshot. `restart_required` now reflects the actual `ReloadPlan` — false when every diff field hot-reloads, true when any field needs a restart. A reload validation failure is surfaced via the new `reload_error` field instead of swallowing the disk write. * M13 `extract_search_keywords` returns a `Vec<String>` of the top 4 distinctive keywords ordered longest-first (post-stop-word filter, post-dedup), and the caller iterates over them unioning per-keyword LIKE recalls until `fetch_limit` is hit. Pre-fix it collapsed the four candidates to the single longest one, wasting the stop-word filter work on the other three and giving the no-embedding fallback path a frequently-too-generic substring (e.g. "analysis") to match against, OR a too-specific compound term that matched nothing. The fallback to the raw-content LIKE is preserved for the no-distinctive-words case so a near-verbatim duplicate is still detectable. * M14 the `decide_action` UPDATE thresholds are now configurable via two new `ProactiveMemoryConfig` fields: `update_threshold_same_category` (default 0.7) and `update_threshold_cross_category` (default 0.8). The trait method signature stays stable — `add_with_decision` stashes the live config values in the new memory's metadata under `_update_threshold_same_cat` / `_update_threshold_cross_cat`, the default `decide_action` reads them out (falling back to the const defaults for direct trait-method callers), and the LLM-backed extractor inherits the same behaviour via its fallback to the default heuristic on driver failure. This separates the per-insertion conflict-resolution threshold (UPDATE vs ADD) from the post-hoc consolidation threshold (`duplicate_threshold`) — pre-fix both were conflated. Drive-by fmt: two stale formatting hunks `cargo fmt` flagged in `runtime/tool_runner/wasm_skill.rs:159` and `api/tests/memory_routes_integration.rs:518` (neither mine, both inherited from main). Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-runtime -p librefang-types -p librefang-api -p librefang-kernel --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 273 passed (4 new regression tests: `extract_search_keywords_returns_multiple_ordered_longest_first`, `extract_search_keywords_empty_for_all_stop_words`, `decide_action_honors_config_update_thresholds`, and the existing suite re-verified against the new `update_threshold_*_category` config fields) * cargo test -p librefang-runtime --lib proactive_memory — 60 passed * cargo test -p librefang-api --lib --test memory_routes_integration --test agent_kv_authz_integration --test auth_public_allowlist — all green * fix(memory): review-followups on #5850 — doc-comment fix, threshold floor, partial-status, dedup-strip, test tightening 8 follow-ups from the code review of the prior commit. All in scope for the same proactive-memory layer. * #1 misattributed doc-comment in `proactive.rs` near `AUTO_CONSOLIDATE_EVERY` / `NEGATION_WORDS`. The `/// Negation/contradiction words …` line was orphaned above `AUTO_CONSOLIDATE_EVERY` when the new const got inserted; both consts now carry their own intended docstring. * #2 lowered `STALE_COUNTER_FLOOR` from `AUTO_CONSOLIDATE_EVERY / 2` (5) to `/ 4` (2). The /2 floor cleaned up "stuck at 1..4" agents but also reset slow-burn agents (single auto_memorize per maintenance tick) before they could climb to 10, so a steady 1-call/hour stream effectively never consolidated. /4 keeps the cold-slot eviction directional while letting low-frequency agents still accumulate to the trigger. * #3 `PATCH /api/memory/config` response shape is now explicit about partial success. `body.status` is `"applied"` when the reload succeeded and `"partial"` when the disk write landed but the live reload failed (e.g. operator hand-edited an unrelated section into an invalid shape between PATCH writes). Clients MUST inspect `status`; the HTTP code stays 200 for both branches since the disk write itself succeeded. 207 / 500 were considered and rejected — 500 misrepresents that the request was rejected (it wasn't) and 207 forces every existing client to re-classify success. Mirrors the `import_agent_memory` partial-success pattern. * #4 documented the M13 cost tradeoff. Up to 4 SQLite roundtrips per insertion on the no-embedding fallback path (versus the pre-fix 1), but the embedding-driver path is unaffected and the loop short-circuits as soon as the union hits fetch_limit, so the common case ("first keyword filled the slate") still runs a single query. Operators on the no-embedding path pay the cost on writes only, against an already-unindexed `content LIKE` scan. * #5 defensively strip the M14 `_update_threshold_*` keys from the enriched item right after `decide_action` returns + assert callers don't pre-populate them. Both insertion branches today build their stored metadata from the original `item.metadata` (not `enriched_item.metadata`), so the threshold keys never reach the column — but stripping + asserting is cheap insurance for the next refactorer who repoints either branch at the enriched copy. * #6 relaxed the `extract_search_keywords_returns_multiple_ordered_longest_first` test: no more exact `kws.len() == 4` — the cap and the "longest distinctive word survives" invariants are what we care about, not the exact count. Future additions to `STOP_WORDS` no longer silently break the test. * #7 dropped the unused `_update_threshold_cross_cat` set in `decide_action_honors_config_update_thresholds`. Both candidate memories share the "preference" category, so the cross-cat threshold branch was unreachable. * #8 moved `STALE_COUNTER_FLOOR` from a `fn`-scope `const` to module scope, alongside `AUTO_CONSOLIDATE_EVERY` from which it derives. Matches the repo convention and lets future readers see the floor / trigger relationship at one site. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-runtime -p librefang-types -p librefang-api --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 273 passed * cargo test -p librefang-api --test memory_routes_integration — 14 passed * fix(memory): review-followups (round 2) — proper M11 idle-window, M12 + M14 regression coverage, comment polish 5 follow-ups from the second review pass on #5850. * B (was #1) M11 done properly. `consolidation_counters` is now `HashMap<String, CounterEntry>` where `CounterEntry` carries both the running count and a `last_touched: DateTime<Utc>` stamp. The maintenance sweep evicts entries whose `last_touched` is older than `STALE_COUNTER_IDLE_WINDOW = 2 hours` (~2× the maintenance rate-limit window), regardless of count. The previous count-threshold fix (followup #2 in the prior commit) mitigated but didn't solve the slow-burn case: any agent firing ≤ 1 × per maintenance window would still be reset before climbing past the count floor. The timestamp-based check closes that gap — an active slot, however slow, is preserved as long as it's been touched within the window; a truly idle slot is reclaimed within ~2 hours of going quiet. * C (was #2) `PATCH /api/memory/config` happy-path test added at `memory_routes_integration::patch_memory_config_hot_reloads_and_reports_applied`. Pre-seeds a minimal `config.toml` (the harness's tempdir previously didn't materialise one, which the file-level docstring flagged as out-of-scope; the docstring updated accordingly) and asserts `body["status"] == "applied"`, `body["reload_error"]` null, and that the PATCHed value round-trips into the response. Without this, the M12 status contract could silently revert and the rest of the suite wouldn't catch it. `RouterHarness._tmp` is exposed as `tmp` to let the test reach the seed location. * D (was #3) M14 strip regression test added at `proactive::tests::add_with_decision_does_not_leak_threshold_keys_to_stored_metadata`. Drives the full `add()` path through `add_with_decision`, then reads back via `list()` and asserts none of the private `_update_threshold_*` / `_embedding` keys leaked into the stored metadata column. Catches the regression where someone repoints the ADD or UPDATE branch at `enriched_item.metadata` (the decision-clone) instead of the original `item.metadata` (the caller's input). * E (was #4) the `debug_assert!` panic messages on the `_update_threshold_*` private keys re-worded from "caller leaked it" to "callers must not pre-populate ..." — neutral phrasing that doesn't presume the caller is buggy. Also added a one-line note that the production path stays safe regardless (the unconditional `insert` overwrites any leaked value before `decide_action` reads it), since `debug_assert!` is compiled out in release. * F (was #5) M13 cost-trade-off comment tightened. The "common case still runs a single query" claim was accurate for agents with sizeable stores (first keyword exhausts fetch_limit) but not for fresh / small stores where no individual keyword has enough matches to short-circuit. The comment now distinguishes the two regimes instead of overgeneralising. Re-stamped `.secrets.baseline` line numbers shifted by the test file edits. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-api --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 274 passed (incl. `add_with_decision_does_not_leak_threshold_keys_to_stored_metadata`) * cargo test -p librefang-api --test memory_routes_integration — 15 passed (incl. `patch_memory_config_hot_reloads_and_reports_applied`) * fix(memory): review-followups (round 3) — M12 test key-presence + accepts partial, M14 strips _embedding too, prune rate-limit, chrono::Duration const portability 6 followups from the third review pass on #5850. * #1 M12 test pinned the contract properly. `serde_json::Value` indexed by a missing key returns `Value::Null`, so the prior `assert_eq!(body["reload_error"], Null)` silently passed even if the field had been removed. Now asserts `body.as_object() .contains_key(...)` for `status`, `restart_required`, `reload_error` first, then asserts their values. * #2 strip + test for the `_embedding` private-stash key. `add_with_decision` now also calls `enriched_item.metadata.remove("_embedding")` after `decide_action` returns, so all three private stash keys (`_update_threshold_*` + `_embedding`) get the same defensive treatment. Test renamed to `add_with_decision_does_not_leak_private_stash_keys_to_stored_metadata` and attaches a tiny mock `EmbeddingFn` so the `_embedding` stash path actually fires — the prior assertion against `_embedding` was decorative because the test had no embedding driver configured. * #3 rate-limited the counter prune via a new `last_counter_prune: Arc<Mutex<Option<DateTime<Utc>>>>` and a `maybe_prune_counters` helper that mirrors the `maybe_decay_confidence` / `maybe_cleanup_expired` once-per-hour pattern. Prior to this the prune ran on every `maybe_run_maintenance` call, reachable from `search` / `auto_retrieve` / `consolidate` at potentially many Hz. The retain itself was microseconds, so this is a wash today, but it brings the three maintenance sub-tasks under the same scheduling budget for future scaling. * #4 docstring on `STALE_COUNTER_IDLE_WINDOW_HOURS` re-phrased to describe the slot-keeping guarantee in terms of the maintenance rate-limit window, not "consecutive prune passes" — the old phrasing happened to be true only because the prune wasn't rate-limited yet (now rectified by #3). * #5 M12 test now accepts either `"applied"` or `"partial"` as the body status — both are valid post-fix outcomes; the pre-fix contract had no `status` field at all. The status field's presence (asserted via the #1 fix) is the actual contract we're pinning. Made the test robust to future `KernelConfig::default()` changes that might cause the seeded toml to fail reload validation. * #6 swapped `const STALE_COUNTER_IDLE_WINDOW: chrono::Duration = chrono::Duration::hours(2)` for `const STALE_COUNTER_IDLE_WINDOW_HOURS: i64 = 2` plus `chrono::Duration::hours(STALE_COUNTER_IDLE_WINDOW_HOURS)` at the call site. `chrono::Duration::hours` is a `const fn` at the currently pinned `chrono` minor but the const-ness isn't a stable contract across `0.4.x` versions, so a lockfile bump could silently break the build. The integer-hours + runtime conversion stays valid regardless. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-api --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 274 passed (incl. `add_with_decision_does_not_leak_private_stash_keys_to_stored_metadata` with embedding-driver coverage) * cargo test -p librefang-api --test memory_routes_integration — 15 passed (incl. tighter `patch_memory_config_hot_reloads_and_reports_applied` contract assertions) * fix(memory): review-followups (round 4) — docstring honesty + direct strip helper + non-empty partial-error 3 follow-ups from the fourth review pass on #5850. All three are about closing gaps between what the code does and what the docstrings / tests claim it does. * A `STALE_COUNTER_IDLE_WINDOW_HOURS` docstring: the "reclaimed within ~2 hours of going quiet" claim was accurate before the round-3 prune rate-limit landed, after which the worst-case reclaim latency is 2-3 hours (idle window + up to one prune rate-limit period because the prune itself only runs ≤ once per hour). Re-phrased with explicit upper/lower bounds; deleted the duplicate "previous fix (round-1 followup #2)" paragraph that had ended up in the doc twice during the round-2 edit. * B `strip_private_stash_keys` extracted into a module-level function driven by a single `ADD_WITH_DECISION_PRIVATE_STASH_KEYS` const, and unit-tested directly via `strip_private_stash_keys_removes_all_private_keys`. The integration test `add_with_decision_does_not_leak_private_stash_keys_to_stored_metadata` only catches a *coordinated two-step regression* (strip removed AND ADD/UPDATE branch repointed at `enriched_item.metadata`) — the current ADD path bypasses `enriched_item.metadata` entirely, so single-step regressions of either kind pass that test. The prior commit's docstring claimed "the strip code on the post-decide path is the only thing keeping it out of the stored column", which was wrong — `item.metadata` (the caller's input) is what actually keeps the keys out today. Updated the docstring to match reality and added the direct unit test on the helper to cover single-step strip regressions. * C `reload_error` partial-branch assertion in `patch_memory_config_hot_reloads_and_reports_applied` now also rejects empty / whitespace-only strings. `is_string()` alone passed `""` / `" "` / any other zero-info value — operators would see status=partial with a useless error blob and have no actionable diagnostic. Trimmed-non-empty makes the contract honest about what "carries the validator output" means. Verification: * cargo check --workspace --lib — clean * cargo clippy -p librefang-memory -p librefang-api --tests -- -D warnings — clean * cargo test -p librefang-memory --lib — 275 passed (1 new: `strip_private_stash_keys_removes_all_private_keys`) * cargo test -p librefang-api --test memory_routes_integration — 15 passed --------- Co-authored-by: Evan <[email protected]>
houko
added a commit
that referenced
this pull request
May 29, 2026
…etector (#6) (#5876) `detect_misplaced_per_agent_overrides` flagged only `proactive_memory` / `skill_workshop` / `compaction`, so the four other AgentManifest fields that override a global KernelConfig section (`thinking`, `exec_policy`, `max_history_messages`, and the tool-exec backend) silently no-opped when written under `[agents.x.<key>]` in config.toml without the targeted "move this to agent.toml" warning #5476 added. The list was hand-maintained with only a comment as a drift guard. Add the missing keys and replace the comment with a compile-time drift guard: a test that exhaustively destructures `AgentManifest`, so any newly-added field stops the build until the contributor classifies it as either a global-section override (add its config.toml-facing key to `PER_AGENT_OVERRIDE_KEYS`) or a non-override (list it in the test's OTHER arm). The schemars-intersection approach used elsewhere does not fit here: `AgentManifest` does not derive JsonSchema, `skill_workshop` has no global KernelConfig section, and the tool-exec backend's manifest field (`tool_exec_backend`) differs from its global section name (`tool_exec`). Both tool-exec spellings are flagged so the operator is caught regardless of which one they copy. Verified in the sanctioned Linux dev image (scoped): cargo check / clippy -D warnings / test config:: / fmt --check all pass for librefang-types. Co-authored-by: Evan <[email protected]>
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.
Bumps rusqlite from 0.31.0 to 0.38.0.
Release notes
Sourced from rusqlite's releases.
... (truncated)
Commits
35b3be2Merge pull request #1777 from gwenn/release03feebcPrepare new release671533cMerge pull request #1775 from Spxg/w/bump6df2072Bumpsqlite-wasm-rsto 0.5.12618364Merge pull request #1774 from rusqlite/vtab-sendb37caffMerge pull request #1773 from rusqlite/clippy44bdbc4Remove useless Send/Sync on Module465d751Fix Clippy warning78d6678Merge pull request #1772 from rusqlite/dependabot/cargo/getrandom-0.36b8fb83Update Cargo.tomlDependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)