Repository navigation
fix(daemon): align ps session selection with inventory - #1186
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to No actionable issue remains in the reviewed change; merge after the normal checks complete. Pre-merge checks |
|
|
Thanks, @XTLine. After a backward wall-clock step, One change requested, to uptime:
After your push, we'll review the change, and then there's a final maintainer check before merge. |
openrig-review
left a comment
There was a problem hiding this comment.
Approved at 1077fa0. The rig ps summary now counts the same running registrations as node inventory and takes uptime from that same set, so after a backward wall-clock step the summary agrees with inventory instead of reading stopped or counting a missing replacement. Wherever the two session orders agree, the count matches main; a confirmed-missing latest session no longer adds uptime, so uptime agrees with the count and status. Required CI passed at this head, on current main.
— dev60-planner@v-openrig-build
|
Merged to main, thanks @XTLine. It will ship in the next release. Thanks too for turning the uptime change around within the hour. |
What a user gets
rig psnow uses the same current, effectively running registrations for its running count and uptime. After a backward wall-clock adjustment within a running daemon, the summary no longer disagrees with node inventory or subtracts a confirmed-missing replacement from a healthy sibling's count. A stopped rig has no uptime, and a missing replacement cannot make a healthy sibling's uptime older.The existing inventory and
isEffectivelyRunningpredicate supply both the count and uptime membership. One archive-filtered batch reads each node's latest-ID registration timestamp; there are no per-node database calls, new public inventory fields, identity-rule changes, migrations or dependencies.How you verified it
Current head:
1077fa0ad6b9f776cb8b56fb368c271e433b7c06, tree-identical to tested local commit8f8698379c16ada97246357f045ed37955c39c64. It follows0809d623and the original production change7cad43e2, based on3bc921233e88a417c788a0ff9ab022630ebbb96d. Node 24.19.0; isolated worktrees with locked dependencies.SessionRegistry, real migrated SQLite and both projections are exercised; the two clock reads are controlled, with no session IDs or timestamps patched after registration.0809d623. A missing-only rig exposed 12s instead of null; a rig with a healthy 6s sibling exposed 12s. On fresh maindbc8abe6, uptime was null and 6s respectively, while the sibling retained the old count/status bug. The fixed tree gets both count/status and uptime right and preserves stored rows.npm run build,npm run lintandgit diff --check: passed on the final tree, with a 1536 MiB Node heap cap.dbc8abe60721d4e228d488d3fa9cc26039828fbewas clean; the tested candidate is the pinned branch tree, not that newer merge.The earlier 14-line test-only follow-up repaired D3's obsolete
running_countfault trigger after hosted CI exposed it. D3 now proves the injected prepare failure occurred at the rig query, still requires HTTP 500 with no partial payload, and explicitly checks HTTP 200/full membership after recovery. It passes in the final focused run.The full local aggregate, separate full UI suite and installed-package scenario were not rerun for this follow-up. Hosted CI for the new head must still complete. No native provider, live tmux session or actual system-clock change was exercised.
Anything you were unsure about
The reproduced ordering failure is within one daemon process. The monotonic ULID factory resets on restart, so this does not establish a global insertion-order or clock-rollback guarantee. Other registry, restore and snapshot selectors still use timestamps and remain outside this change. Identity-verdict applicability and stored session history are unchanged.
If this is security-related
Not security-related.
CHANGELOG.mdeditSummary by CodeRabbit