feat(kernel): proactive V2 — event-driven proactive architecture (#786)

[crrow]

Summary

Implement event-driven proactive architecture (Proactive V2) replacing the polling-only heartbeat mechanism. Three layers:

1. Event Source — ProactiveSignal variants in KernelEvent (SessionIdle, TaskFailed, SessionCompleted, MorningGreeting, DailySummary)
2. Event Filter — Pure rule-based ProactiveFilter with quiet hours, per-signal cooldowns (session-scoped), and global hourly rate limiting. Zero LLM cost.
3. Context Pack — Structured multi-section text blocks replacing the vague heartbeat message, giving Mita actionable context for each event.

Heartbeat demoted from sole driver to fallback patrol — now also uses the structured context pack format.

Type of change

Type	Label
New feature	enhancement

Component

core

Closes

Closes #786

Test plan

• cargo check -p rara-kernel passes
• cargo clippy -p rara-kernel --all-targets --all-features --no-deps -- -D warnings passes
• cargo test -p rara-kernel passes (16 tests, including 14 proactive-specific)
• Code review: 2 rounds, 7 issues found and fixed, clean on re-review
• No TODO/FIXME in diff

[crrow]

Code Review — PR #793: Proactive V2 Event-Driven Signals

Files reviewed: 10 files, +922 / -6 lines
Overall assessment: REQUEST_CHANGES

---

Findings

P0 - Critical

(none)

P1 - High

1. crates/kernel/src/kernel.rs — compute_next_proactive_time_event has a date logic bug

   When current_time >= work_end (after work hours), the function sets target_time = work_start, is_morning = true. Then the date selection logic:

   let target_date = if !is_morning || current_time < work_start {
       today
   } else {
       today.tomorrow().ok()?
   };

   This correctly picks tomorrow. BUT when current_time < work_start (before work hours), it sets is_morning = true and target_time = work_start. The date condition evaluates !true || true → true, so it picks today — correct.

   However, when current_time < work_end (during work hours), it sets is_morning = false, target_time = work_end. The date condition evaluates !false || ... → true, picks today — correct.

   Actually, the logic works but is unnecessarily convoluted. The condition !is_morning || current_time < work_start conflates two independent cases. A clearer formulation:

   let target_date = if current_time >= work_end {
       today.tomorrow().ok()?
   } else {
       today
   };

   This is a P2 readability concern rather than a bug. Downgrading.

2. crates/kernel/src/kernel.rs:~2827 — SessionCompleted emitted for ALL non-failed child sessions, including Mita children

   The SessionCompleted signal is emitted in the has_result_tx block whenever !_turn_failed. This fires for every child/worker session that completes, including Mita's own child dispatches. This could create a feedback loop: Mita dispatches a sub-agent → sub-agent completes → SessionCompleted signal → Mita receives "session completed" → Mita possibly dispatches again.

   The cooldown/rate-limit filter mitigates this, but it still means Mita receives signals about its own orchestrated work. Consider filtering out sessions whose parent is Mita.

3. crates/kernel/src/kernel.rs — emit_idle_signals() fires on every IdleCheck tick for all idle sessions

   The IdleCheck event fires periodically. Each time, emit_idle_signals() iterates ALL sessions and emits SessionIdle for every session idle > 30 minutes. While the cooldown filter blocks duplicate signals for the same kind, it does not distinguish per-session — so if session A triggers session_idle and gets cooldown-blocked, session B's idle signal also gets blocked because they share the same kind_name() = "session_idle".

   This means only ONE idle session per cooldown window will ever produce a signal, regardless of how many sessions are idle. If that's intentional, document it. If not, the cooldown key should incorporate the session key.

P2 - Medium

4. crates/kernel/src/proactive/context.rs:61,67,73 — .unwrap() in non-test code

   Three .unwrap() calls on .last_mut() in build_context_pack. These are technically safe (the vec always has at least one element pushed at line 49), but project convention says "use .expect("context") over unwrap() in non-test code".

   Suggested fix: replace with .expect("sections is non-empty").

5. crates/kernel/src/kernel.rs — hardcoded 30-minute idle threshold

   emit_idle_signals() hardcodes Duration::from_secs(30 * 60). Project convention: "No hardcoded config defaults in Rust — all via YAML". This threshold should be a field on ProactiveConfig.

6. crates/kernel/src/proactive/config.rs — parse_time_str silently returns None on invalid config

   All parsed_* methods return Option and silently disable the feature on malformed config strings (e.g., work_hours_start: "nine_am"). There's no warning log or startup validation. A user with a typo in their config would have proactive signals silently disabled with no feedback.

   Consider adding a validate() method or logging a warning when parsing fails.

7. crates/kernel/src/proactive/filter.rs:58 — should_pass takes &mut self but only reads state (aside from maybe_reset_hourly_window)

   The should_pass doc says "Does NOT update internal state — call record_fired after", but it actually DOES mutate state via maybe_reset_hourly_window(now) on line 92. This is a doc/behavior mismatch. Either:

   • Make should_pass truly read-only and move the window reset to record_fired, or
   • Update the doc to note the window reset side effect.

8. crates/kernel/src/proactive/config.rs — ProactiveConfig re-parses timezone/time strings on every call

   parsed_timezone(), parsed_work_start(), etc. re-parse strings every invocation. In is_quiet_hours() this happens on every signal check. Consider parsing once at construction time and caching the parsed values, or at minimum the timezone (IANA lookup hits the OS tz database).

P3 - Low

9. crates/kernel/src/proactive/signal.rs — ProactiveSignal derives Serialize but not Deserialize

   Minor asymmetry. If signals ever need to be persisted/restored (e.g., for crash recovery of the filter state), Deserialize would be needed. Not blocking.

10. crates/kernel/src/proactive/context.rs:129 — truncate function could be a method or utility

    Small utility function — fine as module-private, but if other modules need truncation it should move to a shared utility.

11. crates/kernel/src/proactive/filter.rs:78 — Duration::from_secs_f64 for elapsed time comparison

    Converting jiff seconds (f64) → Duration → compare with cooldown Duration works but is slightly roundabout. Could compare elapsed_secs directly with cooldown.as_secs_f64().

12. crates/kernel/src/kernel.rs — std::sync::Mutex used in async context

    proactive_filter uses std::sync::Mutex rather than tokio::sync::Mutex. This is actually fine here since the lock is held only briefly (no .await while locked), but worth a comment explaining the choice to prevent future maintainers from "upgrading" it to tokio::sync::Mutex unnecessarily.

---

Architecture Assessment

Strengths:

• Clean separation: signal definition, config, filter, context builder are independent modules
• Rule-based filter with zero LLM cost is a sound design
• try_emit_proactive_signal() as single entry point prevents filter bypass
• Structured context packs give Mita actionable information
• AGENT.md updated with thorough invariant documentation

Concerns:

• The per-kind (not per-session) cooldown for session_idle means multi-session idle detection is effectively broken (P1 chore(deps): bump release-plz/action from 0.5.126 to 0.5.127 #3)
• Potential feedback loop from Mita child sessions emitting SessionCompleted (P1 chore(deps): bump taiki-e/install-action from 2.67.23 to 2.68.16 #2)

---

Verdict: 3 issues to fix

P1 #2 (SessionCompleted feedback loop), P1 #3 (per-kind cooldown blocking multi-session idle signals), P2 #5 (hardcoded idle threshold) should be addressed before merge. The remaining P2/P3 items can be follow-up work.

[crrow]

Fixes Applied

#	Issue	Fix	File:Line
P1-1	SessionCompleted feedback loop for Mita children	Filter out sessions whose parent is the Mita session key	kernel.rs:2825-2845
P1-2	Per-kind cooldown blocks multi-session idle detection	Added cooldown_key() method with session-scoped keys (session_idle:{session_key})	signal.rs:65-78, filter.rs:59-88
P1-3	Hardcoded 30-min idle threshold	Added idle_threshold_secs to ProactiveConfig, read from YAML	config.rs:57-59, kernel.rs:1835-1840
P2-4	.unwrap() in non-test code	Replaced with .expect("sections is non-empty")	context.rs:61,67,73
P2-5	Silent None on invalid config	Added warn! logs for invalid timezone, work_hours_start/end	config.rs:63-90
P2-6	should_pass doc inaccuracy	Updated doc to note hourly window may be reset as side effect	filter.rs:53-58
P3-11	std::sync::Mutex without rationale	Added comment explaining why non-async Mutex is correct here	kernel.rs:198-199

[crrow]

Code Review — Re-review after fixes (PR #793)

Files reviewed: 10 files, +995 / -6 lines
Overall assessment: APPROVE

---

Verification of Previously Reported Issues

#	Issue	Status
P1	SessionCompleted feedback loop for Mita children	Fixed — is_mita_child check at kernel.rs:2846-2849 correctly filters out Mita-spawned agents
P1	Per-kind cooldown blocks multi-session	Fixed — cooldown_key() in signal.rs:69-75 appends session key for SessionIdle and SessionCompleted
P1	Hardcoded idle threshold	Fixed — idle_threshold_secs is now a field on ProactiveConfig, loaded from YAML
P2	.unwrap() usage	Fixed — all .unwrap() removed from proactive module; .expect("sections is non-empty") used with context in context.rs
P2	Silent config errors	Fixed — warn!() logs added in parsed_work_start(), parsed_work_end(), and parsed_timezone()
P2	Doc inaccuracy	Fixed — AGENT.md and doc comments are accurate
P3	Mutex rationale	Fixed — comment at kernel.rs:198-199 explains why std::sync::Mutex is used over tokio::sync::Mutex

All 7 previously identified issues are correctly resolved.

---

New Findings

P0 — Critical

(none)

P1 — High

(none)

P2 — Medium

(none)

P3 — Low

1. [proactive/config.rs:91-102] parsed_quiet_start() / parsed_quiet_end() silently return None on invalid quiet-hour strings, unlike parsed_work_start() / parsed_work_end() which log a warning. Minor inconsistency — the effect is safe (quiet hours disabled) but a warn!() here would help operators debug misconfiguration. Non-blocking.

2. [proactive/config.rs:39] quiet_hours uses Option<(String, String)> — a tuple makes it possible to accidentally swap start/end. A tiny struct QuietHoursRange { start: String, end: String } would be more self-documenting. Very minor, non-blocking.

---

Architecture Assessment

• SRP: Clean separation — signal types, config, filter logic, and context building are each in their own file. The mod.rs is re-exports only. Good.
• OCP: New signal kinds can be added by extending the ProactiveSignal enum + adding a kind_name() arm. The filter is kind-agnostic. Good.
• DIP: ProactiveFilter depends on ProactiveConfig (data struct), not on kernel internals. Signal emission goes through a single try_emit_proactive_signal() entry point. Good.
• Error handling: Config parse failures degrade gracefully (feature disabled). Mutex poison is recovered with unwrap_or_else(|e| e.into_inner()). Event queue push failures are logged. Good.
• Security: No user input flows into signals untrusted — TaskFailed error and SessionCompleted summary are truncated. Mita child check prevents feedback amplification. Good.
• Performance: Filter checks are O(1) lookups (HashMap + counter). std::sync::Mutex is appropriate for brief non-async critical sections. compute_next_proactive_time_event() runs only at startup and after each time event fires. Good.
• Convention compliance: ProactiveConfig uses bon::Builder + Deserialize, no Default. All pub items have English doc comments. jiff for time handling. No wildcard imports. Compliant.

---

Verdict: Clean

Two P3 cosmetic suggestions, neither blocking. The previously reported issues are all properly fixed. The implementation is well-structured with clear separation of concerns and appropriate safety guardrails.

[crrow]

Round 2 Fixes Applied

#	Severity	Issue	Fix
P1-1	High	_turn_failed prefix implies unused	Renamed to turn_failed across all 7 occurrences
P1-2	High	handle_proactive_signal passes None for SessionContext	Builds SessionContext from process table (manifest name, idle duration) for session-scoped signals
P2-3	Medium	parsed_* methods re-parse strings on every call	Cached parsed timezone, quiet_start, quiet_end in ProactiveFilter::new()
P2-4	Medium	last_fired HashMap unbounded growth	Evicts stale entries (>2× max cooldown) during hourly window reset
P2-5	Medium	SessionKey::deterministic("mita") recomputed per iteration	Hoisted outside emit_idle_signals loop
P2-6	Medium	Duplicate truncate() in context.rs and judgment.rs	Shared via proactive/mod.rs pub(crate) fn truncate()
P2-7	Medium	No startup validation for config values	Added ProactiveConfig::validate(), called in ProactiveFilter::new()
P3-8	Low	build_heartbeat_context_pack long format string	Refactored to sections style + shared AVAILABLE_ACTIONS constant
P3-9	Low	Missing Deserialize on ProactiveSignal	Acknowledged, deferred — no current need for deserialization
P3-10	Low	Random session key for proactive events	Confirmed expected — matches mita_heartbeat() pattern; will revisit if event queue sharding is added

Design change: Embedded session_key directly into SessionIdle and SessionCompleted signal variants, removing the separate session_key: Option<&str> parameter from should_pass() and record_fired(). This is cleaner — the signal carries its own identity.

[crrow]

SessionCompleted 信号的触发时机有问题

当前 SessionCompleted 在 has_result_tx 分支（worker/child session turn 完成时）触发，但这不符合"对话自然结束"的语义。

问题：普通用户对话结束时不会触发 SessionCompleted，只有子 session 完成才会。

建议：SessionCompleted 应该基于用户沉默时间判断，和 SessionIdle 统一在 emit_idle_signals 中处理：

• SessionCompleted — 用户 10 分钟没说话（session_completed_secs: 600），对话视为结束，Mita 可做总结/归档
• SessionIdle — 用户 1 小时+ 没说话（idle_threshold_secs: 3600），长时间空闲，Mita 可提醒/跟进

具体改动：

1. 删除 has_result_tx 分支里的 SessionCompleted emit 逻辑
2. 在 ProactiveConfig 中新增 session_completed_secs: u64（默认 600）
3. 在 emit_idle_signals 中同时检查两个阈值，按 last_activity 统一判断
4. cooldown 各自独立（session_completed 和 session_idle 分别有自己的 cooldown key）