Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 46 additions & 11 deletions extensions/positron-r/src/runtime-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ import { createJupyterKernelSpec } from './kernel-spec';
import { LOGGER, supervisorApi } from './extension';
import { POSITRON_R_INTERPRETERS_DEFAULT_SETTING_KEY } from './constants';
import { getDefaultInterpreterPath } from './interpreter-settings.js';
import { dirname } from 'path';
import { getEnvironmentModulesApi } from './provider-module.js';
import { setupArkJupyterKernel } from './kernel';
import { getRTerminalEnvironmentMutations } from './terminal-environment';
import { RSessionManager } from './session-manager';

export class RRuntimeManager implements positron.LanguageRuntimeManager {

Expand All @@ -31,6 +32,26 @@ export class RRuntimeManager implements positron.LanguageRuntimeManager {
constructor(private readonly _context: vscode.ExtensionContext) {
this.onDidDiscoverRuntime = this.onDidDiscoverRuntimeEmitter.event;
this.onDidCompleteDiscovery = this._onDidCompleteDiscoveryEmitter.event;

// Keep the contributed terminal environment in sync with the active R
// console. Creating or restoring a session updates it (see
// createSession/restoreSession); this handles the case where the user
// switches the foreground session between R consoles that are already
// running different R versions, so a newly launched terminal matches the
// console the user is currently working in. When multiple R consoles
// exist, the most recently focused one wins; this ambiguity is expected
// (https://github.com/posit-dev/positron/issues/7403).
this._context.subscriptions.push(
positron.runtime.onDidChangeForegroundSession(async (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.

Do we need to think about how this relates to the pre-existing handler for onDidChangeForegroundSession()? Should they be combined? Or if not, should they look more similar?

private async didChangeForegroundSession(sessionId: string | undefined): Promise<void> {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I do think they should be more similar, after evaluating them! I addressed this by adding a new event so that these codepaths agree on when we should be responding to changes. 785ec2d

if (!sessionId) {
return;
}
const session = await RSessionManager.instance.getSessionById(sessionId);
if (session) {
this.updateEnvironment(session.runtimeMetadata);
}
})
);
}

/**
Expand Down Expand Up @@ -145,17 +166,31 @@ export class RRuntimeManager implements positron.LanguageRuntimeManager {
return;
}

// Update the QUARTO_R environment variable to point to the script path
// of the R runtime, if needed. Note that our 'scriptpath' is the full
// path to the Rscript binary (foo/bar/Rscript), but Quarto expects the
// directory (foo/bar)
if (metadataExtra.scriptpath) {
const currentQuartoR = collection.get('QUARTO_R');
const scriptPath = dirname(metadataExtra.scriptpath);
if (currentQuartoR?.value !== scriptPath) {
collection.replace('QUARTO_R', scriptPath);
LOGGER.debug(`Updated QUARTO_R environment variable to ${scriptPath}`);
// Contribute environment variables so that terminals launched from
// Positron use the same R installation as the active console (PATH,
// R_HOME, QUARTO_R). This ensures that extensions which start R in a
// terminal (Quarto Preview, Shiny Run App, etc.) run against the R the
// user selected. Apply at both process creation and shell integration so
// the variables are present however the terminal resolves them.
const options = { applyAtProcessCreation: true, applyAtShellIntegration: true };
for (const mutation of getRTerminalEnvironmentMutations(metadataExtra)) {
// Skip variables that already hold the desired value, to avoid
// needlessly marking open terminals as stale.
if (collection.get(mutation.variable)?.value === mutation.value) {
continue;
}
switch (mutation.action) {
case 'replace':
collection.replace(mutation.variable, mutation.value, options);
break;
case 'prepend':
collection.prepend(mutation.variable, mutation.value, options);
break;
case 'append':
collection.append(mutation.variable, mutation.value, options);
break;
}
LOGGER.debug(`Updated terminal environment variable ${mutation.variable} (${mutation.action}) to ${mutation.value}`);
}

// Update the ark Jupyter kernel spec with this R's environment.
Expand Down
97 changes: 97 additions & 0 deletions extensions/positron-r/src/terminal-environment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/*---------------------------------------------------------------------------------------------
* Copyright (C) 2026 Posit Software, PBC. All rights reserved.
* Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
*--------------------------------------------------------------------------------------------*/

import { dirname } from 'path';
import { RMetadataExtra } from './r-installation';

/**
* The kind of mutation to apply to a terminal environment variable. Mirrors the
* `replace`/`prepend`/`append` methods on VS Code's
* `EnvironmentVariableCollection`.
*/
export type TerminalEnvironmentAction = 'replace' | 'prepend' | 'append';

/**
* A single mutation to apply to the contributed terminal environment.
*/
export interface TerminalEnvironmentMutation {
readonly action: TerminalEnvironmentAction;
readonly variable: string;
readonly value: string;
}

/**
* Compute the terminal environment variable mutations that make a terminal use
* the same R installation as the active console.
*
* Several extensions (Quarto Preview, Shiny Run App, etc.) start R in a terminal
* to perform background tasks. Without these mutations, those tasks run against
* the system default R rather than the version selected in Positron's console.
*
* When multiple consoles run different R versions, whichever is activated last
* wins; this ambiguity is expected and acceptable (see
* https://github.com/posit-dev/positron/issues/7403).
*
* @param metadataExtra Extra metadata for the active R installation.
* @param platform The platform to compute mutations for. Defaults to the
* current platform; overridable for testing.
* @returns The mutations to apply to the terminal environment collection.
*/
export function getRTerminalEnvironmentMutations(
metadataExtra: RMetadataExtra,
platform: NodeJS.Platform = process.platform
): TerminalEnvironmentMutation[] {
const mutations: TerminalEnvironmentMutation[] = [];
const pathSeparator = platform === 'win32' ? ';' : ':';

// Prepend the directory containing the selected R binary to PATH so that
// `R`, `Rscript`, and tools that shell out to them resolve to the version
// selected in the console rather than the system default. This is the
// primary mechanism by which the terminal's R matches the console's R, and
// mirrors how rig makes a selected R version available (symlinks on PATH).
if (metadataExtra.binpath) {
mutations.push({
action: 'prepend',
variable: 'PATH',
value: dirname(metadataExtra.binpath) + pathSeparator,
});
}

// Set R_HOME so tools that read it (rather than deriving it from the R
// binary) find the selected installation. R_HOME is R-specific, so setting
// it in the terminal does not affect unrelated programs.
if (metadataExtra.homepath) {
mutations.push({
action: 'replace',
variable: 'R_HOME',
value: metadataExtra.homepath,
});
}

// Point QUARTO_R at the directory containing Rscript so that `quarto render`
// (and the bundled Quarto extension) use the selected R version. Note that
// `scriptpath` is the full path to the Rscript binary (foo/bar/Rscript), but
// Quarto expects the directory (foo/bar).
if (metadataExtra.scriptpath) {
mutations.push({
action: 'replace',
variable: 'QUARTO_R',
value: dirname(metadataExtra.scriptpath),
});
}

// We intentionally do NOT set DYLD_LIBRARY_PATH (macOS) or LD_LIBRARY_PATH
// (Linux) here, even though the Ark kernel sets them. Those variables affect
// dynamic linking for *every* program run in the terminal, not just R, and
// can cause unrelated tools to load R's bundled copies of common libraries
// (libcurl, libz, etc.). R's own launcher scripts (`R`/`Rscript`) already
// configure their library paths, so the variables are unnecessary for R to
// work from the terminal. This mirrors rig, which scopes DYLD_LIBRARY_PATH to
// R's launcher script rather than exporting it to the shell. Ark needs the
// variables because it is a compiled binary that loads libR directly,
// bypassing the launcher scripts.

return mutations;
}
116 changes: 116 additions & 0 deletions extensions/positron-r/src/test/terminal-environment.unit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
/*---------------------------------------------------------------------------------------------
* Copyright (C) 2026 Posit Software, PBC. All rights reserved.
* Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
*--------------------------------------------------------------------------------------------*/

import * as assert from 'assert';
import { RMetadataExtra } from '../r-installation';
import { getRTerminalEnvironmentMutations, TerminalEnvironmentMutation } from '../terminal-environment';

/**
* Build an RMetadataExtra with sensible defaults for the paths under test.
*/
function makeMetadataExtra(overrides: Partial<RMetadataExtra> = {}): RMetadataExtra {
return {
homepath: '/opt/R/4.4.0/lib/R',
binpath: '/opt/R/4.4.0/bin/R',
scriptpath: '/opt/R/4.4.0/bin/Rscript',
current: false,
default: false,
reasonDiscovered: null,
...overrides,
};
}

/**
* Find the mutation for a given variable, or undefined if none exists.
*/
function find(mutations: TerminalEnvironmentMutation[], variable: string): TerminalEnvironmentMutation | undefined {
return mutations.find(m => m.variable === variable);
}

suite('getRTerminalEnvironmentMutations', () => {

test('prepends the R binary directory to PATH (posix separator)', () => {
const mutations = getRTerminalEnvironmentMutations(makeMetadataExtra(), 'darwin');
const path = find(mutations, 'PATH');

assert.ok(path, 'expected a PATH mutation');
assert.strictEqual(path!.action, 'prepend');
assert.strictEqual(path!.value, '/opt/R/4.4.0/bin:');
});

test('prepends the R binary directory to PATH (windows separator)', () => {
const mutations = getRTerminalEnvironmentMutations(
makeMetadataExtra({ binpath: 'C:/R/R-4.4.0/bin/x64/R.exe' }),

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.

I think the binpath on Windows probably ends up with backslashes. Would it be a good idea for this fixture to be more realistic?

pth = path.normalize(pth);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done in 173ec3c!

'win32'
);
const path = find(mutations, 'PATH');

assert.ok(path, 'expected a PATH mutation');
assert.strictEqual(path!.action, 'prepend');
assert.strictEqual(path!.value, 'C:/R/R-4.4.0/bin/x64;');
});

test('replaces R_HOME with the home path', () => {
const mutations = getRTerminalEnvironmentMutations(makeMetadataExtra(), 'darwin');
const rHome = find(mutations, 'R_HOME');

assert.ok(rHome, 'expected an R_HOME mutation');
assert.strictEqual(rHome!.action, 'replace');
assert.strictEqual(rHome!.value, '/opt/R/4.4.0/lib/R');
});

test('sets QUARTO_R to the directory containing Rscript, not Rscript itself', () => {
const mutations = getRTerminalEnvironmentMutations(makeMetadataExtra(), 'darwin');
const quartoR = find(mutations, 'QUARTO_R');

assert.ok(quartoR, 'expected a QUARTO_R mutation');
assert.strictEqual(quartoR!.action, 'replace');
assert.strictEqual(quartoR!.value, '/opt/R/4.4.0/bin');
});

test('does not set library-path variables on macOS', () => {
const mutations = getRTerminalEnvironmentMutations(makeMetadataExtra(), 'darwin');

assert.strictEqual(find(mutations, 'DYLD_LIBRARY_PATH'), undefined);
assert.strictEqual(find(mutations, 'LD_LIBRARY_PATH'), undefined);
});

test('does not set library-path variables on Linux', () => {
const mutations = getRTerminalEnvironmentMutations(makeMetadataExtra(), 'linux');

assert.strictEqual(find(mutations, 'LD_LIBRARY_PATH'), undefined);
assert.strictEqual(find(mutations, 'DYLD_LIBRARY_PATH'), undefined);
});

test('omits PATH when there is no binary path', () => {
const mutations = getRTerminalEnvironmentMutations(
makeMetadataExtra({ binpath: '' }),
'darwin'
);

assert.strictEqual(find(mutations, 'PATH'), undefined);
// The other variables are still contributed.
assert.ok(find(mutations, 'R_HOME'));
assert.ok(find(mutations, 'QUARTO_R'));
});

test('omits QUARTO_R when there is no script path', () => {
const mutations = getRTerminalEnvironmentMutations(
makeMetadataExtra({ scriptpath: '' }),
'darwin'
);

assert.strictEqual(find(mutations, 'QUARTO_R'), undefined);
assert.ok(find(mutations, 'PATH'));
assert.ok(find(mutations, 'R_HOME'));
});

test('contributes only the expected variables', () => {
const mutations = getRTerminalEnvironmentMutations(makeMetadataExtra(), 'darwin');
const variables = mutations.map(m => m.variable).sort();

assert.deepStrictEqual(variables, ['PATH', 'QUARTO_R', 'R_HOME']);
});
});
Loading