Skip to content

Bump governor from 0.8.1 to 0.10.4#4

Closed
dependabot[bot] wants to merge 1 commit into
mainfrom
dependabot/cargo/governor-0.10.4
Closed

Bump governor from 0.8.1 to 0.10.4#4
dependabot[bot] wants to merge 1 commit into
mainfrom
dependabot/cargo/governor-0.10.4

Conversation

@dependabot

@dependabot dependabot Bot commented on behalf of github Mar 12, 2026

Copy link
Copy Markdown
Contributor

Bumps governor from 0.8.1 to 0.10.4.

Commits
  • 9f3a79d Merge pull request #291 from boinkor-net/release/governor/0.10.4
  • 9010ee9 Update Changelog
  • 2351660 Release 0.10.4 🎉🎉
  • 61b1754 Merge pull request #290 from boinkor-net/push-pnoqtmytrrpz
  • 1bdc26d Use feature(doc_cfg) instead of feature(doc_auto_cfg) for docsrs
  • 296018b chore: Update ci_rust.yml in governor
  • a29466c chore: Update release_pr_for_crates_io.yml in governor
  • 78a3be2 chore: Update release_to_crates_io.yml in governor
  • 1c5840c Merge pull request #289 from boinkor-net/release/governor/0.10.3
  • fff7f37 Update Changelog for release
  • Additional commits viewable in compare view

Dependabot compatibility score

Dependabot 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 rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
  • @dependabot ignore this major version will 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 version will 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 dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

@dependabot @github

dependabot Bot commented on behalf of github Mar 12, 2026

Copy link
Copy Markdown
Contributor Author

Labels

The following labels could not be found: dependencies. Please create it before Dependabot can add it to a pull request.

Please fix the above issues or remove invalid values from dependabot.yml.

@houko houko closed this Mar 12, 2026
@dependabot @github

dependabot Bot commented on behalf of github Mar 12, 2026

Copy link
Copy Markdown
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 @dependabot ignore this major version or @dependabot ignore this minor version. You can also ignore all major, minor, or patch releases for a dependency by adding an ignore condition with the desired update_types to your config file.

If you change your mind, just re-open this PR and I'll resolve any conflicts on it.

@dependabot
dependabot Bot deleted the dependabot/cargo/governor-0.10.4 branch March 12, 2026 01:59
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
…rd 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.
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.
houko added a commit that referenced this pull request Apr 26, 2026
Two follow-up fixes for the granular MCP taint policy review:

**Low #3 — JSONPath dotted-key limitation**
The pattern parser splits on `.` and treats `[` as the start of array
notation, so a JSON object key containing `.` or `[` (e.g. `"a.b"`,
`"items[0]"`) cannot be addressed precisely. Quoted-segment syntax
(`$.headers."content-type"`) is also not parsed. In practice MCP tool
schemas rarely use such keys, but the matcher fails closed (no false
positive skip) when it bites.

- Add a "Limitation" section to `jsonpath_matches` rustdoc.
- New regression test `test_jsonpath_dotted_keys_are_known_limitation`
  pins the current behaviour so a future change has to opt in.

**Low #4 — silent typos in `tool_policy.rule_sets`**
`lookup_rule_set_action` previously returned `None` when a referenced
name was missing from the registry, so an operator typo (e.g.
`"audit_typo"` instead of `"audit"`) became a silent no-op. Now:

- New `warn_unknown_rule_set_once(name, tool)` emits a one-shot WARN
  on first scan that observes a missing name, deduped per process via
  a static `OnceLock<Mutex<HashSet<String>>>`.
- Wired into `lookup_rule_set_action` at the existing skip site.
- New `GET /api/mcp/taint-rules` exposes the registered
  `[[taint_rules]]` names + actions so the dashboard can validate
  inputs *before* the operator hits Save (separate commit handles the
  frontend wiring; the endpoint is read-only and reuses the existing
  auth chain).
- Regression test verifies the unknown-name path returns `None` and
  doesn't panic / corrupt the dedup set on repeated calls.
houko added a commit that referenced this pull request Apr 26, 2026
Closes the dashboard half of Low #4 from the #3193 review. Backend
already emits a one-shot WARN for unknown names; this wires the
new `GET /api/mcp/taint-rules` endpoint into the editor so typos
surface *before* the operator hits Save.

- `listMcpTaintRules` (api.ts) + `useMcpTaintRules` (queries/mcp.ts)
  wrap the registry endpoint with a 5-minute staleTime — operators
  don't expect rule-set renames to land instantly and there's no
  hot churn here.
- `mcpKeys.taintRules()` factory follows the existing hierarchy
  (`mcpKeys.all` invalidation still nukes it).
- `TaintPolicyEditor` derives `knownRuleSetNames: Set<string>` once
  and threads it down to each `ToolPolicyRow`.
- Each row computes `unknownRuleSetNames` from the comma-typed input
  and renders a red inline hint listing the offenders. Falls back
  silently when the registry is empty (`knownRuleSetNames.size === 0`)
  so a fresh kernel without `[[taint_rules]]` doesn't false-positive.
- New `mcp.taint_rule_sets_unknown_hint` in en/zh locales.
- Regenerated `openapi.json` to include the new path (the pre-commit
  hook missed the previous backend-only commit because the registration
  in `openapi.rs` was added in the immediately following commit).
houko added a commit that referenced this pull request Apr 26, 2026
 #3050) (#3193)

* feat(runtime): granular MCP taint policy with rule sets and dashboard tree editor

- McpTaintToolPolicy gains a 'default' field (scan | skip) so a single
  line can bypass scanning for noisy tools (browser tab handles, DB
  session IDs, etc.) without enumerating every argument path.
- Document JSONPath wildcard semantics ($.foo, $.foo.*, $.foo[*], $.*)
  in McpTaintPathPolicy rustdoc and add a dedicated wildcard test that
  asserts each documented form against the in-tree matcher.
- Introduce top-level [[taint_rules]] with named NamedTaintRuleSet
  entries (action: block | warn | log, plus a list of TaintRuleId).
  Tool policies can reference them via 'rule_sets'; the scanner picks
  the most permissive action across overlapping sets and emits
  structured tracing events for warn/log so operators can build an
  exemption baseline before flipping a set to block.
- Wire the registry from KernelConfig.taint_rules through every
  install/reload path in the kernel into the runtime McpServerConfig
  (taint_rule_sets snapshot) so live taint scans apply the latest
  rule-set actions.
- Surface taint_scanning + taint_policy in /api/mcp/servers and
  /api/mcp/servers/{name} so the dashboard can hydrate without a
  second fetch.
- Dashboard: add TaintPolicyEditor (server -> tool -> path tree with
  in-place add/edit/remove, default action selector, rule_sets
  reference, per-path rule toggles), wire a 'Taint' button into
  ServerCard, and route saves through useUpdateMcpTaintPolicy which
  invalidates mcpKeys.servers / mcpKeys.server / mcpKeys.health so
  the card refreshes without a full config reload.
- Add TaintRuleId / McpTaintToolAction / McpTaintRuleSetAction TS
  types mirroring the snake_case serde tags.
- Round-trip serde tests cover legacy configs (no 'default', no
  'rule_sets'), new tool-level skip, wildcard paths, and all three
  rule-set severity tiers.

Closes #3050

* fix(runtime-mcp): multi-rule masking + kernel snapshot helper

The taint scanner returned only the first rule that fired, so a rule
set authorized to downgrade rule A could silently allow rule B
through when both fired on the same payload (e.g. a Secret-rule warn
downgrade masking a PII-rule fire on `[email protected]`).

- Add `detect_outbound_text_violation_rules_with_skip` returning every
  rule that fires; old `_with_skip` keeps the first-only signature for
  backward-compat callers.
- `walk_taint` now iterates every fired rule and blocks on the first
  one not downgraded by a rule set.
- Regression test asserts the masking case blocks.
- Fix pre-existing `tool_policy_rule_sets_reference_round_trips` that
  used the wrong toml prefix `[mcp_servers.tools.*]` (the policy struct
  has no `mcp_servers` field — test was masked by a `-- taint` filter).
- Extract `LibreFangKernel::snapshot_taint_rules` helper; replace the
  five inline `self.config.load().taint_rules.clone()` call sites.
- Document the snapshot semantics on `KernelConfig.taint_rules`: edits
  do not propagate to already-connected MCP servers without reload.

* feat(runtime-mcp): hot-reload taint_rules + dedicated PATCH /taint endpoint

Addresses the two remaining review concerns from #3193:

**Hot-reload `[[taint_rules]]`**
- `McpServerConfig.taint_rule_sets` is now an `Arc<ArcSwap<Vec<...>>>`
  shared across every connected MCP server (was: per-server `Vec`
  snapshot taken at install time). Operators editing
  `[[taint_rules]]` and reloading config now see the new rules
  applied to in-flight tool calls without restarting servers.
- Kernel owns the master swap; `LibreFangKernel.taint_rules_swap`
  is initialised at boot and `.store(...)`-updated by `reload_config`
  (before swapping `self.config` so no scanner observes a window
  where the two disagree).
- Scanner takes a single `.load()` per scan call so config edits
  during an in-flight argument-tree walk can't mutate rules under
  the scanner.
- `empty_taint_rule_sets_handle()` and `static_taint_rule_sets_handle()`
  helpers exposed for tests and out-of-kernel callers.

**Dedicated PATCH endpoint**
- New `PATCH /api/mcp/servers/{name}/taint` accepts only
  `{ taint_scanning?, taint_policy? }`, removing the dashboard's
  need to round-trip every other server field through PUT. Closes
  the silent-drop risk if a future required field gets added without
  dashboard support.
- `useUpdateMcpTaintPolicy` rewritten to call the new endpoint;
  `TaintPolicyEditor` updated to pass `id` instead of the full
  `existing` server config.

Verified the multi-threaded kernel-test SIGABRT reproduces on
`02eb92af` (the original PR head before any fixes), confirming
it is pre-existing and not introduced by this change.

* fix(kernel): wire `[[taint_rules]]` into reload-plan diff

Live integration smoke surfaced a gap in the previous hot-reload
commit: appending or editing `[[taint_rules]]` returned
`status: no_changes` because `build_reload_plan` did not diff that
field, so `should_apply_hot` returned false and the swap-store was
never reached.

- New `HotAction::ReloadTaintRules` variant; pushed when
  `old.taint_rules != new.taint_rules`.
- Logged from `apply_hot_actions_inner` for audit-trail visibility.
- Derive `PartialEq + Eq` on `NamedTaintRuleSet` so the diff compiles
  (other rule types in the chain — `McpTaintRuleSetAction` and
  `TaintRuleId` — already had it).
- Reverted the temporary `else if` in `reload_config` that performed
  a partial swap when the plan was empty; the diff now correctly
  drives the canonical `should_apply_hot` path so no fallback is
  needed.

Live smoke (`PATCH /api/mcp/servers/{name}/taint` + reload-config
flows on an isolated `LIBREFANG_HOME`) now reports:
- first reload after adding `[[taint_rules]]` → `hot_actions_applied:
  ["UpdateDefaultModel", "ReloadTaintRules"]`
- idempotent second reload → `status: no_changes`
- editing only `action = "warn"` → `action = "log"` → `hot_actions_applied:
  ["ReloadTaintRules"]`

* docs(taint): clarify rule_set overlap semantics + fix stale PUT comment

Two follow-ups from PR review:

1. The TaintPolicyEditor doc comment said the editor saves through the
   "existing PUT /api/mcp/servers/{id} endpoint" but actually goes through
   the new dedicated PATCH /api/mcp/servers/{id}/taint added in this PR.
   Updated the comment to match and noted why PATCH is preferred (no
   revalidation of unrelated McpServerConfig fields on every taint edit).

2. When a tool's `rule_sets` list references multiple sets that overlap
   on the same TaintRuleId, the scanner picks the *most permissive*
   action (Log > Warn > Block). This is intentional but counter-intuitive
   — adding an `audit_only` set with `action = log` will silently
   neutralise any `block` set that overlaps on the same rule.

   Added a prominent rustdoc paragraph on `McpTaintRuleSetAction` and an
   inline UI hint next to the dashboard `rule_sets` field (en + zh
   locales) so operators authoring config aren't surprised.

* fix(kernel): reconnect MCP servers on hot-reload when entry config differs

`HotAction::ReloadMcpServers` previously only refreshed
`effective_mcp_servers` and the health registry; existing connections
held their per-server `McpServerConfig` clone (including `taint_policy`,
`taint_scanning`, `headers`, `env`, `transport`), so edits via
PUT `/api/mcp/servers/{name}`, direct `config.toml` edits, or any
non-PATCH reload path silently kept the old policy alive on
already-connected servers — a security drift hazard for operators
tightening rules and assuming the change took effect.

`taint_rule_sets` was already shared via `Arc<ArcSwap>` in the previous
fix commit; this change covers the rest of the per-server snapshot.

- Snapshot the prior effective list, diff each entry by JSON-serialized
  content (robust against future field additions, no `PartialEq`
  required on `McpOAuthConfig` / transport variants / `McpTaintPolicy`).
- Servers that are removed or whose entry differs go into `to_reconnect`.
- After updating `effective_mcp_servers` and bumping `mcp_generation`,
  fire-and-forget a task that disconnects the stale slots and calls
  `connect_mcp_servers()` (idempotent — re-adds servers missing from
  `mcp_connections` using the freshly-installed effective list).
- The PATCH `/api/mcp/servers/{name}/taint` endpoint kept its explicit
  disconnect+reconnect; this change makes the reload-config path behave
  the same.

* fix(dashboard): disable rule_sets input when tool default = skip

The MCP taint scanner short-circuits when a tool's `default = "skip"`
and returns before the rule-set walker runs, so any `rule_sets` on the
same tool — including audit-only `Log` sets — never fire. The dashboard
editor still let operators fill `rule_sets` on a `default = skip` tool,
which silently became dead config and could give a false sense of "I
have audit logging on this noisy tool".

- Disable the `rule_sets` Input when `policy.default === "skip"` so new
  values can't be added via the UI.
- Show a warning hint when the saved policy already has `rule_sets`
  alongside `default = skip`, telling the operator to switch back to
  `default = scan` if they want audit-only behaviour.
- Existing `rule_sets` values are preserved in state (not silently
  dropped on save) so toggling default back to `scan` restores them.

Adds `mcp.taint_skip_rule_sets_hint` in en/zh locales.

* fix(runtime-mcp,api): warn on typo'd rule_set + document JSONPath limits

Two follow-up fixes for the granular MCP taint policy review:

**Low #3 — JSONPath dotted-key limitation**
The pattern parser splits on `.` and treats `[` as the start of array
notation, so a JSON object key containing `.` or `[` (e.g. `"a.b"`,
`"items[0]"`) cannot be addressed precisely. Quoted-segment syntax
(`$.headers."content-type"`) is also not parsed. In practice MCP tool
schemas rarely use such keys, but the matcher fails closed (no false
positive skip) when it bites.

- Add a "Limitation" section to `jsonpath_matches` rustdoc.
- New regression test `test_jsonpath_dotted_keys_are_known_limitation`
  pins the current behaviour so a future change has to opt in.

**Low #4 — silent typos in `tool_policy.rule_sets`**
`lookup_rule_set_action` previously returned `None` when a referenced
name was missing from the registry, so an operator typo (e.g.
`"audit_typo"` instead of `"audit"`) became a silent no-op. Now:

- New `warn_unknown_rule_set_once(name, tool)` emits a one-shot WARN
  on first scan that observes a missing name, deduped per process via
  a static `OnceLock<Mutex<HashSet<String>>>`.
- Wired into `lookup_rule_set_action` at the existing skip site.
- New `GET /api/mcp/taint-rules` exposes the registered
  `[[taint_rules]]` names + actions so the dashboard can validate
  inputs *before* the operator hits Save (separate commit handles the
  frontend wiring; the endpoint is read-only and reuses the existing
  auth chain).
- Regression test verifies the unknown-name path returns `None` and
  doesn't panic / corrupt the dedup set on repeated calls.

* fix(api): register `list_mcp_taint_rules` with the OpenAPI generator

The handler was added to the router in the previous commit but I
forgot to list it in `openapi.rs::ApiDoc`, so `openapi.json` was
silently re-generated without the new path and the dashboard's typed
client wouldn't see the route in the contract.

Trivial follow-up — adds one line under the MCP block.

* feat(dashboard): inline validation for unknown taint rule_set names

Closes the dashboard half of Low #4 from the #3193 review. Backend
already emits a one-shot WARN for unknown names; this wires the
new `GET /api/mcp/taint-rules` endpoint into the editor so typos
surface *before* the operator hits Save.

- `listMcpTaintRules` (api.ts) + `useMcpTaintRules` (queries/mcp.ts)
  wrap the registry endpoint with a 5-minute staleTime — operators
  don't expect rule-set renames to land instantly and there's no
  hot churn here.
- `mcpKeys.taintRules()` factory follows the existing hierarchy
  (`mcpKeys.all` invalidation still nukes it).
- `TaintPolicyEditor` derives `knownRuleSetNames: Set<string>` once
  and threads it down to each `ToolPolicyRow`.
- Each row computes `unknownRuleSetNames` from the comma-typed input
  and renders a red inline hint listing the offenders. Falls back
  silently when the registry is empty (`knownRuleSetNames.size === 0`)
  so a fresh kernel without `[[taint_rules]]` doesn't false-positive.
- New `mcp.taint_rule_sets_unknown_hint` in en/zh locales.
- Regenerated `openapi.json` to include the new path (the pre-commit
  hook missed the previous backend-only commit because the registration
  in `openapi.rs` was added in the immediately following commit).
houko pushed a commit that referenced this pull request Apr 27, 2026
* feat(skills): add env_passthrough allowlist to skill manifest

Skills can now declare an env_passthrough list in skill.toml that
specifies host environment variables to forward into the skill
subprocess. The default remains empty — full env_clear isolation —
preserving the security posture for third-party skills.

Mirrors the existing [exec_policy].allowed_env_vars mechanism for
shell_exec. Concrete motivation: skills that wrap a CLI requiring
credentials in env (e.g., gog with GOG_KEYRING_PASSWORD for its
file-backed keyring) cannot work today because env_clear strips
those vars before the subprocess sees them.

The allowlist is per-skill, names only — values are not stored in
the manifest. Variables not present in the host environment are
silently skipped. Visibility is opt-in and the manifest is public,
so users can audit what each skill imports.

Plumbed through execute_python, execute_node, and execute_shell via
a small apply_env_passthrough helper. WASM skills are out of scope
(separate sandbox model). Includes serde round-trip tests and a
loader integration test that asserts allowlisted vars reach the
subprocess and non-allowlisted vars do not.

* feat(skills): operator-side env_passthrough config + denied patterns

Adds [skills].env_passthrough_denied_patterns and
[skills].env_passthrough_per_skill to KernelConfig so the operator,
not the skill author, has the final say on which host env vars cross
the env_clear boundary into a skill subprocess.

Defaults to deny-by-default for the long tail of credential
conventions (*_KEY, *_TOKEN, *_PASSWORD, *_SECRET, *_API_KEY, AWS_*,
GITHUB_*). Operators grant exceptions per-skill via
env_passthrough_per_skill — e.g.
  [skills.env_passthrough_per_skill]
  gog = ["GOG_KEYRING_PASSWORD"]

Introduces EnvPassthroughPolicy in librefang-types as the runtime
representation. Construction wired up via from_skills_config; the
loader-side resolution that consumes it lands in the next commit.

Addresses review #1 on PR #3219: skill manifest's env_passthrough
allowlist is author-controlled, but credentials are an operator
concern. Without an operator-side gate, a community skill shipping
env_passthrough = ["AWS_SECRET_ACCESS_KEY"] would silently leak
every secret in the host environment to anyone who installs it
without auditing the manifest.

* fix(skills): block dangerous env vars + apply passthrough before kernel env

Three security fixes to env_passthrough on top of the operator-side
config landed in the previous commit. All three were merge blockers
on PR #3219.

1. Hard-block FORBIDDEN_PASSTHROUGH (review #2). Names like LD_PRELOAD,
   PYTHONPATH, NODE_OPTIONS, DYLD_INSERT_LIBRARIES, etc. are dropped
   regardless of skill manifest or operator config — these inject code
   or redirect imports/library lookup and would defeat the env_clear
   isolation entirely. Not overridable; even per-skill operator
   overrides cannot unblock them.

2. Hard-block KERNEL_RESERVED_ENV. Names the kernel sets explicitly
   per-runtime (PATH, HOME, PYTHONIOENCODING, NODE_NO_WARNINGS, …)
   are dropped. A skill cannot widen a deliberately narrowed kernel
   PATH or override HOME via env_passthrough.

3. Apply passthrough before kernel env settings (review #3).
   tokio::process::Command::env is last-write-wins; the original PR
   set PATH/HOME/PYTHONIOENCODING and *then* called
   apply_env_passthrough, so a skill with env_passthrough = ["PATH"]
   could clobber the kernel-curated PATH. Reordered so passthrough
   runs first; kernel settings always win. Belt-and-suspenders, the
   resolver also strips kernel-reserved names so the ordering is
   defense-in-depth, not the only line of defense.

Plumbing:
- librefang-types: EnvPassthroughPolicy with from_skills_config.
- KernelHandle: skill_env_passthrough_policy() with default-None impl;
  kernel impl pulls from KernelConfig.skills.
- librefang-skills::loader: resolve_effective_passthrough applies the
  full pipeline (forbidden → kernel-reserved → operator deny → per-
  skill override). execute_skill_tool now takes the policy and
  resolves once before dispatching to the per-runtime spawner.
- tool_runner: fetches policy via the kernel handle at the dispatch
  site so the existing execute_tool signature is unchanged.

* test(skills): unit tests for resolver + serialize env-mutation test; docs

Test coverage for the env_passthrough resolver (5 unit tests):
- Forbidden vars (LD_PRELOAD, PYTHONPATH) are blocked.
- Kernel-reserved vars (PATH, HOME, …) are blocked.
- Forbidden match is case-insensitive.
- Operator deny patterns (AWS_*, *_KEY) work.
- Per-skill override unblocks denied vars only for the named skill,
  and cannot bypass the FORBIDDEN_PASSTHROUGH hard block.

Mark the existing env-mutating integration test
#[serial_test::serial(skill_env_passthrough)] (review #4): it calls
std::env::set_var/remove_var which mutates process-global state and
would race with any other env-touching test under the parallel
default of cargo test. The serial harness is already used by hands/
extensions for the same reason.

Docs (review #5): add an Environment Variable Passthrough section to
docs/src/app/agent/skills/page.mdx covering the two-party opt-in
model (skill author declares; operator gates), the resolution order,
the FORBIDDEN list, and a worked gog example. Calls out the
distinction from skill config variables — credentials should go
through config, not env.

* fix(skills): address PR 3219 review — case-insensitive deny, CLI policy, runtime warn

- resolve_effective_passthrough lowercases both pattern and name before
  glob match, closing a bypass where 'aws_secret_access_key' would slip
  past the default 'AWS_*' deny pattern (Windows env vars are
  case-insensitive at the OS level). New unit test asserts mixed-case
  manifest entries are blocked.
- librefang skill test now loads [skills] from ~/.librefang/config.toml
  and applies the same EnvPassthroughPolicy the daemon does, falling
  back to SkillsConfig::default() when no config exists. Dev runs see
  the same gate prod will apply instead of silently passing every
  declared var.
- registry.load_skill emits a tracing::warn when env_passthrough is
  non-empty for Builtin / PromptOnly / Wasm runtimes, since the field
  only flows to subprocess-spawning runtimes (Python / Node / Shell)
  and was previously inert without feedback.
- Drop stale 'Returns None' sentence on EnvPassthroughPolicy::from_skills_config
  doc; the Optional-ness lives on KernelHandle::skill_env_passthrough_policy.

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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
…) (#4572)

* fix(cron): warn on context-window approach + expose session size in API (#3693)

cron_session_max_tokens / cron_session_max_messages already prune
Persistent cron sessions before each fire (since #2989). This PR
closes the operator-visibility gap that the umbrella issue #3693
flagged:

- tracing::warn! when the post-prune session estimate exceeds
  cron_session_warn_fraction (new KernelConfig field, default 0.8)
  of the provider's context window. First indication of pressure
  is now a structured log line, not a hard provider 400.
- Cron job status / detail responses now include
  session_message_count and session_token_count so dashboard /
  operators can graph growth trends.
- docs/architecture/cron-session-sizing.md documents both pruning
  caps + the new warn fraction + the API observability fields.

Two new KernelConfig fields:
  cron_session_warn_fraction: Option<f64>          (default Some(0.8))
  cron_session_warn_total_tokens: Option<u64>      (default Some(200_000))
The warn-fraction is applied against cron_session_max_tokens when
set, otherwise against cron_session_warn_total_tokens as a fallback
ceiling so jobs that have not opted into pruning still get warnings.

Two new fields on the cron job detail / status JSON response:
  session_message_count: u64
  session_token_count:   u64
Both default to 0 when no Persistent cron session exists yet.
Strictly additive — existing clients keep working.

Side effect of the warn wiring: the prune block now only writes
the session back to disk when messages were actually drained
(was: write on every fire that entered the prune branch). This
avoids touching SQLite on warn-only fires.

Gentle summarize-and-trim compaction (issue gap #4) is tracked
separately under #3693 — it requires a synthetic LLM round-trip
and is a larger change.

Closes part of #3693

* chore(codegen): auto-regenerate openapi.json + sdk + schema baselines [skip ci]

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
houko added a commit that referenced this pull request May 4, 2026
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.
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>
houko added a commit that referenced this pull request May 10, 2026
…uture field additions

Review feedback #4 on #4863. The PR has ~30 construction sites all
explicitly setting `reasoning_echo_policy: ReasoningEchoPolicy::default()`.
A `#[derive(Default)]` on `CompletionRequest` makes the next struct
field addition a one-line job (no need to revisit every site again),
and lets new call sites use struct-update syntax when only a few
fields differ from the zero values.

Existing call sites are left untouched in this PR — refactoring 30+
hand-written constructions to `..Default::default()` is a separate,
larger diff with its own review surface. The derive is purely
additive: existing field-by-field construction still works unchanged.

The default value is *not* a usable request — `model` is empty and
`messages` is empty — so this can't accidentally produce a valid LLM
call. Doc-comment on the struct calls that out.

Verification:
  * cargo check -p librefang-llm-driver --lib — clean
  * cargo clippy --workspace --all-targets -- -D warnings — clean
houko added a commit that referenced this pull request May 11, 2026
…) (#4863)

* feat(types,llm-driver,llm-drivers): catalog-driven ReasoningEchoPolicy with substring fallback

Refs #4842 — long-term replacement for the OpenAI
driver's three substring-match helpers (`is_deepseek_reasoner`,
`is_deepseek_v4_thinking_with_tools`, `kimi_needs_reasoning_content`)
that decide how `reasoning_content` is emitted on historical assistant
turns. Pairs with the registry-side change in librefang-registry#90.

This PR is the bedrock + bridge stage. It introduces the data model,
plumbs the field through CompletionRequest, and gives the OpenAI
driver a single dispatch point — without changing observable
behaviour. Every CompletionRequest construction site is filled with
`Default::default()` (= `None`), so the driver falls back to substring
detection. Behaviour is bit-for-bit identical to main on day one.

Changes:

* `librefang-types::model_catalog::ReasoningEchoPolicy` enum models
  the four wire conventions: `None` (omit), `Strip` (DeepSeek R1
  rejects historical reasoning_content), `Echo` (DeepSeek V4 Flash
  requires it on tool_calls turns — #4842), `EmptyString` (Moonshot
  Kimi requires empty + thinking disabled wire-side).

* `ModelCatalogEntry.reasoning_echo_policy` — `#[serde(default)]` so
  older catalog files keep parsing.

* `CompletionRequest.reasoning_echo_policy` — carries policy from
  request-construction sites to the driver. 21 construction sites
  filled with `Default::default()` in this PR; a follow-up will
  thread the catalog lookup into the main path.

* `OpenAIDriver::effective_reasoning_echo_policy` — resolves the
  policy for a request. Catalog metadata wins; `None` falls back to
  `fallback_reasoning_echo_policy`, which encapsulates the three
  legacy substring checks. `build_request` then dispatches via a
  single match on the resolved policy for `reasoning_content`,
  `temperature` pin, and the `thinking: disabled` field — three
  sites that previously each substring-matched independently.

The substring fallback exists because the catalog sync is
best-effort, users can register custom models in
`~/.librefang/model_catalog.toml` without a policy field, and several
auxiliary CompletionRequest construction sites (compactor,
history_fold, proactive_memory, aux_client) don't yet thread the
catalog. It will be removed in a later PR once every site reads from
the catalog.

Tests (12 new + 5 from #4856 = 17 #4842-related, all passing):

  librefang-types
    test_reasoning_echo_policy_serializes_snake_case
    test_reasoning_echo_policy_deserializes_snake_case
    test_reasoning_echo_policy_default_is_none
    test_model_catalog_entry_parses_reasoning_echo_policy_from_toml
    test_model_catalog_entry_defaults_reasoning_echo_policy_when_absent

  librefang-llm-drivers (openai)
    test_catalog_echo_policy_overrides_unmatched_substring
    test_catalog_strip_policy_overrides_unmatched_substring
    test_catalog_empty_string_policy_overrides_unmatched_substring
    test_catalog_none_falls_back_to_substring_for_v4_flash
    (the 5 deepseek tests from #4856 now also exercise the fallback
     path because their CompletionRequests carry default `None`
     policy — they prove fallback wire shape == pre-PR wire shape.)

Verification:

  * cargo check --workspace --all-targets — clean
  * cargo clippy --workspace --all-targets -- -D warnings — clean
  * cargo fmt --all --check — clean
  * cargo test -p librefang-types --lib — all pass
  * cargo test -p librefang-llm-drivers --lib — 474 pass, 0 fail
    (470 baseline + 4 new catalog-policy tests)

Out-of-scope follow-ups:

  * Wire `agent_loop::run_agent_loop` /
    `run_agent_loop_streaming` and the auxiliary construction sites
    to read `reasoning_echo_policy` from the catalog. Requires
    extending `KernelHandle` to expose the catalog or threading it
    through the call chain.
  * Once every site is wired, remove the substring fallback and the
    three legacy helpers (`is_deepseek_reasoner`,
    `is_deepseek_v4_thinking_with_tools`,
    `kimi_needs_reasoning_content`).
  * Decide whether `deepseek-v4-pro` needs `Echo` too — #4842
    reports V4 Pro working out-of-the-box but the registry has both
    V4 variants flagged `supports_thinking = true`.

* docs(types,llm-drivers): clarify ReasoningEchoPolicy semantics; tighten EmptyString override test

Review feedback on #4863:

- ReasoningEchoPolicy::Strip doc now flags that the variant implicitly
  also forces a non-null content field on assistant turns with empty
  text_parts. The two DeepSeek R1 quirks have always co-occurred so they
  share one knob; #[non_exhaustive] keeps room for a split if a future
  provider needs only one.

- effective_reasoning_echo_policy doc now records the bridge-stage
  limitation: an explicit None from the catalog cannot suppress the
  substring fallback, so a registry author can't currently say "this
  kimi-named model genuinely has no special handling". Goes away when
  the fallback path is removed.

- test_catalog_empty_string_policy_overrides_unmatched_substring used
  "mystery-kimi-clone", which contains "kimi" and therefore matches
  the substring fallback — the test passed by coincidence (catalog
  output happened to agree with fallback) rather than by proving
  the override path. Renamed to "mystery-multi-turn-clone" so the
  fallback genuinely can't produce the Kimi wire shape; the override
  is now the only code path that can make the assertions hold.

* feat(kernel-handle,kernel,runtime): wire agent_loop main path to catalog ReasoningEchoPolicy

Refs #4842. Builds on top of the previous commit (catalog data model
+ driver dispatch). Wires the main LLM-call hot path so it actually
reads the policy from the catalog instead of always falling through
to substring detection.

Changes:

* `librefang-kernel-handle`: new `CatalogQuery` role trait with one
  method `reasoning_echo_policy_for(model: &str) -> ReasoningEchoPolicy`,
  default impl returns `None`. Added to the `KernelHandle` supertrait
  alias and the prelude. `StubKernel` test mock gets an empty impl
  alongside the other 18 role traits.

* `librefang-kernel`: new `kernel/handles/catalog_query.rs` provides
  the real impl on `LibreFangKernel` — looks up the model in the
  catalog and returns the entry's `reasoning_echo_policy`, defaulting
  to `None` on catalog miss so the driver fallback still applies.

* `librefang-runtime/agent_loop.rs`: both `run_agent_loop` and
  `run_agent_loop_streaming` now query
  `kernel.reasoning_echo_policy_for(&api_model)` on each turn and
  pass the result through `CompletionRequest.reasoning_echo_policy`.
  `kernel: Option<Arc<dyn KernelHandle>>` was already in scope.

* `librefang-runtime-wasm` + `tool_runner` + 3 integration test
  fixtures: empty `impl CatalogQuery for X {}` on test mocks
  (`RecordingKernel`, `ApprovalKernel`, `ForceHumanCapturingKernel`,
  `NamedWsKernel`, `SpawnCheckKernel`, `CapturingKernel`) so the
  blanket `KernelHandle` impl is satisfied. All defer to the trait
  default (= `None`) since none of these mocks need real catalog
  behaviour.

Auxiliary `CompletionRequest` construction sites (compactor,
history_fold, proactive_memory, aux_client, kernel internals) still
build with `Default::default()` and continue to rely on the driver's
substring fallback. Wiring those will land in a separate commit
(this change keeps the diff focused on the main hot path).

Tests added (1 new, all passing):
  librefang-kernel-handle
    catalog_query_default_returns_none — verifies StubKernel and any
    mock that doesn't override the trait method gets `None`, which
    is the contract drivers rely on to fall back safely.

Verification:
  * cargo check --workspace --all-targets — clean
  * cargo clippy --workspace --all-targets -- -D warnings — clean
  * cargo fmt --all --check — clean
  * cargo test -p librefang-kernel-handle --lib — 4 pass
  * cargo test -p librefang-llm-drivers --lib — 477 pass
  * cargo test -p librefang-types --lib — 799 pass

* refactor(llm-driver): derive Default on CompletionRequest for cheap future field additions

Review feedback #4 on #4863. The PR has ~30 construction sites all
explicitly setting `reasoning_echo_policy: ReasoningEchoPolicy::default()`.
A `#[derive(Default)]` on `CompletionRequest` makes the next struct
field addition a one-line job (no need to revisit every site again),
and lets new call sites use struct-update syntax when only a few
fields differ from the zero values.

Existing call sites are left untouched in this PR — refactoring 30+
hand-written constructions to `..Default::default()` is a separate,
larger diff with its own review surface. The derive is purely
additive: existing field-by-field construction still works unchanged.

The default value is *not* a usable request — `model` is empty and
`messages` is empty — so this can't accidentally produce a valid LLM
call. Doc-comment on the struct calls that out.

Verification:
  * cargo check -p librefang-llm-driver --lib — clean
  * cargo clippy --workspace --all-targets -- -D warnings — clean

* test(llm-drivers): cover Strip policy's force-nonnull-content branch (#4842)

The existing test_catalog_strip_policy_overrides_unmatched_substring
fixture has tool_calls on the assistant turn, so the content-forcing
branch routes through has_tool_calls and the Strip-specific
force_nonnull_content path is never exercised.

Add test_catalog_strip_policy_forces_nonnull_content_without_tool_calls
which uses a thinking-only assistant message followed by a user message
(non-trailing, so strip_trailing_empty_assistant doesn't pop it). Under
default policy the historical assistant gets content: None; under Strip
it gets content: Some(Text("")). Pinning both directions proves the
Strip-specific branch is the only thing that can flip it.

* feat(kernel,runtime): wire all CompletionRequest sites to catalog ReasoningEchoPolicy (#4842)

Closes the bridge-stage limitations called out in PR #4863 review:
auxiliary CompletionRequest sites still hard-coded
`ReasoningEchoPolicy::default()`, leaving the substring fallback as
the only thing carrying the deepseek-r1 / v4-flash / kimi wire shape
on cron compaction, history fold, web search, proactive memory
extraction, etc. Now every production site reads from the model
catalog at request-build time.

Kernel-side sites (LibreFangKernel has direct catalog access):
- agent_execution: complexity-probe request
- agent_runtime: `compact_session` call
- assistant_routing: routed-decision request
- cron_compaction: `try_summarize_trim` (param threaded)
- cron_tick: per-tick lookup before `try_summarize_trim`
- skill_workshop::llm_review: review_candidate (param threaded)
- tools_and_skills: tool-review request (via kernel_weak.upgrade())
- triggers_and_workflow: session-label-gen + classify-complexity

Runtime-side sites (driver-facing aux tasks):
- agent_loop: maybe_fold_stale_tool_results (both run_agent_loop and
  run_agent_loop_streaming now look up via kernel before fold)
- agent_loop: web_search_augment + generate_search_queries (param
  threaded, lookup at agent_loop call site)
- compactor: compact_session / compact_messages / summarize_messages /
  summarize_in_chunks (param threaded all the way down)
- history_fold: fold_stale_tool_results / summarise_group (param
  threaded)
- proactive_memory: LlmMemoryExtractor revives install_kernel_handle
  to actually store a Weak<KernelHandle>; extract_memories +
  decide_action look up via .echo_policy() at request-build time

Catalog-less callers (ContextCompressor, DefaultContextEngine) keep
passing `None` with a comment — they don't have a kernel ref, and the
OpenAI driver's substring fallback handles them.

The substring fallback in `fallback_reasoning_echo_policy` stays:
first boot may still have an empty catalog and users can register
custom models in `~/.librefang/model_catalog.toml` without setting
the field. Catalog data lands via librefang-registry#90.

Helper: `LibreFangKernel::lookup_reasoning_echo_policy` is an
inherent mirror of the trait method so kernel-internal call sites can
dispatch without bringing the `CatalogQuery` trait into scope.

* refactor(runtime): bundle fold knobs into FoldConfig to clear clippy too_many_arguments

Adding reasoning_echo_policy pushed maybe_fold_stale_tool_results to 8
args and fold_stale_tool_results to 7, tripping
`clippy::too_many_arguments`. Bundle fold_after_turns + min_batch_size
into a small FoldConfig struct so both signatures drop back under the
cap and future dispatch fields (provider pin, retry policy, ...) can
land without dragging the lint counter along.

* docs(runtime): restore install_kernel_handle no-op history in doc comment

#4842 repurposed install_kernel_handle from a no-op (kept for backwards
compatibility with kernel init which still calls it on every extractor)
to actually storing the handle. The new doc dropped the historical
framing; restore it so a reader doing git blame on the call sites
understands what shifted at which commit.
houko added a commit that referenced this pull request May 20, 2026
Delete the audit doc + INDEX update per the #5265 convention:
narrative 119→118, Critical 7→6, Total 120→119, drop the bullet
under Critical, and drop the audit-export-401 leg from Pass 1
priority #4 (the entry was a combined 'list-sessions + audit-export'
line; list-sessions is addressed by a separate PR so the
audit-export-401 half is removed in isolation here).
houko added a commit that referenced this pull request May 20, 2026
Delete the audit doc + INDEX update per the #5265 convention:
narrative 119→118, Critical 7→6, Total 120→119, drop the bullet
under Critical, and drop the list-sessions-decode-on-poll leg
from Pass 1 priority #4 (paired with audit-export-401; the other
leg is addressed by a separate PR so this PR only removes its own
half).
houko added a commit that referenced this pull request May 20, 2026
Delete the audit doc + INDEX update per the #5265 convention:
narrative 119→118, Critical 7→6, Total 120→119, drop the bullet
under Critical, and drop the audit-export-401 leg from Pass 1
priority #4 (the entry was a combined 'list-sessions + audit-export'
line; list-sessions is addressed by a separate PR so the
audit-export-401 half is removed in isolation here).
houko added a commit that referenced this pull request May 20, 2026
Delete the audit doc + INDEX update per the #5265 convention:
narrative 119→118, Critical 7→6, Total 120→119, drop the bullet
under Critical, and drop the list-sessions-decode-on-poll leg
from Pass 1 priority #4 (paired with audit-export-401; the other
leg is addressed by a separate PR so this PR only removes its own
half).
houko pushed a commit that referenced this pull request May 20, 2026
…irect tests

PR self-review caught four issues; this commit fixes all of them.

Fix #1 + #2: SeenSet docstring corrections
==========================================

The old docstring claimed two things that weren't true:

* "the adapter classes keep those names available as @Property
  shims pointing at this class's ``ids`` / ``order`` attributes" —
  there are no @Property shims; tests were rewritten to read
  ``adapter._seen.ids`` directly.
* Listed bluesky as a SeenSet consumer — bluesky's ``_mark_seen``
  is a server-side ``updateSeen`` REST POST, NOT a dedupe shim;
  the name collision is coincidental.

Both claims fixed. Webex's empty-id → False quirk is now also
called out explicitly so future readers don't think the shared
behaviour is universal.

Fix #3: direct unit tests for the shared modules
================================================

Added three new test files:

* ``tests/test_common.py`` (40 tests) — covers split_message
  (passthrough, exact-limit, hard-cut, newline-prefer, empty,
  unicode), split_csv (empty, whitespace, dropped empties,
  order), parse_retry_after (missing, integer, decimal, floor,
  custom floor, max-cap, garbage, negative), SeenSet (fresh,
  repeat, distinct ids, empty-id, eviction, contains, len,
  thread safety with 8 concurrent workers, int ids), and
  http_request (200 JSON, request metadata recording, lowercased
  response headers, 4xx/5xx surfaced via status not raise, empty
  body, non-JSON, default timeout, default method).
* ``tests/test_ws.py`` (13 tests) — RFC 6455 constants
  (WS_GUID, opcodes, MAX_FRAME_PAYLOAD), ``_parse_url`` (wss,
  ws-with-port, default-path, query-string preservation, non-ws
  reject, missing-host reject), and ``WebSocketClient.__init__``
  defaults. Full socket-level coverage (handshake, frame
  round-trip) needs a real local WS server fixture that's out of
  scope for this refactor PR.
* ``tests/test_sidecar_fakes.py`` (15 tests) — HdrShim, FakeResp
  CM protocol, FakeUrlopen script handling, call-metadata
  recording (URL/method/timeout/headers/body), JSON + form-encoded
  body decoding, 3-tuple response-headers variant, 4xx/5xx
  HTTPError raising, script-exhaustion AssertionError, empty/bytes
  body passthrough, back-compat underscore aliases.

Fix #4: actually hoist MAX_BACKOFF_SECS / RETRY_AFTER_DEFAULT_SECS
=================================================================

The previous round-2 commit added these constants to
``librefang.sidecar.common`` and the commit message claimed every
adapter would pick up the canonical values from there. **None did**
— all 16 adapters that used them kept their local definitions
unchanged, so the hoist was theoretical.

This commit actually rewires the imports. Each of the 16
adapters that referenced ``MAX_BACKOFF_SECS = 60.0`` now imports
it from ``common``; 11 adapters likewise for
``RETRY_AFTER_DEFAULT_SECS = 30.0``. Local duplicate definitions
are removed.

A handful of adapters have local overrides that aren't ``60.0``
or ``30.0`` (e.g. webex's stricter ``RETRY_AFTER_DEFAULT_SECS = 30.0``
matches but it has a 30-second cap not 60; mastodon's
``RETRY_AFTER_DEFAULT_SECS = 60.0`` is also canonical but different
semantics) — those are intentionally left as local strategy knobs.

Verification
============

``pytest sdk/python/tests/`` — **1095 passed in 5.05s**
(was 1027 in 5.00s; the new test files add 68).

Diff: 20 files changed, +786 / -105.
houko added a commit that referenced this pull request May 22, 2026
…ad (audit: audit-export-401) (#5324)

* fix(dashboard/audit): use canonical getStoredApiKey for export download (audit: audit-export-401)

downloadExport in AuditPage.tsx:110 read the bearer token via
safeStorageGet('librefang-api-key'), which only checks localStorage.
But the credential layer was migrated in #3620 to prefer
sessionStorage and explicitly WIPE localStorage on save — so the
inline accessor always returned '', sent Authorization: Bearer
(empty), and every Export click 401'd for every signed-in user
since that release.

Drop the inline accessor and route through getStoredApiKey() from
api.ts (the same one authHeader() and every other token consumer
goes through — it checks sessionStorage first and falls back to
localStorage for legacy tokens).

Other safeStorageGet sites in pages/ were audited:
- TerminalPage.tsx (terminal.fontSize + hint keys) — different key
- ChatPage.tsx (librefang.chat.show_hand_agents) — different key
None target librefang-api-key, so no further changes needed.

Verified: pnpm typecheck + pnpm build clean.

* docs(issues): retire audit-export-401 (closed by #5324)

Delete the audit doc + INDEX update per the #5265 convention:
narrative 119→118, Critical 7→6, Total 120→119, drop the bullet
under Critical, and drop the audit-export-401 leg from Pass 1
priority #4 (the entry was a combined 'list-sessions + audit-export'
line; list-sessions is addressed by a separate PR so the
audit-export-401 half is removed in isolation here).

* docs(changelog): fix stale api.ts line numbers (3341-3342 → 3595-3596)

* chore(codegen): auto-regenerate openapi.json + sdk + schema baselines [skip ci]

* chore(codegen): auto-regenerate openapi.json + sdk + schema baselines [skip ci]

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
houko added a commit that referenced this pull request May 22, 2026
…ist-sessions-decode-on-poll) (#5326)

* perf(api/config): use count_sessions() instead of list_sessions().len() on status + snapshot (audit: list-sessions-decode-on-poll)

Two routes — /api/status and /api/dashboard/snapshot — computed
session_count via substrate.list_sessions().map(|s| s.len()).
list_sessions() returns Vec<serde_json::Value> with each row's
full rmp-encoded message history decoded, so calling .len() on
the vec required decoding every blob just to count rows.

The dashboard polls /api/dashboard/snapshot every 5s with
refetchIntervalInBackground=false, so every foreground operator
hits it continuously. At 100 sessions × 200 KB each, the daemon
burnt ~20 MB/s of needless decode + rmp alloc on the hot path —
for what is morphologically a SELECT COUNT(*).

MemorySubstrate::count_sessions() (indexed SELECT COUNT(*)) already
exists in librefang-memory/src/substrate.rs:391; both call sites
now use it.

* docs(issues): retire list-sessions-decode-on-poll (closed by #5326)

Delete the audit doc + INDEX update per the #5265 convention:
narrative 119→118, Critical 7→6, Total 120→119, drop the bullet
under Critical, and drop the list-sessions-decode-on-poll leg
from Pass 1 priority #4 (paired with audit-export-401; the other
leg is addressed by a separate PR so this PR only removes its own
half).

* test(api): pin /api/status + /api/dashboard/snapshot session_count

Two #[tokio::test] cases against TestServer that seed N sessions via
the memory substrate and assert the wire field reflects N — catches
the .unwrap_or(0) silent-pin regression class that the dashboard's
existing smoke test (only checks status='running') would miss.
Required by CLAUDE.md route-test policy and the audit doc.

Also: clarify the comment + CHANGELOG figure to '~20 MB per poll
(≈ 4 MB/s)' — the original ~20 MB/s number was off by the 5 s poll
period.

* chore(codegen): auto-regenerate openapi.json + sdk + schema baselines [skip ci]

* chore(codegen): auto-regenerate openapi.json + sdk + schema baselines [skip ci]

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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
… + 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`)
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant