Show an error instead of a 500 when the folder viewer opens a path OSC 367 refuses - #877
Merged
Merged
Conversation
…67 refuses
The folder viewer's select and activate wrote openSequence() unguarded, which
throws for a path validToolOpenPath rejects (a control character, a UNC root,
over 2048 characters); the throw became a 500 and the page showed only
"Folder viewer unavailable". Check the path first and answer { ok: false }.
Also fix the stale "control connection" comment in the folder page and the
dor-tools-lib osc.ts map entry, which omitted the open encoder.
Deploying mouseterm with
|
| Latest commit: |
32db641
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://55b23b64.mouseterm.pages.dev |
| Branch Preview URL: | https://fix-folder-viewer-unopenable.mouseterm.pages.dev |
nedtwigg
approved these changes
Oct 1, 2026
nedtwigg
left a comment
Member
There was a problem hiding this comment.
Reviewed the existing path guard, OSC encoder, HTTP response, and status-line handling. Refused paths now produce a readable error without emitting OSC; the existing trust policy stays intact. CI and bot self-review passed.
nedtwigg
requested a deployment
to
hosted-preview
October 1, 2026 21:06 — with
GitHub Actions
Waiting
This branch is waiting to be deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
builtin:folderviewer turned a click on some files into a 500 and a misleading "Folder viewer unavailable" status. Since #856, its select and activate callopenSequence({ path, preview }), which throwsRangeErrorfor any pathvalidToolOpenPathrejects. The throw escaped the route, sostartCapabilityVieweranswered 500 with its generic unavailable text. This PR checks the path first and answers{ ok: false, error }, which the page already shows as its status line. Nothing is written to the terminal for such a path.Paths that reach this:
\\server\share\…).validToolOpenPathaccepts only/orX:\prefixes, and the viewer listsrealpath(dir), so a folder opened on a network share fails on every click. Before Add the OSC 367 open verb and show failed opens in the preview slot #856 these went throughclient.toolSurface({ file })with no such filter.Whether the host should accept UNC paths in OSC 367
openis a separate question for a maintainer: it widens what a Tool can ask the host to open, so this PR leavesvalidToolOpenPathunchanged and only makes the refusal readable.The new test in
dor-tools-builtin/test/folder-viewer.test.mjsdrives the extractedoscOpenwith one valid path and the three refused shapes. Before this change, the refused shapes threw instead of answering an error. I couldn't run it locally because this sandbox can't reach the npm registry, so CI is the first run.The same commit fixes two stale descriptions left by #856: the folder page's comment still said each POST is "its own control connection" (it is an HTTP request now), and the
osc.tsentry indocs/specs/dor-tools-lib.mdomitted theopenencoder. Thedor-tools-builtin.mdbudget is ratcheted from 1050 to 1100 for the one added clause.