Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions src/server/localBrowseUi.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { describe, expect, it } from 'vitest'
import { decodeBrowsePath } from './localBrowseUi'

describe('decodeBrowsePath', () => {
it('removes the browse-route slash before Windows drive paths', () => {
expect(decodeBrowsePath('/C:/Users/Nulled/video.mp4', 'win32')).toBe('C:/Users/Nulled/video.mp4')
expect(decodeBrowsePath('/%43%3A/Users/Nulled/file.ps1', 'win32')).toBe('C:/Users/Nulled/file.ps1')
})

it('preserves Unix, UNC, and already normalized paths', () => {
expect(decodeBrowsePath('/home/codex/file.txt', 'linux')).toBe('/home/codex/file.txt')
expect(decodeBrowsePath('//server/share/file.txt', 'win32')).toBe('//server/share/file.txt')
expect(decodeBrowsePath('C:/Users/Nulled/file.txt', 'win32')).toBe('C:/Users/Nulled/file.txt')
})

it('leaves malformed URL encoding usable for the normal validation path', () => {
expect(decodeBrowsePath('/tmp/100%/file.txt', 'linux')).toBe('/tmp/100%/file.txt')
})
})
16 changes: 13 additions & 3 deletions src/server/localBrowseUi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,13 +69,23 @@ export function normalizeLocalPath(rawPath: string): string {
return trimmed
}

export function decodeBrowsePath(rawPath: string): string {
export function decodeBrowsePath(rawPath: string, platform: NodeJS.Platform = process.platform): string {
if (!rawPath) return ''
let decoded: string
try {
return decodeURIComponent(rawPath)
decoded = decodeURIComponent(rawPath)
} catch {
return rawPath
decoded = rawPath
}

// Browse URLs keep an absolute-path slash after the route prefix. On Windows,
// that turns `C:/path` into `/C:/path`, which Node resolves as `C:\C:\path`.
// Remove only that synthetic slash; Unix and UNC absolute paths stay intact.
if (platform === 'win32' && /^\/[A-Za-z]:[\\/]/u.test(decoded)) {
return decoded.slice(1)
}
Comment on lines +81 to +86

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Windows browse hrefs broken 🐞 Bug ≡ Correctness

On Windows, decodeBrowsePath now returns drive paths as C:/... (no leading /), but
toBrowseHref/toEditHref concatenate paths directly after
/codex-local-browse//codex-local-edit without inserting a separator slash. This can produce URLs
like /codex-local-browseC:%5CUsers%5Cfile that won’t match the server routes (expecting
/codex-local-browse/*path), breaking directory navigation/back/edit links on Windows.
Agent Prompt
## Issue description
`decodeBrowsePath()` now correctly normalizes Windows drive-letter paths from `/C:/...` to `C:/...`, but URL generation (`toBrowseHref` / `toEditHref`) assumes the path already begins with `/` and concatenates it directly after the route prefix. On Windows this yields malformed URLs (missing the route separator `/`) that don’t match `/codex-local-browse/*path` and `/codex-local-edit/*path`.

## Issue Context
- Server routes require `/codex-local-browse/<path>` and `/codex-local-edit/<path>`.
- After this PR, browse/edit link generation must not rely on the filesystem path having a leading `/`.

## Fix Focus Areas
- Update `toBrowseHref` and `toEditHref` to always insert exactly one `/` after the route prefix, regardless of whether `pathValue` starts with `/`.
- Normalize Windows separators for URL paths (e.g., replace `\\` with `/`) before encoding, so generated hrefs are stable and readable.
- Add a small unit test by factoring URL building into an exported helper (or otherwise testing via existing exported functions) to cover Windows drive paths.

### Code locations
- src/server/localBrowseUi.ts[144-154]
- src/server/localBrowseUi.ts[253-269]
- src/server/httpServer.ts[152-178]
- src/server/localBrowseUi.test.ts[1-19]

## Suggested implementation sketch
- In `toBrowseHref` / `toEditHref`:
  - `const urlPath = pathValue.replace(/\\/gu, '/')`
  - `const encoded = encodeURI(urlPath)`
  - `const suffix = encoded.startsWith('/') ? encoded.slice(1) : encoded`
  - `return `/codex-local-browse/${suffix}${query}`` (and similarly for edit)
- Add tests that assert a Windows input like `C:\\Users\\Me\\file.txt` (and `C:/Users/Me/file.txt`) yields `/codex-local-browse/C:/Users/Me/file.txt` (or equivalent encoding) and matches the route prefix with `/codex-local-browse/`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


return decoded
}

export function isTextEditablePath(pathValue: string): boolean {
Expand Down
1 change: 1 addition & 0 deletions tests/chat-composer-rendering/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Return to the [manual test index](../../tests.md).
| [Feature: Inline thread image payloads are rewritten to renderable local file URLs](inline-thread-image-payloads-are-rewritten-to-renderable-local-file-urls.md) |
| [Feature: Markdown file links with spaces and parentheses in path](markdown-file-links-with-spaces-and-parentheses-in-path.md) |
| [Feature: Markdown link with backticked label renders as file link](markdown-link-with-backticked-label-renders-as-file-link.md) |
| [Feature: Windows absolute file links open through local browse](windows-absolute-file-links-open-local-browse.md) |
| [Feature: Backticked bare filenames render as file links](backticked-bare-filenames-render-as-file-links.md) |
| [Feature: Lazy message rendering (windowed conversation)](lazy-message-rendering-windowed-conversation.md) |
| [Assistant generated image rendering](assistant-generated-image-rendering.md) |
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
### Feature: Windows absolute file links open through local browse

## Prerequisites

- Run CodexApp on Windows.
- Open TestChat with a project that contains an existing text file and MP4 file on a drive-letter path.

## Steps

1. Send a message containing Markdown links to `C:/path/to/file.ps1` and `C:/path/to/video.mp4`.
2. Inspect both rendered links and confirm their `href`, title, and visible text preserve the complete drive-letter paths.
3. Open the text-file link and confirm the request returns the existing file instead of a 404 response.
4. Open the MP4 link and seek within the video.
5. Inspect the MP4 request and confirm byte-range requests return `206 Partial Content` with `Accept-Ranges: bytes`.

## Expected Results

- Windows drive paths are decoded as `C:/...`, not `/C:/...` or `C:\\C:\\...`.
- Existing files return successfully through `/codex-local-browse/C:/...`.
- MP4 files use the correct media content type and support browser range requests.
- Unix absolute paths and UNC paths retain their existing behavior.
Comment on lines +8 to +23

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

2. Missing light/dark verification step 📘 Rule violation ☼ Reliability

The added manual test case for the changed link/browse UI does not require validation in both light
and dark themes. This increases the risk of theme-specific regressions going unnoticed.
Agent Prompt
## Issue description
The manual test entry for the Windows local-browse link behavior does not include explicit verification in both light and dark themes.

## Issue Context
PR Compliance requires changed UI surfaces to be validated in both themes to prevent dark-mode regressions.

## Fix Focus Areas
- tests/chat-composer-rendering/windows-absolute-file-links-open-local-browse.md[8-23]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


## Rollback / Cleanup

- Close the opened file and video tabs. No files are modified by this test.