fix: handle None agent name when generating history session ID#1114
fix: handle None agent name when generating history session ID#1114jnMetaCode wants to merge 1 commit intoMervinPraison:mainfrom
Conversation
When an agent is created with instructions but no explicit name, and memory history is enabled, name can be None. Calling .encode() on None raises AttributeError. Fall back to "agent" when name is None. Signed-off-by: JiangNan <1394485448@qq.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical bug where agents initialized with memory history enabled but without an explicit name would crash due to an AttributeError. The fix ensures that a valid session ID is always generated by providing a fallback name, thereby improving the robustness and stability of agent creation and preventing application failures in such scenarios. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Review Summary by QodoFix AttributeError when agent name is None with history enabled
WalkthroughsDescription• Handle None agent name when generating history session ID • Use fallback "agent" string to prevent AttributeError • Ensures valid session ID generation for unnamed agents Diagramflowchart LR
A["Agent created without name"] -->|history=True| B["Generate session ID"]
B -->|name is None| C["Use fallback 'agent' string"]
C -->|encode and hash| D["Valid session ID created"]
File Changes1. src/praisonai-agents/praisonaiagents/agent/agent.py
|
Code Review by Qodo
1. Unnamed history collision
|
📝 WalkthroughWalkthroughA single-line modification in the Agent initialization method improves hash derivation robustness. When history is enabled without an explicit session ID or name, the code now uses a fallback value to ensure consistent session ID generation instead of potentially failing with a None value. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Code Review
This pull request addresses an AttributeError that occurs when an agent is created with history=True but without an explicit name. The fix correctly handles the None value for the agent name by providing a default fallback string "agent" before encoding it to generate a history session ID. The change is simple, effective, and resolves the described bug.
Note: Security Review did not run due to the size of the PR.
| _agent_hash = _hl.md5((name or "agent").encode()).hexdigest()[:8] | ||
| session_id = f"history_{_agent_hash}" | ||
| _history_session_id = session_id |
There was a problem hiding this comment.
1. Unnamed history collision 🐞 Bug ⛨ Security
When history is enabled and no explicit session_id is provided, falling back to the constant string "agent" makes all unnamed agents compute the same history session_id, causing them to load/append to the same persisted chat history. This can mix conversations between unrelated agents and leak prior prompts/responses into new runs.
Agent Prompt
### Issue description
`history=True` with no explicit `session_id` currently auto-generates a session id by hashing `(name or "agent")`. When `name` is `None` (a supported state), this becomes a constant session id shared by all unnamed agents, causing persisted history mixing / leakage.
### Issue Context
- The history branch executes only when `_history_enabled` is true and both `session_id` and `_history_session_id` are unset.
- The generated `session_id` is used as the key for JSON session persistence and for injecting past chat history into subsequent prompts.
### Fix Focus Areas
- Ensure unnamed agents do **not** converge on a single constant history session id.
- Prefer a unique value when `name` is missing (e.g., `uuid.uuid4().hex` or an already-available per-instance `agent_id`).
- Keep deterministic behavior for explicitly named agents.
- src/praisonai-agents/praisonaiagents/agent/agent.py[988-1001]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/praisonai-agents/praisonaiagents/agent/agent.py`:
- Around line 997-1001: The current logic collapses all unnamed agents into one
persisted history session by using (name or "agent") when computing _agent_hash;
instead, when name is None use a stable-but-unique identifier per agent instance
(e.g., incorporate the agent object's id or generate a uuid4) rather than the
constant "agent". Update the block that sets session_id/_history_session_id (the
variables _history_enabled, session_id, _history_session_id and the md5-based
_agent_hash) so that if name is None you derive the hash from a unique token
(like id(self) or a new uuid) combined with any existing identifying info, then
compute the md5 and set session_id = f"history_{_agent_hash}" to avoid unrelated
unnamed agents sharing the same history file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 86a49c6d-686b-4c67-a296-9f6b19ffaae4
📒 Files selected for processing (1)
src/praisonai-agents/praisonaiagents/agent/agent.py
| elif _history_enabled and session_id is None and _history_session_id is None: | ||
| import hashlib as _hl | ||
| _agent_hash = _hl.md5(name.encode()).hexdigest()[:8] | ||
| _agent_hash = _hl.md5((name or "agent").encode()).hexdigest()[:8] | ||
| session_id = f"history_{_agent_hash}" | ||
| _history_session_id = session_id |
There was a problem hiding this comment.
Don't collapse all unnamed agents into one persisted history session.
(name or "agent") avoids the None.encode() crash, but it also makes every instructions-only agent hash to the same history_<hash>. src/praisonai-agents/tests/unit/agents/test_agent_name_none.py:16-23 documents that name=None is a normal state here, and src/praisonai-agents/praisonaiagents/session/store.py:236-270 uses session_id as the storage key, so unrelated unnamed agents will now read/write the same history file.
Suggested fix
elif _history_enabled and session_id is None and _history_session_id is None:
import hashlib as _hl
- _agent_hash = _hl.md5((name or "agent").encode()).hexdigest()[:8]
+ _session_seed = name or instructions or role or goal or backstory or "agent"
+ _agent_hash = _hl.md5(_session_seed.encode()).hexdigest()[:8]
session_id = f"history_{_agent_hash}"
_history_session_id = session_id🧰 Tools
🪛 Ruff (0.15.4)
[error] 999-999: Probable use of insecure hash functions in hashlib: md5
(S324)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/praisonai-agents/praisonaiagents/agent/agent.py` around lines 997 - 1001,
The current logic collapses all unnamed agents into one persisted history
session by using (name or "agent") when computing _agent_hash; instead, when
name is None use a stable-but-unique identifier per agent instance (e.g.,
incorporate the agent object's id or generate a uuid4) rather than the constant
"agent". Update the block that sets session_id/_history_session_id (the
variables _history_enabled, session_id, _history_session_id and the md5-based
_agent_hash) so that if name is None you derive the hash from a unique token
(like id(self) or a new uuid) combined with any existing identifying info, then
compute the md5 and set session_id = f"history_{_agent_hash}" to avoid unrelated
unnamed agents sharing the same history file.
Problem
When creating an agent with
instructionsbut without an explicitname, and memory history is enabled (history=True), the agent crashes with:This happens at line 999 in
agent.py:When no name is provided,
nameremainsNone, and calling.encode()on it fails.Fix
Use a fallback string when
nameisNone:This ensures a valid session ID is always generated, even for unnamed agents.
How to reproduce
Summary by CodeRabbit