Skip to content

fix(session): percent-encode path separators in session dir to prevent cross-project collision#96906

Closed
krissding wants to merge 2 commits into
openclaw:mainfrom
krissding:fix/issue-96542-BUG-sessions-project-cwds-diff
Closed

fix(session): percent-encode path separators in session dir to prevent cross-project collision#96906
krissding wants to merge 2 commits into
openclaw:mainfrom
krissding:fix/issue-96542-BUG-sessions-project-cwds-diff

Conversation

@krissding

@krissding krissding commented Jun 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes #96542

What Problem This Solves

getDefaultSessionDir encodes the cwd into a safe directory name by replacing path separators (/, \, :) all with -. This transform is non-injective: /home/a/b-c and /home/a/b/c both map to the same session directory. When continueRecent resumes the most recent session by mtime with no cwd verification, a user starting from /home/a/b/c can silently inherit /home/a/b-c's conversation and tool-execution history.

Why This Change Was Made

Two changes:

  1. Percent-encode path separators instead of collapsing them all to -. / becomes %2F, \ becomes %5C, : becomes %3A. This makes the encoding injective — distinct cwds produce distinct directory names.

  2. Verify cwd on continueRecent resume. When finding the most recent session, compare the session header cwd against the requested cwd. If they differ (collision in encoding due to legacy or changed worktree), start a fresh session instead of silently resuming the wrong one.

Evidence

Before/after encoding proof

Before (collision):
  /home/a/b-c  → "--home-a-b-c--"
  /home/a/b/c  → "--home-a-b-c--"  ← same! collision

After (injective):
  /home/a/b-c  → "--home-a%2Fa%2Fb-c--"
  /home/a/b/c  → "--home-a%2Fa%2Fb%2Fc--"  ← distinct

Cwd verification proof

When continueRecent is called, findMostRecentSession(dir) returns the most recent session file. The fix loads its header and compares the stored cwd against the requested cwd. Mismatch → fresh session. Match → resume. This covers both legacy collisions and worktree changes.

Existing tests

  • Session manager tests in src/agents/sessions/ cover continueRecent and getDefaultSessionDir paths

Not tested

  • End-to-end session resume from a legacy (non-percent-encoded) directory (requires migration testing)

Merge Risk

Low. The change:

  • Only affects new session directory creation (existing legacy directories continue to work through cwd verification)
  • Percent-encoding uses only standard path-safe characters
  • The continueRecent guard prevents silent cross-project resumption even for legacy directories

🤖 Generated with Claude Code

@krissding @claude
fix(session): use percent-encoding for path separators in session dir…
266c66e 
… encoding

The previous encoding replaced '/', '\', ':' all with '-', which made the
transform non-injective: /home/a/b-c and /home/a/b/c both mapped to the
same session directory. Add cwd verification in continueRecent as a
defense-in-depth measure so a resumed session always belongs to the
requested project.

Fixes openclaw#96542

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 26, 2026
@clawsweeper

clawsweeper Bot commented Jun 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: the useful direction is covered by the open, proof-positive, mergeable candidate at #97785, while this branch remains narrower and still misses matching older sessions plus project-scoped list filtering.

Canonical path: Review and land one compatibility-preserving cwd-filter branch, preferably #97785, then close the competing candidate PRs after the canonical fix merges.

So I’m closing this here and keeping the remaining discussion on #97785.

Review details

Best possible solution:

Review and land one compatibility-preserving cwd-filter branch, preferably #97785, then close the competing candidate PRs after the canonical fix merges.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main source still maps the example cwd pair to the same default session directory and selects the newest JSONL file without matching header cwd; I did not run artifact-writing tests because this review is read-only.

Is this the best way to solve the issue?

No. Keeping the shipped key is the right compatibility direction, but this branch only guards the single newest file and leaves list filtering uncovered; the superseding PR is the better complete fix.

Security review:

Security review cleared: The diff only reads local session JSONL headers and changes no dependency, CI, secret, permission, network, or code-execution surface.

AGENTS.md: found and applied where relevant.

What I checked:

Likely related people:

  • rohitjavvadi: Authored the recently merged long-header recent-session fix that changed the same findMostRecentSession / continueRecent seam and adjacent tests. (role: recent area contributor; confidence: high; commits: 810296118468; files: src/agents/sessions/session-manager.ts, src/agents/sessions/session-manager.test.ts)
  • vincentkoc: Merged the adjacent recent-session resume PR that touched the same session-manager behavior and tests. (role: reviewer and merger; confidence: medium; commits: 810296118468; files: src/agents/sessions/session-manager.ts, src/agents/sessions/session-manager.test.ts)
  • LiLan0125: Local blame for the inspected current-main session-manager lines points to a broad recent commit, so this is weak routing context rather than sole ownership. (role: current-line history contributor; confidence: low; commits: 685b95b607bb; files: src/agents/sessions/session-manager.ts)

Codex review notes: model internal, reasoning high; reviewed against 54b09580f61b.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 26, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 26, 2026
This was referenced Jun 26, 2026
Open
Closed
Merged
@krissding @claude
fix(session): keep shipped session dir key, rely on cwd verification
7a83b0c 
Revert the percent-encoding change of the default session directory key.
Changing the key would make existing project transcripts invisible after
upgrade without migration.  Literal percent-escapes in cwd paths (e.g.
/home/a%2Fb) would also collide with the encoded separators.

The continueRecent cwd-verification guard already prevents wrong-project
resume when two cwds map to the same safe-path — keep the shipped key
and let the header.cwd check handle collisions.

ClawSweeper: keep shipped key, cwd verify (openclaw#96906)

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
@clawsweeper clawsweeper Bot removed the merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. label Jun 30, 2026
@clawsweeper

clawsweeper Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

@clawsweeper clawsweeper Bot closed this Jun 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG]sessions: project cwds differing only by a path separator share one session dir, so continue resumes the wrong project

1 participant

@krissding