[Fred-Wu]

Closes #1780

Summary

This PR makes standalone R plot viewers session-aware across the JGD, httpgd, and standard backends. Plots remain associated with their originating R process, multiple devices share their session’s history, and retained plots can be reopened after VS Code reloads while R remains running.

New features

• Separate plot viewers for each R session. JGD and standard viewers are associated with their owning session; httpgd devices belonging to the same session share one viewer.
• Shared history across graphics devices. JGD and httpgd plots appear in a session-wide timeline, preserving their order across device changes. Navigation, resizing, export, and removal target the appropriate device.
• Plot restoration after VS Code reload. JGD history and selection are persisted. httpgd retains rendered SVGs, plot order, selection, hidden plots, and connection information. Standard viewers retrieve the latest plot from the reattached R session.
• History survives graphics-device closure. Previously received JGD and httpgd plots remain available for navigation while the owning R process continues running. Cached httpgd plots can also be exported as SVG after their server becomes unavailable.
• Session information in plot viewers. A shared information indicator displays the owning session’s R version and process ID.

What changed

• Show Viewers targets the active attached R session, rather than revealing every available plot viewer. Toolbar commands target the focused viewer. Commands without an explicit viewer target use a viewer owned by the active attached R session.
• Viewer ownership uses the session ID, host, and PID. Device identifiers and httpgd credentials remain associated with their individual devices.
• Closing a JGD or httpgd panel retains its history. Updates to existing plots refresh retained content without reopening the panel; new plots or an explicit Show Viewers action can reopen it.
• Reload and transport cleanup preserve restorable plot state. A confirmed R-process exit releases clients, history, and saved connection information while leaving an already rendered tab visible.
• R runtime reconnection preserves graphics state. VS Code JGD devices are reopened against the new renderer socket with their logical identities, dimensions, DPI, and current plots restored. Existing httpgd servers are rediscovered.
• Device ownership follows the original graphics-device lifetime, including across disconnection, so reused R device numbers do not transfer ownership to caller-created devices.
• Explicit httpgd::hgd() calls are registered with the viewer alongside devices opened through the default graphics-device factory.
• Inactive httpgd devices use cached scaling to avoid replaying into another active R device. Deferred geometry is rendered when the owning device becomes active and the plot is selected.
• History checkpoints run during normal operation and shutdown. Navigation updates save selection metadata separately from plot content, and httpgd URLs containing tokens are stored in SecretStorage.
• Persisted JGD drawing operations and httpgd SVG payloads use a shared disk cache capped at 256 MiB per workspace, located in VS Code’s workspace storage. workspaceState retains metadata and selection. The limit applies to persisted payloads; live viewer history remains in memory.
• Shared viewer registration and lifecycle handling replace duplicated management logic. Obsolete httpgd webview assets are removed, and the bundled sess version advances to 3.4.0.9006.

Fixes

• Prevent standard plot notifications and viewer commands from acting on another R session’s plots.
• Preserve plot history across editor reloads instead of losing it when the extension host and renderer restart.

Testing

• Add regression coverage for session isolation, device ownership, navigation, resizing, restoration, device closure, and process-exit cleanup.
• Cover large persisted histories and cache size limits, device-number reuse during reconnect, preservation of closed-device histories, and caller-owned devices remaining outside sess ownership.
• Add live-R lifecycle tests and a dedicated plot-lifecycle extension-test profile. CI requires the relevant R dependencies and runs the profile across its platform matrix.
• CI installs R and system dependencies through r-lib/actions/setup-r-dependencies. Plot suites are automatically discovered from src/test/plot.
• Latest local Windows validation: 1,292 R checks, 350 Node tests, and 818 extension tests passed, including 208 plot-lifecycle extension tests and seven live-R reconnect tests. Existing platform/environment guards skipped 85 Node tests and 64 extension tests.
• TypeScript compilation, R and TypeScript lint, formatting, build, VSIX packaging, and installed/packaged sess source-identity checks passed. The latest changes still require hosted Linux/macOS/Windows CI confirmation.

[eitsupi]

Reviewed with ChatGPT.

I found several issues and would like to request changes before merging.

1. Unbounded plot history storage

   • JGD drawing operations and httpgd SVG data are persisted in workspaceState, but there appears to be no limit on the serialized data size.
   • Complex plots or long sessions could result in excessive storage usage. Please consider a storage size limit or a separate cache mechanism, and add tests for large plot histories.

2. Plot commands may target another session

   • CommonPlotManager.activeViewer falls back to the most recently used JGD or httpgd viewer without checking whether it belongs to the active R session.
   • Commands invoked without an explicit viewer target could therefore operate on another session's plot history. Please ensure the fallback respects session ownership.

3. CI and test configuration changes

   • .github/workflows/main.yml: Adding master to the workflow triggers seems unrelated to this PR. Please revert it.
   • Installing libtiffxx6 on Linux appears justified by the unigd dependency. But using r-lib action seems better than this.
   • Verifying R package availability and setting VSCR_REQUIRE_PLOT_TESTS=1 are reasonable ways to ensure the live plot tests are actually executed.
   • .vscode-test.mjs: Separating plot lifecycle tests into their own VS Code test profile is reasonable because these tests manipulate global extension state. However, manually maintaining the plotSuites filename list could lead to newly added tests being omitted. Please consider a more maintainable way to group these tests.

4. Overly broad error handling in R/tests/attach_sess.R

   • The modified readLines() handler suppresses all errors rather than only handling files that have disappeared.
   • Please revert this change unless it is necessary. If Windows file locking requires retries, please handle only the expected transient errors and explain why this change belongs in this PR.

5. Questionable synchronization in sessionTerminalLifecycle.test.ts

   • Calling fsp.readFile() while advancing fake timers does not guarantee completion of the original asynchronous operation.
   • Please check whether limiting the fake timers to setTimeout, clearTimeout, and Date is sufficient. If additional synchronization is needed, please use an explicit mechanism rather than relying on unrelated filesystem activity.

6. Implementation-dependent test in sessionTerminalIntegration.test.ts

   • Spying on fsp.rename() couples the test to the file publication implementation rather than its observable behavior.
   • Please consider retaining the original behavior-based assertion, with narrowly scoped handling for transient Windows file access errors if necessary.

The new plot-specific tests provide useful coverage, and I am not suggesting removing them. My concern is primarily with unrelated changes to existing tests and CI configuration that may be working around timing issues without addressing their underlying causes.

Please address the first two issues and reconsider the unrelated CI and test modifications. If any of those modifications are necessary, please explain the underlying failures and why the proposed changes are appropriate.

[renkun-ken]

Reviewed commit 9eb31681e877f271a5ddd7f9592882afae270349. I found two additional reproducible device-lifetime regressions in the reload/reconnect path; details are inline.

• JGD restoration can process a stale closed-device registration after a live device is reopened into its number, overwriting the closed device's retained plot and redirecting subsequent plots to its logical identity.
• Standard restoration retains a numeric null-device ID and ownership registration across disconnection, allowing a caller-created PDF that reuses that number to be captured and later closed by sess.

Validation: compilation and TypeScript checks passed; all 28 focused Node/live-R tests and all 199 plot-lifecycle extension tests passed locally on macOS (VS Code 1.145.2). I also reproduced the two uncovered cases with live R; the standard case was compared with the base revision. All six hosted checks are green on this head.

Please add regression coverage for these device-reuse cases and fix the restoration ownership checks before merging.

[renkun-ken]

Reviewed the changes through 3ec8a47636baa8e98ee2ad63eac53a78fa1de7ec.

Both of my earlier device-reuse findings are fixed. I reran the original live-R reproductions: JGD retains the survivor's logical key, and the caller-owned standard PDF is neither captured nor closed. The active-session command fallback now checks session ID, host and PID. The CI/test grouping and synchronization changes also address the earlier review requests.

I found one remaining P2 in the new disk cache: replacing a plot can evict unchanged history even when the final checkpoint fits the byte budget. I also reproduced a smaller P3 startup race that misses checkpoint cleanup when R exits during asynchronous history loading. Details are inline.

Validation: compilation/typechecking passed; 39 focused Node/live-R/cache tests, 208 plot-lifecycle extension tests, 88 session/terminal extension tests and 62 focused R assertions passed locally. All six hosted checks are green on this head. The two new cases are not covered by those tests.

Please fix the cache replacement eviction before merging and add regression coverage for both cases.

[renkun-ken]

Reviewed 7fba664714773dfc0afa952e9ce307b76d7caf38. Both remaining findings are resolved, and I reran their original reproductions successfully.

The cache now computes the complete retained-reference set before writing replacement payloads, reclaims superseded content first, and preserves references belonging to the other backend. Equal-sized replacement at a full quota retains both current plots. JGD checkpoint observation is now installed before httpgd initialization yields, so an R exit during startup removes both saved metadata and payloads.

I found no additional actionable issues in the follow-up diff. Compilation/typechecking, all 41 focused Node/live-R/cache tests, and all 209 plot-lifecycle extension tests passed locally on macOS. All six hosted checks are green on this head.

[eitsupi]

Reviewed the latest changes with ChatGPT (7fba664).

Most of my previous concerns appear to have been addressed. The plot history now uses a size-limited disk cache, command fallback respects session ownership, and the unrelated workflow trigger and overly broad error handling have been reverted. Moving the plot tests into a dedicated directory also resolves the test grouping concern.

I have two remaining observations:

1. CI dependency installation

   The switch to r-lib/actions/setup-r-dependencies is reasonable, especially because it handles Linux system dependencies automatically.

   However, with working-directory: sess, the action installs all dependencies from sess/DESCRIPTION, including Suggests, plus the six explicitly listed extra-packages. The subsequent pnpm run build still calls remotes::install_local(..., dependencies=TRUE, force=TRUE).

   This seems somewhat redundant. Could we review whether all six extra packages are still needed, particularly renv, and whether dependency installation can be simplified without losing test coverage?

   I would not suggest switching directly to dependencies: "hard", since the plot tests require packages listed under Suggests.

2. Test synchronization

   The changes to sessionTerminalLifecycle.test.ts are an improvement, particularly the explicit control over startup-file reads.

   However, waitForTimerDelay() and several other tests still call fsp.readFile() while waiting for fake timers and asynchronous operations to settle. These reads do not directly synchronize with the operations being tested.

   It would be preferable to use explicit synchronization or controlled filesystem mocks consistently, rather than relying on additional filesystem activity to advance the event loop.

These are primarily maintainability concerns rather than confirmed functional regressions. I am leaving them as comments rather than requesting changes.

[Fred-Wu]

Thanks for all the reviews and suggestions.

The renv tests already existed before this PR and it is necessary for existing test coverage.

Now the installation split the existing build into two steps.

1. Compile the extension and prepare bundled sess.
2. Install bundled sess with dependencies = FALSE, upgrade = "never".

Requested changes were addressed. Reviewer confirmed in their subsequent comment.

[eitsupi]

Why did you merge it before I reviewed it?
Also, what is the reason for not using squash merges? I think it makes the commit history very difficult to read.

@renkun-ken Is it possible to restrict the merge method to squash merge only? (I don't have permission to change the settings.)

[Fred-Wu]

Why did you merge it before I reviewed it? Also, what is the reason for not using squash merges? I think it makes the commit history very difficult to read.

@renkun-ken Is it possible to restrict the merge method to squash merge only? (I don't have permission to change the settings.)

I thought the final comments cleared the past commits, and further changes just small CI improvements and passed all the tests. That was my mistake.

[renkun-ken]

Why did you merge it before I reviewed it? Also, what is the reason for not using squash merges? I think it makes the commit history very difficult to read.

@renkun-ken Is it possible to restrict the merge method to squash merge only? (I don't have permission to change the settings.)

I've done the following changes:

• Keep Allow squash merging checked.
• Uncheck Allow merge commits.
• Uncheck Allow rebase merging.

[eitsupi]

Thanks!