Repository navigation
Conversation
Fixes anomalyco#12589 - adds coerceFsPath utility that safely handles various path-like input shapes (objects, URLs, arrays) before calling Node.js path functions.
|
The following comment was made by an LLM, it may be inaccurate: No duplicate PRs found |
There was a problem hiding this comment.
Pull request overview
This PR addresses a TUI crash when path-normalization code receives non-string “path-like” values from plugins, by introducing a centralized coercion utility and updating the session routes to use it before calling Node/Bun path helpers.
Changes:
- Added
coerceFsPathutility to coerce unknown “path-like” inputs into filesystem path strings. - Updated TUI session route path normalization to call
coerceFsPathbefore usingpath.isAbsolute(...). - Added unit tests covering supported and rejected coercion inputs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| packages/opencode/src/util/coerce-path.ts | New utility for coercing unknown values into safe filesystem path strings. |
| packages/opencode/test/util/coerce-path.test.ts | New unit tests for coercion behavior and error messaging. |
| packages/opencode/src/cli/cmd/tui/routes/session/permission.tsx | Updated normalizePath to coerce unknown inputs before using path operations. |
| packages/opencode/src/cli/cmd/tui/routes/session/index.tsx | Updated normalizePath to coerce unknown inputs before using path operations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Handle URL instance | ||
| if (input instanceof URL) { | ||
| return input.pathname | ||
| } |
There was a problem hiding this comment.
coerceFsPath treats any URL the same and returns url.pathname. For file: URLs this can be incorrect on Windows (e.g. /C:/...) and it also silently accepts non-file URLs (http:, data:) as if they were filesystem paths. Consider only accepting file: URLs and converting with fileURLToPath (from url or bun) so decoding + platform-specific path formatting are handled correctly; otherwise throw a clear error for non-file URLs.
| const pathKeys = ["path", "filePath", "cwd", "value"] as const | ||
| for (const key of pathKeys) { | ||
| if (key in obj) { | ||
| const value = obj[key] | ||
| if (typeof value === "string") { | ||
| return value | ||
| } | ||
| if (value == null) { | ||
| return "" | ||
| } | ||
| // Property exists but is not a string or null | ||
| throw new TypeError( | ||
| `coerceFsPath: object.${key} is not a string (got ${typeof value})${context ? ` in ${context}` : ""}`, | ||
| ) | ||
| } |
There was a problem hiding this comment.
When checking for path-like properties, using key in obj will also match properties on the prototype chain. For safety (and to avoid surprising coercions with objects created via Object.create(...)), prefer an own-property check like Object.hasOwn(obj, key) / Object.prototype.hasOwnProperty.call(obj, key) before reading obj[key].
| function normalizePath(input?: string) { | ||
| if (!input) return "" | ||
| function normalizePath(input?: unknown): string { | ||
| const p = coerceFsPath(input, "permission route normalizePath") |
There was a problem hiding this comment.
coerceFsPath can throw TypeErrors for unexpected shapes (e.g. objects without a recognized key). Since normalizePath is called during render, an exception here will still crash the TUI route. Wrap the coercion in a try/catch and fall back to "" (or some safe placeholder) to fully prevent UI crashes from plugin-provided metadata.
| const p = coerceFsPath(input, "permission route normalizePath") | |
| let p: string | undefined | |
| try { | |
| p = coerceFsPath(input, "permission route normalizePath") | |
| } catch { | |
| // If metadata provides an unexpected shape, avoid crashing the TUI | |
| return "" | |
| } |
| if (path.isAbsolute(input)) { | ||
| return path.relative(process.cwd(), input) || "." | ||
| function normalizePath(input?: unknown): string { | ||
| const p = coerceFsPath(input, "session route normalizePath") |
There was a problem hiding this comment.
coerceFsPath throws on invalid inputs; because normalizePath is invoked during component rendering, this can still take down the session route if a plugin returns an unexpected object shape. Consider guarding the coerceFsPath call with try/catch and returning a safe default so the UI remains resilient.
| const p = coerceFsPath(input, "session route normalizePath") | |
| let p: string | undefined | |
| try { | |
| p = coerceFsPath(input, "session route normalizePath") | |
| } catch { | |
| return "" | |
| } |
| test("extracts pathname from URL instance", () => { | ||
| const url = new URL("file:///path/to/file.txt") | ||
| expect(coerceFsPath(url)).toBe("/path/to/file.txt") | ||
| }) |
There was a problem hiding this comment.
The URL test currently asserts a POSIX pathname (/path/to/file.txt). If URL coercion is corrected to use fileURLToPath (which is important for Windows), the expected value will differ by platform. Consider making this assertion platform-aware (or adding a Windows-specific case) so the test remains portable if unit tests are ever run on Windows.
|
Closing this pull request because it has had no updates for more than 60 days. If you plan to continue working on it, feel free to reopen or open a new PR. |
Fixes #12589 - adds coerceFsPath utility that safely handles various path-like input shapes (objects, URLs, arrays) before calling Node.js path functions.
I've fixed this issue. The root cause was that normalizePath in session/index.tsx called
path.isAbsolute(input)without validating that input was a string. When plugins likeopencode-antigravity-auth@latestreturned objects (e.g.,{ path: string }) instead of raw strings, Node.js threw the TypeError.Fix: Added a coerceFsPath utility that safely handles various path-like input shapes (objects, URLs, arrays) before calling Node.js path functions.
Branch:
fix-path-type-12589Changes: