fix: wire CLI run session continuity to project session store#1949
fix: wire CLI run session continuity to project session store#1949cursor[bot] wants to merge 2 commits into
Conversation
- Restore chat history from project-scoped store on --continue/--session - Replace invalid Agent resume_session kwarg with MemoryConfig + store wiring - Add apply_cli_session_continuity helper and regression tests Fixes broken session resume introduced in #1929 where sessions were discovered in the project store but saved/loaded from the global store. Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
|
@coderabbitai review |
|
/review |
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
✅ Action performedReview finished.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe PR refactors CLI session continuity by introducing a new ChangesCLI Session Continuity with Resume-Session
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
@copilot Do a thorough review of this PR. Read ALL existing reviewer comments above from Qodo, Coderabbit, and Gemini first — incorporate their findings. Review areas:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/praisonai/tests/unit/cli/test_run_session_continuity.py (1)
43-134: ⚡ Quick winAdd a regression that hits
run.py’soutput_mode="actions"branch directly.These tests prove the helper and
handle_direct_prompt()wiring, but they never call_run_prompt(..., output_mode="actions"), which is the branch that previously raised theresume_sessionTypeError. A focused unit test there would catch both the kwargs regression and any future drift betweensrc/praisonai/praisonai/cli/commands/run.pyandsrc/praisonai/praisonai/cli/main.py. As per coding guidelines, test coverage/quality for changed or critical paths should be included before merge.Possible test shape
+def test_run_prompt_actions_mode_wires_session_continuity(monkeypatch): + from praisonai.cli.commands.run import _run_prompt + + captured = {} + + class FakeAgent: + def __init__(self, **kwargs): + captured["agent_config"] = kwargs + + def start(self, prompt): + captured["prompt"] = prompt + return "ok" + + monkeypatch.setattr("praisonaiagents.Agent", FakeAgent) + monkeypatch.setattr( + "praisonai.cli.state.project_sessions.apply_cli_session_continuity", + lambda agent, session_id, project_path=None: captured.setdefault("continuity", session_id), + ) + + _run_prompt( + "follow up question", + output_mode="actions", + session="saved-session", + ) + + memory = captured["agent_config"]["memory"] + assert isinstance(memory, MemoryConfig) + assert memory.session_id == "saved-session" + assert captured["continuity"] == "saved-session" +``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@src/praisonai/tests/unit/cli/test_run_session_continuity.pyaround lines 43
- 134, Add a focused unit test that exercises the run command branch that uses
output_mode="actions" to catch the previous resume_session TypeError: call the
Run command handler/_run_prompt path with output_mode="actions" (the same args
pattern used in test_handle_direct_prompt_wires_project_session) and assert that
resume_session is forwarded into the created agent's MemoryConfig and that the
call returns normally; specifically, add a new test function that patches the
same helpers (praisonaiagents.Agent -> FakeAgent,
PraisonAI._execute_agent_with_budget_handling,
state.project_sessions.apply_cli_session_continuity, etc.), invokes the
run/_run_prompt entry used for CLI actions with resume_session="saved-session",
and asserts MemoryConfig.session_id == "saved-session" and no TypeError is
raised. Ensure the test targets the run command branch (output_mode="actions")
and reuses captured dict assertions similar to
test_handle_direct_prompt_wires_project_session.</details> <!-- cr-comment:v1:4eca202d743ba445e2ede90b --> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In@src/praisonai/praisonai/cli/main.py:
- Around line 4517-4525: The current code overwrites agent_config["memory"] when
setting MemoryConfig for resume (using resume_session_id/save_name), which
discards previously set fields like user_id; change this to merge with any
existing memory configuration instead of replacing it: read the existing value
from agent_config.get("memory") (handle either a dict or a MemoryConfig
instance), preserve all existing fields (e.g., user_id and other keys), then
set/override only session_id, auto_save and history before assigning the merged
result back to agent_config["memory"]; keep the use of MemoryConfig when
constructing the final config so other code continues to expect that type.
Nitpick comments:
In@src/praisonai/tests/unit/cli/test_run_session_continuity.py:
- Around line 43-134: Add a focused unit test that exercises the run command
branch that uses output_mode="actions" to catch the previous resume_session
TypeError: call the Run command handler/_run_prompt path with
output_mode="actions" (the same args pattern used in
test_handle_direct_prompt_wires_project_session) and assert that resume_session
is forwarded into the created agent's MemoryConfig and that the call returns
normally; specifically, add a new test function that patches the same helpers
(praisonaiagents.Agent -> FakeAgent,
PraisonAI._execute_agent_with_budget_handling,
state.project_sessions.apply_cli_session_continuity, etc.), invokes the
run/_run_prompt entry used for CLI actions with resume_session="saved-session",
and asserts MemoryConfig.session_id == "saved-session" and no TypeError is
raised. Ensure the test targets the run command branch (output_mode="actions")
and reuses captured dict assertions similar to
test_handle_direct_prompt_wires_project_session.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `ac86bfea-f618-4c6b-8b1d-02c88c253497` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 2f9677abb2ea68eab864ee8b6a828fd0141612e1 and 59b3ed4dda0c34938298c4a64739fd2adbaa557e. </details> <details> <summary>📒 Files selected for processing (4)</summary> * `src/praisonai/praisonai/cli/commands/run.py` * `src/praisonai/praisonai/cli/main.py` * `src/praisonai/praisonai/cli/state/project_sessions.py` * `src/praisonai/tests/unit/cli/test_run_session_continuity.py` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| resume_session_id = getattr(self.args, 'resume_session', None) | ||
| if resume_session_id: | ||
| from praisonaiagents import MemoryConfig | ||
| save_name = getattr(self.args, 'auto_save', None) or resume_session_id | ||
| agent_config["memory"] = MemoryConfig( | ||
| session_id=resume_session_id, | ||
| auto_save=save_name, | ||
| history=True, | ||
| ) |
There was a problem hiding this comment.
Preserve existing memory isolation fields when enabling --resume.
Line 4521 replaces the whole memory config, which discards fields already set earlier (notably user_id from Line 4502). With --memory --user-id ... --resume ..., memory can silently fall back to non-user-scoped state.
Suggested fix
resume_session_id = getattr(self.args, 'resume_session', None)
if resume_session_id:
from praisonaiagents import MemoryConfig
save_name = getattr(self.args, 'auto_save', None) or resume_session_id
- agent_config["memory"] = MemoryConfig(
- session_id=resume_session_id,
- auto_save=save_name,
- history=True,
- )
+ existing_memory = agent_config.get("memory")
+ if isinstance(existing_memory, MemoryConfig):
+ existing_memory.session_id = resume_session_id
+ existing_memory.auto_save = save_name
+ existing_memory.history = True
+ else:
+ agent_config["memory"] = MemoryConfig(
+ session_id=resume_session_id,
+ auto_save=save_name,
+ history=True,
+ )
print(f"[bold cyan]Resuming session '{resume_session_id}'[/bold cyan]")🤖 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 `@src/praisonai/praisonai/cli/main.py` around lines 4517 - 4525, The current
code overwrites agent_config["memory"] when setting MemoryConfig for resume
(using resume_session_id/save_name), which discards previously set fields like
user_id; change this to merge with any existing memory configuration instead of
replacing it: read the existing value from agent_config.get("memory") (handle
either a dict or a MemoryConfig instance), preserve all existing fields (e.g.,
user_id and other keys), then set/override only session_id, auto_save and
history before assigning the merged result back to agent_config["memory"]; keep
the use of MemoryConfig when constructing the final config so other code
continues to expect that type.
|
@claude You are the FINAL architecture reviewer. If the branch is under MervinPraison/PraisonAI (not a fork), you are able to make modifications to this branch and push directly. SCOPE: Focus ONLY on Python packages (praisonaiagents, praisonai). Do NOT modify praisonai-rust or praisonai-ts. Read ALL comments above from Gemini, Qodo, CodeRabbit, and Copilot carefully before responding. Phase 1: Review per AGENTS.md
Phase 2: FIX Valid Issues Phase 3: Final Verdict |
|
Claude finished @MervinPraison's task —— View job Final Architecture Review Complete ✅Phase 1: Architecture Review per AGENTS.md
Phase 2: Fix Valid Issues from Other Reviewers
Phase 3: Final Verdict
Architecture Review Results ✅✅ Protocol-driven design: Changes properly separate concerns - core protocols in Issues Fixed ✅
Final Verdict: ✅ APPROVEThis PR successfully fixes the CLI session continuity bug and addresses all valid reviewer feedback:
The implementation follows AGENTS.md guidelines with clean separation between protocol (core SDK) and implementation (CLI wrapper). All changes are backward-compatible and focused on the specific bug. Pushed fixes to commit 03326c95 --- • Branch: |
- Fix memory config being completely replaced in main.py, now preserves existing fields like user_id - Add test coverage for actions mode session continuity to prevent TypeError regression Co-authored-by: Mervin Praison <MervinPraison@users.noreply.github.com>
2 participants
Bug and impact
praisonai run --continue/--session(added in #1929) did not actually resume conversation history. Sessions were discovered in the project-scoped store (~/.praisonai/sessions/projects/...) but agents saved and loaded from the global default store. Users saw "Continuing session: …" but the agent had no prior context.Additionally,
praisonai run "…" --continue --output actionscrashed withTypeError: unexpected keyword argument 'resume_session'becauseAgenthas no such parameter.Root cause
run.pysetargs.resume_sessionbuthandle_direct_prompt()never consumed it.MemoryConfig(auto_save=…)without binding the project session store or restoring history.resume_sessiondirectly toAgent(**kwargs).Fix
apply_cli_session_continuity()to bind agents to the project session store and restore prior turns.resume_sessioninhandle_direct_prompt()viaMemoryConfig(session_id=…, history=True).MemoryConfiginstead of invalid kwargs.run.py.Validation
src/praisonai/tests/unit/cli/test_run_session_continuity.pySummary by CodeRabbit
Release Notes
New Features
--resume-sessionflag to resume prior CLI sessions with automatic chat history restoration.Improvements