fix(workspace): dedupe workspaces across Windows path spelling variants#1809
Conversation
The same directory reached the workspace registry as distinct strings on Windows (drive-letter casing, typed vs on-disk casing, slash style), and every identity check compared exact strings, so one folder could appear as multiple workspaces with sessions split across hash-keyed buckets. - add workspaceRootKey (slash-normalize + case-fold Windows-shaped paths) in agent-core, agent-core-v2, and the web app, and compare roots by identity key everywhere instead of exact strings - registry createOrTouch folds alias spellings onto the existing entry instead of minting a new workspace id; session buckets reuse the registered id via a resolver in the v1 session store - list endpoints expand alias buckets (resolveAliasIds / resolveAliasWorkDirs, including session-index-only spellings) so previously split workspaces list all sessions and counts under one merged group; session_index entries use the registry-resolved id
🦋 Changeset detectedLatest commit: 7d940b9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 714f42b688
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const resolveWorkspaceId = | ||
| options.resolveWorkspaceId ?? | ||
| ((workDir: string) => workspaceRegistry.findWorkspaceIdByRoot(workDir)); |
There was a problem hiding this comment.
When this default resolver is active, the first session created from an alias of an already-registered Windows root is placed in the original bucket, but KimiCore.createSession still calls touchWorkspaceRegistry(this.homeDir, workDir) afterwards; that helper mints and stores the alias id. On the next create with the same alias, findWorkspaceIdByRoot(..., preferred alias id) returns that newly persisted alias id, so sessions split into the duplicate bucket again. The touch path needs the same folded lookup or needs to reuse the resolved id.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in cf46bd0. touchWorkspaceRegistry now folds an alias spelling onto the identity-matching existing entry (same workspaceRootKey comparison as the registry service) instead of minting a new id, so the touch can no longer re-introduce a preferred alias bucket. Regression test added for the fold and for POSIX distinctness.
| export function workspaceRootKey(root: string): string { | ||
| const normalized = root.replaceAll('\\', '/').replace(/\/+$/, ''); | ||
| return WIN_SHAPED.test(normalized) ? normalized.toLowerCase() : normalized; |
There was a problem hiding this comment.
For drive-root workspaces such as C:\, the trailing-separator strip turns the normalized key into C: before WIN_SHAPED is tested, so it no longer matches the drive-root pattern and C:\/c:\ are not case-folded. That leaves drive-root workspaces with the same duplicate-id and split-bucket behavior this change is meant to fix; test the Windows shape before removing the only separator or special-case drive roots.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in cf46bd0. The shape test now runs before the trailing-separator strip in all three workspaceRootKey copies (agent-core, agent-core-v2, web), so drive roots like C:\ fold correctly. Regression tests added.
Two gaps in the Windows path-spelling folding, both reachable in the v1 session-create flow: - touchWorkspaceRegistry minted the alias spelling's id outright; the freshly persisted alias entry then became the resolver's preferred id on the next create, splitting sessions into a duplicate bucket again. It now folds onto the identity-matching existing entry, mirroring the registry service. - workspaceRootKey stripped trailing separators before testing the Windows shape, so a drive root (C:\) collapsed to C: and escaped the case-fold. The shape test now runs before the strip in all three copies (agent-core, agent-core-v2, web).
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf46bd04fe
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| // grouped under, the only rendered workspace would look empty. | ||
| // "Same root?" is always decided by the folded key; the first-seen original | ||
| // string is kept for display. | ||
| const hidden = new Set(hiddenWorkspaceRoots.map(workspaceRootKey)); |
There was a problem hiding this comment.
When a user removes C:\Foo and later adds the same Windows directory as c:\foo or with different slashes, the stored hidden root is not cleared because upsertWorkspacePreserveOrder still removes hidden entries by exact string. This new folded hidden set then keeps hiding the re-added workspace, so the add flow can succeed while the group remains absent until localStorage is cleared. Clear hidden entries using the same workspaceRootKey comparison on re-add.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — the hide/unhide sides were asymmetric. upsertWorkspacePreserveOrder now clears hidden entries by the same folded workspaceRootKey comparison (in useWorkspaceState.ts), so re-adding an alias spelling un-hides the workspace. Fixed in 7d940b9 with regression tests (Windows-fold unhide, POSIX case-distinct stays hidden).
| const rootKey = workspaceRootKey(ws.root); | ||
| const existing = byRoot.get(rootKey); | ||
| if (existing === undefined) { | ||
| byRoot.set(ws.root, ws); | ||
| byRoot.set(rootKey, ws); |
There was a problem hiding this comment.
When workspaces.json already has two Windows spellings for one folder, this folded dedupe exposes only one representative, but delete(id) still removes and tombstones only that exact id. After DELETE /workspaces/{id}, the next list() can select the remaining alias for the same root key and the workspace reappears for server/API clients. Delete or tombstone every registered id sharing this workspaceRootKey, or suppress the whole root key when any alias is tombstoned.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — delete now collects the full folded alias set (every registered spelling sharing the root key, plus session-index-only spelling mints) and removes/tombstones all of them, so neither a sibling entry nor the session-index merge can resurrect the directory. Applied symmetrically to the v1 registry as well (including both raw and normalized mint forms for derived tombstones). Fixed in 7d940b9 with regression tests covering deletion, listing, and a fresh-process merge.
… key Two asymmetric spots left the folded comparison one-sided: - the web app matched hidden roots by folded key but cleared them on re-add by exact string, so hiding C:\Foo and re-adding c:\foo kept the workspace hidden forever; clearing now folds too - registry delete (both engines) removed and tombstoned only the exact id, so a legacy split sibling resurfaced as the directory's representative on the next list; delete now removes every registered spelling sharing the root's identity key and tombstones the full alias set (registered ids plus session-index spelling mints), so the session-index merge cannot resurrect the directory either
1 participant
Related Issue
None filed — the problem is described below.
Problem
On Windows, the same folder reaches the server as several different path strings: the drive letter keeps the casing the user typed (
c:\vsC:\), the folder browser returnsrealpathon-disk casing, and slash style varies. Workspace identity is a hash of the raw string and every "same directory?" comparison was exact-string, so:sessions/<wd_id>/. Split spellings meant split buckets, and each sidebar group only pages its own bucket, so history sessions living in the sibling bucket were unreachable from the UI.What changed
Fold, don't rewrite: stored ids, bucket layout, and
workspaces.jsonare untouched (zero migration); only identity comparisons and read queries fold variants.workspaceRootKey()inagent-core,agent-core-v2, and the web app: slash-normalizes, strips trailing separators, and case-folds only Windows-shaped paths (shape-detected, notprocess.platform, so browsers/tests fold identically; POSIX paths never fold). Every "same root?" check — registry dedup, sidebar merge, session-to-workspace assignment, hidden roots — now compares by this key.createOrTouchfolds an alias spelling onto the existing entry and reuses its id in both engines; the v1 session store resolves new sessions into the registered bucket instead of minting a divergent one;session_index.jsonlentries use the registry-resolved workspace id.resolveAliasIds/resolveAliasWorkDirsexpand a workspace to every spelling known to the registry and session index;GET /sessions?workspace_id=returns the union across alias buckets with unchanged ordering/cursor semantics, andsession_countsums alias buckets, so pre-existing duplicates collapse into one complete group with no data migration.klient's sessionIndex contract/facade was updated for the widened
workspaceIds/countActivesignature; the public/api/v1REST wire is unchanged.Approach rationale: ids are deliberately not re-canonicalized — changing the hash input would orphan every existing session bucket (a previous realpath-based incarnation caused exactly that). Folding at comparison/query time fixes new and legacy data alike. Per-directory case sensitivity and 8.3/junction aliases are documented non-goals of the string layer; a realpath probe can extend the alias set later without touching storage.
Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.