fix: create an persist agent session before claude code#1223
fix: create an persist agent session before claude code#1223rushilpatel0 merged 2 commits intodevelopfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. |
|
🎉 This PR is included in version 0.56.13 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
|
LGTM ✅ No issues found. |
|
Found 1 critical and 1 important issue. PR is already closed, so I couldn't attach inline comments to the diff. Sharing precise suggestions below for follow-up PR.
Suggested fix (src/codegen/cli/commands/claude/main.py – secure write with 0o600): # Write session context file for downstream hooks and tools (after hook setup)
try:
SESSION_FILE.parent.mkdir(exist_ok=True)
session_payload = {
"session_id": session_id,
"agent_run_id": agent_run_id,
"org_id": resolved_org_id,
"hook_event": "Startup",
}
# Ensure file is created with 0600 permissions
fd = os.open(SESSION_FILE, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
with os.fdopen(fd, "w") as f:
json.dump(session_payload, f, indent=2)
f.write("\n")
console.print("📝 Wrote session file to ~/.codegen/claude-session.json", style="dim")
except Exception as e:
console.print(f"⚠️ Could not write session file: {e}", style="yellow")
Low-impact change to mitigate: pass a per-session file path via env and teach hooks to read it for fallback. a) In src/codegen/cli/commands/claude/main.py, set a per-session file and export it: # Write session context file for downstream hooks and tools (after hook setup)
try:
SESSION_FILE.parent.mkdir(exist_ok=True)
session_file = SESSION_FILE.parent / f"claude-session-{session_id}.json"
os.environ["CODEGEN_CLAUDE_SESSION_FILE"] = str(session_file)
session_payload = {
"session_id": session_id,
"agent_run_id": agent_run_id,
"org_id": resolved_org_id,
"hook_event": "Startup",
}
fd = os.open(session_file, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
with os.fdopen(fd, "w") as f:
json.dump(session_payload, f, indent=2)
f.write("\n")
console.print(f"📝 Wrote session file to {session_file}", style="dim")
except Exception as e:
console.print(f"⚠️ Could not write session file: {e}", style="yellow")b) In both hooks (src/codegen/cli/commands/claude/config/claude_session_active_hook.py and .../claude_session_stop_hook.py), prefer CODEGEN_CLAUDE_SESSION_FILE when falling back to a file: def read_session_file() -> dict:
"""Read session data written by the CLI, if available."""
env_path = os.environ.get("CODEGEN_CLAUDE_SESSION_FILE")
session_path = Path(env_path) if env_path else (Path.home() / ".codegen" / "claude-session.json")
if not session_path.exists():
return {}
try:
with open(session_path) as f:
return json.load(f)
except Exception:
return {}These changes keep current behavior (hooks still prefer env-based context) while removing cross-session risk when they must fall back to reading a file. The secure write also prevents unintended disclosure of session identifiers. |
|
Additional important issue related to concurrency:
Suggested fix: Avoid unlinking the session file in ensure_claude_hook; perform cleanup at session end only (cleanup_claude_hook already removes the file), or move to per-session files as outlined earlier. # Create .codegen directory if it doesn't exist
CODEGEN_DIR.mkdir(exist_ok=True)
# Do not remove existing session file here to avoid affecting concurrent sessions
# Cleanup should occur at session end (see cleanup_claude_hook)This aligns with the per-session file approach and prevents cross-session interference. |
Motivation
Content
Testing
Please check the following before marking your PR as ready for review