From 4aed1a57a62179a57e0f50e8aed3b9fade48c5e2 Mon Sep 17 00:00:00 2001 From: Kun Ren Date: Sun, 25 Oct 2026 10:32:03 +0800 Subject: [PATCH 1/4] perf: streamline session refresh, replay, and output delivery --- scripts/benchmark-console-output.cjs | 38 ++ scripts/benchmark-journal.cjs | 44 ++ scripts/benchmark-workspace-refresh.R | 30 + sess/R/handlers.R | 65 ++- sess/R/server.R | 2 +- sess/inst/tinytest/test-workspace-refresh.R | 85 +++ src/completions.ts | 112 +++- src/interactive/README.md | 27 + src/interactive/agent.ts | 135 +++-- src/interactive/cellIndex.ts | 23 + src/interactive/client.ts | 96 ++- src/interactive/journal.ts | 547 ++++++++++++------ src/interactive/journalIndex.ts | 520 +++++++++++++++++ src/interactive/manager.ts | 157 +++-- src/interactive/messagePages.ts | 98 ++++ src/interactive/notebookOutput.ts | 96 ++- src/interactive/streamText.ts | 61 ++ src/interactive/transcript.ts | 21 +- src/nativeRpc.ts | 181 ++++++ src/rTerminal.ts | 2 +- src/session.ts | 236 +++----- src/signatureHelp.ts | 39 +- src/test/node/incrementalOutput.test.ts | 276 +++++++++ src/test/node/interactiveBackend.test.ts | 45 ++ src/test/node/journalEfficiency.test.ts | 240 ++++++++ src/test/node/messagePages.test.ts | 319 ++++++++++ src/test/node/nativeRpc.test.ts | 243 ++++++++ src/test/node/workspaceRefresh.test.ts | 105 ++++ src/test/suite/completions.test.ts | 147 +++++ src/test/suite/notebookStream.test.ts | 83 +++ .../suite/sessionWorkspaceRefresh.test.ts | 110 ++++ src/test/suite/signatureHelp.test.ts | 67 ++- src/workspaceRefresh.ts | 80 +++ 33 files changed, 3911 insertions(+), 419 deletions(-) create mode 100644 scripts/benchmark-console-output.cjs create mode 100644 scripts/benchmark-journal.cjs create mode 100644 scripts/benchmark-workspace-refresh.R create mode 100644 sess/inst/tinytest/test-workspace-refresh.R create mode 100644 src/interactive/cellIndex.ts create mode 100644 src/interactive/journalIndex.ts create mode 100644 src/interactive/messagePages.ts create mode 100644 src/interactive/streamText.ts create mode 100644 src/nativeRpc.ts create mode 100644 src/test/node/incrementalOutput.test.ts create mode 100644 src/test/node/journalEfficiency.test.ts create mode 100644 src/test/node/messagePages.test.ts create mode 100644 src/test/node/nativeRpc.test.ts create mode 100644 src/test/node/workspaceRefresh.test.ts create mode 100644 src/test/suite/notebookStream.test.ts create mode 100644 src/test/suite/sessionWorkspaceRefresh.test.ts create mode 100644 src/workspaceRefresh.ts diff --git a/scripts/benchmark-console-output.cjs b/scripts/benchmark-console-output.cjs new file mode 100644 index 000000000..163e6c8e6 --- /dev/null +++ b/scripts/benchmark-console-output.cjs @@ -0,0 +1,38 @@ +// Run after TypeScript compilation: node scripts/benchmark-console-output.cjs +const assert = require('node:assert/strict'); +const { performance } = require('node:perf_hooks'); +const { StreamText } = require('../out/interactive/streamText'); +const { NotebookOutputState, prepareNotebookStream, updateNotebookOutputs } = require('../out/interactive/notebookOutput'); + +async function measure(incremental, writes) { + const stream = new StreamText(''); + const state = new NotebookOutputState(); + const outputs = []; + let delivered = 0; + const normalize = value => 'mime' in value ? [value] : [...value]; + const execution = { + replaceOutput: value => { outputs.splice(0, outputs.length, ...value); return Promise.resolve(); }, + appendOutput: value => { delivered += value.flatMap(output => output.items).reduce((sum, item) => sum + item.data.length, 0); outputs.push(...value); return Promise.resolve(); }, + replaceOutputItems: (value, output) => { const items = normalize(value); delivered += items.reduce((sum, item) => sum + item.data.length, 0); output.items = items; return Promise.resolve(); }, + appendOutputItems: (value, output) => { const items = normalize(value); delivered += items.reduce((sum, item) => sum + item.data.length, 0); output.items = [{ mime: items[0].mime, data: Buffer.concat([...output.items, ...items].map(item => item.data)) }]; return Promise.resolve(); }, + }; + const started = performance.now(); + for (let write = 0; write < writes; write++) { + stream.append('x'.repeat(32768)); + const output = { metadata: { rStreamId: 'console' }, items: [{ mime: 'application/vnd.code.notebook.stdout', data: incremental ? Buffer.alloc(0) : Buffer.from(stream.snapshot().text()) }] }; + if (incremental) prepareNotebookStream(output, stream.snapshot()); + await updateNotebookOutputs(execution, [...outputs], [output], state); + } + assert.equal(outputs[0].items[0].data.length, writes * 32768); + return { incremental, writes, finalMiB: writes / 32, deliveredMiB: delivered / 1024 / 1024, elapsedMs: +(performance.now() - started).toFixed(3) }; +} + +async function main() { + for (const writes of [32, 128, 512]) { + const before = await measure(false, writes); + const after = await measure(true, writes); + assert.equal(after.deliveredMiB, after.finalMiB); + console.log(JSON.stringify({ before, after })); + } +} +main().catch(error => { console.error(error); process.exitCode = 1; }); diff --git a/scripts/benchmark-journal.cjs b/scripts/benchmark-journal.cjs new file mode 100644 index 000000000..9dc9af2a2 --- /dev/null +++ b/scripts/benchmark-journal.cjs @@ -0,0 +1,44 @@ +// Run after compiling TypeScript: node scripts/benchmark-journal.cjs +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); +const { performance } = require('node:perf_hooks'); +const { SessionJournal, readPreviousJournalAsync } = require(process.argv[2] || '../out/interactive/journal'); + +async function main() { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'r-journal-benchmark-')); + try { + for (const outputMiB of [16, 112]) { + const directory = path.join(root, String(outputMiB), 'generation'); + const journal = new SessionJournal(directory, 'generation'); + for (let order = 0; order < outputMiB / 4; order++) { + const record = journal.accept({ id: 'old' + order, code: 'print(large_result)' }).record; + journal.append('accepted', { record }, record.id); + for (let chunk = 0; chunk < 128; chunk++) journal.append('stream', { text: 'x'.repeat(32768), channel: 'stdout' }, record.id); + const final = { ...record, state: 'success', ended: Date.now() }; + journal.update(final); journal.append('finished', { record: final }, record.id); + } + const latest = journal.accept({ id: 'latest', code: '1 + 1' }).record; + journal.append('accepted', { record: latest }, latest.id); + journal.append('stream', { text: '[1] 2\n', channel: 'stdout' }, latest.id); + journal.append('finished', { record: { ...latest, state: 'success' } }, latest.id); + const samples = []; + for (let run = 0; run < 7; run++) { + const start = performance.now(), recent = journal.recent(1); + assert.equal(recent.events.length, 3); + samples.push(performance.now() - start); + } + journal.close(); + const archiveSamples = []; + for (let run = 0; run < 7; run++) { + const start = performance.now(), recent = await readPreviousJournalAsync(path.dirname(directory), 'generation', 1); + assert.equal(recent.events.length, 3); + archiveSamples.push(performance.now() - start); + } + samples.sort((a, b) => a - b); archiveSamples.sort((a, b) => a - b); + console.log(JSON.stringify({ outputMiB, recentMedianMs: +samples[3].toFixed(3), archivedMedianMs: +archiveSamples[3].toFixed(3) })); + } + } finally { fs.rmSync(root, { recursive: true, force: true }); } +} +main().catch(error => { console.error(error); process.exitCode = 1; }); diff --git a/scripts/benchmark-workspace-refresh.R b/scripts/benchmark-workspace-refresh.R new file mode 100644 index 000000000..394a1e2c8 --- /dev/null +++ b/scripts/benchmark-workspace-refresh.R @@ -0,0 +1,30 @@ +# Run from the repository root: Rscript --vanilla scripts/benchmark-workspace-refresh.R +# Metadata is always recomputed; unchanged replies avoid the full JSON encoding. +local({ + helpers <- new.env(parent = globalenv()) + source("sess/R/zzz.R", local = helpers) + source("sess/R/handlers.R", local = helpers) + encode <- function(snapshot) { + jsonlite::toJSON(list(jsonrpc = "2.0", id = 1L, result = snapshot), + auto_unbox = TRUE, null = "null", force = TRUE, digits = NA) + } + for (n in c(1000L, 10000L)) { + symbols <- sprintf("workspace_benchmark_%05d", seq_len(n)) + list2env(setNames(rep(list(1:10), n), symbols), envir = .GlobalEnv) + snapshot <- helpers$get_workspace_data() + invisible(encode(snapshot)) + snapshot <- helpers$get_workspace_data() + full <- replicate(5L, unname(system.time(encode(helpers$get_workspace_data()))[["elapsed"]])) + unchanged <- replicate(5L, unname(system.time( + encode(helpers$get_workspace_data(snapshot$revision)) + )[["elapsed"]])) + cat(jsonlite::toJSON(list( + objects = n, + full_median_ms = median(full) * 1000, + unchanged_median_ms = median(unchanged) * 1000, + full_bytes = nchar(encode(snapshot), type = "bytes"), + unchanged_bytes = nchar(encode(helpers$get_workspace_data(snapshot$revision)), type = "bytes") + ), auto_unbox = TRUE), "\n", sep = "") + rm(list = symbols, envir = .GlobalEnv) + } +}) diff --git a/sess/R/handlers.R b/sess/R/handlers.R index ad0f10f50..51ce0fe4f 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -36,7 +36,28 @@ workspace_child_count <- function(object) { } } -get_workspace_data <- function() { +# Read structural metadata without invoking arbitrary S3 inspection methods. +# Expensive/custom completion still runs through handle_complete() on demand. +workspace_length <- function(object) { + if (is.environment(object)) return(length(workspace_env_names(object))) + if (isS4(object)) return(NULL) + if (inherits(object, "POSIXlt")) return(length(.subset2(object, 1L))) + if (is.data.frame(object)) return(length(attr(object, "names", exact = TRUE))) + if (!is.object(object)) return(length(object)) + dimensions <- attr(object, "dim", exact = TRUE) + if (!is.null(dimensions)) return(prod(dimensions)) + object_names <- attr(object, "names", exact = TRUE) + if (!is.null(object_names)) return(length(object_names)) + if (typeof(object) %in% c("closure", "builtin")) return(1L) + # unclass() can copy an entire classed vector. Check dispatch before using the + # primitive length instead; unknown custom lengths belong to explicit queries. + has_method <- any(vapply(c(class(object), "default"), function(class_name) { + !is.null(utils::getS3method("length", class_name, optional = TRUE, envir = .GlobalEnv)) + }, logical(1))) + if (has_method) NULL else length(object) +} + +get_workspace_data <- function(revision = NULL) { env <- .GlobalEnv all_names <- workspace_env_names(env) @@ -54,8 +75,12 @@ get_workspace_data <- function() { obj <- env[[name]] obj_class <- class(obj) obj_type <- typeof(obj) - obj_length <- length(obj) - obj_dim <- dim(obj) + obj_length <- workspace_length(obj) + obj_dim <- if (is.data.frame(obj)) { + c(.row_names_info(obj, 2L), obj_length) + } else { + attr(obj, "dim", exact = TRUE) + } first_class <- if (length(obj_class)) obj_class[[1]] else obj_type info <- list( class = class(obj), @@ -66,17 +91,25 @@ get_workspace_data <- function() { } else if (obj_type == "environment") { "" } else if (obj_type %in% c("closure", "builtin")) { - trimws(try_capture_str(obj)) + trimws(try_capture_str(if (is.object(obj)) unclass(obj) else obj)) + } else if (is.null(obj_length)) { + first_class } else { paste0(first_class, ", length ", obj_length) }, - has_children = workspace_child_count(obj) > 0L + has_children = if (is.environment(obj)) { + obj_length > 0L + } else if (isS4(obj)) { + length(methods::slotNames(obj)) > 0L + } else { + !inherits(obj, "POSIXlt") && obj_type %in% c("list", "pairlist") && + (is.null(obj_length) || obj_length > 0L) + } ) + if (is.null(obj_length)) info$length <- NULL - obj_names <- if (is.object(obj)) { - utils::.DollarNames(obj, pattern = "") - } else if (is.recursive(obj)) { - names(obj) + obj_names <- if (obj_type %in% c("list", "pairlist")) { + attr(obj, "names", exact = TRUE) } else { NULL } @@ -95,11 +128,23 @@ get_workspace_data <- function() { }) names(objs) <- all_names - list( + snapshot <- list( globalenv = objs, search = search()[-1], loaded_namespaces = loadedNamespaces() ) + # Compare metadata after inspecting every binding: reference objects can mutate + # in place, and R task callbacks do not cover every embedding's evaluation. + if (!identical(snapshot, .sess_env$workspace_snapshot)) { + .sess_env$workspace_snapshot <- snapshot + .sess_env$workspace_revision <- (.sess_env$workspace_revision %||% 0) + 1 + } + current_revision <- .sess_env$workspace_revision + if (length(revision) == 1L && is.numeric(revision) && + is.finite(revision) && identical(as.numeric(revision), current_revision)) { + return(list(revision = current_revision, unchanged = TRUE)) + } + c(snapshot, list(revision = current_revision)) } workspace_object <- function(name, path = list()) { diff --git a/sess/R/server.R b/sess/R/server.R index 0fb37b410..5e9d7acb2 100644 --- a/sess/R/server.R +++ b/sess/R/server.R @@ -478,7 +478,7 @@ dispatch_message <- function(line) { interactive_stop() TRUE }, - "workspace" = function(p) get_workspace_data(), + "workspace" = function(p) get_workspace_data(p$revision), "workspace_children" = function(p) get_workspace_children(p$name, p$path, p$start, p$view_id), "workspace_view" = function(p) handle_workspace_view(p$name, p$path), "hover" = function(p) handle_hover(p$expr), diff --git a/sess/inst/tinytest/test-workspace-refresh.R b/sess/inst/tinytest/test-workspace-refresh.R new file mode 100644 index 000000000..f32588c49 --- /dev/null +++ b/sess/inst/tinytest/test-workspace-refresh.R @@ -0,0 +1,85 @@ +# Automatic metadata stays structural; explicit completion may inspect class methods. +local({ + env <- sess:::.sess_env + previous_snapshot <- env$workspace_snapshot + previous_revision <- env$workspace_revision + symbols <- c("workspace_refresh_custom", "workspace_refresh_frame", "workspace_refresh_env", + "workspace_refresh_active", ".DollarNames.workspace_refresh_expensive", + "length.workspace_refresh_expensive", "dim.workspace_refresh_expensive", + "workspace_refresh_unknown", "workspace_refresh_large") + previous <- mget(intersect(symbols, ls(.GlobalEnv, all.names = TRUE)), .GlobalEnv) + on.exit({ + if (exists("workspace_refresh_active", .GlobalEnv, inherits = FALSE)) { + rm(list = "workspace_refresh_active", envir = .GlobalEnv) + } + rm(list = intersect(symbols, ls(.GlobalEnv, all.names = TRUE)), envir = .GlobalEnv) + list2env(previous, envir = .GlobalEnv) + env$workspace_snapshot <- previous_snapshot + env$workspace_revision <- previous_revision + }) + inspected <- 0L + expensive <- function(...) { + inspected <<- inspected + 1L + stop("automatic metadata invoked a user method") + } + for (method in symbols[5:7]) assign(method, expensive, .GlobalEnv) + assign(symbols[[1L]], structure(list(a = 1L, b = 2L), + class = "workspace_refresh_expensive"), .GlobalEnv) + assign(symbols[[2L]], data.frame(a = 1:3, b = letters[1:3]), .GlobalEnv) + reference <- new.env(parent = emptyenv()) + reference$a <- 1L + assign(symbols[[3L]], reference, .GlobalEnv) + makeActiveBinding(symbols[[4L]], expensive, .GlobalEnv) + + snapshot <- sess:::get_workspace_data() + expect_identical(inspected, 0L) + expect_identical(snapshot$globalenv[[symbols[[1L]]]]$names, c("a", "b")) + expect_identical(snapshot$globalenv[[symbols[[1L]]]]$length, 2L) + expect_equal(snapshot$globalenv[[symbols[[2L]]]]$dim, c(3L, 2L)) + expect_identical(snapshot$globalenv[[symbols[[2L]]]]$names, c("a", "b")) + expect_identical(snapshot$globalenv[[symbols[[4L]]]]$type, "active_binding") + expect_identical(sess:::get_workspace_data(snapshot$revision), + list(revision = snapshot$revision, unchanged = TRUE)) + + # Metadata is recomputed, rather than caching by address or task callback count. + reference$b <- 2L + changed <- sess:::get_workspace_data(snapshot$revision) + expect_true(changed$revision > snapshot$revision) + expect_identical(changed$globalenv[[symbols[[3L]]]]$length, 2L) + expect_null(changed$unchanged) + frame <- get(symbols[[2L]], .GlobalEnv) + frame$c <- TRUE + assign(symbols[[2L]], frame, .GlobalEnv) + renamed <- sess:::get_workspace_data(changed$revision) + expect_identical(renamed$globalenv[[symbols[[2L]]]]$names, c("a", "b", "c")) + expect_true(renamed$revision > changed$revision) + + assign(symbols[[8L]], structure(1:3, class = "workspace_refresh_expensive"), .GlobalEnv) + unknown <- sess:::get_workspace_data()$globalenv[[symbols[[8L]]]] + expect_null(unknown$length) + expect_identical(unknown$str, "workspace_refresh_expensive") + expect_identical(inspected, 0L) + + # Stripping a large classed vector's attributes must not copy its payload. + assign(symbols[[9L]], structure(rep(1, 1000000L), class = "workspace_refresh_vector"), .GlobalEnv) + if (isTRUE(capabilities("profmem"))) { + profile <- tempfile() + on.exit(unlink(profile), add = TRUE) + Rprofmem(profile, threshold = 4000000L) + large <- sess:::get_workspace_data()$globalenv[[symbols[[9L]]]] + Rprofmem(NULL) + allocations <- readLines(profile, warn = FALSE) + expect_false(any(grepl("^[0-9]+", allocations))) + expect_identical(large$length, 1000000L) + } + + if (requireNamespace("data.table", quietly = TRUE)) { + table <- data.table::data.table(a = 1L) + assign(symbols[[2L]], table, .GlobalEnv) + before <- sess:::get_workspace_data() + data.table::setnames(table, "b") + after <- sess:::get_workspace_data(before$revision) + expect_true(after$revision > before$revision) + expect_identical(after$globalenv[[symbols[[2L]]]]$names, "b") + } +}) diff --git a/src/completions.ts b/src/completions.ts index 0a394f053..789acdbe3 100644 --- a/src/completions.ts +++ b/src/completions.ts @@ -3,6 +3,7 @@ import * as vscode from 'vscode'; import * as session from './session'; import { PipelineSourceCache } from './pipelineCompletion'; +import { withRpcCancellation } from './nativeRpc'; import { globalRHelp } from './extension'; import { config } from './util'; import { getChunks } from './rmarkdown'; @@ -62,11 +63,58 @@ const roxygenTagCompletionItems = [ 'usage', ].map((x: string) => new vscode.CompletionItem(`${x} `)); +async function cancellableSessionRequest( + data: Record, + target: session.Session, + token?: vscode.CancellationToken, +): Promise { + if (!token) { + return session.sessionRequest(data, target); + } + const controller = new AbortController(); + const cancellation = token.onCancellationRequested(() => controller.abort()); + if (token.isCancellationRequested) { + controller.abort(); + } + try { + return await session.sessionRequest(data, target, { signal: controller.signal }); + } finally { + cancellation.dispose(); + } +} + +async function ensureWorkspace( + target: session.Session, + token?: vscode.CancellationToken, +): Promise { + const controller = token && new AbortController(); + const cancellation = token?.onCancellationRequested(() => controller?.abort()); + if (token?.isCancellationRequested) { + controller?.abort(); + } + try { + await withRpcCancellation(() => session.ensureSessionWorkspace(target), controller?.signal); + return !token?.isCancellationRequested; + } catch (error) { + if (error instanceof Error && error.name === 'AbortError') { + return false; + } + throw error; + } finally { + cancellation?.dispose(); + } +} + export class HoverProvider implements vscode.HoverProvider { async provideHover( document: vscode.TextDocument, position: vscode.Position, + token?: vscode.CancellationToken, ): Promise { + if (token?.isCancellationRequested) { + return null; + } + const version = document.version; const target = session.sessionForDocument(document.uri); if (!target?.workspaceData?.globalenv) { return null; @@ -98,14 +146,31 @@ export class HoverProvider implements vscode.HoverProvider { .trimEnd(); // Workspace snapshots already contain simple-symbol summaries and function // formals. Use those while R is busy, and avoid an IPC round trip on every hover. - const cached = /[$@:]/.test(prefix.slice(-1)) - ? undefined - : target.workspaceData.globalenv[symbol]; - if (cached && ['closure', 'builtin'].includes(cached.type)) { - return new vscode.Hover( - new vscode.MarkdownString().appendCodeblock(cached.str, 'r'), - hoverRange, - ); + const functionHover = (): vscode.Hover | undefined => { + const cached = /[$@:]/.test(prefix.slice(-1)) + ? undefined + : target.workspaceData.globalenv[symbol]; + if (cached && ['closure', 'builtin'].includes(cached.type)) { + return new vscode.Hover( + new vscode.MarkdownString().appendCodeblock(cached.str, 'r'), + hoverRange, + ); + } + }; + const immediate = functionHover(); + if (immediate) { + return immediate; + } + if ( + !(await ensureWorkspace(target, token)) || + document.version !== version || + session.sessionForDocument(document.uri) !== target + ) { + return null; + } + const refreshed = functionHover(); + if (refreshed) { + return refreshed; } if (session.globalPipePath || target.requester) { @@ -117,14 +182,22 @@ export class HoverProvider implements vscode.HoverProvider { return null; } const expr = document.getText(hoverRange); - const response = (await session.sessionRequest( + const response = (await cancellableSessionRequest( { method: 'hover', params: { expr: expr }, }, target, + token, )) as { str: string }; + if ( + token?.isCancellationRequested || + document.version !== version || + session.sessionForDocument(document.uri) !== target + ) { + return null; + } if (response) { hoverText = response.str; } @@ -221,6 +294,7 @@ export class LiveCompletionItemProvider implements vscode.CompletionItemProvider completionContext: vscode.CompletionContext, ): Promise { const items: vscode.CompletionItem[] = []; + const version = document.version; const activeSession = session.sessionForDocument(document.uri); if (token.isCancellationRequested || !activeSession?.workspaceData?.globalenv) { return items; @@ -241,6 +315,17 @@ export class LiveCompletionItemProvider implements vscode.CompletionItemProvider codeStartLine = chunk.startLine + 1; } + // Existing metadata stays usable while R is busy. The document lookup + // starts dirty refreshes in the background; only an empty cache must wait. + if (!Object.keys(activeSession.workspaceData.globalenv).length) { + if ( + !(await ensureWorkspace(activeSession, token)) || + document.version !== version || + session.sessionForDocument(document.uri) !== activeSession + ) { + return items; + } + } const trigger = completionContext.triggerCharacter; if (trigger === undefined) { @@ -266,12 +351,13 @@ export class LiveCompletionItemProvider implements vscode.CompletionItemProvider ?.with({ end: symbolPosition }); if (exprRange) { const expr = document.getText(exprRange); - const response = (await session.sessionRequest( + const response = (await cancellableSessionRequest( { method: 'completion', params: { expr: expr, trigger: trigger }, }, activeSession, + token, )) as RObjectElement[]; if (response) { @@ -333,7 +419,11 @@ export class LiveCompletionItemProvider implements vscode.CompletionItemProvider ); } - return token.isCancellationRequested ? [] : items; + return token.isCancellationRequested || + document.version !== version || + session.sessionForDocument(document.uri) !== activeSession + ? [] + : items; } } diff --git a/src/interactive/README.md b/src/interactive/README.md index 30dabcb14..290745047 100644 --- a/src/interactive/README.md +++ b/src/interactive/README.md @@ -71,6 +71,33 @@ currently remains limited to Linux/macOS. Backend-specific preflight and private package installation live behind the registry and [sessPreparation.ts](backends/sessPreparation.ts). vscode-R installs the `sess` snapshot bundled with the extension; missing Imports use the configured repository. Common agent settings do not require a sess library or R path. The agent bundle cache is separate from the sess runtime cache. Existing flat launch configurations are normalized by `backendDescriptor()`; `withBackend()` derives compatibility fields for older readers. The [sess README](../../sess/README.md) documents its R hooks and wire protocol. +## Workspace, history, and output delivery + +Workspace invalidations coalesce per session, with one inspection in flight and +one pending refresh. Obsolete connections cannot publish late metadata. Automatic +sess snapshots use structural metadata; custom member inspection remains an +explicit completion operation. Revisions omit unchanged workspace payloads while +metadata is recomputed to account for mutable R objects. + +Interactive snapshots and large workspace events negotiate pull-based message +paging. Each wire frame stays within the protocol limit, including the complete +envelope and workspace. Pages belong to the requesting connection and expire; +reconnection requests a fresh snapshot rather than resubmitting R code. + +Journal indexes map executions to retained byte ranges and committed display +references. They can be rebuilt from the journal after interrupted writes or +legacy restoration. Execution source and event working sets have byte limits; +the durable admission ledger still prevents duplicate execution after eviction. +Archived history reads use asynchronous file operations. Asset collection retains +its conservative behavior when a journal cannot establish which assets are live. + +Console delivery keeps prefix snapshots of buffered text and appends only the +new suffix to existing notebook output. Rich displays keep their stable identities +and continue to replace changed frames. Failed or obsolete renders do not advance +the delivered offset. Notebook content and metadata edits invalidate execution-ID +lookups; output-only changes preserve the cell index. Completion timing still +includes the final output write. + ## Backend contract and ordering Use the types in [backend.ts](backend.ts) as the source of truth. The essential rules are: diff --git a/src/interactive/agent.ts b/src/interactive/agent.ts index 91898a6a1..07ff9a292 100644 --- a/src/interactive/agent.ts +++ b/src/interactive/agent.ts @@ -10,6 +10,7 @@ import { AssetStore } from './assets'; import { BackendEvent, BackendFactory, SessionBackend, inspectionMethod } from './backend'; import { AGENT_PROTOCOL, + MAX_MESSAGE_BYTES, AgentConfig, AgentSnapshot, ExecutionRecord, @@ -20,13 +21,17 @@ import { identifier, sessionLabel, } from './protocol'; -import { searchHistory } from './history'; +import { MessagePages, boundedJson } from './messagePages'; +import { isDeepStrictEqual } from 'node:util'; +import { WorkspaceRefreshScheduler } from '../workspaceRefresh'; interface Client { socket: net.Socket; authenticated: boolean; subscribed: boolean; id: string; + paging?: boolean; + pages: MessagePages; } /** This process never imports vscode and is supervised independently of the editor. */ export class SessionAgent { @@ -44,6 +49,7 @@ export class SessionAgent { private current?: string; private input?: Record; private workspace?: Record; + private workspaceRefresh: WorkspaceRefreshScheduler>; private outputBytes = new Map(); private truncated = new Set(); private retentionWarnings = new Set(); @@ -77,6 +83,30 @@ export class SessionAgent { () => retainedAssetIds(config.storage), ); this.backend = factory(config); + this.workspaceRefresh = new WorkspaceRefreshScheduler( + async () => { + const response = object( + await this.backend.inspect({ + method: 'workspace', + params: { revision: this.workspace?.revision }, + timeout: 2000, + }), + ); + return response.unchanged ? undefined : response; + }, + (workspace) => { + if (!isDeepStrictEqual(workspace, this.workspace)) { + this.workspace = workspace; + this.event('workspace', workspace); + } + }, + () => + !this.stopped && + !this.stopping && + this.backendReady && + !this.current && + this.manifest.capabilities.inspection !== false, + ); this.manifest = { protocol: AGENT_PROTOCOL, id: config.id, @@ -103,7 +133,7 @@ export class SessionAgent { supervision: config.supervision, }; // A previous agent's accepted/running work must never be replayed automatically. - for (const entry of this.journal.executions.values()) { + for (const entry of this.journal.executions.unfinished()) { if (entry.state === 'running' || entry.state === 'queued') { this.journal.update({ ...entry, state: 'unknown', ended: Date.now() }); } @@ -200,17 +230,54 @@ export class SessionAgent { } private send(socket: net.Socket, message: unknown): void { - if (!socket.destroyed) { - if (socket.writableLength > 8 * 1024 * 1024) { + if (socket.destroyed) { + return; + } + if (socket.writableLength > 8 * 1024 * 1024) { + socket.destroy(); + return; + } + let encoded = boundedJson(message); + if (Buffer.byteLength(encoded) > MAX_MESSAGE_BYTES) { + const client = [...this.clients].find((entry) => entry.socket === socket); + const value = message as { id?: number; result?: unknown; event?: SessionEvent }; + if (client?.paging) { + try { + encoded = value.event + ? JSON.stringify({ eventPage: client.pages.retain(value.event) }) + : JSON.stringify({ + id: value.id, + resultPage: client.pages.retain(value.result), + }); + } catch (error) { + if (value.id !== undefined) { + encoded = JSON.stringify({ id: value.id, error: String(error) }); + } else { + socket.destroy(); + return; + } + } + } else if (value.id !== undefined) { + encoded = JSON.stringify({ + id: value.id, + error: 'Response exceeds the message limit; update vscode-R to read paged snapshots', + }); + } else { socket.destroy(); return; } - socket.write(JSON.stringify(message) + '\n'); } + socket.write(encoded + '\n'); } private connectClient(socket: net.Socket): void { - const client: Client = { socket, authenticated: false, subscribed: false, id: '' }; + const client: Client = { + socket, + authenticated: false, + subscribed: false, + id: '', + pages: new MessagePages(), + }; this.clients.add(client); const authTimer = setTimeout(() => { if (!client.authenticated) { @@ -257,6 +324,7 @@ export class SessionAgent { throw new Error('Invalid request'); } const params = object(request.params ?? {}); + client.pages.expire(); if (!client.authenticated) { if ( request.method !== 'hello' || @@ -268,6 +336,7 @@ export class SessionAgent { } client.id = identifier(params.clientId); client.authenticated = true; + client.paging = params.paging === true; this.send(client.socket, { id: request.id, result: this.manifest }); return; } @@ -276,6 +345,9 @@ export class SessionAgent { } let result: unknown = true; switch (request.method) { + case 'messagePage': + result = client.pages.read(String(params.token), Number(params.offset)); + break; case 'claim': if ( !this.lease || @@ -311,7 +383,7 @@ export class SessionAgent { if (!Number.isSafeInteger(before) || Number(before) < 1) { throw new Error('Invalid history cursor'); } - result = searchHistory(this.journal.executions.values(), query, Number(before)); + result = this.journal.executions.history(query, Number(before)); break; } case 'rename': { @@ -333,18 +405,24 @@ export class SessionAgent { case 'snapshot': { const recent = this.journal.recent( Math.max(1, Math.min(Number(params.limit) || 50, this.config.historyLimit)), + this.current, ); // Bound one protocol response. Older/full output remains in the journal. let size = 0; const executions: ExecutionRecord[] = []; - for (const record of recent.executions.slice().reverse()) { + for (const record of recent.executions + .slice() + .sort((a, b) => + a.id === this.current ? -1 : b.id === this.current ? 1 : b.order - a.order, + )) { const bytes = Buffer.byteLength(JSON.stringify(record)); if (size + bytes > 1400 * 1024) { break; } size += bytes; - executions.unshift(record); + executions.push(record); } + executions.sort((a, b) => a.order - b.order); const ids = new Set(executions.map((record) => record.id)); const events: SessionEvent[] = []; for (const event of recent.events.slice().reverse()) { @@ -466,11 +544,14 @@ export class SessionAgent { if (this.backend.capabilities.inspection === false) { throw new Error('This runtime does not support inspection'); } - if (this.current) { - if (params.method === 'workspace') { - result = this.workspace ?? {}; - break; + if (params.method === 'workspace') { + if (!this.current) { + await this.refreshWorkspace(); } + result = this.workspace ?? {}; + break; + } + if (this.current) { throw new Error('R is busy; cached data remains available. Retry when idle.'); } result = await this.backend.inspect({ @@ -648,9 +729,7 @@ export class SessionAgent { this.event(event.type, { method: event.method, params: event.params }, executionId); break; case 'workspaceChanged': - if (!this.current) { - void this.refreshWorkspace(); - } + this.workspaceRefresh.invalidate(25); break; case 'clientRequest': { const id = this.nextRequest++; @@ -699,6 +778,7 @@ export class SessionAgent { break; } this.backendReady = false; + this.workspaceRefresh.invalidate(); if (this.current) { this.uncertain(this.current, event.message); } @@ -809,26 +889,8 @@ export class SessionAgent { } private async refreshWorkspace(): Promise { - if ( - this.stopped || - !this.backendReady || - this.current || - this.manifest.capabilities.inspection === false - ) { - return; - } - try { - const workspace = object( - await this.backend.inspect({ method: 'workspace', params: {}, timeout: 2000 }), - ); - if (this.stopped) { - return; - } - this.workspace = workspace; - this.event('workspace', this.workspace, undefined); - } catch { - /* A busy or disconnected process retains the last workspace snapshot. */ - } + this.workspaceRefresh.invalidate(); + await this.workspaceRefresh.ensure(); } private cancelQueued(): number { @@ -914,6 +976,7 @@ export class SessionAgent { return this.closing; } this.stopped = true; + this.workspaceRefresh.dispose(); this.unsubscribe(); clearTimeout(this.externalTimer); this.assets.close(); diff --git a/src/interactive/cellIndex.ts b/src/interactive/cellIndex.ts new file mode 100644 index 000000000..74348f2f1 --- /dev/null +++ b/src/interactive/cellIndex.ts @@ -0,0 +1,23 @@ +/** Notebook edits invalidate identities; output-only updates leave them intact. */ +export class ExecutionCellIndex { + private cells?: Map; + + invalidate(): void { + this.cells = undefined; + } + + get(id: string, readCells: () => readonly Cell[]): Cell | undefined { + if (!this.cells) { + this.cells = new Map(); + for (const cell of readCells()) { + if (typeof cell.metadata.rExecutionId === 'string') { + // Preserve the first matching cell, as the former find() did. + if (!this.cells.has(cell.metadata.rExecutionId)) { + this.cells.set(cell.metadata.rExecutionId, cell); + } + } + } + } + return this.cells.get(id); + } +} diff --git a/src/interactive/client.ts b/src/interactive/client.ts index 43a78761f..ee5c05930 100644 --- a/src/interactive/client.ts +++ b/src/interactive/client.ts @@ -3,6 +3,7 @@ import { EventEmitter } from 'events'; import { randomUUID } from 'crypto'; import { JsonLines } from './framing'; import { AGENT_PROTOCOL, AgentSnapshot, SessionEvent, SessionManifest } from './protocol'; +import { MessagePage, pageReference } from './messagePages'; export class AgentClient extends EventEmitter { private socket?: net.Socket; @@ -13,6 +14,9 @@ export class AgentClient extends EventEmitter { >(); private heartbeat?: NodeJS.Timeout; private replaying?: SessionEvent[]; + private eventDelivery = Promise.resolve(); + private queuedEventBytes = 0; + private incomingPageBytes = 0; private closing = false; private ready = false; control = false; @@ -29,16 +33,43 @@ export class AgentClient extends EventEmitter { async connect(options: { claim?: boolean; timeout?: number } = {}): Promise { this.close(); + this.eventDelivery = Promise.resolve(); this.closing = false; const socket = net.createConnection(this.manifest.endpoint); this.socket = socket; const parser = new JsonLines((message) => { - if (message.event) { - if (this.replaying) { - this.replaying.push(message.event as SessionEvent); - } else { - this.emitEvent(message.event as SessionEvent); + if (message.event || message.eventPage) { + const queuedBytes = Buffer.byteLength( + JSON.stringify(message.event ?? message.eventPage), + ); + if (this.queuedEventBytes + queuedBytes > 8 * 1024 * 1024) { + socket.destroy(); + return; } + this.queuedEventBytes += queuedBytes; + this.eventDelivery = this.eventDelivery + .then(async () => { + if (this.socket !== socket || socket.destroyed) { + return; + } + const event = message.eventPage + ? await this.readPagedValue(message.eventPage, socket) + : (message.event as SessionEvent); + if (this.socket !== socket || socket.destroyed) { + return; + } + if (this.replaying) { + this.replaying.push(event); + } else { + this.emitEvent(event); + } + }) + .catch(() => { + socket.destroy(); + }) + .finally(() => { + this.queuedEventBytes -= queuedBytes; + }); return; } const id = Number(message.id); @@ -57,7 +88,7 @@ export class AgentClient extends EventEmitter { ), ); } else { - pending.resolve(message.result); + pending.resolve({ value: message.result, page: message.resultPage }); } }); socket.on('data', (chunk: Buffer) => { @@ -106,6 +137,7 @@ export class AgentClient extends EventEmitter { protocol: AGENT_PROTOCOL, token: this.manifest.token, clientId: this.clientId, + paging: true, }, options.timeout, ); @@ -181,6 +213,7 @@ export class AgentClient extends EventEmitter { cursor = event.seq; } if (cursor >= replay.seq) { + await this.eventDelivery; // A socket read can contain both the response and newer events. // Deliver replay first, before the transcript advances its cursor. for (const event of live) { @@ -212,7 +245,7 @@ export class AgentClient extends EventEmitter { if (!socket || socket.destroyed) { return Promise.reject(new Error('Session agent is disconnected')); } - return new Promise((resolve, reject) => { + return new Promise<{ value: unknown; page?: unknown }>((resolve, reject) => { const id = ++this.counter; const timer = setTimeout(() => { this.pending.delete(id); @@ -222,7 +255,11 @@ export class AgentClient extends EventEmitter { ), ); }, timeout); - this.pending.set(id, { resolve: (value) => resolve(value as T), reject, timer }); + this.pending.set(id, { + resolve: (value) => resolve(value as { value: unknown; page?: unknown }), + reject, + timer, + }); socket.write( JSON.stringify({ id, @@ -230,7 +267,48 @@ export class AgentClient extends EventEmitter { params: { generation: this.manifest.generation, ...params }, }) + '\n', ); - }); + }).then(({ value, page }) => (page ? this.readPagedValue(page, socket) : (value as T))); + } + + private async readPagedValue(value: unknown, socket: net.Socket): Promise { + const reference = pageReference(value); + if (!reference) { + return value as T; + } + if (this.incomingPageBytes + reference.bytes > 64 * 1024 * 1024) { + throw new Error('Paged messages exceed memory limit'); + } + this.incomingPageBytes += reference.bytes; + try { + const chunks: Buffer[] = []; + let offset = 0; + while (offset < reference.bytes) { + if (this.socket !== socket || socket.destroyed) { + throw new Error('Session agent disconnected during message read'); + } + const page = await this.request('messagePage', { + token: reference.token, + offset, + }); + if (this.socket !== socket || socket.destroyed) { + throw new Error('Session agent disconnected during message read'); + } + const chunk = Buffer.from(page.data, 'base64'); + if ( + page.offset !== offset || + page.next <= offset || + page.next > reference.bytes || + chunk.length !== page.next - offset + ) { + throw new Error('Invalid message page'); + } + chunks.push(chunk); + offset = page.next; + } + return JSON.parse(Buffer.concat(chunks, reference.bytes).toString('utf8')) as T; + } finally { + this.incomingPageBytes -= reference.bytes; + } } close(): void { diff --git a/src/interactive/journal.ts b/src/interactive/journal.ts index 3424a24b5..fd00d7624 100644 --- a/src/interactive/journal.ts +++ b/src/interactive/journal.ts @@ -2,82 +2,332 @@ import * as fs from 'fs'; import * as path from 'path'; import { createHash } from 'crypto'; import { AgentSnapshot, ExecutionRecord, SessionEvent, Submission, identifier } from './protocol'; +import { + JournalIndex, + getJournalIndex, + getJournalIndexAsync, + indexedAssetIds, + indexEvent, + recentEvents, + recentEventsAsync, + saveJournalIndex, + segmentNames, + repairJournalTail, + replayEvents, + indexSegment, +} from './journalIndex'; -/** Read a completed process without opening a writer or repairing its journal. */ -export function readPreviousJournal( - storage: string, - generation: string, - limit: number, -): Pick { - const directory = path.join(storage, identifier(generation)); - const executions: ExecutionRecord[] = []; - let bytes = 0; - for (const record of readExecutionRecords(directory).slice(-Math.max(1, limit)).reverse()) { - bytes += Buffer.byteLength(JSON.stringify(record)); - if (bytes > 1400 * 1024) { - break; +const maxRecentBytes = 3 * 1024 * 1024; +const maxMemoryBytes = 4 * 1024 * 1024; + +/** Directory timestamps can be coarse; the durable admission set is authoritative. */ +function sameExecutionFiles(entries: [string, unknown][], names: string[]): boolean { + const ids = new Set(entries.map(([id]) => id)); + return ( + entries.length === names.length && + ids.size === names.length && + names.every((name) => ids.has(name.slice(0, -5))) + ); +} + +/** Durable records stay on disk; only a byte-bounded working set retains source code. */ +class ExecutionLedger { + private metadata = new Map(); + private cache = new Map(); + private cacheBytes = 0; + + constructor(private directory: string) { + const folder = path.join(directory, 'executions'); + const names = fs.readdirSync(folder).filter((name) => name.endsWith('.json')); + let entries: [string, { order: number; state: ExecutionRecord['state'] }][] | undefined; + try { + const saved = JSON.parse( + fs.readFileSync(path.join(directory, 'executions-index.json'), 'utf8'), + ) as { data: string; hash: string; mtime: number }; + if ( + saved.mtime === fs.statSync(folder).mtimeMs && + createHash('sha256').update(saved.data).digest('hex') === saved.hash + ) { + entries = JSON.parse(saved.data) as typeof entries; + if ( + !Array.isArray(entries) || + !sameExecutionFiles(entries, names) || + entries.some(([id, data]) => { + identifier(id); + return !Number.isSafeInteger(data.order); + }) + ) { + entries = undefined; + } + } + } catch { + entries = undefined; + } + if (entries) { + this.metadata = new Map(entries); + } else { + for (const name of names) { + const record = JSON.parse( + fs.readFileSync(path.join(folder, name), 'utf8'), + ) as ExecutionRecord; + identifier(record.id); + this.set(record); + } + this.metadata = new Map([...this.metadata].sort((a, b) => a[1].order - b[1].order)); } - executions.unshift(record); } - const ids = new Set(executions.map((record) => record.id)); - const events: SessionEvent[] = []; - let seq = 0; - // Bound restoration as with a live snapshot; full output stays on disk. - for (const name of fs - .readdirSync(directory) - .filter((name) => /^events-\d{6}\.jsonl$/.test(name)) - .sort() - .reverse()) { - const text = fs.readFileSync(path.join(directory, name), 'utf8'); - for (const line of text - .slice(0, text.lastIndexOf('\n') + 1) - .split('\n') - .reverse()) { - if (!line) { - continue; + get size(): number { + return this.metadata.size; + } + get(id: string): ExecutionRecord | undefined { + if (!this.metadata.has(id)) { + return; + } + const cached = this.cache.get(id); + if (cached) { + this.cache.delete(id); + this.cache.set(id, cached); + return cached.record; + } + const record = JSON.parse( + fs.readFileSync( + path.join(this.directory, 'executions', identifier(id) + '.json'), + 'utf8', + ), + ) as ExecutionRecord; + this.remember(record); + return record; + } + set(record: ExecutionRecord): void { + this.metadata.set(record.id, { order: record.order, state: record.state }); + this.remember(record); + } + *values(): Generator { + for (const id of this.metadata.keys()) { + yield this.get(id)!; + } + } + *unfinished(): Generator { + for (const [id, meta] of this.metadata) { + if (meta.state === 'queued' || meta.state === 'running') { + yield this.get(id)!; } - const event = JSON.parse(line) as SessionEvent; - if (event.generation !== generation) { - throw new Error('Journal generation mismatch'); + } + } + recent(limit: number, budget = Number.POSITIVE_INFINITY): ExecutionRecord[] { + const records: ExecutionRecord[] = []; + for (const id of [...this.metadata.keys()].slice(-limit).reverse()) { + const record = this.get(id)!, + bytes = Buffer.byteLength(JSON.stringify(record)); + if (bytes > budget) { + break; } - seq = Math.max(seq, event.seq); - if (!event.executionId || !ids.has(event.executionId)) { + records.unshift(record); + budget -= bytes; + } + return records; + } + history( + query: string, + before: number, + limit = 100, + ): { executions: ExecutionRecord[]; more: boolean } { + const needle = query.toLocaleLowerCase(), + executions: ExecutionRecord[] = []; + let bytes = 0; + for (const id of [...this.metadata.keys()].reverse()) { + if (this.metadata.get(id)!.order >= before) { continue; } - bytes += Buffer.byteLength(line); - if (bytes <= 3 * 1024 * 1024) { - events.push(event); + const record = this.get(id)!; + if (!record.code.toLocaleLowerCase().includes(needle)) { + continue; + } + const size = Buffer.byteLength(JSON.stringify(record)); + if (executions.length >= limit || bytes + size > 2 * 1024 * 1024) { + return { executions, more: true }; } + executions.push(record); + bytes += size; + } + return { executions, more: false }; + } + save(): void { + const data = JSON.stringify([...this.metadata]); + atomicJson(path.join(this.directory, 'executions-index.json'), { + data, + hash: createHash('sha256').update(data).digest('hex'), + mtime: fs.statSync(path.join(this.directory, 'executions')).mtimeMs, + }); + } + private remember(record: ExecutionRecord): void { + const previous = this.cache.get(record.id); + if (previous) { + this.cacheBytes -= previous.bytes; + this.cache.delete(record.id); + } + const bytes = Buffer.byteLength(JSON.stringify(record)); + this.cache.set(record.id, { record, bytes }); + this.cacheBytes += bytes; + while (this.cacheBytes > maxMemoryBytes && this.cache.size) { + const id = this.cache.keys().next().value as string; + this.cacheBytes -= this.cache.get(id)!.bytes; + this.cache.delete(id); } } - events.reverse(); +} + +function recentRecords(directory: string, limit: number): ExecutionRecord[] { + return new ExecutionLedger(directory).recent(Math.max(1, limit), 1400 * 1024); +} +async function recentRecordsAsync(directory: string, limit: number): Promise { + const folder = path.join(directory, 'executions'); + const names = (await fs.promises.readdir(folder)).filter((name) => name.endsWith('.json')); + let metadata: [string, { order: number }][] | undefined; + try { + const saved = JSON.parse( + await fs.promises.readFile(path.join(directory, 'executions-index.json'), 'utf8'), + ) as { data: string; hash: string; mtime: number }; + if ( + saved.mtime === (await fs.promises.stat(folder)).mtimeMs && + createHash('sha256').update(saved.data).digest('hex') === saved.hash + ) { + metadata = JSON.parse(saved.data) as typeof metadata; + if ( + !Array.isArray(metadata) || + !sameExecutionFiles(metadata, names) || + metadata.some(([id, data]) => { + identifier(id); + return !Number.isSafeInteger(data.order); + }) + ) { + metadata = undefined; + } + } + } catch { + metadata = undefined; + } + if (!metadata) { + metadata = []; + for (let start = 0; start < names.length; start += 16) { + const records = await Promise.all( + names + .slice(start, start + 16) + .map( + async (name) => + JSON.parse( + await fs.promises.readFile(path.join(folder, name), 'utf8'), + ) as ExecutionRecord, + ), + ); + for (const record of records) { + identifier(record.id); + metadata.push([record.id, { order: record.order }]); + } + } + metadata.sort((a, b) => a[1].order - b[1].order); + } + const records: ExecutionRecord[] = []; + let budget = 1400 * 1024; + for (const [id] of metadata.slice(-Math.max(1, limit)).reverse()) { + const record = JSON.parse( + await fs.promises.readFile(path.join(folder, identifier(id) + '.json'), 'utf8'), + ) as ExecutionRecord; + const bytes = Buffer.byteLength(JSON.stringify(record)); + if (bytes > budget) { + break; + } + records.unshift(record); + budget -= bytes; + } + return records; +} +function previousSnapshot( + executions: ExecutionRecord[], + events: SessionEvent[], + seq: number, +): Pick { + const admitted = new Set( + events.filter((event) => event.type === 'accepted').map((event) => event.executionId), + ); return { executions, events, seq, truncated: executions - .filter( - (record) => - !events.some( - (event) => event.type === 'accepted' && event.executionId === record.id, - ), - ) + .filter((record) => !admitted.has(record.id)) .map((record) => record.id), }; } +/** Read a completed process without opening a writer or repairing its journal. */ +export function readPreviousJournal( + storage: string, + generation: string, + limit: number, +): Pick { + const directory = path.join(storage, identifier(generation)); + const executions = recentRecords(directory, limit), + index = getJournalIndex(directory, generation); + const budget = + maxRecentBytes - + executions.reduce((total, record) => total + Buffer.byteLength(JSON.stringify(record)), 0); + return previousSnapshot( + executions, + recentEvents(directory, index, new Set(executions.map((record) => record.id)), budget), + index.seq, + ); +} + +/** Cold legacy indexes and event ranges are read in chunks without blocking the editor. */ +export async function readPreviousJournalAsync( + storage: string, + generation: string, + limit: number, +): Promise> { + const directory = path.join(storage, identifier(generation)); + const index = await getJournalIndexAsync(directory, generation); + const executions = await recentRecordsAsync(directory, limit); + const budget = + maxRecentBytes - + executions.reduce((total, record) => total + Buffer.byteLength(JSON.stringify(record)), 0); + return previousSnapshot( + executions, + await recentEventsAsync( + directory, + index, + new Set(executions.map((record) => record.id)), + budget, + ), + index.seq, + ); +} + export function readExecutionRecords(directory: string): ExecutionRecord[] { - return fs - .readdirSync(path.join(directory, 'executions')) - .filter((name) => name.endsWith('.json')) - .map((name) => { - const record = JSON.parse( - fs.readFileSync(path.join(directory, 'executions', name), 'utf8'), - ) as ExecutionRecord; + return [...new ExecutionLedger(directory).values()]; +} + +export async function readExecutionRecordsAsync(directory: string): Promise { + const folder = path.join(directory, 'executions'); + const names = (await fs.promises.readdir(folder)).filter((name) => name.endsWith('.json')); + const records: ExecutionRecord[] = []; + for (let start = 0; start < names.length; start += 16) { + const batch = await Promise.all( + names + .slice(start, start + 16) + .map( + async (name) => + JSON.parse( + await fs.promises.readFile(path.join(folder, name), 'utf8'), + ) as ExecutionRecord, + ), + ); + for (const record of batch) { identifier(record.id); - return record; - }) - .sort((a, b) => a.order - b.order); + records.push(record); + } + } + return records.sort((a, b) => a.order - b.order); } export function atomicJson(file: string, value: unknown): void { @@ -92,36 +342,16 @@ export function atomicJson(file: string, value: unknown): void { fs.renameSync(temporary, file); } -/** Latest display versions in every retained generation; HTML roots protect all dependencies. */ +/** Latest committed display versions; changed/corrupt journals force an index rebuild. */ export function retainedAssetIds(storage: string): Set { const retained = new Set(); for (const generation of fs.readdirSync(storage, { withFileTypes: true })) { if (!generation.isDirectory() || generation.name === 'assets') { continue; } - const directory = path.join(storage, generation.name); - const displays = new Map>(); - for (const name of fs - .readdirSync(directory) - .filter((name) => /^events-\d{6}\.jsonl$/.test(name)) - .sort()) { - const text = fs.readFileSync(path.join(directory, name), 'utf8'); - for (const line of text.slice(0, text.lastIndexOf('\n') + 1).split('\n')) { - if (!line) { - continue; - } - const event = JSON.parse(line) as SessionEvent; - if (event.type === 'display' && typeof event.data.displayId === 'string') { - displays.set(`${event.executionId ?? ''}:${event.data.displayId}`, event.data); - } - } - } - for (const display of displays.values()) { - for (const key of ['svg', 'asset']) { - if (typeof display[key] === 'string') { - retained.add(display[key]); - } - } + const index = getJournalIndex(path.join(storage, generation.name), generation.name, true); + for (const id of indexedAssetIds(index)) { + retained.add(id); } } return retained; @@ -129,7 +359,10 @@ export function retainedAssetIds(storage: string): Set { /** Durable admission ledger plus a bounded, segmented output journal. */ export class SessionJournal { - readonly executions = new Map(); + readonly executions: ExecutionLedger; + private index: JournalIndex; + private memoryBytes = 0; + private eventSizes: number[] = []; private events: SessionEvent[] = []; private fd: number; private size = 0; @@ -145,45 +378,23 @@ export class SessionJournal { ) { fs.mkdirSync(directory, { recursive: true, mode: 0o700 }); fs.mkdirSync(path.join(directory, 'executions'), { recursive: true, mode: 0o700 }); - for (const name of fs.readdirSync(path.join(directory, 'executions'))) { - if (!name.endsWith('.json')) { - continue; - } - const entry = JSON.parse( - fs.readFileSync(path.join(directory, 'executions', name), 'utf8'), - ) as ExecutionRecord; - identifier(entry.id); - this.executions.set(entry.id, entry); - this.nextOrder = Math.max(this.nextOrder, entry.order + 1); - } + this.executions = new ExecutionLedger(directory); + this.nextOrder = (this.executions.recent(1)[0]?.order ?? 0) + 1; const segments = this.segments(); for (const name of segments) { - const file = path.join(directory, name); - const data = fs.readFileSync(file, 'utf8'); - const boundary = data.lastIndexOf('\n') + 1; - if (boundary < data.length) { - // A torn final append has no committed event; remove it before reuse. - fs.truncateSync(file, Buffer.byteLength(data.slice(0, boundary))); - } - for (const line of data.slice(0, boundary).split('\n')) { - if (!line) { - continue; - } - const event = JSON.parse(line) as SessionEvent; - if (event.generation !== generation) { - throw new Error('Journal generation mismatch'); - } - this.seq = Math.max(this.seq, event.seq); - this.events.push(event); - } + repairJournalTail(path.join(directory, name)); } if (segments.length) { this.segment = Number(segments[segments.length - 1].slice(7, -6)); } - this.earliestSeq = this.events[0]?.seq ?? this.seq + 1; + this.index = getJournalIndex(directory, generation); + this.seq = this.index.seq; + this.earliestSeq = + this.index.segments.find((segment) => segment.first)?.first ?? this.seq + 1; const file = this.segmentFile(); this.fd = fs.openSync(file, 'a', 0o600); this.size = fs.fstatSync(this.fd).size; + indexSegment(this.index, path.basename(file), fs.fstatSync(this.fd)); this.trimMemory(); } @@ -212,7 +423,7 @@ export class SessionJournal { path.join(this.directory, 'executions', `${identifier(record.id)}.json`), record, ); - this.executions.set(record.id, record); + this.executions.set(record); } append( @@ -236,7 +447,15 @@ export class SessionJournal { } this.seq = event.seq; this.size += Buffer.byteLength(line); - this.events.push(event); + indexEvent( + this.index, + path.basename(this.segmentFile()), + event, + this.size - Buffer.byteLength(line), + Buffer.byteLength(line), + fs.fstatSync(this.fd), + ); + this.remember(event, Buffer.byteLength(line)); if (this.size >= 4 * 1024 * 1024) { this.rotate(); } @@ -263,73 +482,75 @@ export class SessionJournal { bytes += size; } } else { - let bytes = 0; - for (const name of this.segments()) { - for (const line of fs - .readFileSync(path.join(this.directory, name), 'utf8') - .split('\n')) { - if (!line) { - continue; - } - const event = JSON.parse(line) as SessionEvent; - if (event.seq > after) { - const size = Buffer.byteLength(JSON.stringify(event)); - if (events.length && bytes + size > 3 * 1024 * 1024) { - return { events, reset: false, seq: this.seq }; - } - events.push(event); - bytes += size; - } - if (events.length >= limit) { - return { events, reset: false, seq: this.seq }; - } - } - } + return { + events: replayEvents(this.directory, this.index, after, maxRecentBytes, limit), + reset: false, + seq: this.seq, + }; } return { events, reset: false, seq: this.seq }; } - recent(limit: number): { executions: ExecutionRecord[]; events: SessionEvent[] } { - const executions = [...this.executions.values()] - .sort((a, b) => a.order - b.order) - .slice(-limit); - const ids = new Set(executions.map((record) => record.id)); - const events: SessionEvent[] = []; - for (const name of this.segments()) { - for (const line of fs - .readFileSync(path.join(this.directory, name), 'utf8') - .split('\n')) { - if (!line) { - continue; - } - const event = JSON.parse(line) as SessionEvent; - if (event.executionId && ids.has(event.executionId)) { - events.push(event); - } + recent( + limit: number, + activeId?: string, + ): { executions: ExecutionRecord[]; events: SessionEvent[] } { + const records = this.executions.recent(limit, 1400 * 1024); + const active = activeId && this.executions.get(activeId); + if (active && !records.some((record) => record.id === active.id)) { + records.push(active); + } + records.sort((a, b) => + a.id === activeId ? -1 : b.id === activeId ? 1 : b.order - a.order, + ); + const executions: ExecutionRecord[] = []; + let recordBytes = 0; + for (const record of records) { + const bytes = Buffer.byteLength(JSON.stringify(record)); + if (recordBytes + bytes > 1400 * 1024) { + continue; } + executions.push(record); + recordBytes += bytes; } - return { executions, events }; + executions.sort((a, b) => a.order - b.order); + return { + executions, + events: recentEvents( + this.directory, + this.index, + new Set(executions.map((record) => record.id)), + maxRecentBytes - recordBytes, + ), + }; } close(): void { fs.fsyncSync(this.fd); fs.closeSync(this.fd); + saveJournalIndex(this.directory, this.index); + this.executions.save(); } private segments(): string[] { - return fs - .readdirSync(this.directory) - .filter((name) => /^events-\d{6}\.jsonl$/.test(name)) - .sort(); + return segmentNames(this.directory); } private segmentFile(): string { return path.join(this.directory, `events-${String(this.segment).padStart(6, '0')}.jsonl`); } + private remember(event: SessionEvent, bytes: number): void { + this.events.push(event); + this.eventSizes.push(bytes); + this.memoryBytes += bytes; + this.trimMemory(); + } + private trimMemory(): void { - if (this.events.length > 2000) { - this.events.splice(0, this.events.length - 2000); + while (this.events.length > 2000 || this.memoryBytes > maxMemoryBytes) { + this.events.shift(); + this.memoryBytes -= this.eventSizes.shift()!; } } @@ -338,6 +559,7 @@ export class SessionJournal { fs.closeSync(this.fd); this.segment++; this.fd = fs.openSync(this.segmentFile(), 'a', 0o600); + indexSegment(this.index, path.basename(this.segmentFile()), fs.fstatSync(this.fd)); this.size = 0; const segments = this.segments(); let total = segments.reduce( @@ -349,9 +571,10 @@ export class SessionJournal { total -= fs.statSync(file).size; fs.unlinkSync(file); } - const first = fs - .readFileSync(path.join(this.directory, segments[0]), 'utf8') - .split('\n')[0]; - this.earliestSeq = first ? (JSON.parse(first) as SessionEvent).seq : this.seq + 1; + const retained = new Set(segments); + this.index.segments = this.index.segments.filter((segment) => retained.has(segment.name)); + this.earliestSeq = + this.index.segments.find((segment) => segment.first)?.first ?? this.seq + 1; + saveJournalIndex(this.directory, this.index); } } diff --git a/src/interactive/journalIndex.ts b/src/interactive/journalIndex.ts new file mode 100644 index 000000000..925c84aab --- /dev/null +++ b/src/interactive/journalIndex.ts @@ -0,0 +1,520 @@ +import * as fs from 'fs'; +import * as path from 'path'; +import { createHash } from 'crypto'; +import { SessionEvent } from './protocol'; + +interface SegmentIndex { + name: string; + size: number; + mtime: number; + first: number; + last: number; + ranges: Record; + displays: Record; +} +export interface JournalIndex { + generation: string; + seq: number; + segments: SegmentIndex[]; +} +const caches = new Map(); +const verified = new WeakSet(); +const indexFile = 'events-index.json'; +const chunkBytes = 64 * 1024; + +export function segmentNames(directory: string): string[] { + return fs + .readdirSync(directory) + .filter((name) => /^events-\d{6}\.jsonl$/.test(name)) + .sort(); +} +function emptySegment(name: string, size: number, mtime: number): SegmentIndex { + return { + name, + size, + mtime, + first: 0, + last: 0, + ranges: Object.create(null) as Record, + displays: {}, + }; +} +function include(segment: SegmentIndex, event: SessionEvent, offset: number, bytes: number): void { + segment.first ||= event.seq; + segment.last = event.seq; + if (event.executionId) { + const range = Object.hasOwn(segment.ranges, event.executionId) + ? segment.ranges[event.executionId] + : undefined; + if (range) { + range[1] = offset + bytes; + } else { + segment.ranges[event.executionId] = [offset, offset + bytes]; + } + } + if (event.type === 'display' && typeof event.data.displayId === 'string') { + segment.displays[JSON.stringify([event.executionId ?? '', event.data.displayId])] = { + seq: event.seq, + ids: ['svg', 'asset'].flatMap((key) => + typeof event.data[key] === 'string' ? [event.data[key] as string] : [], + ), + }; + } +} + +/** Read complete lines only. A final uncommitted append is deliberately ignored. */ +function* forwardLines(file: string): Generator<{ text: string; offset: number; bytes: number }> { + const fd = fs.openSync(file, 'r'); + let pending = Buffer.alloc(0), + offset = 0; + try { + const chunk = Buffer.allocUnsafe(chunkBytes); + let size: number; + while ((size = fs.readSync(fd, chunk, 0, chunk.length, null))) { + const data = Buffer.concat([pending, chunk.subarray(0, size)]); + let start = 0, + end: number; + while ((end = data.indexOf(10, start)) >= 0) { + const bytes = end - start + 1; + yield { text: data.toString('utf8', start, end), offset, bytes }; + offset += bytes; + start = end + 1; + } + pending = Buffer.from(data.subarray(start)); + } + } finally { + fs.closeSync(fd); + } +} + +async function* forwardLinesAsync( + file: string, +): AsyncGenerator<{ text: string; offset: number; bytes: number }> { + const handle = await fs.promises.open(file, 'r'); + let pending = Buffer.alloc(0), + offset = 0; + try { + const chunk = Buffer.allocUnsafe(chunkBytes); + let size: number; + while ((size = (await handle.read(chunk, 0, chunk.length, null)).bytesRead)) { + const data = Buffer.concat([pending, chunk.subarray(0, size)]); + let start = 0, + end: number; + while ((end = data.indexOf(10, start)) >= 0) { + const bytes = end - start + 1; + yield { text: data.toString('utf8', start, end), offset, bytes }; + offset += bytes; + start = end + 1; + } + pending = Buffer.from(data.subarray(start)); + } + } finally { + await handle.close(); + } +} + +function decodeIndex(text: string, generation: string): JournalIndex | undefined { + try { + const saved = JSON.parse(text) as { data: string; hash: string }; + if ( + typeof saved.data !== 'string' || + createHash('sha256').update(saved.data).digest('hex') !== saved.hash + ) { + return; + } + const index = JSON.parse(saved.data) as JournalIndex; + if ( + index.generation !== generation || + !Array.isArray(index.segments) || + !Number.isSafeInteger(index.seq) + ) { + return; + } + return index; + } catch { + return; + } +} +function remember(directory: string, index: JournalIndex): JournalIndex { + caches.delete(directory); + caches.set(directory, index); + while (caches.size > 16) { + caches.delete(caches.keys().next().value as string); + } + return index; +} +function sameFiles(index: JournalIndex, names: string[], stats: fs.Stats[]): boolean { + return ( + index.segments.length === names.length && + index.segments.every( + (segment, i) => + segment.name === names[i] && + segment.size === stats[i].size && + segment.mtime === stats[i].mtimeMs, + ) + ); +} +export function getJournalIndex( + directory: string, + generation: string, + verify = false, +): JournalIndex { + const names = segmentNames(directory); + const stats = names.map((name) => fs.statSync(path.join(directory, name))); + let index = caches.get(directory); + if ( + index?.generation === generation && + sameFiles(index, names, stats) && + (!verify || verified.has(index)) + ) { + return index; + } + try { + index = decodeIndex(fs.readFileSync(path.join(directory, indexFile), 'utf8'), generation); + } catch { + index = undefined; + } + if (index && sameFiles(index, names, stats) && !verify) { + return remember(directory, index); + } + index = { generation, seq: 0, segments: [] }; + for (let i = 0; i < names.length; i++) { + const segment = emptySegment(names[i], stats[i].size, stats[i].mtimeMs); + for (const line of forwardLines(path.join(directory, names[i]))) { + if (!line.text) { + continue; + } + const event = JSON.parse(line.text) as SessionEvent; + if (event.generation !== generation) { + throw new Error('Journal generation mismatch'); + } + include(segment, event, line.offset, line.bytes); + index.seq = Math.max(index.seq, event.seq); + } + index.segments.push(segment); + } + verified.add(index); + return remember(directory, index); +} + +export async function getJournalIndexAsync( + directory: string, + generation: string, +): Promise { + const names = (await fs.promises.readdir(directory)) + .filter((name) => /^events-\d{6}\.jsonl$/.test(name)) + .sort(); + const stats = await Promise.all( + names.map((name) => fs.promises.stat(path.join(directory, name))), + ); + let index = caches.get(directory); + if (index?.generation === generation && sameFiles(index, names, stats)) { + return index; + } + try { + index = decodeIndex( + await fs.promises.readFile(path.join(directory, indexFile), 'utf8'), + generation, + ); + } catch { + index = undefined; + } + if (index && sameFiles(index, names, stats)) { + return remember(directory, index); + } + index = { generation, seq: 0, segments: [] }; + for (let i = 0; i < names.length; i++) { + const segment = emptySegment(names[i], stats[i].size, stats[i].mtimeMs); + for await (const line of forwardLinesAsync(path.join(directory, names[i]))) { + if (!line.text) { + continue; + } + const event = JSON.parse(line.text) as SessionEvent; + if (event.generation !== generation) { + throw new Error('Journal generation mismatch'); + } + include(segment, event, line.offset, line.bytes); + index.seq = Math.max(index.seq, event.seq); + } + index.segments.push(segment); + } + verified.add(index); + return remember(directory, index); +} + +export function indexEvent( + index: JournalIndex, + name: string, + event: SessionEvent, + offset: number, + bytes: number, + stat: fs.Stats, +): void { + let segment = index.segments.at(-1); + if (segment?.name !== name) { + segment = emptySegment(name, stat.size, stat.mtimeMs); + index.segments.push(segment); + } + segment.size = stat.size; + segment.mtime = stat.mtimeMs; + include(segment, event, offset, bytes); + index.seq = event.seq; +} +export function saveJournalIndex(directory: string, index: JournalIndex): void { + const data = JSON.stringify(index); + const file = path.join(directory, indexFile), + temporary = file + '.' + process.pid + '.tmp'; + fs.writeFileSync( + temporary, + JSON.stringify({ data, hash: createHash('sha256').update(data).digest('hex') }), + { mode: 0o600 }, + ); + fs.renameSync(temporary, file); + remember(directory, index); +} + +function ranges( + index: JournalIndex, + ids: Set, +): { file: string; start: number; end: number }[] { + const result: { file: string; start: number; end: number }[] = []; + for (const segment of index.segments.slice().reverse()) { + const selected = Object.entries(segment.ranges) + .filter(([id]) => ids.has(id)) + .map(([, range]) => range) + .sort((a, b) => b[1] - a[1]); + // Overlapping execution ranges (interleaved output) are read only once. + for (const [start, end] of selected) { + const previous = result.at(-1); + if (previous?.file === segment.name && end >= previous.start) { + previous.start = Math.min(previous.start, start); + } else { + result.push({ file: segment.name, start, end }); + } + } + } + return result; +} + +function readRange(fd: number, bytes: Buffer, position: number): void { + let offset = 0; + while (offset < bytes.length) { + const size = fs.readSync(fd, bytes, offset, bytes.length - offset, position + offset); + if (!size) { + throw new Error('Journal changed during indexed read'); + } + offset += size; + } +} +async function readRangeAsync( + handle: fs.promises.FileHandle, + bytes: Buffer, + position: number, +): Promise { + let offset = 0; + while (offset < bytes.length) { + const size = (await handle.read(bytes, offset, bytes.length - offset, position + offset)) + .bytesRead; + if (!size) { + throw new Error('Journal changed during indexed read'); + } + offset += size; + } +} +function* reversedLines(fd: number, start: number, end: number): Generator { + let position = end, + pending = Buffer.alloc(0); + while (position > start) { + const bytes = Math.min(chunkBytes, position - start), + chunk = Buffer.allocUnsafe(bytes); + position -= bytes; + readRange(fd, chunk, position); + const data = Buffer.concat([chunk, pending]); + let lineEnd = data.length; + if (data[lineEnd - 1] === 10) { + lineEnd--; + } + let newline: number; + while (lineEnd > 0 && (newline = data.lastIndexOf(10, lineEnd - 1)) >= 0) { + yield data.toString('utf8', newline + 1, lineEnd); + lineEnd = newline; + } + pending = Buffer.from(data.subarray(0, lineEnd)); + } + if (pending.length) { + yield pending.toString('utf8'); + } +} +async function* reversedLinesAsync( + handle: fs.promises.FileHandle, + start: number, + end: number, +): AsyncGenerator { + let position = end, + pending = Buffer.alloc(0); + while (position > start) { + const bytes = Math.min(chunkBytes, position - start), + chunk = Buffer.allocUnsafe(bytes); + position -= bytes; + await readRangeAsync(handle, chunk, position); + const data = Buffer.concat([chunk, pending]); + let lineEnd = data.length; + if (data[lineEnd - 1] === 10) { + lineEnd--; + } + let newline: number; + while (lineEnd > 0 && (newline = data.lastIndexOf(10, lineEnd - 1)) >= 0) { + yield data.toString('utf8', newline + 1, lineEnd); + lineEnd = newline; + } + pending = Buffer.from(data.subarray(0, lineEnd)); + } + if (pending.length) { + yield pending.toString('utf8'); + } +} +function collect( + line: string, + generation: string, + ids: Set, + events: SessionEvent[], + remaining: number, +): number { + if (!line) { + return remaining; + } + const event = JSON.parse(line) as SessionEvent; + if (event.generation !== generation) { + throw new Error('Journal generation mismatch'); + } + if (!event.executionId || !ids.has(event.executionId)) { + return remaining; + } + const bytes = Buffer.byteLength(line) + 1; + if (bytes > remaining) { + return -1; + } + events.push(event); + return remaining - bytes; +} +export function recentEvents( + directory: string, + index: JournalIndex, + ids: Set, + budget: number, +): SessionEvent[] { + const events: SessionEvent[] = []; + for (const range of ranges(index, ids)) { + const fd = fs.openSync(path.join(directory, range.file), 'r'); + try { + for (const line of reversedLines(fd, range.start, range.end)) { + budget = collect(line, index.generation, ids, events, budget); + if (budget < 0) { + return events.sort((a, b) => a.seq - b.seq); + } + } + } finally { + fs.closeSync(fd); + } + } + return events.sort((a, b) => a.seq - b.seq); +} +export async function recentEventsAsync( + directory: string, + index: JournalIndex, + ids: Set, + budget: number, +): Promise { + const events: SessionEvent[] = []; + for (const range of ranges(index, ids)) { + const handle = await fs.promises.open(path.join(directory, range.file), 'r'); + try { + for await (const line of reversedLinesAsync(handle, range.start, range.end)) { + budget = collect(line, index.generation, ids, events, budget); + if (budget < 0) { + return events.sort((a, b) => a.seq - b.seq); + } + } + } finally { + await handle.close(); + } + } + return events.sort((a, b) => a.seq - b.seq); +} +export function indexedAssetIds(index: JournalIndex): Set { + const latest = new Map(); + for (const segment of index.segments) { + for (const [key, display] of Object.entries(segment.displays)) { + latest.set(key, display); + } + } + return new Set([...latest.values()].flatMap((display) => display.ids)); +} + +/** Only a torn tail is repaired; finalized segments need no full-file reread. */ +export function repairJournalTail(file: string): void { + const fd = fs.openSync(file, 'r+'); + try { + let position = fs.fstatSync(fd).size; + if (!position) { + return; + } + const last = Buffer.allocUnsafe(1); + readRange(fd, last, position - 1); + if (last[0] === 10) { + return; + } + while (position > 0) { + const size = Math.min(chunkBytes, position), + bytes = Buffer.allocUnsafe(size); + position -= size; + readRange(fd, bytes, position); + const newline = bytes.lastIndexOf(10); + if (newline >= 0) { + fs.ftruncateSync(fd, position + newline + 1); + return; + } + } + fs.ftruncateSync(fd, 0); + } finally { + fs.closeSync(fd); + } +} +export function replayEvents( + directory: string, + index: JournalIndex, + after: number, + budget: number, + limit: number, +): SessionEvent[] { + const events: SessionEvent[] = []; + for (const segment of index.segments) { + if (segment.last <= after) { + continue; + } + for (const line of forwardLines(path.join(directory, segment.name))) { + if (!line.text) { + continue; + } + const event = JSON.parse(line.text) as SessionEvent; + if (event.generation !== index.generation) { + throw new Error('Journal generation mismatch'); + } + if (event.seq <= after) { + continue; + } + if (events.length && (line.bytes > budget || events.length >= limit)) { + return events; + } + events.push(event); + budget -= line.bytes; + } + } + return events; +} + +export function indexSegment(index: JournalIndex, name: string, stat: fs.Stats): void { + if (index.segments.at(-1)?.name !== name) { + index.segments.push(emptySegment(name, stat.size, stat.mtimeMs)); + } +} diff --git a/src/interactive/manager.ts b/src/interactive/manager.ts index 663b6b252..849dc2ce3 100644 --- a/src/interactive/manager.ts +++ b/src/interactive/manager.ts @@ -15,7 +15,13 @@ import { sessionLabel, } from './protocol'; import { AssetStore, AssetStorageStats, exportedAssetName, readAsset } from './assets'; -import { updateNotebookOutputs } from './notebookOutput'; +import { + NotebookOutputState, + prepareNotebookStream, + updateNotebookOutputs, +} from './notebookOutput'; +import { ExecutionCellIndex } from './cellIndex'; +import type { StreamSnapshot, StreamText } from './streamText'; import { OutputQueue } from './outputQueue'; import { defaultStorage, @@ -47,7 +53,7 @@ import { ensureWorkspaceViewer } from '../extension'; import { queryTablePage } from './tableQuery'; import { tableColumnAlignment, tableDisplayValue, tableSnapshotSummary } from './tableFormatting'; import { HistoryPage, searchHistory } from './history'; -import { readExecutionRecords, readPreviousJournal } from './journal'; +import { readExecutionRecordsAsync, readPreviousJournalAsync } from './journal'; import { sessionAge, sessionPresentation } from './sessionPresentation'; import { runningSessions, stopSession } from './sessionLifecycle'; import { InteractiveInputFeatures } from './inputFeatures'; @@ -67,6 +73,8 @@ interface InteractiveView { >; chain: Promise; outputQueue: OutputQueue; + outputState?: NotebookOutputState; + cellIndex?: ExecutionCellIndex; reconnectTimer?: NodeJS.Timeout; disposed: boolean; prompts: Set; @@ -358,6 +366,18 @@ export class InteractiveManager this.disposeView(view); } }), + vscode.workspace.onDidChangeNotebookDocument((event) => { + if ( + event.contentChanges.length || + event.cellChanges.some((change) => change.metadata !== undefined) + ) { + for (const view of this.views.values()) { + if (view.notebook === event.notebook) { + view.cellIndex?.invalidate(); + } + } + } + }), vscode.workspace.onDidChangeConfiguration((event) => { if (event.affectsConfiguration('r.interactive.tableView')) { for (const view of this.views.values()) { @@ -831,18 +851,22 @@ export class InteractiveManager const before = more ? picker.items.at(-1)?.record.order : undefined; // Orders restart at one for each R process. Use a transcript-wide order for // history pagination after restart, including admission records outside replay. - const records = (): ExecutionRecord[] => + const records = async (): Promise => view.history.length - ? [...view.history, view.model] - .flatMap((model) => - readExecutionRecords( - path.join( - this.root, - view.client.manifest.id, - model.generation, + ? ( + await Promise.all( + [...view.history, view.model].map((model) => + readExecutionRecordsAsync( + path.join( + this.root, + view.client.manifest.id, + model.generation, + ), ), ), ) + ) + .flat() .map((record, index) => ({ ...record, order: index + 1 })) : this.transcriptCells(view).map((cell) => cell.record); const page = @@ -853,7 +877,7 @@ export class InteractiveManager query: picker.value, before, }) - : searchHistory(records(), picker.value, before); + : searchHistory(await records(), picker.value, before); if (closed || request !== version) { return; } @@ -1497,6 +1521,10 @@ export class InteractiveManager if (this.closed) { throw new Error('Interactive manager was closed during connection'); } + const history = await this.readHistory(manifest); + if (this.closed || notebook.isClosed) { + throw new Error('Interactive window closed while reading its history'); + } const target = this.registerTarget(client); session.bindSessionDocument( vscode.Uri.from({ scheme: 'vscode-notebook-cell', path: notebook.uri.path }), @@ -1506,7 +1534,7 @@ export class InteractiveManager client, target, model: new Transcript(manifest.generation), - history: this.readHistory(manifest), + history, notebook, controller, controllers: [native, fallback], @@ -1647,23 +1675,29 @@ export class InteractiveManager }); } - private readHistory(manifest: SessionManifest): Transcript[] { + private async readHistory(manifest: SessionManifest): Promise { const storage = path.join(this.root, manifest.id); const file = path.join(storage, 'config.json'); if (!fs.existsSync(file)) { return []; } - const config = JSON.parse(fs.readFileSync(file, 'utf8')) as AgentConfig; - return (config.previousGenerations ?? []) - .filter((generation) => generation !== manifest.generation) - .map((generation) => { - const model = new Transcript(generation); - model.restore({ - manifest: { ...manifest, generation }, - ...readPreviousJournal(storage, generation, config.historyLimit), - }); - return model; - }); + const config = JSON.parse(await fs.promises.readFile(file, 'utf8')) as AgentConfig; + return Promise.all( + (config.previousGenerations ?? []) + .filter((generation) => generation !== manifest.generation) + .map(async (generation) => { + const model = new Transcript(generation); + model.restore({ + manifest: { ...manifest, generation }, + ...(await readPreviousJournalAsync( + storage, + generation, + config.historyLimit, + )), + }); + return model; + }), + ); } private transcriptCells(view: InteractiveView): TranscriptCell[] { @@ -1695,6 +1729,8 @@ export class InteractiveManager task.end(undefined); } view.executions.clear(); + view.outputState?.reset(); + view.cellIndex?.invalidate(); for (const cell of view.notebook.getCells()) { session.unbindSessionDocument(cell.document.uri); } @@ -1830,7 +1866,15 @@ export class InteractiveManager } private cell(view: InteractiveView, id: string): vscode.NotebookCell | undefined { - return view.notebook.getCells().find((cell) => cell.metadata.rExecutionId === id); + view.cellIndex ??= new ExecutionCellIndex(); + return view.cellIndex.get(id, () => view.notebook.getCells()); + } + + private transcriptCell(view: InteractiveView, id: string): TranscriptCell | undefined { + return ( + view.model.cells.get(id) ?? + view.history.map((model) => model.cells.get(id)).find((cell) => cell !== undefined) + ); } private async submit( @@ -2127,6 +2171,7 @@ export class InteractiveManager view: InteractiveView, cell: TranscriptCell, portable = false, + streamSnapshots?: ReadonlyMap, ): Promise { const outputs: vscode.NotebookCellOutput[] = []; const plots: { output: vscode.NotebookCellOutput; data: Record }[] = []; @@ -2137,8 +2182,9 @@ export class InteractiveManager view.client.connected, !['exited', 'stopping'].includes(view.client.manifest.status), ]); - for (const output of cell.outputs) { + for (const [index, output] of cell.outputs.entries()) { const data = output.data; + const stream = output.stream && streamSnapshots?.get(output.stream); const cachedPlot = !portable && this.renderedPlots.get(data); if (cachedPlot && cachedPlot.context === plotContext) { plots.push(cachedPlot); @@ -2149,8 +2195,8 @@ export class InteractiveManager if (output.type === 'stream') { items = [ data.channel === 'stderr' - ? vscode.NotebookCellOutputItem.stderr(String(data.text)) - : vscode.NotebookCellOutputItem.stdout(String(data.text)), + ? vscode.NotebookCellOutputItem.stderr(stream ? '' : String(data.text)) + : vscode.NotebookCellOutputItem.stdout(stream ? '' : String(data.text)), ]; } else if (output.type === 'condition') { if (data.kind === 'error') { @@ -2272,7 +2318,16 @@ export class InteractiveManager continue; } } - outputs.push(new vscode.NotebookCellOutput(items, { rDisplayId: data.displayId })); + const notebookOutput = new vscode.NotebookCellOutput(items, { + rDisplayId: data.displayId, + ...(output.type === 'stream' + ? { rStreamId: `${cell.generation}:${cell.record.id}:${index}` } + : {}), + }); + if (stream) { + prepareNotebookStream(notebookOutput, stream); + } + outputs.push(notebookOutput); } if (plots.length > 1) { const first = plots[0]; @@ -2352,7 +2407,7 @@ export class InteractiveManager if (view.disposed || view.notebook.isClosed) { return; } - const data = this.transcriptCells(view).find((cell) => cell.record.id === id); + const data = this.transcriptCell(view, id); if (!data || !this.cell(view, id)) { return; } @@ -2366,14 +2421,29 @@ export class InteractiveManager } // Rendering may await asset reads while the transcript receives newer // events. Snapshot mutable streams and defer obsolete output writes. + const streamSnapshots = new Map( + data.outputs.flatMap((output) => + output.stream ? [[output.stream, output.stream.snapshot()] as const] : [], + ), + ); const snapshot = { ...data, - outputs: data.outputs.map((output) => ({ - ...output, - data: output.type === 'stream' ? { ...output.data } : output.data, - })), + outputs: data.outputs.map((output) => { + const stream = output.stream && streamSnapshots.get(output.stream); + return { + ...output, + data: stream + ? { + channel: output.data.channel, + get text() { + return stream.text(); + }, + } + : output.data, + }; + }), }; - const outputs = await this.outputs(view, snapshot); + const outputs = await this.outputs(view, snapshot, false, streamSnapshots); // Asset reads can outlive detach or reconnection. Re-read the live handle. if (view.disposed || view.notebook.isClosed || view.outputQueue.has(id)) { return; @@ -2383,6 +2453,7 @@ export class InteractiveManager return; } const execution = view.executions.get(id); + view.outputState ??= new NotebookOutputState(); if (execution) { // A dispatch failure may have diagnostics but no R start event. if (!execution.started) { @@ -2390,7 +2461,13 @@ export class InteractiveManager execution.task.start(execution.startTime); execution.started = true; } - await updateNotebookOutputs(execution.task, cell.outputs, outputs); + await updateNotebookOutputs( + execution.task, + cell.outputs, + outputs, + view.outputState, + () => cell.outputs, + ); } else { // Updating a late plot must preserve the cell URI, edited code, selection, and diagnostics. const summary = cell.executionSummary; @@ -2398,7 +2475,13 @@ export class InteractiveManager update.executionOrder = summary?.executionOrder; update.start(summary?.timing?.startTime); try { - await updateNotebookOutputs(update, cell.outputs, outputs); + await updateNotebookOutputs( + update, + cell.outputs, + outputs, + view.outputState, + () => cell.outputs, + ); } finally { update.end(summary?.success, summary?.timing?.endTime); } diff --git a/src/interactive/messagePages.ts b/src/interactive/messagePages.ts new file mode 100644 index 000000000..7bb6bebed --- /dev/null +++ b/src/interactive/messagePages.ts @@ -0,0 +1,98 @@ +import { randomUUID } from 'crypto'; + +export interface MessagePageReference { + __rInteractivePage: { token: string; bytes: number }; +} +export interface MessagePage { + offset: number; + next: number; + data: string; +} +export function pageReference( + value: unknown, +): MessagePageReference['__rInteractivePage'] | undefined { + if (!value || typeof value !== 'object') { + return; + } + const page = (value as Partial).__rInteractivePage; + return page && + typeof page.token === 'string' && + Number.isSafeInteger(page.bytes) && + page.bytes >= 0 + ? page + : undefined; +} + +/** Stop serialization before an unexpected payload can create an unbounded scratch buffer. */ +export function boundedJson(value: unknown, limit = 64 * 1024 * 1024): string { + let size = 0; + return JSON.stringify(value, (key: string, item: unknown) => { + size += key ? Buffer.byteLength(JSON.stringify(key)) + 1 : 0; + size += 2; + if (typeof item === 'string') { + if (Buffer.byteLength(item) > limit) { + throw new Error('Message exceeds the 64 MiB paging limit'); + } + size += Buffer.byteLength(JSON.stringify(item)); + } else if (typeof item === 'number' || typeof item === 'boolean' || item === null) { + size += String(item).length; + } + if (size > limit) { + throw new Error('Message exceeds the 64 MiB paging limit'); + } + return item; + }); +} + +/** Pull pages rather than flooding a socket with a large snapshot or workspace event. */ +export class MessagePages { + private pages = new Map(); + private total = 0; + retain(value: unknown): MessagePageReference { + this.expire(); + const bytes = Buffer.from(boundedJson(value)); + if (this.pages.size >= 4 || this.total + bytes.length > 64 * 1024 * 1024) { + throw new Error( + 'Too many large messages are pending; retry after the current read finishes', + ); + } + const token = randomUUID(); + this.pages.set(token, { bytes, expires: Date.now() + 30000 }); + this.total += bytes.length; + return { __rInteractivePage: { token, bytes: bytes.length } }; + } + read(token: string, offset: number): MessagePage { + this.expire(); + const stored = this.pages.get(token); + if ( + !stored || + !Number.isSafeInteger(offset) || + offset < 0 || + offset >= stored.bytes.length + ) { + throw new Error( + 'Message page expired or has an invalid cursor; request a fresh snapshot', + ); + } + stored.expires = Date.now() + 30000; + const next = Math.min(offset + 512 * 1024, stored.bytes.length); + const result = { + offset, + next, + data: stored.bytes.subarray(offset, next).toString('base64'), + }; + if (next === stored.bytes.length) { + this.pages.delete(token); + this.total -= stored.bytes.length; + } + return result; + } + expire(): void { + for (const [token, stored] of this.pages) { + if (stored.expires <= Date.now()) { + this.pages.delete(token); + this.total -= stored.bytes.length; + } + } + } +} diff --git a/src/interactive/notebookOutput.ts b/src/interactive/notebookOutput.ts index 02219e8d1..f7e508fe6 100644 --- a/src/interactive/notebookOutput.ts +++ b/src/interactive/notebookOutput.ts @@ -1,10 +1,59 @@ import type { NotebookCellExecution, NotebookCellOutput } from 'vscode'; +import type { StreamSnapshot, StreamText } from './streamText'; + +const streams = new WeakMap(); + +/** Text is encoded only after a render is confirmed current and ready to write. */ +export function prepareNotebookStream(output: NotebookCellOutput, snapshot: StreamSnapshot): void { + streams.set(output, snapshot); +} + +export class NotebookOutputState { + private delivered = new WeakMap< + StreamText, + { length: number; output: NotebookCellOutput; data: Uint8Array } + >(); + + get( + snapshot: StreamSnapshot, + ): { length: number; output: NotebookCellOutput; data: Uint8Array } | undefined { + return this.delivered.get(snapshot.source); + } + + set(snapshot: StreamSnapshot, output: NotebookCellOutput): void { + this.delivered.set(snapshot.source, { + length: snapshot.length, + output, + data: output.items[0].data, + }); + } + + reset(): void { + this.delivered = new WeakMap(); + } +} + +function materialize(output: NotebookCellOutput): number | undefined { + const snapshot = streams.get(output); + if (!snapshot) { + return; + } + const bytes = Buffer.from(snapshot.text()); + output.items = [{ mime: output.items[0].mime, data: bytes }]; + return bytes.length; +} /** Keep existing output identities while an execution appends or updates results. */ export async function updateNotebookOutputs( - execution: Pick, + execution: Pick< + NotebookCellExecution, + 'replaceOutput' | 'replaceOutputItems' | 'appendOutput' + > & + Partial>, previous: readonly NotebookCellOutput[], next: readonly NotebookCellOutput[], + state = new NotebookOutputState(), + readOutputs?: () => readonly NotebookCellOutput[], ): Promise { const compatible = previous.length <= next.length && @@ -16,12 +65,42 @@ export async function updateNotebookOutputs( next[index].items.map((item) => item.mime).join('\n')), ); if (!compatible) { + next.forEach(materialize); await execution.replaceOutput(next); + next.forEach((output, index) => { + const snapshot = streams.get(output); + if (snapshot) { + state.set(snapshot, readOutputs?.()[index] ?? output); + } + }); return; } for (let index = 0; index < previous.length; index++) { const output = previous[index], - items = next[index].items; + snapshot = streams.get(next[index]), + delivered = snapshot && state.get(snapshot); + if ( + snapshot && + delivered && + snapshot.length >= delivered.length && + delivered.output === output && + output.items.length === 1 && + // Appends normalize CR/backspace; use the acknowledged model buffer. + output.items[0].data === delivered.data && + execution.appendOutputItems + ) { + const bytes = Buffer.from(snapshot.text(delivered.length)); + if (bytes.length) { + await execution.appendOutputItems( + { mime: output.items[0].mime, data: bytes }, + output, + ); + } + state.set(snapshot, readOutputs?.()[index] ?? output); + continue; + } + materialize(next[index]); + const items = next[index].items; if ( output.items.length !== items.length || output.items.some( @@ -32,8 +111,19 @@ export async function updateNotebookOutputs( ) { await execution.replaceOutputItems(items, output); } + if (snapshot) { + state.set(snapshot, readOutputs?.()[index] ?? output); + } } if (next.length > previous.length) { - await execution.appendOutput(next.slice(previous.length)); + const added = next.slice(previous.length); + added.forEach(materialize); + await execution.appendOutput(added); + added.forEach((output, index) => { + const snapshot = streams.get(output); + if (snapshot) { + state.set(snapshot, readOutputs?.()[previous.length + index] ?? output); + } + }); } } diff --git a/src/interactive/streamText.ts b/src/interactive/streamText.ts new file mode 100644 index 000000000..a76866c40 --- /dev/null +++ b/src/interactive/streamText.ts @@ -0,0 +1,61 @@ +/** A prefix stays readable while more console text arrives. */ +export interface StreamSnapshot { + readonly source: StreamText; + readonly length: number; + text(start?: number): string; +} + +/** Keep console chunks until delivery instead of flattening every growing prefix. */ +export class StreamText { + private chunks: string[] = []; + private length = 0; + + constructor(text: string) { + this.append(text); + } + + append(text: string): void { + if (!text) { + return; + } + const last = this.chunks.length - 1; + // Bound small-write overhead. Replacing a chunk only extends its prefix, + // so existing snapshots remain valid through their captured length. + if (last >= 0 && this.chunks[last].length + text.length <= 32768) { + this.chunks[last] += text; + } else { + this.chunks.push(text); + } + this.length += text.length; + } + + snapshot(): StreamSnapshot { + const chunks = this.chunks; + const length = this.length; + return { + source: this, + length, + text(start = 0): string { + const parts: string[] = []; + let offset = 0; + for (const chunk of chunks) { + const end = Math.min(offset + chunk.length, length); + if (end > start) { + parts.push(chunk.slice(Math.max(0, start - offset), end - offset)); + } + offset += chunk.length; + if (offset >= length) { + break; + } + } + return parts.join(''); + }, + }; + } + + seal(): void { + if (this.chunks.length > 1) { + this.chunks = [this.snapshot().text()]; + } + } +} diff --git a/src/interactive/transcript.ts b/src/interactive/transcript.ts index cc2dff5f1..82add4aab 100644 --- a/src/interactive/transcript.ts +++ b/src/interactive/transcript.ts @@ -1,8 +1,10 @@ import { AgentSnapshot, ExecutionRecord, SessionEvent } from './protocol'; +import { StreamText } from './streamText'; export interface TranscriptOutput { type: 'stream' | 'condition' | 'display' | 'truncated'; data: Record; + stream?: StreamText; } export interface TranscriptCell { generation: string; @@ -60,14 +62,25 @@ export class Transcript { cell.record = { ...cell.record, state: 'running', started: event.time }; } else if (event.type === 'finished' || event.type === 'uncertain') { cell.record = event.data.record as ExecutionRecord; + for (const output of cell.outputs) { + output.stream?.seal(); + } } else if (event.type === 'stream') { const previous = cell.outputs.at(-1); if (previous?.type === 'stream' && previous.data.channel === event.data.channel) { - previous.data.text = - (typeof previous.data.text === 'string' ? previous.data.text : '') + - (typeof event.data.text === 'string' ? event.data.text : ''); + previous.stream!.append(typeof event.data.text === 'string' ? event.data.text : ''); } else { - cell.outputs.push({ type: 'stream', data: { ...event.data } }); + const stream = new StreamText( + typeof event.data.text === 'string' ? event.data.text : '', + ); + const data = { ...event.data }; + // Retain the public replay/export data shape, materializing text + // only when a caller explicitly reads or serializes it. + Object.defineProperty(data, 'text', { + enumerable: true, + get: () => stream.snapshot().text(), + }); + cell.outputs.push({ type: 'stream', data, stream }); } } else if (event.type === 'display') { const previous = cell.outputs.find( diff --git a/src/nativeRpc.ts b/src/nativeRpc.ts new file mode 100644 index 000000000..7e8ba4f5a --- /dev/null +++ b/src/nativeRpc.ts @@ -0,0 +1,181 @@ +import type { Socket } from 'node:net'; + +export interface RpcRequestOptions { + signal?: AbortSignal; + timeout?: number; +} + +interface PendingRequest { + resolve(value: unknown): void; + reject(reason: unknown): void; + timer: NodeJS.Timeout; + removeAbortListener?: () => void; +} + +function cancelledRequest(): Error { + const error = new Error('Request cancelled'); + error.name = 'AbortError'; + return error; +} + +/** Cancel an editor's wait without treating cancellation as an R interruption. */ +export function withRpcCancellation( + operation: () => Promise, + signal?: AbortSignal, +): Promise { + if (!signal) { + return operation(); + } + if (signal.aborted) { + return Promise.reject(cancelledRequest()); + } + return new Promise((resolve, reject) => { + const abort = () => { + signal.removeEventListener('abort', abort); + reject(cancelledRequest()); + }; + signal.addEventListener('abort', abort, { once: true }); + Promise.resolve() + .then(() => { + if (signal.aborted) { + throw cancelledRequest(); + } + return operation(); + }) + .then(resolve, reject) + .finally(() => signal.removeEventListener('abort', abort)); + }); +} + +/** Native sess request state belongs to one socket incarnation. */ +export class NativeRpcConnection { + private sequence = 0; + private pending = new Map(); + private closed = false; + + constructor(private readonly socket: Pick) {} + + request(data: Record, options: RpcRequestOptions = {}): Promise { + if (this.closed || this.socket.destroyed) { + return Promise.reject(new Error('IPC socket is not connected')); + } + if (options.signal?.aborted) { + return Promise.reject(cancelledRequest()); + } + if (this.sequence >= Number.MAX_SAFE_INTEGER) { + return Promise.reject(new Error('IPC request IDs exhausted; reconnect the session')); + } + const id = ++this.sequence; + return new Promise((resolve, reject) => { + const finish = (error?: unknown, value?: unknown): void => { + if (this.pending.get(id) !== pending) { + return; + } + this.pending.delete(id); + clearTimeout(pending.timer); + pending.removeAbortListener?.(); + if (error !== undefined) { + reject(error); + } else { + resolve(value); + } + }; + const pending: PendingRequest = { + resolve: (value) => finish(undefined, value), + reject: (error) => finish(error), + timer: setTimeout( + () => finish(new Error('Request timed out')), + options.timeout ?? 5000, + ), + }; + this.pending.set(id, pending); + const signal = options.signal; + if (signal) { + const abort = () => finish(cancelledRequest()); + signal.addEventListener('abort', abort, { once: true }); + pending.removeAbortListener = () => signal.removeEventListener('abort', abort); + } + try { + // Transport IDs always belong to this connection, even when a + // caller supplies a complete JSON-RPC object with its own ID. + this.socket.write(JSON.stringify({ jsonrpc: '2.0', ...data, id }) + '\n'); + } catch (error) { + finish(error); + } + }); + } + + response(message: Record): void { + const pending = this.pending.get(Number(message.id)); + if (!pending) { + return; + } + if (message.error) { + pending.reject(message.error); + } else { + pending.resolve(message.result); + } + } + + close(): void { + if (this.closed) { + return; + } + this.closed = true; + for (const pending of this.pending.values()) { + pending.reject(new Error('IPC socket disconnected')); + } + } +} + +// Native responses can contain full workspace metadata and rendered plot data. +// Keep the ceiling well above Interactive's paged 4 MiB protocol frames. +export const MAX_NATIVE_RPC_FRAME_BYTES = 64 * 1024 * 1024; + +/** Buffer split UTF-8 frames once, with a ceiling before concatenation/decoding. */ +export class NativeRpcFrames { + private chunks: Buffer[] = []; + private length = 0; + + constructor( + private readonly receive: (frame: Buffer) => void, + private readonly limit = MAX_NATIVE_RPC_FRAME_BYTES, + ) {} + + push(data: Buffer): void { + let start = 0; + while (start < data.length) { + const newline = data.indexOf(0x0a, start); + const end = newline < 0 ? data.length : newline; + const incoming = data.subarray(start, end); + if (this.length + incoming.length > this.limit) { + this.clear(); + throw new Error('Native IPC frame exceeds size limit'); + } + if (newline < 0) { + if (incoming.length) { + this.chunks.push(incoming); + this.length += incoming.length; + } + return; + } + let frame = incoming; + if (this.chunks.length) { + if (incoming.length) { + this.chunks.push(incoming); + } + frame = Buffer.concat(this.chunks, this.length + incoming.length); + this.clear(); + } + if (frame.length) { + this.receive(frame); + } + start = end + 1; + } + } + + clear(): void { + this.chunks = []; + this.length = 0; + } +} diff --git a/src/rTerminal.ts b/src/rTerminal.ts index 3d4cd02e7..c1b692a80 100644 --- a/src/rTerminal.ts +++ b/src/rTerminal.ts @@ -676,7 +676,7 @@ async function sendToTerminal( validate?: () => void, ): Promise { validate?.(); - deferWorkspaceRefresh(); + deferWorkspaceRefresh(term); if (config().get('bracketedPaste')) { // Surround with ANSI control characters for bracketed paste mode text = `\x1b[200~${text}\x1b[201~`; diff --git a/src/session.ts b/src/session.ts index 0305f652c..53b23e387 100644 --- a/src/session.ts +++ b/src/session.ts @@ -18,6 +18,13 @@ import { TerminalSessionRegistry } from './terminalSessionRegistry'; import { SessionProcessMonitor } from './sessionProcessMonitor'; import { sessionProcessIdentity } from './sessionIdentity'; import { NativeTerminalPidWaitRegistry } from './nativeTerminalPidWait'; +import { WorkspaceRefreshScheduler } from './workspaceRefresh'; +import { + NativeRpcConnection, + NativeRpcFrames, + RpcRequestOptions, + withRpcCancellation, +} from './nativeRpc'; import { purgeAddinPickerItems, RSEditOperation, RSRange } from './rstudioapi'; import { @@ -72,7 +79,7 @@ export interface GlobalEnv { [key: string]: { class: string[] | string; type: string; - length: number; + length?: number; str: string; dim?: number[]; names?: string[]; @@ -85,6 +92,7 @@ export interface WorkspaceData { search: string[]; loaded_namespaces: string[]; globalenv: GlobalEnv; + revision?: number; } // Thin adapter to track per-socket metadata alongside net.Socket @@ -255,7 +263,11 @@ const sessionDocumentBound = new vscode.EventEmitter(); export const onDidBindSessionDocument = sessionDocumentBound.event; export function sessionForDocument(uri: Uri): Session | undefined { - return boundSessionForDocument(uri) ?? activeSession; + const target = boundSessionForDocument(uri) ?? activeSession; + if (target) { + void ensureSessionWorkspace(target); + } + return target; } export function boundSessionForDocument(uri: Uri): Session | undefined { @@ -270,6 +282,7 @@ export function boundSessionForDocument(uri: Uri): Session | undefined { export function bindSessionDocument(uri: Uri, session: Session): void { const previous = boundSessionForDocument(uri); documentSessions.set(uri.toString(), session); + void ensureSessionWorkspace(session); if (previous !== session) { sessionDocumentBound.fire(uri); } @@ -280,6 +293,7 @@ export function unbindSessionDocument(uri: Uri): void { } export function unregisterSessionTransport(target: Session): void { + workspaceRefreshes.get(target)?.dispose(); for (const [uri, owner] of documentSessions) { if (owner === target) { documentSessions.delete(uri); @@ -302,7 +316,6 @@ export function unregisterSessionTransport(target: Session): void { function clearActiveSession(): Promise { sessionSelectionRevision++; deferWorkspaceRefresh(); - workspaceRefreshPending = false; activeSession = undefined; workspaceData = { search: [], loaded_namespaces: [], globalenv: {} }; workingDir = ''; @@ -366,6 +379,7 @@ export function updateSessionWorkspace(target: Session, data: WorkspaceData): vo /** Move document routing to a new process; existing data viewers keep their old owner. */ export function replaceSessionTransport(previous: Session, next: Session): void { + workspaceRefreshes.get(previous)?.dispose(); for (const [uri, owner] of documentSessions) { if (owner === previous) { documentSessions.set(uri, next); @@ -669,9 +683,7 @@ export let activeSession: Session | undefined; // Older terminal lookups must not overwrite a newer selection or activation. let sessionSelectionRevision = 0; let activeBrowserUri: Uri | undefined; -let workspaceRefreshTimer: NodeJS.Timeout | undefined; -let workspaceRefreshInProgress = false; -let workspaceRefreshPending = false; +const workspaceRefreshes = new WeakMap>(); interface DataViewColumnDef { headerName: string; @@ -903,14 +915,7 @@ function isLiveSessionSocket(socket: IpcSocket): boolean { ); } -const pendingRequests = new Map< - number, - { - resolve: (value: unknown) => void; - reject: (reason?: unknown) => void; - socket: IpcSocket; - } ->(); +const nativeRpcConnections = new WeakMap(); const terminalDiscoveryOperations = new WeakMap>(); @@ -1356,8 +1361,8 @@ function startGlobalSessionServer(generation: number): Promise { console.info('[SessionServer] Client connected via IPC pipe'); activeConnections.add(socket); - let readBuffers: Buffer[] = []; - let readBufferLength = 0; + const rpc = new NativeRpcConnection(socket); + nativeRpcConnections.set(socket, rpc); const handleLine = (buf: Buffer) => { let lineEnd = buf.length; @@ -1376,17 +1381,7 @@ function startGlobalSessionServer(generation: number): Promise { return; } if (message.id !== undefined && !message.method) { - // Response to a request we sent - const id = Number(message.id); - const pending = pendingRequests.get(id); - if (pending?.socket === socket) { - pendingRequests.delete(id); - if (message.error) { - pending.reject(message.error); - } else { - pending.resolve(message.result); - } - } + rpc.response(message); } else if (message.id === undefined || message.id === null) { await handleNotification(message, socket); } else { @@ -1398,45 +1393,21 @@ function startGlobalSessionServer(generation: number): Promise { })(); }; + const frames = new NativeRpcFrames(handleLine); socket.on('data', (data: Buffer) => { - let start = 0; - let newline = data.indexOf(0x0a, start); - - while (newline !== -1) { - const incoming = data.subarray(start, newline); - let buf: Buffer; - - if (readBuffers.length === 0) { - buf = incoming; - } else { - if (incoming.length > 0) { - readBuffers.push(incoming); - readBufferLength += incoming.length; - } - buf = - readBuffers.length === 1 - ? readBuffers[0] - : Buffer.concat(readBuffers, readBufferLength); - readBuffers = []; - readBufferLength = 0; - } - - handleLine(buf); - start = newline + 1; - newline = data.indexOf(0x0a, start); - } - - if (start < data.length) { - const incoming = data.subarray(start); - readBuffers.push(incoming); - readBufferLength += incoming.length; + try { + frames.push(data); + } catch (error) { + console.error('[SessionServer] Invalid IPC frame', error); + socket.destroy(); } }); socket.on('close', () => { console.info('[SessionServer] Client disconnected'); - readBuffers = []; - readBufferLength = 0; + frames.clear(); + rpc.close(); + nativeRpcConnections.delete(socket); activeConnections.delete(socket); for (const [id, pending] of pendingSessionAttachments) { if (pending.socket === socket) { @@ -1444,12 +1415,6 @@ function startGlobalSessionServer(generation: number): Promise { pending.controller.abort(); } } - for (const [id, pending] of pendingRequests.entries()) { - if (pending.socket === socket) { - pendingRequests.delete(id); - pending.reject(new Error('IPC socket disconnected')); - } - } if (pipeClient === socket) { pipeClient = undefined; } @@ -2301,57 +2266,59 @@ async function updatePlot() { await globalPlotManager?.showStandardPlot(); } -export function deferWorkspaceRefresh(): void { - if (workspaceRefreshTimer) { - clearTimeout(workspaceRefreshTimer); - workspaceRefreshTimer = undefined; +export function deferWorkspaceRefresh(terminal?: vscode.Terminal): void { + const target = terminal ? terminalRegistry.ownerOf(terminal) : activeSession; + if (target) { + workspaceRefreshes.get(target)?.defer(); } } -function scheduleWorkspaceRefresh(delayMs: number = 500): void { - workspaceRefreshPending = true; - if (workspaceRefreshTimer) { - clearTimeout(workspaceRefreshTimer); +function workspaceRefreshFor(target: Session): WorkspaceRefreshScheduler { + let refresh = workspaceRefreshes.get(target); + if (!refresh) { + refresh = new WorkspaceRefreshScheduler( + async () => { + const response = await sessionRequest( + { method: 'workspace', params: { revision: target.workspaceData.revision } }, + target, + ); + if (!response || (response as { unchanged?: boolean }).unchanged) { + return undefined; + } + return response as WorkspaceData; + }, + (data) => updateSessionWorkspace(target, data), + () => + sessions.get(target.sessionId) === target && + !target.workspaceUnavailable && + isConnectedSession(target), + (error) => console.error('[updateWorkspace]', error), + ); + workspaceRefreshes.set(target, refresh); } - workspaceRefreshTimer = setTimeout(() => { - workspaceRefreshTimer = undefined; - void runWorkspaceRefresh(); - }, delayMs); + return refresh; } -async function runWorkspaceRefresh(): Promise { - if (workspaceRefreshInProgress || !workspaceRefreshPending) { - return; - } - workspaceRefreshPending = false; - workspaceRefreshInProgress = true; - try { - await updateWorkspace(); - } finally { - workspaceRefreshInProgress = false; - if (workspaceRefreshPending) { - scheduleWorkspaceRefresh(); - } +function scheduleWorkspaceRefresh(delayMs: number = 500, target = activeSession): void { + if (target) { + workspaceRefreshFor(target).invalidate(delayMs); } } -export async function updateWorkspace() { - const requestedSession = activeSession; - if ( - (!globalPipePath && !requestedSession?.requester) || - !requestedSession || - requestedSession.workspaceUnavailable - ) { - return; +/** Resolve dirty metadata for the document's owner without selecting that session. */ +export function ensureSessionWorkspace(target: Session): Promise { + // Interactive agents own invalidation and publish their complete cached workspace. + if (target.requester) { + return Promise.resolve(); } - try { - const response = await sessionRequest({ method: 'workspace' }, requestedSession); - if (response && sessions.get(requestedSession.sessionId) === requestedSession) { - updateSessionWorkspace(requestedSession, response as WorkspaceData); - console.info('[updateWorkspace] Done'); - } - } catch (e) { - console.error(e); + return workspaceRefreshFor(target).ensure(); +} + +export async function updateWorkspace(): Promise { + if (activeSession) { + const refresh = workspaceRefreshFor(activeSession); + refresh.invalidate(); + await refresh.ensure(); } } @@ -3377,6 +3344,9 @@ import * as rstudioapi from './rstudioapi'; export async function activateSession(session: Session): Promise { sessionSelectionRevision++; + if (activeSession && activeSession !== session) { + workspaceRefreshes.get(activeSession)?.defer(); + } activeSession = session; const refreshed = refreshActiveSession(session); if (!session.workspaceUnavailable) { @@ -3810,8 +3780,14 @@ async function handleNotification(message: Record, socket: IpcS } case 'workspace_updated': { - if (socket === activeSession?.socket) { - scheduleWorkspaceRefresh(); + const target = socket._sessionId && sessions.get(socket._sessionId); + if (target && target.socket === socket) { + const refresh = workspaceRefreshFor(target); + if (target === activeSession) { + refresh.invalidate(500); + } else { + refresh.invalidate(); + } } break; } @@ -4070,6 +4046,7 @@ export async function cleanupSession(sessionId: string, closingSocket?: IpcSocke if (!session || (closingSocket && session.socket !== closingSocket)) { return; } + workspaceRefreshes.get(session)?.dispose(); sessions.delete(sessionId); terminalRegistry.releaseSession(session); const disconnected = invalidateSessionConnection(session); @@ -4089,46 +4066,25 @@ export async function cleanupSession(sessionId: string, closingSocket?: IpcSocke export async function sessionRequest( data: Record, target: Session | string | null | undefined = activeSession, + options: RpcRequestOptions = {}, ): Promise { try { const owner = typeof target === 'string' ? sessions.get(target) : target; - if (owner?.requester) { - return await owner.requester(data); + const requester = owner?.requester; + if (requester) { + return await withRpcCancellation(() => requester(data), options.signal); } // An explicitly bound viewer must never fall back to the active session. const socket = owner?.socket ?? (target === undefined ? pipeClient : undefined); - if (!socket || socket.destroyed) { + const rpc = socket && nativeRpcConnections.get(socket); + if (!socket || socket.destroyed || !rpc) { throw new Error('IPC socket is not connected'); } - - return await new Promise((resolve, reject) => { - const id = - data.id !== undefined ? Number(data.id) : Math.floor(Math.random() * 1000000); - const payload = data.jsonrpc - ? data - : { - jsonrpc: '2.0', - id, - ...data, - }; - - pendingRequests.set(id, { resolve, reject, socket }); - - try { - socket.write(JSON.stringify(payload) + '\n'); - } catch (e) { - pendingRequests.delete(id); - reject(e); - } - - setTimeout(() => { - if (pendingRequests.has(id)) { - pendingRequests.delete(id); - reject(new Error('Request timed out')); - } - }, 5000); - }); + return await rpc.request(data, options); } catch (error) { + if (error instanceof Error && error.name === 'AbortError') { + return undefined; + } if (error instanceof Error) { console.log('error message: ', error.message); } else { diff --git a/src/signatureHelp.ts b/src/signatureHelp.ts index 95579ee8d..305cf3e36 100644 --- a/src/signatureHelp.ts +++ b/src/signatureHelp.ts @@ -1,5 +1,6 @@ import * as vscode from 'vscode'; -import { sessionForDocument } from './session'; +import { ensureSessionWorkspace, sessionForDocument } from './session'; +import { withRpcCancellation } from './nativeRpc'; import { getChunks } from './rmarkdown'; interface Token { @@ -215,11 +216,11 @@ export function liveSignature( } export class SessionSignatureHelpProvider implements vscode.SignatureHelpProvider { - provideSignatureHelp( + async provideSignatureHelp( document: vscode.TextDocument, position: vscode.Position, token: vscode.CancellationToken, - ): vscode.SignatureHelp | undefined { + ): Promise { if (!['r', 'rmd'].includes(document.languageId)) { return; } @@ -244,7 +245,37 @@ export class SessionSignatureHelpProvider implements vscode.SignatureHelpProvide if (text.length > 200000) { return; } - const live = liveSignature(text, target.workspaceData.globalenv); + let live = liveSignature(text, target.workspaceData.globalenv); + // Known formals stay immediately available while native R is busy. + // sessionForDocument has already scheduled any dirty cache refresh. + if (!live) { + const version = document.version; + const controller = new AbortController(); + const cancellation = token.onCancellationRequested(() => controller.abort()); + if (token.isCancellationRequested) { + controller.abort(); + } + try { + await withRpcCancellation(() => ensureSessionWorkspace(target), controller.signal); + } catch (error) { + if (error instanceof Error && error.name === 'AbortError') { + return; + } + throw error; + } finally { + cancellation.dispose(); + } + if ( + token.isCancellationRequested || + document.isClosed || + document.version !== version || + sessionForDocument(document.uri) !== target || + target.workspaceUnavailable + ) { + return; + } + live = liveSignature(text, target.workspaceData.globalenv); + } if (!live) { return; } diff --git a/src/test/node/incrementalOutput.test.ts b/src/test/node/incrementalOutput.test.ts new file mode 100644 index 000000000..e50f06d4d --- /dev/null +++ b/src/test/node/incrementalOutput.test.ts @@ -0,0 +1,276 @@ +import * as assert from 'assert'; +import type { NotebookCellExecution, NotebookCellOutput, NotebookCellOutputItem } from 'vscode'; +import { ExecutionCellIndex } from '../../interactive/cellIndex'; +import { + NotebookOutputState, + prepareNotebookStream, + updateNotebookOutputs, +} from '../../interactive/notebookOutput'; +import { StreamText } from '../../interactive/streamText'; + +suite('Incremental Interactive console delivery', () => { + function streamOutput(stream: StreamText, id = 'console'): NotebookCellOutput { + const output = { + metadata: { rStreamId: id }, + items: [{ mime: 'application/vnd.code.notebook.stdout', data: Buffer.alloc(0) }], + }; + prepareNotebookStream(output, stream.snapshot()); + return output; + } + + function notebook() { + const outputs: NotebookCellOutput[] = []; + let bytes = 0; + const calls: string[] = []; + const items = (value: NotebookCellOutputItem | readonly NotebookCellOutputItem[]) => + 'mime' in value ? [value] : [...value]; + const execution: Pick< + NotebookCellExecution, + 'replaceOutput' | 'appendOutput' | 'replaceOutputItems' | 'appendOutputItems' + > = { + replaceOutput(value) { + calls.push('replace'); + const next = 'items' in value ? [value] : [...value]; + bytes += next + .flatMap((output) => output.items) + .reduce((sum, item) => sum + item.data.length, 0); + outputs.splice(0, outputs.length, ...next); + return Promise.resolve(); + }, + appendOutput(value) { + calls.push('append'); + const next = 'items' in value ? [value] : [...value]; + bytes += next + .flatMap((output) => output.items) + .reduce((sum, item) => sum + item.data.length, 0); + outputs.push(...next); + return Promise.resolve(); + }, + replaceOutputItems(value, output) { + calls.push('replaceItems'); + output.items = items(value); + bytes += output.items.reduce((sum, item) => sum + item.data.length, 0); + return Promise.resolve(); + }, + appendOutputItems(value, output) { + calls.push('appendItems'); + const added = items(value); + bytes += added.reduce((sum, item) => sum + item.data.length, 0); + // VS Code combines consecutive stdout items into one stream. + output.items = [ + { + mime: added[0].mime, + data: Buffer.concat([...output.items, ...added].map((item) => item.data)), + }, + ]; + return Promise.resolve(); + }, + }; + return { + outputs, + execution, + calls, + get bytes() { + return bytes; + }, + }; + } + + test('delivers a growing 4 MiB stream with exactly 4 MiB of payload', async () => { + const model = notebook(), + state = new NotebookOutputState(), + text = new StreamText(''); + for (let index = 0; index < 128; index++) { + text.append('x'.repeat(32768)); + await updateNotebookOutputs( + model.execution, + [...model.outputs], + [streamOutput(text)], + state, + ); + } + const handle = model.outputs[0]; + text.seal(); + await updateNotebookOutputs( + model.execution, + [...model.outputs], + [streamOutput(text)], + state, + ); + assert.strictEqual(model.outputs[0], handle); + assert.strictEqual(model.bytes, 4 * 1024 * 1024); + assert.strictEqual(model.outputs[0].items[0].data.length, model.bytes); + assert.strictEqual(model.calls.filter((call) => call === 'appendItems').length, 127); + assert.ok(!model.calls.includes('replaceItems')); + }); + + test('snapshots keep their original prefix through append and compaction', () => { + const text = new StreamText('first'), + before = text.snapshot(); + text.append(' second'); + text.append('🙂'.repeat(20000)); + const during = text.snapshot(); + text.seal(); + text.append('\rnext\b!'); + assert.strictEqual(before.text(), 'first'); + assert.strictEqual(during.text(5), ' second' + '🙂'.repeat(20000)); + assert.strictEqual(text.snapshot().text(during.length), '\rnext\b!'); + }); + + test('restoration and externally replaced output rebuild the current stream once', async () => { + const model = notebook(), + state = new NotebookOutputState(), + text = new StreamText('é🙂'); + await updateNotebookOutputs(model.execution, [], [streamOutput(text)], state); + state.reset(); + text.append(' next'); + await updateNotebookOutputs( + model.execution, + [...model.outputs], + [streamOutput(text)], + state, + ); + model.outputs[0].items = [ + { + mime: model.outputs[0].items[0].mime, + data: Buffer.alloc(Buffer.byteLength('é🙂 next'), 120), + }, + ]; + text.append(' last'); + await updateNotebookOutputs( + model.execution, + [...model.outputs], + [streamOutput(text)], + state, + ); + assert.strictEqual(Buffer.from(model.outputs[0].items[0].data).toString(), 'é🙂 next last'); + assert.strictEqual(model.calls.filter((call) => call === 'replaceItems').length, 2); + }); + + test('a failed append does not acknowledge text which was not delivered', async () => { + const model = notebook(), + state = new NotebookOutputState(), + text = new StreamText('first'); + await updateNotebookOutputs(model.execution, [], [streamOutput(text)], state); + text.append(' second'); + await assert.rejects( + updateNotebookOutputs( + { + ...model.execution, + appendOutputItems: () => Promise.reject(new Error('delivery failed')), + }, + [...model.outputs], + [streamOutput(text)], + state, + ), + /delivery failed/, + ); + await updateNotebookOutputs( + model.execution, + [...model.outputs], + [streamOutput(text)], + state, + ); + assert.strictEqual(Buffer.from(model.outputs[0].items[0].data).toString(), 'first second'); + }); + + test('acknowledges normalized progress output without re-encoding its raw prefix', async () => { + const model = notebook(), + state = new NotebookOutputState(), + text = new StreamText('first'); + const append = model.execution.appendOutputItems.bind(model.execution); + model.execution.appendOutputItems = async (items, output) => { + await append(items, output); + let visible = ''; + for (const char of Buffer.from(output.items[0].data).toString()) { + if (char === '\r') { + visible = visible.slice(0, visible.lastIndexOf('\n') + 1); + } else if (char === '\b') { + visible = visible.slice(0, -1); + } else { + visible += char; + } + } + output.items[0].data = Buffer.from(visible); + }; + await updateNotebookOutputs(model.execution, [], [streamOutput(text)], state); + text.append('\rsecond'); + await updateNotebookOutputs( + model.execution, + [...model.outputs], + [streamOutput(text)], + state, + ); + text.append('\b!'); + await updateNotebookOutputs( + model.execution, + [...model.outputs], + [streamOutput(text)], + state, + ); + assert.strictEqual(Buffer.from(model.outputs[0].items[0].data).toString(), 'secon!'); + assert.ok(!model.calls.includes('replaceItems')); + assert.strictEqual(model.bytes, Buffer.byteLength('first\rsecond\b!')); + }); + + test('interleaved console and rich output retain order and plot identity', async () => { + const model = notebook(), + state = new NotebookOutputState(); + const before = new StreamText('before\n'), + after = new StreamText('after\n'); + const plot: NotebookCellOutput = { + metadata: { rDisplayId: 'plot' }, + items: [{ mime: 'image/svg+xml', data: Buffer.from('') }], + }; + await updateNotebookOutputs( + model.execution, + [], + [streamOutput(before, 'before'), plot], + state, + ); + const plotHandle = model.outputs[1]; + await updateNotebookOutputs( + model.execution, + [...model.outputs], + [streamOutput(before, 'before'), plot, streamOutput(after, 'after')], + state, + ); + after.append('\rprogress\b!'); + await updateNotebookOutputs( + model.execution, + [...model.outputs], + [streamOutput(before, 'before'), plot, streamOutput(after, 'after')], + state, + ); + assert.strictEqual(model.outputs[1], plotHandle); + assert.deepStrictEqual( + model.outputs.map((output) => Buffer.from(output.items[0].data).toString()), + ['before\n', '', 'after\n\rprogress\b!'], + ); + }); + + test('indexes cells once while outputs change and rebuilds after notebook edits', () => { + const index = new ExecutionCellIndex<{ + metadata: { rExecutionId: string }; + value: number; + }>(); + let reads = 0; + const cells = Array.from({ length: 10000 }, (_, value) => ({ + metadata: { rExecutionId: String(value) }, + value, + })); + const read = () => { + reads++; + return cells; + }; + for (let update = 0; update < 128; update++) { + assert.strictEqual(index.get('9999', read), cells[9999]); + } + assert.strictEqual(reads, 1); + cells[9999].metadata.rExecutionId = 'edited'; + index.invalidate(); + assert.strictEqual(index.get('9999', read), undefined); + assert.strictEqual(index.get('edited', read), cells[9999]); + assert.strictEqual(reads, 2); + }); +}); diff --git a/src/test/node/interactiveBackend.test.ts b/src/test/node/interactiveBackend.test.ts index a6ba3d564..192f12eb3 100644 --- a/src/test/node/interactiveBackend.test.ts +++ b/src/test/node/interactiveBackend.test.ts @@ -210,6 +210,51 @@ suite('Interactive backend configuration', () => { await client.request('stop'); assert.strictEqual((await client.snapshot()).manifest.status, 'exited'); }); + test('coalesces workspace notifications and discards metadata invalidated during inspection', async () => { + let calls = 0; + let release!: (value: unknown) => void; + backend.inspectResult = () => { + calls++; + return calls === 1 + ? new Promise((resolve) => { + release = resolve; + }) + : Promise.resolve({ revision: 2, globalenv: { latest: {} } }); + }; + for (let i = 0; i < 20; i++) { + backend.emit({ type: 'workspaceChanged' }); + } + await until(() => calls === 1); + for (let i = 0; i < 20; i++) { + backend.emit({ type: 'workspaceChanged' }); + } + assert.strictEqual(calls, 1); + release({ revision: 1, globalenv: { stale: {} } }); + await until(() => + events.some((event) => event.type === 'workspace' && event.data.revision === 2), + ); + assert.strictEqual(calls, 2); + assert.ok(!events.some((event) => event.type === 'workspace' && event.data.revision === 1)); + assert.deepStrictEqual(await client.request('inspect', { method: 'workspace' }), { + revision: 2, + globalenv: { latest: {} }, + }); + }); + + test('unchanged workspace replies retain the complete cache without publishing another event', async () => { + let calls = 0; + const previous = (await client.snapshot()).workspace; + const published = events.filter((event) => event.type === 'workspace').length; + backend.inspectResult = () => { + calls++; + return Promise.resolve({ revision: 1, unchanged: true }); + }; + backend.emit({ type: 'workspaceChanged' }); + await until(() => calls === 1); + assert.deepStrictEqual((await client.snapshot()).workspace, previous); + assert.strictEqual(events.filter((event) => event.type === 'workspace').length, published); + }); + test('handles completion before transport acknowledgement and deduplicates submissions', async () => { let acknowledge!: () => void; backend.dispatchResult = () => diff --git a/src/test/node/journalEfficiency.test.ts b/src/test/node/journalEfficiency.test.ts new file mode 100644 index 000000000..43e2e5c33 --- /dev/null +++ b/src/test/node/journalEfficiency.test.ts @@ -0,0 +1,240 @@ +import * as assert from 'assert'; +import fs = require('fs'); +import * as os from 'os'; +import * as path from 'path'; +import * as sinon from 'sinon'; +import { createHash } from 'crypto'; +import { + SessionJournal, + readPreviousJournalAsync, + retainedAssetIds, + atomicJson, +} from '../../interactive/journal'; +import { getJournalIndex } from '../../interactive/journalIndex'; + +suite('Indexed journal working sets', () => { + let root: string; + setup(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'r-journal-index-')); + }); + teardown(() => { + fs.rmSync(root, { recursive: true, force: true }); + }); + + test('reads only the latest execution range after old output rotates into multiple segments', () => { + const directory = path.join(root, 'generation'); + const journal = new SessionJournal(directory, 'generation', 32 * 1024 * 1024); + try { + const old = journal.accept({ id: 'old', code: 'print(large)' }).record; + journal.append('accepted', { record: old }, old.id); + for (let i = 0; i < 24; i++) { + journal.append('stream', { text: 'x'.repeat(512 * 1024) }, old.id); + } + const latest = journal.accept({ id: 'latest', code: '1 + 1' }).record; + journal.append('accepted', { record: latest }, latest.id); + journal.append('stream', { text: '[1] 2\n' }, latest.id); + let readBytes = 0; + const original = fs.readSync; + const stub = sinon.stub(fs, 'readSync').callsFake((( + ...args: Parameters + ) => { + const size = Reflect.apply(original, fs, args) as number; + readBytes += size; + return size; + }) as typeof fs.readSync); + try { + const recent = journal.recent(1); + assert.deepStrictEqual( + recent.executions.map((record) => record.id), + ['latest'], + ); + assert.deepStrictEqual( + recent.events.map((event) => event.type), + ['accepted', 'stream'], + ); + assert.ok( + readBytes < 64 * 1024, + 'A one-cell read must not scan the old 12 MiB output', + ); + } finally { + stub.restore(); + } + } finally { + journal.close(); + } + }); + + test('reopens indexed output without rereading full segments and repairs a torn tail', () => { + const directory = path.join(root, 'generation'); + let journal = new SessionJournal(directory, 'generation'); + const record = journal.accept({ id: 'old', code: 'print(x)' }).record; + journal.append('accepted', { record }, record.id); + for (let i = 0; i < 10; i++) { + journal.append('stream', { text: 'x'.repeat(512 * 1024) }, record.id); + } + const seq = journal.seq; + journal.close(); + const stub = sinon.stub(fs, 'readFileSync').callsFake((( + ...args: Parameters + ) => { + assert.ok( + !/events-\d{6}\.jsonl$/.test(String(args[0])), + 'Warm open should use its verified index', + ); + return Reflect.apply(stub.wrappedMethod, fs, args) as Buffer; + }) as typeof fs.readFileSync); + try { + journal = new SessionJournal(directory, 'generation'); + assert.strictEqual(journal.seq, seq); + journal.close(); + } finally { + stub.restore(); + } + const file = getJournalIndex(directory, 'generation').segments.at(-1)!.name; + fs.appendFileSync(path.join(directory, file), '{"torn":'); + journal = new SessionJournal(directory, 'generation'); + try { + assert.strictEqual(journal.seq, seq); + assert.strictEqual( + journal.accept({ id: record.id, code: record.code }).duplicate, + true, + ); + assert.throws( + () => journal.accept({ id: record.id, code: 'different()' }), + /different code/, + ); + } finally { + journal.close(); + } + }); + + test('reads legacy archived records and output asynchronously without repairing files', async () => { + const directory = path.join(root, 'generation'); + const journal = new SessionJournal(directory, 'generation'); + for (const id of ['old', 'latest']) { + const record = journal.accept({ id, code: 'print("' + id + '")' }).record; + journal.append('accepted', { record }, id); + journal.append('stream', { text: id }, id); + } + journal.close(); + fs.rmSync(path.join(directory, 'executions-index.json')); + fs.rmSync(path.join(directory, 'events-index.json')); + const file = path.join(directory, 'events-000000.jsonl'); + fs.appendFileSync(file, '{"torn":'); + const before = fs.readFileSync(file); + const stub = sinon + .stub(fs, 'readFileSync') + .throws(new Error('Archive disk reads must be asynchronous')); + try { + const snapshot = await readPreviousJournalAsync(root, 'generation', 1); + assert.deepStrictEqual( + snapshot.executions.map((record) => record.id), + ['latest'], + ); + assert.deepStrictEqual( + snapshot.events.map((event) => event.type), + ['accepted', 'stream'], + ); + assert.strictEqual(snapshot.seq, 4); + } finally { + stub.restore(); + } + assert.deepStrictEqual(fs.readFileSync(file), before); + }); + + test('checks authoritative events before trusting a manipulated cold asset index', () => { + const storage = path.join(root, 'original'), + copy = path.join(root, 'copied'); + const journal = new SessionJournal(path.join(storage, 'generation'), 'generation'); + journal.append('display', { displayId: 'plot', svg: 'kept.svg' }, 'cell'); + journal.close(); + fs.cpSync(storage, copy, { recursive: true }); + const file = path.join(copy, 'generation', 'events-index.json'); + const saved = JSON.parse(fs.readFileSync(file, 'utf8')) as { data: string; hash: string }; + const index = JSON.parse(saved.data) as { + segments: { name: string; mtime: number; displays: object }[]; + }; + for (const segment of index.segments) { + segment.mtime = fs.statSync(path.join(copy, 'generation', segment.name)).mtimeMs; + segment.displays = {}; + } + saved.data = JSON.stringify(index); + saved.hash = createHash('sha256').update(saved.data).digest('hex'); + fs.writeFileSync(file, JSON.stringify(saved)); + // Read-only restoration may use the checksum-valid offset index. Collection + // must still verify its display references against the source at least once. + getJournalIndex(path.join(copy, 'generation'), 'generation'); + assert.deepStrictEqual(retainedAssetIds(copy), new Set(['kept.svg'])); + fs.appendFileSync(path.join(copy, 'generation', 'events-000000.jsonl'), '{invalid}\n'); + assert.throws(() => retainedAssetIds(copy)); + }); + + test('recognizes new durable admissions even when a directory timestamp is unchanged', async () => { + const directory = path.join(root, 'generation'), + folder = path.join(directory, 'executions'); + let journal = new SessionJournal(directory, 'generation'); + const first = journal.accept({ id: 'old', code: 'old()' }).record; + const timestamp = new Date(1700000000000); + fs.utimesSync(folder, timestamp, timestamp); + journal.close(); + const saved = JSON.parse( + fs.readFileSync(path.join(directory, 'executions-index.json'), 'utf8'), + ) as { mtime: number }; + const added = { + ...first, + id: 'added', + code: 'new()', + hash: createHash('sha256').update('new()').digest('hex'), + order: first.order + 1, + }; + atomicJson(path.join(folder, 'added.json'), added); + fs.utimesSync(folder, timestamp, timestamp); + assert.strictEqual( + fs.statSync(folder).mtimeMs, + saved.mtime, + 'Simulate a coarse unchanged directory timestamp', + ); + const snapshot = await readPreviousJournalAsync(root, 'generation', 1); + assert.deepStrictEqual( + snapshot.executions.map((record) => record.id), + ['added'], + ); + journal = new SessionJournal(directory, 'generation'); + try { + assert.strictEqual(journal.accept({ id: added.id, code: added.code }).duplicate, true); + assert.throws( + () => journal.accept({ id: added.id, code: 'different()' }), + /different code/, + ); + assert.strictEqual( + journal.accept({ id: 'next', code: 'next()' }).record.order, + added.order + 1, + ); + } finally { + journal.close(); + } + }); + + test('keeps deduplication after source-code working-set eviction', () => { + const journal = new SessionJournal(path.join(root, 'generation'), 'generation'); + try { + for (let i = 0; i < 8; i++) { + journal.accept({ id: 'id' + i, code: 'x'.repeat(1024 * 1024) }); + } + assert.strictEqual(journal.executions.size, 8); + const cache = journal.executions as unknown as { cacheBytes: number }; + assert.ok(cache.cacheBytes <= 4 * 1024 * 1024); + assert.strictEqual( + journal.accept({ id: 'id0', code: 'x'.repeat(1024 * 1024) }).duplicate, + true, + ); + assert.throws( + () => journal.accept({ id: 'id0', code: 'different()' }), + /different code/, + ); + assert.ok(journal.executions.history('', Number.MAX_SAFE_INTEGER).more); + } finally { + journal.close(); + } + }); +}); diff --git a/src/test/node/messagePages.test.ts b/src/test/node/messagePages.test.ts new file mode 100644 index 000000000..a074af6db --- /dev/null +++ b/src/test/node/messagePages.test.ts @@ -0,0 +1,319 @@ +import * as assert from 'assert'; +import * as fs from 'fs'; +import * as net from 'net'; +import * as os from 'os'; +import * as path from 'path'; +import { randomUUID } from 'crypto'; +import * as sinon from 'sinon'; +import { SessionAgent } from '../../interactive/agent'; +import { AgentClient } from '../../interactive/client'; +import { BackendEvent, ClientReply, SessionBackend } from '../../interactive/backend'; +import { AgentConfig, SessionEvent, SessionManifest, Submission } from '../../interactive/protocol'; +import { JsonLines } from '../../interactive/framing'; +import { MessagePages, boundedJson } from '../../interactive/messagePages'; + +class BackendFixture implements SessionBackend { + readonly ownership = 'managed' as const; + readonly capabilities = { inspection: true }; + listener?: (event: BackendEvent) => void; + workspace: Record = {}; + onEvent(listener: (event: BackendEvent) => void): () => void { + this.listener = listener; + return () => { + this.listener = undefined; + }; + } + start(): Promise { + this.listener?.({ type: 'ready', metadata: {}, capabilities: this.capabilities }); + return Promise.resolve(); + } + dispatch(record: Submission): Promise { + this.listener?.({ type: 'started', executionId: record.id }); + return Promise.resolve(); + } + inspect(): Promise { + return Promise.resolve(this.workspace); + } + replyInput(): Promise { + return Promise.resolve(); + } + replyClientRequest(_id: string, _reply: ClientReply): Promise { + return Promise.resolve(); + } + interrupt(): Promise { + return Promise.resolve(); + } + stop(): Promise { + return Promise.resolve(); + } + dispose(): Promise { + return Promise.resolve(); + } +} + +suite('Bounded message pages', () => { + test('renews an idle expiry while pages continue to make progress', () => { + const clock = sinon.useFakeTimers(); + try { + const pages = new MessagePages(); + const reference = pages.retain({ + text: 'x'.repeat(2 * 1024 * 1024), + }).__rInteractivePage; + clock.tick(20000); + const first = pages.read(reference.token, 0); + clock.tick(20000); + const second = pages.read(reference.token, first.next); + assert.ok(second.next > first.next); + clock.tick(30001); + assert.throws(() => pages.read(reference.token, second.next), /expired/); + } finally { + clock.restore(); + } + }); + test('limits scratch serialization and rejects expired or foreign cursors', () => { + const pages = new MessagePages(); + const first = pages.retain({ text: 'x'.repeat(1024 * 1024) }).__rInteractivePage; + assert.throws(() => pages.read('foreign', 0), /expired|cursor/); + assert.throws(() => pages.read(first.token, -1), /cursor/); + let offset = 0; + const chunks: Buffer[] = []; + while (offset < first.bytes) { + const page = pages.read(first.token, offset); + chunks.push(Buffer.from(page.data, 'base64')); + offset = page.next; + } + assert.strictEqual( + (JSON.parse(Buffer.concat(chunks).toString()) as { text: string }).text.length, + 1024 * 1024, + ); + assert.throws(() => pages.read(first.token, 0), /expired/); + assert.throws(() => boundedJson({ text: 'x'.repeat(1024) }, 100), /limit/); + }); +}); + +(process.platform === 'win32' ? suite.skip : suite)( + 'Paged agent snapshots and events', + function () { + this.timeout(15000); + let root: string; + setup(() => { + root = fs.mkdtempSync(path.join(os.tmpdir(), 'r-message-pages-')); + }); + teardown(() => { + fs.rmSync(root, { recursive: true, force: true }); + }); + + test('keeps large workspace metadata and the active execution within bounded wire frames', async () => { + const backend = new BackendFixture(), + globalenv: Record = {}; + for (let i = 0; i < 10000; i++) { + globalenv['object_' + i] = { type: 'integer', str: 'x'.repeat(200) }; + } + backend.workspace = { + globalenv, + search: ['package:base'], + loaded_namespaces: ['base'], + revision: 1, + }; + const config: AgentConfig = { + id: randomUUID(), + generation: randomUUID(), + label: 'pages', + directory: root, + storage: path.join(root, 'storage'), + provider: 'r', + supervision: 'test', + historyLimit: 50, + maxOutputBytes: 4 * 1024 * 1024, + maxJournalBytes: 32 * 1024 * 1024, + }; + const agent = new SessionAgent(config, () => backend); + const client = new AgentClient(await agent.start()); + try { + await client.connect(); + await client.request('inspect', { method: 'workspace' }); + const userValue = { __rInteractivePage: { token: 'user-data', bytes: 10 } }; + const originalWorkspace = backend.workspace; + backend.workspace = userValue; + assert.deepStrictEqual( + await client.request('inspect', { method: 'hover', params: { expr: 'x' } }), + userValue, + ); + backend.workspace = originalWorkspace; + let references = 0; + const parser = new JsonLines((message) => { + if (message.resultPage) { + references++; + } + }); + (client as unknown as { socket: net.Socket }).socket.on('data', (chunk: Buffer) => + parser.push(chunk), + ); + const id = randomUUID(); + await client.request('submit', { submission: { id, code: 'print(x)' } }); + for (let i = 0; i < 128; i++) { + backend.listener?.({ + type: 'stream', + executionId: id, + channel: 'stdout', + text: 'x'.repeat(32768), + }); + } + const queued = randomUUID(); + await client.request('submit', { + submission: { id: queued, code: 'x'.repeat(700 * 1024) }, + }); + const snapshot = await client.snapshot(1); + assert.strictEqual( + Object.keys(snapshot.workspace!.globalenv as object).length, + 10000, + ); + assert.strictEqual(snapshot.workspace!.revision, 1); + assert.ok( + snapshot.executions.some( + (record) => record.id === id && record.state === 'running', + ), + ); + assert.ok( + references > 0, + 'Oversized snapshots must be paged rather than disconnecting', + ); + assert.ok(client.connected); + } finally { + client.close(); + await agent.close(); + } + }); + + test('delivers a large live event before later small events in the same socket read', async () => { + const manifest = { + id: 'events', + generation: 'generation', + protocol: 1, + endpoint: path.join(root, 'events.sock'), + token: 'token', + } as SessionManifest; + const pages = new MessagePages(); + let connected!: net.Socket; + const server = net.createServer((socket) => { + connected = socket; + const parser = new JsonLines((message) => { + let result: unknown; + if (message.method === 'hello') { + result = manifest; + } else if (message.method === 'subscribe') { + result = { events: [], seq: 0, reset: false }; + } else if (message.method === 'messagePage') { + const params = message.params as { token: string; offset: number }; + result = pages.read(params.token, params.offset); + } + socket.write(JSON.stringify({ id: message.id, result }) + '\n'); + }); + socket.on('data', (chunk: Buffer) => parser.push(chunk)); + socket.on('error', () => undefined); + }); + const client = new AgentClient(manifest); + try { + await new Promise((resolve) => server.listen(manifest.endpoint, resolve)); + await client.connect({ claim: false }); + await client.subscribe(0); + const events: SessionEvent[] = []; + let done!: () => void; + const delivered = new Promise((resolve) => { + done = resolve; + }); + client.on('event', (event: SessionEvent) => { + events.push(event); + if (events.length === 2) { + done(); + } + }); + const event = { + seq: 1, + generation: manifest.generation, + time: 1, + type: 'workspace', + data: { text: 'λ'.repeat(3 * 1024 * 1024) }, + } as SessionEvent; + connected.write( + JSON.stringify({ eventPage: pages.retain(event) }) + + '\n' + + JSON.stringify({ + event: { ...event, seq: 2, type: 'state', data: { status: 'idle' } }, + }) + + '\n', + ); + await delivered; + assert.deepStrictEqual( + events.map((value) => value.seq), + [1, 2], + ); + assert.strictEqual((events[0].data.text as string).length, 3 * 1024 * 1024); + } finally { + client.close(); + connected?.destroy(); + await new Promise((resolve) => server.close(() => resolve())); + } + }); + + test('does not carry an old page token onto a reconnected socket', async () => { + const manifest = { + id: 'reconnect', + generation: 'generation', + protocol: 1, + endpoint: path.join(root, 'reconnect.sock'), + token: 'token', + } as SessionManifest; + const pages = new MessagePages(); + let connections = 0, + oldPageStarted!: () => void; + const oldPage = new Promise((resolve) => { + oldPageStarted = resolve; + }); + const methods: string[][] = []; + const sockets = new Set(); + const server = net.createServer((socket) => { + sockets.add(socket); + const index = connections++; + methods.push([]); + const parser = new JsonLines((message) => { + methods[index].push(String(message.method)); + if (message.method === 'hello') { + socket.write(JSON.stringify({ id: message.id, result: manifest }) + '\n'); + } + if (message.method === 'snapshot') { + socket.write( + JSON.stringify({ + id: message.id, + resultPage: pages.retain({ text: 'x'.repeat(5 * 1024 * 1024) }), + }) + '\n', + ); + } + if (message.method === 'messagePage') { + oldPageStarted(); + } + }); + socket.on('data', (chunk: Buffer) => parser.push(chunk)); + socket.on('error', () => undefined); + }); + const client = new AgentClient(manifest); + try { + await new Promise((resolve) => server.listen(manifest.endpoint, resolve)); + await client.connect({ claim: false }); + const request = client.snapshot(); + const rejected = assert.rejects(request, /disconnected/); + await oldPage; + await client.connect({ claim: false }); + await rejected; + assert.deepStrictEqual(methods[1], ['hello']); + assert.ok(client.connected); + } finally { + client.close(); + for (const socket of sockets) { + socket.destroy(); + } + await new Promise((resolve) => server.close(() => resolve())); + } + }); + }, +); diff --git a/src/test/node/nativeRpc.test.ts b/src/test/node/nativeRpc.test.ts new file mode 100644 index 000000000..b257e4354 --- /dev/null +++ b/src/test/node/nativeRpc.test.ts @@ -0,0 +1,243 @@ +import * as assert from 'node:assert/strict'; +import * as sinon from 'sinon'; +import type { Socket } from 'node:net'; +import { + MAX_NATIVE_RPC_FRAME_BYTES, + NativeRpcConnection, + NativeRpcFrames, + withRpcCancellation, +} from '../../nativeRpc'; + +suite('Native sess RPC lifecycle', () => { + let clock: sinon.SinonFakeTimers; + const connections: NativeRpcConnection[] = []; + + setup(() => { + clock = sinon.useFakeTimers(); + }); + + teardown(() => { + for (const connection of connections.splice(0)) { + connection.close(); + } + clock.restore(); + }); + + function connection() { + const sent: Record[] = []; + const socket = { + destroyed: false, + write: (text: string) => { + sent.push(JSON.parse(text) as Record); + return true; + }, + }; + const rpc = new NativeRpcConnection(socket as Pick); + connections.push(rpc); + return { rpc, sent, socket }; + } + + test('assigns independent monotonic IDs even with repeated caller IDs', async () => { + const a = connection(), + b = connection(); + const first = a.rpc.request({ method: 'hover', id: 123 }); + const second = a.rpc.request({ method: 'completion', id: 123 }); + const other = b.rpc.request({ method: 'hover', id: 123 }); + assert.deepEqual( + a.sent.map((message) => message.id), + [1, 2], + ); + assert.equal(b.sent[0].id, 1); + a.rpc.response({ id: 2, result: 'second' }); + b.rpc.response({ id: 1, result: 'other socket' }); + a.rpc.response({ id: 1, result: 'first' }); + assert.deepEqual(await Promise.all([first, second, other]), [ + 'first', + 'second', + 'other socket', + ]); + assert.equal(clock.countTimers(), 0); + }); + + test('clears completed timers before another request starts', async () => { + const { rpc, sent } = connection(); + const first = rpc.request({ method: 'hover' }); + rpc.response({ id: sent[0].id, result: 'done' }); + assert.equal(await first, 'done'); + assert.equal(clock.countTimers(), 0); + await clock.tickAsync(4000); + const second = rpc.request({ method: 'completion' }); + await clock.tickAsync(1000); + assert.equal(clock.countTimers(), 1); + // A late duplicate response cannot complete the newer request. + rpc.response({ id: sent[0].id, result: 'late' }); + rpc.response({ id: sent[1].id, result: 'new' }); + assert.equal(await second, 'new'); + assert.equal(clock.countTimers(), 0); + }); + + test('times out one request without affecting others or retaining its timer', async () => { + const { rpc, sent } = connection(); + const expired = assert.rejects( + rpc.request({ method: 'hover' }, { timeout: 10 }), + /timed out/, + ); + const live = rpc.request({ method: 'completion' }, { timeout: 100 }); + await clock.tickAsync(10); + await expired; + assert.equal(clock.countTimers(), 1); + rpc.response({ id: sent[0].id, result: 'late' }); + rpc.response({ id: sent[1].id, result: 'live' }); + assert.equal(await live, 'live'); + assert.equal(clock.countTimers(), 0); + }); + + test('disconnect rejects pending calls and prevents further writes', async () => { + const { rpc, sent } = connection(); + const first = assert.rejects(rpc.request({ method: 'hover' }), /disconnected/); + const second = assert.rejects(rpc.request({ method: 'completion' }), /disconnected/); + rpc.close(); + rpc.close(); + await Promise.all([first, second]); + assert.equal(clock.countTimers(), 0); + await assert.rejects(rpc.request({ method: 'workspace' }), /not connected/); + assert.equal(sent.length, 2); + }); + + test('cancellation releases its timeout and ignores late responses', async () => { + const { rpc, sent } = connection(); + const controller = new AbortController(); + const cancelled = assert.rejects( + rpc.request({ method: 'hover' }, { signal: controller.signal }), + { name: 'AbortError' }, + ); + controller.abort(); + await cancelled; + assert.equal(clock.countTimers(), 0); + rpc.response({ id: sent[0].id, result: 'late' }); + const next = rpc.request({ method: 'completion' }); + rpc.response({ id: sent[1].id, result: 'next' }); + assert.equal(await next, 'next'); + }); + + test('does not send requests cancelled before admission', async () => { + const { rpc, sent } = connection(); + const controller = new AbortController(); + controller.abort(); + await assert.rejects(rpc.request({ method: 'hover' }, { signal: controller.signal }), { + name: 'AbortError', + }); + assert.equal(sent.length, 0); + assert.equal(clock.countTimers(), 0); + }); + + test('write errors and RPC errors settle requests and remove timers', async () => { + const { rpc, socket, sent } = connection(); + socket.write = () => { + throw new Error('write failed'); + }; + await assert.rejects(rpc.request({ method: 'hover' }), /write failed/); + assert.equal(clock.countTimers(), 0); + socket.write = (text) => { + sent.push(JSON.parse(text) as Record); + return true; + }; + const failed = assert.rejects(rpc.request({ method: 'hover' }), { + code: -32603, + message: 'R handler failed', + }); + rpc.response({ id: sent[0].id, error: { code: -32603, message: 'R handler failed' } }); + await failed; + assert.equal(clock.countTimers(), 0); + }); + + test('handles a reply delivered synchronously from write', async () => { + const { rpc, socket } = connection(); + socket.write = (text) => { + rpc.response({ id: (JSON.parse(text) as { id: number }).id, result: 'reply' }); + return true; + }; + assert.equal(await rpc.request({ method: 'hover' }), 'reply'); + assert.equal(clock.countTimers(), 0); + }); + + test('cancels a requester wait while leaving the backend operation running', async () => { + let complete!: (value: string) => void; + const backend = new Promise((resolve) => { + complete = resolve; + }); + const controller = new AbortController(); + const cancelled = assert.rejects( + withRpcCancellation(() => backend, controller.signal), + { name: 'AbortError' }, + ); + await Promise.resolve(); + controller.abort(); + await cancelled; + complete('backend still completes'); + assert.equal(await backend, 'backend still completes'); + }); + + test('does not start a requester operation when already cancelled', async () => { + const controller = new AbortController(); + controller.abort(); + await assert.rejects( + withRpcCancellation(() => assert.fail('operation must not start'), controller.signal), + { name: 'AbortError' }, + ); + }); +}); + +suite('Native sess RPC framing', () => { + test('preserves split UTF-8, CRLF and multiple complete frames', () => { + const lines: string[] = []; + const frames = new NativeRpcFrames((frame) => lines.push(frame.toString('utf8'))); + const payload = Buffer.from('{"result":"数据"}\r\n{"id":2}\n'); + const split = payload.indexOf(0xe6) + 1; + frames.push(payload.subarray(0, split)); + frames.push(payload.subarray(split, split + 1)); + frames.push(payload.subarray(split + 1)); + assert.deepEqual(lines, ['{"result":"数据"}\r', '{"id":2}']); + }); + + test('accepts native responses above Interactive limits', () => { + const payload = Buffer.from('x'.repeat(8 * 1024 * 1024) + '\n'); + let received = 0; + const frames = new NativeRpcFrames((frame) => { + received = frame.length; + }); + frames.push(payload.subarray(0, 1024)); + frames.push(payload.subarray(1024)); + assert.equal(received, 8 * 1024 * 1024); + assert.ok(received < MAX_NATIVE_RPC_FRAME_BYTES); + }); + + test('rejects an unterminated oversized frame before decoding it', () => { + const frames = new NativeRpcFrames(() => assert.fail(), 8); + frames.push(Buffer.from('12345678')); + assert.throws(() => frames.push(Buffer.from('9')), /frame exceeds size limit/); + }); + + test('applies the bound to completed and fragmented frames equally', () => { + const frames = new NativeRpcFrames(() => assert.fail(), 8); + assert.throws(() => frames.push(Buffer.from('123456789\n')), /frame exceeds size limit/); + frames.push(Buffer.from('1234')); + assert.throws(() => frames.push(Buffer.from('56789\n')), /frame exceeds size limit/); + }); + + test('counts bytes per frame rather than per socket read', () => { + const lines: string[] = []; + const frames = new NativeRpcFrames((frame) => lines.push(frame.toString()), 8); + frames.push(Buffer.from('12345678\nabcdefgh\n\nijklmnop\n')); + assert.deepEqual(lines, ['12345678', 'abcdefgh', 'ijklmnop']); + }); + + test('releases unfinished bytes when the connection closes', () => { + const lines: string[] = []; + const frames = new NativeRpcFrames((frame) => lines.push(frame.toString()), 8); + frames.push(Buffer.from('stale')); + frames.clear(); + frames.push(Buffer.from('new\n')); + assert.deepEqual(lines, ['new']); + }); +}); diff --git a/src/test/node/workspaceRefresh.test.ts b/src/test/node/workspaceRefresh.test.ts new file mode 100644 index 000000000..f427b8886 --- /dev/null +++ b/src/test/node/workspaceRefresh.test.ts @@ -0,0 +1,105 @@ +import * as assert from 'assert'; +import { WorkspaceRefreshScheduler } from '../../workspaceRefresh'; + +function deferred(): { promise: Promise; resolve(value: T): void } { + let resolve!: (value: T) => void; + const promise = new Promise((done) => { + resolve = done; + }); + return { promise, resolve }; +} + +suite('Workspace refresh scheduling', () => { + test('coalesces invalidations during an inspection and publishes only the newest metadata', async () => { + const first = deferred(); + let calls = 0; + const published: number[] = []; + const scheduler = new WorkspaceRefreshScheduler( + () => (++calls === 1 ? first.promise : Promise.resolve(2)), + (value) => published.push(value), + () => true, + ); + const running = scheduler.ensure(); + await Promise.resolve(); + for (let i = 0; i < 20; i++) { + scheduler.invalidate(); + assert.strictEqual(scheduler.ensure(), running); + } + assert.strictEqual(calls, 1); + first.resolve(1); + await running; + assert.strictEqual(calls, 2); + assert.deepStrictEqual(published, [2]); + await scheduler.ensure(); + assert.strictEqual(calls, 2); + scheduler.dispose(); + }); + + test('retains dirty metadata without demand and drops a reply after the owner becomes unavailable', async () => { + let available = false; + let calls = 0; + const first = deferred(); + const published: number[] = []; + const scheduler = new WorkspaceRefreshScheduler( + () => (++calls === 1 ? first.promise : Promise.resolve(2)), + (value) => published.push(value), + () => available, + ); + scheduler.invalidate(0); + await scheduler.ensure(); + assert.strictEqual(calls, 0); + available = true; + const running = scheduler.ensure(); + await Promise.resolve(); + available = false; + first.resolve(1); + await running; + assert.deepStrictEqual(published, []); + available = true; + await scheduler.ensure(); + assert.deepStrictEqual(published, [2]); + scheduler.dispose(); + }); + + test('disposal suppresses replies from an obsolete connection', async () => { + const result = deferred(); + const published: number[] = []; + const scheduler = new WorkspaceRefreshScheduler( + () => result.promise, + (value) => published.push(value), + () => true, + ); + const running = scheduler.ensure(); + await Promise.resolve(); + scheduler.dispose(); + result.resolve(1); + await running; + scheduler.invalidate(0); + await scheduler.ensure(); + assert.deepStrictEqual(published, []); + }); + + test('deferred sends keep invalidation and inspection failures do not spin', async () => { + let calls = 0; + const errors: unknown[] = []; + const scheduler = new WorkspaceRefreshScheduler( + () => { + calls++; + return Promise.reject(new Error('R busy')); + }, + () => assert.fail('A failed inspection must preserve the previous workspace'), + () => true, + (error) => errors.push(error), + ); + scheduler.invalidate(1000); + scheduler.defer(); + await scheduler.ensure(); + await scheduler.ensure(); + assert.strictEqual(calls, 1); + assert.strictEqual(errors.length, 1); + scheduler.invalidate(); + await scheduler.ensure(); + assert.strictEqual(calls, 2); + scheduler.dispose(); + }); +}); diff --git a/src/test/suite/completions.test.ts b/src/test/suite/completions.test.ts index f800162ca..79b72eaff 100644 --- a/src/test/suite/completions.test.ts +++ b/src/test/suite/completions.test.ts @@ -50,6 +50,153 @@ suite('Session hover and completion', () => { sinon.assert.notCalled(sessionRequest); }); + test('cancelled hover releases its native request wait', async () => { + const document = await openDocument(); + const cancellation = new vscode.CancellationTokenSource(); + let signal!: AbortSignal; + sessionRequest.callsFake((_data, _target, options: { signal: AbortSignal }) => { + signal = options.signal; + return new Promise((resolve) => { + signal.addEventListener('abort', () => resolve(undefined), { once: true }); + }); + }); + try { + const hover = new HoverProvider().provideHover( + document, + new vscode.Position(2, 0), + cancellation.token, + ); + await new Promise((resolve) => setImmediate(resolve)); + sinon.assert.calledOnce(sessionRequest); + cancellation.cancel(); + assert.equal(await hover, null); + assert.equal(signal.aborted, true); + } finally { + cancellation.dispose(); + } + }); + + test('completed hover disposes its cancellation subscription', async () => { + const document = await openDocument(); + const cancellation = new vscode.CancellationTokenSource(); + sessionRequest.resolves({ str: 'integer, length 1' }); + try { + const hover = await new HoverProvider().provideHover( + document, + new vscode.Position(2, 0), + cancellation.token, + ); + assert.ok(hover); + const signal = (sessionRequest.firstCall.args[2] as { signal: AbortSignal }).signal; + cancellation.cancel(); + assert.equal(signal.aborted, false); + } finally { + cancellation.dispose(); + } + }); + + test('hover awaits fresh metadata from its owning workspace', async () => { + const document = await openDocument(); + let finish!: () => void; + const refreshed = new Promise((resolve) => { + finish = resolve; + }); + sandbox.stub(session, 'ensureSessionWorkspace').returns(refreshed); + const hover = new HoverProvider().provideHover(document, new vscode.Position(2, 0)); + session.activeSession!.workspaceData.globalenv.x = { + class: 'function', + type: 'closure', + length: 1, + str: 'function (new_arg)', + }; + finish(); + const result = await hover; + assert.ok(result); + assert.ok((result.contents[0] as vscode.MarkdownString).value.includes('new_arg')); + sinon.assert.notCalled(sessionRequest); + }); + + test('cached function hover remains available while workspace refresh waits for busy R', async () => { + const document = await openDocument(); + let finish!: () => void; + sandbox.stub(session, 'ensureSessionWorkspace').returns( + new Promise((resolve) => { + finish = resolve; + }), + ); + session.activeSession!.workspaceData.globalenv.x = { + class: 'function', + type: 'closure', + length: 1, + str: 'function (cached_arg)', + }; + try { + const hover = await new HoverProvider().provideHover( + document, + new vscode.Position(2, 0), + ); + assert.ok(hover); + assert.ok((hover.contents[0] as vscode.MarkdownString).value.includes('cached_arg')); + sinon.assert.notCalled(sessionRequest); + } finally { + finish(); + } + }); + + test('hover cancels while awaiting dirty workspace metadata', async () => { + const document = await openDocument(); + const cancellation = new vscode.CancellationTokenSource(); + let finish!: () => void; + sandbox.stub(session, 'ensureSessionWorkspace').returns( + new Promise((resolve) => { + finish = resolve; + }), + ); + try { + const hover = new HoverProvider().provideHover( + document, + new vscode.Position(2, 0), + cancellation.token, + ); + cancellation.cancel(); + assert.equal(await hover, null); + sinon.assert.notCalled(sessionRequest); + } finally { + finish(); + cancellation.dispose(); + } + }); + + test('cancelled member completion releases its native request wait', async () => { + const document = await vscode.workspace.openTextDocument({ language: 'r', content: 'x$' }); + const cancellation = new vscode.CancellationTokenSource(); + let signal!: AbortSignal; + sessionRequest.callsFake((_data, _target, options: { signal: AbortSignal }) => { + signal = options.signal; + return new Promise((resolve) => { + signal.addEventListener('abort', () => resolve(undefined), { once: true }); + }); + }); + try { + const items = new LiveCompletionItemProvider().provideCompletionItems( + document, + new vscode.Position(0, 2), + cancellation.token, + { + triggerKind: vscode.CompletionTriggerKind.TriggerCharacter, + triggerCharacter: '$', + }, + ); + await new Promise((resolve) => setImmediate(resolve)); + sinon.assert.calledOnce(sessionRequest); + cancellation.cancel(); + assert.deepEqual(await items, []); + assert.equal(signal.aborted, true); + } finally { + cancellation.dispose(); + } + }); + test('completion after a call result sends no request', async () => { const document = await openDocument(); const token = new vscode.CancellationTokenSource().token; diff --git a/src/test/suite/notebookStream.test.ts b/src/test/suite/notebookStream.test.ts new file mode 100644 index 000000000..5cbc93bde --- /dev/null +++ b/src/test/suite/notebookStream.test.ts @@ -0,0 +1,83 @@ +import * as assert from 'assert'; +import * as vscode from 'vscode'; +import { StreamText } from '../../interactive/streamText'; +import { + NotebookOutputState, + prepareNotebookStream, + updateNotebookOutputs, +} from '../../interactive/notebookOutput'; + +suite('Interactive console append API', () => { + test('uses only new suffixes after VS Code normalizes progress and Unicode text', async () => { + const type = 'r-interactive'; + const controller = vscode.notebooks.createNotebookController( + 'r-stream-api-test', + type, + 'Console API test', + ); + controller.supportedLanguages = ['r']; + const document = await vscode.workspace.openNotebookDocument( + type, + new vscode.NotebookData([ + new vscode.NotebookCellData( + vscode.NotebookCellKind.Code, + '# console delivery', + 'r', + ), + ]), + ); + const editor = await vscode.window.showNotebookDocument(document); + try { + await vscode.commands.executeCommand('notebook.selectKernel', { + id: controller.id, + extension: 'REditorSupport.r', + notebookEditor: editor, + }); + const cell = document.cellAt(0); + const execution = controller.createNotebookCellExecution(cell); + execution.start(); + const text = new StreamText('abcdef'); + const state = new NotebookOutputState(); + const starts: number[] = []; + const write = async () => { + const snapshot = text.snapshot(); + const read = snapshot.text.bind(snapshot); + snapshot.text = (start = 0) => { + starts.push(start); + return read(start); + }; + const output = new vscode.NotebookCellOutput( + [vscode.NotebookCellOutputItem.stdout('')], + { rStreamId: 'test-stream' }, + ); + prepareNotebookStream(output, snapshot); + await updateNotebookOutputs( + execution, + cell.outputs, + [output], + state, + () => cell.outputs, + ); + }; + try { + await write(); + text.append('\rxy'); + await write(); + text.append('\b!'); + await write(); + text.append('\nλ🙂\n'); + await write(); + assert.deepStrictEqual(starts, [0, 6, 9, 11]); + assert.strictEqual( + Buffer.from(cell.outputs[0].items[0].data).toString(), + 'xycde!\nλ🙂\n', + ); + } finally { + execution.end(true); + } + } finally { + controller.dispose(); + await vscode.commands.executeCommand('workbench.action.revertAndCloseActiveEditor'); + } + }); +}); diff --git a/src/test/suite/sessionWorkspaceRefresh.test.ts b/src/test/suite/sessionWorkspaceRefresh.test.ts new file mode 100644 index 000000000..e4d98671d --- /dev/null +++ b/src/test/suite/sessionWorkspaceRefresh.test.ts @@ -0,0 +1,110 @@ +import * as assert from 'assert'; +import * as path from 'path'; +import * as sinon from 'sinon'; +import * as vscode from 'vscode'; +import * as extension from '../../extension'; +import * as session from '../../session'; +import * as util from '../../util'; +import { SessConsentService } from '../../sessConsent'; +import { mockExtensionContext } from '../common/mockvscode'; +import { SessionConnections, waitForValue } from '../common/sessionConnections'; + +suite('Session workspace refresh demand', () => { + let sandbox: sinon.SinonSandbox; + let connections: SessionConnections; + let foreground: session.Session; + const extensionRoot = path.join(__dirname, '..', '..', '..'); + + setup(async () => { + await session.shutdownSessionWatcher(); + sandbox = sinon.createSandbox(); + connections = new SessionConnections(); + mockExtensionContext(extensionRoot, sandbox); + sandbox.stub(extension, 'enableSessionWatcher').value(true); + sandbox.stub(extension, 'rWorkspace').value(undefined); + sandbox.stub(vscode.commands, 'executeCommand').resolves(); + sandbox.stub(vscode.window, 'terminals').value([]); + sandbox.stub(vscode.window, 'activeTerminal').value(undefined); + sandbox + .stub(util, 'config') + .returns({ get: () => undefined } as unknown as vscode.WorkspaceConfiguration); + sandbox.stub(SessConsentService.prototype, 'start').resolves(); + sandbox.stub(SessConsentService.prototype, 'stop').resolves(); + session.deploySessionWatcher(extensionRoot); + await session.getGlobalPipePath(); + }); + + teardown(async () => { + if (foreground) { + session.unregisterSessionTransport(foreground); + } + await connections.dispose(); + await session.shutdownSessionWatcher(); + sandbox.restore(); + }); + + test('background invalidation refreshes a bound document without changing selection', async () => { + const socket = await connections.attach('workspace-background', 55101, 'external-host', { + autoActivate: false, + attachmentId: 'workspace-background-fixture', + }); + await session.activateSessionById('workspace-background'); + const background = session.activeSession!; + foreground = session.registerSessionTransport('workspace-foreground', 'host', '/tmp', () => + Promise.resolve({ globalenv: {}, search: [], loaded_namespaces: [] }), + ); + await session.activateSession(foreground); + const requests: { id: number; params: { revision?: number } }[] = []; + sandbox.stub(socket, 'write').callsFake((data: string | Uint8Array) => { + const message = JSON.parse(data.toString()) as { + method?: string; + id: number; + params: { revision?: number }; + }; + if (message.method === 'workspace') { + requests.push(message); + } + return true; + }); + const receive = (message: unknown) => + socket.emit('data', Buffer.from(`${JSON.stringify(message)}\n`)); + receive({ jsonrpc: '2.0', method: 'workspace_updated' }); + assert.strictEqual(requests.length, 0); + const uri = vscode.Uri.parse('untitled:workspace-background.R'); + try { + session.bindSessionDocument(uri, background); + const request = await waitForValue(() => requests[0]); + receive({ + jsonrpc: '2.0', + id: request.id, + result: { + revision: 1, + globalenv: { + latest: { + class: 'integer', + type: 'integer', + length: 1, + str: 'integer, length 1', + }, + }, + search: [], + loaded_namespaces: [], + }, + }); + await session.ensureSessionWorkspace(background); + assert.ok(background.workspaceData.globalenv.latest); + assert.strictEqual(session.activeSession, foreground); + receive({ jsonrpc: '2.0', method: 'workspace_updated' }); + assert.strictEqual(requests.length, 1); + const refresh = session.ensureSessionWorkspace(background); + const next = await waitForValue(() => requests[1]); + assert.strictEqual(next.params.revision, 1); + receive({ jsonrpc: '2.0', id: next.id, result: { revision: 1, unchanged: true } }); + await refresh; + assert.ok(background.workspaceData.globalenv.latest); + assert.strictEqual(session.activeSession, foreground); + } finally { + session.unbindSessionDocument(uri); + } + }); +}); diff --git a/src/test/suite/signatureHelp.test.ts b/src/test/suite/signatureHelp.test.ts index 617c3961e..bb1da125f 100644 --- a/src/test/suite/signatureHelp.test.ts +++ b/src/test/suite/signatureHelp.test.ts @@ -1,5 +1,8 @@ import * as assert from 'assert'; -import { liveSignature } from '../../signatureHelp'; +import * as vscode from 'vscode'; +import * as sinon from 'sinon'; +import * as session from '../../session'; +import { liveSignature, SessionSignatureHelpProvider } from '../../signatureHelp'; suite('Live R function signatures', () => { const summaries = { @@ -79,3 +82,65 @@ suite('Live R function signatures', () => { ); }); }); + +suite('Session signature workspace demand', () => { + test('known cached signatures do not wait for a busy native workspace', async () => { + const document = await vscode.workspace.openTextDocument({ + language: 'r', + content: 'cached(', + }); + const target = { + workspaceData: { globalenv: { cached: { type: 'closure', str: 'function (x)' } } }, + } as unknown as session.Session; + const owner = sinon.stub(session, 'sessionForDocument').returns(target); + const refresh = sinon + .stub(session, 'ensureSessionWorkspace') + .returns(new Promise(() => {})); + const cancellation = new vscode.CancellationTokenSource(); + try { + const help = await new SessionSignatureHelpProvider().provideSignatureHelp( + document, + new vscode.Position(0, 7), + cancellation.token, + ); + assert.strictEqual(help?.signatures[0].label, 'cached(x)'); + assert.ok(!refresh.called); + } finally { + cancellation.dispose(); + owner.restore(); + refresh.restore(); + } + }); + + test('a missing signature refreshes its owner without changing the selection', async () => { + const document = await vscode.workspace.openTextDocument({ + language: 'r', + content: 'fresh(', + }); + const target = { workspaceData: { globalenv: {} } } as unknown as session.Session; + const owner = sinon.stub(session, 'sessionForDocument').returns(target); + const refresh = sinon.stub(session, 'ensureSessionWorkspace').callsFake(() => { + target.workspaceData.globalenv.fresh = { + class: 'function', + type: 'closure', + str: 'function (x)', + length: 1, + }; + return Promise.resolve(); + }); + const cancellation = new vscode.CancellationTokenSource(); + try { + const help = await new SessionSignatureHelpProvider().provideSignatureHelp( + document, + new vscode.Position(0, 6), + cancellation.token, + ); + assert.strictEqual(help?.signatures[0].label, 'fresh(x)'); + assert.ok(refresh.calledOnceWithExactly(target)); + } finally { + cancellation.dispose(); + owner.restore(); + refresh.restore(); + } + }); +}); diff --git a/src/workspaceRefresh.ts b/src/workspaceRefresh.ts new file mode 100644 index 000000000..4cb925328 --- /dev/null +++ b/src/workspaceRefresh.ts @@ -0,0 +1,80 @@ +/** One inspection in flight, with one pending invalidation for the owning session. */ +export class WorkspaceRefreshScheduler { + private dirty = true; + private revision = 0; + private timer?: NodeJS.Timeout; + private running?: Promise; + private disposed = false; + + constructor( + private readonly read: () => Promise, + private readonly publish: (value: T) => void, + private readonly canRefresh: () => boolean, + private readonly report: (error: unknown) => void = () => {}, + ) {} + + invalidate(delayMs?: number): void { + if (this.disposed) { + return; + } + this.dirty = true; + this.revision++; + this.defer(); + if (delayMs !== undefined && this.canRefresh()) { + this.timer = setTimeout(() => { + this.timer = undefined; + void this.ensure(); + }, delayMs); + } + } + + /** Cancel a scheduled read, retaining its invalidation until there is demand. */ + defer(): void { + clearTimeout(this.timer); + this.timer = undefined; + } + + ensure(): Promise { + if (this.running) { + return this.running; + } + if (this.disposed || !this.dirty || !this.canRefresh()) { + return Promise.resolve(); + } + this.defer(); + this.running = Promise.resolve() + .then(async () => { + while (!this.disposed && this.dirty && this.canRefresh()) { + const revision = this.revision; + this.dirty = false; + try { + const value = await this.read(); + if (this.disposed) { + return; + } + if (!this.canRefresh()) { + this.dirty = true; + return; + } + if (revision === this.revision && value !== undefined) { + this.publish(value); + } + } catch (error) { + this.report(error); + } + } + }) + .finally(() => { + this.running = undefined; + if (!this.disposed && this.dirty && this.canRefresh()) { + void this.ensure(); + } + }); + return this.running; + } + + dispose(): void { + this.disposed = true; + this.defer(); + } +} From 774e5f9c05cb0dfcccaff15435f15616cf8292a5 Mon Sep 17 00:00:00 2001 From: Kun Ren Date: Sun, 25 Oct 2026 10:59:04 +0800 Subject: [PATCH 2/4] fix: preserve replay and workspace state during refresh and cleanup --- sess/R/handlers.R | 8 +- sess/inst/tinytest/test-workspace-refresh.R | 37 +++++++++- src/interactive/agent.ts | 9 ++- src/interactive/journal.ts | 6 +- src/interactive/journalIndex.ts | 8 ++ src/interactive/manager.ts | 21 ++++++ src/interactive/notebookOutput.ts | 69 ++++++++++++++--- src/session.ts | 23 +++++- src/test/node/incrementalOutput.test.ts | 63 ++++++++++++++++ src/test/node/interactiveBackend.test.ts | 52 ++++++++++++- src/test/node/journalEfficiency.test.ts | 42 +++++++++++ src/test/node/messagePages.test.ts | 74 ++++++++++++++++++- src/test/node/workspaceRefresh.test.ts | 25 +++++++ src/test/suite/interactiveEditor.test.ts | 28 +++++++ .../suite/sessionWorkspaceRefresh.test.ts | 49 +++++++++++- src/test/suite/workspaceViewer.test.ts | 28 +++++++ src/workspaceViewer.ts | 11 +++ 17 files changed, 531 insertions(+), 22 deletions(-) diff --git a/sess/R/handlers.R b/sess/R/handlers.R index 51ce0fe4f..4f3f615f0 100644 --- a/sess/R/handlers.R +++ b/sess/R/handlers.R @@ -51,8 +51,12 @@ workspace_length <- function(object) { if (typeof(object) %in% c("closure", "builtin")) return(1L) # unclass() can copy an entire classed vector. Check dispatch before using the # primitive length instead; unknown custom lengths belong to explicit queries. - has_method <- any(vapply(c(class(object), "default"), function(class_name) { - !is.null(utils::getS3method("length", class_name, optional = TRUE, envir = .GlobalEnv)) + registry <- get(".__S3MethodsTable__.", envir = .BaseNamespaceEnv, inherits = FALSE) + has_method <- any(vapply(paste0("length.", c(class(object), "default")), function(method) { + # exists(mode = "any") does not force active bindings or promises. Treat an + # ambiguous binding as a method; getS3method's function lookup would force it. + exists(method, envir = .GlobalEnv, mode = "any", inherits = TRUE) || + exists(method, envir = registry, mode = "any", inherits = FALSE) }, logical(1))) if (has_method) NULL else length(object) } diff --git a/sess/inst/tinytest/test-workspace-refresh.R b/sess/inst/tinytest/test-workspace-refresh.R index f32588c49..be17e862a 100644 --- a/sess/inst/tinytest/test-workspace-refresh.R +++ b/sess/inst/tinytest/test-workspace-refresh.R @@ -6,7 +6,9 @@ local({ symbols <- c("workspace_refresh_custom", "workspace_refresh_frame", "workspace_refresh_env", "workspace_refresh_active", ".DollarNames.workspace_refresh_expensive", "length.workspace_refresh_expensive", "dim.workspace_refresh_expensive", - "workspace_refresh_unknown", "workspace_refresh_large") + "workspace_refresh_unknown", "workspace_refresh_large", + "workspace_refresh_active_method", "length.workspace_refresh_active_method", + "length.workspace_refresh_delayed_method") previous <- mget(intersect(symbols, ls(.GlobalEnv, all.names = TRUE)), .GlobalEnv) on.exit({ if (exists("workspace_refresh_active", .GlobalEnv, inherits = FALSE)) { @@ -60,6 +62,39 @@ local({ expect_identical(unknown$str, "workspace_refresh_expensive") expect_identical(inspected, 0L) + # Method detection must not force active bindings or promised methods either. + makeActiveBinding("length.workspace_refresh_active_method", expensive, .GlobalEnv) + assign("workspace_refresh_active_method", + structure(1:3, class = "workspace_refresh_active_method"), .GlobalEnv) + active_method <- sess:::get_workspace_data()$globalenv$workspace_refresh_active_method + expect_null(active_method$length) + expect_identical(inspected, 0L) + delayedAssign("length.workspace_refresh_delayed_method", { + expensive() + }, assign.env = .GlobalEnv) + expect_null(sess:::workspace_length(structure(1:3, class = "workspace_refresh_delayed_method"))) + expect_identical(inspected, 0L) + rm(list = "length.workspace_refresh_delayed_method", envir = .GlobalEnv) + + registry <- get(".__S3MethodsTable__.", envir = .BaseNamespaceEnv, inherits = FALSE) + registered_name <- "length.workspace_refresh_registered_method" + previous_method <- get0(registered_name, envir = registry, inherits = FALSE) + on.exit({ + if (is.null(previous_method)) { + rm(list = registered_name, envir = registry) + } else { + assign(registered_name, previous_method, envir = registry) + } + }, add = TRUE) + registerS3method("length", "workspace_refresh_registered_method", expensive, + envir = .BaseNamespaceEnv) + registered <- structure(1:3, class = "workspace_refresh_registered_method") + expect_null(sess:::workspace_length(registered)) + rm(list = registered_name, envir = registry) + makeActiveBinding(registered_name, expensive, registry) + expect_null(sess:::workspace_length(registered)) + expect_identical(inspected, 0L) + # Stripping a large classed vector's attributes must not copy its payload. assign(symbols[[9L]], structure(rep(1, 1000000L), class = "workspace_refresh_vector"), .GlobalEnv) if (isTRUE(capabilities("profmem"))) { diff --git a/src/interactive/agent.ts b/src/interactive/agent.ts index 07ff9a292..ad4221a2e 100644 --- a/src/interactive/agent.ts +++ b/src/interactive/agent.ts @@ -92,12 +92,14 @@ export class SessionAgent { timeout: 2000, }), ); - return response.unchanged ? undefined : response; + return response.unchanged ? this.workspace : response; }, (workspace) => { if (!isDeepStrictEqual(workspace, this.workspace)) { this.workspace = workspace; this.event('workspace', workspace); + } else { + this.event('workspaceInvalidated', {}); } }, () => @@ -407,7 +409,8 @@ export class SessionAgent { Math.max(1, Math.min(Number(params.limit) || 50, this.config.historyLimit)), this.current, ); - // Bound one protocol response. Older/full output remains in the journal. + // Bound historical output while preserving active code whose JSON + // escapes exceed the allowance. send() pages the complete envelope. let size = 0; const executions: ExecutionRecord[] = []; for (const record of recent.executions @@ -416,7 +419,7 @@ export class SessionAgent { a.id === this.current ? -1 : b.id === this.current ? 1 : b.order - a.order, )) { const bytes = Buffer.byteLength(JSON.stringify(record)); - if (size + bytes > 1400 * 1024) { + if (record.id !== this.current && size + bytes > 1400 * 1024) { break; } size += bytes; diff --git a/src/interactive/journal.ts b/src/interactive/journal.ts index fd00d7624..ae2b93421 100644 --- a/src/interactive/journal.ts +++ b/src/interactive/journal.ts @@ -507,7 +507,9 @@ export class SessionJournal { let recordBytes = 0; for (const record of records) { const bytes = Buffer.byteLength(JSON.stringify(record)); - if (recordBytes + bytes > 1400 * 1024) { + // Raw code is bounded at admission, but JSON escaping can exceed the + // history allowance. Active work must survive reconnect regardless. + if (record.id !== activeId && recordBytes + bytes > 1400 * 1024) { continue; } executions.push(record); @@ -520,7 +522,7 @@ export class SessionJournal { this.directory, this.index, new Set(executions.map((record) => record.id)), - maxRecentBytes - recordBytes, + Math.max(0, maxRecentBytes - recordBytes), ), }; } diff --git a/src/interactive/journalIndex.ts b/src/interactive/journalIndex.ts index 925c84aab..608d4b8ea 100644 --- a/src/interactive/journalIndex.ts +++ b/src/interactive/journalIndex.ts @@ -130,6 +130,14 @@ function decodeIndex(text: string, generation: string): JournalIndex | undefined ) { return; } + for (const segment of index.segments) { + // JSON restores Object.prototype. Valid execution IDs such as + // __proto__ must remain ordinary keys when this segment is appended. + segment.ranges = Object.assign( + Object.create(null) as Record, + segment.ranges, + ); + } return index; } catch { return; diff --git a/src/interactive/manager.ts b/src/interactive/manager.ts index 849dc2ce3..310cd5de2 100644 --- a/src/interactive/manager.ts +++ b/src/interactive/manager.ts @@ -374,6 +374,25 @@ export class InteractiveManager for (const view of this.views.values()) { if (view.notebook === event.notebook) { view.cellIndex?.invalidate(); + for (const change of event.contentChanges) { + for (const cell of change.removedCells) { + const id = cell.metadata.rExecutionId as string | undefined; + const generation = cell.metadata.rGeneration as + | string + | undefined; + const model = generation + ? [view.model, ...view.history].find( + (model) => model.generation === generation, + ) + : view.model; + for (const output of (id && model?.cells.get(id)?.outputs) || + []) { + if (output.stream) { + view.outputState?.forget(output.stream); + } + } + } + } } } } @@ -2023,6 +2042,8 @@ export class InteractiveManager view.target, event.data as unknown as session.WorkspaceData, ); + } else if (event.type === 'workspaceInvalidated') { + session.invalidateWorkspaceChildren(view.target); } else if (event.type === 'input' && view.client.control) { void this.prompt(view, event.data); } else if (event.type === 'clientRequest' && view.client.control) { diff --git a/src/interactive/notebookOutput.ts b/src/interactive/notebookOutput.ts index f7e508fe6..6de17864c 100644 --- a/src/interactive/notebookOutput.ts +++ b/src/interactive/notebookOutput.ts @@ -3,33 +3,74 @@ import type { StreamSnapshot, StreamText } from './streamText'; const streams = new WeakMap(); +interface DeliveryToken { + generation: number; + revision: number; +} + /** Text is encoded only after a render is confirmed current and ready to write. */ export function prepareNotebookStream(output: NotebookCellOutput, snapshot: StreamSnapshot): void { streams.set(output, snapshot); } export class NotebookOutputState { + private generation = 0; + private revisions = new WeakMap(); private delivered = new WeakMap< StreamText, - { length: number; output: NotebookCellOutput; data: Uint8Array } + { length: number; output: WeakRef; data: WeakRef } >(); get( snapshot: StreamSnapshot, ): { length: number; output: NotebookCellOutput; data: Uint8Array } | undefined { - return this.delivered.get(snapshot.source); + const delivered = this.delivered.get(snapshot.source); + const output = delivered?.output.deref(), + data = delivered?.data.deref(); + return output && data ? { length: delivered!.length, output, data } : undefined; + } + + capture(snapshot: StreamSnapshot): DeliveryToken { + return { + generation: this.generation, + revision: this.revisions.get(snapshot.source) ?? 0, + }; } - set(snapshot: StreamSnapshot, output: NotebookCellOutput): void { + set( + snapshot: StreamSnapshot, + output: NotebookCellOutput, + token = this.capture(snapshot), + ): void { + if ( + token.generation !== this.generation || + token.revision !== (this.revisions.get(snapshot.source) ?? 0) + ) { + return; + } + const item = output.items.length === 1 ? output.items[0] : undefined; + if (!item) { + // Output can be cleared while an awaited notebook write completes. + this.delivered.delete(snapshot.source); + return; + } this.delivered.set(snapshot.source, { length: snapshot.length, - output, - data: output.items[0].data, + // Transcript history outlives removed cells and replaced buffers. + output: new WeakRef(output), + data: new WeakRef(item.data), }); } reset(): void { + this.generation++; this.delivered = new WeakMap(); + this.revisions = new WeakMap(); + } + + forget(stream: StreamText): void { + this.delivered.delete(stream); + this.revisions.set(stream, (this.revisions.get(stream) ?? 0) + 1); } } @@ -55,6 +96,12 @@ export async function updateNotebookOutputs( state = new NotebookOutputState(), readOutputs?: () => readonly NotebookCellOutput[], ): Promise { + const tokens = new Map( + next.flatMap((output) => { + const snapshot = streams.get(output); + return snapshot ? [[snapshot.source, state.capture(snapshot)] as const] : []; + }), + ); const compatible = previous.length <= next.length && previous.every( @@ -70,7 +117,7 @@ export async function updateNotebookOutputs( next.forEach((output, index) => { const snapshot = streams.get(output); if (snapshot) { - state.set(snapshot, readOutputs?.()[index] ?? output); + state.set(snapshot, readOutputs?.()[index] ?? output, tokens.get(snapshot.source)); } }); return; @@ -96,7 +143,7 @@ export async function updateNotebookOutputs( output, ); } - state.set(snapshot, readOutputs?.()[index] ?? output); + state.set(snapshot, readOutputs?.()[index] ?? output, tokens.get(snapshot.source)); continue; } materialize(next[index]); @@ -112,7 +159,7 @@ export async function updateNotebookOutputs( await execution.replaceOutputItems(items, output); } if (snapshot) { - state.set(snapshot, readOutputs?.()[index] ?? output); + state.set(snapshot, readOutputs?.()[index] ?? output, tokens.get(snapshot.source)); } } if (next.length > previous.length) { @@ -122,7 +169,11 @@ export async function updateNotebookOutputs( added.forEach((output, index) => { const snapshot = streams.get(output); if (snapshot) { - state.set(snapshot, readOutputs?.()[previous.length + index] ?? output); + state.set( + snapshot, + readOutputs?.()[previous.length + index] ?? output, + tokens.get(snapshot.source), + ); } }); } diff --git a/src/session.ts b/src/session.ts index 53b23e387..b96681c93 100644 --- a/src/session.ts +++ b/src/session.ts @@ -377,6 +377,16 @@ export function updateSessionWorkspace(target: Session, data: WorkspaceData): vo } } +export function invalidateWorkspaceChildren(target: Session): void { + if ( + sessions.get(target.sessionId) === target && + isConnectedSession(target) && + !target.workspaceUnavailable + ) { + rWorkspace?.invalidateChildren(target); + } +} + /** Move document routing to a new process; existing data viewers keep their old owner. */ export function replaceSessionTransport(previous: Session, next: Session): void { workspaceRefreshes.get(previous)?.dispose(); @@ -2282,12 +2292,21 @@ function workspaceRefreshFor(target: Session): WorkspaceRefreshScheduler updateSessionWorkspace(target, data), + (data) => { + if (data === target.workspaceData) { + invalidateWorkspaceChildren(target); + } else { + updateSessionWorkspace(target, data); + } + }, () => sessions.get(target.sessionId) === target && !target.workspaceUnavailable && diff --git a/src/test/node/incrementalOutput.test.ts b/src/test/node/incrementalOutput.test.ts index e50f06d4d..812bf475b 100644 --- a/src/test/node/incrementalOutput.test.ts +++ b/src/test/node/incrementalOutput.test.ts @@ -174,6 +174,69 @@ suite('Incremental Interactive console delivery', () => { assert.strictEqual(Buffer.from(model.outputs[0].items[0].data).toString(), 'first second'); }); + test('removing a console output releases its delivery even while an append is pending', async () => { + const model = notebook(), + state = new NotebookOutputState(); + const removed = new StreamText('removed'), + retained = new StreamText('retained'); + await updateNotebookOutputs( + model.execution, + [], + [streamOutput(removed, 'removed'), streamOutput(retained, 'retained')], + state, + ); + let release!: () => void; + const blocked = new Promise((resolve) => { + release = resolve; + }); + const append = model.execution.appendOutputItems.bind(model.execution); + model.execution.appendOutputItems = async (items, output) => { + await blocked; + await append(items, output); + }; + removed.append(' tail'); + const pending = updateNotebookOutputs( + model.execution, + [...model.outputs], + [streamOutput(removed, 'removed'), streamOutput(retained, 'retained')], + state, + ); + state.forget(removed); + assert.strictEqual(state.get(removed.snapshot()), undefined); + assert.ok(state.get(retained.snapshot())); + release(); + await pending; + assert.strictEqual( + state.get(removed.snapshot()), + undefined, + 'Late acknowledgements must not retain a removed buffer again', + ); + assert.ok( + state.get(retained.snapshot()), + 'Other outputs retain their incremental delivery', + ); + }); + + test('cleared output items cannot be acknowledged after a notebook append', async () => { + const model = notebook(), + state = new NotebookOutputState(), + text = new StreamText('before'); + await updateNotebookOutputs(model.execution, [], [streamOutput(text)], state); + model.execution.appendOutputItems = (_items, output) => { + output.items = []; + return Promise.resolve(); + }; + text.append(' after'); + await updateNotebookOutputs( + model.execution, + [...model.outputs], + [streamOutput(text)], + state, + () => model.outputs, + ); + assert.strictEqual(state.get(text.snapshot()), undefined); + }); + test('acknowledges normalized progress output without re-encoding its raw prefix', async () => { const model = notebook(), state = new NotebookOutputState(), diff --git a/src/test/node/interactiveBackend.test.ts b/src/test/node/interactiveBackend.test.ts index 192f12eb3..61fd303cb 100644 --- a/src/test/node/interactiveBackend.test.ts +++ b/src/test/node/interactiveBackend.test.ts @@ -245,14 +245,64 @@ suite('Interactive backend configuration', () => { let calls = 0; const previous = (await client.snapshot()).workspace; const published = events.filter((event) => event.type === 'workspace').length; + const invalidations = events.filter( + (event) => event.type === 'workspaceInvalidated', + ).length; backend.inspectResult = () => { calls++; return Promise.resolve({ revision: 1, unchanged: true }); }; - backend.emit({ type: 'workspaceChanged' }); + for (let i = 0; i < 20; i++) { + backend.emit({ type: 'workspaceChanged' }); + } await until(() => calls === 1); + await until( + () => + events.filter((event) => event.type === 'workspaceInvalidated').length === + invalidations + 1, + ); assert.deepStrictEqual((await client.snapshot()).workspace, previous); assert.strictEqual(events.filter((event) => event.type === 'workspace').length, published); + assert.deepStrictEqual( + events.find((event) => event.type === 'workspaceInvalidated')!.data, + {}, + ); + }); + + test('equal legacy workspace snapshots still invalidate expanded child values', async () => { + const previous = (await client.snapshot()).workspace; + const invalidations = events.filter( + (event) => event.type === 'workspaceInvalidated', + ).length; + backend.inspectResult = () => Promise.resolve(previous); + backend.emit({ type: 'workspaceChanged' }); + await until( + () => + events.filter((event) => event.type === 'workspaceInvalidated').length === + invalidations + 1, + ); + assert.deepStrictEqual((await client.snapshot()).workspace, previous); + }); + + test('manual workspace inspection invalidates children when metadata is unchanged', async () => { + const previous = (await client.snapshot()).workspace; + const published = events.filter((event) => event.type === 'workspace').length; + const invalidations = events.filter( + (event) => event.type === 'workspaceInvalidated', + ).length; + let calls = 0; + backend.inspectResult = () => { + calls++; + return Promise.resolve({ revision: 1, unchanged: true }); + }; + assert.deepStrictEqual(await client.request('inspect', { method: 'workspace' }), previous); + await until( + () => + events.filter((event) => event.type === 'workspaceInvalidated').length === + invalidations + 1, + ); + assert.strictEqual(calls, 1); + assert.strictEqual(events.filter((event) => event.type === 'workspace').length, published); }); test('handles completion before transport acknowledgement and deduplicates submissions', async () => { diff --git a/src/test/node/journalEfficiency.test.ts b/src/test/node/journalEfficiency.test.ts index 43e2e5c33..c29aa607d 100644 --- a/src/test/node/journalEfficiency.test.ts +++ b/src/test/node/journalEfficiency.test.ts @@ -108,6 +108,48 @@ suite('Indexed journal working sets', () => { } }); + test('indexes prototype-named execution IDs after a cold persisted-index reopen', () => { + const original = path.join(root, 'original'), + directory = path.join(root, 'cold'); + let journal = new SessionJournal(original, 'generation'); + journal.accept({ id: 'old', code: '1' }); + journal.append('stream', { text: 'old output' }, 'old'); + journal.close(); + fs.cpSync(original, directory, { recursive: true }); + // Keep a valid persisted index while using a path outside the hot cache. + const file = path.join(directory, 'events-index.json'); + const saved = JSON.parse(fs.readFileSync(file, 'utf8')) as { data: string; hash: string }; + const index = JSON.parse(saved.data) as { segments: { name: string; mtime: number }[] }; + for (const segment of index.segments) { + segment.mtime = fs.statSync(path.join(directory, segment.name)).mtimeMs; + } + saved.data = JSON.stringify(index); + saved.hash = createHash('sha256').update(saved.data).digest('hex'); + fs.writeFileSync(file, JSON.stringify(saved)); + journal = new SessionJournal(directory, 'generation'); + try { + const ids = ['__proto__', 'constructor', 'toString']; + for (const id of ids) { + const record = journal.accept({ id, code: id }).record; + journal.append('accepted', { record }, id); + journal.append('stream', { text: id }, id); + } + const recent = journal.recent(3); + assert.deepStrictEqual( + recent.executions.map((record) => record.id), + ids, + ); + assert.deepStrictEqual( + recent.events + .filter((event) => event.type === 'stream') + .map((event) => event.data.text), + ids, + ); + } finally { + journal.close(); + } + }); + test('reads legacy archived records and output asynchronously without repairing files', async () => { const directory = path.join(root, 'generation'); const journal = new SessionJournal(directory, 'generation'); diff --git a/src/test/node/messagePages.test.ts b/src/test/node/messagePages.test.ts index a074af6db..663d5faf2 100644 --- a/src/test/node/messagePages.test.ts +++ b/src/test/node/messagePages.test.ts @@ -8,7 +8,13 @@ import * as sinon from 'sinon'; import { SessionAgent } from '../../interactive/agent'; import { AgentClient } from '../../interactive/client'; import { BackendEvent, ClientReply, SessionBackend } from '../../interactive/backend'; -import { AgentConfig, SessionEvent, SessionManifest, Submission } from '../../interactive/protocol'; +import { + AgentConfig, + MAX_CODE_BYTES, + SessionEvent, + SessionManifest, + Submission, +} from '../../interactive/protocol'; import { JsonLines } from '../../interactive/framing'; import { MessagePages, boundedJson } from '../../interactive/messagePages'; @@ -185,6 +191,72 @@ suite('Bounded message pages', () => { } }); + test('restores active escaped code after reconnect when its JSON exceeds the history allowance', async () => { + const backend = new BackendFixture(); + const globalenv: Record = {}; + for (let i = 0; i < 13000; i++) { + globalenv['object_' + i] = { type: 'integer', str: 'x'.repeat(200) }; + } + backend.workspace = { globalenv, revision: 1 }; + const dispatch = sinon.spy(backend, 'dispatch'); + const config: AgentConfig = { + id: randomUUID(), + generation: randomUUID(), + label: 'escaped code', + directory: root, + storage: path.join(root, 'escaped-storage'), + provider: 'r', + supervision: 'test', + historyLimit: 50, + maxOutputBytes: 4 * 1024 * 1024, + maxJournalBytes: 32 * 1024 * 1024, + }; + const agent = new SessionAgent(config, () => backend); + const manifest = await agent.start(); + const client = new AgentClient(manifest); + const reconnected = new AgentClient(manifest); + try { + await client.connect(); + await client.request('inspect', { method: 'workspace' }); + const id = randomUUID(); + // Quotes and newlines make valid raw source much larger in JSON. + const code = '"a"\n'.repeat(MAX_CODE_BYTES / 4); + assert.strictEqual(Buffer.byteLength(code), MAX_CODE_BYTES); + await client.request('submit', { submission: { id, code } }); + client.close(); + await reconnected.connect({ claim: false }); + let paged = false; + const parser = new JsonLines((message) => { + paged ||= message.resultPage !== undefined; + }); + (reconnected as unknown as { socket: net.Socket }).socket.on( + 'data', + (chunk: Buffer) => parser.push(chunk), + ); + const snapshot = await reconnected.snapshot(1); + const active = snapshot.executions.find((record) => record.id === id); + assert.ok(active, 'Reconnect must preserve the admitted active execution'); + assert.strictEqual(active.code, code); + assert.strictEqual(active.state, 'running'); + assert.ok(Buffer.byteLength(JSON.stringify(active)) > 1400 * 1024); + assert.strictEqual(snapshot.manifest.status, 'busy'); + assert.strictEqual( + Object.keys(snapshot.workspace!.globalenv as object).length, + 13000, + ); + assert.ok( + paged, + 'The complete envelope must be paged rather than dropping active code', + ); + assert.ok(reconnected.connected); + sinon.assert.calledOnce(dispatch); + } finally { + client.close(); + reconnected.close(); + await agent.close(); + } + }); + test('delivers a large live event before later small events in the same socket read', async () => { const manifest = { id: 'events', diff --git a/src/test/node/workspaceRefresh.test.ts b/src/test/node/workspaceRefresh.test.ts index f427b8886..d6e29f054 100644 --- a/src/test/node/workspaceRefresh.test.ts +++ b/src/test/node/workspaceRefresh.test.ts @@ -79,6 +79,31 @@ suite('Workspace refresh scheduling', () => { assert.deepStrictEqual(published, []); }); + test('coalesces successful unchanged reads and suppresses invalidated child notifications', async () => { + const cached = { version: 1 }; + const first = deferred(); + let reads = 0; + let invalidations = 0; + const scheduler = new WorkspaceRefreshScheduler( + () => (++reads === 1 ? first.promise : Promise.resolve(cached)), + (value) => { + assert.strictEqual(value, cached); + invalidations++; + }, + () => true, + ); + const running = scheduler.ensure(); + await Promise.resolve(); + for (let i = 0; i < 20; i++) { + scheduler.invalidate(); + } + first.resolve(cached); + await running; + assert.strictEqual(reads, 2); + assert.strictEqual(invalidations, 1); + scheduler.dispose(); + }); + test('deferred sends keep invalidation and inspection failures do not spin', async () => { let calls = 0; const errors: unknown[] = []; diff --git a/src/test/suite/interactiveEditor.test.ts b/src/test/suite/interactiveEditor.test.ts index 5e6197d81..89ff5d79e 100644 --- a/src/test/suite/interactiveEditor.test.ts +++ b/src/test/suite/interactiveEditor.test.ts @@ -15,6 +15,7 @@ import { InteractiveSerializer, DISPLAY_MIME } from '../../interactive/notebook' import { AgentConfig, SessionManifest } from '../../interactive/protocol'; import type { SessionEvent } from '../../interactive/protocol'; import type { Transcript, TranscriptCell } from '../../interactive/transcript'; +import type { NotebookOutputState } from '../../interactive/notebookOutput'; import { AssetStorageStats, exportedAssetName, readAsset } from '../../interactive/assets'; import { resolveExecutable } from '../../interactive/executable'; import { shellQuote } from '../../interactive/launcher'; @@ -3489,7 +3490,34 @@ par(mfrow=c(1,1))`, document: notebook.uri, }); await until(() => manifests[0].status === 'busy'); + const manager = interactiveManager() as unknown as { + views: Map< + string, + { + client: AgentClient; + model: Transcript; + outputState?: NotebookOutputState; + } + >; + }; + const view = [...manager.views.values()].find( + (view) => view.client.manifest.id === manifests[0].id, + ); + assert.ok(view); + const removedStreams = [...view.model.cells.values()] + .filter((cell) => !['queued', 'running', 'unknown'].includes(cell.record.state)) + .flatMap((cell) => + cell.outputs.flatMap((output) => (output.stream ? [output.stream] : [])), + ); + assert.ok(removedStreams.length, 'Exercise clearing completed console output'); await vscode.commands.executeCommand('r.interactive.clear', notebook.uri); + for (const stream of removedStreams) { + assert.strictEqual( + view.outputState?.get(stream.snapshot()), + undefined, + 'Clearing cells must release their notebook delivery handles and buffers', + ); + } assert.ok( notebook.getCells().some((cell) => cell.document.getText() === '# unfinished draft'), ); diff --git a/src/test/suite/sessionWorkspaceRefresh.test.ts b/src/test/suite/sessionWorkspaceRefresh.test.ts index e4d98671d..cceb635a6 100644 --- a/src/test/suite/sessionWorkspaceRefresh.test.ts +++ b/src/test/suite/sessionWorkspaceRefresh.test.ts @@ -13,6 +13,7 @@ suite('Session workspace refresh demand', () => { let sandbox: sinon.SinonSandbox; let connections: SessionConnections; let foreground: session.Session; + let workspace: sinon.SinonStub; const extensionRoot = path.join(__dirname, '..', '..', '..'); setup(async () => { @@ -21,7 +22,7 @@ suite('Session workspace refresh demand', () => { connections = new SessionConnections(); mockExtensionContext(extensionRoot, sandbox); sandbox.stub(extension, 'enableSessionWatcher').value(true); - sandbox.stub(extension, 'rWorkspace').value(undefined); + workspace = sandbox.stub(extension, 'rWorkspace').value(undefined); sandbox.stub(vscode.commands, 'executeCommand').resolves(); sandbox.stub(vscode.window, 'terminals').value([]); sandbox.stub(vscode.window, 'activeTerminal').value(undefined); @@ -107,4 +108,50 @@ suite('Session workspace refresh demand', () => { session.unbindSessionDocument(uri); } }); + + test('native unchanged replies invalidate child values once after a notification burst', async () => { + const socket = await connections.attach( + 'workspace-native-unchanged', + 55102, + 'external-host', + { + autoActivate: false, + attachmentId: 'workspace-native-unchanged-fixture', + }, + ); + await session.activateSessionById('workspace-native-unchanged'); + session.deferWorkspaceRefresh(); + const target = session.activeSession!; + target.workspaceData = { revision: 1, globalenv: {}, search: [], loaded_namespaces: [] }; + const metadata = target.workspaceData; + const invalidateChildren = sandbox.spy(); + const refresh = sandbox.spy(); + workspace.value({ invalidateChildren, refresh }); + let requests = 0; + sandbox.stub(socket, 'write').callsFake((data: string | Uint8Array) => { + const message = JSON.parse(data.toString()) as { id: number; method?: string }; + if (message.method === 'workspace') { + requests++; + socket.emit( + 'data', + Buffer.from( + `${JSON.stringify({ + jsonrpc: '2.0', + id: message.id, + result: { revision: 1, unchanged: true }, + })}\n`, + ), + ); + } + return true; + }); + for (let i = 0; i < 20; i++) { + socket.emit('data', Buffer.from('{"jsonrpc":"2.0","method":"workspace_updated"}\n')); + } + await session.ensureSessionWorkspace(target); + assert.strictEqual(requests, 1); + sinon.assert.calledOnceWithExactly(invalidateChildren, target); + sinon.assert.notCalled(refresh); + assert.strictEqual(target.workspaceData, metadata); + }); }); diff --git a/src/test/suite/workspaceViewer.test.ts b/src/test/suite/workspaceViewer.test.ts index 39f9d941a..f88704146 100644 --- a/src/test/suite/workspaceViewer.test.ts +++ b/src/test/suite/workspaceViewer.test.ts @@ -150,6 +150,34 @@ suite('Workspace Viewer', () => { ); }); + test('unchanged metadata refresh reloads child values without replacing the workspace', async () => { + session.updateSessionWorkspace(first, { ...data('mutable'), revision: 1 }); + const metadata = first.workspaceData; + let value = 1; + let childQueries = 0; + first.requester = (request) => { + if (request.method === 'workspace') { + return Promise.resolve({ revision: 1, unchanged: true }); + } + childQueries++; + return Promise.resolve({ + children: [ + { str: `int ${value}`, class: 'integer', type: 'integer', has_children: false }, + ], + }); + }; + const node = (await envNodes())[0]; + assert.strictEqual((await provider.getChildren(node))[0].description, 'int 1'); + value = 2; + session.invalidateWorkspaceChildren(second); + assert.strictEqual((await provider.getChildren(node))[0].description, 'int 1'); + assert.strictEqual(childQueries, 1); + await session.updateWorkspace(); + assert.strictEqual(first.workspaceData, metadata); + assert.strictEqual((await provider.getChildren(node))[0].description, 'int 2'); + assert.strictEqual(childQueries, 2); + }); + test('readiness updates reenable workspace actions without another focus event', async () => { first.workspaceUnavailable = 'R is starting…'; session.updateSessionWorkspace(first, first.workspaceData); diff --git a/src/workspaceViewer.ts b/src/workspaceViewer.ts index dc5122555..f498165be 100644 --- a/src/workspaceViewer.ts +++ b/src/workspaceViewer.ts @@ -99,6 +99,17 @@ export class WorkspaceDataProvider implements TreeDataProvider { this._onDidChangeTreeData.fire(undefined); } + /** Values can change without changing the top-level structural metadata. */ + public invalidateChildren(owner: Session): void { + if (this.owner !== owner) { + return; + } + this.childPageGeneration++; + this.childPages.clear(); + this.childPageLoads.clear(); + this._onDidChangeTreeData.fire(this.globalEnvRootItem); + } + public constructor() { this.attachedNamespacesRootItem = new TreeItem( 'Attached Namespaces', From 72cf1ed539a5ef5e4c26f3e9b12dbf96c082519a Mon Sep 17 00:00:00 2001 From: Kun Ren Date: Sun, 25 Oct 2026 11:46:37 +0800 Subject: [PATCH 3/4] test: stabilize consent scans and isolate editor command fixtures --- .vscode-test.mjs | 17 +++- R/tests/attach_sess.R | 75 +++++++++++++--- src/test/common/editorState.ts | 49 +++++++++++ src/test/suite/completionMiddleware.test.ts | 15 +++- .../suite/interactiveInputFeatures.test.ts | 85 ++++++++++--------- src/test/suite/rmarkdown.test.ts | 4 + 6 files changed, 193 insertions(+), 52 deletions(-) create mode 100644 src/test/common/editorState.ts diff --git a/.vscode-test.mjs b/.vscode-test.mjs index 6734a69ed..d022829a0 100644 --- a/.vscode-test.mjs +++ b/.vscode-test.mjs @@ -22,8 +22,9 @@ const common = { } }; -// Keep mock/provider tests independent of R language-server startup. The real -// Interactive integration suite has its own window with the built-in LSP enabled. +// Keep mock/provider tests independent of R language-server startup. Editor +// command tests also need a fresh window after fixtures create terminals and +// webviews. The real Interactive suite has its own window with the LSP enabled. function testProfile(name, lspEnabled) { const directory = path.resolve('.vscode-test/user-data', name); mkdirSync(path.join(directory, 'User'), { recursive: true }); @@ -35,10 +36,20 @@ export default defineConfig([ { ...common, label: 'provider-tests', - files: 'out/test/suite/**/!(interactiveEditor).test.js', + files: 'out/test/suite/**/!(interactiveEditor|interactiveInputFeatures|completionMiddleware).test.js', mocha: { ...common.mocha, require: './out/test/common/extensionSetup.js' }, launchArgs: testProfile('provider-tests', false) }, + { + ...common, + label: 'editor-command-tests', + files: [ + 'out/test/suite/interactiveInputFeatures.test.js', + 'out/test/suite/completionMiddleware.test.js' + ], + mocha: { ...common.mocha, require: './out/test/common/extensionSetup.js' }, + launchArgs: testProfile('editor-command-tests', false) + }, { ...common, label: 'interactive-integration', diff --git a/R/tests/attach_sess.R b/R/tests/attach_sess.R index bbb4b7b47..e528cb52a 100644 --- a/R/tests/attach_sess.R +++ b/R/tests/attach_sess.R @@ -414,9 +414,13 @@ writeLines(c( "while (length(ids) < count && Sys.time() < deadline) {", " requests <- list.files(directory, pattern='\\\\.request$', full.names=TRUE)", " for (request in requests) {", - " lines <- readLines(request, warn=FALSE)", - " if (length(lines) != 5L || lines[[1L]] != 'vscode-r-sess-consent-v1') stop('bad request')", - " id <- lines[[2L]]; if (id %in% ids) next", + " id <- sub('\\\\.request$', '', basename(request))", + " if (id %in% ids) next", + " lines <- suppressWarnings(tryCatch(readLines(request, warn=FALSE),", + " error=function(e) { if (!file.exists(request)) NULL else stop(e) }))", + " if (is.null(lines)) next", + " if (length(lines) != 5L || lines[[1L]] != 'vscode-r-sess-consent-v1' ||", + " lines[[2L]] != id) stop('bad request')", " if (nzchar(wait_marker) && !file.exists(wait_marker)) next", " ids <- c(ids, id); temporary <- tempfile(tmpdir=directory)", " identities <- c(identities, lines[[4L]])", @@ -609,7 +613,7 @@ run_case <- function(mode) { if (!file.exists(status_path)) stop(paste("consent agent did not finish:", mode)) agent_status <- readLines(status_path, warn = FALSE) if (!identical(agent_status, "ok")) { - stop(paste("consent agent failed:", paste(agent_status, collapse = " "))) + stop(paste("consent agent failed in", mode, ":", paste(agent_status, collapse = " "))) } log_path <- file.path(case_root, "consent-ids") ids <- readLines(log_path, warn = FALSE) @@ -647,7 +651,8 @@ wait_for_request <- function(directory, seconds) { requests[[1L]] } -start_consent_agent <- function(case_root, answer, wait_marker = "", label = "consent") { +start_consent_agent <- function(case_root, answer, wait_marker = "", label = "consent", + count = 1L, script = agent) { consent <- file.path(case_root, "consent") log <- file.path(case_root, paste0(label, "-ids")) identity_log <- file.path(case_root, paste0(label, "-identities")) @@ -657,8 +662,8 @@ start_consent_agent <- function(case_root, answer, wait_marker = "", label = "co system2( r_binary, shQuote(c( - "--vanilla", "--slave", paste0("--file=", agent), "--args", - consent, "1", answer, log, identity_log, status, wait_marker + "--vanilla", "--slave", paste0("--file=", script), "--args", + consent, as.character(count), answer, log, identity_log, status, wait_marker )), wait = FALSE, stdout = stdout, @@ -668,7 +673,7 @@ start_consent_agent <- function(case_root, answer, wait_marker = "", label = "co stdout = stdout, stderr = stderr) } -wait_for_agent <- function(paths) { +wait_for_agent <- function(paths, count = 1L) { wait_for_file(paths$status, 10, "consent-agent completion") result <- readLines(paths$status, warn = FALSE) if (!identical(result, "ok")) { @@ -676,17 +681,67 @@ wait_for_agent <- function(paths) { } ids <- readLines(paths$log, warn = FALSE) identities <- readLines(paths$identity_log, warn = FALSE) - stopifnot(length(ids) == 1L, grepl("^[A-Za-z0-9_-]{16,64}$", ids)) + stopifnot(length(ids) == count, !anyDuplicated(ids), + all(grepl("^[A-Za-z0-9_-]{16,64}$", ids))) minor <- strsplit(R.version$minor, ".", fixed = TRUE)[[1L]][1L] expected_runtime <- paste( R.version$platform, paste(R.version$major, minor, sep = "."), sep = "|" ) - stopifnot(identical(identities, expected_runtime)) + stopifnot(identical(identities, rep(expected_runtime, count))) ids } +# Force the list/read race instead of relying on platform scheduling: one request +# vanishes after listing, and a handled request remains while the next arrives. +scan_root <- file.path(root, "consent-scan-race") +scan_consent <- file.path(scan_root, "consent") +dir.create(scan_consent, recursive = TRUE) +expired_id <- paste0(strrep("0", 16), "expired") +first_id <- strrep("a", 20) +second_id <- strrep("b", 20) +expired_request <- file.path(scan_consent, paste0(expired_id, ".request")) +first_request <- file.path(scan_consent, paste0(first_id, ".request")) +second_request <- file.path(scan_consent, paste0(second_id, ".request")) +first_response <- file.path(scan_consent, paste0(first_id, ".response")) +scan_reads <- file.path(scan_root, "first-request-reads") +request_lines <- function(id) { + c("vscode-r-sess-consent-v1", id, revision, sess_runtime_identity(), "missing") +} +writeLines(request_lines(expired_id), expired_request) +writeLines(request_lines(first_id), first_request) +scan_agent <- file.path(scan_root, "raced-agent.R") +writeLines(c( + paste0("expired <- ", r_literal(expired_request)), + paste0("first <- ", r_literal(first_request)), + paste0("second <- ", r_literal(second_request)), + paste0("response <- ", r_literal(first_response)), + "reads <- 0L", + "list.files <- function(...) {", + paste0(" if (file.exists(response) && !file.exists(second)) writeLines(", + "c(", paste(vapply(request_lines(second_id), r_literal, ""), collapse = ", "), + "), second)"), + " files <- base::list.files(...)", + " if (expired %in% files) unlink(expired)", + " files", + "}", + "readLines <- function(con, ...) {", + " if (identical(con, first)) {", + " reads <<- reads + 1L", + " if (reads > 1L) stop('handled request read twice')", + " }", + " base::readLines(con, ...)", + "}", + paste0("source(", r_literal(agent), ")"), + paste0("writeLines(as.character(reads), ", r_literal(scan_reads), ")") +), scan_agent) +scan_paths <- start_consent_agent(scan_root, "decline", count = 2L, script = scan_agent) +scan_ids <- wait_for_agent(scan_paths, count = 2L) +wait_for_file(scan_reads, 5, "consent scan fixture completion") +stopifnot(identical(scan_ids, c(first_id, second_id)), !file.exists(expired_request), + identical(readLines(scan_reads), "1")) + start_parallel_child <- function(case_root, mode, role, result_file, install_helper = "") { environment_names <- c("VSCODE_R_TEST_INSTALL_HELPER", "VSCODE_R_TEST_REQUEST_LOG") previous <- Sys.getenv(environment_names, unset = NA_character_) diff --git a/src/test/common/editorState.ts b/src/test/common/editorState.ts new file mode 100644 index 000000000..207d74fd5 --- /dev/null +++ b/src/test/common/editorState.ts @@ -0,0 +1,49 @@ +import * as assert from 'assert'; +import * as vscode from 'vscode'; + +export async function waitForEditorState(predicate: () => boolean, message: string): Promise { + const deadline = Date.now() + 5000; + while (!predicate()) { + assert.ok(Date.now() < deadline, message); + await new Promise((resolve) => setTimeout(resolve, 10)); + } +} + +/** Selection setters are asynchronous RPCs; wait for the workbench round trip. */ +export async function acknowledgeEditorSelection( + editor: vscode.TextEditor, + selection: vscode.Selection, +): Promise { + editor.selection = selection; + // The command is processed after the selection RPC, even if the requested + // selection is unchanged and no selection-change notification is emitted. + await vscode.commands.executeCommand('revealLine', { + lineNumber: selection.active.line, + at: 'simple', + }); + await waitForEditorState( + () => + vscode.window.activeTextEditor === editor && + editor.selections.length === 1 && + editor.selection.isEqual(selection), + 'Timed out waiting for the focused editor selection', + ); +} + +/** Native editor commands can return before their document changes are observed. */ +export async function executeEditorCommand( + editor: vscode.TextEditor, + command: 'type' | 'paste', + text: string, +): Promise { + const version = editor.document.version; + await vscode.commands.executeCommand(command, { text }); + await waitForEditorState( + () => editor.document.version > version, + `The ${command} command did not change the intended document`, + ); + await vscode.commands.executeCommand('revealLine', { + lineNumber: editor.selection.active.line, + at: 'simple', + }); +} diff --git a/src/test/suite/completionMiddleware.test.ts b/src/test/suite/completionMiddleware.test.ts index 9470022b8..b66ecef63 100644 --- a/src/test/suite/completionMiddleware.test.ts +++ b/src/test/suite/completionMiddleware.test.ts @@ -4,6 +4,7 @@ import { PassThrough } from 'stream'; import * as vscode from 'vscode'; import * as lsp from 'vscode-languageclient/node'; import { completionMiddleware } from '../../completionMiddleware'; +import { acknowledgeEditorSelection } from '../common/editorState'; suite('Completion signature help', () => { let sandbox: sinon.SinonSandbox; @@ -227,6 +228,7 @@ suite('Completion signature help acceptance', () => { const toClient = new PassThrough(); const server = lsp.createProtocolConnection(toServer, toClient); const signatures: lsp.SignatureHelpParams[] = []; + const openedDocuments = new Set(); let resolutions = 0; server.onRequest(lsp.InitializeRequest.type, (): lsp.InitializeResult => ({ capabilities: { @@ -256,6 +258,9 @@ suite('Completion signature help acceptance', () => { }; }); server.onRequest(lsp.ShutdownRequest.type, () => undefined); + server.onNotification(lsp.DidOpenTextDocumentNotification.type, (params) => { + openedDocuments.add(params.textDocument.uri); + }); server.listen(); const client = new lsp.LanguageClient( 'completion-acceptance-test', @@ -273,7 +278,14 @@ suite('Completion signature help acceptance', () => { content: 'fun', }); const editor = await vscode.window.showTextDocument(document); - editor.selection = new vscode.Selection(0, 3, 0, 3); + await until(() => openedDocuments.has(document.uri.toString())); + await until( + () => + vscode.workspace + .getConfiguration('editor', document) + .get('parameterHints.enabled') === enabled, + ); + await acknowledgeEditorSelection(editor, new vscode.Selection(0, 3, 0, 3)); await vscode.commands.executeCommand('editor.action.triggerSuggest'); await until(() => resolutions > 0); assert.strictEqual( @@ -291,6 +303,7 @@ suite('Completion signature help acceptance', () => { await new Promise((resolve) => setTimeout(resolve, 100)); assert.strictEqual(signatures.length, 0); } + await until(() => editor.selection.active.character === 5); assert.strictEqual(editor.selection.active.character, 5); } finally { await vscode.commands.executeCommand('closeParameterHints'); diff --git a/src/test/suite/interactiveInputFeatures.test.ts b/src/test/suite/interactiveInputFeatures.test.ts index 4c4d8fb0e..56ac77154 100644 --- a/src/test/suite/interactiveInputFeatures.test.ts +++ b/src/test/suite/interactiveInputFeatures.test.ts @@ -3,11 +3,13 @@ import * as path from 'path'; import { createRequire } from 'module'; import * as vscode from 'vscode'; import type { InteractiveManager } from '../../interactive/manager'; +import { acknowledgeEditorSelection, executeEditorCommand } from '../common/editorState'; suite('Interactive input formatting with independent providers', () => { let editor: vscode.TextEditor; let controller: vscode.NotebookController; let notebook: vscode.NotebookDocument; + let inputUri: vscode.Uri; let originalTabs: Set; let providers: vscode.Disposable[]; let previousOnType: boolean | undefined; @@ -23,11 +25,14 @@ suite('Interactive input formatting with independent providers', () => { } }; const replaceInput = async (text: string): Promise => { - await editor.edit((builder) => - builder.replace(new vscode.Range(0, 0, editor.document.lineCount, 0), text), + assert.ok( + await editor.edit((builder) => + builder.replace(new vscode.Range(0, 0, editor.document.lineCount, 0), text), + ), ); + await until(() => editor.document.getText() === text); const end = editor.document.positionAt(text.length); - editor.selection = new vscode.Selection(end, end); + await acknowledgeEditorSelection(editor, new vscode.Selection(end, end)); }; const focusInput = async (): Promise => { await vscode.commands.executeCommand( @@ -36,8 +41,25 @@ suite('Interactive input formatting with independent providers', () => { notebook.uri, ); await vscode.commands.executeCommand('interactive.input.focus'); - await until(() => vscode.window.activeTextEditor?.document.uri.scheme === selector.scheme); + await until( + () => vscode.window.activeTextEditor?.document.uri.toString() === inputUri.toString(), + ); editor = vscode.window.activeTextEditor!; + await acknowledgeEditorSelection(editor, editor.selection); + }; + const setFormatSetting = async ( + name: 'formatOnType' | 'formatOnPaste', + value: boolean | undefined, + ): Promise => { + await settings().update(name, value, vscode.ConfigurationTarget.Global, true); + await until(() => settings().inspect(name)?.globalLanguageValue === value); + if (value !== undefined) { + await until( + () => + vscode.workspace.getConfiguration('editor', editor.document).get(name) === + value, + ); + } }; const formatExpression = (document: vscode.TextDocument): vscode.TextEdit[] => [ vscode.TextEdit.replace( @@ -70,6 +92,7 @@ suite('Interactive input formatting with independent providers', () => { 'Independent formatting', ); notebook = await vscode.workspace.openNotebookDocument(opened.notebookUri); + inputUri = opened.inputUri; await vscode.languages.setTextDocumentLanguage( await vscode.workspace.openTextDocument(opened.inputUri), 'r', @@ -87,8 +110,8 @@ suite('Interactive input formatting with independent providers', () => { setup(async () => { providers = []; await focusInput(); - await settings().update('formatOnType', true, vscode.ConfigurationTarget.Global, true); - await settings().update('formatOnPaste', true, vscode.ConfigurationTarget.Global, true); + await setFormatSetting('formatOnType', true); + await setFormatSetting('formatOnPaste', true); await replaceInput(''); }); teardown(async () => { @@ -96,22 +119,12 @@ suite('Interactive input formatting with independent providers', () => { provider.dispose(); }); await focusInput(); - await settings().update('formatOnType', false, vscode.ConfigurationTarget.Global, true); + await setFormatSetting('formatOnType', false); await replaceInput(''); }); suiteTeardown(async () => { - await settings().update( - 'formatOnType', - previousOnType, - vscode.ConfigurationTarget.Global, - true, - ); - await settings().update( - 'formatOnPaste', - previousOnPaste, - vscode.ConfigurationTarget.Global, - true, - ); + await setFormatSetting('formatOnType', previousOnType); + await setFormatSetting('formatOnPaste', previousOnPaste); controller.dispose(); await vscode.window.tabGroups.close( vscode.window.tabGroups.all @@ -141,7 +154,7 @@ suite('Interactive input formatting with independent providers', () => { await replaceInput(''); const before = calls.length; for (const ch of text) { - await vscode.commands.executeCommand('type', { text: ch }); + await executeEditorCommand(editor, 'type', ch); } await until(() => editor.document.getText() === text.replace('1+1', '1 + 1')); assert.ok(calls.length > before); @@ -159,7 +172,7 @@ suite('Interactive input formatting with independent providers', () => { ); assert.deepStrictEqual(calls[0].options, { tabSize: 2, insertSpaces: true }); await replaceInput('(1+1'); - await vscode.commands.executeCommand('type', { text: ')' }); + await executeEditorCommand(editor, 'type', ')'); await until(() => editor.document.getText() === '(1 + 1)'); await vscode.commands.executeCommand('undo'); await until(() => editor.document.getText() === '(1+1'); @@ -179,14 +192,14 @@ suite('Interactive input formatting with independent providers', () => { ')', ), ); - await settings().update('formatOnType', false, vscode.ConfigurationTarget.Global, true); + await setFormatSetting('formatOnType', false); await replaceInput('(1+1'); - await vscode.commands.executeCommand('type', { text: ')' }); + await executeEditorCommand(editor, 'type', ')'); await new Promise((resolve) => setTimeout(resolve, 100)); assert.strictEqual(editor.document.getText(), '(1+1)'); assert.strictEqual(calls, 0); - await settings().update('formatOnType', true, vscode.ConfigurationTarget.Global, true); - await vscode.commands.executeCommand('type', { text: '!' }); + await setFormatSetting('formatOnType', true); + await executeEditorCommand(editor, 'type', '!'); await new Promise((resolve) => setTimeout(resolve, 100)); assert.strictEqual(calls, 0); }); @@ -215,14 +228,9 @@ suite('Interactive input formatting with independent providers', () => { }), ); for (const enabled of [false, true]) { - await settings().update( - 'formatOnPaste', - enabled, - vscode.ConfigurationTarget.Global, - true, - ); + await setFormatSetting('formatOnPaste', enabled); await replaceInput('(1+1'); - await vscode.commands.executeCommand('paste', { text: ')' }); + await executeEditorCommand(editor, 'paste', ')'); // On-type formatting runs asynchronously from document-change events. await new Promise((resolve) => setTimeout(resolve, 100)); assert.strictEqual( @@ -235,7 +243,7 @@ suite('Interactive input formatting with independent providers', () => { } assert.deepStrictEqual(ranges[0], new vscode.Range(0, 4, 0, 5)); await replaceInput('(1+1'); - await vscode.commands.executeCommand('type', { text: ')' }); + await executeEditorCommand(editor, 'type', ')'); await until(() => editor.document.getText() === '(1 + 1)'); assert.strictEqual(typeCalls, 1, 'Typing still invokes the on-type provider after paste'); }); @@ -250,7 +258,7 @@ suite('Interactive input formatting with independent providers', () => { }, }), ); - await vscode.commands.executeCommand('paste', { text: '(1+1)' }); + await executeEditorCommand(editor, 'paste', '(1+1)'); assert.strictEqual(editor.document.getText(), '(1 + 1)'); assert.deepStrictEqual(ranges[0], new vscode.Range(0, 0, 0, 5)); assert.strictEqual(editor.document.offsetAt(editor.selection.active), 7); @@ -258,17 +266,18 @@ suite('Interactive input formatting with independent providers', () => { await until(() => editor.document.getText() === '(1+1)'); await vscode.commands.executeCommand('undo'); await until(() => editor.document.getText() === ''); - await settings().update('formatOnPaste', false, vscode.ConfigurationTarget.Global, true); - await vscode.commands.executeCommand('paste', { text: '(1+1)' }); + await setFormatSetting('formatOnPaste', false); + await executeEditorCommand(editor, 'paste', '(1+1)'); assert.strictEqual(editor.document.getText(), '(1+1)'); assert.strictEqual(ranges.length, 1); const source = await vscode.workspace.openTextDocument({ language: 'plaintext', content: '', }); - await vscode.window.showTextDocument(source); + const sourceEditor = await vscode.window.showTextDocument(source); await until(() => vscode.window.activeTextEditor?.document === source); - await vscode.commands.executeCommand('paste', { text: '(1+1)' }); + await acknowledgeEditorSelection(sourceEditor, new vscode.Selection(0, 0, 0, 0)); + await executeEditorCommand(sourceEditor, 'paste', '(1+1)'); assert.strictEqual(source.getText(), '(1+1)'); await focusInput(); }); diff --git a/src/test/suite/rmarkdown.test.ts b/src/test/suite/rmarkdown.test.ts index 526df3f6c..5eb50ff40 100644 --- a/src/test/suite/rmarkdown.test.ts +++ b/src/test/suite/rmarkdown.test.ts @@ -133,7 +133,11 @@ suite('R Markdown execution', () => { file: string, format?: string, ): Promise; + findSiteParam(): Promise; }; + // Literal quoting is independent of whichever editor a previous suite + // left active and whether its directory contains a Markdown site. + sandbox.stub(manager, 'findSiteParam').resolves(undefined); const file = util.ToRStringLiteral(`/tmp/it's "report".Rmd`, '"'); assert.strictEqual( await manager.getKnitCommand({ site: '' }, file), From 3eda3ecc79f72dda0a3eedb48107554c7608ade8 Mon Sep 17 00:00:00 2001 From: Kun Ren Date: Sun, 25 Oct 2026 12:06:25 +0800 Subject: [PATCH 4/4] fix: retain failed workspace refreshes for the next demand --- src/test/node/workspaceRefresh.test.ts | 60 +++++++++++++++++-- .../suite/sessionWorkspaceRefresh.test.ts | 59 ++++++++++++++++++ src/workspaceRefresh.ts | 15 ++++- 3 files changed, 126 insertions(+), 8 deletions(-) diff --git a/src/test/node/workspaceRefresh.test.ts b/src/test/node/workspaceRefresh.test.ts index d6e29f054..2c2195cba 100644 --- a/src/test/node/workspaceRefresh.test.ts +++ b/src/test/node/workspaceRefresh.test.ts @@ -104,27 +104,75 @@ suite('Workspace refresh scheduling', () => { scheduler.dispose(); }); - test('deferred sends keep invalidation and inspection failures do not spin', async () => { + test('deferred sends retain failed reads for the next explicit demand without spinning', async () => { let calls = 0; const errors: unknown[] = []; - const scheduler = new WorkspaceRefreshScheduler( + const published: number[] = []; + const scheduler = new WorkspaceRefreshScheduler( () => { calls++; - return Promise.reject(new Error('R busy')); + return calls === 1 ? Promise.reject(new Error('R busy')) : Promise.resolve(2); }, - () => assert.fail('A failed inspection must preserve the previous workspace'), + (value) => published.push(value), () => true, (error) => errors.push(error), ); scheduler.invalidate(1000); scheduler.defer(); await scheduler.ensure(); - await scheduler.ensure(); + await new Promise((resolve) => setImmediate(resolve)); assert.strictEqual(calls, 1); assert.strictEqual(errors.length, 1); - scheduler.invalidate(); + assert.deepStrictEqual(published, []); + await scheduler.ensure(); + assert.strictEqual(calls, 2); + assert.deepStrictEqual(published, [2]); + await scheduler.ensure(); + assert.strictEqual(calls, 2, 'A successful refresh clears the pending demand'); + scheduler.dispose(); + }); + + test('an undefined read stays pending without an automatic retry', async () => { + let calls = 0; + const published: number[] = []; + const scheduler = new WorkspaceRefreshScheduler( + () => Promise.resolve(++calls === 1 ? undefined : 2), + (value) => published.push(value), + () => true, + ); + await scheduler.ensure(); + await new Promise((resolve) => setImmediate(resolve)); + assert.strictEqual(calls, 1); + assert.deepStrictEqual(published, []); + await scheduler.ensure(); + assert.strictEqual(calls, 2); + assert.deepStrictEqual(published, [2]); + scheduler.dispose(); + }); + + test('invalidations during a failed read remain coalesced until the next demand', async () => { + const first = deferred(); + let calls = 0; + const published: number[] = []; + const scheduler = new WorkspaceRefreshScheduler( + () => (++calls === 1 ? first.promise : Promise.resolve(2)), + (value) => published.push(value), + () => true, + ); + const running = scheduler.ensure(); + await Promise.resolve(); + for (let i = 0; i < 20; i++) { + scheduler.invalidate(); + assert.strictEqual(scheduler.ensure(), running); + } + first.resolve(undefined); + await running; + await new Promise((resolve) => setImmediate(resolve)); + assert.strictEqual(calls, 1); + assert.deepStrictEqual(published, []); await scheduler.ensure(); assert.strictEqual(calls, 2); + assert.deepStrictEqual(published, [2]); scheduler.dispose(); }); }); diff --git a/src/test/suite/sessionWorkspaceRefresh.test.ts b/src/test/suite/sessionWorkspaceRefresh.test.ts index cceb635a6..fb9235b4b 100644 --- a/src/test/suite/sessionWorkspaceRefresh.test.ts +++ b/src/test/suite/sessionWorkspaceRefresh.test.ts @@ -154,4 +154,63 @@ suite('Session workspace refresh demand', () => { sinon.assert.notCalled(refresh); assert.strictEqual(target.workspaceData, metadata); }); + + test('native workspace access retries a failed request without a new invalidation', async () => { + const socket = await connections.attach('workspace-retry-demand', 55103, 'external-host', { + autoActivate: false, + attachmentId: 'workspace-retry-demand-fixture', + }); + await session.activateSessionById('workspace-retry-demand'); + session.deferWorkspaceRefresh(); + const target = session.activeSession!; + const previous: session.WorkspaceData = { + revision: 1, + globalenv: {}, + search: [], + loaded_namespaces: [], + }; + target.workspaceData = previous; + let requests = 0; + sandbox.stub(socket, 'write').callsFake((data: string | Uint8Array) => { + const request = JSON.parse(data.toString()) as { method?: string; id: number }; + if (request.method === 'workspace') { + requests++; + const response = + requests === 1 + ? { error: { code: -32603, message: 'Transient inspection failure' } } + : { + result: { + revision: 2, + globalenv: { + latest: { + class: 'integer', + type: 'integer', + length: 1, + str: 'integer, length 1', + }, + }, + search: [], + loaded_namespaces: [], + }, + }; + socket.emit( + 'data', + Buffer.from( + `${JSON.stringify({ jsonrpc: '2.0', id: request.id, ...response })}\n`, + ), + ); + } + return true; + }); + await session.ensureSessionWorkspace(target); + await new Promise((resolve) => setImmediate(resolve)); + assert.strictEqual(requests, 1, 'A failed read must not trigger automatic retries'); + assert.strictEqual(target.workspaceData, previous); + await session.ensureSessionWorkspace(target); + assert.strictEqual(requests, 2); + assert.ok(target.workspaceData.globalenv.latest); + assert.strictEqual(session.activeSession, target); + await session.ensureSessionWorkspace(target); + assert.strictEqual(requests, 2, 'Successful metadata stays cached until invalidated'); + }); }); diff --git a/src/workspaceRefresh.ts b/src/workspaceRefresh.ts index 4cb925328..99a22776d 100644 --- a/src/workspaceRefresh.ts +++ b/src/workspaceRefresh.ts @@ -42,6 +42,7 @@ export class WorkspaceRefreshScheduler { return Promise.resolve(); } this.defer(); + let retryOnDemand = false; this.running = Promise.resolve() .then(async () => { while (!this.disposed && this.dirty && this.canRefresh()) { @@ -52,21 +53,31 @@ export class WorkspaceRefreshScheduler { if (this.disposed) { return; } + if (value === undefined) { + this.dirty = true; + retryOnDemand = true; + return; + } if (!this.canRefresh()) { this.dirty = true; return; } - if (revision === this.revision && value !== undefined) { + if (revision === this.revision) { this.publish(value); } } catch (error) { + this.dirty = true; + retryOnDemand = true; this.report(error); + return; } } }) .finally(() => { this.running = undefined; - if (!this.disposed && this.dirty && this.canRefresh()) { + // Failed reads retain the invalidation, but a later caller or + // notification must request the retry instead of spinning here. + if (!retryOnDemand && !this.disposed && this.dirty && this.canRefresh()) { void this.ensure(); } });