From 22f4d23d62aa4a473b6273eb5da29da206eefb53 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 30 Aug 2026 00:47:42 +0200 Subject: [PATCH 01/30] Add a JSON-RPC server mode to F# Interactive An editor hosting F# Interactive today drives it through standard input and recovers results by looking for a "SERVER-PROMPT>" marker in the output text. That protocol cannot say which output belongs to which submission and carries no structured diagnostics, which has pushed the Visual Studio window into a set of workarounds: a line directive wrapped around every selection, a temporary file the first submission writes its own process id into because under "dotnet fsi" the launched process is not the one evaluating code, counting output lines to skip what that discovery printed, and a separate remoting channel for interrupts. This adds "--fsi-server-jsonrpc:", in which a host submits interactions over a named pipe and receives results as data: diagnostics with positions, escaping exceptions, the evaluating process id, and the working directory. Program output keeps flowing through the redirected console streams, so the control channel and the script's own output no longer have to be told apart after the fact, and the prompt is suppressed rather than filtered back out. The transport is StreamJsonRpc over a HeaderDelimitedMessageHandler, the same combination Roslyn's interactive host uses. The protocol types live in one file so that a host can share the source rather than restate them. Threading is unchanged in the way that matters: interactions are evaluated on the event loop thread through EventLoopInvoke, exactly as the standard input path already does, so scripts that create user interface objects behave as they do at the console. Interactions queue onto a single worker and complete their tasks there, which leaves the library free to dispatch an interrupt while one is still running - an interrupt that waited its turn behind the interaction it is meant to stop would never arrive. The changes to fsi.fs are five hunks, all guarded by the new option and with no public API change: register the option, treat the mode as a server mode so the console reader is not used, suppress the prompt, skip the standard input reader thread, and keep the legacy interrupt channel for the legacy server mode only. The session watches the process id it was given and exits when it goes, so a crashed editor does not leave an orphan behind. Tested by 23 tests that launch a real fsi process and drive the protocol over the pipe; the parts most likely to break only exist across a process boundary. Co-Authored-By: Claude Fable 5 --- FSharp.slnx | 3 + .../.FSharp.Compiler.Service/11.0.100.md | 4 + eng/Packages.props | 4 + src/Compiler/Interactive/fsi.fs | 33 +- src/fsi/fsi.targets | 8 + src/fsi/fsimain.fs | 16 +- src/fsi/fsiserver.fs | 415 ++++++++++++++++++ src/fsi/interactiveProtocol.fs | 111 +++++ ...p.Compiler.Interactive.Server.Tests.fsproj | 51 +++ .../FsiJsonRpcServerTests.fs | 340 ++++++++++++++ .../FsiServerHarness.fs | 298 +++++++++++++ 11 files changed, 1278 insertions(+), 5 deletions(-) create mode 100644 src/fsi/fsiserver.fs create mode 100644 src/fsi/interactiveProtocol.fs create mode 100644 tests/FSharp.Compiler.Interactive.Server.Tests/FSharp.Compiler.Interactive.Server.Tests.fsproj create mode 100644 tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs create mode 100644 tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs diff --git a/FSharp.slnx b/FSharp.slnx index 50819fbfb6b..bedbd53bf36 100644 --- a/FSharp.slnx +++ b/FSharp.slnx @@ -27,6 +27,9 @@ + + + diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index b1a0fbd89bb..172e89b961b 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,3 +1,7 @@ +### Added + +* F# Interactive gains a JSON-RPC server mode, `--fsi-server-jsonrpc:`, in which a host submits interactions over a named pipe and receives structured results — diagnostics with positions, escaping exceptions, and the session's own process id — instead of recovering them by looking for a `SERVER-PROMPT>` marker in the output text. Program output continues to flow through the redirected console streams. ([PR #20360](https://github.com/dotnet/fsharp/pull/20360)) + ### Fixed * Fix internal error "no 'value__' field found for enumeration type" when an `enum` constraint is checked against an enum declared in the same recursive group (`module rec` or an `and` group). The constraint is now solved once the representations of the group are established. ([Issue #14580](https://github.com/dotnet/fsharp/issues/14580), [PR #20454](https://github.com/dotnet/fsharp/pull/20454)) diff --git a/eng/Packages.props b/eng/Packages.props index c6b2eabb7a2..eac871c2d58 100644 --- a/eng/Packages.props +++ b/eng/Packages.props @@ -116,6 +116,10 @@ + + diff --git a/src/Compiler/Interactive/fsi.fs b/src/Compiler/Interactive/fsi.fs index ce2ec781e91..3835bb36bb4 100644 --- a/src/Compiler/Interactive/fsi.fs +++ b/src/Compiler/Interactive/fsi.fs @@ -988,11 +988,19 @@ type internal FsiCommandLineOptions(fsi: FsiEvaluationSessionHostConfig, argv: s let mutable fsiServerOutputCodePage = None let mutable fsiLCID = None + /// Set by --fsi-server-jsonrpc. The host submits interactions over a named pipe rather than + /// through standard input, and reads structured results rather than parsing the output text. + let mutable fsiServerJsonRpcPipe = "" + // internal options let mutable probeToSeeIfConsoleWorks = true let mutable peekAheadOnConsoleToPermitTyping = true - let isInteractiveServer () = fsiServerName <> "" + let isJsonRpcServer () = fsiServerJsonRpcPipe <> "" + + // Both server modes are driven by a host rather than a user at a console, so neither one uses + // the console reader. + let isInteractiveServer () = fsiServerName <> "" || isJsonRpcServer () let recordExplicitArg arg = explicitArgs <- explicitArgs @ [ arg ] let executableFileNameWithoutExtension = @@ -1074,6 +1082,7 @@ type internal FsiCommandLineOptions(fsi: FsiEvaluationSessionHostConfig, argv: s [ // Make internal fsi-server* options. Do not print in the help. They are used by VFSI. CompilerOption("fsi-server-report-references", "", OptionString(fun s -> writeReferencesAndExit <- Some s), None, None) CompilerOption("fsi-server", "", OptionString(fun s -> fsiServerName <- s), None, None) // "FSI server mode on given named channel"); + CompilerOption("fsi-server-jsonrpc", "", OptionString(fun s -> fsiServerJsonRpcPipe <- s), None, None) // "FSI server mode speaking JSON-RPC over the given named pipe" CompilerOption("fsi-server-input-codepage", "", OptionInt(fun n -> fsiServerInputCodePage <- Some(n)), None, None) // " Set the input codepage for the console"); CompilerOption("fsi-server-output-codepage", "", OptionInt(fun n -> fsiServerOutputCodePage <- Some(n)), None, None) // " Set the output codepage for the console"); CompilerOption( @@ -1385,6 +1394,13 @@ type internal FsiCommandLineOptions(fsi: FsiEvaluationSessionHostConfig, argv: s member _.IsInteractiveServer = isInteractiveServer () + /// True when the host drives this session over JSON-RPC. Interactions then arrive on the + /// control channel instead of standard input, and no prompt is written to the output. + /// + /// The pipe name itself is not surfaced here: the server lives in the process entry point, + /// which reads it from the command line directly. + member _.IsJsonRpcServer = isJsonRpcServer () + member _.ProbeToSeeIfConsoleWorks = probeToSeeIfConsoleWorks member _.EnableConsoleKeyProcessing = enableConsoleKeyProcessing @@ -1477,7 +1493,10 @@ type internal FsiConsolePrompt(fsiOptions: FsiCommandLineOptions, fsiConsoleOutp // A prompt gets "printed ahead" at start up. Tells users to start type while initialisation completes. // A prompt can be skipped by "silent directives", e.g. ones sent to FSI by VS. let mutable dropPrompt = 0 - let mutable showPrompt = true + + // A host driving the session over JSON-RPC learns when an interaction finished from the + // response to its request, so a prompt in the output stream would be noise. + let mutable showPrompt = not fsiOptions.IsJsonRpcServer // NOTE: SERVER-PROMPT is not user displayed, rather it's a prefix that code elsewhere // uses to identify the prompt, see service\FsPkgs\FSharp.VS.FSI\fsiSessionToolWindow.fs @@ -5108,7 +5127,9 @@ type FsiEvaluationSession // We later switch to doing interaction-by-interaction processing on the "event loop" thread let ctokRun = AssumeCompilationThreadWithoutEvidence() - if fsiOptions.IsInteractiveServer then + // The legacy server channel carries interrupt requests only. A JSON-RPC host carries them + // on its own connection, and starts that server from the process entry point. + if fsiOptions.IsInteractiveServer && not fsiOptions.IsJsonRpcServer then SpawnInteractiveServer(fsi, fsiOptions, fsiConsoleOutput) use _ = UseBuildPhase BuildPhase.Interactive @@ -5127,7 +5148,11 @@ type FsiEvaluationSession | _ -> ()) fsiInteractionProcessor.LoadInitialFiles(ctokRun, diagnosticsLogger) - fsiInteractionProcessor.StartStdinReadAndProcessThread(tcConfigB.diagnosticsOptions, diagnosticsLogger) + + // A JSON-RPC host submits interactions on the control channel, which leaves standard + // input to the script being run. + if not fsiOptions.IsJsonRpcServer then + fsiInteractionProcessor.StartStdinReadAndProcessThread(tcConfigB.diagnosticsOptions, diagnosticsLogger) DriveFsiEventLoop(fsi, fsiInterruptController, fsiConsoleOutput) diff --git a/src/fsi/fsi.targets b/src/fsi/fsi.targets index b38960f7f0e..39a27777f9e 100644 --- a/src/fsi/fsi.targets +++ b/src/fsi/fsi.targets @@ -36,6 +36,8 @@ + + {{FSCoreVersion}} @@ -70,4 +72,10 @@ + + + + + \ No newline at end of file diff --git a/src/fsi/fsimain.fs b/src/fsi/fsimain.fs index 314b01b5c31..9c09e8163cb 100644 --- a/src/fsi/fsimain.fs +++ b/src/fsi/fsimain.fs @@ -183,6 +183,11 @@ let evaluateSession (argv: string[]) = Console.InputEncoding <- System.Text.Encoding.UTF8 Console.OutputEncoding <- System.Text.Encoding.UTF8 + // A host may ask for the JSON-RPC server mode, in which interactions arrive on a named pipe + // instead of standard input. Recognised here because the server is driven from this entry + // point, alongside the event loop it evaluates on. + let jsonRpcPipeName = FSharp.Compiler.Interactive.Server.tryGetPipeName argv + try // Create the console reader let console = new FSharp.Compiler.Interactive.ReadLineConsole() @@ -190,7 +195,10 @@ let evaluateSession (argv: string[]) = // Define the function we pass to the FsiEvaluationSession let getConsoleReadLine (probeToSeeIfConsoleWorks) = let consoleIsOperational = - if probeToSeeIfConsoleWorks then + if jsonRpcPipeName.IsSome then + // The session is driven by a host, so there is no user at a console to read from. + false + elif probeToSeeIfConsoleWorks then //if progress then fprintfn outWriter "probing to see if console works..." try // Probe to see if the console looks functional on this version of .NET @@ -341,6 +349,12 @@ let evaluateSession (argv: string[]) = | None -> s2 )) + // Serve the host on a background thread, leaving this thread to Run() and the event loop + // that interactions are evaluated on. + match jsonRpcPipeName with + | Some pipeName -> FSharp.Compiler.Interactive.Server.startOnBackgroundThread fsiSession fsiConfig pipeName Console.Out Console.Error + | None -> () + // Start the session fsiSession.Run() 0 diff --git a/src/fsi/fsiserver.fs b/src/fsi/fsiserver.fs new file mode 100644 index 00000000000..ab2b1ddbef8 --- /dev/null +++ b/src/fsi/fsiserver.fs @@ -0,0 +1,415 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +/// The JSON-RPC server mode of F# Interactive, activated by `--fsi-server-jsonrpc:`. +/// +/// An editor hosting F# Interactive needs two things from the process: a control channel to submit +/// interactions and receive structured results, and the program's own console output. This server +/// keeps those apart. Control traffic is JSON-RPC over a named pipe; everything the script itself +/// prints continues to flow through the redirected standard output and error streams, exactly as it +/// does for a console session. That separation is what removes the need for a host to recognise +/// prompts in the output text in order to tell one interaction's results from the next. +/// +/// The transport is StreamJsonRpc over a header-delimited stream, the same combination Roslyn's +/// interactive host uses, so a client built on that library talks to this one with its stock +/// message handler. +/// +/// Threading mirrors the standard input path of a console session. Interactions are evaluated on +/// the event loop thread by way of `EventLoopInvoke`, so scripts that create user interface objects +/// behave as they do at the console. Interactions are queued onto a single worker so that they run +/// in the order they arrived, while requests that must not wait behind them — an interrupt above +/// all — are served as they arrive. +module internal FSharp.Compiler.Interactive.Server + +open System +open System.Collections.Concurrent +open System.Diagnostics +open System.IO +open System.IO.Pipes +open System.Runtime.InteropServices +open System.Threading +open System.Threading.Tasks + +open StreamJsonRpc + +open FSharp.Compiler.Diagnostics +open FSharp.Compiler.Interactive.Protocol +open FSharp.Compiler.Interactive.Shell + +/// The name of the command line option that turns on this server. +[] +let JsonRpcServerOption = "--fsi-server-jsonrpc:" + +/// File name reported for interactions that the host did not attribute to a source file. +[] +let private DefaultInteractionName = "stdin.fsx" + +//------------------------------------------------------------------------- +// Shaping results for the wire +//------------------------------------------------------------------------- + +let private severityText (severity: FSharpDiagnosticSeverity) = + match severity with + | FSharpDiagnosticSeverity.Error -> "error" + | FSharpDiagnosticSeverity.Warning -> "warning" + | FSharpDiagnosticSeverity.Info -> "info" + | FSharpDiagnosticSeverity.Hidden -> "hidden" + +let private toDiagnosticInfo (diagnostic: FSharpDiagnostic) = + { + severity = severityText diagnostic.Severity + message = diagnostic.Message + errorNumber = diagnostic.ErrorNumber + subcategory = diagnostic.Subcategory + fileName = diagnostic.FileName + startLine = diagnostic.StartLine + startColumn = diagnostic.StartColumn + endLine = diagnostic.EndLine + endColumn = diagnostic.EndColumn + } + +let private toExecutionResult (outcome: Choice) (diagnostics: FSharpDiagnostic[]) (cancelled: bool) = + let hasErrors = + diagnostics + |> Array.exists (fun d -> d.Severity = FSharpDiagnosticSeverity.Error) + + let failure = + match outcome with + | Choice1Of2 _ -> None + // When the interaction failed to compile, the diagnostics already say everything there is + // to say. The exception raised to stop processing carries no more information, and a host + // that reported it alongside them would be saying the same thing twice. + | Choice2Of2 _ when hasErrors -> None + | Choice2Of2 e -> Some e + + { + success = not hasErrors && failure.IsNone && not cancelled + cancelled = cancelled + diagnostics = diagnostics |> Array.map toDiagnosticInfo + ``exception`` = + match failure with + | Some e -> + { + ``type`` = e.GetType().FullName + message = e.Message + stackTrace = + match e.StackTrace with + | null -> "" + | trace -> trace + } + | None -> Unchecked.defaultof + workingDirectory = Directory.GetCurrentDirectory() + } + +//------------------------------------------------------------------------- +// The server +//------------------------------------------------------------------------- + +/// Serialises the interactions submitted by the host onto a single worker, so that they are +/// evaluated strictly in the order they were received. +[] +type private ExecutionQueue() = + let queue = new BlockingCollection unit>() + + let worker = + Thread( + (fun () -> + for job in queue.GetConsumingEnumerable() do + // A job reports its own failures to the host; nothing here may escape and kill + // the worker, or the session would stop responding to every later request. + try + job () + with _ -> + ()), + Name = "FSI-JsonRpc-Execute", + IsBackground = true + ) + + do worker.Start() + + member _.Enqueue(job: unit -> unit) = + if not queue.IsAddingCompleted then + queue.Add job + + member _.Complete() = + if not queue.IsAddingCompleted then + queue.CompleteAdding() + +/// The object the host calls into. +/// +/// Everything that evaluates code goes onto the execution queue and completes its task when the +/// interaction finishes, which leaves StreamJsonRpc free to dispatch an interrupt in the meantime. +[] +type internal FsiRpcTarget + ( + fsiSession: FsiEvaluationSession, + fsiConfig: FsiEvaluationSessionHostConfig, + outWriter: TextWriter, + errorWriter: TextWriter, + shutdownRequested: TaskCompletionSource + ) = + + let executionQueue = ExecutionQueue() + let interruptLock = obj () + let mutable currentCancellation: CancellationTokenSource = null + let mutable initialized = false + + /// Evaluate on the event loop thread, the same thread a console session evaluates on. + /// + /// `EvalInteractionNonThrowing` reports diagnostics and execution failures through its result, + /// but a failure inside the event loop machinery itself would still escape, so it is caught + /// here and reported as an ordinary failed interaction. + let evaluateOnEventLoop (evaluate: unit -> Choice * FSharpDiagnostic[]) = + try + fsiConfig.EventLoopInvoke evaluate + with e -> + Choice2Of2 e, [||] + + /// Flush everything the interaction printed before answering, so that a host which shows + /// standard output and RPC results side by side sees them in the order they were produced. + let flushConsole () = + try + outWriter.Flush() + errorWriter.Flush() + with _ -> + () + + let runInteraction (code: string) (scriptPath: string) = + let cancellation = new CancellationTokenSource() + + lock interruptLock (fun () -> currentCancellation <- cancellation) + + try + let outcome, diagnostics = + evaluateOnEventLoop (fun () -> fsiSession.EvalInteractionNonThrowing(code, scriptPath, cancellation.Token)) + + flushConsole () + toExecutionResult outcome diagnostics cancellation.IsCancellationRequested + finally + lock interruptLock (fun () -> currentCancellation <- null) + cancellation.Dispose() + + /// Queue an interaction and hand back the task the host is waiting on. + let queueInteraction (run: unit -> ExecutionResult) = + let completion = + TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously) + + executionQueue.Enqueue(fun () -> + try + completion.TrySetResult(run ()) |> ignore + with e -> + completion.TrySetException e |> ignore) + + completion.Task + + /// Prefix the submitted text with a line directive so that diagnostics point back at the + /// editor's own file and line rather than at the position within the submission. + let positionInteraction (code: string) (sourcePath: string) (startLine: Nullable) = + if String.IsNullOrEmpty sourcePath || not startLine.HasValue then + code + else + sprintf "# %d @\"%s\"\n%s" startLine.Value sourcePath code + + /// Refuse anything that arrives before the handshake, so that a mis-sequenced host gets a clear + /// answer rather than an obscure failure later on. + let requireInitialized () = + if not initialized then + raise (LocalRpcException("'fsi/initialize' must be called first", ErrorCode = -32000)) + + /// Watch the process that owns this session, so that an F# Interactive left behind by a + /// crashed host does not survive as an orphan. + let attachToClientProcess (clientProcessId: int) = + try + let client = Process.GetProcessById clientProcessId + client.EnableRaisingEvents <- true + client.Exited.Add(fun _ -> exit 0) + + // The host may already have gone by the time the handler was attached. + if client.HasExited then + exit 0 + with _ -> + // An unknown process id is not fatal: the session simply loses orphan protection. + () + + member _.Complete() = executionQueue.Complete() + + [] + member _.Initialize(request: InitializeRequest) : InitializeResult = + if request.clientProcessId > 0 then + attachToClientProcess request.clientProcessId + + initialized <- true + + { + processId = Process.GetCurrentProcess().Id + frameworkDescription = RuntimeInformation.FrameworkDescription + processArchitecture = string RuntimeInformation.ProcessArchitecture + fsiVersion = + match typeof.Assembly.GetName().Version with + | null -> "" + | version -> string version + workingDirectory = Directory.GetCurrentDirectory() + supportsInterrupt = true + } + + [] + member _.Execute(request: ExecuteRequest) : Task = + requireInitialized () + + let text = positionInteraction request.code request.sourcePath request.startLine + + let scriptPath = + if String.IsNullOrEmpty request.sourcePath then + DefaultInteractionName + else + request.sourcePath + + queueInteraction (fun () -> runInteraction text scriptPath) + + [] + member _.ExecuteFile(request: ExecuteFileRequest) : Task = + requireInitialized () + + // Routed through #load so that the file joins the session the same way it would from a + // script, rather than being replayed as anonymous text. + queueInteraction (fun () -> runInteraction (sprintf "#load @\"%s\"" request.path) request.path) + + /// Apply the host's notion of where to look for sources and references, expressed as the + /// directives a script would use. + [] + member _.SetPaths(request: SetPathsRequest) : Task = + requireInitialized () + + let directives = ResizeArray() + + if not (String.IsNullOrWhiteSpace request.workingDirectory) + && Directory.Exists request.workingDirectory then + // Two different notions of "current directory" have to agree here. The directive moves + // the compiler's, which is what relative #load and #r resolve against; the process one + // is what the running script sees when it opens a file by relative path. + try + Directory.SetCurrentDirectory request.workingDirectory + with _ -> + () + + directives.Add(sprintf "#silentCd @\"%s\"" request.workingDirectory) + + match request.includePaths with + | null -> () + | paths -> + for path in paths do + if not (String.IsNullOrWhiteSpace path) then + directives.Add(sprintf "#I @\"%s\"" path) + + if directives.Count = 0 then + queueInteraction (fun () -> toExecutionResult (Choice1Of2 None) [||] false) + else + queueInteraction (fun () -> runInteraction (String.Join("\n", directives)) DefaultInteractionName) + + /// Interrupt the interaction in flight. + /// + /// Served straight away rather than queued, which is the point: an interrupt that waited its + /// turn behind the interaction it is meant to stop would never arrive. + [] + member _.Interrupt() : InterruptResult = + requireInitialized () + + let cancellation = lock interruptLock (fun () -> currentCancellation) + + match cancellation with + | null -> { interrupted = false } + | cts -> + // Cancel the token the interaction is running under, then ask the session to interrupt + // the evaluation thread, which is what stops code already inside a long-running call. + try + cts.Cancel() + with _ -> + () + + try + fsiSession.Interrupt() + with _ -> + () + + { interrupted = true } + + [] + member _.Shutdown() : unit = + requireInitialized () + shutdownRequested.TrySetResult() |> ignore + +/// Wait for the host to connect, then serve requests until it disconnects or asks to shut down. +let private runServer + (fsiSession: FsiEvaluationSession) + (fsiConfig: FsiEvaluationSessionHostConfig) + (pipeName: string) + (outWriter: TextWriter) + (errorWriter: TextWriter) + = + use pipe = + new NamedPipeServerStream( + pipeName, + PipeDirection.InOut, + maxNumberOfServerInstances = 1, + transmissionMode = PipeTransmissionMode.Byte, + options = PipeOptions.Asynchronous + ) + + pipe.WaitForConnection() + + let shutdownRequested = + TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously) + + let target = + FsiRpcTarget(fsiSession, fsiConfig, outWriter, errorWriter, shutdownRequested) + + use rpc = new JsonRpc(new HeaderDelimitedMessageHandler(pipe, new JsonMessageFormatter())) + rpc.AddLocalRpcTarget(target, JsonRpcTargetOptions(NotifyClientOfEvents = false, AllowNonPublicInvocation = false)) + rpc.StartListening() + + // Either the host goes away or it asks to stop. Both end the session. + Task.WaitAny(rpc.Completion, shutdownRequested.Task) |> ignore + + if shutdownRequested.Task.IsCompleted then + // Give the reply to the shutdown request its moment to reach the host before the process + // disappears from under it. + Task.Delay(250).Wait() + + target.Complete() + +/// Start the server on a background thread and return, leaving the caller's thread free to drive +/// the event loop. Mirrors how a console session spawns its standard input reader. +let startOnBackgroundThread + (fsiSession: FsiEvaluationSession) + (fsiConfig: FsiEvaluationSessionHostConfig) + (pipeName: string) + (outWriter: TextWriter) + (errorWriter: TextWriter) + = + let thread = + Thread( + (fun () -> + try + runServer fsiSession fsiConfig pipeName outWriter errorWriter + with e -> + errorWriter.WriteLine(sprintf "F# Interactive server terminated: %s" (e.ToString())) + errorWriter.Flush() + + // The session exists only to serve this host. Once the connection is gone there is + // nothing left to do, and lingering would leak a process. + exit 0), + Name = "FSI-JsonRpc-Dispatch", + IsBackground = true + ) + + thread.Start() + +/// Recognise `--fsi-server-jsonrpc:` in a command line, returning the pipe name. +let tryGetPipeName (argv: string[]) = + argv + |> Array.tryPick (fun arg -> + if arg.StartsWith(JsonRpcServerOption, StringComparison.Ordinal) then + let name = arg.Substring(JsonRpcServerOption.Length).Trim('"') + + if String.IsNullOrWhiteSpace name then None else Some name + else + None) diff --git a/src/fsi/interactiveProtocol.fs b/src/fsi/interactiveProtocol.fs new file mode 100644 index 00000000000..519941092cc --- /dev/null +++ b/src/fsi/interactiveProtocol.fs @@ -0,0 +1,111 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +/// The types exchanged between F# Interactive and the editor hosting it. +/// +/// This file is compiled into fsi and linked into the host, so that the two ends of the protocol +/// cannot drift apart. Roslyn achieves the same by having both sides reference one assembly; fsi +/// exposes no public surface to reference, so the source is shared instead. +/// +/// The members are named as they appear on the wire, and the records are `CLIMutable` so that the +/// JSON-RPC formatter can construct them. +namespace FSharp.Compiler.Interactive.Protocol + +/// Method names. Both ends use these rather than repeating string literals. +module Methods = + let [] Initialize = "fsi/initialize" + let [] Execute = "fsi/execute" + let [] ExecuteFile = "fsi/executeFile" + let [] SetPaths = "fsi/setPaths" + let [] Interrupt = "fsi/interrupt" + let [] Shutdown = "fsi/shutdown" + +[] +type InitializeRequest = + { + /// The process that owns this session. F# Interactive watches it and exits when it goes, so + /// that a crashed editor does not leave an orphan behind. + clientProcessId: int + } + +[] +type InitializeResult = + { + /// The process actually evaluating code, which is what a debugger attaches to. + /// + /// On .NET this is not the process the host launched: `dotnet fsi` starts a second process, + /// and it is the inner one that matters. + processId: int + + frameworkDescription: string + processArchitecture: string + fsiVersion: string + workingDirectory: string + supportsInterrupt: bool + } + +[] +type ExecuteRequest = + { + code: string + + /// Where the text came from, when the host is executing a selection from a file. Together + /// with `startLine` this makes diagnostics point at the user's own source rather than at a + /// position within the submission. + sourcePath: string + + startLine: System.Nullable + } + +[] +type ExecuteFileRequest = { path: string } + +[] +type SetPathsRequest = + { + includePaths: string[] + workingDirectory: string + } + +/// One diagnostic. Lines are one-based and columns zero-based, as they are throughout the compiler. +[] +type DiagnosticInfo = + { + severity: string + message: string + errorNumber: int + subcategory: string + fileName: string + startLine: int + startColumn: int + endLine: int + endColumn: int + } + +/// An exception that escaped an interaction. Null when the interaction merely failed to compile, +/// because the diagnostics already describe that. +[] +type ExceptionInfo = + { + ``type``: string + message: string + stackTrace: string + } + +[] +type ExecutionResult = + { + /// The interaction was accepted and ran to completion: no error diagnostic, no escaping + /// exception, not interrupted. Warnings do not affect it. + success: bool + + cancelled: bool + diagnostics: DiagnosticInfo[] + ``exception``: ExceptionInfo + + /// Reported after every interaction so that the host can keep its own view of the session + /// in step with one that changed directory. + workingDirectory: string + } + +[] +type InterruptResult = { interrupted: bool } diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FSharp.Compiler.Interactive.Server.Tests.fsproj b/tests/FSharp.Compiler.Interactive.Server.Tests/FSharp.Compiler.Interactive.Server.Tests.fsproj new file mode 100644 index 00000000000..f77de46baa7 --- /dev/null +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FSharp.Compiler.Interactive.Server.Tests.fsproj @@ -0,0 +1,51 @@ + + + + $(FSharpNetCoreProductTargetFramework) + + Exe + false + + false + true + true + false + false + $(NoWarn);FS0988 + + + + + + + + + + interactiveProtocol.fs + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs new file mode 100644 index 00000000000..e0bf3c8cfd4 --- /dev/null +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs @@ -0,0 +1,340 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module FSharp.Compiler.Interactive.Server.Tests.FsiJsonRpcServerTests + +open System +open System.IO +open System.Threading +open Xunit + +open FSharp.Compiler.Interactive.Protocol +open FSharp.Compiler.Interactive.Server.Tests.FsiServerHarness + +/// Start a session, hand it to the test, and shut it down afterwards. +let private withSession (test: FsiServerHarness -> unit) = + use session = new FsiServerHarness() + test session + +/// Start a session that has already completed the handshake. +let private withInitializedSession (test: FsiServerHarness -> unit) = + withSession (fun session -> + session.Initialize() |> ignore + test session) + +/// Include the result and the session's output in a failure, since a protocol result alone rarely +/// explains what the session actually did. +let private describe (session: FsiServerHarness) (result: ExecutionResult) = + sprintf + "result: %s\nstandard output:\n%s\nstandard error:\n%s" + (describeResult result) + session.StandardOutput + session.StandardError + +//------------------------------------------------------------------------- +// Handshake +//------------------------------------------------------------------------- + +[] +let ``initialize reports the session process`` () = + withSession (fun session -> + let result = session.Initialize() + + // The reported identifier is what a host attaches a debugger to, so it must be the process + // actually evaluating code rather than any launcher in front of it. + Assert.Equal(session.ProcessId, result.processId) + + Assert.StartsWith(".NET", result.frameworkDescription) + Assert.True result.supportsInterrupt + + Assert.True( + Directory.Exists result.workingDirectory, + sprintf "'%s' is not a directory" result.workingDirectory + )) + +[] +let ``requests before initialize are refused`` () = + withSession (fun session -> + match session.RequestExpectingError(Methods.Execute, FsiServerHarness.ExecuteParams "1 + 1") with + | None -> failwith "the session accepted an interaction before the handshake" + | Some code -> Assert.Equal(-32000, code)) + +[] +let ``unknown methods are refused`` () = + withInitializedSession (fun session -> + match session.RequestExpectingError("fsi/doesNotExist", obj ()) with + | None -> failwith "the session accepted an unknown method" + | Some code -> Assert.Equal(-32601, code)) + +//------------------------------------------------------------------------- +// Evaluating interactions +//------------------------------------------------------------------------- + +[] +let ``evaluates an interaction and prints its result`` () = + withInitializedSession (fun session -> + let result = session.Execute "1 + 1" + + Assert.True(succeeded result, describe session result) + Assert.Empty(diagnostics result) + + // The value is reported the way a console session reports it: printed to standard output. + Assert.True(session.WaitForOutput "val it: int = 2", describe session result)) + +[] +let ``keeps bindings across interactions`` () = + withInitializedSession (fun session -> + let bound = session.Execute "let x = 40" + Assert.True(succeeded bound, describe session bound) + + let result = session.Execute "x + 2" + Assert.True(succeeded result, describe session result) + Assert.True(session.WaitForOutput "val it: int = 42", describe session result)) + +[] +let ``reports what the interaction printed`` () = + withInitializedSession (fun session -> + let result = session.Execute "printfn \"hello from the session\"" + Assert.True(succeeded result, describe session result) + Assert.True(session.WaitForOutput "hello from the session", describe session result)) + +[] +let ``evaluates multi-line interactions`` () = + withInitializedSession (fun session -> + let code = String.Join("\n", [ "let add a b ="; " a + b"; ""; "add 20 22" ]) + + let result = session.Execute code + Assert.True(succeeded result, describe session result) + Assert.True(session.WaitForOutput "val it: int = 42", describe session result)) + +[] +let ``carries text that has to survive JSON escaping`` () = + withInitializedSession (fun session -> + // Quotes, backslashes and non-ASCII all have to make the round trip intact, in both the + // request and the output that comes back. + let result = session.Execute "printfn \"%s\" \"quote \\\" backslash \\\\ Ф# ✓\"" + + Assert.True(succeeded result, describe session result) + Assert.True(session.WaitForOutput "quote \" backslash \\ Ф# ✓", describe session result)) + +//------------------------------------------------------------------------- +// Diagnostics +//------------------------------------------------------------------------- + +[] +let ``reports type errors as structured diagnostics`` () = + withInitializedSession (fun session -> + let result = session.Execute "1 + \"text\"" + + Assert.False(succeeded result, describe session result) + + let reported = errors result + Assert.NotEmpty reported + + // FS0001 is the type mismatch error, and it must carry a usable position. + let error = reported[0] + Assert.Equal(1, error.errorNumber) + Assert.True(error.startLine >= 1, sprintf "unexpected start line %d" error.startLine) + Assert.False(String.IsNullOrWhiteSpace error.message)) + +[] +let ``reports undefined identifiers`` () = + withInitializedSession (fun session -> + let result = session.Execute "thisNameIsNotDefined" + + Assert.False(succeeded result, describe session result) + + // FS0039: the value or constructor is not defined. + Assert.True(errors result |> Array.exists (fun d -> d.errorNumber = 39), describe session result)) + +[] +let ``warnings do not fail an interaction`` () = + withInitializedSession (fun session -> + // An incomplete pattern match warns, but the interaction still runs. + let result = session.Execute "let f (x: int option) = match x with Some v -> v" + + Assert.True(succeeded result, describe session result) + Assert.NotEmpty(warnings result) + Assert.Empty(errors result)) + +[] +let ``attributes diagnostics to the host's file and line`` () = + withInitializedSession (fun session -> + // A host executing a selection tells the session where that selection came from, so that + // the reported position lands on the user's own source rather than within the submission. + let path = Path.Combine(Path.GetTempPath(), "Library.fs") + let result = session.Execute("1 + \"text\"", sourcePath = path, startLine = 120) + + let reported = errors result + Assert.NotEmpty reported + Assert.Equal(120, reported[0].startLine) + Assert.EndsWith("Library.fs", reported[0].fileName)) + +[] +let ``reports an escaping exception`` () = + withInitializedSession (fun session -> + // Annotated so that the interaction compiles: a bare `failwith` is generic and would fail + // the value restriction instead of ever running. + let result = session.Execute "(failwith \"boom\": unit)" + + Assert.False(succeeded result, describe session result) + Assert.Empty(errors result) + Assert.Equal(Some "boom", exceptionMessage result)) + +[] +let ``does not report an exception for a compilation failure`` () = + withInitializedSession (fun session -> + // The diagnostics already describe the failure. Reporting the exception fsi raises to stop + // processing would make a host show the same problem twice. + let result = session.Execute "1 + \"text\"" + + Assert.NotEmpty(errors result) + Assert.Equal(None, exceptionMessage result)) + +[] +let ``keeps serving after a failed interaction`` () = + withInitializedSession (fun session -> + Assert.False(succeeded (session.Execute "1 + \"text\"")) + Assert.False(succeeded (session.Execute "(failwith \"boom\": unit)")) + + // A session that stopped responding after an error would make the window useless. + let result = session.Execute "2 * 21" + Assert.True(succeeded result, describe session result) + Assert.True(session.WaitForOutput "val it: int = 42", describe session result)) + +//------------------------------------------------------------------------- +// Files and search paths +//------------------------------------------------------------------------- + +[] +let ``loads a script file`` () = + withInitializedSession (fun session -> + let script = + Path.Combine(Path.GetTempPath(), sprintf "fsiServerTest_%s.fsx" (Guid.NewGuid().ToString "N")) + + File.WriteAllText(script, "printfn \"the script ran\"\n") + + try + let loaded = + session.Request(Methods.ExecuteFile, { path = script }) + + Assert.True(succeeded loaded, describe session loaded) + + // The file is loaded, not replayed as anonymous text, so its effects are what prove it + // reached the session. Its definitions land in a module named after the file, which is + // ordinary `#load` behaviour and not something to assert on here. + Assert.True(session.WaitForOutput "the script ran", describe session loaded) + finally + try + File.Delete script + with _ -> + ()) + +[] +let ``setPaths changes the working directory`` () = + withInitializedSession (fun session -> + let directory = + Path.Combine(Path.GetTempPath(), sprintf "fsiServerTest_%s" (Guid.NewGuid().ToString "N")) + + Directory.CreateDirectory directory |> ignore + + try + let result = + session.Request( + Methods.SetPaths, + { + includePaths = [| directory |] + workingDirectory = directory + } + ) + + Assert.True(succeeded result, describe session result) + + // The host mirrors this value so that its own reference resolution matches the session. + let expected = Path.GetFullPath(directory).TrimEnd(Path.DirectorySeparatorChar) + let actual = Path.GetFullPath(result.workingDirectory).TrimEnd(Path.DirectorySeparatorChar) + Assert.Equal(expected, actual) + finally + try + Directory.Delete(directory, true) + with _ -> + ()) + +[] +let ``reports the working directory after every interaction`` () = + withInitializedSession (fun session -> + let result = session.Execute "1" + Assert.True(Directory.Exists result.workingDirectory, describe session result)) + +//------------------------------------------------------------------------- +// Interrupting +//------------------------------------------------------------------------- + +[] +let ``interrupts a running interaction`` () = + withInitializedSession (fun session -> + // Warm the session up first, so that the interrupt below meets a session that is genuinely + // executing the loop rather than still starting up. + Assert.True(succeeded (session.Execute "1")) + + let running = + session.BeginRequest( + Methods.Execute, + FsiServerHarness.ExecuteParams "while true do System.Threading.Thread.Sleep 10" + ) + + Thread.Sleep 3000 + + // Interactions queue behind one another, but an interrupt is served as it arrives — which + // is the whole point, since one that waited its turn would never stop anything. + let interrupted = + session.Request(Methods.Interrupt, TimeSpan.FromSeconds 30.0) + + Assert.True interrupted.interrupted + + // The interrupted interaction must come back rather than hang forever. + try + let result = session.EndRequest(running, TimeSpan.FromSeconds 60.0) + Assert.False(succeeded result, describe session result) + with _ -> + // Reported as a failed call rather than a failed interaction; either is acceptable. + ()) + +[] +let ``interrupt is harmless when nothing is running`` () = + withInitializedSession (fun session -> + let result = session.Request Methods.Interrupt + Assert.False result.interrupted) + +//------------------------------------------------------------------------- +// Lifetime +//------------------------------------------------------------------------- + +[] +let ``shutdown ends the session`` () = + withInitializedSession (fun session -> + session.Request Methods.Shutdown |> ignore + + Assert.True(session.WaitForExit 30_000, "the session did not exit after shutdown")) + +[] +let ``the session exits when the host disconnects`` () = + let session = new FsiServerHarness() + session.Initialize() |> ignore + + // Closing the control channel is what happens when the editor process dies. A session that + // survived it would leak a process for every crash. + (session :> IDisposable).Dispose() + +[] +let ``the session exits when its host process exits`` () = + // A second session stands in for the editor: it is a real, live process to attach to, and + // killing it must bring down the session that named it as its host. + use host = new FsiServerHarness() + use session = new FsiServerHarness() + + session.Initialize(clientProcessId = host.ProcessId) |> ignore + Assert.False session.HasExited + + (host :> IDisposable).Dispose() + + Assert.True(session.WaitForExit 30_000, "the session outlived its host process") diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs new file mode 100644 index 00000000000..596384a5216 --- /dev/null +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs @@ -0,0 +1,298 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +/// Drives a real F# Interactive process in its JSON-RPC server mode, the way an editor would. +/// +/// The tests exercise the shipped protocol end to end rather than an in-process stand-in, because +/// the parts most likely to break are the ones that only exist across a process boundary: the +/// handshake, the lifetime of the session, and the interaction between the control channel and the +/// output streams. The client here is StreamJsonRpc, the same library the window uses. +module FSharp.Compiler.Interactive.Server.Tests.FsiServerHarness + +open System +open System.Diagnostics +open System.IO +open System.IO.Pipes +open System.Runtime.InteropServices +open System.Text +open System.Threading +open System.Threading.Tasks + +open StreamJsonRpc + +open FSharp.Compiler.Interactive.Protocol + +/// How long to wait for the session to answer a request. Generous, because the first interaction +/// of a session pays for the type checker warming up. +let private defaultTimeout = TimeSpan.FromSeconds 120.0 + +/// Locate the fsi built by this repository, alongside the test assembly's own output. +/// +/// Test output lives at `/bin///`, and fsi is its +/// sibling at `/bin/fsi//`. +let private locateFsi () = + let baseDirectory = + DirectoryInfo(AppContext.BaseDirectory.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)) + + let framework = baseDirectory.Name + let configuration = baseDirectory.Parent.Name + let binDirectory = baseDirectory.Parent.Parent.Parent + + let fsi = + Path.Combine(binDirectory.FullName, "fsi", configuration, framework, "fsi.dll") + + if not (File.Exists fsi) then + failwithf "Could not find the fsi under test at '%s'. Build src/fsi first." fsi + + fsi + +/// Prefer the .NET host this repository provisions, so that the session runs on the same runtime +/// as the rest of the build. +let private locateDotnetHost () = + let executable = + if RuntimeInformation.IsOSPlatform OSPlatform.Windows then + "dotnet.exe" + else + "dotnet" + + let rec search (directory: DirectoryInfo) = + match directory with + | null -> executable + | directory -> + let candidate = Path.Combine(directory.FullName, ".dotnet", executable) + + if File.Exists candidate then + candidate + else + search directory.Parent + + search (DirectoryInfo(AppContext.BaseDirectory)) + +/// A running session, plus everything needed to talk to it and to explain a failure. +[] +type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = + let pipeName = "FsiServerTests_" + Guid.NewGuid().ToString("N") + let standardOutput = StringBuilder() + let standardError = StringBuilder() + let outputLock = obj () + + let startInfo = + let arguments = + [ + locateFsi () + "--nologo" + "--fsi-server-jsonrpc:" + pipeName + yield! defaultArg extraArguments [] + ] + + let startInfo = + ProcessStartInfo( + FileName = locateDotnetHost (), + UseShellExecute = false, + CreateNoWindow = true, + RedirectStandardInput = true, + RedirectStandardOutput = true, + RedirectStandardError = true, + StandardOutputEncoding = Encoding.UTF8, + StandardErrorEncoding = Encoding.UTF8, + WorkingDirectory = defaultArg workingDirectory (Path.GetTempPath()) + ) + + for argument in arguments do + startInfo.ArgumentList.Add argument + + startInfo + + let session = new Process(StartInfo = startInfo) + + do + session.OutputDataReceived.Add(fun e -> + match e.Data with + | null -> () + | line -> lock outputLock (fun () -> standardOutput.AppendLine line |> ignore)) + + session.ErrorDataReceived.Add(fun e -> + match e.Data with + | null -> () + | line -> lock outputLock (fun () -> standardError.AppendLine line |> ignore)) + + session.Start() |> ignore + session.BeginOutputReadLine() + session.BeginErrorReadLine() + + let pipe = + let pipe = + new NamedPipeClientStream(".", pipeName, PipeDirection.InOut, PipeOptions.Asynchronous) + + try + pipe.Connect 60_000 + with e -> + let detail = + if session.HasExited then + sprintf "The session exited with code %d." session.ExitCode + else + "The session is still running." + + failwithf "Could not connect to the session on pipe '%s'. %s\n%s" pipeName detail e.Message + + pipe + + let rpc = + let rpc = new JsonRpc(new HeaderDelimitedMessageHandler(pipe, new JsonMessageFormatter())) + rpc.StartListening() + rpc + + let await (work: Task<'T>) (timeout: TimeSpan) = + if not (work.Wait timeout) then + failwith "The session did not answer in time." + + work.Result + + member _.StandardOutput = lock outputLock (fun () -> standardOutput.ToString()) + + member _.StandardError = lock outputLock (fun () -> standardError.ToString()) + + member _.HasExited = session.HasExited + + member _.ProcessId = session.Id + + member _.WaitForExit(milliseconds: int) = session.WaitForExit milliseconds + + /// Wait until the session's own output contains the given text, which is how a test observes + /// what a script printed rather than what the protocol returned. + member this.WaitForOutput(text: string, ?timeout: TimeSpan) = + let deadline = DateTime.UtcNow + defaultArg timeout (TimeSpan.FromSeconds 30.0) + + let rec wait () = + if this.StandardOutput.Contains text then true + elif DateTime.UtcNow > deadline then false + else + Thread.Sleep 50 + wait () + + wait () + + /// Send a request whose parameters are a single object, as every method of this protocol but + /// the argument-less ones expects. + member _.BeginRequest<'T>(method: string, parameters: obj) : Task<'T> = + rpc.InvokeWithParameterObjectAsync<'T>(method, parameters) + + member _.BeginRequest<'T>(method: string) : Task<'T> = rpc.InvokeAsync<'T>(method) + + member _.EndRequest(work: Task<'T>, ?timeout: TimeSpan) = + await work (defaultArg timeout defaultTimeout) + + member this.Request<'T>(method: string, parameters: obj, ?timeout: TimeSpan) : 'T = + await (this.BeginRequest<'T>(method, parameters)) (defaultArg timeout defaultTimeout) + + member this.Request<'T>(method: string, ?timeout: TimeSpan) : 'T = + await (this.BeginRequest<'T> method) (defaultArg timeout defaultTimeout) + + /// Issue a request expected to fail, returning the JSON-RPC error code the session reported. + /// + /// An unknown method surfaces as its own exception type rather than as a reported error, so it + /// is mapped back to the code the specification gives it. + member this.RequestExpectingError(method: string, parameters: obj) = + let classify (e: exn) = + match e with + | :? RemoteMethodNotFoundException -> Some -32601 + | :? RemoteInvocationException as remote -> Some remote.ErrorCode + | _ -> None + + try + this.Request(method, parameters) |> ignore + None + with e -> + let reported = + match e with + | :? AggregateException as aggregate -> classify aggregate.InnerException + | e -> classify e + + match reported with + | Some code -> Some code + | None -> raise e + + /// Perform the handshake every host makes before submitting anything. + member this.Initialize(?clientProcessId: int) = + let clientProcessId = + defaultArg clientProcessId (Process.GetCurrentProcess().Id) + + this.Request(Methods.Initialize, { clientProcessId = clientProcessId }) + + static member ExecuteParams(code: string, ?sourcePath: string, ?startLine: int) : ExecuteRequest = + { + code = code + sourcePath = Option.toObj sourcePath + startLine = + match startLine with + | Some line -> Nullable line + | None -> Nullable() + } + + /// Submit one interaction and return the structured result. + member this.Execute(code: string, ?sourcePath: string, ?startLine: int, ?timeout: TimeSpan) = + this.Request( + Methods.Execute, + FsiServerHarness.ExecuteParams(code, ?sourcePath = sourcePath, ?startLine = startLine), + ?timeout = timeout + ) + + interface IDisposable with + member _.Dispose() = + try + rpc.Dispose() + with _ -> + () + + try + pipe.Dispose() + with _ -> + () + + try + if not session.HasExited then + session.Kill() + + session.WaitForExit 10_000 |> ignore + with _ -> + () + + session.Dispose() + +//------------------------------------------------------------------------- +// Reading the pieces of a result +//------------------------------------------------------------------------- + +let diagnostics (result: ExecutionResult) = + match result.diagnostics with + | null -> [||] + | items -> items + +let errors result = + diagnostics result |> Array.filter (fun d -> d.severity = "error") + +let warnings result = + diagnostics result |> Array.filter (fun d -> d.severity = "warning") + +let succeeded (result: ExecutionResult) = result.success + +let exceptionMessage (result: ExecutionResult) = + match box result.``exception`` with + | null -> None + | _ -> Some result.``exception``.message + +/// Render a result for a failure message. +let describeResult (result: ExecutionResult) = + let diagnosticText = + diagnostics result + |> Array.map (fun d -> sprintf "%s(%d,%d): %s FS%04d: %s" d.fileName d.startLine d.startColumn d.severity d.errorNumber d.message) + |> String.concat "\n " + + sprintf + "success=%b cancelled=%b workingDirectory=%s exception=%s\n %s" + result.success + result.cancelled + result.workingDirectory + (match exceptionMessage result with + | Some m -> m + | None -> "") + diagnosticText From f76451cc1096ff280023974efba7f2b9b619ade3 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 30 Aug 2026 00:49:10 +0200 Subject: [PATCH 02/30] Point the release notes entry at the pull request Co-Authored-By: Claude Fable 5 --- docs/release-notes/.FSharp.Compiler.Service/11.0.100.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index 172e89b961b..fde6bba6430 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,6 +1,6 @@ ### Added -* F# Interactive gains a JSON-RPC server mode, `--fsi-server-jsonrpc:`, in which a host submits interactions over a named pipe and receives structured results — diagnostics with positions, escaping exceptions, and the session's own process id — instead of recovering them by looking for a `SERVER-PROMPT>` marker in the output text. Program output continues to flow through the redirected console streams. ([PR #20360](https://github.com/dotnet/fsharp/pull/20360)) +* F# Interactive gains a JSON-RPC server mode, `--fsi-server-jsonrpc:`, in which a host submits interactions over a named pipe and receives structured results — diagnostics with positions, escaping exceptions, and the session's own process id — instead of recovering them by looking for a `SERVER-PROMPT>` marker in the output text. Program output continues to flow through the redirected console streams. ([PR #20396](https://github.com/dotnet/fsharp/pull/20396)) ### Fixed From 43524ba4a2838ec42f8d89a031e9c07c1395fa24 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 30 Aug 2026 01:25:22 +0200 Subject: [PATCH 03/30] Trim the comments to the repository style rule NoBloat asks that comments answer a "why" no name can express, and keeps rationale for design choices in the commit message rather than in the code. Comments only; no code changes. Co-Authored-By: Claude Fable 5 --- src/Compiler/Interactive/fsi.fs | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/Compiler/Interactive/fsi.fs b/src/Compiler/Interactive/fsi.fs index 3835bb36bb4..ea7a426b205 100644 --- a/src/Compiler/Interactive/fsi.fs +++ b/src/Compiler/Interactive/fsi.fs @@ -988,8 +988,6 @@ type internal FsiCommandLineOptions(fsi: FsiEvaluationSessionHostConfig, argv: s let mutable fsiServerOutputCodePage = None let mutable fsiLCID = None - /// Set by --fsi-server-jsonrpc. The host submits interactions over a named pipe rather than - /// through standard input, and reads structured results rather than parsing the output text. let mutable fsiServerJsonRpcPipe = "" // internal options @@ -998,8 +996,7 @@ type internal FsiCommandLineOptions(fsi: FsiEvaluationSessionHostConfig, argv: s let isJsonRpcServer () = fsiServerJsonRpcPipe <> "" - // Both server modes are driven by a host rather than a user at a console, so neither one uses - // the console reader. + // Neither server mode has a user at a console, so neither uses the console reader. let isInteractiveServer () = fsiServerName <> "" || isJsonRpcServer () let recordExplicitArg arg = explicitArgs <- explicitArgs @ [ arg ] @@ -1394,11 +1391,8 @@ type internal FsiCommandLineOptions(fsi: FsiEvaluationSessionHostConfig, argv: s member _.IsInteractiveServer = isInteractiveServer () - /// True when the host drives this session over JSON-RPC. Interactions then arrive on the - /// control channel instead of standard input, and no prompt is written to the output. - /// - /// The pipe name itself is not surfaced here: the server lives in the process entry point, - /// which reads it from the command line directly. + /// The pipe name is not surfaced: the server lives in the process entry point, which reads it + /// from the command line directly. member _.IsJsonRpcServer = isJsonRpcServer () member _.ProbeToSeeIfConsoleWorks = probeToSeeIfConsoleWorks @@ -1494,8 +1488,7 @@ type internal FsiConsolePrompt(fsiOptions: FsiCommandLineOptions, fsiConsoleOutp // A prompt can be skipped by "silent directives", e.g. ones sent to FSI by VS. let mutable dropPrompt = 0 - // A host driving the session over JSON-RPC learns when an interaction finished from the - // response to its request, so a prompt in the output stream would be noise. + // A JSON-RPC host learns an interaction finished from the response to its request. let mutable showPrompt = not fsiOptions.IsJsonRpcServer // NOTE: SERVER-PROMPT is not user displayed, rather it's a prefix that code elsewhere @@ -5127,8 +5120,8 @@ type FsiEvaluationSession // We later switch to doing interaction-by-interaction processing on the "event loop" thread let ctokRun = AssumeCompilationThreadWithoutEvidence() - // The legacy server channel carries interrupt requests only. A JSON-RPC host carries them - // on its own connection, and starts that server from the process entry point. + // The JSON-RPC server carries interrupts on its own connection and is started by the + // process entry point. if fsiOptions.IsInteractiveServer && not fsiOptions.IsJsonRpcServer then SpawnInteractiveServer(fsi, fsiOptions, fsiConsoleOutput) @@ -5149,8 +5142,7 @@ type FsiEvaluationSession fsiInteractionProcessor.LoadInitialFiles(ctokRun, diagnosticsLogger) - // A JSON-RPC host submits interactions on the control channel, which leaves standard - // input to the script being run. + // Interactions arrive on the control channel, leaving stdin to the script. if not fsiOptions.IsJsonRpcServer then fsiInteractionProcessor.StartStdinReadAndProcessThread(tcConfigB.diagnosticsOptions, diagnosticsLogger) From 833c01d29cab17ded999a2e0ba5e45f9c6b782ba Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Wed, 2 Sep 2026 21:33:20 +0200 Subject: [PATCH 04/30] Run fantomas on the files it actually covers vsintegration is in .fantomasignore; src/fsi and src/Compiler are not, and the JSON-RPC work never ran the formatter over them. CheckCodeFormatting failed on exactly these four files. Co-Authored-By: Claude Fable 5 --- src/Compiler/Interactive/fsi.fs | 4 +++- src/fsi/fsimain.fs | 3 ++- src/fsi/fsiserver.fs | 10 +++++++--- src/fsi/interactiveProtocol.fs | 23 +++++++++++++++++------ 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/Compiler/Interactive/fsi.fs b/src/Compiler/Interactive/fsi.fs index ea7a426b205..dea14d2b6a5 100644 --- a/src/Compiler/Interactive/fsi.fs +++ b/src/Compiler/Interactive/fsi.fs @@ -997,7 +997,9 @@ type internal FsiCommandLineOptions(fsi: FsiEvaluationSessionHostConfig, argv: s let isJsonRpcServer () = fsiServerJsonRpcPipe <> "" // Neither server mode has a user at a console, so neither uses the console reader. - let isInteractiveServer () = fsiServerName <> "" || isJsonRpcServer () + let isInteractiveServer () = + fsiServerName <> "" || isJsonRpcServer () + let recordExplicitArg arg = explicitArgs <- explicitArgs @ [ arg ] let executableFileNameWithoutExtension = diff --git a/src/fsi/fsimain.fs b/src/fsi/fsimain.fs index 9c09e8163cb..6e7774feef4 100644 --- a/src/fsi/fsimain.fs +++ b/src/fsi/fsimain.fs @@ -352,7 +352,8 @@ let evaluateSession (argv: string[]) = // Serve the host on a background thread, leaving this thread to Run() and the event loop // that interactions are evaluated on. match jsonRpcPipeName with - | Some pipeName -> FSharp.Compiler.Interactive.Server.startOnBackgroundThread fsiSession fsiConfig pipeName Console.Out Console.Error + | Some pipeName -> + FSharp.Compiler.Interactive.Server.startOnBackgroundThread fsiSession fsiConfig pipeName Console.Out Console.Error | None -> () // Start the session diff --git a/src/fsi/fsiserver.fs b/src/fsi/fsiserver.fs index ab2b1ddbef8..bca16b7c740 100644 --- a/src/fsi/fsiserver.fs +++ b/src/fsi/fsiserver.fs @@ -281,8 +281,10 @@ type internal FsiRpcTarget let directives = ResizeArray() - if not (String.IsNullOrWhiteSpace request.workingDirectory) - && Directory.Exists request.workingDirectory then + if + not (String.IsNullOrWhiteSpace request.workingDirectory) + && Directory.Exists request.workingDirectory + then // Two different notions of "current directory" have to agree here. The directive moves // the compiler's, which is what relative #load and #r resolve against; the process one // is what the running script sees when it opens a file by relative path. @@ -362,7 +364,9 @@ let private runServer let target = FsiRpcTarget(fsiSession, fsiConfig, outWriter, errorWriter, shutdownRequested) - use rpc = new JsonRpc(new HeaderDelimitedMessageHandler(pipe, new JsonMessageFormatter())) + use rpc = + new JsonRpc(new HeaderDelimitedMessageHandler(pipe, new JsonMessageFormatter())) + rpc.AddLocalRpcTarget(target, JsonRpcTargetOptions(NotifyClientOfEvents = false, AllowNonPublicInvocation = false)) rpc.StartListening() diff --git a/src/fsi/interactiveProtocol.fs b/src/fsi/interactiveProtocol.fs index 519941092cc..e54939a6533 100644 --- a/src/fsi/interactiveProtocol.fs +++ b/src/fsi/interactiveProtocol.fs @@ -12,12 +12,23 @@ namespace FSharp.Compiler.Interactive.Protocol /// Method names. Both ends use these rather than repeating string literals. module Methods = - let [] Initialize = "fsi/initialize" - let [] Execute = "fsi/execute" - let [] ExecuteFile = "fsi/executeFile" - let [] SetPaths = "fsi/setPaths" - let [] Interrupt = "fsi/interrupt" - let [] Shutdown = "fsi/shutdown" + [] + let Initialize = "fsi/initialize" + + [] + let Execute = "fsi/execute" + + [] + let ExecuteFile = "fsi/executeFile" + + [] + let SetPaths = "fsi/setPaths" + + [] + let Interrupt = "fsi/interrupt" + + [] + let Shutdown = "fsi/shutdown" [] type InitializeRequest = From 3a5fb8e011b190d5a2bbe39a5b3ae0e4043a1e3a Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Wed, 2 Sep 2026 21:33:20 +0200 Subject: [PATCH 05/30] Exempt StreamJsonRpc's serialization stack from Microsoft signing fsi's JSON-RPC server mode ships StreamJsonRpc for real, so its dependencies land in the VSIX's Tools folder and go through official signing. The pipeline refuses to stamp a Microsoft certificate on a third-party-copyrighted binary, which is exactly what broke every Windows leg (WindowsCompressedMetadata, WindowsLangVersionPreview, WindowsNoRealsig): SIGN004 on MessagePack.dll, MessagePack.Annotations.dll, Nerdbank.MessagePack.dll and PolyType.dll. Nerdbank.Streams.dll and Newtonsoft.Json.dll are already exempted here the same way, for the same reason. Co-Authored-By: Claude Fable 5 --- eng/Signing.props | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/eng/Signing.props b/eng/Signing.props index 222ad3dc47a..a5b0fdc9a7c 100644 --- a/eng/Signing.props +++ b/eng/Signing.props @@ -3,6 +3,11 @@ + + + + + From decf7c1ebea64969ade7a8a7a446e37f5a52f622 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Wed, 2 Sep 2026 21:33:21 +0200 Subject: [PATCH 06/30] Exempt the new test project from the embedded-pdb check AssemblyCheck globs every FSharp*.dll under artifacts/bin; every existing test project is listed here because tests/Directory.Build.props sets DebugType to portable, not embedded. The new one wasn't added, so ILVerify failed with 'The following assemblies don't have an embedded pdb'. Co-Authored-By: Claude Fable 5 --- buildtools/AssemblyCheck/SkipVerifyEmbeddedPdb.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/buildtools/AssemblyCheck/SkipVerifyEmbeddedPdb.txt b/buildtools/AssemblyCheck/SkipVerifyEmbeddedPdb.txt index f7d25f2ca25..378d1be728d 100644 --- a/buildtools/AssemblyCheck/SkipVerifyEmbeddedPdb.txt +++ b/buildtools/AssemblyCheck/SkipVerifyEmbeddedPdb.txt @@ -1,6 +1,7 @@ FSharp.Build.UnitTests.dll FSharp.Benchmarks.Common.dll FSharp.Compiler.Benchmarks.dll +FSharp.Compiler.Interactive.Server.Tests.dll FSharp.Compiler.ComponentTests.dll FSharp.Test.Utilities.dll FSharp.Compiler.LanguageServer.Tests.dll From 666d54dbe5376b66212575c6c8161bafcfadec3b Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Wed, 2 Sep 2026 22:52:50 +0200 Subject: [PATCH 07/30] Register the new test project with TestSplit.fsx The interactive server tests are a genuine, currently-passing test project (23 tests, real fsi subprocesses), not a shared library or one already covered by a dedicated job, so otherProjects is where it belongs rather than the exclusion lists. Single TFM (net11.0), so it runs from the coreclr leg only, alongside its Service.Tests and Scripting.UnitTests siblings in batch 2. --validate was the failure the last push actually hit: CheckCodeFormatting's job runs both the fantomas check (which passed) and this registration check (which didn't) as separate steps, and a job fails if either does. Co-Authored-By: Claude Fable 5 --- eng/tests/TestSplit.fsx | 1 + 1 file changed, 1 insertion(+) diff --git a/eng/tests/TestSplit.fsx b/eng/tests/TestSplit.fsx index dfb7724142c..488fa771e28 100644 --- a/eng/tests/TestSplit.fsx +++ b/eng/tests/TestSplit.fsx @@ -48,6 +48,7 @@ let otherProjects = "tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj", 2, "all" "tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.Tests.fsproj", 2, "all" "tests/FSharp.Compiler.Private.Scripting.UnitTests/FSharp.Compiler.Private.Scripting.UnitTests.fsproj", 2, "all" + "tests/FSharp.Compiler.Interactive.Server.Tests/FSharp.Compiler.Interactive.Server.Tests.fsproj", 2, "coreclr" "tests/fsharp/FSharpSuite.Tests.fsproj", 3, "desktop" ] From dfc0b8966011e777d616498d78794244e7f3d806 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Wed, 2 Sep 2026 23:47:18 +0200 Subject: [PATCH 08/30] Add the new test project to VisualFSharp.slnx too FSharp.slnx and VisualFSharp.slnx are two different solutions: Windows CI legs build the latter (it carries vsintegration) but always test the former, via separate BuildSolution and TestUsingMSBuild calls in eng/Build.ps1. The project was only added to FSharp.slnx, so those legs never built it, yet `dotnet test --solution FSharp.slnx --no-build` still found it listed there with no build output behind it - which it reported as "using VSTest test runner" instead of a build failure. Its siblings (Service.Tests, Private.Scripting.UnitTests) are in both files; this makes the new project consistent with them. Co-Authored-By: Claude Fable 5 --- VisualFSharp.slnx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/VisualFSharp.slnx b/VisualFSharp.slnx index 9f6eed02f50..caa059a34ee 100644 --- a/VisualFSharp.slnx +++ b/VisualFSharp.slnx @@ -40,6 +40,9 @@ + + + From 9299f03c4274a345c0481387a90831b50bfeb430 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 3 Sep 2026 19:56:46 +0200 Subject: [PATCH 09/30] Run the interactive server tests on desktop fsi too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Multi-targeted the project the way its siblings already are, with the same Unix/BUILDING_USING_DOTNET downgrade to coreclr-only that they carry. Without it, the testDesktop CI leg tried to launch a net472 exe that was never built, failing outright before a single test ran. That flavor needed its own launch, not just a build target: net472's fsi is a native executable that runs directly, not a managed dll under the dotnet host, so locateFsi now returns which to use, matching the split InteractiveHost.fs already makes for the window. net472 also has no ProcessStartInfo.ArgumentList, so the command line is built by hand on every target now — one fewer thing that differs between the two hosting flavors. Verified locally on both net11.0 and net472 (23/23), with BUILDING_USING_DOTNET=false to match how the CI build phase actually invokes this. Co-Authored-By: Claude Fable 5 --- ...p.Compiler.Interactive.Server.Tests.fsproj | 3 +- .../FsiServerHarness.fs | 95 +++++++++++-------- 2 files changed, 59 insertions(+), 39 deletions(-) diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FSharp.Compiler.Interactive.Server.Tests.fsproj b/tests/FSharp.Compiler.Interactive.Server.Tests/FSharp.Compiler.Interactive.Server.Tests.fsproj index f77de46baa7..e9e568cc6a1 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FSharp.Compiler.Interactive.Server.Tests.fsproj +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FSharp.Compiler.Interactive.Server.Tests.fsproj @@ -1,7 +1,8 @@ - $(FSharpNetCoreProductTargetFramework) + net472;$(FSharpNetCoreProductTargetFramework) + $(FSharpNetCoreProductTargetFramework) Exe false diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs index 596384a5216..4181b9a2523 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs @@ -25,26 +25,6 @@ open FSharp.Compiler.Interactive.Protocol /// of a session pays for the type checker warming up. let private defaultTimeout = TimeSpan.FromSeconds 120.0 -/// Locate the fsi built by this repository, alongside the test assembly's own output. -/// -/// Test output lives at `/bin///`, and fsi is its -/// sibling at `/bin/fsi//`. -let private locateFsi () = - let baseDirectory = - DirectoryInfo(AppContext.BaseDirectory.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)) - - let framework = baseDirectory.Name - let configuration = baseDirectory.Parent.Name - let binDirectory = baseDirectory.Parent.Parent.Parent - - let fsi = - Path.Combine(binDirectory.FullName, "fsi", configuration, framework, "fsi.dll") - - if not (File.Exists fsi) then - failwithf "Could not find the fsi under test at '%s'. Build src/fsi first." fsi - - fsi - /// Prefer the .NET host this repository provisions, so that the session runs on the same runtime /// as the rest of the build. let private locateDotnetHost () = @@ -67,6 +47,37 @@ let private locateDotnetHost () = search (DirectoryInfo(AppContext.BaseDirectory)) +/// Locate the fsi built by this repository, alongside the test assembly's own output, and how to +/// launch it. +/// +/// Test output lives at `/bin///`, and fsi is its +/// sibling at `/bin/fsi//`. net472's fsi is a native +/// executable that runs directly; every other framework's is a managed dll run under the dotnet +/// host — the same split `InteractiveHost.fs` makes for the window. +let private locateFsi () = + let baseDirectory = + DirectoryInfo(AppContext.BaseDirectory.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)) + + let framework = baseDirectory.Name + let configuration = baseDirectory.Parent.Name + let binDirectory = baseDirectory.Parent.Parent.Parent + let fsiDirectory = Path.Combine(binDirectory.FullName, "fsi", configuration, framework) + + if framework = "net472" then + let fsi = Path.Combine(fsiDirectory, "fsi.exe") + + if not (File.Exists fsi) then + failwithf "Could not find the fsi under test at '%s'. Build src/fsi first." fsi + + fsi, [] + else + let fsi = Path.Combine(fsiDirectory, "fsi.dll") + + if not (File.Exists fsi) then + failwithf "Could not find the fsi under test at '%s'. Build src/fsi first." fsi + + locateDotnetHost (), [ fsi ] + /// A running session, plus everything needed to talk to it and to explain a failure. [] type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = @@ -75,32 +86,40 @@ type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = let standardError = StringBuilder() let outputLock = obj () + /// .NET Framework has no `ProcessStartInfo.ArgumentList`, so the command line is built by hand + /// on every target — one fewer thing that differs between fsi's two hosting flavors. + let quoteIfNeeded (argument: string) = + if + argument.IndexOf(" ", StringComparison.Ordinal) >= 0 + && not (argument.StartsWith("\"", StringComparison.Ordinal)) + then + "\"" + argument + "\"" + else + argument + let startInfo = + let fsiHost, leadingArguments = locateFsi () + let arguments = [ - locateFsi () + yield! leadingArguments "--nologo" "--fsi-server-jsonrpc:" + pipeName yield! defaultArg extraArguments [] ] - let startInfo = - ProcessStartInfo( - FileName = locateDotnetHost (), - UseShellExecute = false, - CreateNoWindow = true, - RedirectStandardInput = true, - RedirectStandardOutput = true, - RedirectStandardError = true, - StandardOutputEncoding = Encoding.UTF8, - StandardErrorEncoding = Encoding.UTF8, - WorkingDirectory = defaultArg workingDirectory (Path.GetTempPath()) - ) - - for argument in arguments do - startInfo.ArgumentList.Add argument - - startInfo + ProcessStartInfo( + FileName = fsiHost, + Arguments = String.Join(" ", arguments |> List.map quoteIfNeeded), + UseShellExecute = false, + CreateNoWindow = true, + RedirectStandardInput = true, + RedirectStandardOutput = true, + RedirectStandardError = true, + StandardOutputEncoding = Encoding.UTF8, + StandardErrorEncoding = Encoding.UTF8, + WorkingDirectory = defaultArg workingDirectory (Path.GetTempPath()) + ) let session = new Process(StartInfo = startInfo) From 71a7b30d48b758f0be20fec0f4a644175e9db1f2 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 3 Sep 2026 20:08:15 +0200 Subject: [PATCH 10/30] Surface what StreamJsonRpc the session loaded on a method-not-found failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every CI leg but testDesktop fails all 23 tests the same way: the pipe connects, but the target's own Initialize is reported as not found. That requires an active, listening server that plainly doesn't have the method registered — the code makes that impossible by construction (AddLocalRpcTarget completes before StartListening even runs), and four different repros (Release, --solution with siblings, the packaging build's own second compile of fsi.dll) all pass locally. The one thing every failing repro shares that I haven't been able to reproduce is CI's own machine: if that fsi process resolves a second, different copy of StreamJsonRpc.dll somewhere on its probing path, its JsonRpcMethodAttribute wouldn't equal the one FsiRpcTarget was compiled against, and AddLocalRpcTarget's reflection scan would find nothing to register — exactly this symptom, and only there. fsi now writes which StreamJsonRpc assembly and version it loaded to its own stderr right after registering the target, and a request that fails folds the session's captured stdout and stderr into the exception instead of letting it report just a bare message. The next CI failure carries its own answer. RequestExpectingError's classification now descends through causes instead of unwrapping one level, since the enriched exception is itself a wrapper. Co-Authored-By: Claude Fable 5 --- src/fsi/fsiserver.fs | 9 ++++++ .../FsiServerHarness.fs | 29 ++++++++++++------- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/src/fsi/fsiserver.fs b/src/fsi/fsiserver.fs index bca16b7c740..fcede5e08fa 100644 --- a/src/fsi/fsiserver.fs +++ b/src/fsi/fsiserver.fs @@ -368,6 +368,15 @@ let private runServer new JsonRpc(new HeaderDelimitedMessageHandler(pipe, new JsonMessageFormatter())) rpc.AddLocalRpcTarget(target, JsonRpcTargetOptions(NotifyClientOfEvents = false, AllowNonPublicInvocation = false)) + + // Diagnostic breadcrumb: a host that gets "method not found" against a target that plainly + // declares the method has almost certainly loaded a second, different copy of this library, so + // its identity here is worth more than the rest of the trace. + errorWriter.WriteLine( + sprintf "FSI-SERVER: StreamJsonRpc %O from %s" (typeof.Assembly.GetName().Version) typeof.Assembly.Location + ) + + errorWriter.Flush() rpc.StartListening() // Either the host goes away or it asks to stop. Both end the session. diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs index 4181b9a2523..1c6a9e1f78a 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs @@ -160,11 +160,18 @@ type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = rpc.StartListening() rpc + /// On failure, fold in what the session itself printed — the only way to see, say, which + /// StreamJsonRpc the session actually loaded if a request comes back "method not found" + /// against a target that plainly declares it. let await (work: Task<'T>) (timeout: TimeSpan) = - if not (work.Wait timeout) then - failwith "The session did not answer in time." + try + if not (work.Wait timeout) then + failwith "The session did not answer in time." - work.Result + work.Result + with e -> + let output, error = lock outputLock (fun () -> standardOutput.ToString(), standardError.ToString()) + raise (Exception($"{e.Message}\n-- stdout --\n{output}-- stderr --\n{error}", e)) member _.StandardOutput = lock outputLock (fun () -> standardOutput.ToString()) @@ -211,22 +218,22 @@ type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = /// An unknown method surfaces as its own exception type rather than as a reported error, so it /// is mapped back to the code the specification gives it. member this.RequestExpectingError(method: string, parameters: obj) = - let classify (e: exn) = + // `await` folds diagnostic text into a wrapping exception on any failure (see above), so + // the type this classifies on is found by descending through causes, not just one level. + let rec classify (e: exn) = match e with | :? RemoteMethodNotFoundException -> Some -32601 | :? RemoteInvocationException as remote -> Some remote.ErrorCode - | _ -> None + | _ -> + match e.InnerException with + | null -> None + | inner -> classify inner try this.Request(method, parameters) |> ignore None with e -> - let reported = - match e with - | :? AggregateException as aggregate -> classify aggregate.InnerException - | e -> classify e - - match reported with + match classify e with | Some code -> Some code | None -> raise e From 2318726b926ad35f74c536264e14e1a738b13769 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 3 Sep 2026 21:49:59 +0200 Subject: [PATCH 11/30] Stop --realsig- from hiding FsiRpcTarget's methods from StreamJsonRpc The real root cause of every remaining CI failure: coreclr_release, transparent_compiler_release, testCoreclr and Linux all failed every one of the 23 tests with StreamJsonRpc.RemoteMethodNotFoundException on all six RPC methods, not just the one each test happened to call first. That is what AddLocalRpcTarget's reflection scan finding zero eligible members looks like, not a race - the ordering that rules out a race was already right. Confirmed by diffing FsiRpcTarget's compiled metadata between a --realsig+ and a --realsig- build of the same source, using System.Reflection.Metadata directly against the TypeDef/MethodDef tables: Initialize and Execute carry IL attribute Public under realsig+, but only Assembly under realsig-. fsi's own diagnostic breadcrumb (added while chasing this) had already shown a correctly located, single copy of StreamJsonRpc, ruling out the version-skew theory that prompted it. --realsig- is not a rare configuration - it is Build.ps1's own default ($buildnorealsig = $true unless a caller opts out), and no CI invocation opts out, so every leg actually building product code builds this way. Every local build in this session defaulted to --realsig+ instead (Directory.Build.props' own fallback when BuildNoRealsig is unset), which is exactly why 23/23 kept passing locally while CI failed the same 23 every time. The mechanism: under --realsig-, a member's own IL visibility is capped by its enclosing scope's, all the way out. FsiRpcTarget's members carry no explicit accessibility (public by default), and the type itself was declared without `internal` too - but it lived inside `module internal FSharp.Compiler.Interactive.Server`, and that module's own internal-ness was still enough to cap every member inside it under realsig-, regardless of what the type or its members individually declared. realsig+ does not apply that cap, which is why an explicit `internal` directly on the type earlier in this investigation changed nothing: the type's own modifier was never the operative one. The module is now public, with everything but FsiRpcTarget marked `private` or explicit `internal` to keep its actual surface exactly as narrow as before - JsonRpcServerOption, startOnBackgroundThread and tryGetPipeName stay assembly- internal for fsimain.fs to call; FsiRpcTarget is the one thing that genuinely needs to be reflectable from outside the assembly, because that is what AddLocalRpcTarget does to the instance it is handed. Verified 23/23 under both --realsig- (BuildNoRealsig=true, matching every CI invocation) and --realsig+ (the local default), so this is not a trade against the setting nobody was testing under. Co-Authored-By: Claude Fable 5 --- src/fsi/fsiserver.fs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/fsi/fsiserver.fs b/src/fsi/fsiserver.fs index fcede5e08fa..2244b1b270f 100644 --- a/src/fsi/fsiserver.fs +++ b/src/fsi/fsiserver.fs @@ -18,7 +18,12 @@ /// behave as they do at the console. Interactions are queued onto a single worker so that they run /// in the order they arrived, while requests that must not wait behind them — an interrupt above /// all — are served as they arrive. -module internal FSharp.Compiler.Interactive.Server +/// +/// Not `module internal`: `FsiRpcTarget` needs its members to be genuinely public IL, and under +/// `--realsig-` a member's own accessibility is capped by its enclosing module's, so an internal +/// module would take that away no matter what the type itself declares. Everything else here goes +/// back to `private`/`internal` explicitly instead of inheriting it from the module. +module FSharp.Compiler.Interactive.Server open System open System.Collections.Concurrent @@ -37,7 +42,7 @@ open FSharp.Compiler.Interactive.Shell /// The name of the command line option that turns on this server. [] -let JsonRpcServerOption = "--fsi-server-jsonrpc:" +let internal JsonRpcServerOption = "--fsi-server-jsonrpc:" /// File name reported for interactions that the host did not attribute to a source file. [] @@ -138,8 +143,13 @@ type private ExecutionQueue() = /// /// Everything that evaluates code goes onto the execution queue and completes its task when the /// interaction finishes, which leaves StreamJsonRpc free to dispatch an interrupt in the meantime. +/// +/// Public, not `internal`: AddLocalRpcTarget discovers `[]` members by reflecting +/// over the instance it is handed, and under `--realsig-` a member's own IL visibility is capped by +/// its enclosing scope's, so an internal type (or an internal module around a public one) would +/// take away the public visibility that reflection needs regardless of what the members declare. [] -type internal FsiRpcTarget +type FsiRpcTarget ( fsiSession: FsiEvaluationSession, fsiConfig: FsiEvaluationSessionHostConfig, @@ -391,7 +401,7 @@ let private runServer /// Start the server on a background thread and return, leaving the caller's thread free to drive /// the event loop. Mirrors how a console session spawns its standard input reader. -let startOnBackgroundThread +let internal startOnBackgroundThread (fsiSession: FsiEvaluationSession) (fsiConfig: FsiEvaluationSessionHostConfig) (pipeName: string) @@ -417,7 +427,7 @@ let startOnBackgroundThread thread.Start() /// Recognise `--fsi-server-jsonrpc:` in a command line, returning the pipe name. -let tryGetPipeName (argv: string[]) = +let internal tryGetPipeName (argv: string[]) = argv |> Array.tryPick (fun arg -> if arg.StartsWith(JsonRpcServerOption, StringComparison.Ordinal) then From 337991c4f6d90cda90a1ebf24c736bfea5bc61a2 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 10 Sep 2026 02:57:39 +0200 Subject: [PATCH 12/30] Keep the execution queue off the wire and move the directory on it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit StreamJsonRpc offers every public member of the target it is given, not only the attributed ones, so `Complete` was reachable as a request: a host could close the execution queue and every later interaction would wait on a task nothing completes. The queue now belongs to the server loop, the target takes it through an internal constructor, and enqueueing after the close fails the request instead of dropping it. `fsi/setPaths` moved the process directory as the request arrived, which is before its own `#silentCd` runs and while an earlier interaction may still be executing — that interaction would see the later request's directory. The whole of it now runs on the queue. Co-Authored-By: Claude Opus 5 (1M context) --- src/fsi/fsiserver.fs | 107 +++++++++++------- src/fsi/interactiveProtocol.fs | 4 + .../FsiJsonRpcServerTests.fs | 65 +++++++++++ 3 files changed, 134 insertions(+), 42 deletions(-) diff --git a/src/fsi/fsiserver.fs b/src/fsi/fsiserver.fs index 2244b1b270f..a9cde8c323b 100644 --- a/src/fsi/fsiserver.fs +++ b/src/fsi/fsiserver.fs @@ -111,8 +111,11 @@ let private toExecutionResult (outcome: Choice) (diagnosti /// Serialises the interactions submitted by the host onto a single worker, so that they are /// evaluated strictly in the order they were received. +/// +/// Owned by the server loop rather than by the target the host calls into: closing the queue ends +/// the session's willingness to run anything, and must not be reachable from the wire. [] -type private ExecutionQueue() = +type internal ExecutionQueue() = let queue = new BlockingCollection unit>() let worker = @@ -131,13 +134,17 @@ type private ExecutionQueue() = do worker.Start() - member _.Enqueue(job: unit -> unit) = - if not queue.IsAddingCompleted then + /// False once the queue is closed, when the job will never run. Checking `IsAddingCompleted` + /// first would still race with the close, and a job silently dropped leaves the host waiting on + /// a task nothing completes. + member _.TryEnqueue(job: unit -> unit) = + try queue.Add job + true + with :? InvalidOperationException -> + false - member _.Complete() = - if not queue.IsAddingCompleted then - queue.CompleteAdding() + member _.Complete() = queue.CompleteAdding() /// The object the host calls into. /// @@ -148,17 +155,22 @@ type private ExecutionQueue() = /// over the instance it is handed, and under `--realsig-` a member's own IL visibility is capped by /// its enclosing scope's, so an internal type (or an internal module around a public one) would /// take away the public visibility that reflection needs regardless of what the members declare. +/// +/// StreamJsonRpc offers every public member, not only the attributed ones, so the public members +/// here are exactly the handlers the protocol defines. The construction the server loop needs goes +/// through the internal constructor instead. [] type FsiRpcTarget + internal ( fsiSession: FsiEvaluationSession, fsiConfig: FsiEvaluationSessionHostConfig, outWriter: TextWriter, errorWriter: TextWriter, - shutdownRequested: TaskCompletionSource + shutdownRequested: TaskCompletionSource, + executionQueue: ExecutionQueue ) = - let executionQueue = ExecutionQueue() let interruptLock = obj () let mutable currentCancellation: CancellationTokenSource = null let mutable initialized = false @@ -198,16 +210,23 @@ type FsiRpcTarget lock interruptLock (fun () -> currentCancellation <- null) cancellation.Dispose() - /// Queue an interaction and hand back the task the host is waiting on. + /// Queue an interaction and hand back the task the host is waiting on. A request that arrives + /// once the session has stopped accepting work fails, rather than waiting for a turn that will + /// never come. let queueInteraction (run: unit -> ExecutionResult) = let completion = TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously) - executionQueue.Enqueue(fun () -> - try - completion.TrySetResult(run ()) |> ignore - with e -> - completion.TrySetException e |> ignore) + let queued = + executionQueue.TryEnqueue(fun () -> + try + completion.TrySetResult(run ()) |> ignore + with e -> + completion.TrySetException e |> ignore) + + if not queued then + completion.TrySetException(LocalRpcException("The F# Interactive session is shutting down", ErrorCode = -32001)) + |> ignore completion.Task @@ -240,8 +259,6 @@ type FsiRpcTarget // An unknown process id is not fatal: the session simply loses orphan protection. () - member _.Complete() = executionQueue.Complete() - [] member _.Initialize(request: InitializeRequest) : InitializeResult = if request.clientProcessId > 0 then @@ -289,33 +306,37 @@ type FsiRpcTarget member _.SetPaths(request: SetPathsRequest) : Task = requireInitialized () - let directives = ResizeArray() - - if - not (String.IsNullOrWhiteSpace request.workingDirectory) - && Directory.Exists request.workingDirectory - then - // Two different notions of "current directory" have to agree here. The directive moves - // the compiler's, which is what relative #load and #r resolve against; the process one - // is what the running script sees when it opens a file by relative path. - try - Directory.SetCurrentDirectory request.workingDirectory - with _ -> - () + // The process directory moves on the queue, alongside the directive that moves the + // compiler's: doing it as the request arrives would move it under an earlier interaction + // that is still running. + queueInteraction (fun () -> + let directives = ResizeArray() + + if + not (String.IsNullOrWhiteSpace request.workingDirectory) + && Directory.Exists request.workingDirectory + then + // Two different notions of "current directory" have to agree here. The directive + // moves the compiler's, which is what relative #load and #r resolve against; the + // process one is what the running script sees when it opens a file by relative path. + try + Directory.SetCurrentDirectory request.workingDirectory + with _ -> + () - directives.Add(sprintf "#silentCd @\"%s\"" request.workingDirectory) + directives.Add(sprintf "#silentCd @\"%s\"" request.workingDirectory) - match request.includePaths with - | null -> () - | paths -> - for path in paths do - if not (String.IsNullOrWhiteSpace path) then - directives.Add(sprintf "#I @\"%s\"" path) + match request.includePaths with + | null -> () + | paths -> + for path in paths do + if not (String.IsNullOrWhiteSpace path) then + directives.Add(sprintf "#I @\"%s\"" path) - if directives.Count = 0 then - queueInteraction (fun () -> toExecutionResult (Choice1Of2 None) [||] false) - else - queueInteraction (fun () -> runInteraction (String.Join("\n", directives)) DefaultInteractionName) + if directives.Count = 0 then + toExecutionResult (Choice1Of2 None) [||] false + else + runInteraction (String.Join("\n", directives)) DefaultInteractionName) /// Interrupt the interaction in flight. /// @@ -371,8 +392,10 @@ let private runServer let shutdownRequested = TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously) + let executionQueue = ExecutionQueue() + let target = - FsiRpcTarget(fsiSession, fsiConfig, outWriter, errorWriter, shutdownRequested) + FsiRpcTarget(fsiSession, fsiConfig, outWriter, errorWriter, shutdownRequested, executionQueue) use rpc = new JsonRpc(new HeaderDelimitedMessageHandler(pipe, new JsonMessageFormatter())) @@ -397,7 +420,7 @@ let private runServer // disappears from under it. Task.Delay(250).Wait() - target.Complete() + executionQueue.Complete() /// Start the server on a background thread and return, leaving the caller's thread free to drive /// the event loop. Mirrors how a console session spawns its standard input reader. diff --git a/src/fsi/interactiveProtocol.fs b/src/fsi/interactiveProtocol.fs index e54939a6533..800fd1bd7c3 100644 --- a/src/fsi/interactiveProtocol.fs +++ b/src/fsi/interactiveProtocol.fs @@ -8,6 +8,10 @@ /// /// The members are named as they appear on the wire, and the records are `CLIMutable` so that the /// JSON-RPC formatter can construct them. +/// +/// Public rather than internal, even in fsi's own copy: the handlers that carry them have to be +/// public for StreamJsonRpc to find them by reflection, and a public member cannot expose a type +/// less accessible than itself. namespace FSharp.Compiler.Interactive.Protocol /// Method names. Both ends use these rather than repeating string literals. diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs index e0bf3c8cfd4..dc3c24b59ce 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs @@ -65,6 +65,19 @@ let ``unknown methods are refused`` () = | None -> failwith "the session accepted an unknown method" | Some code -> Assert.Equal(-32601, code)) +[] +let ``only the protocol's own methods are reachable`` () = + withInitializedSession (fun session -> + // StreamJsonRpc offers every public member of the target it is given, so a member that + // closed the execution queue would let a host silently stop the session from ever running + // another interaction. + match session.RequestExpectingError("Complete", obj ()) with + | None -> failwith "the session accepted a method that is not part of the protocol" + | Some code -> Assert.Equal(-32601, code) + + let result = session.Execute "1 + 1" + Assert.True(succeeded result, describe session result)) + //------------------------------------------------------------------------- // Evaluating interactions //------------------------------------------------------------------------- @@ -259,6 +272,58 @@ let ``setPaths changes the working directory`` () = with _ -> ()) +[] +let ``setPaths waits its turn behind a running interaction`` () = + withInitializedSession (fun session -> + let directory = + Path.Combine(Path.GetTempPath(), sprintf "fsiServerTest_%s" (Guid.NewGuid().ToString "N")) + + Directory.CreateDirectory directory |> ignore + + try + // Warm the session up, so that the interaction below is genuinely running by the time + // the request to move the directory arrives. + let warmUp = session.Execute "1" + Assert.True(succeeded warmUp, describe session warmUp) + + let running = + session.BeginRequest( + Methods.Execute, + FsiServerHarness.ExecuteParams + """ +System.Threading.Thread.Sleep 5000 +printfn "interaction saw [%s]" (System.IO.Directory.GetCurrentDirectory()) +""" + ) + + Thread.Sleep 2000 + + let moved = + session.Request( + Methods.SetPaths, + { + includePaths = [||] + workingDirectory = directory + } + ) + + Assert.True(succeeded moved, describe session moved) + + let result = session.EndRequest(running, TimeSpan.FromSeconds 60.0) + Assert.True(succeeded result, describe session result) + + // The process directory moves on the queue like everything else, so an interaction that + // was already running keeps the directory it started in. + Assert.True( + session.WaitForOutput(sprintf "interaction saw [%s]" warmUp.workingDirectory), + describe session result + ) + finally + try + Directory.Delete(directory, true) + with _ -> + ()) + [] let ``reports the working directory after every interaction`` () = withInitializedSession (fun session -> From f2c92ec7ed11034e9f0e75bfbb627b9566b38fb7 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 10 Sep 2026 11:45:48 +0200 Subject: [PATCH 13/30] Interpolate strings and mark up the doc comments in the new code Every `sprintf`/`failwithf` in the server, the protocol and their tests becomes an interpolated string, keeping the format specifiers so the types are still checked. Doc comments that carry more than one paragraph get `` and ``, with code references in `` rather than backticks; `` inside the option's description is escaped, since it now sits in XML. Co-Authored-By: Claude Opus 5 (1M context) --- src/fsi/fsiserver.fs | 107 +++++++++++------- src/fsi/interactiveProtocol.fs | 32 ++++-- .../FsiJsonRpcServerTests.fs | 22 ++-- .../FsiServerHarness.fs | 29 ++--- 4 files changed, 114 insertions(+), 76 deletions(-) diff --git a/src/fsi/fsiserver.fs b/src/fsi/fsiserver.fs index a9cde8c323b..93d38ab1913 100644 --- a/src/fsi/fsiserver.fs +++ b/src/fsi/fsiserver.fs @@ -1,28 +1,37 @@ // Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. -/// The JSON-RPC server mode of F# Interactive, activated by `--fsi-server-jsonrpc:`. -/// +/// +/// The JSON-RPC server mode of F# Interactive, activated by --fsi-server-jsonrpc:<pipe name>. +/// +/// +/// /// An editor hosting F# Interactive needs two things from the process: a control channel to submit /// interactions and receive structured results, and the program's own console output. This server /// keeps those apart. Control traffic is JSON-RPC over a named pipe; everything the script itself /// prints continues to flow through the redirected standard output and error streams, exactly as it /// does for a console session. That separation is what removes the need for a host to recognise /// prompts in the output text in order to tell one interaction's results from the next. -/// +/// +/// /// The transport is StreamJsonRpc over a header-delimited stream, the same combination Roslyn's /// interactive host uses, so a client built on that library talks to this one with its stock /// message handler. -/// +/// +/// /// Threading mirrors the standard input path of a console session. Interactions are evaluated on -/// the event loop thread by way of `EventLoopInvoke`, so scripts that create user interface objects -/// behave as they do at the console. Interactions are queued onto a single worker so that they run -/// in the order they arrived, while requests that must not wait behind them — an interrupt above -/// all — are served as they arrive. -/// -/// Not `module internal`: `FsiRpcTarget` needs its members to be genuinely public IL, and under -/// `--realsig-` a member's own accessibility is capped by its enclosing module's, so an internal -/// module would take that away no matter what the type itself declares. Everything else here goes -/// back to `private`/`internal` explicitly instead of inheriting it from the module. +/// the event loop thread by way of EventLoopInvoke, so scripts that create user interface +/// objects behave as they do at the console. Interactions are queued onto a single worker so that +/// they run in the order they arrived, while requests that must not wait behind them — an interrupt +/// above all — are served as they arrive. +/// +/// +/// Not module internal: FsiRpcTarget needs its members to be genuinely public IL, and +/// under --realsig- a member's own accessibility is capped by its enclosing module's, so an +/// internal module would take that away no matter what the type itself declares. Everything else +/// here goes back to private/internal explicitly instead of inheriting it from the +/// module. +/// +/// module FSharp.Compiler.Interactive.Server open System @@ -109,11 +118,14 @@ let private toExecutionResult (outcome: Choice) (diagnosti // The server //------------------------------------------------------------------------- +/// /// Serialises the interactions submitted by the host onto a single worker, so that they are /// evaluated strictly in the order they were received. -/// +/// +/// /// Owned by the server loop rather than by the target the host calls into: closing the queue ends /// the session's willingness to run anything, and must not be reachable from the wire. +/// [] type internal ExecutionQueue() = let queue = new BlockingCollection unit>() @@ -134,9 +146,11 @@ type internal ExecutionQueue() = do worker.Start() - /// False once the queue is closed, when the job will never run. Checking `IsAddingCompleted` - /// first would still race with the close, and a job silently dropped leaves the host waiting on - /// a task nothing completes. + /// + /// False once the queue is closed, when the job will never run. Checking + /// IsAddingCompleted first would still race with the close, and a job silently dropped + /// leaves the host waiting on a task nothing completes. + /// member _.TryEnqueue(job: unit -> unit) = try queue.Add job @@ -146,19 +160,25 @@ type internal ExecutionQueue() = member _.Complete() = queue.CompleteAdding() -/// The object the host calls into. -/// +/// The object the host calls into. +/// +/// /// Everything that evaluates code goes onto the execution queue and completes its task when the /// interaction finishes, which leaves StreamJsonRpc free to dispatch an interrupt in the meantime. -/// -/// Public, not `internal`: AddLocalRpcTarget discovers `[]` members by reflecting -/// over the instance it is handed, and under `--realsig-` a member's own IL visibility is capped by -/// its enclosing scope's, so an internal type (or an internal module around a public one) would -/// take away the public visibility that reflection needs regardless of what the members declare. -/// +/// +/// +/// Public, not internal: AddLocalRpcTarget discovers JsonRpcMethod members by +/// reflecting over the instance it is handed, and under --realsig- a member's own IL +/// visibility is capped by its enclosing scope's, so an internal type (or an internal module around +/// a public one) would take away the public visibility that reflection needs regardless of what the +/// members declare. +/// +/// /// StreamJsonRpc offers every public member, not only the attributed ones, so the public members /// here are exactly the handlers the protocol defines. The construction the server loop needs goes /// through the internal constructor instead. +/// +/// [] type FsiRpcTarget internal @@ -175,11 +195,14 @@ type FsiRpcTarget let mutable currentCancellation: CancellationTokenSource = null let mutable initialized = false + /// /// Evaluate on the event loop thread, the same thread a console session evaluates on. - /// - /// `EvalInteractionNonThrowing` reports diagnostics and execution failures through its result, - /// but a failure inside the event loop machinery itself would still escape, so it is caught - /// here and reported as an ordinary failed interaction. + /// + /// + /// EvalInteractionNonThrowing reports diagnostics and execution failures through its + /// result, but a failure inside the event loop machinery itself would still escape, so it is + /// caught here and reported as an ordinary failed interaction. + /// let evaluateOnEventLoop (evaluate: unit -> Choice * FSharpDiagnostic[]) = try fsiConfig.EventLoopInvoke evaluate @@ -210,9 +233,11 @@ type FsiRpcTarget lock interruptLock (fun () -> currentCancellation <- null) cancellation.Dispose() + /// /// Queue an interaction and hand back the task the host is waiting on. A request that arrives /// once the session has stopped accepting work fails, rather than waiting for a turn that will /// never come. + /// let queueInteraction (run: unit -> ExecutionResult) = let completion = TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously) @@ -236,7 +261,7 @@ type FsiRpcTarget if String.IsNullOrEmpty sourcePath || not startLine.HasValue then code else - sprintf "# %d @\"%s\"\n%s" startLine.Value sourcePath code + $"# %d{startLine.Value} @\"%s{sourcePath}\"\n%s{code}" /// Refuse anything that arrives before the handshake, so that a mis-sequenced host gets a clear /// answer rather than an obscure failure later on. @@ -298,7 +323,7 @@ type FsiRpcTarget // Routed through #load so that the file joins the session the same way it would from a // script, rather than being replayed as anonymous text. - queueInteraction (fun () -> runInteraction (sprintf "#load @\"%s\"" request.path) request.path) + queueInteraction (fun () -> runInteraction $"#load @\"%s{request.path}\"" request.path) /// Apply the host's notion of where to look for sources and references, expressed as the /// directives a script would use. @@ -324,24 +349,25 @@ type FsiRpcTarget with _ -> () - directives.Add(sprintf "#silentCd @\"%s\"" request.workingDirectory) + directives.Add $"#silentCd @\"%s{request.workingDirectory}\"" match request.includePaths with | null -> () | paths -> for path in paths do if not (String.IsNullOrWhiteSpace path) then - directives.Add(sprintf "#I @\"%s\"" path) + directives.Add $"#I @\"%s{path}\"" if directives.Count = 0 then toExecutionResult (Choice1Of2 None) [||] false else runInteraction (String.Join("\n", directives)) DefaultInteractionName) - /// Interrupt the interaction in flight. - /// + /// Interrupt the interaction in flight. + /// /// Served straight away rather than queued, which is the point: an interrupt that waited its /// turn behind the interaction it is meant to stop would never arrive. + /// [] member _.Interrupt() : InterruptResult = requireInitialized () @@ -405,9 +431,8 @@ let private runServer // Diagnostic breadcrumb: a host that gets "method not found" against a target that plainly // declares the method has almost certainly loaded a second, different copy of this library, so // its identity here is worth more than the rest of the trace. - errorWriter.WriteLine( - sprintf "FSI-SERVER: StreamJsonRpc %O from %s" (typeof.Assembly.GetName().Version) typeof.Assembly.Location - ) + let streamJsonRpc = typeof.Assembly + errorWriter.WriteLine $"FSI-SERVER: StreamJsonRpc %O{streamJsonRpc.GetName().Version} from %s{streamJsonRpc.Location}" errorWriter.Flush() rpc.StartListening() @@ -437,7 +462,7 @@ let internal startOnBackgroundThread try runServer fsiSession fsiConfig pipeName outWriter errorWriter with e -> - errorWriter.WriteLine(sprintf "F# Interactive server terminated: %s" (e.ToString())) + errorWriter.WriteLine $"F# Interactive server terminated: %O{e}" errorWriter.Flush() // The session exists only to serve this host. Once the connection is gone there is @@ -449,7 +474,9 @@ let internal startOnBackgroundThread thread.Start() -/// Recognise `--fsi-server-jsonrpc:` in a command line, returning the pipe name. +/// +/// Recognise --fsi-server-jsonrpc:<pipe name> in a command line, returning the pipe name. +/// let internal tryGetPipeName (argv: string[]) = argv |> Array.tryPick (fun arg -> diff --git a/src/fsi/interactiveProtocol.fs b/src/fsi/interactiveProtocol.fs index 800fd1bd7c3..bfb29b85767 100644 --- a/src/fsi/interactiveProtocol.fs +++ b/src/fsi/interactiveProtocol.fs @@ -1,17 +1,22 @@ // Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. -/// The types exchanged between F# Interactive and the editor hosting it. -/// +/// The types exchanged between F# Interactive and the editor hosting it. +/// +/// /// This file is compiled into fsi and linked into the host, so that the two ends of the protocol /// cannot drift apart. Roslyn achieves the same by having both sides reference one assembly; fsi /// exposes no public surface to reference, so the source is shared instead. -/// -/// The members are named as they appear on the wire, and the records are `CLIMutable` so that the -/// JSON-RPC formatter can construct them. -/// +/// +/// +/// The members are named as they appear on the wire, and the records are CLIMutable so that +/// the JSON-RPC formatter can construct them. +/// +/// /// Public rather than internal, even in fsi's own copy: the handlers that carry them have to be /// public for StreamJsonRpc to find them by reflection, and a public member cannot expose a type /// less accessible than itself. +/// +/// namespace FSharp.Compiler.Interactive.Protocol /// Method names. Both ends use these rather than repeating string literals. @@ -45,10 +50,13 @@ type InitializeRequest = [] type InitializeResult = { + /// /// The process actually evaluating code, which is what a debugger attaches to. - /// - /// On .NET this is not the process the host launched: `dotnet fsi` starts a second process, - /// and it is the inner one that matters. + /// + /// + /// On .NET this is not the process the host launched: dotnet fsi starts a second + /// process, and it is the inner one that matters. + /// processId: int frameworkDescription: string @@ -63,9 +71,11 @@ type ExecuteRequest = { code: string + /// /// Where the text came from, when the host is executing a selection from a file. Together - /// with `startLine` this makes diagnostics point at the user's own source rather than at a - /// position within the submission. + /// with startLine this makes diagnostics point at the user's own source rather than + /// at a position within the submission. + /// sourcePath: string startLine: System.Nullable diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs index dc3c24b59ce..c9790e30957 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs @@ -24,11 +24,11 @@ let private withInitializedSession (test: FsiServerHarness -> unit) = /// Include the result and the session's output in a failure, since a protocol result alone rarely /// explains what the session actually did. let private describe (session: FsiServerHarness) (result: ExecutionResult) = - sprintf - "result: %s\nstandard output:\n%s\nstandard error:\n%s" - (describeResult result) - session.StandardOutput - session.StandardError + $"""result: {describeResult result} +standard output: +{session.StandardOutput} +standard error: +{session.StandardError}""" //------------------------------------------------------------------------- // Handshake @@ -48,7 +48,7 @@ let ``initialize reports the session process`` () = Assert.True( Directory.Exists result.workingDirectory, - sprintf "'%s' is not a directory" result.workingDirectory + $"'%s{result.workingDirectory}' is not a directory" )) [] @@ -146,7 +146,7 @@ let ``reports type errors as structured diagnostics`` () = // FS0001 is the type mismatch error, and it must carry a usable position. let error = reported[0] Assert.Equal(1, error.errorNumber) - Assert.True(error.startLine >= 1, sprintf "unexpected start line %d" error.startLine) + Assert.True(error.startLine >= 1, $"unexpected start line %d{error.startLine}") Assert.False(String.IsNullOrWhiteSpace error.message)) [] @@ -222,7 +222,7 @@ let ``keeps serving after a failed interaction`` () = let ``loads a script file`` () = withInitializedSession (fun session -> let script = - Path.Combine(Path.GetTempPath(), sprintf "fsiServerTest_%s.fsx" (Guid.NewGuid().ToString "N")) + Path.Combine(Path.GetTempPath(), $"""fsiServerTest_%s{Guid.NewGuid().ToString "N"}.fsx""") File.WriteAllText(script, "printfn \"the script ran\"\n") @@ -246,7 +246,7 @@ let ``loads a script file`` () = let ``setPaths changes the working directory`` () = withInitializedSession (fun session -> let directory = - Path.Combine(Path.GetTempPath(), sprintf "fsiServerTest_%s" (Guid.NewGuid().ToString "N")) + Path.Combine(Path.GetTempPath(), $"""fsiServerTest_%s{Guid.NewGuid().ToString "N"}""") Directory.CreateDirectory directory |> ignore @@ -276,7 +276,7 @@ let ``setPaths changes the working directory`` () = let ``setPaths waits its turn behind a running interaction`` () = withInitializedSession (fun session -> let directory = - Path.Combine(Path.GetTempPath(), sprintf "fsiServerTest_%s" (Guid.NewGuid().ToString "N")) + Path.Combine(Path.GetTempPath(), $"""fsiServerTest_%s{Guid.NewGuid().ToString "N"}""") Directory.CreateDirectory directory |> ignore @@ -315,7 +315,7 @@ printfn "interaction saw [%s]" (System.IO.Directory.GetCurrentDirectory()) // The process directory moves on the queue like everything else, so an interaction that // was already running keeps the directory it started in. Assert.True( - session.WaitForOutput(sprintf "interaction saw [%s]" warmUp.workingDirectory), + session.WaitForOutput $"interaction saw [%s{warmUp.workingDirectory}]", describe session result ) finally diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs index 1c6a9e1f78a..75d2fe089b6 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs @@ -67,14 +67,14 @@ let private locateFsi () = let fsi = Path.Combine(fsiDirectory, "fsi.exe") if not (File.Exists fsi) then - failwithf "Could not find the fsi under test at '%s'. Build src/fsi first." fsi + failwith $"Could not find the fsi under test at '%s{fsi}'. Build src/fsi first." fsi, [] else let fsi = Path.Combine(fsiDirectory, "fsi.dll") if not (File.Exists fsi) then - failwithf "Could not find the fsi under test at '%s'. Build src/fsi first." fsi + failwith $"Could not find the fsi under test at '%s{fsi}'. Build src/fsi first." locateDotnetHost (), [ fsi ] @@ -147,11 +147,11 @@ type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = with e -> let detail = if session.HasExited then - sprintf "The session exited with code %d." session.ExitCode + $"The session exited with code %d{session.ExitCode}." else "The session is still running." - failwithf "Could not connect to the session on pipe '%s'. %s\n%s" pipeName detail e.Message + failwith $"Could not connect to the session on pipe '%s{pipeName}'. %s{detail}\n%s{e.Message}" pipe @@ -310,15 +310,16 @@ let exceptionMessage (result: ExecutionResult) = let describeResult (result: ExecutionResult) = let diagnosticText = diagnostics result - |> Array.map (fun d -> sprintf "%s(%d,%d): %s FS%04d: %s" d.fileName d.startLine d.startColumn d.severity d.errorNumber d.message) + |> Array.map (fun d -> + $"%s{d.fileName}(%d{d.startLine},%d{d.startColumn}): %s{d.severity} FS%04d{d.errorNumber}: %s{d.message}") |> String.concat "\n " - sprintf - "success=%b cancelled=%b workingDirectory=%s exception=%s\n %s" - result.success - result.cancelled - result.workingDirectory - (match exceptionMessage result with - | Some m -> m - | None -> "") - diagnosticText + let exceptionText = + match exceptionMessage result with + | Some message -> message + | None -> "" + + let outcome = + $"success=%b{result.success} cancelled=%b{result.cancelled} workingDirectory=%s{result.workingDirectory}" + + $"%s{outcome} exception=%s{exceptionText}\n %s{diagnosticText}" From 1532d0eab490241ec9528d42449dab889df83842 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 10 Sep 2026 12:06:30 +0200 Subject: [PATCH 14/30] Use .NET format specifiers for the generated names in the tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `{Guid.NewGuid():N}` reads better than `%s{Guid.NewGuid().ToString "N"}` and drops the triple quotes the nested literal forced. The two remaining concatenations in the harness — the pipe name and the server switch — go the same way. Co-Authored-By: Claude Opus 5 (1M context) --- .../FsiJsonRpcServerTests.fs | 6 +++--- .../FsiServerHarness.fs | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs index c9790e30957..ee5fa846e22 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs @@ -222,7 +222,7 @@ let ``keeps serving after a failed interaction`` () = let ``loads a script file`` () = withInitializedSession (fun session -> let script = - Path.Combine(Path.GetTempPath(), $"""fsiServerTest_%s{Guid.NewGuid().ToString "N"}.fsx""") + Path.Combine(Path.GetTempPath(), $"fsiServerTest_{Guid.NewGuid():N}.fsx") File.WriteAllText(script, "printfn \"the script ran\"\n") @@ -246,7 +246,7 @@ let ``loads a script file`` () = let ``setPaths changes the working directory`` () = withInitializedSession (fun session -> let directory = - Path.Combine(Path.GetTempPath(), $"""fsiServerTest_%s{Guid.NewGuid().ToString "N"}""") + Path.Combine(Path.GetTempPath(), $"fsiServerTest_{Guid.NewGuid():N}") Directory.CreateDirectory directory |> ignore @@ -276,7 +276,7 @@ let ``setPaths changes the working directory`` () = let ``setPaths waits its turn behind a running interaction`` () = withInitializedSession (fun session -> let directory = - Path.Combine(Path.GetTempPath(), $"""fsiServerTest_%s{Guid.NewGuid().ToString "N"}""") + Path.Combine(Path.GetTempPath(), $"fsiServerTest_{Guid.NewGuid():N}") Directory.CreateDirectory directory |> ignore diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs index 75d2fe089b6..870e0349e2a 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs @@ -81,7 +81,7 @@ let private locateFsi () = /// A running session, plus everything needed to talk to it and to explain a failure. [] type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = - let pipeName = "FsiServerTests_" + Guid.NewGuid().ToString("N") + let pipeName = $"FsiServerTests_{Guid.NewGuid():N}" let standardOutput = StringBuilder() let standardError = StringBuilder() let outputLock = obj () @@ -93,7 +93,7 @@ type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = argument.IndexOf(" ", StringComparison.Ordinal) >= 0 && not (argument.StartsWith("\"", StringComparison.Ordinal)) then - "\"" + argument + "\"" + $"\"{argument}\"" else argument @@ -104,7 +104,7 @@ type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = [ yield! leadingArguments "--nologo" - "--fsi-server-jsonrpc:" + pipeName + $"--fsi-server-jsonrpc:{pipeName}" yield! defaultArg extraArguments [] ] From 11e9159dec15b9162f3b93a5d6211acac3e69a89 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 10 Sep 2026 12:13:55 +0200 Subject: [PATCH 15/30] Drop the % specifiers from the interpolated strings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plain `{expr}` throughout, with a .NET format string where the text needs one — `FS{d.errorNumber:D4}` keeps the four-digit error number. Co-Authored-By: Claude Opus 5 (1M context) --- src/fsi/fsiserver.fs | 12 ++++++------ .../FsiJsonRpcServerTests.fs | 6 +++--- .../FsiServerHarness.fs | 14 +++++++------- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/fsi/fsiserver.fs b/src/fsi/fsiserver.fs index 93d38ab1913..b8d19c10517 100644 --- a/src/fsi/fsiserver.fs +++ b/src/fsi/fsiserver.fs @@ -261,7 +261,7 @@ type FsiRpcTarget if String.IsNullOrEmpty sourcePath || not startLine.HasValue then code else - $"# %d{startLine.Value} @\"%s{sourcePath}\"\n%s{code}" + $"# {startLine.Value} @\"{sourcePath}\"\n{code}" /// Refuse anything that arrives before the handshake, so that a mis-sequenced host gets a clear /// answer rather than an obscure failure later on. @@ -323,7 +323,7 @@ type FsiRpcTarget // Routed through #load so that the file joins the session the same way it would from a // script, rather than being replayed as anonymous text. - queueInteraction (fun () -> runInteraction $"#load @\"%s{request.path}\"" request.path) + queueInteraction (fun () -> runInteraction $"#load @\"{request.path}\"" request.path) /// Apply the host's notion of where to look for sources and references, expressed as the /// directives a script would use. @@ -349,14 +349,14 @@ type FsiRpcTarget with _ -> () - directives.Add $"#silentCd @\"%s{request.workingDirectory}\"" + directives.Add $"#silentCd @\"{request.workingDirectory}\"" match request.includePaths with | null -> () | paths -> for path in paths do if not (String.IsNullOrWhiteSpace path) then - directives.Add $"#I @\"%s{path}\"" + directives.Add $"#I @\"{path}\"" if directives.Count = 0 then toExecutionResult (Choice1Of2 None) [||] false @@ -432,7 +432,7 @@ let private runServer // declares the method has almost certainly loaded a second, different copy of this library, so // its identity here is worth more than the rest of the trace. let streamJsonRpc = typeof.Assembly - errorWriter.WriteLine $"FSI-SERVER: StreamJsonRpc %O{streamJsonRpc.GetName().Version} from %s{streamJsonRpc.Location}" + errorWriter.WriteLine $"FSI-SERVER: StreamJsonRpc {streamJsonRpc.GetName().Version} from {streamJsonRpc.Location}" errorWriter.Flush() rpc.StartListening() @@ -462,7 +462,7 @@ let internal startOnBackgroundThread try runServer fsiSession fsiConfig pipeName outWriter errorWriter with e -> - errorWriter.WriteLine $"F# Interactive server terminated: %O{e}" + errorWriter.WriteLine $"F# Interactive server terminated: {e}" errorWriter.Flush() // The session exists only to serve this host. Once the connection is gone there is diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs index ee5fa846e22..b14c5560d57 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs @@ -48,7 +48,7 @@ let ``initialize reports the session process`` () = Assert.True( Directory.Exists result.workingDirectory, - $"'%s{result.workingDirectory}' is not a directory" + $"'{result.workingDirectory}' is not a directory" )) [] @@ -146,7 +146,7 @@ let ``reports type errors as structured diagnostics`` () = // FS0001 is the type mismatch error, and it must carry a usable position. let error = reported[0] Assert.Equal(1, error.errorNumber) - Assert.True(error.startLine >= 1, $"unexpected start line %d{error.startLine}") + Assert.True(error.startLine >= 1, $"unexpected start line {error.startLine}") Assert.False(String.IsNullOrWhiteSpace error.message)) [] @@ -315,7 +315,7 @@ printfn "interaction saw [%s]" (System.IO.Directory.GetCurrentDirectory()) // The process directory moves on the queue like everything else, so an interaction that // was already running keeps the directory it started in. Assert.True( - session.WaitForOutput $"interaction saw [%s{warmUp.workingDirectory}]", + session.WaitForOutput $"interaction saw [{warmUp.workingDirectory}]", describe session result ) finally diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs index 870e0349e2a..1055a3734fb 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs @@ -67,14 +67,14 @@ let private locateFsi () = let fsi = Path.Combine(fsiDirectory, "fsi.exe") if not (File.Exists fsi) then - failwith $"Could not find the fsi under test at '%s{fsi}'. Build src/fsi first." + failwith $"Could not find the fsi under test at '{fsi}'. Build src/fsi first." fsi, [] else let fsi = Path.Combine(fsiDirectory, "fsi.dll") if not (File.Exists fsi) then - failwith $"Could not find the fsi under test at '%s{fsi}'. Build src/fsi first." + failwith $"Could not find the fsi under test at '{fsi}'. Build src/fsi first." locateDotnetHost (), [ fsi ] @@ -147,11 +147,11 @@ type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = with e -> let detail = if session.HasExited then - $"The session exited with code %d{session.ExitCode}." + $"The session exited with code {session.ExitCode}." else "The session is still running." - failwith $"Could not connect to the session on pipe '%s{pipeName}'. %s{detail}\n%s{e.Message}" + failwith $"Could not connect to the session on pipe '{pipeName}'. {detail}\n{e.Message}" pipe @@ -311,7 +311,7 @@ let describeResult (result: ExecutionResult) = let diagnosticText = diagnostics result |> Array.map (fun d -> - $"%s{d.fileName}(%d{d.startLine},%d{d.startColumn}): %s{d.severity} FS%04d{d.errorNumber}: %s{d.message}") + $"{d.fileName}({d.startLine},{d.startColumn}): {d.severity} FS{d.errorNumber:D4}: {d.message}") |> String.concat "\n " let exceptionText = @@ -320,6 +320,6 @@ let describeResult (result: ExecutionResult) = | None -> "" let outcome = - $"success=%b{result.success} cancelled=%b{result.cancelled} workingDirectory=%s{result.workingDirectory}" + $"success={result.success} cancelled={result.cancelled} workingDirectory={result.workingDirectory}" - $"%s{outcome} exception=%s{exceptionText}\n %s{diagnosticText}" + $"{outcome} exception={exceptionText}\n {diagnosticText}" From 27d7ce322b1dee0a7d0eed880ac45faec38cc0b7 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Tue, 15 Sep 2026 12:11:59 +0200 Subject: [PATCH 16/30] Shorten the test pipe name to fit the macOS socket path limit On Unix a named pipe is a socket under $TMPDIR, and macOS caps socket paths at 104 characters; with its long $TMPDIR every session failed to start. A failed connection now also reports the session's stderr. Co-Authored-By: Claude Opus 5 (1M context) --- .../FsiServerHarness.fs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs index 1055a3734fb..310448db45c 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs @@ -81,7 +81,8 @@ let private locateFsi () = /// A running session, plus everything needed to talk to it and to explain a failure. [] type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = - let pipeName = $"FsiServerTests_{Guid.NewGuid():N}" + // On Unix the pipe is a socket under $TMPDIR, and macOS caps socket paths at 104 characters. + let pipeName = $"fsi{Guid.NewGuid():N}".Substring(0, 15) let standardOutput = StringBuilder() let standardError = StringBuilder() let outputLock = obj () @@ -151,7 +152,8 @@ type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = else "The session is still running." - failwith $"Could not connect to the session on pipe '{pipeName}'. {detail}\n{e.Message}" + let error = lock outputLock (fun () -> standardError.ToString()) + failwith $"Could not connect to the session on pipe '{pipeName}'. {detail}\n{e.Message}\n-- stderr --\n{error}" pipe From 8d266192c329f411d204e1be476456968153dc2c Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Wed, 16 Sep 2026 13:36:26 +0200 Subject: [PATCH 17/30] Strip the macOS /private prefix before comparing working directories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Path.GetTempPath() returns /var/folders/..., but the session reports /private/var/folders/... after chdir — the kernel resolves /var through /private when it canonicalises a path, and Path.GetFullPath does not. setPaths changes the working directory failed on every macOS CI run. Co-Authored-By: Claude Sonnet 5 --- .../FsiJsonRpcServerTests.fs | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs index b14c5560d57..62a6a7ce0ae 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs @@ -4,6 +4,7 @@ module FSharp.Compiler.Interactive.Server.Tests.FsiJsonRpcServerTests open System open System.IO +open System.Runtime.InteropServices open System.Threading open Xunit @@ -15,6 +16,16 @@ let private withSession (test: FsiServerHarness -> unit) = use session = new FsiServerHarness() test session +/// macOS resolves `/tmp`, `/var` and `/etc` through their `/private` targets when a path is +/// canonicalised by the kernel — which is what happens to the session's own working directory +/// after it changes there — but not when `Path.GetTempPath()`/`GetFullPath` builds one in this +/// process. The two would otherwise disagree on the very directory both sides just agreed on. +let private stripMacPrivatePrefix (path: string) = + if RuntimeInformation.IsOSPlatform OSPlatform.OSX && path.StartsWith("/private/", StringComparison.Ordinal) then + path.Substring "/private".Length + else + path + /// Start a session that has already completed the handshake. let private withInitializedSession (test: FsiServerHarness -> unit) = withSession (fun session -> @@ -263,8 +274,13 @@ let ``setPaths changes the working directory`` () = Assert.True(succeeded result, describe session result) // The host mirrors this value so that its own reference resolution matches the session. - let expected = Path.GetFullPath(directory).TrimEnd(Path.DirectorySeparatorChar) - let actual = Path.GetFullPath(result.workingDirectory).TrimEnd(Path.DirectorySeparatorChar) + let expected = + Path.GetFullPath(directory).TrimEnd(Path.DirectorySeparatorChar) |> stripMacPrivatePrefix + + let actual = + Path.GetFullPath(result.workingDirectory).TrimEnd(Path.DirectorySeparatorChar) + |> stripMacPrivatePrefix + Assert.Equal(expected, actual) finally try From ddea77a033a3829fcb4e81ea44e92a43835e65c9 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Fri, 18 Sep 2026 02:43:50 +0200 Subject: [PATCH 18/30] Address FSI JSON-RPC review findings Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../Microsoft.FSharp.Compiler.MSBuild.csproj | 7 ++ src/fsi/fsiserver.fs | 114 +++++++++++++++++- .../FsiJsonRpcServerTests.fs | 33 +++++ 3 files changed, 150 insertions(+), 4 deletions(-) diff --git a/setup/Swix/Microsoft.FSharp.Compiler.MSBuild/Microsoft.FSharp.Compiler.MSBuild.csproj b/setup/Swix/Microsoft.FSharp.Compiler.MSBuild/Microsoft.FSharp.Compiler.MSBuild.csproj index a6cf0324ca9..ea70d1b9a1f 100644 --- a/setup/Swix/Microsoft.FSharp.Compiler.MSBuild/Microsoft.FSharp.Compiler.MSBuild.csproj +++ b/setup/Swix/Microsoft.FSharp.Compiler.MSBuild/Microsoft.FSharp.Compiler.MSBuild.csproj @@ -88,6 +88,13 @@ folder "InstallDir:Common7\IDE\CommonExtensions\Microsoft\FSharp\Tools" file source="$(BinariesFolder)fscArm64\$(Configuration)\$(TargetFramework)\fscArm64.exe.config" file source="$(BinariesFolder)fsi\$(Configuration)\$(TargetFramework)\fsi.exe" vs.file.ngen=yes vs.file.ngenArchitecture=X86 vs.file.ngenPriority=2 vs.file.ngenApplication="[installDir]\Common7\IDE\CommonExtensions\Microsoft\FSharp\Tools\fsi.exe" file source="$(BinariesFolder)fsi\$(Configuration)\$(TargetFramework)\fsi.exe.config" + file source="$(BinariesFolder)fsi\$(Configuration)\$(TargetFramework)\MessagePack.dll" + file source="$(BinariesFolder)fsi\$(Configuration)\$(TargetFramework)\MessagePack.Annotations.dll" + file source="$(BinariesFolder)fsi\$(Configuration)\$(TargetFramework)\Nerdbank.MessagePack.dll" + file source="$(BinariesFolder)fsi\$(Configuration)\$(TargetFramework)\Nerdbank.Streams.dll" + file source="$(BinariesFolder)fsi\$(Configuration)\$(TargetFramework)\Newtonsoft.Json.dll" + file source="$(BinariesFolder)fsi\$(Configuration)\$(TargetFramework)\PolyType.dll" + file source="$(BinariesFolder)fsi\$(Configuration)\$(TargetFramework)\StreamJsonRpc.dll" file source="$(BinariesFolder)fsiAnyCpu\$(Configuration)\$(TargetFramework)\fsiAnyCpu.exe" vs.file.ngen=yes vs.file.ngenArchitecture=X64 vs.file.ngenPriority=2 vs.file.ngenApplication="[installDir]\Common7\IDE\CommonExtensions\Microsoft\FSharp\Tools\fsiAnyCpu.exe" file source="$(BinariesFolder)fsiAnyCpu\$(Configuration)\$(TargetFramework)\fsiAnyCpu.exe.config" file source="$(BinariesFolder)fsiArm64\$(Configuration)\$(TargetFramework)\fsiArm64.exe" vs.file.ngen=yes vs.file.ngenArchitecture=arm64 vs.file.ngenPriority=2 vs.file.ngenApplication="[installDir]\Common7\IDE\CommonExtensions\Microsoft\FSharp\Tools\fsiAnyCpu.exe" diff --git a/src/fsi/fsiserver.fs b/src/fsi/fsiserver.fs index b8d19c10517..fb32496dd4d 100644 --- a/src/fsi/fsiserver.fs +++ b/src/fsi/fsiserver.fs @@ -40,6 +40,7 @@ open System.Diagnostics open System.IO open System.IO.Pipes open System.Runtime.InteropServices +open System.Text open System.Threading open System.Threading.Tasks @@ -114,6 +115,84 @@ let private toExecutionResult (outcome: Choice) (diagnosti workingDirectory = Directory.GetCurrentDirectory() } +let private splitInteractions (code: string) = + let interactions = ResizeArray() + let current = StringBuilder() + let mutable index = 0 + let mutable inString = false + let mutable inChar = false + let mutable inLineComment = false + let mutable blockCommentDepth = 0 + + let addInteraction () = + let text = current.ToString().Trim() + + if text.Length > 0 then + interactions.Add text + + current.Clear() |> ignore + + while index < code.Length do + let character = code[index] + let nextCharacter = if index + 1 < code.Length then code[index + 1] else '\000' + + if inLineComment then + current.Append character |> ignore + inLineComment <- character <> '\n' && character <> '\r' + elif blockCommentDepth > 0 then + current.Append character |> ignore + + if character = '(' && nextCharacter = '*' then + current.Append nextCharacter |> ignore + blockCommentDepth <- blockCommentDepth + 1 + index <- index + 1 + elif character = '*' && nextCharacter = ')' then + current.Append nextCharacter |> ignore + blockCommentDepth <- blockCommentDepth - 1 + index <- index + 1 + elif inString then + current.Append character |> ignore + + if character = '\\' && index + 1 < code.Length then + current.Append code[index + 1] |> ignore + index <- index + 1 + elif character = '"' then + inString <- false + elif inChar then + current.Append character |> ignore + + if character = '\\' && index + 1 < code.Length then + current.Append code[index + 1] |> ignore + index <- index + 1 + elif character = '\'' then + inChar <- false + elif character = '/' && nextCharacter = '/' then + current.Append character |> ignore + current.Append nextCharacter |> ignore + inLineComment <- true + index <- index + 1 + elif character = '(' && nextCharacter = '*' then + current.Append character |> ignore + current.Append nextCharacter |> ignore + blockCommentDepth <- 1 + index <- index + 1 + elif character = '"' then + current.Append character |> ignore + inString <- true + elif character = '\'' then + current.Append character |> ignore + inChar <- true + elif character = ';' && nextCharacter = ';' then + addInteraction () + index <- index + 1 + else + current.Append character |> ignore + + index <- index + 1 + + addInteraction () + interactions.ToArray() + //------------------------------------------------------------------------- // The server //------------------------------------------------------------------------- @@ -224,11 +303,37 @@ type FsiRpcTarget lock interruptLock (fun () -> currentCancellation <- cancellation) try - let outcome, diagnostics = - evaluateOnEventLoop (fun () -> fsiSession.EvalInteractionNonThrowing(code, scriptPath, cancellation.Token)) + let outcomes = ResizeArray>() + let diagnostics = ResizeArray() + let mutable stop = false + + for interaction in splitInteractions code do + if not stop then + let outcome, interactionDiagnostics = + evaluateOnEventLoop (fun () -> fsiSession.EvalInteractionNonThrowing(interaction, scriptPath, cancellation.Token)) + + outcomes.Add outcome + diagnostics.AddRange interactionDiagnostics + + stop <- + match outcome with + | Choice2Of2 _ -> true + | Choice1Of2 _ -> + interactionDiagnostics + |> Array.exists (fun diagnostic -> diagnostic.Severity = FSharpDiagnosticSeverity.Error) + + let outcome = + match + outcomes + |> Seq.tryFindBack (function + | Choice2Of2 _ -> true + | Choice1Of2 _ -> false) + with + | Some outcome -> outcome + | None -> Choice1Of2 None flushConsole () - toExecutionResult outcome diagnostics cancellation.IsCancellationRequested + toExecutionResult outcome (diagnostics.ToArray()) cancellation.IsCancellationRequested finally lock interruptLock (fun () -> currentCancellation <- null) cancellation.Dispose() @@ -323,7 +428,8 @@ type FsiRpcTarget // Routed through #load so that the file joins the session the same way it would from a // script, rather than being replayed as anonymous text. - queueInteraction (fun () -> runInteraction $"#load @\"{request.path}\"" request.path) + let path = request.path.Replace("\"", "\"\"") + queueInteraction (fun () -> runInteraction $"#load @\"{path}\"" request.path) /// Apply the host's notion of where to look for sources and references, expressed as the /// directives a script would use. diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs index 62a6a7ce0ae..555f24063d7 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs @@ -114,6 +114,16 @@ let ``keeps bindings across interactions`` () = Assert.True(succeeded result, describe session result) Assert.True(session.WaitForOutput "val it: int = 42", describe session result)) +[] +let ``evaluates every interaction in a request`` () = + withInitializedSession (fun session -> + let result = session.Execute "let first = 11;; let second = 22;;" + Assert.True(succeeded result, describe session result) + + let next = session.Execute "second" + Assert.True(succeeded next, describe session next) + Assert.True(session.WaitForOutput "val it: int = 22", describe session next)) + [] let ``reports what the interaction printed`` () = withInitializedSession (fun session -> @@ -253,6 +263,29 @@ let ``loads a script file`` () = with _ -> ()) +[] +let ``loads a script file whose path contains quotes`` () = + if RuntimeInformation.IsOSPlatform OSPlatform.Windows then + () + else + withInitializedSession (fun session -> + let directory = + Path.Combine(Path.GetTempPath(), $"fsiServerTest_{Guid.NewGuid():N}\"quoted") + + Directory.CreateDirectory directory |> ignore + let script = Path.Combine(directory, "script.fsx") + File.WriteAllText(script, "printfn \"quoted path loaded\"\n") + + try + let loaded = session.Request(Methods.ExecuteFile, { path = script }) + Assert.True(succeeded loaded, describe session loaded) + Assert.True(session.WaitForOutput "quoted path loaded", describe session loaded) + finally + try + Directory.Delete(directory, true) + with _ -> + ()) + [] let ``setPaths changes the working directory`` () = withInitializedSession (fun session -> From c22be51eb34230e25ce3f185f89d7e0da079be07 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Fri, 18 Sep 2026 02:48:59 +0200 Subject: [PATCH 19/30] Return evaluated FSI values over JSON-RPC Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fsi/fsiserver.fs | 30 +++++++++++++++++-- src/fsi/interactiveProtocol.fs | 9 ++++++ .../FsiJsonRpcServerTests.fs | 7 +++++ 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/fsi/fsiserver.fs b/src/fsi/fsiserver.fs index fb32496dd4d..463928be28a 100644 --- a/src/fsi/fsiserver.fs +++ b/src/fsi/fsiserver.fs @@ -82,7 +82,22 @@ let private toDiagnosticInfo (diagnostic: FSharpDiagnostic) = endColumn = diagnostic.EndColumn } -let private toExecutionResult (outcome: Choice) (diagnostics: FSharpDiagnostic[]) (cancelled: bool) = +let private toValueInfo (name: string) (value: FsiValue) = + { + name = name + typeName = + match value.ReflectionType with + | null -> "" + | typeInfo -> typeInfo.FullName + value = sprintf "%A" value.ReflectionValue + } + +let private toExecutionResult + (outcome: Choice) + (diagnostics: FSharpDiagnostic[]) + (values: ValueInfo[]) + (cancelled: bool) + = let hasErrors = diagnostics |> Array.exists (fun d -> d.Severity = FSharpDiagnosticSeverity.Error) @@ -112,6 +127,7 @@ let private toExecutionResult (outcome: Choice) (diagnosti | trace -> trace } | None -> Unchecked.defaultof + values = values workingDirectory = Directory.GetCurrentDirectory() } @@ -273,6 +289,13 @@ type FsiRpcTarget let interruptLock = obj () let mutable currentCancellation: CancellationTokenSource = null let mutable initialized = false + let values = ResizeArray() + + do + fsiConfig.OnEvaluation.Add(fun evaluation -> + match evaluation.FsiValue with + | Some value -> values.Add(toValueInfo evaluation.Name value) + | None -> ()) /// /// Evaluate on the event loop thread, the same thread a console session evaluates on. @@ -303,6 +326,7 @@ type FsiRpcTarget lock interruptLock (fun () -> currentCancellation <- cancellation) try + values.Clear() let outcomes = ResizeArray>() let diagnostics = ResizeArray() let mutable stop = false @@ -333,7 +357,7 @@ type FsiRpcTarget | None -> Choice1Of2 None flushConsole () - toExecutionResult outcome (diagnostics.ToArray()) cancellation.IsCancellationRequested + toExecutionResult outcome (diagnostics.ToArray()) (values.ToArray()) cancellation.IsCancellationRequested finally lock interruptLock (fun () -> currentCancellation <- null) cancellation.Dispose() @@ -465,7 +489,7 @@ type FsiRpcTarget directives.Add $"#I @\"{path}\"" if directives.Count = 0 then - toExecutionResult (Choice1Of2 None) [||] false + toExecutionResult (Choice1Of2 None) [||] [||] false else runInteraction (String.Join("\n", directives)) DefaultInteractionName) diff --git a/src/fsi/interactiveProtocol.fs b/src/fsi/interactiveProtocol.fs index bfb29b85767..1aa352f7ff6 100644 --- a/src/fsi/interactiveProtocol.fs +++ b/src/fsi/interactiveProtocol.fs @@ -116,6 +116,14 @@ type ExceptionInfo = stackTrace: string } +[] +type ValueInfo = + { + name: string + typeName: string + value: string + } + [] type ExecutionResult = { @@ -126,6 +134,7 @@ type ExecutionResult = cancelled: bool diagnostics: DiagnosticInfo[] ``exception``: ExceptionInfo + values: ValueInfo[] /// Reported after every interaction so that the host can keep its own view of the session /// in step with one that changed directory. diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs index 555f24063d7..c03b2f6ae1d 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs @@ -104,6 +104,13 @@ let ``evaluates an interaction and prints its result`` () = // The value is reported the way a console session reports it: printed to standard output. Assert.True(session.WaitForOutput "val it: int = 2", describe session result)) +[] +let ``returns evaluated values`` () = + withInitializedSession (fun session -> + let result = session.Execute "let answer = 42" + Assert.True(succeeded result, describe session result) + Assert.Contains(result.values, fun value -> value.name = "answer" && value.value = "42")) + [] let ``keeps bindings across interactions`` () = withInitializedSession (fun session -> From 57625b8366b2f10a357646371c225056e05527aa Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Fri, 18 Sep 2026 17:38:32 +0200 Subject: [PATCH 20/30] Fix remaining FSI JSON-RPC review issues Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fsi/fsiserver.fs | 61 +++++++++++++++---- .../FsiJsonRpcServerTests.fs | 23 +++++++ 2 files changed, 71 insertions(+), 13 deletions(-) diff --git a/src/fsi/fsiserver.fs b/src/fsi/fsiserver.fs index 463928be28a..483a3f3f640 100644 --- a/src/fsi/fsiserver.fs +++ b/src/fsi/fsiserver.fs @@ -140,6 +140,9 @@ let private splitInteractions (code: string) = let mutable inLineComment = false let mutable blockCommentDepth = 0 + let isIdentifierPart (character: char) = + Char.IsLetterOrDigit character || character = '_' || character = '\'' + let addInteraction () = let text = current.ToString().Trim() @@ -195,7 +198,7 @@ let private splitInteractions (code: string) = elif character = '"' then current.Append character |> ignore inString <- true - elif character = '\'' then + elif character = '\'' && (index = 0 || not (isIdentifierPart code[index - 1])) then current.Append character |> ignore inChar <- true elif character = ';' && nextCharacter = ';' then @@ -461,16 +464,19 @@ type FsiRpcTarget member _.SetPaths(request: SetPathsRequest) : Task = requireInitialized () + if + not (String.IsNullOrWhiteSpace request.workingDirectory) + && not (Directory.Exists request.workingDirectory) + then + raise (LocalRpcException($"The working directory '{request.workingDirectory}' does not exist.", ErrorCode = -32002)) + // The process directory moves on the queue, alongside the directive that moves the // compiler's: doing it as the request arrives would move it under an earlier interaction // that is still running. queueInteraction (fun () -> let directives = ResizeArray() - if - not (String.IsNullOrWhiteSpace request.workingDirectory) - && Directory.Exists request.workingDirectory - then + if not (String.IsNullOrWhiteSpace request.workingDirectory) then // Two different notions of "current directory" have to agree here. The directive // moves the compiler's, which is what relative #load and #r resolve against; the // process one is what the running script sees when it opens a file by relative path. @@ -608,11 +614,40 @@ let internal startOnBackgroundThread /// Recognise --fsi-server-jsonrpc:<pipe name> in a command line, returning the pipe name. /// let internal tryGetPipeName (argv: string[]) = - argv - |> Array.tryPick (fun arg -> - if arg.StartsWith(JsonRpcServerOption, StringComparison.Ordinal) then - let name = arg.Substring(JsonRpcServerOption.Length).Trim('"') - - if String.IsNullOrWhiteSpace name then None else Some name - else - None) + let optionName = JsonRpcServerOption.TrimStart('-') + let optionPrefixes = [| JsonRpcServerOption; "-" + optionName; "/" + optionName |] + + let rec scan (args: string list) = + match args with + | [] -> None + | arg :: rest -> + let prefix = + optionPrefixes + |> Array.tryFind (fun prefix -> arg.StartsWith(prefix, StringComparison.Ordinal)) + + match prefix with + | Some prefix -> + let name = arg.Substring(prefix.Length).Trim('"') + if String.IsNullOrWhiteSpace name then None else Some name + | None when + arg.Equals("--fsi-server-jsonrpc", StringComparison.Ordinal) + || arg.Equals("-fsi-server-jsonrpc", StringComparison.Ordinal) + || arg.Equals("/fsi-server-jsonrpc", StringComparison.Ordinal) + -> + match rest with + | name :: _ when not (String.IsNullOrWhiteSpace name) -> Some (name.Trim('"')) + | _ -> None + | None when arg.StartsWith("@", StringComparison.Ordinal) -> + let responseFile = arg.Substring(1) + + if File.Exists responseFile then + let arguments = + File.ReadAllText(responseFile) + |> fun text -> text.Split([| ' '; '\t'; '\r'; '\n' |], StringSplitOptions.RemoveEmptyEntries) + + scan (Array.toList arguments @ rest) + else + scan rest + | None -> scan rest + + scan (Array.toList argv) diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs index c03b2f6ae1d..e83254d41f1 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs @@ -131,6 +131,13 @@ let ``evaluates every interaction in a request`` () = Assert.True(succeeded next, describe session next) Assert.True(session.WaitForOutput "val it: int = 22", describe session next)) +[] +let ``keeps apostrophe-terminated identifiers intact`` () = + withInitializedSession (fun session -> + let result = session.Execute "let value' = 42;; value' + 1" + Assert.True(succeeded result, describe session result) + Assert.True(session.WaitForOutput "val it: int = 43", describe session result)) + [] let ``reports what the interaction printed`` () = withInitializedSession (fun session -> @@ -328,6 +335,22 @@ let ``setPaths changes the working directory`` () = with _ -> ()) +[] +let ``setPaths rejects a missing working directory`` () = + withInitializedSession (fun session -> + let directory = Path.Combine(Path.GetTempPath(), $"fsiServerTest_{Guid.NewGuid():N}") + let error = + session.RequestExpectingError( + Methods.SetPaths, + { + includePaths = [||] + workingDirectory = directory + } + ) + + Assert.Equal(Some -32002, error) + ) + [] let ``setPaths waits its turn behind a running interaction`` () = withInitializedSession (fun session -> From f8f30bfdd04563ddc333041b141b35002c16eaa6 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Fri, 18 Sep 2026 18:35:09 +0200 Subject: [PATCH 21/30] Format FSI server fixup Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fsi/fsiserver.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fsi/fsiserver.fs b/src/fsi/fsiserver.fs index 483a3f3f640..d12078e9e7f 100644 --- a/src/fsi/fsiserver.fs +++ b/src/fsi/fsiserver.fs @@ -635,7 +635,7 @@ let internal tryGetPipeName (argv: string[]) = || arg.Equals("/fsi-server-jsonrpc", StringComparison.Ordinal) -> match rest with - | name :: _ when not (String.IsNullOrWhiteSpace name) -> Some (name.Trim('"')) + | name :: _ when not (String.IsNullOrWhiteSpace name) -> Some(name.Trim('"')) | _ -> None | None when arg.StartsWith("@", StringComparison.Ordinal) -> let responseFile = arg.Substring(1) From 70fce7d544f2dfc3521fa599e0bd143b4f89f11f Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sat, 19 Sep 2026 02:48:19 +0200 Subject: [PATCH 22/30] Watch FSI host before pipe connection Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/Compiler/Interactive/fsi.fs | 1 + src/fsi/fsimain.fs | 12 ++++- src/fsi/fsiserver.fs | 54 +++++++++++++------ .../FsiServerHarness.fs | 1 + 4 files changed, 50 insertions(+), 18 deletions(-) diff --git a/src/Compiler/Interactive/fsi.fs b/src/Compiler/Interactive/fsi.fs index dea14d2b6a5..3f7ef008d17 100644 --- a/src/Compiler/Interactive/fsi.fs +++ b/src/Compiler/Interactive/fsi.fs @@ -1082,6 +1082,7 @@ type internal FsiCommandLineOptions(fsi: FsiEvaluationSessionHostConfig, argv: s CompilerOption("fsi-server-report-references", "", OptionString(fun s -> writeReferencesAndExit <- Some s), None, None) CompilerOption("fsi-server", "", OptionString(fun s -> fsiServerName <- s), None, None) // "FSI server mode on given named channel"); CompilerOption("fsi-server-jsonrpc", "", OptionString(fun s -> fsiServerJsonRpcPipe <- s), None, None) // "FSI server mode speaking JSON-RPC over the given named pipe" + CompilerOption("fsi-server-client-pid", "", OptionString(ignore), None, None) // "Process id of the host for FSI server lifetime management" CompilerOption("fsi-server-input-codepage", "", OptionInt(fun n -> fsiServerInputCodePage <- Some(n)), None, None) // " Set the input codepage for the console"); CompilerOption("fsi-server-output-codepage", "", OptionInt(fun n -> fsiServerOutputCodePage <- Some(n)), None, None) // " Set the output codepage for the console"); CompilerOption( diff --git a/src/fsi/fsimain.fs b/src/fsi/fsimain.fs index 6e7774feef4..51903596fcf 100644 --- a/src/fsi/fsimain.fs +++ b/src/fsi/fsimain.fs @@ -188,6 +188,10 @@ let evaluateSession (argv: string[]) = // point, alongside the event loop it evaluates on. let jsonRpcPipeName = FSharp.Compiler.Interactive.Server.tryGetPipeName argv + let jsonRpcClientProcessId = + FSharp.Compiler.Interactive.Server.tryGetClientProcessId argv + |> ValueOption.ofOption + try // Create the console reader let console = new FSharp.Compiler.Interactive.ReadLineConsole() @@ -353,7 +357,13 @@ let evaluateSession (argv: string[]) = // that interactions are evaluated on. match jsonRpcPipeName with | Some pipeName -> - FSharp.Compiler.Interactive.Server.startOnBackgroundThread fsiSession fsiConfig pipeName Console.Out Console.Error + FSharp.Compiler.Interactive.Server.startOnBackgroundThread + fsiSession + fsiConfig + pipeName + jsonRpcClientProcessId + Console.Out + Console.Error | None -> () // Start the session diff --git a/src/fsi/fsiserver.fs b/src/fsi/fsiserver.fs index d12078e9e7f..3117005cbad 100644 --- a/src/fsi/fsiserver.fs +++ b/src/fsi/fsiserver.fs @@ -54,6 +54,9 @@ open FSharp.Compiler.Interactive.Shell [] let internal JsonRpcServerOption = "--fsi-server-jsonrpc:" +[] +let internal JsonRpcClientProcessIdOption = "--fsi-server-client-pid:" + /// File name reported for interactions that the host did not attribute to a source file. [] let private DefaultInteractionName = "stdin.fsx" @@ -92,6 +95,21 @@ let private toValueInfo (name: string) (value: FsiValue) = value = sprintf "%A" value.ReflectionValue } +/// Watch the process that owns this session, so that an F# Interactive left behind by a +/// crashed host does not survive as an orphan. +let private watchClientProcess (clientProcessId: int) = + try + let client = Process.GetProcessById clientProcessId + client.EnableRaisingEvents <- true + client.Exited.Add(fun _ -> exit 0) + + // The host may already have gone by the time the handler was attached. + if client.HasExited then + exit 0 + with _ -> + // An unknown process id is not fatal: the session simply loses orphan protection. + () + let private toExecutionResult (outcome: Choice) (diagnostics: FSharpDiagnostic[]) @@ -401,25 +419,10 @@ type FsiRpcTarget if not initialized then raise (LocalRpcException("'fsi/initialize' must be called first", ErrorCode = -32000)) - /// Watch the process that owns this session, so that an F# Interactive left behind by a - /// crashed host does not survive as an orphan. - let attachToClientProcess (clientProcessId: int) = - try - let client = Process.GetProcessById clientProcessId - client.EnableRaisingEvents <- true - client.Exited.Add(fun _ -> exit 0) - - // The host may already have gone by the time the handler was attached. - if client.HasExited then - exit 0 - with _ -> - // An unknown process id is not fatal: the session simply loses orphan protection. - () - [] member _.Initialize(request: InitializeRequest) : InitializeResult = if request.clientProcessId > 0 then - attachToClientProcess request.clientProcessId + watchClientProcess request.clientProcessId initialized <- true @@ -537,9 +540,12 @@ let private runServer (fsiSession: FsiEvaluationSession) (fsiConfig: FsiEvaluationSessionHostConfig) (pipeName: string) + (clientProcessId: int voption) (outWriter: TextWriter) (errorWriter: TextWriter) = + clientProcessId |> ValueOption.iter watchClientProcess + use pipe = new NamedPipeServerStream( pipeName, @@ -589,6 +595,7 @@ let internal startOnBackgroundThread (fsiSession: FsiEvaluationSession) (fsiConfig: FsiEvaluationSessionHostConfig) (pipeName: string) + (clientProcessId: int voption) (outWriter: TextWriter) (errorWriter: TextWriter) = @@ -596,7 +603,7 @@ let internal startOnBackgroundThread Thread( (fun () -> try - runServer fsiSession fsiConfig pipeName outWriter errorWriter + runServer fsiSession fsiConfig pipeName clientProcessId outWriter errorWriter with e -> errorWriter.WriteLine $"F# Interactive server terminated: {e}" errorWriter.Flush() @@ -651,3 +658,16 @@ let internal tryGetPipeName (argv: string[]) = | None -> scan rest scan (Array.toList argv) + +/// Recognise --fsi-server-client-pid:<pid>, returning the process that owns the session. +let internal tryGetClientProcessId (argv: string[]) = + argv + |> Array.tryPick (fun arg -> + if arg.StartsWith(JsonRpcClientProcessIdOption, StringComparison.Ordinal) then + let value = arg.Substring(JsonRpcClientProcessIdOption.Length) + + match Int32.TryParse value with + | true, processId when processId > 0 -> Some processId + | _ -> None + else + None) diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs index 310448db45c..1b15fdbd717 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs @@ -106,6 +106,7 @@ type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = yield! leadingArguments "--nologo" $"--fsi-server-jsonrpc:{pipeName}" + $"--fsi-server-client-pid:{Process.GetCurrentProcess().Id}" yield! defaultArg extraArguments [] ] From 64479084819a842c4aa5eb994658b0c4cd288131 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sat, 19 Sep 2026 04:29:41 +0200 Subject: [PATCH 23/30] Register FSI RPC handlers explicitly Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fsi/fsiserver.fs | 47 +++++++++++-------- src/fsi/interactiveProtocol.fs | 5 +- .../FsiServerHarness.fs | 2 +- 3 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/fsi/fsiserver.fs b/src/fsi/fsiserver.fs index 3117005cbad..c3fab87f659 100644 --- a/src/fsi/fsiserver.fs +++ b/src/fsi/fsiserver.fs @@ -24,13 +24,6 @@ /// they run in the order they arrived, while requests that must not wait behind them — an interrupt /// above all — are served as they arrive. /// -/// -/// Not module internal: FsiRpcTarget needs its members to be genuinely public IL, and -/// under --realsig- a member's own accessibility is capped by its enclosing module's, so an -/// internal module would take that away no matter what the type itself declares. Everything else -/// here goes back to private/internal explicitly instead of inheriting it from the -/// module. -/// /// module FSharp.Compiler.Interactive.Server @@ -283,20 +276,12 @@ type internal ExecutionQueue() = /// interaction finishes, which leaves StreamJsonRpc free to dispatch an interrupt in the meantime. /// /// -/// Public, not internal: AddLocalRpcTarget discovers JsonRpcMethod members by -/// reflecting over the instance it is handed, and under --realsig- a member's own IL -/// visibility is capped by its enclosing scope's, so an internal type (or an internal module around -/// a public one) would take away the public visibility that reflection needs regardless of what the -/// members declare. -/// -/// -/// StreamJsonRpc offers every public member, not only the attributed ones, so the public members -/// here are exactly the handlers the protocol defines. The construction the server loop needs goes -/// through the internal constructor instead. +/// The server loop registers the six handlers explicitly with StreamJsonRpc, so this implementation +/// type is internal and no extra members are exposed as RPC methods. /// /// [] -type FsiRpcTarget +type internal FsiRpcTarget internal ( fsiSession: FsiEvaluationSession, @@ -568,7 +553,31 @@ let private runServer use rpc = new JsonRpc(new HeaderDelimitedMessageHandler(pipe, new JsonMessageFormatter())) - rpc.AddLocalRpcTarget(target, JsonRpcTargetOptions(NotifyClientOfEvents = false, AllowNonPublicInvocation = false)) + let initialize = + Func(fun request -> target.Initialize request) + + rpc.AddLocalRpcMethod(Methods.Initialize, initialize) |> ignore + + let execute = + Func>(fun request -> target.Execute request) + + rpc.AddLocalRpcMethod(Methods.Execute, execute) |> ignore + + let executeFile = + Func>(fun request -> target.ExecuteFile request) + + rpc.AddLocalRpcMethod(Methods.ExecuteFile, executeFile) |> ignore + + let setPaths = + Func>(fun request -> target.SetPaths request) + + rpc.AddLocalRpcMethod(Methods.SetPaths, setPaths) |> ignore + + rpc.AddLocalRpcMethod(Methods.Interrupt, Func(fun () -> target.Interrupt())) + |> ignore + + rpc.AddLocalRpcMethod(Methods.Shutdown, Action(fun () -> target.Shutdown())) + |> ignore // Diagnostic breadcrumb: a host that gets "method not found" against a target that plainly // declares the method has almost certainly loaded a second, different copy of this library, so diff --git a/src/fsi/interactiveProtocol.fs b/src/fsi/interactiveProtocol.fs index 1aa352f7ff6..24a3d8e7f52 100644 --- a/src/fsi/interactiveProtocol.fs +++ b/src/fsi/interactiveProtocol.fs @@ -12,9 +12,8 @@ /// the JSON-RPC formatter can construct them. /// /// -/// Public rather than internal, even in fsi's own copy: the handlers that carry them have to be -/// public for StreamJsonRpc to find them by reflection, and a public member cannot expose a type -/// less accessible than itself. +/// Public in the shared source so the host can use the same wire types. The server registers its +/// handlers explicitly; these DTOs are not exposed by reflection as part of the server target. /// /// namespace FSharp.Compiler.Interactive.Protocol diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs index 1b15fdbd717..14a65856bde 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs @@ -203,7 +203,7 @@ type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = /// Send a request whose parameters are a single object, as every method of this protocol but /// the argument-less ones expects. member _.BeginRequest<'T>(method: string, parameters: obj) : Task<'T> = - rpc.InvokeWithParameterObjectAsync<'T>(method, parameters) + rpc.InvokeAsync<'T>(method, parameters) member _.BeginRequest<'T>(method: string) : Task<'T> = rpc.InvokeAsync<'T>(method) From 7292816c4bf9977772cb48efad53adab358cca51 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 20 Sep 2026 22:09:11 +0200 Subject: [PATCH 24/30] Evaluate every interaction EvalInteraction is given and expose the server options on the session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EvalInteraction parsed one interaction and discarded the text after its first `;;`, although its documentation promises one or more. It now commits each completed interaction and parses the next from the same lexer, as the standard-input loop does, so a host can hand over a whole selection and a line directive at its top holds for all of it. The JSON-RPC pipe name and the host process id become members of FsiEvaluationSession, read from the options the session has already parsed — response files included — instead of a second scan of argv in the entry point. The console reader is gated on the server options rather than asserted absent. Co-Authored-By: Claude Fable 5.1 --- .../.FSharp.Compiler.Service/11.0.100.md | 3 +- src/Compiler/Interactive/fsi.fs | 63 ++++++++++++++----- src/Compiler/Interactive/fsi.fsi | 7 +++ .../InteractiveSession/Misc.fs | 16 ++--- ...iler.Service.SurfaceArea.netstandard20.bsl | 4 ++ .../FSharp.Compiler.Service.Tests/FsiTests.fs | 25 ++++++++ tests/FSharp.Test.Utilities/CompilerAssert.fs | 4 ++ 7 files changed, 98 insertions(+), 24 deletions(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index fde6bba6430..b088e1f5f85 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,12 +1,13 @@ ### Added -* F# Interactive gains a JSON-RPC server mode, `--fsi-server-jsonrpc:`, in which a host submits interactions over a named pipe and receives structured results — diagnostics with positions, escaping exceptions, and the session's own process id — instead of recovering them by looking for a `SERVER-PROMPT>` marker in the output text. Program output continues to flow through the redirected console streams. ([PR #20396](https://github.com/dotnet/fsharp/pull/20396)) +* F# Interactive gains a JSON-RPC server mode, `--fsi-server-jsonrpc:`, in which a host submits interactions over a named pipe and receives structured results — diagnostics with positions, escaping exceptions, the values each interaction bound, and the session's own process id — instead of recovering them by looking for a `SERVER-PROMPT>` marker in the output text. Program output continues to flow through the redirected console streams. The pipe admits only the user running the session; `--fsi-server-client-pid:` names the host process whose exit ends the session. `FsiEvaluationSession` exposes both options as `JsonRpcServerPipeName` and `JsonRpcClientProcessId`. The mode is part of the .NET fsi only. ([PR #20396](https://github.com/dotnet/fsharp/pull/20396)) ### Fixed * Fix internal error "no 'value__' field found for enumeration type" when an `enum` constraint is checked against an enum declared in the same recursive group (`module rec` or an `and` group). The constraint is now solved once the representations of the group are established. ([Issue #14580](https://github.com/dotnet/fsharp/issues/14580), [PR #20454](https://github.com/dotnet/fsharp/pull/20454)) * Better diagnostic for a bare `enum` constraint: `'T : enum` now reports FS0699 "An 'enum' constraint must be of the form 'enum'" instead of "Unexpected identifier: 'enum (4)'", and parser messages for invalid constraints no longer leak internal ` (2)`/` (3)`/` (4)` markers. ([Issue #14580](https://github.com/dotnet/fsharp/issues/14580), [PR #20454](https://github.com/dotnet/fsharp/pull/20454)) * Fix FSI `--multiemit+ --debug-` unnecessarily generating and loading portable PDB data for every submission. ([Issue #17306](https://github.com/dotnet/fsharp/issues/17306), [PR #20394](https://github.com/dotnet/fsharp/pull/20394), [downstream report](https://github.com/bryanedds/Nu/issues/1090)) +* `FsiEvaluationSession.EvalInteraction` and `EvalInteractionNonThrowing` evaluate every `;;`-separated interaction in the text they are given, as their documentation has always said, instead of silently discarding everything after the first. ([PR #20396](https://github.com/dotnet/fsharp/pull/20396)) * Fix reference-assembly MVID collisions when a public member is renamed in an early file of a large (>~32 file) project under `--optimize-`. The per-file signature hashes were folded with a left-shift-by-one combiner over a 32-bit `Hash`, which truncated the contribution of any file more than ~32 positions from the end of the compile order, so MSBuild's `CopyRefAssembly` saw an unchanged MVID and kept a stale reference assembly (surfacing as `FS0039` downstream). The combiner is now an FNV-1a multiply mix over a 64-bit `Hash`. ([Issue #20389](https://github.com/dotnet/fsharp/issues/20389)) * Fix internal error FS0192 "Iterate2D" when a `[]` parameter auto-quotes an argument that captures a not-yet-generalized use of an inferred generically-recursive function. The auto-quoted (`Expr.WithValue`) copy now keeps a fresh link to the recursive-value use so it receives the same inferred type arguments as the executable expression at the letrec point. ([Issue #20379](https://github.com/dotnet/fsharp/issues/20379)) * Fix `StackOverflowException` when checking a long `seq { ... }` body. ([PR #20480](https://github.com/dotnet/fsharp/pull/20480)) diff --git a/src/Compiler/Interactive/fsi.fs b/src/Compiler/Interactive/fsi.fs index 3f7ef008d17..67a4560c5de 100644 --- a/src/Compiler/Interactive/fsi.fs +++ b/src/Compiler/Interactive/fsi.fs @@ -989,6 +989,7 @@ type internal FsiCommandLineOptions(fsi: FsiEvaluationSessionHostConfig, argv: s let mutable fsiLCID = None let mutable fsiServerJsonRpcPipe = "" + let mutable fsiServerClientProcessId = None // internal options let mutable probeToSeeIfConsoleWorks = true @@ -1082,7 +1083,7 @@ type internal FsiCommandLineOptions(fsi: FsiEvaluationSessionHostConfig, argv: s CompilerOption("fsi-server-report-references", "", OptionString(fun s -> writeReferencesAndExit <- Some s), None, None) CompilerOption("fsi-server", "", OptionString(fun s -> fsiServerName <- s), None, None) // "FSI server mode on given named channel"); CompilerOption("fsi-server-jsonrpc", "", OptionString(fun s -> fsiServerJsonRpcPipe <- s), None, None) // "FSI server mode speaking JSON-RPC over the given named pipe" - CompilerOption("fsi-server-client-pid", "", OptionString(ignore), None, None) // "Process id of the host for FSI server lifetime management" + CompilerOption("fsi-server-client-pid", "", OptionInt(fun n -> fsiServerClientProcessId <- Some n), None, None) // "Process id of the host; the JSON-RPC server exits when it does" CompilerOption("fsi-server-input-codepage", "", OptionInt(fun n -> fsiServerInputCodePage <- Some(n)), None, None) // " Set the input codepage for the console"); CompilerOption("fsi-server-output-codepage", "", OptionInt(fun n -> fsiServerOutputCodePage <- Some(n)), None, None) // " Set the output codepage for the console"); CompilerOption( @@ -1394,10 +1395,16 @@ type internal FsiCommandLineOptions(fsi: FsiEvaluationSessionHostConfig, argv: s member _.IsInteractiveServer = isInteractiveServer () - /// The pipe name is not surfaced: the server lives in the process entry point, which reads it - /// from the command line directly. member _.IsJsonRpcServer = isJsonRpcServer () + member _.JsonRpcServerPipeName = + if isJsonRpcServer () then + Some fsiServerJsonRpcPipe + else + None + + member _.JsonRpcClientProcessId = fsiServerClientProcessId + member _.ProbeToSeeIfConsoleWorks = probeToSeeIfConsoleWorks member _.EnableConsoleKeyProcessing = enableConsoleKeyProcessing @@ -1535,17 +1542,13 @@ type internal FsiConsoleInput let consoleOpt = // The "console.fs" code does a limited form of "TAB-completion". - // Currently, it turns on if it looks like we have a console. - if fsiOptions.EnableConsoleKeyProcessing then + // Currently, it turns on if it looks like we have a console. A session driven by a host has + // no user at a console, whatever the probe would say. + if fsiOptions.EnableConsoleKeyProcessing && not fsiOptions.IsInteractiveServer then fsi.GetOptionalConsoleReadLine(fsiOptions.ProbeToSeeIfConsoleWorks) else None - // When VFSI is running, there should be no "console", and in particular the console.fs readline code should not to run. - do - if fsiOptions.IsInteractiveServer then - assert consoleOpt.IsNone - /// This threading event gets set after the first-line-reader has finished its work let consoleReaderStartupDone = new ManualResetEvent(false) @@ -4428,11 +4431,37 @@ type FsiInteractionProcessor let tokenizer = fsiStdinLexerProvider.CreateBufferLexer(scriptFileName, lexbuf, diagnosticsLogger) - currState - |> InteractiveCatch diagnosticsLogger (fun istate -> - let expr = ParseInteraction tcConfigB.diagnosticsOptions tokenizer - ExecuteParsedInteractionOnMainThread(ctok, diagnosticsLogger, expr, istate, cancellationToken)) - |> commitResult + // The text may hold several interactions, as standard input would. Each one that completes + // is committed before the next is parsed, so that a failure later in the text keeps what ran + // before it; the value reported is that of the last interaction that produced one. + let rec run istate lastValue = + let errorsBefore = diagnosticsLogger.ErrorCount + + let istate, status = + istate + |> InteractiveCatch diagnosticsLogger (fun istate -> + match ParseInteraction tcConfigB.diagnosticsOptions tokenizer with + | Some(ParsedScriptInteraction.Definitions([], _)) -> istate, Completed lastValue + | expr -> ExecuteParsedInteractionOnMainThread(ctok, diagnosticsLogger, expr, istate, cancellationToken)) + + let status = + match status with + | Completed value -> Completed(Option.orElse lastValue value) + | status -> status + + match status with + | Completed value when + diagnosticsLogger.ErrorCount = errorsBefore + && not tokenizer.LexBuffer.IsPastEndOfStream + -> + if cancellationToken.IsCancellationRequested then + istate, CtrlC + else + setCurrState istate + run istate value + | _ -> istate, status + + run currState None |> commitResult member this.EvalScript(ctok, scriptPath, diagnosticsLogger) = // Todo: this runs the script as expected but errors are displayed one line to far in debugger @@ -4974,6 +5003,10 @@ type FsiEvaluationSession /// A host calls this to get the active language ID if provided by fsi-server-lcid member _.LCID = fsiOptions.FsiLCID + member _.JsonRpcServerPipeName = fsiOptions.JsonRpcServerPipeName + + member _.JsonRpcClientProcessId = fsiOptions.JsonRpcClientProcessId + /// A host calls this to report an unhandled exception in a standard way, e.g. an exception on the GUI thread gets printed to stderr member x.ReportUnhandledException exn = x.ReportUnhandledExceptionSafe true exn diff --git a/src/Compiler/Interactive/fsi.fsi b/src/Compiler/Interactive/fsi.fsi index 14bda032a76..d04d191cdb9 100644 --- a/src/Compiler/Interactive/fsi.fsi +++ b/src/Compiler/Interactive/fsi.fsi @@ -310,6 +310,13 @@ type FsiEvaluationSession = /// A host calls this to get the active language ID if provided by fsi-server-lcid member LCID: int option + /// The named pipe requested with `--fsi-server-jsonrpc`, when a host is to drive the session over + /// JSON-RPC instead of standard input. + member JsonRpcServerPipeName: string option + + /// The host process named with `--fsi-server-client-pid`, whose exit ends the session. + member JsonRpcClientProcessId: int option + /// A host calls this to report an unhandled exception in a standard way, e.g. an exception on the GUI thread gets printed to stderr member ReportUnhandledException: exn: exn -> unit diff --git a/tests/FSharp.Compiler.ComponentTests/InteractiveSession/Misc.fs b/tests/FSharp.Compiler.ComponentTests/InteractiveSession/Misc.fs index 1ace807a8fd..7ee54b8656c 100644 --- a/tests/FSharp.Compiler.ComponentTests/InteractiveSession/Misc.fs +++ b/tests/FSharp.Compiler.ComponentTests/InteractiveSession/Misc.fs @@ -1212,13 +1212,13 @@ System.Threading.Thread.Sleep(50);; |> ignore // Span type in FSI - [] + [] let ``SpanType - System.Span usage``() = Fsx """ open System;; let arr = [|1;2;3;4;5|];; -let span = arr.AsSpan();; -if span.Length <> 5 then failwith "test assertion failed";; +let spanLength () = arr.AsSpan().Length;; +if spanLength () <> 5 then failwith "test assertion failed";; () """ |> withOptions ["--nologo"] @@ -1552,13 +1552,13 @@ type IOps<'T> = abstract Zero : 'T;; /// Create an instance of an F77Array and capture its operation set -type Matrix<'T> internal (ops: IOps<'T>, arr: 'T[,]) = - member internal x.Ops = ops - member internal x.Data = arr;; +type Matrix<'T> (ops: IOps<'T>, arr: 'T[,]) = + member x.Ops = ops + member x.Data = arr;; type Matrix = - /// A function to capture operations - static member inline private captureOps() = + /// A function to capture operations + static member inline captureOps() = { new IOps<_> with member x.Add(a,b) = a + b member x.Zero = LanguagePrimitives.GenericZero<_> } diff --git a/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.bsl b/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.bsl index 53cacc1a885..a31be5a83f5 100644 --- a/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.bsl +++ b/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.bsl @@ -5061,8 +5061,12 @@ FSharp.Compiler.Interactive.Shell+FsiEvaluationSession: Microsoft.FSharp.Control FSharp.Compiler.Interactive.Shell+FsiEvaluationSession: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Interactive.Shell+FsiBoundValue] TryFindBoundValue(System.String) FSharp.Compiler.Interactive.Shell+FsiEvaluationSession: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Interactive.Shell+FsiValue] EvalExpression(System.String) FSharp.Compiler.Interactive.Shell+FsiEvaluationSession: Microsoft.FSharp.Core.FSharpOption`1[FSharp.Compiler.Interactive.Shell+FsiValue] EvalExpression(System.String, System.String) +FSharp.Compiler.Interactive.Shell+FsiEvaluationSession: Microsoft.FSharp.Core.FSharpOption`1[System.Int32] JsonRpcClientProcessId FSharp.Compiler.Interactive.Shell+FsiEvaluationSession: Microsoft.FSharp.Core.FSharpOption`1[System.Int32] LCID +FSharp.Compiler.Interactive.Shell+FsiEvaluationSession: Microsoft.FSharp.Core.FSharpOption`1[System.Int32] get_JsonRpcClientProcessId() FSharp.Compiler.Interactive.Shell+FsiEvaluationSession: Microsoft.FSharp.Core.FSharpOption`1[System.Int32] get_LCID() +FSharp.Compiler.Interactive.Shell+FsiEvaluationSession: Microsoft.FSharp.Core.FSharpOption`1[System.String] JsonRpcServerPipeName +FSharp.Compiler.Interactive.Shell+FsiEvaluationSession: Microsoft.FSharp.Core.FSharpOption`1[System.String] get_JsonRpcServerPipeName() FSharp.Compiler.Interactive.Shell+FsiEvaluationSession: System.Collections.Generic.IEnumerable`1[System.String] GetCompletions(System.String) FSharp.Compiler.Interactive.Shell+FsiEvaluationSession: System.Reflection.Assembly[] DynamicAssemblies FSharp.Compiler.Interactive.Shell+FsiEvaluationSession: System.Reflection.Assembly[] get_DynamicAssemblies() diff --git a/tests/FSharp.Compiler.Service.Tests/FsiTests.fs b/tests/FSharp.Compiler.Service.Tests/FsiTests.fs index 152c46867a1..2cdc4279330 100644 --- a/tests/FSharp.Compiler.Service.Tests/FsiTests.fs +++ b/tests/FSharp.Compiler.Service.Tests/FsiTests.fs @@ -703,3 +703,28 @@ module FsiTests = let byName name = fsiSession.GetBoundValues() |> List.find (fun v -> v.Name = name) Assert.shouldBe (box "haho") ((byName "r1").Value.ReflectionValue) Assert.shouldBe (box "hoha") ((byName "r2").Value.ReflectionValue) + + [] + [] + [] + [] + [] + let ``EvalInteractionNonThrowing keeps the last value a multi-interaction text produced`` (code: string) = + use fsiSession = createFsiSession false + + match fsiSession.EvalInteractionNonThrowing code with + | Choice1Of2(Some value), [||] -> Assert.shouldBe (box 42) value.ReflectionValue + | result, diagnostics -> failwith $"expected the value 42, got %A{result} with %A{diagnostics}" + + [] + let ``EvalInteractionNonThrowing evaluates every interaction in the text`` () = + use fsiSession = createFsiSession false + + match fsiSession.EvalInteractionNonThrowing "let a = 1;; let b = a + 1;; let c = b + 1" with + | Choice1Of2 _, [||] -> () + | result, diagnostics -> failwith $"expected success, got %A{result} with %A{diagnostics}" + + let byName name = + fsiSession.GetBoundValues() |> List.find (fun v -> v.Name = name) + + Assert.shouldBe (box 3) (byName "c").Value.ReflectionValue diff --git a/tests/FSharp.Test.Utilities/CompilerAssert.fs b/tests/FSharp.Test.Utilities/CompilerAssert.fs index b226fb2e75f..29dfa609eff 100644 --- a/tests/FSharp.Test.Utilities/CompilerAssert.fs +++ b/tests/FSharp.Test.Utilities/CompilerAssert.fs @@ -1021,6 +1021,10 @@ Updated automatically, please check diffs in your pull request, changes must be use errStream = new StringWriter() use script = new FSharpScript(additionalArgs = Array.append [| "--noninteractive" |] options, quiet = false, outWriter = outStream, errWriter = errStream) script.ApplyExitShadowing() + // Scripts written for piped stdin end with `#q;;`, which exits the process: drop that last + // line instead of letting it end the test host. Only the last one, so that text which merely + // contains it, in a string say, is left as written. + let source = System.Text.RegularExpressions.Regex.Replace(source, @"(? Seq.map _.Message) From 668912695316c18690c9ed1f26d1d1bf507ca75a Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 20 Sep 2026 22:09:16 +0200 Subject: [PATCH 25/30] Admit only the current user to the fsi server pipe and let the session split and print The pipe accepted any local client, and whoever connected first ran code as the user. It now opens with PipeOptions.CurrentUserOnly. The process the server watches is the one named on the command line, so a host that dies before connecting takes the session with it. The hand-written `;;` splitter is gone: EvalInteractionNonThrowing evaluates the whole text, so a verbatim string ending in a backslash or a `(*)` operator no longer misleads the server, and a line directive covers the entire selection. Values are formatted by the session's own printer under its print settings instead of `%A`, and a ToString that throws becomes the value's text rather than a failed interaction. Paths spliced into #load, #I and #silentCd are verbatim literals, so a quote or a trailing backslash in a path survives. Requests carry their parameters as one object, the shape the protocol documents. The server is compiled into the .NET fsi only: the .NET Framework fsi does not carry StreamJsonRpc and a source-only build cannot provide it. A session asked for the mode without it exits with a message instead of idling. Co-Authored-By: Claude Fable 5.1 --- src/fsi/fsi.targets | 21 +- src/fsi/fsimain.fs | 36 +- src/fsi/fsiserver.fs | 322 +++++------------- src/fsi/interactiveProtocol.fs | 8 - ...p.Compiler.Interactive.Server.Tests.fsproj | 4 +- .../FsiJsonRpcServerTests.fs | 293 ++++++++++++++-- .../FsiServerHarness.fs | 89 +++-- 7 files changed, 441 insertions(+), 332 deletions(-) diff --git a/src/fsi/fsi.targets b/src/fsi/fsi.targets index 39a27777f9e..a82cef02782 100644 --- a/src/fsi/fsi.targets +++ b/src/fsi/fsi.targets @@ -22,6 +22,13 @@ $(DefineConstants);FSI_SHADOW_COPY_REFERENCES;FSI_SERVER + + + true + $(DefineConstants);FSI_JSONRPC_SERVER + + true @@ -29,6 +36,12 @@ true + + + + + + LegacyResolver.txt @@ -36,8 +49,6 @@ - - {{FSCoreVersion}} @@ -72,10 +83,4 @@ - - - - - \ No newline at end of file diff --git a/src/fsi/fsimain.fs b/src/fsi/fsimain.fs index 51903596fcf..75ee1d13d04 100644 --- a/src/fsi/fsimain.fs +++ b/src/fsi/fsimain.fs @@ -183,15 +183,6 @@ let evaluateSession (argv: string[]) = Console.InputEncoding <- System.Text.Encoding.UTF8 Console.OutputEncoding <- System.Text.Encoding.UTF8 - // A host may ask for the JSON-RPC server mode, in which interactions arrive on a named pipe - // instead of standard input. Recognised here because the server is driven from this entry - // point, alongside the event loop it evaluates on. - let jsonRpcPipeName = FSharp.Compiler.Interactive.Server.tryGetPipeName argv - - let jsonRpcClientProcessId = - FSharp.Compiler.Interactive.Server.tryGetClientProcessId argv - |> ValueOption.ofOption - try // Create the console reader let console = new FSharp.Compiler.Interactive.ReadLineConsole() @@ -199,10 +190,7 @@ let evaluateSession (argv: string[]) = // Define the function we pass to the FsiEvaluationSession let getConsoleReadLine (probeToSeeIfConsoleWorks) = let consoleIsOperational = - if jsonRpcPipeName.IsSome then - // The session is driven by a host, so there is no user at a console to read from. - false - elif probeToSeeIfConsoleWorks then + if probeToSeeIfConsoleWorks then //if progress then fprintfn outWriter "probing to see if console works..." try // Probe to see if the console looks functional on this version of .NET @@ -263,6 +251,12 @@ let evaluateSession (argv: string[]) = let legacyReferenceResolver = LegacyMSBuildReferenceResolver.getResolver () +#if FSI_JSONRPC_SERVER + // Set when startup scripts are done and the loop interactions run on is the final one: a + // startup script may replace fsi.EventLoop, and a request posted to the old one is lost. + let eventLoopStarted = new System.Threading.ManualResetEvent(false) +#endif + // Update the configuration to include 'StartServer', WinFormsEventLoop and 'GetOptionalConsoleReadLine()' let rec fsiConfig = { new FsiEvaluationSessionHostConfig() with @@ -281,6 +275,9 @@ let evaluateSession (argv: string[]) = fsiConfig0.ReportUserCommandLineArgs args member _.EventLoopRun() = +#if FSI_JSONRPC_SERVER + eventLoopStarted.Set() |> ignore +#endif #if !FX_NO_WINFORMS match (if fsiSession.IsGui then fsiWinFormsLoop.Value else None) with | Some l -> (l :> IEventLoop).Run() @@ -353,17 +350,26 @@ let evaluateSession (argv: string[]) = | None -> s2 )) + match fsiSession.JsonRpcServerPipeName with +#if FSI_JSONRPC_SERVER // Serve the host on a background thread, leaving this thread to Run() and the event loop // that interactions are evaluated on. - match jsonRpcPipeName with | Some pipeName -> FSharp.Compiler.Interactive.Server.startOnBackgroundThread fsiSession fsiConfig pipeName - jsonRpcClientProcessId + fsiSession.JsonRpcClientProcessId + eventLoopStarted Console.Out Console.Error +#else + // Without the server the session would sit in Run() with nothing feeding it: no prompt, no + // standard input reader. An exit code gives the host something to report instead. + | Some _ -> + eprintfn "The JSON-RPC server mode is not available in this build of F# Interactive." + exit 1 +#endif | None -> () // Start the session diff --git a/src/fsi/fsiserver.fs b/src/fsi/fsiserver.fs index c3fab87f659..8ae812d7b28 100644 --- a/src/fsi/fsiserver.fs +++ b/src/fsi/fsiserver.fs @@ -32,8 +32,8 @@ open System.Collections.Concurrent open System.Diagnostics open System.IO open System.IO.Pipes +open System.Reflection open System.Runtime.InteropServices -open System.Text open System.Threading open System.Threading.Tasks @@ -42,13 +42,7 @@ open StreamJsonRpc open FSharp.Compiler.Diagnostics open FSharp.Compiler.Interactive.Protocol open FSharp.Compiler.Interactive.Shell - -/// The name of the command line option that turns on this server. -[] -let internal JsonRpcServerOption = "--fsi-server-jsonrpc:" - -[] -let internal JsonRpcClientProcessIdOption = "--fsi-server-client-pid:" +open FSharp.Compiler.Symbols /// File name reported for interactions that the host did not attribute to a source file. [] @@ -78,18 +72,12 @@ let private toDiagnosticInfo (diagnostic: FSharpDiagnostic) = endColumn = diagnostic.EndColumn } -let private toValueInfo (name: string) (value: FsiValue) = - { - name = name - typeName = - match value.ReflectionType with - | null -> "" - | typeInfo -> typeInfo.FullName - value = sprintf "%A" value.ReflectionValue - } +/// A path spliced into a directive as a verbatim string literal, in which only a quote needs escaping. +let private verbatimString (text: string) = + "@\"" + text.Replace("\"", "\"\"") + "\"" -/// Watch the process that owns this session, so that an F# Interactive left behind by a -/// crashed host does not survive as an orphan. +/// Watch the process that owns this session, so that an F# Interactive left behind by a crashed +/// host does not survive as an orphan. let private watchClientProcess (clientProcessId: int) = try let client = Process.GetProcessById clientProcessId @@ -142,87 +130,6 @@ let private toExecutionResult workingDirectory = Directory.GetCurrentDirectory() } -let private splitInteractions (code: string) = - let interactions = ResizeArray() - let current = StringBuilder() - let mutable index = 0 - let mutable inString = false - let mutable inChar = false - let mutable inLineComment = false - let mutable blockCommentDepth = 0 - - let isIdentifierPart (character: char) = - Char.IsLetterOrDigit character || character = '_' || character = '\'' - - let addInteraction () = - let text = current.ToString().Trim() - - if text.Length > 0 then - interactions.Add text - - current.Clear() |> ignore - - while index < code.Length do - let character = code[index] - let nextCharacter = if index + 1 < code.Length then code[index + 1] else '\000' - - if inLineComment then - current.Append character |> ignore - inLineComment <- character <> '\n' && character <> '\r' - elif blockCommentDepth > 0 then - current.Append character |> ignore - - if character = '(' && nextCharacter = '*' then - current.Append nextCharacter |> ignore - blockCommentDepth <- blockCommentDepth + 1 - index <- index + 1 - elif character = '*' && nextCharacter = ')' then - current.Append nextCharacter |> ignore - blockCommentDepth <- blockCommentDepth - 1 - index <- index + 1 - elif inString then - current.Append character |> ignore - - if character = '\\' && index + 1 < code.Length then - current.Append code[index + 1] |> ignore - index <- index + 1 - elif character = '"' then - inString <- false - elif inChar then - current.Append character |> ignore - - if character = '\\' && index + 1 < code.Length then - current.Append code[index + 1] |> ignore - index <- index + 1 - elif character = '\'' then - inChar <- false - elif character = '/' && nextCharacter = '/' then - current.Append character |> ignore - current.Append nextCharacter |> ignore - inLineComment <- true - index <- index + 1 - elif character = '(' && nextCharacter = '*' then - current.Append character |> ignore - current.Append nextCharacter |> ignore - blockCommentDepth <- 1 - index <- index + 1 - elif character = '"' then - current.Append character |> ignore - inString <- true - elif character = '\'' && (index = 0 || not (isIdentifierPart code[index - 1])) then - current.Append character |> ignore - inChar <- true - elif character = ';' && nextCharacter = ';' then - addInteraction () - index <- index + 1 - else - current.Append character |> ignore - - index <- index + 1 - - addInteraction () - interactions.ToArray() - //------------------------------------------------------------------------- // The server //------------------------------------------------------------------------- @@ -236,12 +143,17 @@ let private splitInteractions (code: string) = /// the session's willingness to run anything, and must not be reachable from the wire. /// [] -type internal ExecutionQueue() = +type internal ExecutionQueue(ready: WaitHandle) = let queue = new BlockingCollection unit>() let worker = Thread( (fun () -> + // Requests are accepted from the moment the host connects but run only once the session + // has finished its startup scripts: their bindings would otherwise be reported as the + // first request's, and one posted to an event loop a script then replaces is never run. + ready.WaitOne() |> ignore + for job in queue.GetConsumingEnumerable() do // A job reports its own failures to the host; nothing here may escape and kill // the worker, or the session would stop responding to every later request. @@ -276,8 +188,8 @@ type internal ExecutionQueue() = /// interaction finishes, which leaves StreamJsonRpc free to dispatch an interrupt in the meantime. /// /// -/// The server loop registers the six handlers explicitly with StreamJsonRpc, so this implementation -/// type is internal and no extra members are exposed as RPC methods. +/// The server loop registers the six handlers one by one, so this type stays internal and nothing +/// beyond the protocol is callable from the wire. /// /// [] @@ -297,11 +209,35 @@ type internal FsiRpcTarget let mutable initialized = false let values = ResizeArray() + /// Formatted by the session's own printer, so that the text matches the console's and obeys the + /// session's print settings. Formatting runs user code — a ToString override, a lazy + /// value — so a failure there becomes the value's text rather than a failed interaction. + let toValueInfo (name: string) (value: FsiValue) = + { + name = name + typeName = + match value.ReflectionType with + | null -> "" + | reflectionType -> reflectionType.FullName + value = + try + fsiSession.FormatValue(value.ReflectionValue, value.ReflectionType) + with e -> + $"<{e.GetType().Name}: {e.Message}>" + } + + /// The console prints what the user bound, not the helper bindings the compiler introduces + /// around it, such as the `patternInput` of `let a, b = …`. + let isUserBinding (evaluation: EvaluationEventArgs) = + match evaluation.Symbol with + | :? FSharpMemberOrFunctionOrValue as value -> not value.IsCompilerGenerated + | _ -> true + do fsiConfig.OnEvaluation.Add(fun evaluation -> match evaluation.FsiValue with - | Some value -> values.Add(toValueInfo evaluation.Name value) - | None -> ()) + | Some value when isUserBinding evaluation -> values.Add(toValueInfo evaluation.Name value) + | _ -> ()) /// /// Evaluate on the event loop thread, the same thread a console session evaluates on. @@ -333,37 +269,12 @@ type internal FsiRpcTarget try values.Clear() - let outcomes = ResizeArray>() - let diagnostics = ResizeArray() - let mutable stop = false - - for interaction in splitInteractions code do - if not stop then - let outcome, interactionDiagnostics = - evaluateOnEventLoop (fun () -> fsiSession.EvalInteractionNonThrowing(interaction, scriptPath, cancellation.Token)) - - outcomes.Add outcome - diagnostics.AddRange interactionDiagnostics - - stop <- - match outcome with - | Choice2Of2 _ -> true - | Choice1Of2 _ -> - interactionDiagnostics - |> Array.exists (fun diagnostic -> diagnostic.Severity = FSharpDiagnosticSeverity.Error) - - let outcome = - match - outcomes - |> Seq.tryFindBack (function - | Choice2Of2 _ -> true - | Choice1Of2 _ -> false) - with - | Some outcome -> outcome - | None -> Choice1Of2 None + + let outcome, diagnostics = + evaluateOnEventLoop (fun () -> fsiSession.EvalInteractionNonThrowing(code, scriptPath, cancellation.Token)) flushConsole () - toExecutionResult outcome (diagnostics.ToArray()) (values.ToArray()) cancellation.IsCancellationRequested + toExecutionResult outcome diagnostics (values.ToArray()) cancellation.IsCancellationRequested finally lock interruptLock (fun () -> currentCancellation <- null) cancellation.Dispose() @@ -396,7 +307,7 @@ type internal FsiRpcTarget if String.IsNullOrEmpty sourcePath || not startLine.HasValue then code else - $"# {startLine.Value} @\"{sourcePath}\"\n{code}" + $"# {startLine.Value} {verbatimString sourcePath}\n{code}" /// Refuse anything that arrives before the handshake, so that a mis-sequenced host gets a clear /// answer rather than an obscure failure later on. @@ -404,11 +315,7 @@ type internal FsiRpcTarget if not initialized then raise (LocalRpcException("'fsi/initialize' must be called first", ErrorCode = -32000)) - [] - member _.Initialize(request: InitializeRequest) : InitializeResult = - if request.clientProcessId > 0 then - watchClientProcess request.clientProcessId - + member _.Initialize() : InitializeResult = initialized <- true { @@ -423,7 +330,6 @@ type internal FsiRpcTarget supportsInterrupt = true } - [] member _.Execute(request: ExecuteRequest) : Task = requireInitialized () @@ -437,18 +343,15 @@ type internal FsiRpcTarget queueInteraction (fun () -> runInteraction text scriptPath) - [] member _.ExecuteFile(request: ExecuteFileRequest) : Task = requireInitialized () // Routed through #load so that the file joins the session the same way it would from a // script, rather than being replayed as anonymous text. - let path = request.path.Replace("\"", "\"\"") - queueInteraction (fun () -> runInteraction $"#load @\"{path}\"" request.path) + queueInteraction (fun () -> runInteraction $"#load {verbatimString request.path}" request.path) /// Apply the host's notion of where to look for sources and references, expressed as the /// directives a script would use. - [] member _.SetPaths(request: SetPathsRequest) : Task = requireInitialized () @@ -473,14 +376,14 @@ type internal FsiRpcTarget with _ -> () - directives.Add $"#silentCd @\"{request.workingDirectory}\"" + directives.Add $"#silentCd {verbatimString request.workingDirectory}" match request.includePaths with | null -> () | paths -> for path in paths do if not (String.IsNullOrWhiteSpace path) then - directives.Add $"#I @\"{path}\"" + directives.Add $"#I {verbatimString path}" if directives.Count = 0 then toExecutionResult (Choice1Of2 None) [||] [||] false @@ -492,7 +395,6 @@ type internal FsiRpcTarget /// Served straight away rather than queued, which is the point: an interrupt that waited its /// turn behind the interaction it is meant to stop would never arrive. /// - [] member _.Interrupt() : InterruptResult = requireInitialized () @@ -515,7 +417,6 @@ type internal FsiRpcTarget { interrupted = true } - [] member _.Shutdown() : unit = requireInitialized () shutdownRequested.TrySetResult() |> ignore @@ -525,19 +426,25 @@ let private runServer (fsiSession: FsiEvaluationSession) (fsiConfig: FsiEvaluationSessionHostConfig) (pipeName: string) - (clientProcessId: int voption) + (clientProcessId: int option) + (eventLoopStarted: WaitHandle) (outWriter: TextWriter) (errorWriter: TextWriter) = - clientProcessId |> ValueOption.iter watchClientProcess + // Watched before the host has connected: a host that dies while starting up must not leave a + // session waiting on the pipe forever. + clientProcessId |> Option.iter watchClientProcess + // Any local process could otherwise open the pipe, and whoever connects first runs code as this + // user. CurrentUserOnly limits the pipe's access list to the current user and rejects a client + // running as anyone else. use pipe = new NamedPipeServerStream( pipeName, PipeDirection.InOut, maxNumberOfServerInstances = 1, transmissionMode = PipeTransmissionMode.Byte, - options = PipeOptions.Asynchronous + options = (PipeOptions.Asynchronous ||| PipeOptions.CurrentUserOnly) ) pipe.WaitForConnection() @@ -545,7 +452,7 @@ let private runServer let shutdownRequested = TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously) - let executionQueue = ExecutionQueue() + let executionQueue = ExecutionQueue eventLoopStarted let target = FsiRpcTarget(fsiSession, fsiConfig, outWriter, errorWriter, shutdownRequested, executionQueue) @@ -553,39 +460,26 @@ let private runServer use rpc = new JsonRpc(new HeaderDelimitedMessageHandler(pipe, new JsonMessageFormatter())) - let initialize = - Func(fun request -> target.Initialize request) - - rpc.AddLocalRpcMethod(Methods.Initialize, initialize) |> ignore - - let execute = - Func>(fun request -> target.Execute request) - - rpc.AddLocalRpcMethod(Methods.Execute, execute) |> ignore - - let executeFile = - Func>(fun request -> target.ExecuteFile request) - - rpc.AddLocalRpcMethod(Methods.ExecuteFile, executeFile) |> ignore - - let setPaths = - Func>(fun request -> target.SetPaths request) - - rpc.AddLocalRpcMethod(Methods.SetPaths, setPaths) |> ignore - - rpc.AddLocalRpcMethod(Methods.Interrupt, Func(fun () -> target.Interrupt())) - |> ignore - - rpc.AddLocalRpcMethod(Methods.Shutdown, Action(fun () -> target.Shutdown())) - |> ignore + // Registered one by one rather than by reflecting over the target, so that exactly the + // protocol's methods are callable. A request carrying its parameters as one object — the shape + // the protocol documents — lands in the handler's single parameter. + let register (rpcMethod: string) (takesRequestObject: bool) (handlerName: string) = + let handler = + typeof.GetMethod(handlerName, BindingFlags.Instance ||| BindingFlags.Public ||| BindingFlags.NonPublic) + + rpc.AddLocalRpcMethod( + handler, + target, + JsonRpcMethodAttribute(rpcMethod, UseSingleObjectParameterDeserialization = takesRequestObject) + ) - // Diagnostic breadcrumb: a host that gets "method not found" against a target that plainly - // declares the method has almost certainly loaded a second, different copy of this library, so - // its identity here is worth more than the rest of the trace. - let streamJsonRpc = typeof.Assembly - errorWriter.WriteLine $"FSI-SERVER: StreamJsonRpc {streamJsonRpc.GetName().Version} from {streamJsonRpc.Location}" + register Methods.Initialize false (nameof target.Initialize) + register Methods.Execute true (nameof target.Execute) + register Methods.ExecuteFile true (nameof target.ExecuteFile) + register Methods.SetPaths true (nameof target.SetPaths) + register Methods.Interrupt false (nameof target.Interrupt) + register Methods.Shutdown false (nameof target.Shutdown) - errorWriter.Flush() rpc.StartListening() // Either the host goes away or it asks to stop. Both end the session. @@ -604,7 +498,8 @@ let internal startOnBackgroundThread (fsiSession: FsiEvaluationSession) (fsiConfig: FsiEvaluationSessionHostConfig) (pipeName: string) - (clientProcessId: int voption) + (clientProcessId: int option) + (eventLoopStarted: WaitHandle) (outWriter: TextWriter) (errorWriter: TextWriter) = @@ -612,7 +507,7 @@ let internal startOnBackgroundThread Thread( (fun () -> try - runServer fsiSession fsiConfig pipeName clientProcessId outWriter errorWriter + runServer fsiSession fsiConfig pipeName clientProcessId eventLoopStarted outWriter errorWriter with e -> errorWriter.WriteLine $"F# Interactive server terminated: {e}" errorWriter.Flush() @@ -625,58 +520,3 @@ let internal startOnBackgroundThread ) thread.Start() - -/// -/// Recognise --fsi-server-jsonrpc:<pipe name> in a command line, returning the pipe name. -/// -let internal tryGetPipeName (argv: string[]) = - let optionName = JsonRpcServerOption.TrimStart('-') - let optionPrefixes = [| JsonRpcServerOption; "-" + optionName; "/" + optionName |] - - let rec scan (args: string list) = - match args with - | [] -> None - | arg :: rest -> - let prefix = - optionPrefixes - |> Array.tryFind (fun prefix -> arg.StartsWith(prefix, StringComparison.Ordinal)) - - match prefix with - | Some prefix -> - let name = arg.Substring(prefix.Length).Trim('"') - if String.IsNullOrWhiteSpace name then None else Some name - | None when - arg.Equals("--fsi-server-jsonrpc", StringComparison.Ordinal) - || arg.Equals("-fsi-server-jsonrpc", StringComparison.Ordinal) - || arg.Equals("/fsi-server-jsonrpc", StringComparison.Ordinal) - -> - match rest with - | name :: _ when not (String.IsNullOrWhiteSpace name) -> Some(name.Trim('"')) - | _ -> None - | None when arg.StartsWith("@", StringComparison.Ordinal) -> - let responseFile = arg.Substring(1) - - if File.Exists responseFile then - let arguments = - File.ReadAllText(responseFile) - |> fun text -> text.Split([| ' '; '\t'; '\r'; '\n' |], StringSplitOptions.RemoveEmptyEntries) - - scan (Array.toList arguments @ rest) - else - scan rest - | None -> scan rest - - scan (Array.toList argv) - -/// Recognise --fsi-server-client-pid:<pid>, returning the process that owns the session. -let internal tryGetClientProcessId (argv: string[]) = - argv - |> Array.tryPick (fun arg -> - if arg.StartsWith(JsonRpcClientProcessIdOption, StringComparison.Ordinal) then - let value = arg.Substring(JsonRpcClientProcessIdOption.Length) - - match Int32.TryParse value with - | true, processId when processId > 0 -> Some processId - | _ -> None - else - None) diff --git a/src/fsi/interactiveProtocol.fs b/src/fsi/interactiveProtocol.fs index 24a3d8e7f52..ebc4aa050c9 100644 --- a/src/fsi/interactiveProtocol.fs +++ b/src/fsi/interactiveProtocol.fs @@ -38,14 +38,6 @@ module Methods = [] let Shutdown = "fsi/shutdown" -[] -type InitializeRequest = - { - /// The process that owns this session. F# Interactive watches it and exits when it goes, so - /// that a crashed editor does not leave an orphan behind. - clientProcessId: int - } - [] type InitializeResult = { diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FSharp.Compiler.Interactive.Server.Tests.fsproj b/tests/FSharp.Compiler.Interactive.Server.Tests/FSharp.Compiler.Interactive.Server.Tests.fsproj index e9e568cc6a1..6c96a8f7e7d 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FSharp.Compiler.Interactive.Server.Tests.fsproj +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FSharp.Compiler.Interactive.Server.Tests.fsproj @@ -1,8 +1,8 @@ - net472;$(FSharpNetCoreProductTargetFramework) - $(FSharpNetCoreProductTargetFramework) + + $(FSharpNetCoreProductTargetFramework) Exe false diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs index e83254d41f1..9068c03a04e 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs @@ -6,6 +6,7 @@ open System open System.IO open System.Runtime.InteropServices open System.Threading +open System.Xml.Linq open Xunit open FSharp.Compiler.Interactive.Protocol @@ -41,6 +42,9 @@ standard output: standard error: {session.StandardError}""" +let private temporaryPath (suffix: string) = + Path.Combine(Path.GetTempPath(), $"fsiServerTest_{Guid.NewGuid():N}{suffix}") + //------------------------------------------------------------------------- // Handshake //------------------------------------------------------------------------- @@ -79,9 +83,9 @@ let ``unknown methods are refused`` () = [] let ``only the protocol's own methods are reachable`` () = withInitializedSession (fun session -> - // StreamJsonRpc offers every public member of the target it is given, so a member that - // closed the execution queue would let a host silently stop the session from ever running - // another interaction. + // The handlers are registered one by one, so an implementation member — here the one that + // would close the execution queue and stop the session from ever running another + // interaction — is not a method a host can call. match session.RequestExpectingError("Complete", obj ()) with | None -> failwith "the session accepted a method that is not part of the protocol" | Some code -> Assert.Equal(-32601, code) @@ -89,6 +93,39 @@ let ``only the protocol's own methods are reachable`` () = let result = session.Execute "1 + 1" Assert.True(succeeded result, describe session result)) +//------------------------------------------------------------------------- +// The command line +//------------------------------------------------------------------------- + +[] +let ``accepts the switch in its slash spelling`` () = + // fsi's own option parser recognises the switch, so the other spelling it takes for a long option + // turns the server on too. + use session = + new FsiServerHarness(serverSwitches = fun pipeName -> [ $"/fsi-server-jsonrpc:{pipeName}" ]) + + Assert.Equal(session.ProcessId, session.Initialize().processId) + +[] +let ``accepts the switch from a response file`` () = + let responseFile = temporaryPath ".rsp" + + try + use session = + new FsiServerHarness( + serverSwitches = + fun pipeName -> + File.WriteAllText(responseFile, $"--fsi-server-jsonrpc:{pipeName}{Environment.NewLine}") + [ $"@{responseFile}" ] + ) + + Assert.Equal(session.ProcessId, session.Initialize().processId) + finally + try + File.Delete responseFile + with _ -> + () + //------------------------------------------------------------------------- // Evaluating interactions //------------------------------------------------------------------------- @@ -111,6 +148,73 @@ let ``returns evaluated values`` () = Assert.True(succeeded result, describe session result) Assert.Contains(result.values, fun value -> value.name = "answer" && value.value = "42")) +[] +let ``returns the value of an expression as it`` () = + withInitializedSession (fun session -> + let result = session.Execute "6 * 7" + Assert.True(succeeded result, describe session result) + Assert.Contains(result.values, fun value -> value.name = "it" && value.value = "42")) + +[] +let ``does not report the helper bindings the compiler introduces`` () = + withInitializedSession (fun session -> + // `let a, b = …` compiles through a `patternInput` binding of its own, which must not be + // taken for the user's. + let result = session.Execute """let patternInput = "user";; let a, b = (1, 2)""" + Assert.True(succeeded result, describe session result) + + let names = result.values |> Array.map _.name |> Array.sort + Assert.Equal([| "a"; "b"; "patternInput" |], names)) + +[] +let ``runs requests only after the startup scripts are done`` () = + let startup = temporaryPath ".fsx" + + // Long enough for the request to arrive while the script is still running, and it replaces the + // event loop the way a script that drives its own UI toolkit does. + File.WriteAllText( + startup, + """ +System.Threading.Thread.Sleep 3000 +fsi.EventLoop <- System.Activator.CreateInstance(fsi.EventLoop.GetType(), true) :?> FSharp.Compiler.Interactive.IEventLoop +let startupOnly = 123 +""" + ) + + try + use session = new FsiServerHarness(extraArguments = [ $"--use:{startup}" ]) + session.Initialize() |> ignore + + let result = session.Execute("let requested = 42", timeout = TimeSpan.FromSeconds 60.0) + Assert.True(succeeded result, describe session result) + + // The script's own binding is not this request's. + Assert.Equal([| "requested" |], result.values |> Array.map _.name) + finally + try + File.Delete startup + with _ -> + () + +[] +let ``formats values with the session's own printer`` () = + withInitializedSession (fun session -> + let result = session.Execute "let many = [ 1 .. 200 ]" + Assert.True(succeeded result, describe session result) + + // The session's print length applies, so the text is bounded the way the console's is. + let many = result.values |> Array.find (fun value -> value.name = "many") + Assert.Contains("...", many.value)) + +[] +let ``a value whose ToString throws does not fail the interaction`` () = + withInitializedSession (fun session -> + let result = + session.Execute "type Loud() = override _.ToString() = failwith \"boom\";; let loud = Loud()" + + Assert.True(succeeded result, describe session result) + Assert.Contains(result.values, fun value -> value.name = "loud")) + [] let ``keeps bindings across interactions`` () = withInitializedSession (fun session -> @@ -138,6 +242,34 @@ let ``keeps apostrophe-terminated identifiers intact`` () = Assert.True(succeeded result, describe session result) Assert.True(session.WaitForOutput "val it: int = 43", describe session result)) +[] +let ``splits interactions the way the lexer does`` () = + withInitializedSession (fun session -> + // A verbatim string ending in a backslash, and `(*)` — the operator, not a comment. Each would + // fool a splitter that only looks for `;;` outside strings and comments. + let result = + session.Execute """let path = @"C:\";; let times = (*);; let after = path.Length + times 2 3""" + + Assert.True(succeeded result, describe session result) + + let next = session.Execute "after" + Assert.True(succeeded next, describe session next) + Assert.True(session.WaitForOutput "val it: int = 9", describe session next)) + +[] +let ``keeps the bindings that ran before a failing interaction`` () = + withInitializedSession (fun session -> + let result = session.Execute "let kept = 1;; let broken: int = \"text\";; let never = 2" + Assert.False(succeeded result, describe session result) + Assert.NotEmpty(errors result) + + let kept = session.Execute "kept" + Assert.True(succeeded kept, describe session kept) + Assert.True(session.WaitForOutput "val it: int = 1", describe session kept) + + // Nothing after the failure ran. + Assert.False(succeeded (session.Execute "never"))) + [] let ``reports what the interaction printed`` () = withInitializedSession (fun session -> @@ -217,6 +349,19 @@ let ``attributes diagnostics to the host's file and line`` () = Assert.Equal(120, reported[0].startLine) Assert.EndsWith("Library.fs", reported[0].fileName)) +[] +let ``positions every interaction of a selection against the host's lines`` () = + withInitializedSession (fun session -> + // The line directive covers the whole selection, not just the text before its first `;;`. + let path = Path.Combine(Path.GetTempPath(), "Library.fs") + + let result = + session.Execute("let ok = 1;;\nlet bad: int = \"text\"", sourcePath = path, startLine = 120) + + let reported = errors result + Assert.NotEmpty reported + Assert.Equal(121, reported[0].startLine)) + [] let ``reports an escaping exception`` () = withInitializedSession (fun session -> @@ -256,9 +401,7 @@ let ``keeps serving after a failed interaction`` () = [] let ``loads a script file`` () = withInitializedSession (fun session -> - let script = - Path.Combine(Path.GetTempPath(), $"fsiServerTest_{Guid.NewGuid():N}.fsx") - + let script = temporaryPath ".fsx" File.WriteAllText(script, "printfn \"the script ran\"\n") try @@ -283,9 +426,7 @@ let ``loads a script file whose path contains quotes`` () = () else withInitializedSession (fun session -> - let directory = - Path.Combine(Path.GetTempPath(), $"fsiServerTest_{Guid.NewGuid():N}\"quoted") - + let directory = temporaryPath "\"quoted" Directory.CreateDirectory directory |> ignore let script = Path.Combine(directory, "script.fsx") File.WriteAllText(script, "printfn \"quoted path loaded\"\n") @@ -303,9 +444,7 @@ let ``loads a script file whose path contains quotes`` () = [] let ``setPaths changes the working directory`` () = withInitializedSession (fun session -> - let directory = - Path.Combine(Path.GetTempPath(), $"fsiServerTest_{Guid.NewGuid():N}") - + let directory = temporaryPath "" Directory.CreateDirectory directory |> ignore try @@ -338,25 +477,21 @@ let ``setPaths changes the working directory`` () = [] let ``setPaths rejects a missing working directory`` () = withInitializedSession (fun session -> - let directory = Path.Combine(Path.GetTempPath(), $"fsiServerTest_{Guid.NewGuid():N}") let error = session.RequestExpectingError( Methods.SetPaths, { includePaths = [||] - workingDirectory = directory + workingDirectory = temporaryPath "" } ) - Assert.Equal(Some -32002, error) - ) + Assert.Equal(Some -32002, error)) [] let ``setPaths waits its turn behind a running interaction`` () = withInitializedSession (fun session -> - let directory = - Path.Combine(Path.GetTempPath(), $"fsiServerTest_{Guid.NewGuid():N}") - + let directory = temporaryPath "" Directory.CreateDirectory directory |> ignore try @@ -474,11 +609,127 @@ let ``the session exits when its host process exits`` () = // A second session stands in for the editor: it is a real, live process to attach to, and // killing it must bring down the session that named it as its host. use host = new FsiServerHarness() - use session = new FsiServerHarness() + use session = new FsiServerHarness(clientProcessId = host.ProcessId) - session.Initialize(clientProcessId = host.ProcessId) |> ignore + session.Initialize() |> ignore Assert.False session.HasExited (host :> IDisposable).Dispose() Assert.True(session.WaitForExit 30_000, "the session outlived its host process") + +//------------------------------------------------------------------------- +// What ships +//------------------------------------------------------------------------- + +/// The `Microsoft.FSharp.Compiler` package is what the .NET SDK lays out as `dotnet fsi`: a file its +/// manifest does not list is a file no installed SDK has. +module private CompilerPackage = + + let private repositoryRoot () = + let rec search (directory: DirectoryInfo) = + match directory with + | null -> failwith "the repository root was not found above the test output" + | directory -> + if File.Exists(Path.Combine(directory.FullName, "src", "Microsoft.FSharp.Compiler", "Microsoft.FSharp.Compiler.nuspec")) then + directory.FullName + else + search directory.Parent + + search (DirectoryInfo AppContext.BaseDirectory) + + let private projectDirectory () = + Path.Combine(repositoryRoot (), "src", "Microsoft.FSharp.Compiler") + + let private localName (name: string) (element: XElement) = element.Name.LocalName = name + + /// The assemblies the project file adds next to fsi.dll for the JSON-RPC server. + let serverAssemblies () = + XDocument.Load(Path.Combine(projectDirectory (), "Microsoft.FSharp.Compiler.fsproj")).Descendants() + |> Seq.filter (localName "FsiJsonRpcServerAssembly") + |> Seq.map (fun element -> element.Attribute(XName.Get "Include").Value) + |> Seq.toArray + + /// The assemblies the manifest puts into the package's lib folder beside fsi.dll — by name, since + /// fsi's own output holds the same builds of them that the pack step picks up. + let libraryAssemblies () = + XDocument.Load(Path.Combine(projectDirectory (), "Microsoft.FSharp.Compiler.nuspec")).Descendants() + |> Seq.filter (localName "file") + |> Seq.choose (fun element -> + let source = element.Attribute(XName.Get "src").Value + let target = element.Attribute(XName.Get "target").Value + + // Resource satellites are globbed. The compiler driver and the MSBuild tasks share the + // folder but are not fsi's to load, and fsi's own build does not produce them. + // The nuspec spells paths with backslashes, which Path.GetFileName only splits on Windows. + let name = source.Substring(source.LastIndexOf '\\' + 1) + + if + target.StartsWith("lib", StringComparison.Ordinal) + && source.IndexOf("**", StringComparison.Ordinal) < 0 + && name.EndsWith(".dll", StringComparison.Ordinal) + && name <> "fsc.dll" + && name <> "FSharp.Build.dll" + then + Some name + else + None) + |> Seq.toArray + + /// Assemblies the SDK provides beside fsi from its own build, so the package leaves them out. + let providedBySdk (fileName: string) = + fileName.StartsWith("Microsoft.Build.", StringComparison.Ordinal) + || fileName.StartsWith("Microsoft.NET.StringTools", StringComparison.Ordinal) + || fileName.StartsWith("System.", StringComparison.Ordinal) + +[] +let ``the compiler package lists every assembly the server loads`` () = + // Whatever fsi's build restored beyond what this repository builds and what the SDK provides is + // there for the server, and has to be in the package or the shipped fsi cannot start the server. + let restoredForServer = + Directory.EnumerateFiles(fsiOutputDirectory (), "*.dll") + |> Seq.map Path.GetFileName + |> Seq.filter (fun name -> + not (name.StartsWith("FSharp.", StringComparison.Ordinal)) + && name <> "fsi.dll" + && not (CompilerPackage.providedBySdk name)) + |> Seq.sort + |> Seq.toArray + + Assert.Equal(restoredForServer, CompilerPackage.serverAssemblies () |> Array.sort) + +[] +let ``the shipped files are enough to start the server`` () = + // Stage exactly what an SDK has beside fsi.dll — the package's lib folder plus the assemblies the + // SDK adds from its own build — and start a session from there. + let staged = temporaryPath "" + Directory.CreateDirectory staged |> ignore + + let fsiDirectory = fsiOutputDirectory () + + let stage (fileName: string) = + File.Copy(Path.Combine(fsiDirectory, fileName), Path.Combine(staged, fileName), true) + + try + CompilerPackage.libraryAssemblies () |> Array.iter stage + CompilerPackage.serverAssemblies () |> Array.iter stage + + Directory.EnumerateFiles(fsiDirectory, "*.dll") + |> Seq.map Path.GetFileName + |> Seq.filter CompilerPackage.providedBySdk + |> Seq.iter stage + + // No fsi.deps.json: the SDK generates its own, and without one the host probes the directory, + // so the files themselves are what is under test. + stage "fsi.runtimeconfig.json" + + use session = new FsiServerHarness(fsiDirectory = staged) + session.Initialize() |> ignore + + let result = session.Execute "1 + 1" + Assert.True(succeeded result, describe session result) + finally + try + Directory.Delete(staged, true) + with _ -> + () diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs index 14a65856bde..4b7637602e7 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs @@ -47,40 +47,48 @@ let private locateDotnetHost () = search (DirectoryInfo(AppContext.BaseDirectory)) -/// Locate the fsi built by this repository, alongside the test assembly's own output, and how to -/// launch it. -/// -/// Test output lives at `/bin///`, and fsi is its -/// sibling at `/bin/fsi//`. net472's fsi is a native -/// executable that runs directly; every other framework's is a managed dll run under the dotnet -/// host — the same split `InteractiveHost.fs` makes for the window. -let private locateFsi () = +/// Where this build put its outputs: `/bin`, and the configuration and framework this +/// test assembly was built for, which fsi shares. +let buildOutput () = let baseDirectory = DirectoryInfo(AppContext.BaseDirectory.TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar)) - let framework = baseDirectory.Name - let configuration = baseDirectory.Parent.Name - let binDirectory = baseDirectory.Parent.Parent.Parent - let fsiDirectory = Path.Combine(binDirectory.FullName, "fsi", configuration, framework) - - if framework = "net472" then - let fsi = Path.Combine(fsiDirectory, "fsi.exe") + struct {| + BinDirectory = baseDirectory.Parent.Parent.Parent.FullName + Configuration = baseDirectory.Parent.Name + Framework = baseDirectory.Name + |} - if not (File.Exists fsi) then - failwith $"Could not find the fsi under test at '{fsi}'. Build src/fsi first." +/// The fsi built by this repository: a sibling of the test output at +/// `/bin/fsi//`. +let fsiOutputDirectory () = + let output = buildOutput () + Path.Combine(output.BinDirectory, "fsi", output.Configuration, output.Framework) - fsi, [] - else - let fsi = Path.Combine(fsiDirectory, "fsi.dll") +/// fsi is a managed dll run under the dotnet host — the same way `InteractiveHost.fs` launches it +/// for the window. +let private locateFsi (fsiDirectory: string) = + let fsi = Path.Combine(fsiDirectory, "fsi.dll") - if not (File.Exists fsi) then - failwith $"Could not find the fsi under test at '{fsi}'. Build src/fsi first." + if not (File.Exists fsi) then + failwith $"Could not find the fsi under test at '{fsi}'. Build src/fsi first." - locateDotnetHost (), [ fsi ] + locateDotnetHost (), [ fsi ] /// A running session, plus everything needed to talk to it and to explain a failure. +/// +/// `serverSwitches` spells the switch that turns the server on, for tests of the forms fsi accepts; +/// `fsiDirectory` points at an fsi other than the build's own, for tests of what ships; +/// `clientProcessId` names the process whose exit ends the session, this one by default. [] -type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = +type FsiServerHarness + ( + ?extraArguments: string list, + ?workingDirectory: string, + ?serverSwitches: string -> string list, + ?fsiDirectory: string, + ?clientProcessId: int + ) = // On Unix the pipe is a socket under $TMPDIR, and macOS caps socket paths at 104 characters. let pipeName = $"fsi{Guid.NewGuid():N}".Substring(0, 15) let standardOutput = StringBuilder() @@ -99,14 +107,18 @@ type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = argument let startInfo = - let fsiHost, leadingArguments = locateFsi () + let fsiHost, leadingArguments = + locateFsi (defaultArg fsiDirectory (fsiOutputDirectory ())) + + let serverSwitches = + defaultArg serverSwitches (fun pipeName -> [ $"--fsi-server-jsonrpc:{pipeName}" ]) let arguments = [ yield! leadingArguments "--nologo" - $"--fsi-server-jsonrpc:{pipeName}" - $"--fsi-server-client-pid:{Process.GetCurrentProcess().Id}" + yield! serverSwitches pipeName + $"--fsi-server-client-pid:{defaultArg clientProcessId (Process.GetCurrentProcess().Id)}" yield! defaultArg extraArguments [] ] @@ -141,8 +153,15 @@ type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = session.BeginErrorReadLine() let pipe = + // The server admits its own user only, and a client that says so too is turned away from + // a pipe somebody else opened under the same name. let pipe = - new NamedPipeClientStream(".", pipeName, PipeDirection.InOut, PipeOptions.Asynchronous) + new NamedPipeClientStream( + ".", + pipeName, + PipeDirection.InOut, + PipeOptions.Asynchronous ||| PipeOptions.CurrentUserOnly + ) try pipe.Connect 60_000 @@ -163,9 +182,8 @@ type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = rpc.StartListening() rpc - /// On failure, fold in what the session itself printed — the only way to see, say, which - /// StreamJsonRpc the session actually loaded if a request comes back "method not found" - /// against a target that plainly declares it. + /// On failure, fold in what the session itself printed: a protocol error alone rarely says what + /// the session actually did. let await (work: Task<'T>) (timeout: TimeSpan) = try if not (work.Wait timeout) then @@ -203,7 +221,7 @@ type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = /// Send a request whose parameters are a single object, as every method of this protocol but /// the argument-less ones expects. member _.BeginRequest<'T>(method: string, parameters: obj) : Task<'T> = - rpc.InvokeAsync<'T>(method, parameters) + rpc.InvokeWithParameterObjectAsync<'T>(method, parameters) member _.BeginRequest<'T>(method: string) : Task<'T> = rpc.InvokeAsync<'T>(method) @@ -241,11 +259,8 @@ type FsiServerHarness(?extraArguments: string list, ?workingDirectory: string) = | None -> raise e /// Perform the handshake every host makes before submitting anything. - member this.Initialize(?clientProcessId: int) = - let clientProcessId = - defaultArg clientProcessId (Process.GetCurrentProcess().Id) - - this.Request(Methods.Initialize, { clientProcessId = clientProcessId }) + member this.Initialize() = + this.Request Methods.Initialize static member ExecuteParams(code: string, ?sourcePath: string, ?startLine: int) : ExecuteRequest = { From 0a63a81a72490c9056b4905f397e45bc847fc8e2 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 20 Sep 2026 22:09:16 +0200 Subject: [PATCH 26/30] Ship the server's assembly closure in Microsoft.FSharp.Compiler, not in the VS setup The SDK lays out `dotnet fsi` from this package's lib folder, and the Visual Studio installer entries added before reached no SDK. The project file lists the closure once and feeds the nuspec through a token that is empty under a source-only build. A test checks the list against what fsi's build restores and starts a session from the staged files alone. Co-Authored-By: Claude Fable 5.1 --- .../Microsoft.FSharp.Compiler.MSBuild.csproj | 7 ------- .../Microsoft.FSharp.Compiler.fsproj | 12 ++++++++++++ .../Microsoft.FSharp.Compiler.nuspec | 4 ++++ .../FsiJsonRpcServerTests.fs | 4 ++-- 4 files changed, 18 insertions(+), 9 deletions(-) diff --git a/setup/Swix/Microsoft.FSharp.Compiler.MSBuild/Microsoft.FSharp.Compiler.MSBuild.csproj b/setup/Swix/Microsoft.FSharp.Compiler.MSBuild/Microsoft.FSharp.Compiler.MSBuild.csproj index ea70d1b9a1f..a6cf0324ca9 100644 --- a/setup/Swix/Microsoft.FSharp.Compiler.MSBuild/Microsoft.FSharp.Compiler.MSBuild.csproj +++ b/setup/Swix/Microsoft.FSharp.Compiler.MSBuild/Microsoft.FSharp.Compiler.MSBuild.csproj @@ -88,13 +88,6 @@ folder "InstallDir:Common7\IDE\CommonExtensions\Microsoft\FSharp\Tools" file source="$(BinariesFolder)fscArm64\$(Configuration)\$(TargetFramework)\fscArm64.exe.config" file source="$(BinariesFolder)fsi\$(Configuration)\$(TargetFramework)\fsi.exe" vs.file.ngen=yes vs.file.ngenArchitecture=X86 vs.file.ngenPriority=2 vs.file.ngenApplication="[installDir]\Common7\IDE\CommonExtensions\Microsoft\FSharp\Tools\fsi.exe" file source="$(BinariesFolder)fsi\$(Configuration)\$(TargetFramework)\fsi.exe.config" - file source="$(BinariesFolder)fsi\$(Configuration)\$(TargetFramework)\MessagePack.dll" - file source="$(BinariesFolder)fsi\$(Configuration)\$(TargetFramework)\MessagePack.Annotations.dll" - file source="$(BinariesFolder)fsi\$(Configuration)\$(TargetFramework)\Nerdbank.MessagePack.dll" - file source="$(BinariesFolder)fsi\$(Configuration)\$(TargetFramework)\Nerdbank.Streams.dll" - file source="$(BinariesFolder)fsi\$(Configuration)\$(TargetFramework)\Newtonsoft.Json.dll" - file source="$(BinariesFolder)fsi\$(Configuration)\$(TargetFramework)\PolyType.dll" - file source="$(BinariesFolder)fsi\$(Configuration)\$(TargetFramework)\StreamJsonRpc.dll" file source="$(BinariesFolder)fsiAnyCpu\$(Configuration)\$(TargetFramework)\fsiAnyCpu.exe" vs.file.ngen=yes vs.file.ngenArchitecture=X64 vs.file.ngenPriority=2 vs.file.ngenApplication="[installDir]\Common7\IDE\CommonExtensions\Microsoft\FSharp\Tools\fsiAnyCpu.exe" file source="$(BinariesFolder)fsiAnyCpu\$(Configuration)\$(TargetFramework)\fsiAnyCpu.exe.config" file source="$(BinariesFolder)fsiArm64\$(Configuration)\$(TargetFramework)\fsiArm64.exe" vs.file.ngen=yes vs.file.ngenArchitecture=arm64 vs.file.ngenPriority=2 vs.file.ngenApplication="[installDir]\Common7\IDE\CommonExtensions\Microsoft\FSharp\Tools\fsiAnyCpu.exe" diff --git a/src/Microsoft.FSharp.Compiler/Microsoft.FSharp.Compiler.fsproj b/src/Microsoft.FSharp.Compiler/Microsoft.FSharp.Compiler.fsproj index 76a3201346d..6d22630f336 100644 --- a/src/Microsoft.FSharp.Compiler/Microsoft.FSharp.Compiler.fsproj +++ b/src/Microsoft.FSharp.Compiler/Microsoft.FSharp.Compiler.fsproj @@ -26,6 +26,18 @@ + + + MessagePack.dll;MessagePack.Annotations.dll;Microsoft.VisualStudio.Threading.dll;Microsoft.VisualStudio.Validation.dll;Nerdbank.MessagePack.dll;Nerdbank.Streams.dll;Newtonsoft.Json.dll;PolyType.dll;StreamJsonRpc.dll + $([System.Text.RegularExpressions.Regex]::Replace('$(FsiJsonRpcServerAssemblies)', '([^;]+);?', '<file src="fsi\$(Configuration)\$(FSharpNetCoreProductTargetFramework)\$1" target="lib\$(FSharpNetCoreProductTargetFramework)" /> ')) + + + + + diff --git a/src/Microsoft.FSharp.Compiler/Microsoft.FSharp.Compiler.nuspec b/src/Microsoft.FSharp.Compiler/Microsoft.FSharp.Compiler.nuspec index c8b40cf588d..f2a141f6d7d 100644 --- a/src/Microsoft.FSharp.Compiler/Microsoft.FSharp.Compiler.nuspec +++ b/src/Microsoft.FSharp.Compiler/Microsoft.FSharp.Compiler.nuspec @@ -28,6 +28,10 @@ + + $fsiJsonRpcServerFiles$ diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs index 9068c03a04e..43a8bc3bec6 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs @@ -646,8 +646,8 @@ module private CompilerPackage = /// The assemblies the project file adds next to fsi.dll for the JSON-RPC server. let serverAssemblies () = XDocument.Load(Path.Combine(projectDirectory (), "Microsoft.FSharp.Compiler.fsproj")).Descendants() - |> Seq.filter (localName "FsiJsonRpcServerAssembly") - |> Seq.map (fun element -> element.Attribute(XName.Get "Include").Value) + |> Seq.filter (localName "FsiJsonRpcServerAssemblies") + |> Seq.collect (fun element -> element.Value.Split([| ';' |], StringSplitOptions.RemoveEmptyEntries)) |> Seq.toArray /// The assemblies the manifest puts into the package's lib folder beside fsi.dll — by name, since From cf9482c0067acdbf94a5dccd448db7943d165e1a Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Sun, 20 Sep 2026 22:22:12 +0200 Subject: [PATCH 27/30] Compare the reported framework and file name ordinally xUnit's StartsWith and EndsWith default to the current culture. Co-Authored-By: Claude Fable 5.1 --- .../FsiJsonRpcServerTests.fs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs index 43a8bc3bec6..366ae5d096d 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs @@ -58,7 +58,7 @@ let ``initialize reports the session process`` () = // actually evaluating code rather than any launcher in front of it. Assert.Equal(session.ProcessId, result.processId) - Assert.StartsWith(".NET", result.frameworkDescription) + Assert.StartsWith(".NET", result.frameworkDescription, StringComparison.Ordinal) Assert.True result.supportsInterrupt Assert.True( @@ -347,7 +347,7 @@ let ``attributes diagnostics to the host's file and line`` () = let reported = errors result Assert.NotEmpty reported Assert.Equal(120, reported[0].startLine) - Assert.EndsWith("Library.fs", reported[0].fileName)) + Assert.EndsWith("Library.fs", reported[0].fileName, StringComparison.Ordinal)) [] let ``positions every interaction of a selection against the host's lines`` () = From c3057e6f87789d0fcb4e2ef68b2dea06edec6d1a Mon Sep 17 00:00:00 2001 From: perf-bundle Date: Mon, 21 Sep 2026 16:19:40 +0200 Subject: [PATCH 28/30] Harden FSI JSON-RPC protocol handling Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/Compiler/Interactive/fsi.fs | 2 +- src/fsi/fsiserver.fs | 227 +++++++++++++----- .../Scripting/Interactive.fs | 9 + .../InteractiveSession/Misc/Array2D01.fs | 4 +- .../Misc/NativeIntSuffix01.fs | 2 - .../Misc/PipingWithDirectives.fs | 2 - .../Misc/UNativeIntSuffix01.fs | 2 - .../FsiJsonRpcServerTests.fs | 125 ++++++++-- .../FsiServerHarness.fs | 16 +- .../FSharp.Compiler.Service.Tests/FsiTests.fs | 13 + 10 files changed, 309 insertions(+), 93 deletions(-) diff --git a/src/Compiler/Interactive/fsi.fs b/src/Compiler/Interactive/fsi.fs index 67a4560c5de..c96f008a233 100644 --- a/src/Compiler/Interactive/fsi.fs +++ b/src/Compiler/Interactive/fsi.fs @@ -4458,7 +4458,7 @@ type FsiInteractionProcessor istate, CtrlC else setCurrState istate - run istate value + run istate (Option.orElse value lastValue) | _ -> istate, status run currState None |> commitResult diff --git a/src/fsi/fsiserver.fs b/src/fsi/fsiserver.fs index 8ae812d7b28..40a03503ecf 100644 --- a/src/fsi/fsiserver.fs +++ b/src/fsi/fsiserver.fs @@ -37,6 +37,8 @@ open System.Runtime.InteropServices open System.Threading open System.Threading.Tasks +open Newtonsoft.Json +open Newtonsoft.Json.Linq open StreamJsonRpc open FSharp.Compiler.Diagnostics @@ -79,17 +81,89 @@ let private verbatimString (text: string) = /// Watch the process that owns this session, so that an F# Interactive left behind by a crashed /// host does not survive as an orphan. let private watchClientProcess (clientProcessId: int) = - try - let client = Process.GetProcessById clientProcessId - client.EnableRaisingEvents <- true - client.Exited.Add(fun _ -> exit 0) - - // The host may already have gone by the time the handler was attached. - if client.HasExited then - exit 0 - with _ -> - // An unknown process id is not fatal: the session simply loses orphan protection. - () + let client = Process.GetProcessById clientProcessId + client.EnableRaisingEvents <- true + client.Exited.Add(fun _ -> exit 0) + + // The host may already have gone by the time the handler was attached. + if client.HasExited then + exit 0 + +let private invalidParams message = + LocalRpcException(message, ErrorCode = -32602) + +[] +type private StrictStringJsonConverter() = + inherit JsonConverter() + + override _.CanConvert(objectType) = objectType = typeof + override _.CanWrite = false + + override _.ReadJson(reader, _, _, _) = + match reader.TokenType with + | JsonToken.String -> reader.Value + | JsonToken.Null -> null + | token -> raise (invalidParams $"Expected a string or null, but found {token}.") + + override _.WriteJson(_, _, _) = raise (NotSupportedException()) + +[] +type private StrictStringArrayJsonConverter() = + inherit JsonConverter() + + override _.CanConvert(objectType) = objectType = typeof + override _.CanWrite = false + + override _.ReadJson(reader, _, _, _) = + let token = JToken.ReadFrom reader + + match token.Type with + | JTokenType.Null -> null + | JTokenType.Array -> + token.Children() + |> Seq.map (fun item -> + match item.Type with + | JTokenType.String -> item.Value() + | JTokenType.Null -> null + | itemType -> raise (invalidParams $"Expected a string or null, but found {itemType}.")) + |> Seq.toArray + |> box + | tokenType -> raise (invalidParams $"Expected an array or null, but found {tokenType}.") + + override _.WriteJson(_, _, _) = raise (NotSupportedException()) + +[] +type private StrictNullableInt32JsonConverter() = + inherit JsonConverter() + + override _.CanConvert(objectType) = objectType = typeof> + override _.CanWrite = false + + override _.ReadJson(reader, _, _, _) = + match reader.TokenType with + | JsonToken.Null -> null + | JsonToken.Integer -> + try + box (Nullable(Convert.ToInt32 reader.Value)) + with :? OverflowException -> + raise (invalidParams "The integer is outside the supported range.") + | token -> raise (invalidParams $"Expected an integer or null, but found {token}.") + + override _.WriteJson(_, _, _) = raise (NotSupportedException()) + +let private requireRequest methodName request = + if obj.ReferenceEquals(request, null) then + raise (invalidParams $"'{methodName}' requires a request object.") + +let private requireText fieldName (value: string) = + if isNull value then + raise (invalidParams $"'{fieldName}' is required.") + +let private requireDirectivePath fieldName (value: string) = + requireText fieldName value + + if value.IndexOf('"') >= 0 || value.IndexOf('\r') >= 0 || value.IndexOf('\n') >= 0 then + raise (invalidParams $"'{fieldName}' contains characters that F# Interactive directives cannot represent.") let private toExecutionResult (outcome: Choice) @@ -204,10 +278,10 @@ type internal FsiRpcTarget executionQueue: ExecutionQueue ) = - let interruptLock = obj () - let mutable currentCancellation: CancellationTokenSource = null + let interactionLock = obj () + let mutable currentInteraction = ValueNone let mutable initialized = false - let values = ResizeArray() + let mutable currentValues: ResizeArray = null /// Formatted by the session's own printer, so that the text matches the console's and obeys the /// session's print settings. Formatting runs user code — a ToString override, a lazy @@ -253,8 +327,8 @@ type internal FsiRpcTarget with e -> Choice2Of2 e, [||] - /// Flush everything the interaction printed before answering, so that a host which shows - /// standard output and RPC results side by side sees them in the order they were produced. + /// Flush buffered console writers before answering. The output and RPC streams are independent, + /// so their reader-visible ordering is deliberately unspecified. let flushConsole () = try outWriter.Flush() @@ -264,11 +338,12 @@ type internal FsiRpcTarget let runInteraction (code: string) (scriptPath: string) = let cancellation = new CancellationTokenSource() + let values = ResizeArray() - lock interruptLock (fun () -> currentCancellation <- cancellation) + lock interactionLock (fun () -> currentInteraction <- ValueSome(cancellation, false)) try - values.Clear() + lock interactionLock (fun () -> currentValues <- values) let outcome, diagnostics = evaluateOnEventLoop (fun () -> fsiSession.EvalInteractionNonThrowing(code, scriptPath, cancellation.Token)) @@ -276,7 +351,10 @@ type internal FsiRpcTarget flushConsole () toExecutionResult outcome diagnostics (values.ToArray()) cancellation.IsCancellationRequested finally - lock interruptLock (fun () -> currentCancellation <- null) + lock interactionLock (fun () -> + currentValues <- null + currentInteraction <- ValueNone) + cancellation.Dispose() /// @@ -332,6 +410,11 @@ type internal FsiRpcTarget member _.Execute(request: ExecuteRequest) : Task = requireInitialized () + requireRequest Methods.Execute request + requireText "code" request.code + + if request.startLine.HasValue && request.startLine.Value < 1 then + raise (invalidParams "'startLine' must be at least one.") let text = positionInteraction request.code request.sourcePath request.startLine @@ -345,6 +428,11 @@ type internal FsiRpcTarget member _.ExecuteFile(request: ExecuteFileRequest) : Task = requireInitialized () + requireRequest Methods.ExecuteFile request + requireText "path" request.path + + if String.IsNullOrWhiteSpace request.path then + raise (invalidParams "'path' must not be empty.") // Routed through #load so that the file joins the session the same way it would from a // script, rather than being replayed as anonymous text. @@ -354,6 +442,17 @@ type internal FsiRpcTarget /// directives a script would use. member _.SetPaths(request: SetPathsRequest) : Task = requireInitialized () + requireRequest Methods.SetPaths request + requireText "workingDirectory" request.workingDirectory + + match request.includePaths with + | null -> raise (invalidParams "'includePaths' is required.") + | paths -> + paths + |> Array.iteri (fun index path -> requireDirectivePath $"includePaths[{index}]" path) + + if not (String.IsNullOrWhiteSpace request.workingDirectory) then + requireDirectivePath "workingDirectory" request.workingDirectory if not (String.IsNullOrWhiteSpace request.workingDirectory) @@ -361,34 +460,36 @@ type internal FsiRpcTarget then raise (LocalRpcException($"The working directory '{request.workingDirectory}' does not exist.", ErrorCode = -32002)) - // The process directory moves on the queue, alongside the directive that moves the - // compiler's: doing it as the request arrives would move it under an earlier interaction - // that is still running. queueInteraction (fun () -> let directives = ResizeArray() if not (String.IsNullOrWhiteSpace request.workingDirectory) then - // Two different notions of "current directory" have to agree here. The directive - // moves the compiler's, which is what relative #load and #r resolve against; the - // process one is what the running script sees when it opens a file by relative path. - try - Directory.SetCurrentDirectory request.workingDirectory - with _ -> - () - directives.Add $"#silentCd {verbatimString request.workingDirectory}" - match request.includePaths with - | null -> () - | paths -> - for path in paths do - if not (String.IsNullOrWhiteSpace path) then - directives.Add $"#I {verbatimString path}" + for path in request.includePaths do + if not (String.IsNullOrWhiteSpace path) then + directives.Add $"#I {verbatimString path}" if directives.Count = 0 then toExecutionResult (Choice1Of2 None) [||] [||] false else - runInteraction (String.Join("\n", directives)) DefaultInteractionName) + let previousDirectory = Directory.GetCurrentDirectory() + let result = runInteraction (String.Join("\n", directives)) DefaultInteractionName + + if result.success && not (String.IsNullOrWhiteSpace request.workingDirectory) then + try + Directory.SetCurrentDirectory request.workingDirectory + + { result with + workingDirectory = Directory.GetCurrentDirectory() + } + with e -> + runInteraction $"#silentCd {verbatimString previousDirectory}" DefaultInteractionName + |> ignore + + toExecutionResult (Choice2Of2 e) [||] [||] false + else + result) /// Interrupt the interaction in flight. /// @@ -398,24 +499,24 @@ type internal FsiRpcTarget member _.Interrupt() : InterruptResult = requireInitialized () - let cancellation = lock interruptLock (fun () -> currentCancellation) + lock interactionLock (fun () -> + match currentInteraction with + | ValueNone + | ValueSome(_, true) -> { interrupted = false } + | ValueSome(cancellation, false) -> + currentInteraction <- ValueSome(cancellation, true) - match cancellation with - | null -> { interrupted = false } - | cts -> - // Cancel the token the interaction is running under, then ask the session to interrupt - // the evaluation thread, which is what stops code already inside a long-running call. - try - cts.Cancel() - with _ -> - () + try + cancellation.Cancel() + with _ -> + () - try - fsiSession.Interrupt() - with _ -> - () + try + fsiSession.Interrupt() + with _ -> + () - { interrupted = true } + { interrupted = true }) member _.Shutdown() : unit = requireInitialized () @@ -457,8 +558,12 @@ let private runServer let target = FsiRpcTarget(fsiSession, fsiConfig, outWriter, errorWriter, shutdownRequested, executionQueue) - use rpc = - new JsonRpc(new HeaderDelimitedMessageHandler(pipe, new JsonMessageFormatter())) + let formatter = new JsonMessageFormatter() + formatter.JsonSerializer.Converters.Add(new StrictStringJsonConverter()) + formatter.JsonSerializer.Converters.Add(new StrictStringArrayJsonConverter()) + formatter.JsonSerializer.Converters.Add(new StrictNullableInt32JsonConverter()) + + use rpc = new JsonRpc(new HeaderDelimitedMessageHandler(pipe, formatter)) // Registered one by one rather than by reflecting over the target, so that exactly the // protocol's methods are callable. A request carrying its parameters as one object — the shape @@ -482,8 +587,12 @@ let private runServer rpc.StartListening() - // Either the host goes away or it asks to stop. Both end the session. - Task.WaitAny(rpc.Completion, shutdownRequested.Task) |> ignore + // Either the host goes away or it asks to stop. A faulted transport is not an orderly + // disconnect: observe it so the process reports failure instead of a successful shutdown. + let completed = Task.WaitAny(rpc.Completion, shutdownRequested.Task) + + if completed = 0 then + rpc.Completion.GetAwaiter().GetResult() if shutdownRequested.Task.IsCompleted then // Give the reply to the shutdown request its moment to reach the host before the process @@ -508,13 +617,11 @@ let internal startOnBackgroundThread (fun () -> try runServer fsiSession fsiConfig pipeName clientProcessId eventLoopStarted outWriter errorWriter + exit 0 with e -> errorWriter.WriteLine $"F# Interactive server terminated: {e}" errorWriter.Flush() - - // The session exists only to serve this host. Once the connection is gone there is - // nothing left to do, and lingering would leak a process. - exit 0), + exit 1), Name = "FSI-JsonRpc-Dispatch", IsBackground = true ) diff --git a/tests/FSharp.Compiler.ComponentTests/Scripting/Interactive.fs b/tests/FSharp.Compiler.ComponentTests/Scripting/Interactive.fs index da5ef2a3e2b..fe797fe78ce 100644 --- a/tests/FSharp.Compiler.ComponentTests/Scripting/Interactive.fs +++ b/tests/FSharp.Compiler.ComponentTests/Scripting/Interactive.fs @@ -37,6 +37,15 @@ module ``Interactive tests`` = (Warning 2304, Line 1, Col 3, Line 1, Col 13, "Functions with [] are not invoked in FSI. 'myFunc' was not invoked. Execute 'myFunc ' in order to invoke 'myFunc' with the appropriate string array of command line arguments.") ] + [] + let ``RunScript preserves quit directives inside multiline strings`` () = + let _, output, _ = + global.FSharp.Test.CompilerAssert.RunScriptWithOptionsAndReturnResult + [||] + "let text = \"\"\"\n#quit\n\"\"\"\nprintf \"%s\" text" + + Assert.Contains("#quit", output) + [] [] [] diff --git a/tests/FSharp.Compiler.ComponentTests/resources/tests/InteractiveSession/Misc/Array2D01.fs b/tests/FSharp.Compiler.ComponentTests/resources/tests/InteractiveSession/Misc/Array2D01.fs index 9a64aed8a0a..d37877161ad 100644 --- a/tests/FSharp.Compiler.ComponentTests/resources/tests/InteractiveSession/Misc/Array2D01.fs +++ b/tests/FSharp.Compiler.ComponentTests/resources/tests/InteractiveSession/Misc/Array2D01.fs @@ -29,6 +29,4 @@ type Array2D1<'T> = { };; -Array2D1 (array2D [[1];[2]]) |> ignore;; - -#q;; \ No newline at end of file +Array2D1 (array2D [[1];[2]]) |> ignore;; \ No newline at end of file diff --git a/tests/FSharp.Compiler.ComponentTests/resources/tests/InteractiveSession/Misc/NativeIntSuffix01.fs b/tests/FSharp.Compiler.ComponentTests/resources/tests/InteractiveSession/Misc/NativeIntSuffix01.fs index a20926bacfe..d002a1675cc 100644 --- a/tests/FSharp.Compiler.ComponentTests/resources/tests/InteractiveSession/Misc/NativeIntSuffix01.fs +++ b/tests/FSharp.Compiler.ComponentTests/resources/tests/InteractiveSession/Misc/NativeIntSuffix01.fs @@ -4,5 +4,3 @@ //val it: nativeint = 2n nativeint 2;; -#q;; - diff --git a/tests/FSharp.Compiler.ComponentTests/resources/tests/InteractiveSession/Misc/PipingWithDirectives.fs b/tests/FSharp.Compiler.ComponentTests/resources/tests/InteractiveSession/Misc/PipingWithDirectives.fs index 0f55da3f2a6..7eec2170a09 100644 --- a/tests/FSharp.Compiler.ComponentTests/resources/tests/InteractiveSession/Misc/PipingWithDirectives.fs +++ b/tests/FSharp.Compiler.ComponentTests/resources/tests/InteractiveSession/Misc/PipingWithDirectives.fs @@ -22,5 +22,3 @@ let test3 x = match x with | x -> Some(x) | _ -> None - -#quit diff --git a/tests/FSharp.Compiler.ComponentTests/resources/tests/InteractiveSession/Misc/UNativeIntSuffix01.fs b/tests/FSharp.Compiler.ComponentTests/resources/tests/InteractiveSession/Misc/UNativeIntSuffix01.fs index d5e9e10a97d..a417903b8ba 100644 --- a/tests/FSharp.Compiler.ComponentTests/resources/tests/InteractiveSession/Misc/UNativeIntSuffix01.fs +++ b/tests/FSharp.Compiler.ComponentTests/resources/tests/InteractiveSession/Misc/UNativeIntSuffix01.fs @@ -4,5 +4,3 @@ //val it: unativeint = 2un unativeint 2;; -#q;; - diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs index 366ae5d096d..928db446ff6 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs @@ -93,6 +93,41 @@ let ``only the protocol's own methods are reachable`` () = let result = session.Execute "1 + 1" Assert.True(succeeded result, describe session result)) +[] +let ``request DTOs reject missing required fields as invalid params`` () = + withInitializedSession (fun session -> + let malformedRequests = + [ Methods.Execute, box {| sourcePath = null; startLine = Nullable() |} + Methods.ExecuteFile, obj () + Methods.SetPaths, box {| workingDirectory = "" |} + Methods.Execute, box {| code = 42; sourcePath = null; startLine = Nullable() |} + Methods.ExecuteFile, box {| path = 42 |} + Methods.Execute, box {| code = "1"; sourcePath = null; startLine = "1" |} + Methods.SetPaths, box {| includePaths = 42; workingDirectory = "" |} + Methods.SetPaths, box {| includePaths = Array.empty; workingDirectory = 42 |} ] + + for methodName, parameters in malformedRequests do + Assert.Equal(Some -32602, session.RequestExpectingError(methodName, parameters))) + +[] +let ``initialize fails when an explicit owner process cannot be watched`` () = + withInitializedSession (fun session -> + let error = + session.RequestExpectingError( + Methods.Initialize, + { clientProcessId = Int32.MaxValue } + ) + + Assert.Equal(Some -32602, error)) + +[] +let ``fails when the command-line owner process cannot be watched`` () = + let error = + Assert.ThrowsAny(fun () -> + new FsiServerHarness(clientProcessId = Int32.MaxValue) |> ignore) + + Assert.Contains("exited with code 1", error.Message) + //------------------------------------------------------------------------- // The command line //------------------------------------------------------------------------- @@ -474,6 +509,37 @@ let ``setPaths changes the working directory`` () = with _ -> ()) +[] +let ``setPaths rejects an unrepresentable path without changing the working directory`` () = + if RuntimeInformation.IsOSPlatform OSPlatform.Windows then + () + else + withInitializedSession (fun session -> + let directory = temporaryPath "\"quoted" + Directory.CreateDirectory directory |> ignore + + try + let before = session.Execute "1" + + let error = + session.RequestExpectingError( + Methods.SetPaths, + { + includePaths = [| directory |] + workingDirectory = directory + } + ) + + Assert.Equal(Some -32602, error) + + let after = session.Execute "2" + Assert.Equal(before.workingDirectory, after.workingDirectory) + finally + try + Directory.Delete(directory, true) + with _ -> + ()) +[] [] let ``setPaths rejects a missing working directory`` () = withInitializedSession (fun session -> @@ -551,32 +617,37 @@ let ``reports the working directory after every interaction`` () = [] let ``interrupts a running interaction`` () = withInitializedSession (fun session -> - // Warm the session up first, so that the interrupt below meets a session that is genuinely - // executing the loop rather than still starting up. - Assert.True(succeeded (session.Execute "1")) - let running = session.BeginRequest( Methods.Execute, - FsiServerHarness.ExecuteParams "while true do System.Threading.Thread.Sleep 10" + FsiServerHarness.ExecuteParams + """ +printfn "interrupt target started" +while true do System.Threading.Thread.Sleep 10 +""" ) - Thread.Sleep 3000 + Assert.True(session.WaitForOutput "interrupt target started") - // Interactions queue behind one another, but an interrupt is served as it arrives — which - // is the whole point, since one that waited its turn would never stop anything. - let interrupted = - session.Request(Methods.Interrupt, TimeSpan.FromSeconds 30.0) + let next = + session.BeginRequest(Methods.Execute, FsiServerHarness.ExecuteParams "40 + 2") - Assert.True interrupted.interrupted + let interrupts = + Array.init 8 (fun _ -> session.BeginRequest Methods.Interrupt) - // The interrupted interaction must come back rather than hang forever. - try - let result = session.EndRequest(running, TimeSpan.FromSeconds 60.0) - Assert.False(succeeded result, describe session result) - with _ -> - // Reported as a failed call rather than a failed interaction; either is acceptable. - ()) + let interruptResults = + interrupts + |> Array.map (fun request -> session.EndRequest(request, TimeSpan.FromSeconds 30.0)) + + Assert.Equal(1, interruptResults |> Array.filter _.interrupted |> Array.length) + + let interrupted = session.EndRequest(running, TimeSpan.FromSeconds 60.0) + Assert.True(interrupted.cancelled, describe session interrupted) + Assert.False(interrupted.success, describe session interrupted) + + let subsequent = session.EndRequest(next, TimeSpan.FromSeconds 60.0) + Assert.True(subsequent.success, describe session subsequent) + Assert.True(session.WaitForOutput "val it: int = 42", describe session subsequent)) [] let ``interrupt is harmless when nothing is running`` () = @@ -597,12 +668,22 @@ let ``shutdown ends the session`` () = [] let ``the session exits when the host disconnects`` () = - let session = new FsiServerHarness() + use session = new FsiServerHarness() + session.Initialize() |> ignore + + session.CloseControlChannel() + Assert.True(session.WaitForExit 30_000, "the session did not exit after the control channel closed") + Assert.Equal(0, session.ExitCode) + +[] +let ``a faulted control channel exits with failure`` () = + use session = new FsiServerHarness() session.Initialize() |> ignore - // Closing the control channel is what happens when the editor process dies. A session that - // survived it would leak a process for every crash. - (session :> IDisposable).Dispose() + session.CorruptControlChannel() + Assert.True(session.WaitForExit 30_000, "the session did not exit after the control channel faulted") + Assert.NotEqual(0, session.ExitCode) + Assert.Contains("F# Interactive server terminated:", session.StandardError) [] let ``the session exits when its host process exits`` () = diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs index 4b7637602e7..a8644c3fdd7 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs @@ -200,10 +200,24 @@ type FsiServerHarness member _.HasExited = session.HasExited + member _.ExitCode = session.ExitCode + member _.ProcessId = session.Id member _.WaitForExit(milliseconds: int) = session.WaitForExit milliseconds + /// Close only the JSON-RPC connection, leaving the child process to observe EOF and exit. + member _.CloseControlChannel() = + rpc.Dispose() + pipe.Dispose() + + /// Send an invalid header and close the channel, forcing an unrecoverable transport fault. + member _.CorruptControlChannel() = + let bytes = Encoding.ASCII.GetBytes "Content-Length: invalid\r\n\r\n" + pipe.Write(bytes, 0, bytes.Length) + pipe.Flush() + pipe.Dispose() + /// Wait until the session's own output contains the given text, which is how a test observes /// what a script printed rather than what the protocol returned. member this.WaitForOutput(text: string, ?timeout: TimeSpan) = @@ -243,8 +257,8 @@ type FsiServerHarness // the type this classifies on is found by descending through causes, not just one level. let rec classify (e: exn) = match e with - | :? RemoteMethodNotFoundException -> Some -32601 | :? RemoteInvocationException as remote -> Some remote.ErrorCode + | :? RemoteMethodNotFoundException as remote -> Some(int remote.ErrorCode) | _ -> match e.InnerException with | null -> None diff --git a/tests/FSharp.Compiler.Service.Tests/FsiTests.fs b/tests/FSharp.Compiler.Service.Tests/FsiTests.fs index 2cdc4279330..0bf19ed0b44 100644 --- a/tests/FSharp.Compiler.Service.Tests/FsiTests.fs +++ b/tests/FSharp.Compiler.Service.Tests/FsiTests.fs @@ -29,6 +29,19 @@ module FsiTests = let values = fsiSession.GetBoundValues() Assert.shouldBeEmpty values + [] + [] + [] + let ``EvalInteractionNonThrowing keeps the last produced value`` source = + use fsiSession = createFsiSession false + let result, diagnostics = fsiSession.EvalInteractionNonThrowing source + + Assert.shouldBeEmpty diagnostics + + match result with + | Choice1Of2(Some value) -> Assert.shouldBe (box 42) value.ReflectionValue + | _ -> failwith $"Expected the value 42, got {result}" + [] let ``Bound value has correct name`` () = use fsiSession = createFsiSession false From 737f671dba55cf6b2edbe90aaa5c0de2456be0b3 Mon Sep 17 00:00:00 2001 From: perf-bundle Date: Mon, 21 Sep 2026 16:46:46 +0200 Subject: [PATCH 29/30] Compact FSI JSON-RPC remediation Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- src/fsi/fsiserver.fs | 140 ++++++------------ .../FsiJsonRpcServerTests.fs | 92 ++++-------- .../FsiServerHarness.fs | 16 +- .../FSharp.Compiler.Service.Tests/FsiTests.fs | 14 +- 4 files changed, 80 insertions(+), 182 deletions(-) diff --git a/src/fsi/fsiserver.fs b/src/fsi/fsiserver.fs index 40a03503ecf..a228eeb001f 100644 --- a/src/fsi/fsiserver.fs +++ b/src/fsi/fsiserver.fs @@ -38,7 +38,6 @@ open System.Threading open System.Threading.Tasks open Newtonsoft.Json -open Newtonsoft.Json.Linq open StreamJsonRpc open FSharp.Compiler.Diagnostics @@ -93,77 +92,41 @@ let private invalidParams message = LocalRpcException(message, ErrorCode = -32602) [] -type private StrictStringJsonConverter() = +type private StrictRequestJsonConverter() = inherit JsonConverter() - override _.CanConvert(objectType) = objectType = typeof - override _.CanWrite = false - - override _.ReadJson(reader, _, _, _) = - match reader.TokenType with - | JsonToken.String -> reader.Value - | JsonToken.Null -> null - | token -> raise (invalidParams $"Expected a string or null, but found {token}.") - - override _.WriteJson(_, _, _) = raise (NotSupportedException()) + override _.CanConvert(objectType) = + objectType = typeof + || objectType = typeof + || objectType = typeof> -[] -type private StrictStringArrayJsonConverter() = - inherit JsonConverter() - - override _.CanConvert(objectType) = objectType = typeof override _.CanWrite = false - override _.ReadJson(reader, _, _, _) = - let token = JToken.ReadFrom reader - - match token.Type with - | JTokenType.Null -> null - | JTokenType.Array -> - token.Children() - |> Seq.map (fun item -> - match item.Type with - | JTokenType.String -> item.Value() - | JTokenType.Null -> null - | itemType -> raise (invalidParams $"Expected a string or null, but found {itemType}.")) - |> Seq.toArray - |> box - | tokenType -> raise (invalidParams $"Expected an array or null, but found {tokenType}.") - override _.WriteJson(_, _, _) = raise (NotSupportedException()) -[] -type private StrictNullableInt32JsonConverter() = - inherit JsonConverter() - - override _.CanConvert(objectType) = objectType = typeof> - override _.CanWrite = false - - override _.ReadJson(reader, _, _, _) = + override this.ReadJson(reader, objectType, _, _) = match reader.TokenType with | JsonToken.Null -> null - | JsonToken.Integer -> - try - box (Nullable(Convert.ToInt32 reader.Value)) - with :? OverflowException -> - raise (invalidParams "The integer is outside the supported range.") - | token -> raise (invalidParams $"Expected an integer or null, but found {token}.") - - override _.WriteJson(_, _, _) = raise (NotSupportedException()) + | JsonToken.String when objectType = typeof -> reader.Value + | JsonToken.Integer when objectType = typeof> -> + match reader.Value with + | :? int64 as value when value >= int64 Int32.MinValue && value <= int64 Int32.MaxValue -> box (Nullable(int value)) + | _ -> raise (invalidParams "The integer is outside the supported range.") + | JsonToken.StartArray when objectType = typeof -> + let items = ResizeArray() -let private requireRequest methodName request = - if obj.ReferenceEquals(request, null) then - raise (invalidParams $"'{methodName}' requires a request object.") + while reader.Read() && reader.TokenType <> JsonToken.EndArray do + items.Add(this.ReadJson(reader, typeof, null, null) :?> string) -let private requireText fieldName (value: string) = - if isNull value then - raise (invalidParams $"'{fieldName}' is required.") + box (items.ToArray()) + | token -> raise (invalidParams $"A JSON {token} cannot be read as {objectType.Name}.") -let private requireDirectivePath fieldName (value: string) = - requireText fieldName value +let inline private require condition message = + if not condition then + raise (invalidParams message) - if value.IndexOf('"') >= 0 || value.IndexOf('\r') >= 0 || value.IndexOf('\n') >= 0 then - raise (invalidParams $"'{fieldName}' contains characters that F# Interactive directives cannot represent.") +let private isDirectivePath (value: string) = + not (isNull value) && value.IndexOfAny [| '"'; '\r'; '\n' |] < 0 let private toExecutionResult (outcome: Choice) @@ -278,10 +241,10 @@ type internal FsiRpcTarget executionQueue: ExecutionQueue ) = - let interactionLock = obj () - let mutable currentInteraction = ValueNone + let interruptLock = obj () + let mutable currentCancellation: CancellationTokenSource = null let mutable initialized = false - let mutable currentValues: ResizeArray = null + let values = ResizeArray() /// Formatted by the session's own printer, so that the text matches the console's and obeys the /// session's print settings. Formatting runs user code — a ToString override, a lazy @@ -338,12 +301,11 @@ type internal FsiRpcTarget let runInteraction (code: string) (scriptPath: string) = let cancellation = new CancellationTokenSource() - let values = ResizeArray() - lock interactionLock (fun () -> currentInteraction <- ValueSome(cancellation, false)) + lock interruptLock (fun () -> currentCancellation <- cancellation) try - lock interactionLock (fun () -> currentValues <- values) + values.Clear() let outcome, diagnostics = evaluateOnEventLoop (fun () -> fsiSession.EvalInteractionNonThrowing(code, scriptPath, cancellation.Token)) @@ -351,10 +313,7 @@ type internal FsiRpcTarget flushConsole () toExecutionResult outcome diagnostics (values.ToArray()) cancellation.IsCancellationRequested finally - lock interactionLock (fun () -> - currentValues <- null - currentInteraction <- ValueNone) - + lock interruptLock (fun () -> currentCancellation <- null) cancellation.Dispose() /// @@ -410,11 +369,8 @@ type internal FsiRpcTarget member _.Execute(request: ExecuteRequest) : Task = requireInitialized () - requireRequest Methods.Execute request - requireText "code" request.code - - if request.startLine.HasValue && request.startLine.Value < 1 then - raise (invalidParams "'startLine' must be at least one.") + require (not (isNull request.code)) "'code' is required." + require (not request.startLine.HasValue || request.startLine.Value >= 1) "'startLine' must be at least one." let text = positionInteraction request.code request.sourcePath request.startLine @@ -428,11 +384,7 @@ type internal FsiRpcTarget member _.ExecuteFile(request: ExecuteFileRequest) : Task = requireInitialized () - requireRequest Methods.ExecuteFile request - requireText "path" request.path - - if String.IsNullOrWhiteSpace request.path then - raise (invalidParams "'path' must not be empty.") + require (not (String.IsNullOrWhiteSpace request.path)) "'path' must not be empty." // Routed through #load so that the file joins the session the same way it would from a // script, rather than being replayed as anonymous text. @@ -442,17 +394,12 @@ type internal FsiRpcTarget /// directives a script would use. member _.SetPaths(request: SetPathsRequest) : Task = requireInitialized () - requireRequest Methods.SetPaths request - requireText "workingDirectory" request.workingDirectory - - match request.includePaths with - | null -> raise (invalidParams "'includePaths' is required.") - | paths -> - paths - |> Array.iteri (fun index path -> requireDirectivePath $"includePaths[{index}]" path) + require (isDirectivePath request.workingDirectory) "'workingDirectory' must be a path without quotes or newlines." + require (not (isNull request.includePaths)) "'includePaths' is required." - if not (String.IsNullOrWhiteSpace request.workingDirectory) then - requireDirectivePath "workingDirectory" request.workingDirectory + request.includePaths + |> Array.iteri (fun index path -> + require (isDirectivePath path) $"'includePaths[{index}]' must be a path without quotes or newlines.") if not (String.IsNullOrWhiteSpace request.workingDirectory) @@ -499,12 +446,11 @@ type internal FsiRpcTarget member _.Interrupt() : InterruptResult = requireInitialized () - lock interactionLock (fun () -> - match currentInteraction with - | ValueNone - | ValueSome(_, true) -> { interrupted = false } - | ValueSome(cancellation, false) -> - currentInteraction <- ValueSome(cancellation, true) + lock interruptLock (fun () -> + match currentCancellation with + | null -> { interrupted = false } + | cancellation -> + currentCancellation <- null try cancellation.Cancel() @@ -559,9 +505,7 @@ let private runServer FsiRpcTarget(fsiSession, fsiConfig, outWriter, errorWriter, shutdownRequested, executionQueue) let formatter = new JsonMessageFormatter() - formatter.JsonSerializer.Converters.Add(new StrictStringJsonConverter()) - formatter.JsonSerializer.Converters.Add(new StrictStringArrayJsonConverter()) - formatter.JsonSerializer.Converters.Add(new StrictNullableInt32JsonConverter()) + formatter.JsonSerializer.Converters.Add(new StrictRequestJsonConverter()) use rpc = new JsonRpc(new HeaderDelimitedMessageHandler(pipe, formatter)) diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs index 928db446ff6..47476113f6f 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiJsonRpcServerTests.fs @@ -104,22 +104,13 @@ let ``request DTOs reject missing required fields as invalid params`` () = Methods.ExecuteFile, box {| path = 42 |} Methods.Execute, box {| code = "1"; sourcePath = null; startLine = "1" |} Methods.SetPaths, box {| includePaths = 42; workingDirectory = "" |} - Methods.SetPaths, box {| includePaths = Array.empty; workingDirectory = 42 |} ] + Methods.SetPaths, box {| includePaths = Array.empty; workingDirectory = 42 |} + Methods.Execute, box {| code = "1"; sourcePath = null; startLine = 3000000000L |} + Methods.SetPaths, box {| includePaths = [| box 42 |]; workingDirectory = "wd" |} ] for methodName, parameters in malformedRequests do Assert.Equal(Some -32602, session.RequestExpectingError(methodName, parameters))) -[] -let ``initialize fails when an explicit owner process cannot be watched`` () = - withInitializedSession (fun session -> - let error = - session.RequestExpectingError( - Methods.Initialize, - { clientProcessId = Int32.MaxValue } - ) - - Assert.Equal(Some -32602, error)) - [] let ``fails when the command-line owner process cannot be watched`` () = let error = @@ -176,19 +167,14 @@ let ``evaluates an interaction and prints its result`` () = // The value is reported the way a console session reports it: printed to standard output. Assert.True(session.WaitForOutput "val it: int = 2", describe session result)) -[] -let ``returns evaluated values`` () = - withInitializedSession (fun session -> - let result = session.Execute "let answer = 42" - Assert.True(succeeded result, describe session result) - Assert.Contains(result.values, fun value -> value.name = "answer" && value.value = "42")) - -[] -let ``returns the value of an expression as it`` () = +[] +[] +[] +let ``returns evaluated values`` code name = withInitializedSession (fun session -> - let result = session.Execute "6 * 7" + let result = session.Execute code Assert.True(succeeded result, describe session result) - Assert.Contains(result.values, fun value -> value.name = "it" && value.value = "42")) + Assert.Contains(result.values, fun value -> value.name = name && value.value = "42")) [] let ``does not report the helper bindings the compiler introduces`` () = @@ -511,35 +497,22 @@ let ``setPaths changes the working directory`` () = [] let ``setPaths rejects an unrepresentable path without changing the working directory`` () = - if RuntimeInformation.IsOSPlatform OSPlatform.Windows then - () - else - withInitializedSession (fun session -> - let directory = temporaryPath "\"quoted" - Directory.CreateDirectory directory |> ignore - - try - let before = session.Execute "1" + withInitializedSession (fun session -> + let before = session.Execute "1" + let path = temporaryPath "\"quoted" - let error = - session.RequestExpectingError( - Methods.SetPaths, - { - includePaths = [| directory |] - workingDirectory = directory - } - ) + let error = + session.RequestExpectingError( + Methods.SetPaths, + { + includePaths = [| path |] + workingDirectory = path + } + ) - Assert.Equal(Some -32602, error) + Assert.Equal(Some -32602, error) + Assert.Equal(before.workingDirectory, (session.Execute "2").workingDirectory)) - let after = session.Execute "2" - Assert.Equal(before.workingDirectory, after.workingDirectory) - finally - try - Directory.Delete(directory, true) - with _ -> - ()) -[] [] let ``setPaths rejects a missing working directory`` () = withInitializedSession (fun session -> @@ -666,24 +639,19 @@ let ``shutdown ends the session`` () = Assert.True(session.WaitForExit 30_000, "the session did not exit after shutdown")) -[] -let ``the session exits when the host disconnects`` () = +[] +[] +[] +let ``the session exits when the control channel closes`` corrupt = use session = new FsiServerHarness() session.Initialize() |> ignore - session.CloseControlChannel() + session.CloseControlChannel corrupt Assert.True(session.WaitForExit 30_000, "the session did not exit after the control channel closed") - Assert.Equal(0, session.ExitCode) - -[] -let ``a faulted control channel exits with failure`` () = - use session = new FsiServerHarness() - session.Initialize() |> ignore + Assert.Equal(corrupt, session.ExitCode <> 0) - session.CorruptControlChannel() - Assert.True(session.WaitForExit 30_000, "the session did not exit after the control channel faulted") - Assert.NotEqual(0, session.ExitCode) - Assert.Contains("F# Interactive server terminated:", session.StandardError) + if corrupt then + Assert.Contains("F# Interactive server terminated:", session.StandardError) [] let ``the session exits when its host process exits`` () = diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs index a8644c3fdd7..a7bdc61a423 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs @@ -206,16 +206,14 @@ type FsiServerHarness member _.WaitForExit(milliseconds: int) = session.WaitForExit milliseconds - /// Close only the JSON-RPC connection, leaving the child process to observe EOF and exit. - member _.CloseControlChannel() = - rpc.Dispose() - pipe.Dispose() + member _.CloseControlChannel(corrupt: bool) = + if corrupt then + let bytes = Encoding.ASCII.GetBytes "Content-Length: invalid\r\n\r\n" + pipe.Write(bytes, 0, bytes.Length) + pipe.Flush() + else + rpc.Dispose() - /// Send an invalid header and close the channel, forcing an unrecoverable transport fault. - member _.CorruptControlChannel() = - let bytes = Encoding.ASCII.GetBytes "Content-Length: invalid\r\n\r\n" - pipe.Write(bytes, 0, bytes.Length) - pipe.Flush() pipe.Dispose() /// Wait until the session's own output contains the given text, which is how a test observes diff --git a/tests/FSharp.Compiler.Service.Tests/FsiTests.fs b/tests/FSharp.Compiler.Service.Tests/FsiTests.fs index 0bf19ed0b44..897573f124e 100644 --- a/tests/FSharp.Compiler.Service.Tests/FsiTests.fs +++ b/tests/FSharp.Compiler.Service.Tests/FsiTests.fs @@ -29,19 +29,6 @@ module FsiTests = let values = fsiSession.GetBoundValues() Assert.shouldBeEmpty values - [] - [] - [] - let ``EvalInteractionNonThrowing keeps the last produced value`` source = - use fsiSession = createFsiSession false - let result, diagnostics = fsiSession.EvalInteractionNonThrowing source - - Assert.shouldBeEmpty diagnostics - - match result with - | Choice1Of2(Some value) -> Assert.shouldBe (box 42) value.ReflectionValue - | _ -> failwith $"Expected the value 42, got {result}" - [] let ``Bound value has correct name`` () = use fsiSession = createFsiSession false @@ -718,6 +705,7 @@ module FsiTests = Assert.shouldBe (box "hoha") ((byName "r2").Value.ReflectionValue) [] + [] [] [] [] From 68e6f0ada1e4b5bd294d67cee9efe2a955ea0082 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 21 Sep 2026 18:32:58 +0200 Subject: [PATCH 30/30] Keep the last value EvalInteraction produced, and let the harness lose the pipe race The remediation passed the two values to Option.orElse the wrong way round, so a later value was replaced by an earlier one: `41;; 42;;` gave 41. The interaction's own status already carries the merged value. Closing a corrupted control channel can find the pipe disposed already, when the session hangs up and the client's JsonRpc disposes it first. Co-Authored-By: Claude Sonnet 5 --- src/Compiler/Interactive/fsi.fs | 2 +- .../FsiServerHarness.fs | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Compiler/Interactive/fsi.fs b/src/Compiler/Interactive/fsi.fs index c96f008a233..67a4560c5de 100644 --- a/src/Compiler/Interactive/fsi.fs +++ b/src/Compiler/Interactive/fsi.fs @@ -4458,7 +4458,7 @@ type FsiInteractionProcessor istate, CtrlC else setCurrState istate - run istate (Option.orElse value lastValue) + run istate value | _ -> istate, status run currState None |> commitResult diff --git a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs index a7bdc61a423..5763454c980 100644 --- a/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs +++ b/tests/FSharp.Compiler.Interactive.Server.Tests/FsiServerHarness.fs @@ -209,8 +209,15 @@ type FsiServerHarness member _.CloseControlChannel(corrupt: bool) = if corrupt then let bytes = Encoding.ASCII.GetBytes "Content-Length: invalid\r\n\r\n" - pipe.Write(bytes, 0, bytes.Length) - pipe.Flush() + + // The session hangs up on the malformed header, and the client's own JsonRpc, listening on + // the same pipe, disposes it in turn, possibly before this write or flush is done. + try + pipe.Write(bytes, 0, bytes.Length) + pipe.Flush() + with + | :? ObjectDisposedException + | :? IOException -> () else rpc.Dispose()