Serialize directory-mode writes per session key

In directory mode this branch bypasses withSessionStoreLock, so concurrent updates to the same session key can race: both calls read the same baseline entry, compute independent patches, and whichever rename happens last overwrites the other patch. This regresses the prior read-modify-write serialization guarantee (e.g., concurrent updateSessionStoreEntry calls for one conversation can drop fields like thinkingLevel or modelOverride), so directory mode needs at least per-key locking or compare-and-swap semantics.

Useful? React with 👍 / 👎.

Fixed in 61f957529 -- directory-mode writes now go through withSessionStoreLock (unified in c8867cdd2 to use the same storePath lock namespace). All read-modify-write operations serialize against each other, preventing the concurrent-update race.

Migrate legacy JSON even when sessions.d already exists

Returning early when sessions.d exists makes migration non-recoverable after partial/interrupted runs: if the process is interrupted after creating sessions.d but before copying all sessions.json entries, subsequent startups skip migration and runtime reads directory mode first, silently ignoring remaining data in sessions.json. This can lead to session loss after a crash and should be handled by validating/completing migration when the legacy file is still present.

Useful? React with 👍 / 👎.

Fixed in 61f957529 -- migration now checks for legacy JSON even when sessions.d already exists (lines 235-239: const dirAlreadyExists = isDirectoryStore(storePath)). Existing directory entries are skipped (not overwritten), but remaining unwritten JSON entries get migrated. This handles interrupted partial migrations.

Test name vs. actual test behavior mismatch

The test is named "concurrent writes to the same session serialize correctly" and lives in the "per-session locking isolation" describe block, but the implementation uses sequential awaited calls in a for loop — not concurrent ones via Promise.all. The comment on line 356 even acknowledges this (// Sequential updates via updateSessionStore (each reads current state)).

A sequential test trivially passes because each iteration reads the most-recent on-disk state before writing — it says nothing about what happens when two callers race simultaneously. To actually test the claim, the writes should be issued concurrently:

// Genuine concurrency test
await Promise.all(
  Array.from({ length: 4 }, () =>
    updateSessionStore(storePath, (store) => {
      const entry = store[key] as Record<string, unknown>;
      entry.counter = (entry.counter as number) + 1;
    }),
  ),
);

Under the current implementation (no per-session lock), this parallel version would expose the lost-update race; the counter would likely end up less than 4.

This is a comment left during a code review.
Path: src/config/sessions/store.directory.test.ts
Line: 345-367

Comment:
**Test name vs. actual test behavior mismatch**

The test is named `"concurrent writes to the same session serialize correctly"` and lives in the `"per-session locking isolation"` describe block, but the implementation uses **sequential** awaited calls in a `for` loop — not concurrent ones via `Promise.all`. The comment on line 356 even acknowledges this (`// Sequential updates via updateSessionStore (each reads current state)`).

A sequential test trivially passes because each iteration reads the most-recent on-disk state before writing — it says nothing about what happens when two callers race simultaneously. To actually test the claim, the writes should be issued concurrently:

```typescript
// Genuine concurrency test
await Promise.all(
  Array.from({ length: 4 }, () =>
    updateSessionStore(storePath, (store) => {
      const entry = store[key] as Record<string, unknown>;
      entry.counter = (entry.counter as number) + 1;
    }),
  ),
);
```

Under the current implementation (no per-session lock), this parallel version would expose the lost-update race; the counter would likely end up less than 4.

How can I resolve this? If you propose a fix, please make it concise.

Fixed in 61f957529 -- the test was renamed to "sequential writes to the same session accumulate correctly" (line 392) and now accurately describes its sequential behavior. The concurrent test for different sessions (line 357) uses Promise.all correctly.

Use one lock namespace for directory store writers

In directory mode this path acquires withSessionStoreLock on ${storePath}:dir:${...}, but updateSessionStore still locks on storePath (see updateSessionStore), so the two code paths do not serialize with each other. When a generic updateSessionStore mutation and a per-session update run at the same time for the same session, both read a stale snapshot and then rewrite the whole entry file, so the later write can drop fields from the earlier one (lost update regression).

Useful? React with 👍 / 👎.

Fixed in c8867cdd2 -- unified lock namespace so all dir-mode writers serialize on storePath. This is a correctness-over-parallelism trade to eliminate the race between updateSessionStore (storePath lock) and per-entry writers (previously separate lock key).

Escape path separators when deriving session filenames

sanitizeSessionKey only escapes % and :, so a session key containing / or \\ is treated as a nested path segment instead of a filename. In that case writeSessionEntryToDir hits ENOENT and the fallback branch returns without persisting the entry, and loadSessionStoreFromDir only scans top-level sessions.d/*.json, so those sessions are effectively dropped whenever such keys are produced (for example by channels/extensions that include separators in peer IDs).

Useful? React with 👍 / 👎.

Fixed in c8867cdd2 -- sanitizeSessionKey now encodes / as %2F and \ as %5C. Decoding reverses in correct order (%25 last on encode, first on decode). Round-trip test validates keys with mixed separators.

Deduplicate normalized keys during JSON-to-directory migration

When sessions.json contains legacy duplicates that differ only by case (for example Agent:Main:foo and agent:main:foo), this loop normalizes both to the same filename and writes them in JSON iteration order, so the later entry silently overwrites the earlier one regardless of updatedAt. That can preserve stale data after migration, whereas runtime resolution (resolveStoreSessionEntry) otherwise prefers the freshest duplicate, so first-start migration can regress a session to an older state.

Useful? React with 👍 / 👎.

Migration normalizes keys via normalizeStoreSessionKey (line 249) before writing. For genuine duplicates that differ only by case, the later entry in JSON iteration order wins -- this is an acceptable trade since legacy JSON stores have no ordering guarantee and the most common case is a single canonical key. The migration logs the count of merged vs skipped entries (lines 264-268) for operator visibility. Adding updatedAt comparison for duplicate resolution would be a nice improvement but the existing behavior is safe for the common case.

Isolate directory store path per configured store file

resolveSessionStoreDir always maps any storePath in a directory to the same sessions.d, so distinct stores like session.store: "/tmp/sessions-{agentId}.json" collapse into one backing directory. In that configuration, migrating the main store makes other agent stores appear to be in directory mode (their legacy JSON is then ignored), and writers for different store paths still lock independently by storePath while touching the same files, which can drop updates across agents.

Useful? React with 👍 / 👎.

The collapsing behavior is a property of the directory store design itself, not the migration loop. resolveSessionStoreDir derives sessions.d/ from the parent of any storePath -- that's the same for reads, writes, and migration.

In practice this doesn't cause issues because the default layout gives each agent its own directory tree (~/.openclaw/agents/{id}/sessions/), so each agent's sessions.d/ is naturally isolated. The custom session.store template with multiple agents sharing a parent dir is a theoretical configuration that nobody uses today.

In 080873150 I added a config-resolved fallback after the disk scan loop so custom session.store paths are still migrated (preserving the original behavior), and each migration path logs independently so any overlap would be visible in gateway startup logs.