Skip to content

Fix/Use active R session package for editor language completion - #1816

Open
Fred-Wu wants to merge 26 commits into
REditorSupport:mainfrom
Fred-Wu:fix/session-package-completion
Open

Fred-Wu wants to merge 26 commits into
REditorSupport:mainfrom
Fred-Wu:fix/session-package-completion

Conversation

@Fred-Wu

@Fred-Wu Fred-Wu commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

Related to #1536, #1593

Summary

Use packages attached in the active R session for editor language completion, even when those packages are not referenced or through source() in the active script.

Currently, completion mainly relies on package information that languageserver can discover from the script or its static source dependencies. This means a package attached interactively in the R session will not receive completion suggestions when it is not mentioned in the script.

This change synchronises the active R session package state, provided via sess, with the existing language server.

Behaviour after this change

  • Single-server mode: packages attached in the active R session are available for completion across documents handled by the single language server, even when they are not referenced in the script.

  • Multi-server mode: packages attached in an R session are synchronised with the language server associated with that session's workspace. Package state remains isolated between different workspace language servers.

  • Unscoped files: scripts opened outside an existing workspace or no workspace folder is open, use the active unscoped R session package state for completion.

When the active R session changes, its attached package state replaces the previous session state. When the session disconnects, the session-derived package state is cleared.

Existing static package discovery, including resolvable source() dependencies, remains unchanged.

@Fred-Wu
Fred-Wu marked this pull request as ready for review October 3, 2026 06:58

@renkun-ken renkun-ken left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach makes interactively attached packages available to the existing completion machinery, and the current CI checks pass. I found three synchronization/routing issues that should be addressed before merging; the inline comments include their triggers and suggested fixes.

Validation: reviewed head 4cc6b7b; TypeScript type checking, ESLint for all changed TypeScript files, and lintr for R/languageServer.R passed locally. Focused reproductions exercised the PR's attach handler and LanguageService startup with mocked VS Code/client dependencies, and its R synchronization handler with languageserver 0.3.20, including the actual workspace-folder notification path. These reproduced incorrect routing for a wrapped R process, a lost update during client startup, and missing completion in a workspace added after synchronization. Please add regression coverage for these cases.

Comment thread src/session.ts Outdated
session.info = (params.info as SessionInfo | undefined) ?? { version: session.rVer, command: '', start_time: '' };
session.sessionDir = String(params.tempdir);
session.workingDir = String(params.wd);
session.resource = terminal ? rTerminal.getTerminalResource(terminal) : undefined;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Resolve the workspace for sessions whose PID differs from the terminal PID

terminal is found only when Terminal.processId equals params.pid, but VS Code reports the shell/console process PID and sess reports Sys.getpid(). With a wrapped console or R started inside a shell, those can differ even when both the terminal cwd and params.wd are inside the workspace. This makes session.resource undefined, so multi-server mode sends that session's packages to unscoped instead of its workspace server; it can also replace package state for unrelated unscoped files. I reproduced this with terminal PID 41000, R PID 41001, and both paths in the same workspace. Please resolve the workspace independently of exact PID equality, using a reliable terminal/session association or the local session's reported working directory as a fallback, while preserving truly unscoped sessions.

Comment thread src/languageService.ts Outdated
Comment on lines +249 to +251
const sessionState = this.sessionStates.get(sessionScope)?.state;
if (sessionState) {
await this.applySessionState(client, sessionState);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Register the client before awaiting its initial state synchronization

The caller adds this client to clients/clientScopes only after createClient returns. If a package update or disconnect arrives while this initial sendRequest is pending, syncSessionState updates/deletes the cache but cannot deliver the change to this client. The client is then registered with the old server state, and identical later workspace refreshes are skipped by the state-key check. In a controlled reproduction, synchronizing [base], then [dplyr, base] before the first request completed left the cache at [dplyr, base] but sent only [base]. Please register the client and its scope before the asynchronous sync, or reconcile the latest state (including a cleared state) before making startup complete.

Comment thread R/languageServer.R Outdated
Comment on lines +56 to +59
for (workspace in self$workspaces$values()) {
workspace$startup_packages <- if (length(attached_packages)) {
# languageserver resolves package conflicts from the end of this list.
rev(attached_packages)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Apply the current session state to workspaces created after this request

This updates only the workspaces that already exist when r/syncSessionState runs. In single-server mode, adding another folder to an existing multi-root workspace creates a new Workspace through workspace/didChangeWorkspaceFolders, and that workspace starts with the default packages. Since the session/package state is unchanged, the TypeScript deduplication does not send another sync. I reproduced this by syncing dplyr, then invoking the real workspace-folder notification handler: the existing workspace offered mutate, while the newly added workspace did not. Please retain the active state on the server and apply it when adding a workspace, or explicitly replay it after workspace-folder additions so completion remains available across all documents handled by the single server.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rechecked at d53815d: this case is still unresolved. The R handler still updates only self$workspaces$values() at synchronization time, and no replay is triggered for added folders. Using this revision's handler with languageserver 0.3.20, I synced dplyr, invoked workspace_did_change_workspace_folders to add a folder, and requested workspace completion: mutate was present for the existing workspace and absent for the added workspace. Please retain/apply the active state when a workspace is created, or replay synchronization after folder additions, and add coverage for that path.

@Fred-Wu
Fred-Wu force-pushed the fix/session-package-completion branch from 4cc6b7b to 0a5cf6f Compare October 5, 2026 05:58

@renkun-ken renkun-ken left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed d53815d. The local working-directory fallback and registering the client before its initial synchronization address the first two findings from my previous review. Focused reproductions now route a session without a terminal match to its workspace and deliver both updates and disconnects while the first sync is pending. The new bound-session separation also passed the four language-service regression tests in a mocked runtime.

The third finding remains: a workspace folder added after synchronization does not inherit the active session packages. I reran the actual languageserver workspace-folder notification path with this revision: the existing workspace offered mutate after syncing dplyr, while the newly added workspace did not. I have added an update to the original thread.

All three CI test jobs currently stop during TypeScript compilation because of the two new-test errors noted inline; the VS Code test suite never runs. Both errors also reproduce locally. Build and lint CI jobs pass, and local lintr for R/languageServer.R passes. The focused mocked/R checks above do not substitute for running the full test suite once these compile errors are fixed.

Comment thread src/test/suite/languageService.test.ts Outdated
service.syncSessionState({
search: ['.GlobalEnv', 'package:base'],
loaded_namespaces: ['base'],
globalenv: {},

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Remove the extra property from the synchronization fixture

This direct object literal is passed to syncSessionState, whose SessionWorkspaceData parameter contains only search and loaded_namespaces. TypeScript rejects globalenv here with TS2353, so the new test stops the pretest compilation step on macOS, Linux, and Windows before any VS Code tests execute. The same error reproduces locally. Please omit globalenv from this literal or pass a separately typed WorkspaceData fixture, then rerun the test jobs.

Comment thread src/test/suite/session.test.ts Outdated
})}\n`);
const attached = await waitFor(() => session.activeSession?.sessionId === 'workspace-fallback'
? session.activeSession : undefined);
assert.strictEqual(attached.resource?.toString(), workspaceUri.toString());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Narrow the possibly undefined result before reading resource

waitFor returns Promise without narrowing T, and this callback returns Session | undefined, so attached remains possibly undefined here. With strict checking enabled, this line produces TS18048 in every CI test job and locally. The optional access on resource does not guard attached. Please add assert.ok(attached) before accessing attached.resource (as the test already does for socket), or give waitFor an appropriately narrowed return contract, so pretest can compile and the regression test can run.

Preserve session package synchronization with the updated language client and terminal registry. Retain workspace fallback coverage alongside the relocated terminal lifecycle tests.
- Track successful delivery per client and serialise updates to preserve
the latest state. Retry failures on refresh or completion, retain
disconnect clears, and guard against replaced clients.

- Add regression tests and real R server coverage for workspace replay
and disconnect cleanup.
@Fred-Wu
Fred-Wu marked this pull request as ready for review October 8, 2026 04:09
@Fred-Wu
Fred-Wu requested a review from renkun-ken October 8, 2026 04:09
@Fred-Wu

Fred-Wu commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

@renkun-ken Could you take a look at this, and determine whether this is still needed?

Autocompletion from source()ed packages, I think, is covered by the latest languageserver , even for the script with packages is not being opened. Then the value of this is just for the packages submitted in the terminal, but not referenced anywhere.

image

@renkun-ken

Copy link
Copy Markdown
Member

The static analysis for project-wide indexing (including source() handling) was implemented in REditorSupport/languageserver#751.

@Fred-Wu

Fred-Wu commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

The static analysis for project-wide indexing (including source() handling) was implemented in REditorSupport/languageserver#751.

Do you think this PR is still necessary then?

@renkun-ken

Copy link
Copy Markdown
Member

I think this PR still has value, but languageserver#751 narrows its purpose to completion from runtime session state.

The current languageserver already discovers packages referenced in the script or in closed files reached through statically resolvable source() dependencies. This PR adds something different: packages attached only in the running R session, including packages loaded dynamically that static analysis cannot discover. For example, after typing library(dplyr) only in the terminal, mutate should become available in the editor even without a static package reference.

I checked this with real LSP requests against languageserver 0.3.20 and this PR's launcher: completion through a closed sourced setup file works already; a script without a static package reference gains mutate after session synchronization; disconnecting removes that session-only completion while preserving the static completion. Local TypeScript checking and the current CI jobs pass.

I found two remaining routing issues in head 96afb8ef9bddcf11feaadd7a6ee7821a4204cde8, reproduced with the actual synchronization code and mocked clients:

  1. Multi-server workspace state becomes stale when another session is selected. updateSessionWorkspace() passes active = false, and only the globally selected session also calls refreshActiveSession(). Consequently, an unselected session's updates reach its bound Interactive servers but skip the workspace server, which retains the earlier package snapshot. Updates should still reach a workspace scope when that session is its recorded owner, without allowing other sessions to overwrite it. See session.ts and languageService.ts.

  2. Removing a workspace before disconnecting leaves stale session state. Cleanup recalculates the scope from the currently open workspace folders, so after removal it clears unscoped rather than the original workspace key. Reopening that folder replays packages from the disconnected session. Cleanup should use the recorded session owner/scope rather than reclassifying the resource. See languageService.ts.

There is also a completion UX concern: languageserver's static analysis and vscode-R's live session provider can suggest the same symbol, with different provenance, documentation, or insertion behavior. For example, a function defined in the script and then evaluated into .GlobalEnv can have both a static suggestion and a [session] suggestion. The combined list can be confusing, especially when the source and runtime versions differ. This overlap already exists; this PR sends package state into the existing languageserver completion machinery rather than adding a separate package completion provider. Nevertheless, we should define precedence and deduplication for the combined static/live results, preserve genuinely different candidates, and add coverage for the overlapping-symbol case.

For maintainability, I would also prefer a supported session-state handler in languageserver over replacing languageserver::run() and mutating private workspace fields from vscode-R.

My recommendation is to keep the feature, describe it specifically as session-only package completion, and address the routing issues and static/live completion UX before merging.

@Fred-Wu
Fred-Wu marked this pull request as draft October 13, 2026 09:48
@Fred-Wu Fred-Wu added this to the 3.x milestone Oct 13, 2026
Update owned workspace language servers even when their session is not globally selected, and clear recorded scopes on disconnect after folders are removed. Add regression tests for session handoff, competing updates and folder removal. Apply oxfmt to the PR's changed TypeScript sources.

@renkun-ken renkun-ken left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The feature still has value with the current languageserver: static discovery can follow resolvable source() dependencies, but it cannot infer a package attached only in the running session. I found remaining synchronization and compatibility problems that should be addressed before merging; details and reproductions are inline.

The earlier client-startup and single-server workspace replay fixes are present, and all six CI checks passed on this head. Locally, TypeScript type checking passed. I exercised the current languageserver at REditorSupport/languageserver@0add7cc with focused provider and session-state reproductions, tested the 0.3.18 handler, and used mocked clients/source-level probes for routing and Interactive lifecycle cases. Those probes are not full VS Code UI tests.

Comment thread R/languageServer.R Outdated
} else {
languageserver:::workspace_startup_packages()
}
workspace$update_loaded_packages()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Synchronize the current server's package metadata and resolution context

Updating startup_packages and loaded_packages alone does not make a session-only package available to the latest server's member and S3 providers. Against languageserver 0add7cc, after syncing an attached polars, pl$all()$su produced no member suggestions until its member metadata was prepared. Similarly, after syncing dplyr, arrange(data.frame(x=1), ) returned the generic signature without .locale, whereas a static library(dplyr) produced the data-frame method signature. Preparing the S3 catalog alone did not fix the latter because its resolver also needs the effective session package context. Please integrate session packages with the server's metadata preparation and provider context, preferably through a supported server-side interface, and cover attach/detach for these providers.

Comment thread src/languageService.ts
attachedPackages: data.search
.filter((value) => value.startsWith('package:'))
.map((value) => value.substring(8)),
loadedNamespaces: [...data.loaded_namespaces],

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Carry the attached packages' library locations to the server

This snapshot sends only names, while ordinary workspace/global servers use their own configured R library paths. If the session attaches a package from a session-specific .libPaths() or library(..., lib.loc=...), workspace$get_namespace() cannot find it and the handler silently reports success, so the advertised completion never appears. I reproduced this with a package installed only in a temporary session library: it was in loaded_packages, but the current languageserver returned NULL for its namespace. Please synchronize enough library/package-location information to resolve the actual attached packages, or explicitly constrain and surface this limitation.

Comment thread R/languageServer.R Outdated
# languageserver resolves package conflicts from the end of this list.
rev(attached_packages)
} else {
languageserver:::workspace_startup_packages()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Handle installed languageserver versions without this internal function

The extension does not enforce a minimum languageserver version, and released 0.3.18 has workspaces but no workspace_startup_packages(). With that version I synced splines successfully, then sent the empty state: the request failed with object 'workspace_startup_packages' not found and bs remained in completion after disconnect. Version 0.3.17 also lacks self$workspaces. Please preserve a compatible reset path or gate this integration with an explicit minimum-version check and graceful fallback, and test the supported versions.

Comment thread src/session.ts Outdated
session.sessionDir = params.tempdir;
session.workingDir = params.wd;
session.resource =
(native ? rTerminal.getTerminalResource(native.terminal) : undefined) ??

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Do not use the configuration fallback to route an external session

getTerminalResource() calls getCurrentWorkspaceFolder(), which falls back to the active or first workspace when the terminal cwd is outside all folders or missing. Consequently, with multi-server enabled, an associated terminal and R process both in /outside can be assigned to the unrelated /project workspace; its packages go to the project server instead of the unscoped server. I reproduced that exact routing with the PR's functions. Please use an actual terminal/session association or the local session's reported working directory without the configuration fallback when deciding the session scope.

Comment thread src/session.ts Outdated
: (previous ?? new Session(id, host, '', '', new net.Socket()));
target.requester = requester;
target.workingDir = directory;
target.resource = vscode.workspace.getWorkspaceFolder(Uri.file(directory))?.uri;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Reclassify sessions when their directory is added to the workspace

This stores undefined if an Interactive session starts outside the folders currently open, losing its directory as a routing resource. With folder A already open, I started a session in B, added B to the workspace, and opened B's R file: the new workspace client was created, but the session remained in unscoped and no package state reached B even on refresh. The same loss occurs in the unassociated local attach fallback. Preserve the local directory and re-evaluate/replay affected sessions when folders are added.

Comment thread src/languageService.ts Outdated
} else if (ownedScope && this.sessionStates.get(ownedScope)?.sessionId === sessionId) {
// An unselected session may refresh its own workspace, but never
// replace the state now owned by another session.
this.syncSessionScope(ownedScope, data, sessionId);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Refresh the owned background session before syncing its workspace

This branch handles an inactive session only once updateSessionWorkspace() has received fresh data. For native terminal sessions, however, workspace_updated calls refresh.invalidate() without a delay when the session is inactive, so the existing scheduler does not fetch that data until a later explicit demand or selection. An ordinary file in the owned workspace does not demand that background session. I reproduced a B workspace retaining stats after B was invalidated with splines; only an explicit ensure() delivered the new state. Please arrange a coalesced refresh on invalidation or on completion demand for the recorded owner; the new direct-sync test bypasses this path.

Comment thread src/session.ts Outdated

export function updateSessionWorkspace(target: Session, data: WorkspaceData): void {
target.workspaceData = data;
rLanguageService?.syncSessionState(data, target.resource, target.sessionId, false);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Clear package state when an Interactive R process stops

Stopping an Interactive session retains its view and transcript. InteractiveManager.updateStatus() sets workspaceUnavailable on exit and calls updateSessionWorkspace() with the old cached data, so this line re-sends the stopped process's packages rather than clearing them; unregisterSessionTransport() is only called when the view is disposed. I reproduced that data flow with a bound client. Please clear the session-derived state on a definitive stop/exit and avoid replaying target.workspaceData when a client is later created for that stopped target, while handling transient reconnects deliberately.

Comment thread src/session.ts

/** Move document routing to a new process; existing data viewers keep their old owner. */
export function replaceSessionTransport(previous: Session, next: Session): void {
rLanguageService?.syncSessionState(undefined, previous.resource, previous.sessionId);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Rebind language clients when an Interactive session is restarted

replaceSessionTransport() clears the old session scope, then changes documentSessions to the new generation without firing onDidBindSessionDocument. The retained input client therefore keeps session:<old id> as its scope and never receives the new process's packages. Calling bindSessionDocument() afterward also emits no event because the owner was already replaced. A mocked run of the actual listeners retained one client on the old scope after restart. Please notify/restart or migrate affected clients during replacement, and test the retained Interactive input across a restart.

…e lifecycles

Use actual R session paths when routing workspaces, reconcile ownership as folders change, and refresh background owners. Clear exited and replaced process state, rebind retained Interactive clients, and propagate process-specific package library paths. Prepare member and S3 metadata in languageserver workers and fall back cleanly when its integration internals are unavailable.

Cover scope changes, stop/restart/reconnect, custom libraries, background refresh, and provider resolution.
Persist the R language server's session-owned package baseline across synchronization requests so detaching a private-library package restores the actual workspace imports. Apply repository oxfmt output, narrow test callback types for oxlint, and compare Windows resource URIs without drive-letter casing assumptions.
…adata

Restore configured language-server library paths on detach and evict session-only namespace caches. Settle canceled metadata RPCs and prevent stale worker results from restoring old packages. Retain failed synchronization for retry and test server rejection. Resolve R lint indentation findings.

TypeScript sources and regression test were checked using oxfmt, typecheck and oxlint on the validation candidate.
Track asynchronous sync reply completion in an explicit environment instead of superassignment. In the managed jgd preparation test, retry transient startup.status read failures while the child process writes the file, using the existing bounded startup deadline.

Keep production jgd preparation behavior unchanged.
Distinguish permanently unsupported language servers from transient sync
failures, preserving ordinary completion while avoiding repeated requests.
Test nested member completion before attachment, after attachment, and after
detachment using a private-library package fixture.
Derive project NAMESPACE imports independently of session-injected packages,
including when a package belongs to both sets. Avoid retaining removed R
workspace objects across syncs. Cover imports added and removed while a
session is attached using real language-server completion requests.

Assert that the pending startup deadline is cleared on expiration rather than
counting timers belonging to unrelated extension-host components.
Add a focused Linux extension-test job using a pinned languageserver
revision with member_resolve_packages. Keep the existing released-version
matrix unchanged, and require the provider API so the nested-member and
S3 assertions cannot be silently skipped in the development-server job.
Remove the separately introduced development languageserver job and its
mandatory-provider test flag. Restore the previously passing CI configuration
and integration test without changing production code.
@Fred-Wu
Fred-Wu marked this pull request as ready for review October 30, 2026 07:33
@Fred-Wu
Fred-Wu requested a review from renkun-ken October 30, 2026 07:33

@renkun-ken renkun-ken left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new changes address several earlier findings: focused probes now route local external sessions correctly, move session ownership when folders are added, and rebind Interactive clients on restart; older servers are explicitly treated as unsupported. There are still blocking problems in the current-server integration and several remaining state/lifecycle cases, detailed inline.

Validation: all six hosted checks passed on this head, and local TypeScript checking passed. Against languageserver REditorSupport/languageserver@cf116dd, I ran the PR's real VS Code session-completion integration test: it failed at nested member completion, with the server log reporting object 'prepare' not found. CI installs the released languageserver and conditionally skips the newer member/S3 assertions when that API is absent, so those green jobs do not establish compatibility with the current development server. Other findings were exercised with the actual R handler/provider or mocked VS Code/session clients; the routing and lifecycle probes are not full UI tests.

Comment thread R/languageServer.R
return(invisible(NULL))
}
task <- create_task(
target = package_call(prepare),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Pass the actual metadata function name to the worker

languageserver:::package_call() captures the unevaluated symbol with substitute(). Passing the local variable prepare creates a worker that calls get("prepare", asNamespace("languageserver")), but the current server has no such function. With languageserver cf116dd, every nonempty session sync returns FALSE, and completion repeatedly retries without preparing member/S3 metadata. I reproduced the callr error and ran the PR's VS Code integration test against that revision; it failed at the nested-member assertion. A temporary change to pass the member_resolve_packages symbol made the worker reply successfully, although the nested-member integration assertion still failed and needs investigation too. Please also run this test against a pinned current languageserver in CI so the provider branch cannot be skipped.

Comment thread R/languageServer.R
for (workspace in self$workspaces$values()) {
if (!is.null(result$members) && !is.null(workspace$member_metadata)) {
for (pkg in names(result$members)) {
workspace$member_metadata$set(pkg, result$members[[pkg]])

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Make session-only exported factories visible to member inference

Even after correcting the worker target, caching the package snapshot here is not sufficient on languageserver cf116dd. I reran the PR's VS Code integration test with that single temporary correction: sessionChain()$all()$su still had no member suggestions. A focused provider check showed the cache contained the fixture and the workspace startup list included it; adding library(sessionfixture) to the document then produced sunrise. The current server promotes exported function roots for static library() imports, but not for its startup packages. Please coordinate a server-side integration that exposes session-attached factories to member inference and run the integration test against that server.

Comment thread src/languageService.ts
);
return;
}
if (applied === false) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Invalidate the previous acknowledgment when the server returns false

The R handler can mutate the server before replying FALSE: it changes the library paths and workspace package lists before checking for missing packages or a worker error. If A was acknowledged, B partially applies and returns false, and the session returns to A, syncedStateKey still equals A, so no restoring request is sent. I reproduced this both with the actual client delivery code and the R handler; the client wanted stats while the server retained B's missing-package state. Clear the acknowledged key on false, as the exception path already does, and cover A → failed B → A.

Comment thread R/languageServer.R
workspace$imported_packages <- union(base_imports, attached_packages)
workspace$update_loaded_packages()
for (pkg in unique(c(attached_packages, loaded_namespaces))) {
try(workspace$get_namespace(pkg), silent = TRUE)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Refresh package identity when the active session's library changes

Two sessions can attach different installed versions of the same package. Switching A → B updates .libPaths(), but the name remains in new_startup, so it is not in removed; get_namespace() returns A's cached object, and R also keeps A's namespace loaded. Against cf116dd I installed two versions in separate libraries: after switching to B, basic completion still exposed A's alpha_only and not B's beta_only. The current worker failure is separate: both syncs returned FALSE, but the partially updated server still served A's namespace. Please reconcile namespace identity by path/version, using a safe restart or isolation strategy where needed.

Comment thread R/languageServer.R
new_startup <- if (length(attached_packages)) rev(attached_packages) else default_startup
removed <- setdiff(previous, unique(c(new_startup, base_imports)))
workspace$startup_packages <- new_startup
workspace$imported_packages <- union(base_imports, attached_packages)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Preserve R search-path precedence for S3 resolution

attached_packages comes from search() in highest-priority-first order, but the current S3 resolver reverses workspace$imported_packages when choosing a bare generic. Putting that vector into imports unchanged therefore reverses conflicts. With two attached packages exporting generic_probe, I reproduced workspace$guess_namespace() and R selecting the first package while S3 signature help selected the second package's method. This used the current handler and a normal document metadata worker, so it is observable even while the session worker has the separate error above. Please preserve attachment precedence in both the resolver context and metadata-worker inputs, and cover masking between two attached packages.

Comment thread src/session.ts
// refreshes just like the foreground session.
refresh.invalidate(
target === activeSession ||
rLanguageService?.ownsWorkspaceScope(target.sessionId)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Bootstrap a background session before requiring scope ownership

An unselected native session can attach with autoActivate: false while another session is active. That attach returns without scheduling its initial workspace read; subsequent workspace_updated notifications reach this condition with ownsWorkspaceScope() false, so they mark the scheduler dirty without scheduling a read. Nothing ever publishes the first snapshot needed to claim its distinct workspace. In a focused reproduction B stayed unsynchronized until an explicit ensure() or selection. Please arrange an initial/coalesced read for eligible unselected sessions without allowing them to overwrite another scope owner.

Comment thread src/languageService.ts
}
const owned = previous && this.sessionStates.get(previous)?.sessionId === sessionId;
if (owned) {
this.syncSessionScope(previous, undefined, sessionId);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Reclassify already-open files when adding their workspace folder

With multi-server mode and another folder already open, open /project/script.R as an unscoped file, attach a session there, then add /project as a workspace folder. This reconciliation clears the old unscoped state and moves the owner to the project scope, but the existing per-file client remains scoped unscoped: the document listener does not restart it, and the folder listener handles only removals. I reproduced the existing client receiving only the clear and no project client being created. Re-evaluate affected open documents when folders are added so their completion continues without reopening them.

Comment thread src/session.ts
export function updateSessionWorkspace(target: Session, data: WorkspaceData): void {
target.workspaceData = data;
rLanguageService?.syncSessionState(
target.processExited ? undefined : data,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Clear state on the actual Interactive stop-to-exit sequence

The new processExited guard is only reached when updateSessionWorkspace() runs. The normal Stop command first emits stopping, which changes workspaceUnavailable and publishes the old snapshot while processExited is false; it then emits exited, but both statuses have the same unavailable message, so updateStatus() does not publish again after setting processExited to true. The retained view's package state remains in its clients. I reproduced the real manager event/refresh/status methods with mocked dependencies; the new test explicitly calls updateSessionWorkspace() after setting the flag and therefore misses this sequence. Clear on definitive exit and test the actual event sequence.

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.

2 participants