Skip to content

fix(cli): render rule-history timestamps as dates, not a day count - #4089

Closed
udsy19 wants to merge 1 commit into
NVIDIA:mainfrom
udsy19:fix/rule-history-timestamp
Closed

udsy19 wants to merge 1 commit into
NVIDIA:mainfrom
udsy19:fix/rule-history-timestamp

Conversation

@udsy19

@udsy19 udsy19 commented Oct 1, 2026

Copy link
Copy Markdown

Summary

In the sandbox Rule History view, each entry's time renders as a days-since-1970 count (e.g. 19675d 22:13) instead of a wall-clock date.

Root cause

sandbox_draft_history (crates/openshell-cli/src/run.rs) formats each entry's event_time with format_timestamp_ms (crates/openshell-cli/src/commands/common.rs):

let days = secs / 86400;
if days > 0 {
    format!("{days}d {hours:02}:{mins:02}")
} else { ... }

But event_time is an absolute google.protobuf.Timestamp (epoch ms, via proto_timestamp_ms → timestamp_to_millis), not an elapsed duration — so days is ~19,675 and every entry prints garbage like 19675d 22:13. format_timestamp_ms was the only helper in the crate that treated an absolute timestamp as a duration; the TUI's format_timestamp and the CLI's format_epoch_ms/format_optional_epoch_ms all correctly render epoch-ms as YYYY-MM-DD HH:MM[:SS], and this was its sole caller.

Fix

Render the absolute timestamp with the existing format_optional_epoch_ms (same helper provider.rs uses for event times), which also preserves the - placeholder for a missing/zero timestamp. The mis-implemented format_timestamp_ms helper is removed (no remaining references).

Tests

Added a regression test in common.rs: format_optional_epoch_ms(1_700_000_000_000) == "2023-11-14 22:13:20" and format_optional_epoch_ms(0) == "-". Verified with cargo test -p openshell-cli --lib (fails on the old helper with 19675d 22:13, passes after).

The sandbox rule-history view rendered each entry's event_time with
format_timestamp_ms, which divided the epoch milliseconds by 86_400_000
to produce a days component. Because event_time is an absolute
google.protobuf.Timestamp, that days component was days-since-1970, so
every entry printed a nonsensical count such as "19675d 22:13" instead
of a wall-clock date.

Drop the buggy helper and format the timestamp with the existing
format_optional_epoch_ms, matching how provider history and other
absolute-time columns are rendered (and preserving the "-" placeholder
for a missing/zero timestamp).

Signed-off-by: Udaya Tejas <udayatejas2004@gmail.com>
@udsy19
udsy19 requested review from a team, derekwaynecarr, mrunalp and sjenning as code owners October 1, 2026 23:29
@copy-pr-bot

copy-pr-bot Bot commented Oct 1, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown

Thank you for your interest in contributing to OpenShell, @udsy19.

This project uses a vouch system for first-time contributors. Before submitting a pull request, you need to be vouched by a maintainer.

To get vouched:

  1. Open a Vouch Request discussion.
  2. Describe what you want to change and why.
  3. Write in your own words — do not have an AI generate the request.
  4. A maintainer will comment /vouch if approved.
  5. Once vouched, open a new PR (preferred) or reopen this one after a few minutes.

See CONTRIBUTING.md for details.

@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown

Thank you for your submission! We ask that you sign our Developer Certificate of Origin before we can accept your contribution. You can sign the DCO by adding a comment below using this text:


I have read the DCO document and I hereby sign the DCO.


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the DCO Assistant Lite bot.

@github-actions github-actions Bot closed this Oct 1, 2026
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