-
Notifications
You must be signed in to change notification settings - Fork 62
fix: minor cleanup #1217
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
fix: minor cleanup #1217
Conversation
|
🎉 This PR is included in version 0.56.7 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
| import sys | ||
| from pathlib import Path | ||
|
|
||
| from codegen.cli.commands.claude.claude_session_api import create_claude_session |
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.
CRITICAL: Runtime crash risk due to import order regression
sys.path is modified after attempting to import from codegen. When this hook runs outside the package environment (common for CLI hooks), the early imports will fail with ImportError and the script will exit before reaching main().
| from codegen.cli.commands.claude.claude_session_api import create_claude_session | |
| #!/usr/bin/env python3 | |
| """Claude Code session hook script for API integration. | |
| This script is called by Claude Code on SessionStart to: | |
| 1. Create a session in the backend API | |
| 2. Write session data to local file for tracking | |
| """ | |
| import json | |
| import os | |
| import sys | |
| from pathlib import Path | |
| # Add the codegen CLI to the path so we can import from it | |
| script_dir = Path(__file__).parent | |
| codegen_cli_dir = script_dir.parent.parent.parent | |
| sys.path.insert(0, str(codegen_cli_dir)) | |
| try: | |
| from codegen.cli.commands.claude.claude_session_api import create_claude_session | |
| from codegen.cli.utils.org import resolve_org_id | |
| except ImportError: | |
| # Fallback if imports fail - just write basic session data | |
| create_claude_session = None | |
| resolve_org_id = None |
This restores safe behavior: ensure the path is set before imports and retain the previous fallback so the hook doesn’t crash if imports aren’t available.
| # Build the shell command that will create session via API and write session data | ||
| start_hook_script_path = Path(__file__).parent / "config" / "claude_session_hook.py" | ||
| start_hook_command = f"mkdir -p {CODEGEN_DIR} && python3 {start_hook_script_path} > {SESSION_FILE}" | ||
| start_hook_command = f"python3 {start_hook_script_path} > {SESSION_FILE}" |
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.
IMPORTANT: Regression can break session file creation at runtime
start_hook_command no longer ensures the target directory exists. If ~/.codegen is removed or missing when the hook runs (e.g., after cleanup or on a fresh environment), shell redirection to claude-session.json will fail and the hook won’t write session data. This breaks downstream hooks and session tracking.
| start_hook_command = f"python3 {start_hook_script_path} > {SESSION_FILE}" | |
| # Build the shell command that will create session via API and write session data | |
| start_hook_script_path = Path(__file__).parent / "config" / "claude_session_hook.py" | |
| start_hook_command = f"mkdir -p {CODEGEN_DIR} && python3 {start_hook_script_path} > {SESSION_FILE}" |
Restoring mkdir -p makes the hook robust against missing directories at execution time.
|
Found 1 critical and 1 important issues. Please review my inline comments above. |
Motivation
Content
Testing
Please check the following before marking your PR as ready for review