Skip to content

Commit 06dae0c

Browse files
fix: address reviewer feedback for session TTL implementation
- Replace time.time() with time.monotonic() for reliable TTL calculations - Add validation for negative session_ttl values with descriptive error - Fix close() method to save chat histories before clearing agents - Fix is_expired() boundary condition to use >= instead of > - Remove silent exception swallowing in knowledge cleanup Fixes identified issues from CodeRabbit and Greptile reviews. Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
1 parent d124dd4 commit 06dae0c

1 file changed

Lines changed: 12 additions & 7 deletions

File tree

  • src/praisonai-agents/praisonaiagents/session

src/praisonai-agents/praisonaiagents/session/api.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,13 @@ def __init__(
8282
self.is_remote = agent_url is not None
8383

8484
# TTL (time-to-live) functionality
85+
if session_ttl is not None and session_ttl < 0:
86+
raise ValueError(
87+
f"Invalid session_ttl={session_ttl} for session_id={self.session_id}. "
88+
"Use None (no expiry) or a non-negative number of seconds."
89+
)
8590
self.session_ttl = session_ttl
86-
self._created_at = time.time()
91+
self._created_at = time.monotonic()
8792

8893
# Validate agent_url format
8994
if self.is_remote:
@@ -600,13 +605,16 @@ def is_expired(self) -> bool:
600605
"""Check if the session has expired based on TTL."""
601606
if self.session_ttl is None:
602607
return False
603-
return time.time() - self._created_at > self.session_ttl
608+
return time.monotonic() - self._created_at >= self.session_ttl
604609

605610
def close(self) -> None:
606611
"""Close and cleanup the session."""
607612
if self.is_remote:
608613
return # No cleanup needed for remote sessions
609614

615+
# Save agent chat histories before clearing agents
616+
self._save_agent_chat_histories()
617+
610618
# Properly cleanup memory
611619
if hasattr(self, '_memory') and self._memory:
612620
try:
@@ -620,10 +628,7 @@ def close(self) -> None:
620628

621629
# Clear knowledge
622630
if hasattr(self, '_knowledge') and self._knowledge:
623-
try:
624-
self._knowledge = None
625-
except Exception:
626-
pass # Ignore cleanup errors
631+
self._knowledge = None
627632

628633
# Clear agents
629634
if hasattr(self, '_agents'):
@@ -633,7 +638,7 @@ def time_to_expiry(self) -> Optional[float]:
633638
"""Get seconds until session expires, or None if no TTL set."""
634639
if self.session_ttl is None:
635640
return None
636-
elapsed = time.time() - self._created_at
641+
elapsed = time.monotonic() - self._created_at
637642
return max(0, self.session_ttl - elapsed)
638643

639644
def __str__(self) -> str:

0 commit comments

Comments
 (0)