Repository navigation
Conversation
refreshRoots() cleared allowedLocalDirs unconditionally whenever listRoots() succeeded, so a host that advertises the roots capability but returns an empty list wiped directories passed as CLI args. Mirror the existing cliLocalFiles precedent: track CLI-arg directories in a new cliLocalDirs set and re-add them after each clear(). Client roots keep clear-and-replace semantics, so root revocation still works while the client can no longer revoke launch-time CLI directories.
This branch has not been 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.
Fixes #644. Directories passed as CLI args to pdf-server now survive a client roots refresh.
refreshRoots()inexamples/pdf-server/server.tsclearedallowedLocalDirson every successfullistRoots(). In--stdiomodeuseClientRootsis always on, so a host that advertisesrootsbut returns an empty list wiped every CLI-arg directory. After that,list_pdfsreportedallowedDirectories: []anddisplay_pdfrejected all local paths.Fix
main()now also records CLI-arg directories in a newcliLocalDirsset, following the existingcliLocalFilespattern.refreshRoots()re-adds them right after the clear. Client roots keep clear-and-replace semantics, so a root the client removes is still revoked.The issue suggests either clearing only on a non-empty list or dropping the clear. Both stop the client from revoking roots: the first for its last root, the second for any root. A provenance set protects only the launch-time grants. If you prefer the smaller non-empty check, I can switch to it.
Tests
Two new tests in
examples/pdf-server/server.test.tsrun the real initialize handshake overInMemoryTransportwith a client that declaresroots:roots/listat init keeps the CLI directory and drops a directory from an earlier refresh.notifications/roots/list_changedis revoked, and the CLI directory stays.Both fail without the re-add in
refreshRoots()and pass with it.Not in this PR
A comment on the issue reports that
fileUrlToPath()turnsfile:///C:/xinto/C:/x, so Windows file roots are dropped. That is a separate bug. With this PR, CLI-arg directories survive on Windows, but client roots there still need that fix.