feat(runtime): cache_hit_ratio metric + trajectory field (M2/2)#3149
Merged
Conversation
Builds on PR-1 (#3126, the system_and_3 stamping). Adds the single-value observability metric the dashboard surfaces so users can see at a glance whether prompt caching is paying off for their workload. Pure observability — no behaviour change. `cache_hit_ratio = cache_read / (cache_read + cache_creation)`, in `[0.0, 1.0]`. Returns `0.0` when both numerators are zero (caching not active for this turn / model doesn't support it). Wire-up: - `crates/librefang-runtime/src/agent_loop.rs`: new `cache_hit_ratio` helper alongside `accumulate_token_usage`. Per-turn `tracing::info!` on `librefang::cache` target with agent / hit_ratio / creation / read so existing log pipelines (Tempo, Grafana) pick it up without dashboard changes. - `crates/librefang-kernel/src/trajectory/mod.rs`: `TrajectoryMetadata` grows `cache_hit_ratio: Option<f32>` (`#[serde(default, skip_serializing_if = "Option::is_none")]`) so legacy trajectories deserialise to None and replays don't need to know about caching to round-trip. Tests (6 new): - cache_hit_ratio_zero_when_no_caching (both 0) - cache_hit_ratio_full_hit (read=100, creation=0 → 1.0) - cache_hit_ratio_partial (read=70, creation=30 → 0.7) - cache_hit_ratio_cold_start (read=0, creation=100 → 0.0) - trajectory_metadata_cache_hit_ratio_serde_round_trip - trajectory_metadata_cache_hit_ratio_legacy_compat (legacy JSON without the field deserialises to None, no panic) Companion fix: `scheduler.rs:555` baseline `clippy::manual_pattern_char_comparison` (`s.find(|c: char| c == '/' || c == '?' || c == '#')` → `s.find(['/', '?', '#'])`). Same one-line fix is also queued in PR #3144 / #3147; whichever lands first the others rebase cleanly (identical content). Verification: - cargo test -p librefang-runtime --lib agent_loop: 167 ok - cargo test -p librefang-kernel --lib trajectory: 14 ok - cargo clippy -p librefang-runtime -p librefang-kernel --all-targets -- -D warnings: clean Stack: independent, base main. Closes the prompt-caching-system-and-3 plan — system_and_3 stamping (PR-1) plus this PR cover the full "borrow hermes-agent's cache strategy" scope.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
houko
added a commit
that referenced
this pull request
Apr 25, 2026
… fix) Closes a high-severity gap from PR #3145 review: `PreScript.env` was unvalidated, so an attacker-controlled cron config could set `LD_PRELOAD=/tmp/x.so` (or the Darwin equivalent `DYLD_INSERT_LIBRARIES`, or `PATH=/tmp/evil:$PATH`, or `IFS=...`) and fully bypass the path allowlist on `argv[0]`. The "scripts under ~/.librefang/scripts only" promise is moot if the spawned binary gets hijacked at link / shell-parse time. Changes: - New `DANGEROUS_ENV_KEYS` const listing 8 dynamic-linker / PATH / IFS knobs that we refuse to honor in `PreScript.env`. Match is case-insensitive (Windows env keys are case-insensitive natively; defence-in-depth elsewhere). - New `PreScriptValidationError::DangerousEnvKey { key }` variant. - `validate_pre_script` rejects before any file IO so the cheap check fails first. - Hooked into the `CronJob::validate_action` path so config-load rejects bad pre_scripts even before the dispatcher runs them (review medium issue: `validate_pre_script` was previously dead code outside its own unit tests). Companion: same `scheduler.rs` host-parse `clippy::manual_pattern_char_comparison` fix as in #3144 / #3147 / #3149 (the line moved from :555 to :785 once the denylist + tests landed; identical 3-char fix). Whichever PR lands first the others rebase cleanly. Tests (5 new in `scheduler::tests`): - pre_script_validation_rejects_ld_preload - pre_script_validation_rejects_path_override - pre_script_validation_rejects_dyld_insert - pre_script_validation_rejects_dangerous_env_case_insensitive - pre_script_validation_accepts_safe_env_keys Verification: - cargo test -p librefang-types --lib scheduler: 81 ok - cargo clippy -p librefang-types -p librefang-kernel --all-targets -- -D warnings: clean
4 tasks
houko
added a commit
that referenced
this pull request
Apr 25, 2026
… fix) Closes a high-severity gap from PR #3145 review: `PreScript.env` was unvalidated, so an attacker-controlled cron config could set `LD_PRELOAD=/tmp/x.so` (or the Darwin equivalent `DYLD_INSERT_LIBRARIES`, or `PATH=/tmp/evil:$PATH`, or `IFS=...`) and fully bypass the path allowlist on `argv[0]`. The "scripts under ~/.librefang/scripts only" promise is moot if the spawned binary gets hijacked at link / shell-parse time. Changes: - New `DANGEROUS_ENV_KEYS` const listing 8 dynamic-linker / PATH / IFS knobs that we refuse to honor in `PreScript.env`. Match is case-insensitive (Windows env keys are case-insensitive natively; defence-in-depth elsewhere). - New `PreScriptValidationError::DangerousEnvKey { key }` variant. - `validate_pre_script` rejects before any file IO so the cheap check fails first. - Hooked into the `CronJob::validate_action` path so config-load rejects bad pre_scripts even before the dispatcher runs them (review medium issue: `validate_pre_script` was previously dead code outside its own unit tests). Companion: same `scheduler.rs` host-parse `clippy::manual_pattern_char_comparison` fix as in #3144 / #3147 / #3149 (the line moved from :555 to :785 once the denylist + tests landed; identical 3-char fix). Whichever PR lands first the others rebase cleanly. Tests (5 new in `scheduler::tests`): - pre_script_validation_rejects_ld_preload - pre_script_validation_rejects_path_override - pre_script_validation_rejects_dyld_insert - pre_script_validation_rejects_dangerous_env_case_insensitive - pre_script_validation_accepts_safe_env_keys Verification: - cargo test -p librefang-types --lib scheduler: 81 ok - cargo clippy -p librefang-types -p librefang-kernel --all-targets -- -D warnings: clean
houko
added a commit
that referenced
this pull request
Apr 25, 2026
* feat(types): cron pre_script + silent_marker schema (PR-1/3)
Adds the configuration schema for cron pre-script injection +
silent-response markers. Pure schema work — no scheduler dispatch
code reads these fields yet (M2 will wire execution; M3 will add
the dashboard editor + optional TOTP approval).
`CronAction::AgentTurn` grows two optional fields:
- `pre_script: Option<PreScript>` — when set, the M2 dispatcher
will run the argv before the scheduled prompt fires, capture
stdout, and inject it into the LLM context. Distinct from the
existing `pre_check_script` (which gates whether the agent runs
at all via `{"wakeAgent": false}`); both can coexist.
- `silent_marker: Option<String>` — when present at the end of
the response, the dispatcher suppresses delivery. Match is
"last non-empty trimmed line == marker" — strict, doesn't
trigger on mid-response mentions of `[SILENT]`. None means
delivery always fires.
`PreScript` (new struct in `librefang-types::scheduler`):
- argv: Vec<String> — split form, no shell parsing.
- cwd: Option<String> — defaults to workspace root.
- env: HashMap<String, String> — added on top of daemon env;
per-job secrets should still go through LIBREFANG_VAULT_KEY.
- `#[serde(deny_unknown_fields)]` so typos surface instead of
being silently ignored.
`validate_pre_script(script, home_dir)` enforces a path allowlist:
argv[0] must canonicalize to a file under `<home_dir>/scripts/`.
Component-level prefix comparison via `Path::starts_with` (so
`/home/X-other` doesn't match `/home/X`). `..` escapes are
rejected after canonicalize. Errors typed via
`PreScriptValidationError` (thiserror): EmptyArgv,
OutsideAllowlist { path, home_dir }, NotFound { path }.
Companion changes (mechanical, fields propagated to existing
`CronAction::AgentTurn` construction sites):
- librefang-api/src/channel_bridge.rs: 2 lines (pre_script +
silent_marker = None).
- librefang-api/src/routes/workflows.rs: 2 lines (same).
- librefang-types/Cargo.toml: 1 line (test-only addition).
Tests (8 new in `scheduler::tests`):
- pre_script_validation_accepts_path_under_scripts_dir
- pre_script_validation_rejects_outside_allowlist
- pre_script_validation_rejects_dot_dot_escape
- pre_script_validation_rejects_empty_argv
- pre_script_validation_rejects_nonexistent_path
- cron_action_agent_turn_serde_round_trip_with_pre_script
- cron_action_agent_turn_serde_compat_no_pre_script (legacy
config without the field deserialises to None)
- silent_marker_default_via_serde_skip (None doesn't serialise)
Verification:
- cargo test -p librefang-types --lib: 628 ok
- cargo check --workspace --lib: clean
- cargo clippy -p librefang-types -p librefang-api --all-targets
-- -D warnings: clean
Stack: independent, base main. PR-2 (next) will wire kernel
dispatcher: argv exec + stdout capture + prompt injection +
silent_marker check + wake-gate path-whitelist hardening.
* style: cargo fmt for scheduler.rs
* fix(types): reject dangerous env keys in PreScript validation (review fix)
Closes a high-severity gap from PR #3145 review: `PreScript.env` was
unvalidated, so an attacker-controlled cron config could set
`LD_PRELOAD=/tmp/x.so` (or the Darwin equivalent
`DYLD_INSERT_LIBRARIES`, or `PATH=/tmp/evil:$PATH`, or `IFS=...`)
and fully bypass the path allowlist on `argv[0]`. The "scripts under
~/.librefang/scripts only" promise is moot if the spawned binary
gets hijacked at link / shell-parse time.
Changes:
- New `DANGEROUS_ENV_KEYS` const listing 8 dynamic-linker / PATH /
IFS knobs that we refuse to honor in `PreScript.env`. Match is
case-insensitive (Windows env keys are case-insensitive natively;
defence-in-depth elsewhere).
- New `PreScriptValidationError::DangerousEnvKey { key }` variant.
- `validate_pre_script` rejects before any file IO so the cheap
check fails first.
- Hooked into the `CronJob::validate_action` path so config-load
rejects bad pre_scripts even before the dispatcher runs them
(review medium issue: `validate_pre_script` was previously
dead code outside its own unit tests).
Companion: same `scheduler.rs` host-parse `clippy::manual_pattern_char_comparison`
fix as in #3144 / #3147 / #3149 (the line moved from :555 to :785
once the denylist + tests landed; identical 3-char fix). Whichever
PR lands first the others rebase cleanly.
Tests (5 new in `scheduler::tests`):
- pre_script_validation_rejects_ld_preload
- pre_script_validation_rejects_path_override
- pre_script_validation_rejects_dyld_insert
- pre_script_validation_rejects_dangerous_env_case_insensitive
- pre_script_validation_accepts_safe_env_keys
Verification:
- cargo test -p librefang-types --lib scheduler: 81 ok
- cargo clippy -p librefang-types -p librefang-kernel
--all-targets -- -D warnings: clean
* style: post-rebase fmt drift
Two parallel implementations of the same cache-ratio math (one in agent_loop with f32/0.0-on-empty, one in trajectory with Option<f32>) were drifting apart in semantics. Move the authoritative implementation to TokenUsage::cache_hit_ratio() in librefang-types and have both callers delegate: - runtime: tracing log uses .unwrap_or(0.0) for the f32 field, with a comment explaining how to disambiguate "no caching" from "0% hit" via the creation/read totals. - kernel: trajectory::compute_cache_hit_ratio() becomes a thin re-export; with_cache_hit_ratio builder doc clarified that the exporter never sees TokenUsage and call-site wiring is a follow-up. Test coverage for the math is now centralized in librefang-types (4 tests), runtime drops its 4 redundant tests, kernel keeps a smoke test for the re-export plus the builder/serde/legacy-compat tests.
Merged
6 tasks
houko
added a commit
that referenced
this pull request
Apr 26, 2026
…n log tee (en + zh) - #3064 + #3149 — bundled observability stack (otel-collector / Tempo / Grafana), bring-up commands, [telemetry] config, business spans, cache_hit_ratio metric formula and where it surfaces - #3022 — `librefang start --foreground` now tees logs to ~/.librefang/logs/daemon-YYYY-MM-DD.log; same-day append; 7-day rotation pruned on daemon start
houko
added a commit
that referenced
this pull request
Apr 26, 2026
…pi / observability (#3189) * docs(providers): add Novita / SearXHG / Bedrock entries * docs(config): document [parallel_tools] section + max_history_messages * docs(scheduler): cron multi-delivery + pre_script + silent_marker * docs: live context.md, chat attachments, CLI slash commands Salvages three of the six items from the (quota-killed) misc-features agent run. Each is a focused append, no rewrite of existing content. - agent/templates: live Live Context section explains the per-turn re-read of `context.md` (#3115), the .identity/ vs root path lookup, the 32 KB cap with stale-cache fallback, and the cache_context opt-out. - integrations/api/agents: attachment-resolution table (image / PDF / text+code) covering the supported MIME / extension list and the 200 KB truncation, plus a note about adjacent-text coalescing for small chat-tuned local models (#3094). - integrations/cli/commands: 'Slash Commands in Chat' section listing the registry-backed CLI commands (/model /status /help /clear /kill /exit) and pointing at channel-side commands for the broader set (#3122 #3123). * docs: tool invoke API, trajectory export, doctor audit, CLI default-pick, SDK codegen Picks up the items the second misc-features agent didn't get to before hitting the org quota. - integrations/api/system: POST /api/tools/{name}/invoke (#3025) — fail- closed allowlist, agent_id requirement for approval-gated tools. - integrations/api/agents: GET /sessions/{id}/trajectory (#3097) — json/jsonl formats, redactor pipeline, audit use case. - operations/troubleshooting: 'Audit checks' subsection enumerating the three registered AuditChecks (#3082) — VaultKey, ApiListenAddr, ConfigTomlSchema — with what each asserts and how to extend. - configuration/providers/tools: 'CLI logins as first-class default providers' section (#3061) — detection priority claude-code > codex- cli > gemini-cli > qwen-code, override via [default_model] in config.toml. - integrations/sdk: 'Auto-generated from openapi.json' section (#3046) — scripts/codegen-sdks.py, pre-commit hook, openai-tag skip rule. * docs(zh): mirror providers / config-core / cron / features-index sections * docs(zh): mirror agent / api / cli / providers-tools / troubleshooting / sdk additions Round 2 of the zh translation pass — picks up the seven mid-priority files that the first commit didn't cover. All sections mirror the EN content from #3189 but read naturally as Chinese (terms like agent / session / fallback / NoEntry left in English where they're API surface, prose translated to 中文). - agent/templates: Live Context section, .identity/ vs root fallback, cache_context = true opt-out. - api/agents: attachment-resolution table + trajectory export endpoint (#3025 / #3094 / #3097). - api/system: POST /api/tools/{name}/invoke endpoint + fail-closed allowlist explanation. - cli/commands: 'Slash Commands in Chat' section listing CLI registry commands and pointing at channel commands. - providers/tools: 'CLI logins as first-class default providers' detection priority and override. - troubleshooting: 'Audit checks' subsection + table. - sdk: 'Auto-generated from openapi.json' section. * docs: refresh /new slash-command semantics (#3071 review fix) PR review on #3189 (houko) caught two stale references describing the pre-#3071 behavior of /new ('wipe the session' / 'clear history'). The new semantics fork off a fresh session while the previous one stays resumable on the same channel — phrasing updated in 4 places (en + zh × 2 files), plus a one-line clarifier in the slash-command roster section so the cross-reference doesn't silently inherit the old mental model: - configuration/channels: 'wipe the session' → 'fork off into a new session (the prior conversation stays resumable on the channel)' - integrations/api/realtime: '(clear history)' → 'forks into a fresh session — the prior session is preserved and remains resumable' - integrations/cli/commands roster paragraph: explicit note that /new is a fork, not a wipe. Same wording mirrored to docs/src/app/zh/. * docs(channels): 9 channel-related Apr 22-25 PRs (en + zh) Covers the channel + bridge feature wave: - #3020 per-agent ChannelOverrides — fixes the contradicting 'without modifying the agent manifest' phrasing in both en and zh. - #3077 prefix_agent_name + Signal plain-text default - #3009 Slack reactions_enabled + #3005 Feishu @mention preservation consolidated into a 'Reactions and Processing State' section - #3008 Signal media attachments - #3012 WhatsApp send_voice + dm_policy / group_policy - #3011 Webhook deliver_only mode - #2972 channel file downloads (file_download_dir / max_bytes config) * docs(config-core): provider timeout / browser CDP / cron session caps (en + zh) Three new `config.toml` sections that landed Apr 22-23: - #3004 `[provider_request_timeout_secs]` map - #2993 / #2991 `[browser].cdp_endpoint` + `cdp_auth_token_env` - #2994 `[kernel].cron_session_max_tokens` / `cron_session_max_messages` * docs(api): GET /attach + Owner Notice envelope (en + zh) - #3078 SSE attach endpoint for multi-client co-watching — describes the per-session SessionStreamHub fan-out, attacher behaviour, and the curl example. - #2965 ReplyEnvelope.owner_notice + notify_owner tool — message body field, StreamEvent variant, WhatsApp OWNER_JIDS fan-out. * docs(observability + ops): Tempo stack, cache_hit_ratio metric, daemon log tee (en + zh) - #3064 + #3149 — bundled observability stack (otel-collector / Tempo / Grafana), bring-up commands, [telemetry] config, business spans, cache_hit_ratio metric formula and where it surfaces - #3022 — `librefang start --foreground` now tees logs to ~/.librefang/logs/daemon-YYYY-MM-DD.log; same-day append; 7-day rotation pruned on daemon start * docs: hooks transform / lazy tools / mcp taint / capability detection / providers extras Final batch of catch-up docs covering the remaining Apr 22-25 user-facing features: - agent/hooks: transform_tool_result hook contract (#3003) - agent/templates: tool_search / tool_load lazy loading + lazy_tools opt-out (#3047) - integrations/mcp-a2a: TaintRuleId enum + per-tool skip_rules policy (#2999) - configuration/features: compaction summaries in user's conversation language (#3007) - configuration/providers/local: embedding auto-detection priority list (#3099) + Ollama capability detection (Modality / metadata pipeline, #3074 / #3133 / #3134 / #3140) - configuration/providers/management: custom image-gen provider via generic OpenAI-compat driver (#2998) + Moonshot file-upload helper via /v1/files (#2966) All sections mirrored to docs/src/app/zh/. * docs: address houko fix-PR audit (8 items, en + zh) PR review caught 1 stale-doc contradiction + 7 undocumented behaviour changes from the fix-PR cohort. - #3114 — configuration/core: 'api_key empty = unauthenticated' was stale; now documents the loopback-only policy (non-loopback bypass was closed) - #3170 — features/observability: auto_start is opt-in; home_dir- scoped Docker labels; RAII cleanup - #2989 — api/agents POST /message: per-request session_id override - #3048 — Interrupting a Running Agent Turn: /stop cascades into agent_send subagents (recursive) - #3056 + #3000 — desktop: dedicated Connection screen via custom URI scheme with retry / open-in-browser / uninstall - #2952 — triggers: response routes to agent's home channel - #2955 — triggers: task_posted assignee_match = 'self' | 'any' - #3069 — providers/local: 60-second reprobe + manual 'Test connection' refresh All sections mirrored in docs/src/app/zh/.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Builds on PR-1 (#3126, the
system_and_3stamping). Adds the single-value observability metric the dashboard surfaces so users can see at a glance whether prompt caching is paying off for their workload. Pure observability — no behaviour change.Some(1.0)means a perfect hit (everything was prefix-cached).Some(0.0)is a cold start where caching was active but nothing was hit yet.None(in the trajectory metadata) means caching wasn't active at all for the run — the runtime log folds this to0.0for the float field, withcreationandreadtotals alongside so readers can disambiguate.Changes
crates/librefang-types/src/message.rs:TokenUsage::cache_hit_ratio() -> Option<f32>— the authoritative implementation. Both runtime and kernel callers delegate.crates/librefang-runtime/src/agent_loop.rs:tracing::info!on thelibrefang::cachetarget withagent,hit_ratio,creation,readfields so existing log pipelines (Tempo, Grafana) pick it up without dashboard changes.hit_ratiofield uses.unwrap_or(0.0)for log-friendliness.crates/librefang-kernel/src/trajectory/mod.rs:TrajectoryMetadatagrowscache_hit_ratio: Option<f32>(#[serde(default, skip_serializing_if = "Option::is_none")]).None; replays don't need to know about caching to round-trip cleanly.TrajectoryBundle::with_cache_hit_ratio(usage)builder added for the HTTP export route / CLI exporter to stamp the bundle once they've aggregated cumulativeTokenUsagefrom the metering layer. (Wiring those call sites is a follow-up — the exporter itself doesn't see token counts.)compute_cache_hit_ratio()is a thin re-export ofTokenUsage::cache_hit_ratio()for callers already routing through this module.Tests
Authoritative coverage lives with the implementation in
librefang-types:token_usage_cache_hit_ratio_none_when_no_cachingNonetoken_usage_cache_hit_ratio_full_hitSome(1.0)token_usage_cache_hit_ratio_partialSome(0.7)token_usage_cache_hit_ratio_cold_start_returns_some_zeroSome(0.0)Plus, in
librefang-kernel:trajectory_metadata_cache_hit_ratio_serde_round_tripSome(0.85)preservedtrajectory_metadata_cache_hit_ratio_legacy_compatNone; re-serialization omitscompute_cache_hit_ratio_delegates_to_token_usagebundle_with_cache_hit_ratio_stamps_metadatametadataTest plan
cargo test -p librefang-types --lib token_usage— 8 okcargo test -p librefang-runtime --lib agent_loop— 163 okcargo test -p librefang-kernel --lib trajectory— 12 okcargo clippy -p librefang-types -p librefang-runtime -p librefang-kernel --all-targets -- -D warnings— cleanStack
Independent, base
main. Closes theprompt-caching-system-and-3plan — system_and_3 stamping (PR-1) plus this PR cover the full "borrow hermes-agent's cache strategy" scope.See
.plans/prompt-caching-system-and-3.mdfor full design.