-
Notifications
You must be signed in to change notification settings - Fork 62
Rpatel/create session on claude #1222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report❌ Patch coverage is
|
|
|
||
| # Proactively create the backend session as a fallback in case hooks fail | ||
| try: | ||
| agent_run_id = create_claude_session(session_id, resolved_org_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic error: Potential duplicate session creation leading to multiple agent runs
The proactive create_claude_session call runs before hooks are installed, but the SessionStart hook also creates the session. This can cause duplicate sessions or noisy errors if the backend isn’t strictly idempotent on session_id.
| agent_run_id = create_claude_session(session_id, resolved_org_id) | |
| # Set up Claude hook for session tracking first | |
| hook_ready = ensure_claude_hook() | |
| if not hook_ready: | |
| console.print("⚠️ Failed to set up session tracking hook", style="yellow") | |
| # Fallback: proactively create the backend session only if hooks failed | |
| try: | |
| agent_run_id = create_claude_session(session_id, resolved_org_id) | |
| if agent_run_id: | |
| console.print("✅ Backend session created", style="green") | |
| else: | |
| console.print("⚠️ Could not create backend session at startup (will rely on hooks)", style="yellow") | |
| except Exception as e: | |
| console.print(f"⚠️ Session creation error at startup: {e}", style="yellow") |
This preserves your fallback while avoiding duplicate creation when hooks succeed.
|
Found 0 critical and 1 important issue. Please review my inline comment above. 🔍 View my analysis: https://codegen.com/agent/trace/77640 |
Motivation
Content
Testing
Please check the following before marking your PR as ready for review