fix(sessions): make session path encoding injective and verify cwd on resume by maweibin · Pull Request #96679 · openclaw/openclaw

maweibin · 2026-06-25T06:08:45Z

What Problem This Solves

Issue #96542 reports that two genuinely different project directories whose paths differ only by a path separator (e.g., client/app vs client-app) get encoded to the same session directory. When a user runs continueRecent to resume the most recent session, it silently loads the other project's conversation history. The agent then executes tools in the requested cwd while carrying an unrelated project's transcript — with no cwd check ever catching the mismatch.

Why This Change Was Made

Two linked defects:

Non-injective encoding in getDefaultSessionDir: /, \, : all mapped to -, so client/app and client-app produced the same encoded dir.
No cwd verification on resume: findMostRecentSession picks the newest file by mtime without checking header.cwd against the requested cwd.

The fix replaces the lossy mapping with percent-encoding (/ → %2F, \ → %5C, : → %3A, with % escaped first) and adds cwd verification in continueRecent. Only src/agents/sessions/session-manager.ts changed.

Evidence

Encoding is now injective — colliding path pairs produce different session dirs, normal paths unchanged, percent signs escaped:

$ node --import tsx -e "
import { tmpdir } from 'node:os';
import { mkdirSync } from 'node:fs';
import { getDefaultSessionDir } from './src/agents/sessions/session-manager.ts';
const base = tmpdir() + '/openclaw-' + Date.now();
mkdirSync(base, { recursive: true });
const cases = [
  { label: '/ vs -', a: '/home/user/client/app', b: '/home/user/client-app' },
  { label: 'normal project', a: '/home/user/project' },
  { label: 'percent sign', a: '/home/user/100%25done' },
  { label: 'sub vs sub-', a: '/home/user/sub/proj', b: '/home/user/sub-proj' },
];
for (const c of cases) {
  const dirA = getDefaultSessionDir(c.a, base);
  if (c.b) {
    const dirB = getDefaultSessionDir(c.b, base);
    console.log(JSON.stringify({ [c.label]: { a: dirA.replace(base,'~'), b: dirB.replace(base,'~'), collision: dirA===dirB } }));
  } else {
    console.log(JSON.stringify({ [c.label]: dirA.replace(base,'~') }));
  }
}
"
{
  "/ vs -": {
    "a": "~/sessions/--home%2Fuser%2Fclient%2Fapp--",
    "b": "~/sessions/--home%2Fuser%2Fclient-app--",
    "collision": false
  }
}
{
  "normal project": "~/sessions/--home%2Fuser%2Fproject--"
}
{
  "percent sign": "~/sessions/--home%2Fuser%2F100%2525done--"
}
{
  "sub vs sub-": {
    "a": "~/sessions/--home%2Fuser%2Fsub%2Fproj--",
    "b": "~/sessions/--home%2Fuser%2Fsub-proj--",
    "collision": false
  }
}

Regression tests: 94 tests pass (2 files, including new coverage for encoding and cwd resume verification).

 Test Files  2 passed (2)
      Tests  94 passed (94)

Format / lint: oxfmt --check clean, oxlint clean, git diff --check no output.

AI-assisted: Claude Code (Anthropic).

… resume (openclaw#96542) getDefaultSessionDir replaces path separators with '-' which means "client/app" and "client-app" produce identical encoded paths, causing session cross-contamination. Fix: use percent-encoding (%2F/%5C/%3A) to distinguish separators from literal hyphens. Also add cwd verification in continueRecent so that if a session file from a colliding directory is found, the code scans remaining files for one matching the requested cwd before falling back to a fresh session. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

clawsweeper · 2026-06-25T06:09:54Z

Codex review: needs real behavior proof before merge. Reviewed June 25, 2026, 9:04 PM ET / 01:04 UTC.

Summary
The PR changes getDefaultSessionDir to percent-encode path separators and makes continueRecent compare the selected session header cwd against the requested cwd in src/agents/sessions/session-manager.ts.

PR surface: Source +39. Total +39 across 1 file.

Reproducibility: yes. source-reproducible: current main and v2026.6.10 fold path separators into -, and continueRecent opens the newest valid file without a header-cwd match. I did not run an artifact-writing repro during this read-only review.

Review metrics: 1 noteworthy metric.

Default session directory key: 1 changed. This helper controls persisted project transcript lookup, so upgrade behavior must be proven before merge.

Stored data model
Persistent data-model change detected: serialized state: src/agents/sessions/session-manager.ts, unknown-data-model-change: src/agents/sessions/session-manager.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96542
Summary: The canonical issue is the cwd separator-collision report; this PR and another open PR are competing candidate fixes, while nearby session PRs cover adjacent behavior.

Members:

canonical: [BUG]sessions: project cwds differing only by a path separator share one session dir, so continue resumes the wrong project #96542 - The issue describes the same cwd separator collision and wrong-project resume behavior that this PR attempts to fix.
fixed_by_candidate: fix(session): use injective cwd encoding to prevent cross-project session collision (fixes #96542) (AI-assisted) #96547 - That open PR targets the same canonical issue but has its own compatibility blockers, so it is related but not a safe superseding landing path.
adjacent_distinct: fix(security): enforce private directory mode 0o700 for managed state dirs #96336 - That PR edits getDefaultSessionDir for managed-directory permissions, not cwd encoding or cwd-aware resume.
adjacent_distinct: continue recent can skip valid sessions with long headers #94577 - That closed issue covered long JSONL headers causing valid sessions to be skipped, not colliding project directories.
adjacent_distinct: Fix recent session resume with long headers #94578 - That merged PR fixed long-header recent-session discovery in the same module but does not supersede this collision fix.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Preserve or migrate existing default session directories before changing the lookup key.
[P1] Validate header cwd values before calling resolve() during resume.
[P1] Add redacted terminal/live output for colliding-cwd resume and legacy upgraded-directory discovery; redact private paths, tokens, phone numbers, IPs, and non-public endpoints.

Proof guidance:

[P1] Needs stronger real behavior proof before merge: The PR body includes copied encoder output and test summaries, but not after-fix real resume behavior or upgraded legacy-directory discovery proof; add redacted terminal/live output or logs and update the PR body for re-review.

Risk before merge

[P1] Existing default project transcripts can become invisible to continueRecent and SessionManager.list(cwd) after upgrade because the PR computes a new directory name without migration or legacy lookup.
[P2] A malformed but otherwise accepted session header with non-string cwd can make continueRecent throw before it reaches the fallback scan.
[P1] The PR body's copied output shows the new encoder path, but not an after-fix colliding-cwd resume or upgraded legacy-directory discovery path.

Maintainer options:

Preserve or migrate shipped session directories (recommended)
Keep existing default session directories reachable, or add a tested migration before adopting the new percent-encoded canonical key.
Accept the upgrade break intentionally
Maintainers could intentionally accept that existing default sessions appear missing, but that should be an explicit release and support decision.
Pause competing collision fixes
Because fix(session): use injective cwd encoding to prevent cross-project session collision (fixes #96542) (AI-assisted) #96547 targets the same canonical issue, maintainers can choose one persistent-layout direction and rework or close the other.

Next step before merge

[P1] Contributor or maintainer follow-up is needed because the patch has upgrade-safe code blockers and still needs contributor-supplied real behavior proof; this read-only review should not dispatch repair markers.

Security
Cleared: The diff changes local session path derivation and JSONL header reads; no dependency, CI, secret, permission, or code-execution surface expansion was found.

Review findings

[P1] Preserve existing default session directories — src/agents/sessions/session-manager.ts:440-443
[P2] Validate session cwd before resolving it — src/agents/sessions/session-manager.ts:1210

Review details

Best possible solution:

Preserve or migrate shipped default session directories while making resume/list select only sessions with a validated string header.cwd matching the requested cwd, with focused fresh-install and upgrade proof.

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

Yes, source-reproducible: current main and v2026.6.10 fold path separators into -, and continueRecent opens the newest valid file without a header-cwd match. I did not run an artifact-writing repro during this read-only review.

Is this the best way to solve the issue?

No. The PR touches the right session-manager boundary, but changing the shipped directory key without migration or legacy discovery is not upgrade-safe, and the new cwd check should validate the header type before resolving it.

Full review comments:

[P1] Preserve existing default session directories — src/agents/sessions/session-manager.ts:440-443
Changing this encoder moves existing default sessions from shipped names like --home-alice-dev-- to new names like --home%2Falice%2Fdev--. Since continueRecent() and list(cwd) compute only this new directory, upgraded users' transcripts become undiscoverable unless the PR adds migration or legacy lookup.
Confidence: 0.94
[P2] Validate session cwd before resolving it — src/agents/sessions/session-manager.ts:1210
readSessionCwd() returns parsed.cwd without checking that it is a string, while isValidSessionFile() only requires a session header with a string id. A malformed but accepted header can make resolve(sessionCwd) throw in continueRecent; return only string cwd values or keep resolution inside the helper's catch path.
Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

P2: The PR targets a deterministic session-state bug with limited blast radius, but it needs focused compatibility repair before merge.
merge-risk: 🚨 compatibility: Changing the default encoded session directory without migration can break upgraded users' access to existing transcripts.
merge-risk: 🚨 session-state: The diff changes how project transcripts are located and resumed, which can make history appear missing or crash on malformed headers.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes copied encoder output and test summaries, but not after-fix real resume behavior or upgraded legacy-directory discovery proof; add redacted terminal/live output or logs and update the PR body for re-review.

Evidence reviewed

PR surface:

Source +39. Total +39 across 1 file.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	41	2	+39
Tests	0	0	0	0
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	1	41	2	+39

Acceptance criteria:

[P1] node scripts/run-vitest.mjs src/agents/sessions/session-manager.test.ts.
[P1] git diff --check.

What I checked:

Repository policy applied: Root AGENTS.md and scoped src/agents/AGENTS.md were read fully; session state, persisted paths, upgrade compatibility, and whole-path PR review guidance affect this review. (AGENTS.md:14, a0b397748fa3)
Current main still has the reported collision path: Current main folds /, \, and : to - in the default session directory key, so separator differences and literal hyphens can collide. (src/agents/sessions/session-manager.ts:439, a0b397748fa3)
Default callers depend on that persisted key: create, continueRecent, forkFrom, and list(cwd) all derive the default project session directory through getDefaultSessionDir when no explicit session directory is supplied. (src/agents/sessions/session-manager.ts:2894, a0b397748fa3)
Latest release shipped the old key: v2026.6.10 contains the same separator-to-hyphen default session directory encoding, so changing it is an upgrade-sensitive persisted-session change. (src/agents/sessions/session-manager.ts:436, aa69b12d0086)
PR changes lookup key without migration: The PR replaces the shipped directory key with percent escaping, but the diff does not add migration or legacy lookup before continueRecent and list(cwd) compute the new path. (src/agents/sessions/session-manager.ts:440, 1e886d4b2ee7)
Header cwd type is not validated: The PR returns parsed.cwd directly and later calls resolve(sessionCwd); current validation only requires a session header with a string id, so a malformed but accepted header can throw during resume. (src/agents/sessions/session-manager.ts:1210, 1e886d4b2ee7)

Likely related people:

obviyus: Current checkout blame for getDefaultSessionDir, findMostRecentSession, and continueRecent points to ecd29fe57269, whose GitHub metadata maps the committer to this handle; the commit is broad, so this is a routing signal rather than sole ownership. (role: current line-history committer; confidence: medium; commits: ecd29fe57269; files: src/agents/sessions/session-manager.ts)
rohitjavvadi: Authored the recently merged long-header recent-session resume fix touching the same findMostRecentSession and continueRecent path plus adjacent tests. (role: recent area contributor; confidence: high; commits: 810296118468, 695cea68f546; files: src/agents/sessions/session-manager.ts, src/agents/sessions/session-manager.test.ts)
vincentkoc: Merged and reviewed the adjacent long-header recent-session fix in the same module, with co-authored metadata on the merge commit. (role: reviewer and merger; confidence: medium; commits: 695cea68f546; files: src/agents/sessions/session-manager.ts, src/agents/sessions/session-manager.test.ts)
steipete: The agent-runtime internalization commit brought in the current OpenClaw-owned session-manager surface that this PR modifies. (role: introduced current session-manager shape; confidence: medium; commits: bb46b79d3c14; files: src/agents/sessions/session-manager.ts, src/agents/sessions/agent-session-runtime.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

maweibin · 2026-06-25T14:21:05Z

@clawsweeper re-review

clawsweeper · 2026-06-25T14:21:07Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

maweibin · 2026-06-26T00:57:55Z

@clawsweeper re-review

maweibin · 2026-06-29T10:43:37Z

Closing as this PR has low merge likelihood (unranked/silver rating, missing proof, security concerns). The fix approach may be revisited with better evidence in a future PR.

openclaw-barnacle Bot added agents Agent runtime and tooling size: XS labels Jun 25, 2026

maweibin closed this Jun 29, 2026

clawsweeper Bot mentioned this pull request Jun 29, 2026

fix(sessions): avoid cross-cwd recent resumes #97785

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(sessions): make session path encoding injective and verify cwd on resume#96679

fix(sessions): make session path encoding injective and verify cwd on resume#96679
maweibin wants to merge 1 commit into
openclaw:mainfrom
maweibin:fix/session-path-collision-96542

maweibin commented Jun 25, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading

Uh oh!

maweibin commented Jun 25, 2026

Uh oh!

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading

Uh oh!

maweibin commented Jun 26, 2026

Uh oh!

maweibin commented Jun 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

maweibin commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

Evidence

Uh oh!

clawsweeper Bot commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maweibin commented Jun 25, 2026

Uh oh!

clawsweeper Bot commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maweibin commented Jun 26, 2026

Uh oh!

maweibin commented Jun 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

maweibin commented Jun 25, 2026 •

edited

Loading

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading