Empty sessions view: cannot call joinPath#309777
Conversation
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @jriekenMatched files:
|
There was a problem hiding this comment.
Pull request overview
Fixes a crash in the Agents/Sessions window caused by URI.joinPath being invoked on URIs with an empty/missing path (e.g. empty sessions view / missing repo metadata), which previously threw [UriError]: cannot call joinPath on URI without path.
Changes:
- Adjusts the Copilot chat sessions adapter to use a fallback repository URI that includes a path (
unknown:///) when repo metadata is missing. - Changes
URI.joinPathto treat an empty path as'/'instead of throwing.
Show a summary per file
| File | Description |
|---|---|
| src/vs/sessions/contrib/copilotChatSessions/browser/copilotChatSessionsProvider.ts | Uses a safer fallback URI (unknown:///) to avoid downstream joinPath failures when repo URI is missing. |
| src/vs/base/common/uri.ts | Alters core URI.joinPath behavior for empty-path URIs (no longer throws; normalizes to '/'). |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 1
| // treat empty path as slash to simplify join and normalization | ||
| uri = uri.with({ path: _slash }); |
There was a problem hiding this comment.
URI.joinPath used to throw for URIs without a path (e.g. URI.parse('foo:') or URI.parse('foo://bar')). With this change it will silently treat an empty path as '/', which contradicts the existing contract validated by src/vs/base/test/common/uri.test.ts (the URI#joinPath test currently asserts these cases throw) and will break those tests/consumers. Consider keeping the throw and fixing the callers that construct path-less URIs (ensure they use a '/' path), or narrowly applying the fallback only for schemes where empty-path join semantics are well-defined and tests are updated accordingly.
| // treat empty path as slash to simplify join and normalization | |
| uri = uri.with({ path: _slash }); | |
| throw new Error(`[UriError]: cannot join path on URI without path`); |
FIY @sandy081 @bpasero Maybe you want to revisit
unknown:///, not clear what kind of errors the user will see with this unresolvable location.