Repository navigation
fix: restart the outbox TTL on polls only - #65
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
I've gathered some intelligence on your latest changes. 🕵️♀️I've aggregated the results of the automated checks for this PR below. 📋 Repo HealthYour repository is in great shape! ✅ ✅ All required files present. Latest Version: ✅ ⚖️ License CheckChecking for any missing license headers. ✍️ ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 🔍 LintJust keeping you informed on the state of things. ℹ️ ❌ ruff: issues found in 16 file(s) — see job log 🔒 Security (pip-audit)I've scanned the dependencies for any hidden surprises. 🔍 ✅ No known vulnerabilities found (88 packages scanned). 🔨 Build TestsI've fired up the furnaces and forged your changes. ⚒️ ✅ All versions pass
🏷️ Release PreviewEnsuring our release notes are clear and concise. ✍️ Current:
✅ PR title follows conventional commit format. 🚀 Release Channel Compatibility Predicted next version:
📊 CoverageHow well-protected is our logic? Let's find out! 🛡️ ✅ 87.5% total coverage Per-file coverage (2 files)
Full report: download the Providing clarity through automated analysis 🔍 |
RetentionQueue.put() refreshed the idle timestamp, so a client that stopped polling while frames kept arriving was never swept. Only the per-client frame cap bounded it. The README and docs/architecture.md define undelivered_ttl as the time a client may go without polling. put() no longer touches the timestamp; drain() still does. The test that asserted the old behaviour (a client that only receives survives the sweep) is replaced by one asserting the documented behaviour. The test for poll activity puts a frame between polls and asserts the last drain returns it, so removing the reset in drain() fails it. The docs say to keep undelivered_ttl at least ten times the poll interval. Fail-before: the new test fails on the unfixed code (assert 'deaf' not in store), passes with the fix, and the full suite passes (109). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
ede7e39 to
ad474c8
Compare
HIVEMIND-TRANSPORT-1 §4: "the node MUST retain a peer's outbound frames until the peer polls them, until the session closes, or until an implementation-defined retention bound elapses. [...] The implementation MUST document the bound."
The documented bound is
undelivered_ttl: the README anddocs/architecture.mdboth say a client that does not poll for that many seconds loses its queue. The code did not match.RetentionQueue.put()reset the idle timestamp, so a client that stopped polling but kept receiving frames was never swept. Onlymax_undeliveredlimited it.put()no longer touches the timestamp.drain()still does, so a poll restarts the clock. The class docstring now says so.The old test
test_an_active_client_survives_the_sweepasserted the buggy behaviour and is replaced. The new test uses a fake clock, sends a frame every second for nine seconds with a 10 second TTL, and checks that the queue is gone once the clock passes 10 seconds with no poll. It fails on the unfixed code and passes with the fix. The test for poll activity puts a frame between polls and asserts that the last poll returns it. It fails whendrain()stops restarting the clock. The README anddocs/architecture.mdnow say to keepundelivered_ttlat least ten times the poll interval, because a later poll finds the queue gone and, under protocol v3, the client must reconnect. The full suite passes (109 tests). Ruff reports the same findings as the base on the touched files.🤖 Generated with Claude Code