fix: filter session-start injection by cwd/project to prevent cross-project contamination#1054
Merged
Merged
Conversation
…roject contamination The SessionStart hook previously selected the most recent session file purely by timestamp, ignoring the current working directory. This caused Claude to receive a previous project's session context when switching between projects, leading to incorrect file reads and project analysis. session-end.js already writes **Project:** and **Worktree:** header fields into each session file. This commit adds selectMatchingSession() which uses those fields with the following priority: 1. Exact worktree (cwd) match — most recent 2. Same project name match — most recent 3. Fallback to overall most recent (preserves backward compatibility) No new dependencies. Gracefully falls back to original behavior when no matching session exists.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplaces fixed “most recent session” selection with a selector that canonicalizes paths, reads and caches session files, and chooses a session by: exact worktree match → project-name match (via getProjectName()) → newest readable session; logs chosen path and matchReason and injects cached content. Changes
Sequence Diagram(s)sequenceDiagram
participant Hook as "session-start hook"
participant Utils as "utils.getProjectName()"
participant FS as "filesystem (session files)"
participant Injector as "context injector"
Hook->>Utils: getProjectName()
Note over Hook,FS: iterate deduplicated recentSessions (newest-first), read & cache readable files, normalize paths
Hook->>FS: read session file `#1`
alt Worktree matches cwd
FS-->>Hook: cached session content (worktree)
Hook->>Injector: inject content (matchReason: worktree)
else Project matches currentProject
FS-->>Hook: cached session content (project)
Hook->>Injector: inject content (matchReason: project)
else Readable files exist (no match)
FS-->>Hook: cached session content (recency)
Hook->>Injector: inject content (matchReason: recency-fallback)
else No readable files
FS-->>Hook: read errors
Hook-->>Hook: log and return null
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
scripts/hooks/session-start.js (3)
122-136: Session file is read twice.The selected session's content is read once inside
selectMatchingSession(line 76) and again here (line 132). Consider havingselectMatchingSessionreturn the content it already read to avoid redundant I/O.♻️ Return content from selectMatchingSession
Modify
selectMatchingSessionto track and return the content:// In selectMatchingSession, when finding a match: return { session, matchReason: 'worktree', content }; // Then in main(): const { session: selected, matchReason, content } = selectMatchingSession(...); const strippedContent = stripAnsi(content);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hooks/session-start.js` around lines 122 - 136, selectMatchingSession currently reads the chosen session file and then the caller reads it again; change selectMatchingSession to return the already-read raw content along with the selected session and match reason (e.g., return { session, matchReason, content }) and update the caller in the session-start.js main flow to destructure those values (selected/session, matchReason/_matchReason, and content), use the returned content (stripAnsi on it) for additionalContextParts, and remove the redundant readFile(selected.path) call and second file read; also ensure the log still reports selected.path and the match reason from the returned value.
85-86: Mutating input objects violates immutable patterns guideline.The function modifies the passed-in session objects by adding
_matchReason. Consider returning a wrapper object instead to avoid side effects.♻️ Immutable alternative
- if (sessionWorktree && sessionWorktree === cwd) { - session._matchReason = 'worktree'; - return session; - } + if (sessionWorktree && sessionWorktree === cwd) { + return { session, matchReason: 'worktree' }; + } ... - if (sessionProject && sessionProject === currentProject) { - projectMatch = session; - projectMatch._matchReason = 'project'; - } + if (sessionProject && sessionProject === currentProject) { + projectMatch = { session, matchReason: 'project' }; + } ... - sessions[0]._matchReason = 'recency-fallback'; - return sessions[0]; + return { session: sessions[0], matchReason: 'recency-fallback' };Then update the caller to destructure:
const { session: selected, matchReason } = selectMatchingSession(...); log(`[SessionStart] Selected: ${selected.path} (match: ${matchReason})`);As per coding guidelines: "Always create new objects, never mutate existing ones."
Also applies to: 94-95, 103-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hooks/session-start.js` around lines 85 - 86, The code is mutating the passed-in session by assigning session._matchReason (seen in the session._matchReason = 'worktree' line and similar assignments at the other locations); change selectMatchingSession (and the other branches where _matchReason is set) to return a new wrapper object like { session: originalSession, matchReason: 'worktree' } instead of mutating the session, and update callers to destructure the result (e.g., const { session: selected, matchReason } = selectMatchingSession(...)) so no input objects are modified.
128-129: Missing null check before accessingselectedproperties.If
selectMatchingSessionis updated to returnnullfor edge cases (or if the caller's guard is accidentally removed), this would throw.🛡️ Defensive check
const selected = selectMatchingSession(recentSessions, cwd, currentProject); + if (!selected) { + log('[SessionStart] No matching session found'); + return; + } log(`[SessionStart] Selected: ${selected.path} (match: ${selected._matchReason})`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/hooks/session-start.js` around lines 128 - 129, The code assumes selectMatchingSession(recentSessions, cwd, currentProject) always returns an object; add a defensive null/undefined check for the returned value (the selected variable) before accessing selected.path or selected._matchReason (in the session-start logic that logs the selection). If selected is null/undefined, log a suitable fallback message (e.g., "[SessionStart] No matching session found") or handle the case appropriately (skip accessing properties or return early) so the logger and subsequent code never dereference selected when it's falsy.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/hooks/session-start.js`:
- Around line 103-104: The function selectMatchingSession currently assumes the
sessions array has at least one element and sets sessions[0]._matchReason then
returns sessions[0], which will throw if sessions is empty; modify
selectMatchingSession to defensively check if sessions is empty and return null
(or undefined) early instead of accessing sessions[0], and update callers (e.g.,
the SessionStart flow that calls selectMatchingSession(recentSessions, cwd,
currentProject)) to handle a null return by logging '[SessionStart] No matching
session found' and skipping the selection logic when null is returned.
---
Nitpick comments:
In `@scripts/hooks/session-start.js`:
- Around line 122-136: selectMatchingSession currently reads the chosen session
file and then the caller reads it again; change selectMatchingSession to return
the already-read raw content along with the selected session and match reason
(e.g., return { session, matchReason, content }) and update the caller in the
session-start.js main flow to destructure those values (selected/session,
matchReason/_matchReason, and content), use the returned content (stripAnsi on
it) for additionalContextParts, and remove the redundant readFile(selected.path)
call and second file read; also ensure the log still reports selected.path and
the match reason from the returned value.
- Around line 85-86: The code is mutating the passed-in session by assigning
session._matchReason (seen in the session._matchReason = 'worktree' line and
similar assignments at the other locations); change selectMatchingSession (and
the other branches where _matchReason is set) to return a new wrapper object
like { session: originalSession, matchReason: 'worktree' } instead of mutating
the session, and update callers to destructure the result (e.g., const {
session: selected, matchReason } = selectMatchingSession(...)) so no input
objects are modified.
- Around line 128-129: The code assumes selectMatchingSession(recentSessions,
cwd, currentProject) always returns an object; add a defensive null/undefined
check for the returned value (the selected variable) before accessing
selected.path or selected._matchReason (in the session-start logic that logs the
selection). If selected is null/undefined, log a suitable fallback message
(e.g., "[SessionStart] No matching session found") or handle the case
appropriately (skip accessing properties or return early) so the logger and
subsequent code never dereference selected when it's falsy.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d3835056-8ff1-4dba-b9d9-d6b3c700664e
📒 Files selected for processing (1)
scripts/hooks/session-start.js
Contributor
Greptile SummaryThis PR fixes cross-project session contamination in the Key changes:
Confidence Score: 5/5Safe to merge — all previously raised P1 concerns are resolved and no new defects found All three prior review thread issues (double I/O, session/content mismatch, misleading fallback log) are correctly addressed. The logic is clean: fallbackSession/fallbackContent are set together ensuring consistency, the selected content is reused from the scan rather than re-read, and the fallback log only fires when fallbackSession is null (i.e., all files unreadable). No P0 or P1 issues remain. No files require special attention Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[SessionStart hook fires] --> B[dedupeRecentSessions\nsorted newest-first]
B --> C{recentSessions\n.length > 0?}
C -- No --> Z[No sessions to inject]
C -- Yes --> D[getProjectName + process.cwd]
D --> E[selectMatchingSession]
E --> F[For each session in order]
F --> G{readFile returns content?}
G -- No --> F
G -- Yes --> H{fallbackSession already set?}
H -- No --> I[Set fallbackSession = this session]
I --> J{sessionWorktree === normalizedCwd?}
H -- Yes --> J
J -- Yes --> K[Return worktree match immediately]
J -- No --> L{projectMatch already set?}
L -- No --> M{sessionProject === currentProject?}
M -- Yes --> N[Set projectMatch = this session]
N --> F
M -- No --> F
L -- Yes --> F
F -- loop done --> O{projectMatch found?}
O -- Yes --> P[Return project match]
O -- No --> Q{fallbackSession set?}
Q -- Yes --> R[Return recency fallback]
Q -- No --> S[Log: all unreadable / Return null]
K --> T[main: stripAnsi content]
P --> T
R --> T
T --> U{content has real data?}
U -- Yes --> V[Push to additionalContextParts]
U -- No --> W[Skip injection]
Reviews (4): Last reviewed commit: "fix: normalize worktree paths to handle ..." | Re-trigger Greptile |
Contributor
There was a problem hiding this comment.
2 issues found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/hooks/session-start.js">
<violation number="1" location="scripts/hooks/session-start.js:76">
P2: SessionStart now performs blocking per-session reads during selection and re-reads the chosen file, increasing startup latency.</violation>
<violation number="2" location="scripts/hooks/session-start.js:129">
P3: Only log a session as selected after confirming its file content can be read; otherwise this path reports a successful selection while injecting nothing.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
…ds, improve docstrings
- Return { session, content, matchReason } from selectMatchingSession()
to avoid reading the same file twice (coderabbitai, greptile P2)
- Add empty array guard: return null when sessions.length === 0 (coderabbitai)
- Stop mutating input objects — no more session._matchReason (coderabbitai)
- Add null check on result before accessing properties (coderabbitai)
- Only log "selected" after confirming content is readable (cubic-dev-ai P3)
- Add full JSDoc with @param/@returns (docstring coverage)
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="scripts/hooks/session-start.js">
<violation number="1" location="scripts/hooks/session-start.js:123">
P2: Fallback may return a different session’s content than the returned session path when the newest file is unreadable.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
When sessions[0] is unreadable, fallbackContent came from a later session (e.g. sessions[1]) while the returned session object still pointed to sessions[0]. This caused misleading logs and injected content from the wrong session — the exact problem this PR fixes. Now tracks fallbackSession alongside fallbackContent so the returned pair is always consistent. Addresses greptile-apps P1 review feedback.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/hooks/session-start.js`:
- Around line 101-107: Normalize and compare canonical paths instead of doing a
raw string equality between the extracted sessionWorktree and the current
working directory: resolve and realpath each side (use path.resolve +
fs.realpathSync) to collapse relative segments and symlinks, and on Windows also
compare in a case-insensitive way (toLowerCase both values); then replace the
direct equality check (the block using sessionWorktree and cwd / process.cwd())
with a comparison of the normalized values, falling back to resolved paths if
realpath fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 84b20ba3-6c51-43b0-970c-c4d375565ba7
📒 Files selected for processing (1)
scripts/hooks/session-start.js
On macOS /var is a symlink to /private/var, and on Windows paths may differ in casing (C:\repo vs c:\repo). Use fs.realpathSync() to resolve both sides before comparison so worktree matching is reliable across symlinked and case-insensitive filesystems. cwd is normalized once outside the loop to avoid repeated syscalls. Addresses coderabbitai Major review feedback.
peiking88
pushed a commit
to peiking88/everything-claude-code
that referenced
this pull request
Apr 4, 2026
…roject contamination (affaan-m#1054) * fix: filter session-start injection by cwd/project to prevent cross-project contamination The SessionStart hook previously selected the most recent session file purely by timestamp, ignoring the current working directory. This caused Claude to receive a previous project's session context when switching between projects, leading to incorrect file reads and project analysis. session-end.js already writes **Project:** and **Worktree:** header fields into each session file. This commit adds selectMatchingSession() which uses those fields with the following priority: 1. Exact worktree (cwd) match — most recent 2. Same project name match — most recent 3. Fallback to overall most recent (preserves backward compatibility) No new dependencies. Gracefully falls back to original behavior when no matching session exists. * fix: address review feedback — eliminate duplicate I/O, add null guards, improve docstrings - Return { session, content, matchReason } from selectMatchingSession() to avoid reading the same file twice (coderabbitai, greptile P2) - Add empty array guard: return null when sessions.length === 0 (coderabbitai) - Stop mutating input objects — no more session._matchReason (coderabbitai) - Add null check on result before accessing properties (coderabbitai) - Only log "selected" after confirming content is readable (cubic-dev-ai P3) - Add full JSDoc with @param/@returns (docstring coverage) * fix: track fallback session object to prevent session/content mismatch When sessions[0] is unreadable, fallbackContent came from a later session (e.g. sessions[1]) while the returned session object still pointed to sessions[0]. This caused misleading logs and injected content from the wrong session — the exact problem this PR fixes. Now tracks fallbackSession alongside fallbackContent so the returned pair is always consistent. Addresses greptile-apps P1 review feedback. * fix: normalize worktree paths to handle symlinks and case differences On macOS /var is a symlink to /private/var, and on Windows paths may differ in casing (C:\repo vs c:\repo). Use fs.realpathSync() to resolve both sides before comparison so worktree matching is reliable across symlinked and case-insensitive filesystems. cwd is normalized once outside the loop to avoid repeated syscalls. Addresses coderabbitai Major review feedback. --------- Co-authored-by: kuqili <kuqili@tencent.com>
slurpyb
added a commit
to slurpyb/everything-claude-code
that referenced
this pull request
Apr 17, 2026
…sion fallback When no session matches the current worktree or project name, selectMatchingSession() falls back to the most recent session regardless of project. This injects unrelated context that silently influences Claude's behavior in the wrong project. Add opt-in env var ECC_SESSION_STRICT_MATCH. When set to "1", the recency fallback is skipped — only worktree-matched or project-matched sessions are injected. Backward compatible: unset preserves existing behavior. Builds on affaan-m#1054 which introduced selectMatchingSession().
24 tasks
This was referenced Apr 19, 2026
FrancescoRosciano
pushed a commit
to FRosciano-Mambo/everything-claude-code
that referenced
this pull request
Jun 1, 2026
…roject contamination (affaan-m#1054) * fix: filter session-start injection by cwd/project to prevent cross-project contamination The SessionStart hook previously selected the most recent session file purely by timestamp, ignoring the current working directory. This caused Claude to receive a previous project's session context when switching between projects, leading to incorrect file reads and project analysis. session-end.js already writes **Project:** and **Worktree:** header fields into each session file. This commit adds selectMatchingSession() which uses those fields with the following priority: 1. Exact worktree (cwd) match — most recent 2. Same project name match — most recent 3. Fallback to overall most recent (preserves backward compatibility) No new dependencies. Gracefully falls back to original behavior when no matching session exists. * fix: address review feedback — eliminate duplicate I/O, add null guards, improve docstrings - Return { session, content, matchReason } from selectMatchingSession() to avoid reading the same file twice (coderabbitai, greptile P2) - Add empty array guard: return null when sessions.length === 0 (coderabbitai) - Stop mutating input objects — no more session._matchReason (coderabbitai) - Add null check on result before accessing properties (coderabbitai) - Only log "selected" after confirming content is readable (cubic-dev-ai P3) - Add full JSDoc with @param/@returns (docstring coverage) * fix: track fallback session object to prevent session/content mismatch When sessions[0] is unreadable, fallbackContent came from a later session (e.g. sessions[1]) while the returned session object still pointed to sessions[0]. This caused misleading logs and injected content from the wrong session — the exact problem this PR fixes. Now tracks fallbackSession alongside fallbackContent so the returned pair is always consistent. Addresses greptile-apps P1 review feedback. * fix: normalize worktree paths to handle symlinks and case differences On macOS /var is a symlink to /private/var, and on Windows paths may differ in casing (C:\repo vs c:\repo). Use fs.realpathSync() to resolve both sides before comparison so worktree matching is reliable across symlinked and case-insensitive filesystems. cwd is normalized once outside the loop to avoid repeated syscalls. Addresses coderabbitai Major review feedback. --------- Co-authored-by: kuqili <kuqili@tencent.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What Changed
Added
selectMatchingSession()toscripts/hooks/session-start.jsthat matches session files against the current working directory and project name before injecting into Claude's context.Priority:
Why This Change
The
SessionStarthook previously always injected the most recent session summary regardless of the current working directory. When users switch between projects, Claude receives the wrong project's session context (including file paths, project metadata, and task history) and incorrectly believes it's still in the previous project.session-end.jsalready writes**Project:**and**Worktree:**header fields into each session file — this PR simply uses them for matching.Testing Done
node tests/run-all.js)Type of Change
fix:Bug fixSecurity & Quality Checklist
Documentation
Summary by cubic
Filters session injection by current working directory and project to prevent cross‑project contamination. Normalizes worktree/cwd paths for reliable matching, keeps fallback session and content in sync, and only injects readable summaries.
selectMatchingSession()returning{ session, content, matchReason }; avoids duplicate I/O and keeps session/content aligned on fallbacks.fs.realpathSync()to handle symlinks and case differences; cwd normalized once.Written for commit f610b38. Summary will update on new commits.
Summary by CodeRabbit