feat: scope hook logs to per-session directories

[philoserf]

Summary

• Hook logs (hook-events.log, git-commands.log) now write to ~/.claude/logs/<session-id>/ using the last 8 chars of $CLAUDE_SESSION_ID
• Session cleanup replaces size-based rotation with 7-day age-based directory cleanup, consistent with debug and shell-snapshot retention
• Registers session-cleanup.sh as a SessionEnd hook in settings.json

Test plan

• Start a new session — confirm logs appear in ~/.claude/logs/<session-id>/
• Verify $CLAUDE_SESSION_ID last 8 chars match the directory name
• Confirm legacy flat files in logs/ are cleaned up after session ends
• Verify debug and shell-snapshot cleanup still works unchanged

🤖 Generated with Claude Code

[philoserf]

@claude please look at this one

[claude]

Claude finished @philoserf's task in 1m 10s —— View job

---

PR Review: feat: scope hook logs to per-session directories

• Gather context and read changed files
• Review hook scripts
• Review settings.json changes
• Review README updates
• Post final review

---

Overall this is clean, well-scoped work. The approach is consistent with the existing debug/shell-snapshot retention pattern. A few observations:

---

hooks/log-hook-event.sh & hooks/log-git-commands.sh

Fallback to default/: When CLAUDE_SESSION_ID is unset or empty, both scripts fall back to a default directory:

session_id="${CLAUDE_SESSION_ID: -8}"
session_id="${session_id:-default}"

This means logs from all unidentified invocations accumulate in a single default/ directory across sessions. The 7-day cleanup will eventually handle it, but if hooks are ever run manually or in test contexts, they'll pile up there. A timestamp-based fallback (e.g. $(date +%Y%m%d-%H%M%S)) would be more isolated, though probably over-engineered for the actual use case.

8-char suffix collision: Truncating to 8 chars is practically fine (4.3B combinations), but worth a comment explaining the tradeoff if anyone ever wonders why it's 8 and not the full ID.

mkdir -p on every event: In log-hook-event.sh, the mkdir -p runs unconditionally on every hook event (not just when there's something to log). This matches the old behavior and is idempotent, so no regression — just noting it.

---

hooks/session-cleanup.sh

The script is straightforward and well-guarded. A couple notes:

log_dir variable scope: The tilde in log_dir=~/.claude/logs expands correctly in bash assignment context, but debug and shell-snapshots still use hardcoded ~/.claude/... paths rather than variables. Minor inconsistency, no functional impact.

find ... -exec rm -rf {} +: Safe — -mindepth 1 -maxdepth 1 constrains deletion to direct subdirectories only, so the logs/ root itself is never touched. The 2>/dev/null || true correctly handles the case where the directory doesn't exist yet.

Legacy file cleanup is one-time: The rm -f for legacy flat files will become a no-op after the first run. Could eventually be removed once the transition is complete, but there's no rush.

---

settings.json

session-cleanup.sh is registered before log-hook-event.sh SessionEnd. This means cleanup runs, then the SessionEnd event gets logged. That ordering is correct — cleanup removes 7+ day old directories, leaving the current session's directory intact for the subsequent log write.

---

README.md

The updated examples are accurate. The ls logs/ example will also show legacy flat files during the transition window, which is fine.

---

Summary

No blocking issues. The main practical consideration is the default fallback behavior if CLAUDE_SESSION_ID is ever absent, but that's an edge case. The implementation achieves the goal cleanly.

[Copilot]

Pull request overview

This PR scopes hook-related logs to per-session directories under ~/.claude/logs/<session-id>/ (based on the last 8 chars of $CLAUDE_SESSION_ID) and introduces a SessionEnd cleanup hook that removes old session log directories and legacy flat log files.

Changes:

• Write hook-events.log and git-commands.log into ~/.claude/logs/<session-id>/ instead of a single flat logs/ location.
• Add session-cleanup.sh and register it as a SessionEnd hook to remove logs/debug/snapshots older than 7 days and delete legacy flat log files.
• Update README wording/examples to reflect per-session log layout.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
settings.json	Registers session-cleanup.sh to run on SessionEnd.
hooks/session-cleanup.sh	New cleanup script for old session log directories and legacy flat logs (plus debug/snapshot retention).
hooks/log-hook-event.sh	Routes hook event logs into a per-session directory.
hooks/log-git-commands.sh	Routes git command logs into a per-session directory.
README.md	Updates docs to describe per-session logs and new cleanup hook.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[philoserf]

@claude We added some changes. Look again.

[claude]

Claude finished @philoserf's task in 1m 40s —— View job

---

Re-review: feat/session-scoped-hook-logs

• Gather context
• Read changed files
• Review updates against previous feedback
• Post updated review

---

Good progress. Here's what changed and what remains.

---

What's new in this revision

log-git-commands.sh stdin fix: The previous version called jq directly on stdin twice, which would silently fail on the second read. Reading into input=$(cat) once and reusing it is the correct approach. Same pattern was already used in log-hook-event.sh, now consistent.

Session ID source: Both scripts now extract .session_id from the JSON payload instead of $CLAUDE_SESSION_ID. This is more reliable and matches the commit message.

Legacy cleanup: hook-events.log.1 was added to the rm -f line, partially addressing the Copilot suggestion.

---

Remaining items

git-commands.log.1 still missing (session-cleanup.sh:11):
The Copilot suggestion included it, but the current line is:

rm -f "$log_dir/hook-events.log" "$log_dir/hook-events.log.1" "$log_dir/git-commands.log"

"$log_dir/git-commands.log.1" is still absent. Minor, but worth being complete if you're handling rotated variants at all. Fix this →

log-git-commands.sh README description is stale (README.md:180):

log-git-commands.sh - Logs all git/gh commands to stderr for tracking

It no longer logs to stderr — it writes to ~/.claude/logs/<session-id>/git-commands.log. Fix this →

Header comment misleads (session-cleanup.sh:2):

# SessionEnd cleanup - rotate logs and archive stale session data

The script deletes, not rotates or archives. Still unaddressed from the Copilot review. Fix this →

No session_id sanitization (both logging scripts):
Copilot flagged path traversal risk from unsanitized session_id. In practice, session_id comes from Claude's own JSON payload, so the trust boundary is narrow — but if you want to address it, stripping non-alphanumerics is a one-liner:

session_id="${session_id//[^a-zA-Z0-9]/_}"

Up to you whether the defense-in-depth is worth it for an internal tool.

rm -rf -- {} vs rm -rf {} (session-cleanup.sh:8):
Copilot suggested -- to guard against directory names starting with -. Low-probability edge case for session ID directories, but trivial to add.

---

Hook count

Counted 11 hooks listed in the README (validate-config, validate-bash-commands, protect-secrets, log-git-commands, log-hook-event, auto-format, session-start, session-end, load-session-context, evaluate-session, session-cleanup). Count is correct — notify-idle was removed in a prior commit and session-cleanup replaces it here.

---

Summary

The two substantive issues worth fixing before merge: the stale stderr description in README and the git-commands.log.1 omission. The comment header and sanitization are lower priority. The core implementation is solid.