Skip to content

Bump Ark to posit-dev/ark#1362 - #15174

Open
lionel- wants to merge 1 commit into
mainfrom
bump-ark/pr-1362
Open

Bump Ark to posit-dev/ark#1362#15174
lionel- wants to merge 1 commit into
mainfrom
bump-ark/pr-1362

Conversation

@github-actions

Copy link
Copy Markdown

E2E Tests 🚀
This PR will run tests tagged with: @:critical @:ark @:win @:web

Why these tags?
Tag Source
@:critical Always runs (required)
@:ark PR description
@:win Ark submodule bump
@:web Ark submodule bump

More on automatic tags from changed files.

readme  valid tags

lionel- added a commit to posit-dev/ark that referenced this pull request Jul 30, 2026
Closes #1361

Test suite passed at posit-dev/positron#15174

Because `tower_lsp` doesn't handle `exit` notifications and is
unmaintained, we've pinned it to a fork that includes a patch for
`exit`: https://github.com/lionel-/tower-lsp/tree/bugfix/patches.
However it turns out this patch is buggy in case of unexpected
disconnections.

To fix both the exit handling and the unexpected disconnection issue,
this PR switches us to `tower_lsp_server`, a maintained community fork
of `tower_lsp`
(https://github.com/tower-lsp-community/tower-lsp-server/). It's used by
Biome, which indicates reliability.

#### tower_lsp_server and the disconnection fix

The first two commits add integration tests for shutdown and
disconnection. The failing test output with our current version of
tower-lsp matches the panics documented in the linked issue:

```
thread 'lsp::test_lsp_survives_abrupt_disconnect' (1793302) panicked at crates/ark/tests/integration/lsp.rs:56:9:
`ark-lsp` thread panicked on abrupt disconnect: panicked at /Users/lionel/.cargo/git/checkouts/tower-lsp-ad3208de34ae5bea/49ef549/src/transport.rs:167:41:
internal error: entered unreachable code
test lsp::test_lsp_survives_abrupt_disconnect ... FAILED
```

The third commit is the mechanical switch to the community fork, and it
fixes the failing test.

The third commit also copies `aether_lsp_utils` from the Air repo,
because the latter uses a different incompatible `lsp_types` crate (from
tower-lsp). Ideally we'd move it to posit-dev/aether and update it to
use the community fork, then point both Ark and Air to it, but this can
wait until later.

#### tower_lsp_server and the `Uri` type

The fourth commit streamlines the Uri/FilePath boundary. The main change
from switching to `tower_lsp_server` is that LSP URLs are represented by
`ls_types::Uri` which wraps
https://docs.rs/fluent-uri/latest/fluent_uri. This
type has different properties than `url::Url`, which we use for Jupyter
and `tower_lsp` used to use for LSP. `Uri` implements RFC 3986, which is
recommended by the LSP protocol, whereas `Url` implements WHATWG. The
main
differences are:

- `Url` does some normalisation, whereas `Uri` stores bytes as is.
- `Url` accepts `| [ ] ^` characters, `Uri` rejects them.

Given that `Uri` doesn't do any normalisation, it's a better
representation of wire URLs which we were already tracking for sending
back URLs to the frontend exactly as we got them in requests. It's a
good thing that we had to switch to follow `tower_lsp_server`.

Here are the principles that guided this refactor (these already
existed, but we are reinforcing them):

- Identity is governed by our normalised `FilePath`, not by any kind of
URLs. For non-virtual paths, this wraps `AbsPathBuf`.
- Wire values are stored byte for byte. `Uri` now plays that role.
- `Uri` and `FilePath` are converted at the LSP boundary.
- File system queries take `AbsPathBuf` (unwrapped from `FilePath`).
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.

R Data Viewer fails to display contents of a numeric matrix converted from a data.frame subset

1 participant