Canonicalize workspace roots for session visibility#158
Conversation
Review Summary by QodoCanonicalize workspace roots for consistent session visibility
WalkthroughsDescription• Canonicalize workspace roots and thread cwd values through realpath • Ensures sessions remain visible with symlinked workspace paths • Exports canonicalization functions for workspace roots state • Adds comprehensive test coverage and manual regression tests Diagramflowchart LR
A["Thread/List Response"] -->|canonicalizeThreadListResponseForRead| B["Canonicalized CWD Values"]
C["Workspace Roots State"] -->|canonicalizeWorkspaceRootsStateForRead| D["Canonicalized Paths"]
B --> E["Sessions Visible Regardless of Symlink"]
D --> E
File Changes1. src/server/codexAppServerBridge.ts
|
Code Review by Qodo
1. Label overwrite race
|
| const labels: Record<string, string> = { ...state.labels } | ||
| await Promise.all(Object.entries(state.labels).map(async ([key, label]) => { | ||
| const canonicalKey = await canonicalizeWorkspaceRootPath(key, pathRealpath) | ||
| labels[canonicalKey] = label | ||
| })) |
There was a problem hiding this comment.
1. Label overwrite race 🐞 Bug ≡ Correctness
canonicalizeWorkspaceRootsStateForRead concurrently assigns canonicalized label keys into a shared object; if multiple original keys (e.g., symlink + target) realpath to the same canonical key, the winning label becomes dependent on async completion order and can flip between reads. This is user-visible because the sidebar hydrates display names from rootsState.labels.
Agent Prompt
## Issue description
`canonicalizeWorkspaceRootsStateForRead` builds `labels` by copying `state.labels` and then `Promise.all`-writing `labels[canonicalKey] = label` concurrently. If two different keys canonicalize to the same `canonicalKey` (common when both symlink and target were saved), the final label is nondeterministic (depends on which `realpath` resolves last).
## Issue Context
This function is used on every `/codex-api/workspace-roots-state` read, and the UI iterates `rootsState.labels` to hydrate project display names.
## Fix Focus Areas
- src/server/codexAppServerBridge.ts[3896-3917]
## Suggested fix approach
- Build a **new** `labels` map deterministically instead of mutating a shared object from concurrent tasks.
- Make collision policy explicit, e.g.:
- Prefer an entry whose original key is already canonical (`key === canonicalKey`) over a symlink-derived key.
- Or process entries sequentially in stable key order.
- Consider dropping non-canonical keys from the returned `labels` to avoid returning both forms.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Summary
canonicalize saved workspace roots with local realpath
canonicalize thread/list cwd values before sidebar filtering
keep sessions visible when symlink and target paths are mixed
add regression tests and manual test coverage
Verification