Skip to content

fix: cleanup stale discovery nodes correctly#2

Merged
cursor[bot] merged 2 commits intomasterfrom
cursor/pr-bug-scan-fixes-16e3
Apr 13, 2026
Merged

fix: cleanup stale discovery nodes correctly#2
cursor[bot] merged 2 commits intomasterfrom
cursor/pr-bug-scan-fixes-16e3

Conversation

@LycanW
Copy link
Copy Markdown
Owner

@LycanW LycanW commented Apr 13, 2026

Summary

  • add a focused regression test for stale-node cleanup behavior
  • fix ServiceRegistry::cleanup time-unit mismatch (seconds vs milliseconds)
  • keep change minimal: convert now and timeout to milliseconds before retain filter

Reproduction Evidence (before fix)

A standalone Rust repro (dependency-free) that mirrors current cleanup logic shows stale nodes are incorrectly retained:

node_present_after_cleanup=true
thread 'main' panicked at <anon>:17:5:
BUG reproduced: stale node was not removed

Repro scenario:

  • store node timestamp in milliseconds (now_ms - 20_000)
  • call cleanup with timeout_secs = 10
  • buggy code computes now in seconds and compares against millisecond timestamps, so stale nodes never expire

Fix

src/discovery.rs:

  • now switched from as_secs() to as_millis()
  • timeout_secs converted to timeout_ms
  • retain condition now compares milliseconds to milliseconds

Why this is high-confidence

  • Heartbeat timestamps are written in milliseconds at send time.
  • Cleanup previously compared those values against seconds.
  • Unit mismatch deterministically prevents expiry.

Validation

  • Repro before fix: fails as shown above.
  • Post-fix equivalent logic behaves correctly (stale node removed).

Environment Notes

In this cloud environment, crate-level cargo test is currently blocked by toolchain/build issues unrelated to this logic:

  1. stable cargo 1.83 cannot parse time = 0.3.47 (edition2024 metadata)
  2. nightly build then fails in zmq-sys C++ header resolution (<string>, <vector> not found)

So validation used a dependency-free Rust repro to satisfy /repro and verify behavior.

Open in Web Open in Cursor 

cursoragent and others added 2 commits April 13, 2026 12:06
Co-authored-by: LycanW <LycanW@users.noreply.github.com>
Co-authored-by: LycanW <LycanW@users.noreply.github.com>
@cursor cursor Bot changed the title test: add reproduction for discovery cleanup expiry bug fix: cleanup stale discovery nodes correctly Apr 13, 2026
@cursor cursor Bot merged commit 3b8068b into master Apr 13, 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.

2 participants