fix: align peer keepalive handling

[artrixdotdev]

Closes #205

Summary by CodeRabbit

• Bug Fixes
  • Improved peer connection stability through optimized keepalive and disconnect timing adjustments.

[coderabbitai]

Walkthrough

This PR adjusts the peer actor's keepalive protocol to align with BEP 3 specifications. It updates silence-based disconnect timing constants, clarifies the documentation for keepalive/disconnect thresholds in the next() method, and removes keepalive response sending—the handler now only traces received keepalive messages instead of echoing them back.

Changes

Keepalive Protocol Alignment

Layer / File(s)	Summary
BEP 3 keepalive timing and response behavior crates/libtortillas/src/peer/actor.rs	Peer keepalive/disconnect silence thresholds are increased via constant adjustments (lines 32–33), documentation is updated to reflect the two-stage keepalive timing logic (lines 371–372), and the KeepAlive message handler is simplified to only trace receipt instead of sending a response back (lines 484–485).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'fix: align peer keepalive handling' directly and concisely describes the main change: adjusting peer keepalive behavior to align with standards.
Linked Issues check	✅ Passed	The PR adjusts keepalive timing constants and removes the keepalive reply behavior, aligning with BEP 3's guidance that keepalives should be ignored rather than replied to.
Out of Scope Changes check	✅ Passed	All changes in the PR (timing adjustments, comment updates, and reply behavior removal) are directly related to the keepalive handling objectives specified in issue #205.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/align-keepalive-handling

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/libtortillas/src/peer/actor.rs (1)

368-408: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Drive keepalive/disconnect from an actual timer, not only next() re-entry.

The two-minute/four-minute policy only runs when next() is entered. During a truly silent connection this select! can block forever, so no keepalive or disconnect fires at all. And once silence is already past Line 380, every mailbox-driven loop can send another keepalive because nothing records that one was already sent for the current idle window. Add a timed branch in next() and track the keepalive phase explicitly.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/libtortillas/src/peer/actor.rs` around lines 368 - 408, The idle
keepalive/disconnect logic in next() must be driven by a timed select branch
instead of only running on re-entry; compute the Instant when the next action is
due from last_message (using PEER_KEEPALIVE_TIMEOUT and
PEER_DISCONNECT_TIMEOUT), add a tokio::time::sleep_until (or sleep) branch into
the tokio::select! so the task wakes when the timer fires, and replace the
current ad-hoc last_message checks with handling in that timer branch so you
only send stream.send(PeerMessages::KeepAlive) once per idle window and only
call supervisor.tell(torrent::commands::KillPeer { id }) when the disconnect
deadline is reached. Track the keepalive phase explicitly on the actor struct
(e.g., a bool or enum field like keepalive_sent or IdlePhase) so subsequent
timer firings or mailbox events won’t resend keepalive until activity resets the
phase, and ensure check_message_signal resets that field when an actual message
arrives.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@crates/libtortillas/src/peer/actor.rs`:
- Around line 368-408: The idle keepalive/disconnect logic in next() must be
driven by a timed select branch instead of only running on re-entry; compute the
Instant when the next action is due from last_message (using
PEER_KEEPALIVE_TIMEOUT and PEER_DISCONNECT_TIMEOUT), add a
tokio::time::sleep_until (or sleep) branch into the tokio::select! so the task
wakes when the timer fires, and replace the current ad-hoc last_message checks
with handling in that timer branch so you only send
stream.send(PeerMessages::KeepAlive) once per idle window and only call
supervisor.tell(torrent::commands::KillPeer { id }) when the disconnect deadline
is reached. Track the keepalive phase explicitly on the actor struct (e.g., a
bool or enum field like keepalive_sent or IdlePhase) so subsequent timer firings
or mailbox events won’t resend keepalive until activity resets the phase, and
ensure check_message_signal resets that field when an actual message arrives.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d4520bec-9047-4b47-99bf-a62fba9843e8

📥 Commits

Reviewing files that changed from the base of the PR and between 8ab7f1a and ac816db.

📒 Files selected for processing (1)

• crates/libtortillas/src/peer/actor.rs