Skip to content

fix(pdf-server): keep CLI-arg directories across client roots refreshes - #805

Open
cpruijsen wants to merge 1 commit into
modelcontextprotocol:mainfrom
cpruijsen:fix/issue-644-5aba0400
Open

cpruijsen wants to merge 1 commit into
modelcontextprotocol:mainfrom
cpruijsen:fix/issue-644-5aba0400

Conversation

@cpruijsen

Copy link
Copy Markdown

Fixes #644. Directories passed as CLI args to pdf-server now survive a client roots refresh.

refreshRoots() in examples/pdf-server/server.ts cleared allowedLocalDirs on every successful listRoots(). In --stdio mode useClientRoots is always on, so a host that advertises roots but returns an empty list wiped every CLI-arg directory. After that, list_pdfs reported allowedDirectories: [] and display_pdf rejected all local paths.

Fix

main() now also records CLI-arg directories in a new cliLocalDirs set, following the existing cliLocalFiles pattern. 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.ts run the real initialize handshake over InMemoryTransport with a client that declares roots:

  • An empty roots/list at init keeps the CLI directory and drops a directory from an earlier refresh.
  • A root removed via notifications/roots/list_changed is 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() turns file:///C:/x into /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.

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

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v1.7.0 refreshRoots() clears allowedLocalDirs when host advertises roots capability but returns empty list — wipes CLI-arg directories

1 participant