🔍 Issue Description

While looking at PR #1182, I noticed the fix only touched 4 files in mofa-kernel and mofa-runtime. After running a quick grep across the whole workspace I found 124 more places doing the exact same unsafe cast pattern that #1182 was supposed to fix. A few of them are genuinely buggy — not just a style thing — so I wanted to flag it.

📌 Issue Type

• Bug
• Feature Request
• Enhancement
• Documentation
• Refactor
• Other (please specify)

📝 Description

How I found this

After #1182 merged I ran:

grep -rn "timestamp_millis() as u64\|\.as_millis() as u64\|num_milliseconds() as u64" \
  --include="*.rs" crates/ tests/ examples/ | grep -v target/

Got 124 hits across 8 crates that weren't touched. I split them into three groups based on risk:

Group 1 — timestamp_millis() as u64 · 5 lines · actually buggy

chrono::DateTime::timestamp_millis() returns i64. If the machine clock is before the Unix epoch (happens more often than you'd think — Docker containers, CI VMs with misconfigured time, NTP backward jumps), that i64 is negative. Casting it to u64 wraps it around to something near u64::MAX, which completely corrupts any audit trail or telemetry timestamp stored with it.

Lines I found:

• crates/mofa-foundation/src/workflow/executor.rs — lines 187 and 200 (inside create_review_context)
• crates/mofa-foundation/src/agent/components/tool.rs — line 336
• crates/mofa-kernel/src/hitl/audit.rs — line 75 (inside ReviewAuditEvent::new)
• examples/workflow_viz/src/main.rs — line 567

Group 2 — num_milliseconds() as u64 · 1 line · actually buggy

This one felt the worst to me. In workflow/executor.rs line 209:

now.signed_duration_since(paused_at).num_milliseconds() as u64

signed_duration_since returns a signed duration — it's negative if paused_at happens to be in the future (e.g. NTP synced the clock forward while a workflow was paused). num_milliseconds() returns i64. Casting that negative value to u64 makes total_wait_time_ms explode to ~u64::MAX, which would totally break any HITL wait-time analytics built on top.

Simple fix:

u64::try_from(
    now.signed_duration_since(paused_at).num_milliseconds().max(0)
).unwrap_or(0)

Group 3 — as_millis() as u64 · 118 lines · coding standard violation

These come from Instant::elapsed().as_millis() or Duration::as_millis(), which return u128. Casting directly to u64 is what §VII.2 explicitly bans:

"All numeric conversions must explicitly handle overflow cases. Silent wrapping via 'as' casts is prohibited for conversions that may lose data or change sign."

In practice these won't overflow until the year 584,554 AD so there's no immediate correctness risk, but it's the same pattern #1182 was fixing and it's spread everywhere.

Per crate:

🎯 Proposed Solution

I'd split this into two PRs to keep things reviewable:

PR 1 — the actual bugs (6 lines changed):

• Add pub fn chrono_now_ms() -> u64 to mofa-kernel/src/utils.rs (mirrors the existing now_ms() but using chrono).
• Swap the 5 Group 1 sites to use mofa_kernel::utils::chrono_now_ms().
• Fix the Group 2 site with the .max(0) clamp.

PR 2 — the standards sweep (118 lines):

• Mechanically replace as_millis() as u64 with u64::try_from(…).unwrap_or(u64::MAX) everywhere, matching the style from fix: replace unsafe as u64 casts with u64::try_from in timestamp conversions #1182.

📎 Additional Context

• [REFACTOR] mofa-kernel: refactor timestamp boilerplate and fix lossy u128 to u64 cast #394 — original proposal to centralize timestamp boilerplate (closed)
• refactor(kernel): centralize timestamp logic and fix lossy u128 cast #395 — added utils::now_ms() (merged)
• [Bug] Correctness: Unsafe u64 -> i64 casts in ExponentialWithJitter jitter calculation #1172 — similar unsafe cast issue in ExponentialWithJitter (still open)
• fix: replace unsafe as u64 casts with u64::try_from in timestamp conversions #1182 — the partial fix that prompted this (merged Mar 13, only covered 4 files)

🙋 Claiming This Issue

• I'm willing to solve this issue by myself