diff --git a/CHANGELOG.md b/CHANGELOG.md index 328e654..a1c72f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -56,6 +56,12 @@ All notable changes to this project will be documented in this file. ### Added +- Structured Rune compilation errors and warnings through a read-only + `check_source` protocol operation and compilation-failure error details. +- Automatic VS Code inline/Problems diagnostics for open image sources and + actor-specific behavior documents, including unsaved edits, correct UTF-16 + locations, and stale-response/session isolation. + - Source file creation, rename/move with confirmed actor path updates and history preservation, and deletion guarded against running actor users. - Server-owned actor-specific temporary behaviors, safe-boundary compile-on-save, diff --git a/README.md b/README.md index a8e6a0c..677ba35 100644 --- a/README.md +++ b/README.md @@ -211,8 +211,11 @@ Use **Runa: Spawn Actor** to choose a source path and supply JSON initial state. Actor context menus offer structured messages and system lifecycle commands. **Runa: Advance Scheduler** explicitly executes ticks; sending alone only queues a message. The extension does not start or shut down the image. Disconnecting -leaves actors in the server. Detailed runtime event subscriptions and structured -compilation diagnostics are not implemented yet. See the [extension guide](vscode/README.md) +leaves actors in the server. Detailed runtime event subscriptions and source-level +debugging are not implemented yet. Rune compilation errors and warnings +appear inline and in Problems for open image sources and actor-specific behavior +documents, including unsaved edits. Validation does not save, commit, or install +the editor text. See the [extension guide](vscode/README.md) for connection restrictions, snapshot semantics, and tests. ## Repairing actors and reading logs diff --git a/crates/runa-code/src/lib.rs b/crates/runa-code/src/lib.rs index 229dbfa..bd963c6 100644 --- a/crates/runa-code/src/lib.rs +++ b/crates/runa-code/src/lib.rs @@ -4,6 +4,7 @@ use std::collections::BTreeMap; use std::sync::Arc; use runa_core::{ImplementationId, SourceId}; +pub use runa_rune::CompilationFailure; use runa_rune::{CompiledProgram, ExperimentResult}; use rune::Module; @@ -109,6 +110,9 @@ pub struct CodeStore { } impl CodeStore { + pub fn check_source(&self, source: &str) -> ExperimentResult> { + CompiledProgram::check(source) + } pub fn compile(&mut self, source: &str) -> ExperimentResult> { self.compile_using(source, CompileFingerprint::current(), || { CompiledProgram::compile(source) diff --git a/crates/runa-core/src/diagnostic.rs b/crates/runa-core/src/diagnostic.rs new file mode 100644 index 0000000..f061472 --- /dev/null +++ b/crates/runa-core/src/diagnostic.rs @@ -0,0 +1,29 @@ +use serde::{Deserialize, Serialize}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum DiagnosticSeverity { + Error, + Warning, +} + +/// Zero-based line and UTF-16 code-unit column, matching editor coordinates. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub struct SourcePosition { + pub line: usize, + pub character: usize, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub struct SourceRange { + pub start: SourcePosition, + pub end: SourcePosition, +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct SourceDiagnostic { + pub severity: DiagnosticSeverity, + pub message: String, + /// None means Rune did not provide a location in the submitted source. + pub range: Option, +} diff --git a/crates/runa-core/src/lib.rs b/crates/runa-core/src/lib.rs index 202b28c..ea169be 100644 --- a/crates/runa-core/src/lib.rs +++ b/crates/runa-core/src/lib.rs @@ -2,8 +2,10 @@ use std::fmt; +mod diagnostic; mod log; mod value; +pub use diagnostic::{DiagnosticSeverity, SourceDiagnostic, SourcePosition, SourceRange}; pub use log::{LogEntry, LogLevel}; pub use value::{ActorRef, MAX_VALUE_BYTES, MAX_VALUE_DEPTH, MAX_VALUE_NODES, Value}; diff --git a/crates/runa-image-api/src/lib.rs b/crates/runa-image-api/src/lib.rs index db3c7f3..afa70a9 100644 --- a/crates/runa-image-api/src/lib.rs +++ b/crates/runa-image-api/src/lib.rs @@ -98,6 +98,10 @@ impl ImageApi { self.code.sources() } + pub fn check_source(&self, source: &str) -> RuntimeResult> { + self.code.check_source(source) + } + pub fn source(&self, path: &str) -> Option<&SourceFile> { self.code.source(path) } @@ -567,6 +571,37 @@ mod tests { Ok(()) } + #[test] + fn validation_never_mutates_sources_actors_or_compile_cache() -> RuntimeResult<()> { + let mut image = ImageApi::new(); + let file = image.write_source( + "actors/checked", + None, + "pub fn handle(ctx, state, sender, message) { state }", + )?; + let actor = image.spawn(&file.path, Value::Integer(3), capacity(2))?; + let before = image.inspect(actor)?; + let artifacts = image.code.artifact_count(); + let sources = image.code.source_count(); + assert!( + image + .check_source("pub fn handle( {")? + .iter() + .any(|diagnostic| diagnostic.severity == runa_core::DiagnosticSeverity::Error) + ); + assert!(image.check_source("pub fn main() { 1 }")?.is_empty()); + assert_eq!(image.source(&file.path), Some(&file)); + let after = image.inspect(actor)?; + assert_eq!(after.implementation, before.implementation); + assert_eq!(after.state, before.state); + assert_eq!(after.status, before.status); + assert_eq!(after.queued, before.queued); + assert_eq!(after.reserved, before.reserved); + assert_eq!(image.code.artifact_count(), artifacts); + assert_eq!(image.code.source_count(), sources); + Ok(()) + } + #[test] fn source_rename_moves_history_and_guards_installed_and_pending_users() -> RuntimeResult<()> { let mut image = ImageApi::new(); diff --git a/crates/runa-protocol/src/lib.rs b/crates/runa-protocol/src/lib.rs index d99f6e4..0aedfd3 100644 --- a/crates/runa-protocol/src/lib.rs +++ b/crates/runa-protocol/src/lib.rs @@ -1,6 +1,7 @@ //! Versioned, transport-neutral wire contracts for Runa clients and servers. pub use runa_core::Value; +pub use runa_core::{DiagnosticSeverity, SourceDiagnostic, SourcePosition, SourceRange}; pub use runa_core::{LogEntry, LogLevel}; use serde::{Deserialize, Serialize}; @@ -46,6 +47,7 @@ impl Request { } match &self.command { + Command::CheckSource { .. } => {} Command::RenameSource { path, target, .. } => { validate_name(path)?; validate_name(target)?; @@ -171,6 +173,9 @@ impl Request { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "operation", content = "parameters", rename_all = "snake_case")] pub enum Command { + CheckSource { + source: String, + }, RenameSource { path: String, target: String, @@ -318,6 +323,9 @@ pub struct Response { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "kind", content = "data", rename_all = "snake_case")] pub enum Reply { + SourceChecked { + diagnostics: Vec, + }, ActorBehavior { source_id: String, source: String, @@ -498,6 +506,8 @@ pub enum ErrorCode { pub struct ProtocolError { pub code: ErrorCode, pub message: String, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub diagnostics: Vec, } impl ProtocolError { @@ -505,6 +515,7 @@ impl ProtocolError { Self { code, message: message.into(), + diagnostics: Vec::new(), } } } diff --git a/crates/runa-rune/src/diagnostics.rs b/crates/runa-rune/src/diagnostics.rs new file mode 100644 index 0000000..f961753 --- /dev/null +++ b/crates/runa-rune/src/diagnostics.rs @@ -0,0 +1,120 @@ +use std::fmt; + +use runa_core::{DiagnosticSeverity, SourceDiagnostic, SourcePosition, SourceRange}; +use rune::ast::{Span, Spanned}; +use rune::diagnostics::{Diagnostic, FatalDiagnosticKind}; + +#[derive(Debug)] +pub struct CompilationFailure { + pub diagnostics: Vec, +} + +impl fmt::Display for CompilationFailure { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(formatter, "Rune compilation failed")?; + for diagnostic in &self.diagnostics { + if diagnostic.severity == DiagnosticSeverity::Error { + write!(formatter, ": {}", diagnostic.message)?; + } + } + Ok(()) + } +} + +impl std::error::Error for CompilationFailure {} + +pub(crate) fn collect( + diagnostics: &rune::Diagnostics, + source_id: rune::SourceId, + source: &str, +) -> Vec { + let mut output = Vec::new(); + for diagnostic in diagnostics.diagnostics() { + let (severity, message, spans) = match diagnostic { + Diagnostic::Fatal(fatal) => { + let spans = match fatal.kind() { + FatalDiagnosticKind::CompileError(error) => { + vec![(error.span(), fatal.source_id())] + } + FatalDiagnosticKind::LinkError( + rune::compile::LinkerError::MissingFunction { spans, .. }, + ) => spans.iter().copied().collect(), + _ => Vec::new(), + }; + (DiagnosticSeverity::Error, fatal.to_string(), spans) + } + Diagnostic::Warning(warning) => ( + DiagnosticSeverity::Warning, + warning.to_string(), + vec![(warning.span(), warning.source_id())], + ), + // Compilation currently emits only fatal and warning diagnostics. + other => ( + DiagnosticSeverity::Error, + format!("Unexpected Rune compile diagnostic: {other:?}"), + Vec::new(), + ), + }; + let ranges: Vec<_> = spans + .into_iter() + .filter(|(_, id)| *id == source_id) + .map(|(span, _)| source_range(source, span)) + .collect(); + if ranges.is_empty() { + output.push(SourceDiagnostic { + severity, + message, + range: None, + }); + } else { + for range in ranges { + output.push(SourceDiagnostic { + severity, + message: message.clone(), + range, + }); + } + } + } + output +} + +fn source_range(source: &str, span: Span) -> Option { + let range = span.range(); + let start = source.get(..range.start)?; + let end = source.get(..range.end)?; + let position = |prefix: &str| SourcePosition { + line: prefix.bytes().filter(|byte| *byte == b'\n').count(), + character: prefix + .rsplit('\n') + .next() + .unwrap_or("") + .encode_utf16() + .count(), + }; + Some(SourceRange { + start: position(start), + end: position(end), + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn ranges_convert_utf8_offsets_to_utf16_columns() { + let source = "// é😀\r\n\"é😀\" + missing"; + let start = source.find("missing").unwrap(); + let range = source_range(source, Span::new(start, source.len())).unwrap(); + assert_eq!( + range.start, + SourcePosition { + line: 1, + character: 8 + } + ); + assert_eq!(range.end.character, 15); + assert!(source_range(source, Span::new(4, 5)).is_none()); + } +} diff --git a/crates/runa-rune/src/lib.rs b/crates/runa-rune/src/lib.rs index c83f2e5..c6208a0 100644 --- a/crates/runa-rune/src/lib.rs +++ b/crates/runa-rune/src/lib.rs @@ -21,6 +21,8 @@ mod value; pub use value::{value_from_rune, value_to_rune}; mod context; pub use context::{ActorContext, ActorLogs, SendEffect}; +mod diagnostics; +pub use diagnostics::CompilationFailure; /// Errors remain visible to the caller; compilation also emits source diagnostics. pub type ExperimentResult = Result>; @@ -49,17 +51,37 @@ async fn native_suspend(value: i64) -> i64 { pub struct CompiledProgram { runtime: Arc, unit: Arc, + diagnostics: Vec, } impl CompiledProgram { pub fn compile(source: &str) -> ExperimentResult { + Self::compile_default(source, true) + } + + /// Compile without terminal output for editor validation, without executing code. + pub fn check(source: &str) -> ExperimentResult> { + match Self::compile_default(source, false) { + Ok(program) => Ok(program.diagnostics), + Err(error) => match error.downcast::() { + Ok(failure) => Ok(failure.diagnostics), + Err(error) => Err(error), + }, + } + } + + fn compile_default(source: &str, emit: bool) -> ExperimentResult { let mut module = Module::new(); module.function("native_double", native_double).build()?; module.function("native_suspend", native_suspend).build()?; - Self::compile_with_module(source, module) + Self::compile_module(source, module, emit) } pub fn compile_with_module(source: &str, module: Module) -> ExperimentResult { + Self::compile_module(source, module, true) + } + + fn compile_module(source: &str, module: Module, emit: bool) -> ExperimentResult { let mut context = Context::with_default_modules()?; let mut values = Module::with_crate("runa")?; values.ty::()?; @@ -93,19 +115,37 @@ impl CompiledProgram { context.install(module)?; let mut sources = Sources::new(); - sources.insert(Source::memory(source)?)?; + let source_id = sources.insert(Source::memory(source)?)?; let mut diagnostics = Diagnostics::new(); let result = rune::prepare(&mut sources) .with_context(&context) .with_diagnostics(&mut diagnostics) .build(); - if !diagnostics.is_empty() { + if emit && !diagnostics.is_empty() { diagnostics.emit(&mut StandardStream::stderr(ColorChoice::Never), &sources)?; } + let mut diagnostics = diagnostics::collect(&diagnostics, source_id, source); + let unit = match result { + Ok(unit) => unit, + Err(error) => { + if !diagnostics + .iter() + .any(|diagnostic| diagnostic.severity == runa_core::DiagnosticSeverity::Error) + { + diagnostics.push(runa_core::SourceDiagnostic { + severity: runa_core::DiagnosticSeverity::Error, + message: error.to_string(), + range: None, + }); + } + return Err(CompilationFailure { diagnostics }.into()); + } + }; Ok(Self { runtime: Arc::new(context.runtime()?), - unit: Arc::new(result?), + unit: Arc::new(unit), + diagnostics, }) } @@ -262,6 +302,46 @@ mod tests { assert!(CompiledProgram::compile("pub fn broken( {").is_err()); } + #[test] + fn structured_compile_diagnostics_preserve_unicode_locations_and_warnings() + -> ExperimentResult<()> { + let source = "pub fn main() {\r\n \"é😀\" + missing\r\n}"; + let diagnostics = CompiledProgram::check(source)?; + let error = diagnostics + .iter() + .find(|diagnostic| diagnostic.severity == runa_core::DiagnosticSeverity::Error) + .ok_or("expected an error diagnostic")?; + assert!(error.message.contains("missing"), "{}", error.message); + let range = error.range.ok_or("expected missing-name location")?; + assert_eq!( + range.start, + runa_core::SourcePosition { + line: 1, + character: 12 + } + ); + assert_eq!( + range.end, + runa_core::SourcePosition { + line: 1, + character: 19 + } + ); + let warnings = CompiledProgram::check("pub fn main() { `hello` }")?; + assert!( + warnings.iter().any(|diagnostic| diagnostic.severity + == runa_core::DiagnosticSeverity::Warning + && diagnostic.range.is_some()), + "{warnings:?}" + ); + assert!(CompiledProgram::check("pub fn main() { 1 }")?.is_empty()); + let error = CompiledProgram::compile(source) + .err() + .ok_or("expected compile failure")?; + assert!(error.downcast_ref::().is_some()); + Ok(()) + } + #[test] fn independent_stacks_and_stepping() -> ExperimentResult<()> { let program = CompiledProgram::compile(SOURCE)?; diff --git a/crates/runa/src/lib.rs b/crates/runa/src/lib.rs index a40abac..cf406d2 100644 --- a/crates/runa/src/lib.rs +++ b/crates/runa/src/lib.rs @@ -65,6 +65,11 @@ impl Server { fn dispatch_command(&mut self, command: Command) -> Result { match command { + Command::CheckSource { source } => self + .image + .check_source(&source) + .map(|diagnostics| Reply::SourceChecked { diagnostics }) + .map_err(api_error), Command::RenameSource { path, target, @@ -497,7 +502,8 @@ fn event_dto(event: &Event) -> EventDto { } } -fn api_error(error: impl std::fmt::Display) -> ProtocolError { +fn api_error(error: impl Into>) -> ProtocolError { + let error = error.into(); let message = error.to_string(); let code = if message.starts_with("behavior revision conflict:") || message.starts_with("source identity conflict:") @@ -527,7 +533,11 @@ fn api_error(error: impl std::fmt::Display) -> ProtocolError { } else { ErrorCode::RuntimeFailure }; - ProtocolError::new(code, message) + let mut result = ProtocolError::new(code, message); + if let Some(failure) = error.downcast_ref::() { + result.diagnostics = failure.diagnostics.clone(); + } + result } fn invalid_request(message: impl Into) -> ProtocolError { diff --git a/crates/runa/src/network.rs b/crates/runa/src/network.rs index be3159d..7d6602f 100644 --- a/crates/runa/src/network.rs +++ b/crates/runa/src/network.rs @@ -76,7 +76,7 @@ async fn serve(listener: TcpListener, mut server: Server) -> io::Result<()> { Command::GetBehaviorSource { .. } | Command::InspectActor { .. } | Command::ListActors | Command::GetEvents | Command::ListSources | Command::ReadSource { .. } | Command::SourceHistory { .. } | - Command::RevisionSource { .. } | Command::ActorBehavior { .. } | Command::Logs { .. }); + Command::RevisionSource { .. } | Command::ActorBehavior { .. } | Command::CheckSource { .. } | Command::Logs { .. }); let response = server.dispatch(dispatch.request); let changed = mutates && response.result.is_ok(); // Dispatch may commit even if the initiating client has disconnected. diff --git a/docs/architecture.md b/docs/architecture.md index 1673ee5..2c45e77 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -529,7 +529,11 @@ creation, guarded deletion, and confirmed rename/move with actor path updates. Actor-specific temporary behavior documents compile on save without changing shared source; reload clears them and a confirmed action can copy them back to working source. These behaviors remain in-memory, not durable. -Structured compilation diagnostics and detailed runtime event subscriptions remain planned. +Structured Rune compilation errors/warnings now flow through the image API and +protocol into VS Code Problems for open sources and temporary behaviors, +including unsaved text. Validation does not mutate the image; source positions +use UTF-16 editor coordinates. Single-source checking is implemented, not an LSP +or a cross-image module resolver. Detailed runtime event subscriptions remain planned. Non-destructive image-change notifications and automatic snapshot refresh are implemented over the shared `runa-debug` WebSocket transport. diff --git a/docs/protocol.md b/docs/protocol.md index 8117308..08bf851 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -53,6 +53,7 @@ behavior aliases. They exclude the `source/` root and `.rn` extension. | --- | --- | --- | | `list_sources` | none | `sources`: file array | | `read_source` | `path` | `source`: file | +| `check_source` | `source` (including unsaved text) | `source_checked`: `{diagnostics}` | | `write_source` | `path`, `expected_source_id` (null to create), `source` | `source`: file | | `rename_source` | `path`, `target`, `expected_source_id`, `update_actors` | `source`: renamed file | | `delete_source` | `path`, `expected_source_id` | `acknowledged` | @@ -93,6 +94,27 @@ Actor snapshots add `behavior_path` (nullable installed source path), `temporary_behavior` (boolean). These are additive v5 metadata fields; older v5 snapshots may omit them and clients default to null/false. +### Compilation diagnostics + +`check_source` compiles a standalone text buffer with the image's native modules. +It does not execute, save, commit, cache, install, or emit `image_changed`. +Both invalid and valid source return `source_checked`: compiler errors/warnings +are validation results; infrastructure failures still use the error envelope. + +Each diagnostic is `{severity: "error"|"warning", message, range}`. A non-null +range is `{start: {line, character}, end: {line, character}}`, with zero-based +lines, UTF-16 code-unit columns, and an exclusive end. Ranges refer to the exact +submitted text, not the current contents of a source path. `range: null` denotes +an error without a location in the submitted source. There are no synthetic +locations inferred from compiler display strings. + +Compilation failures from spawn/reload/become/temporary behavior writes also +carry an optional `diagnostics` array in `ProtocolError`, retaining the existing +`runtime_failure` code and human-readable `message`. This is an additive v5 +field: absent diagnostics deserialize as an empty list and are omitted when +empty. Source checking is single-file; it does not resolve other image files +as modules. + System message shape is `{kind: "stop"|"kill"|"restart"|"pause"|"resume"|"reload"}` or `{kind: "become", path: "actors/other"}`. This is a trusted host request; actor `ctx.send` and `ctx.send_from` accept only ordinary data. Stop runs optional diff --git a/docs/runtime.md b/docs/runtime.md index 9e6156b..4a480af 100644 --- a/docs/runtime.md +++ b/docs/runtime.md @@ -181,6 +181,15 @@ first. Failed actors may keep loaded artifacts after deleting their source; reload requires recreating that path. Source mutations and temporary text are in-memory only. +The read-only `check_source` image service compiles submitted text using the same +native modules as actor execution, returning structured Rune errors/warnings and +optional source ranges. Rune's byte spans are converted to zero-based UTF-16 +editor coordinates. Checks do not execute scripts, mutate source/history/actors, +or retain compile artifacts. Normal compile failures also preserve structured +diagnostics through the protocol error envelope while retaining terminal +diagnostics for non-validation compilation. This is compilation checking only, +not runtime source mapping or state-schema validation. + Runtime callers that need a state transformation can use `install_with_migration`, which receives the committed state and computes its replacement before changing either state or implementation. The operation diff --git a/vscode/README.md b/vscode/README.md index 6940507..ef9ba3f 100644 --- a/vscode/README.md +++ b/vscode/README.md @@ -203,7 +203,8 @@ paths are canonical and exclude the `source/` prefix and `.rn` suffix. Directory rename/delete and cross-image moves are not supported. - **Spawn** uses the latest saved working source, committed or not. If the editor has unsaved changes, spawn and commit ask to save first; cancelling - does nothing. Unsaved editor text is never sent implicitly. + does nothing. Unsaved editor text is never used for spawning/committing; + automatic diagnostics do send it to the connected trusted image for validation. - **Spawn with State...** opens a JSON input field; cancelling the field does not spawn an actor. - **Runa: Commit Source** records the saved working source as a new revision @@ -221,6 +222,33 @@ automation, `runa.commitSource` accepts a source URI and returns the revision; document URI; `runa.rollbackSource` accepts a URI and optional revision and returns the new `source_id` after confirmation. +## Compilation diagnostics + +Open image sources and actor-specific `behavior` documents are checked after a +300 ms edit debounce. Rune errors and warnings appear inline and in **Problems**, +with zero-based UTF-16 ranges converted from Rune's UTF-8 byte spans. Non-ASCII +text and CRLF line endings retain correct editor locations. Diagnostics without +a source location are displayed at the start of the document. + +Validation compiles the current editor text through the connected image's native +context; it never executes the script, saves a source, commits, installs an +implementation, or changes actor state/mailboxes. Validation artifacts are +discarded rather than filling the image's compile cache on every keystroke. +Shared source may still be saved/committed with compilation errors; spawn and +reload require successful compilation. Temporary behavior saves still reject +invalid code without replacing the previously installed implementation. + +Checks are serialized and edits coalesced. Responses for old document versions, +closed editors, or disconnected sessions are discarded. Closing a document or +disconnecting clears its diagnostics; reconnect validates current open image +sources but not old-session actor documents. Compiler errors are normal +validation results, not connection failures. Transport and compiler-service +failures remain explicit notifications in the Runa output channel. + +This is single-source compilation checking, not an LSP or debugger: there is no +completion, cross-file import resolution through image source paths, quick fixes, +runtime source mapping, breakpoints, or variable inspection yet. + ## Development and validation From the repository root: @@ -249,5 +277,5 @@ well as session isolation. On headless Linux: xvfb-run -a mise run test-extension-host ``` -Compilation diagnostics, detailed runtime event subscriptions, packaging for +Detailed runtime event subscriptions, packaging for distribution, and debugging remain future work. diff --git a/vscode/integration/diagnostics.test.ts b/vscode/integration/diagnostics.test.ts new file mode 100644 index 0000000..a44fb44 --- /dev/null +++ b/vscode/integration/diagnostics.test.ts @@ -0,0 +1,56 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { ImageClient } from "../src/client"; +import { ProtocolError } from "../src/protocol"; +import { parseValue } from "../src/values"; +import { startImage } from "./server"; + +test( + "source diagnostics validate unsaved text and compile failures preserve actor behavior", + { timeout: 10000 }, + async (t) => { + const image = await startImage(); + let changes = 0; + const client = await ImageClient.connect( + image.address, + (error) => assert.equal(error, undefined), + () => changes++, + ); + t.after(async () => { + await client.close(); + await image.stop(); + }); + const file = await client.readSource("actors/counter"); + const actor = await client.spawn(file.path, parseValue("0")); + const before = await client.inspectActor(actor); + const initialChanges = changes; + const diagnostics = await client.checkSource('pub fn main() {\r\n "é😀" + missing\r\n}'); + const error = diagnostics.find((diagnostic) => diagnostic.severity === "error"); + assert.ok(error); + assert.match(error.message, /missing/); + assert.deepEqual(error.range, { + start: { line: 1, character: 12 }, + end: { line: 1, character: 19 }, + }); + const warnings = await client.checkSource("pub fn main() { `hello` }"); + assert.ok(warnings.some((diagnostic) => diagnostic.severity === "warning")); + assert.deepEqual(await client.checkSource("pub fn main() { 1 }"), []); + assert.equal(changes, initialChanges, "validation does not emit image_changed"); + assert.doesNotMatch( + image.diagnostics(), + /missing/, + "editor validation does not emit terminal diagnostics", + ); + await assert.rejects( + client.writeActorBehavior(actor, file.source_id, "pub fn handle( {"), + (error) => + error instanceof ProtocolError && + error.diagnostics.some( + (diagnostic) => diagnostic.severity === "error" && diagnostic.range !== null, + ), + ); + assert.deepEqual(await client.inspectActor(actor), before); + assert.deepEqual(await client.readSource(file.path), file); + assert.deepEqual(await client.sourceHistory(file.path), []); + }, +); diff --git a/vscode/integration/host.ts b/vscode/integration/host.ts index 8e086d0..763f963 100644 --- a/vscode/integration/host.ts +++ b/vscode/integration/host.ts @@ -4,6 +4,8 @@ import { control, spawnCounter, startImage } from "./server"; import { ImageSourceTree, RevisionDocuments } from "../src/sourceProvider"; import { actorFileUri } from "../src/actorFiles"; import type { activate } from "../src/extension"; +import { SourceDiagnostics } from "../src/diagnostics"; +import { SourceDiagnostic } from "../src/protocol"; export async function run(): Promise { const image = await startImage(); @@ -38,6 +40,7 @@ export async function run(): Promise { folders, ); await editSources(image.address); + await checkSourceDiagnostics(image.address); await editActorFiles(image.address); const spawned = await vscode.commands.executeCommand("runa.spawn", { path: "actors/counter", @@ -229,6 +232,7 @@ export async function run(): Promise { await assert.rejects(async () => { await vscode.workspace.fs.writeFile(repaired.uri, Buffer.from(repaired.getText())); }, /paused or failed/); + await checkDiagnosticRaces(image.address); console.log( "Runa extension host: spawn, send, system messages, ticks, inspection, automatic refresh, reconnect passed", ); @@ -447,6 +451,198 @@ async function replaceText(document: vscode.TextDocument, text: string): Promise assert.ok(await vscode.workspace.applyEdit(edit)); } +async function checkSourceDiagnostics(address: string): Promise { + await vscode.commands.executeCommand("runa.newSource", undefined, "tests/diagnostics.rn"); + const document = vscode.window.activeTextEditor?.document; + assert.ok(document); + const original = document.getText(); + const invalid = 'pub fn handle(ctx, state, sender, message) {\r\n "é😀" + missing\r\n}'; + await replaceText(document, invalid); + await diagnosticsChanged(document.uri, (diagnostics) => + diagnostics.some((diagnostic) => diagnostic.severity === vscode.DiagnosticSeverity.Error), + ); + const error = vscode.languages + .getDiagnostics(document.uri) + .find((diagnostic) => diagnostic.severity === vscode.DiagnosticSeverity.Error); + assert.ok(error); + assert.equal(error.source, "Rune"); + assert.equal(error.range.start.line, 1); + assert.equal(error.range.start.character, 12); + assert.equal(error.range.end.character, 19); + assert.equal(document.isDirty, true); + assert.equal( + Buffer.from(await vscode.workspace.fs.readFile(document.uri)).toString(), + original, + "validation does not save unsaved source", + ); + await replaceText(document, "pub fn handle(ctx, state, sender, message) { `hello`; state }"); + await diagnosticsChanged( + document.uri, + (diagnostics) => + diagnostics.some((diagnostic) => diagnostic.severity === vscode.DiagnosticSeverity.Warning) && + diagnostics.every((diagnostic) => diagnostic.severity !== vscode.DiagnosticSeverity.Error), + ); + await replaceText(document, original); + await diagnosticsChanged(document.uri, (diagnostics) => diagnostics.length === 0); + assert.ok(await document.save()); + + const ref = await vscode.commands.executeCommand("runa.spawn", { + path: "tests/diagnostics", + state: "0", + }); + assert.ok(ref); + const selection = { actor_ref: ref }; + await vscode.commands.executeCommand("runa.editBehavior", selection); + const behavior = vscode.window.activeTextEditor?.document; + assert.ok(behavior); + await replaceText(behavior, invalid); + await diagnosticsChanged(behavior.uri, (diagnostics) => + diagnostics.some((diagnostic) => diagnostic.severity === vscode.DiagnosticSeverity.Error), + ); + assert.equal( + vscode.languages.getDiagnostics(document.uri).length, + 0, + "temporary diagnostics belong to the actor document", + ); + await assert.rejects( + async () => vscode.workspace.fs.writeFile(behavior.uri, Buffer.from(invalid)), + /Rune compilation failed/, + ); + const inspection = JSON.stringify( + await control(address, { operation: "inspect_actor", parameters: { actor_ref: ref } }), + ); + assert.match(inspection, /"temporary_behavior":false/); + await replaceText(behavior, original); + await diagnosticsChanged(behavior.uri, (diagnostics) => diagnostics.length === 0); + assert.ok(await behavior.save()); + // Rapid successive edits cannot leave diagnostics for an older document version. + await replaceText(behavior, invalid); + await replaceText(behavior, original); + await new Promise((resolve) => setTimeout(resolve, 600)); + assert.equal(vscode.languages.getDiagnostics(behavior.uri).length, 0); + await replaceText(behavior, invalid); + await diagnosticsChanged(behavior.uri, (diagnostics) => diagnostics.length > 0); + await vscode.commands.executeCommand("runa.disconnect"); + assert.equal(vscode.languages.getDiagnostics(behavior.uri).length, 0); + await vscode.commands.executeCommand("runa.connect", address); + assert.equal( + vscode.languages.getDiagnostics(behavior.uri).length, + 0, + "old-session actor documents are not validated", + ); + await replaceText(behavior, original); + await vscode.commands.executeCommand("runa.kill", selection); + console.log( + "Runa compile diagnostics: unsaved source, warnings, UTF-16 locations, temporary isolation and reconnect passed", + ); +} + +function diagnosticsChanged( + uri: vscode.Uri, + expected: (diagnostics: readonly vscode.Diagnostic[]) => boolean, +): Promise { + if (expected(vscode.languages.getDiagnostics(uri))) return Promise.resolve(); + return new Promise((resolve, reject) => { + const subscription = vscode.languages.onDidChangeDiagnostics((event) => { + if ( + event.uris.some((changed) => changed.toString() === uri.toString()) && + expected(vscode.languages.getDiagnostics(uri)) + ) { + clearTimeout(timeout); + subscription.dispose(); + resolve(); + } + }); + const timeout = setTimeout(() => { + subscription.dispose(); + reject( + new Error( + `Diagnostics did not update for ${uri.toString()}: ${JSON.stringify(vscode.languages.getDiagnostics(uri))}`, + ), + ); + }, 5000); + }); +} + +async function checkDiagnosticRaces(address: string): Promise { + await vscode.commands.executeCommand("runa.newSource", undefined, "tests/races.rn"); + const document = vscode.window.activeTextEditor?.document; + assert.ok(document); + const original = document.getText(); + await vscode.commands.executeCommand("runa.disconnect"); + type Check = { source: string; resolve: (diagnostics: SourceDiagnostic[]) => void }; + const checks: Check[] = []; + let started: (() => void) | undefined; + const client = { + checkSource: (source: string) => + new Promise((resolve) => { + checks.push({ source, resolve }); + started?.(); + }), + }; + let connected = true; + const errors: unknown[] = []; + const diagnostics = new SourceDiagnostics( + () => (connected ? { client, endpoint: address, session: 100 } : undefined), + (error) => errors.push(error), + ); + const nextCheck = () => + new Promise((resolve, reject) => { + const timer = setTimeout( + () => reject(new Error("Diagnostic validation did not start")), + 5000, + ); + started = () => { + clearTimeout(timer); + started = undefined; + resolve(); + }; + }); + const error: SourceDiagnostic = { severity: "error", message: "old response", range: null }; + const observed: vscode.Diagnostic[] = []; + const subscription = vscode.languages.onDidChangeDiagnostics(() => + observed.push(...vscode.languages.getDiagnostics(document.uri)), + ); + try { + let checking = nextCheck(); + await replaceText(document, "pub fn broken( {"); + await checking; + assert.equal(checks.length, 1); + checking = nextCheck(); + await replaceText(document, original); + checks[0]!.resolve([error]); + await checking; + assert.equal(checks[1]?.source, original); + checks[1]!.resolve([]); + await new Promise((resolve) => setTimeout(resolve, 50)); + assert.ok( + observed.every((diagnostic) => diagnostic.message !== "old response"), + "an in-flight result cannot annotate a newer document", + ); + checking = nextCheck(); + await replaceText(document, "pub fn broken( {"); + await checking; + diagnostics.clear(); + connected = false; + checks[2]!.resolve([error]); + await new Promise((resolve) => setTimeout(resolve, 50)); + assert.equal( + vscode.languages.getDiagnostics(document.uri).length, + 0, + "an in-flight result cannot repopulate disconnected diagnostics", + ); + assert.deepEqual(errors, []); + } finally { + subscription.dispose(); + diagnostics.dispose(); + for (const check of checks) check.resolve([]); + await replaceText(document, original); + } + console.log( + "Runa compile diagnostics: stale in-flight version and disconnect responses discarded", + ); +} + async function editActorFiles(address: string): Promise { const extension = vscode.extensions.all.find( (candidate) => candidate.packageJSON.name === "runa", diff --git a/vscode/src/client.ts b/vscode/src/client.ts index 2c7e037..72f95fc 100644 --- a/vscode/src/client.ts +++ b/vscode/src/client.ts @@ -17,6 +17,7 @@ import { Reply, SourceCommit, SourceFile, + SourceDiagnostic, SystemMessage, Value, WEBSOCKET_SUBPROTOCOL, @@ -126,6 +127,12 @@ export class ImageClient { return reply.data; } + async checkSource(source: string): Promise { + const reply = await this.request({ operation: "check_source", parameters: { source } }); + if (reply.kind !== "source_checked") throw new Error("Expected source diagnostics"); + return reply.data.diagnostics; + } + async actorBehavior(actorRef: string): Promise<{ source: string; source_id: string }> { const reply = await this.request({ operation: "actor_behavior", diff --git a/vscode/src/diagnostics.ts b/vscode/src/diagnostics.ts new file mode 100644 index 0000000..55c41df --- /dev/null +++ b/vscode/src/diagnostics.ts @@ -0,0 +1,145 @@ +import * as vscode from "vscode"; +import { ImageClient } from "./client"; +import { SourceDiagnostic } from "./protocol"; +import { ACTOR_SCHEME } from "./actorFiles"; +import { SOURCE_SCHEME } from "./sourceProvider"; + +interface Connection { + session: number; + endpoint: string; + client: Pick; +} + +/** Validate editor text without saving, publishing, or installing it in the image. */ +export class SourceDiagnostics implements vscode.Disposable { + private readonly collection = vscode.languages.createDiagnosticCollection("runa"); + private readonly pending = new Map(); + private readonly subscriptions: vscode.Disposable[]; + private timer?: ReturnType; + private running = false; + private generation = 0; + private disposed = false; + + constructor( + private readonly connection: () => Connection | undefined, + private readonly report: (error: unknown) => void, + ) { + this.subscriptions = [ + vscode.workspace.onDidOpenTextDocument((document) => this.schedule(document)), + vscode.workspace.onDidChangeTextDocument(({ document }) => this.schedule(document)), + vscode.workspace.onDidCloseTextDocument((document) => { + this.pending.delete(document.uri.toString()); + this.collection.delete(document.uri); + }), + ]; + } + + private matches(document: vscode.TextDocument, connection: Connection): boolean { + return ( + !document.isClosed && + ((document.uri.scheme === SOURCE_SCHEME && + document.uri.authority === connection.endpoint && + document.uri.path.endsWith(".rn")) || + (document.uri.scheme === ACTOR_SCHEME && + document.uri.authority === String(connection.session) && + document.uri.path.endsWith("/behavior"))) + ); + } + + refresh(): void { + for (const document of vscode.workspace.textDocuments) this.schedule(document); + } + + clear(): void { + this.generation++; + clearTimeout(this.timer); + this.timer = undefined; + this.pending.clear(); + this.collection.clear(); + } + + private schedule(document: vscode.TextDocument): void { + const connection = this.connection(); + if (this.disposed || !connection || !this.matches(document, connection)) return; + this.collection.delete(document.uri); + this.pending.set(document.uri.toString(), document); + if (this.running) return; + clearTimeout(this.timer); + this.timer = setTimeout(() => { + this.timer = undefined; + void this.run(); + }, 300); + } + + private async run(): Promise { + this.running = true; + try { + while (this.pending.size && !this.disposed) { + const document = this.pending.values().next().value; + if (!document) break; + this.pending.delete(document.uri.toString()); + const connection = this.connection(); + if (!connection || !this.matches(document, connection)) continue; + const generation = this.generation; + const version = document.version; + try { + const diagnostics = await connection.client.checkSource(document.getText()); + if ( + this.disposed || + generation !== this.generation || + this.connection()?.client !== connection.client || + document.isClosed || + document.version !== version + ) + continue; + this.collection.set( + document.uri, + diagnostics.map((diagnostic) => this.toEditor(document, diagnostic)), + ); + } catch (error) { + if ( + !this.disposed && + generation === this.generation && + this.connection()?.client === connection.client && + !document.isClosed && + document.version === version + ) + this.report(error); + } + } + } finally { + this.running = false; + } + } + + private toEditor(document: vscode.TextDocument, diagnostic: SourceDiagnostic): vscode.Diagnostic { + let range = new vscode.Range(0, 0, 0, 0); + if (diagnostic.range !== null) { + const { start, end } = diagnostic.range; + if ( + end.line >= document.lineCount || + start.line >= document.lineCount || + start.character > document.lineAt(start.line).text.length || + end.character > document.lineAt(end.line).text.length + ) + throw new Error("Rune diagnostic range exceeds the validated document"); + range = new vscode.Range(start.line, start.character, end.line, end.character); + } + const result = new vscode.Diagnostic( + range, + diagnostic.message, + diagnostic.severity === "error" + ? vscode.DiagnosticSeverity.Error + : vscode.DiagnosticSeverity.Warning, + ); + result.source = "Rune"; + return result; + } + + dispose(): void { + this.disposed = true; + this.clear(); + for (const subscription of this.subscriptions) subscription.dispose(); + this.collection.dispose(); + } +} diff --git a/vscode/src/extension.ts b/vscode/src/extension.ts index 2222620..aca5472 100644 --- a/vscode/src/extension.ts +++ b/vscode/src/extension.ts @@ -12,6 +12,7 @@ import { RefreshQueue } from "./refresh"; import { parseValue, formatValue } from "./values"; import { SnapshotDocuments } from "./snapshotProvider"; import { canEditActor, mailboxValue, mailboxEdit } from "./snapshotEdits"; +import { SourceDiagnostics } from "./diagnostics"; import { ActorFiles, ACTOR_SCHEME, ACTOR_FILES, actorFileUri } from "./actorFiles"; import { ImageSourceFileSystem, @@ -125,6 +126,10 @@ export function activate(context: vscode.ExtensionContext): { const output = vscode.window.createOutputChannel("Runa"); const status = vscode.window.createStatusBarItem(vscode.StatusBarAlignment.Left); const reported = new WeakSet(); + const diagnostics = new SourceDiagnostics( + () => (client ? { client, session, endpoint } : undefined), + (error) => report(error), + ); let client: ImageClient | undefined; let session = 0; let endpoint = ""; @@ -170,6 +175,7 @@ export function activate(context: vscode.ExtensionContext): { const previous = client; client = undefined; documents.disconnect(session); + diagnostics.clear(); actorFiles.disconnect(session); sources.store.detach(); session++; @@ -235,6 +241,7 @@ export function activate(context: vscode.ExtensionContext): { updates.dispose(); tree.replace([]); documents.disconnect(currentSession); + diagnostics.clear(); actorFiles.disconnect(currentSession); sources.store.detach(); sourceTree.refresh(); @@ -263,6 +270,7 @@ export function activate(context: vscode.ExtensionContext): { } if (connected !== client) return; connectionStatus(true); + diagnostics.refresh(); view.message = tree.actors.length ? undefined : "No actors. Spawn a .rn file from the source folder."; @@ -759,6 +767,7 @@ export function activate(context: vscode.ExtensionContext): { revisions, documents, actorFiles, + diagnostics, view, output, status, diff --git a/vscode/src/protocol.ts b/vscode/src/protocol.ts index 42e434f..e0eec8f 100644 --- a/vscode/src/protocol.ts +++ b/vscode/src/protocol.ts @@ -72,7 +72,17 @@ export interface SourceCommit { source_id: string; } +export interface SourceDiagnostic { + severity: "error" | "warning"; + message: string; + range: { + start: { line: number; character: number }; + end: { line: number; character: number }; + } | null; +} + export type Command = + | { operation: "check_source"; parameters: { source: string } } | { operation: "actor_behavior"; parameters: { actor_ref: string } } | { operation: "write_actor_behavior"; @@ -130,6 +140,7 @@ export class ProtocolError extends Error { constructor( public readonly code: string, message: string, + public readonly diagnostics: SourceDiagnostic[] = [], ) { super(`${code}: ${message}`); } @@ -301,7 +312,35 @@ function decodeSourceCommit(input: unknown): SourceCommit { return { revision: commit.revision, source_id }; } +function decodeSourceDiagnostic(input: unknown): SourceDiagnostic { + const diagnostic = object(input); + if (diagnostic.severity !== "error" && diagnostic.severity !== "warning") + throw new Error("Invalid diagnostic severity"); + const message = text(diagnostic.message); + if (!message) throw new Error("Empty diagnostic message"); + const position = (input: unknown) => { + const value = object(input); + const line = uint(value.line), + character = uint(value.character); + if (line > BigInt(Number.MAX_SAFE_INTEGER) || character > BigInt(Number.MAX_SAFE_INTEGER)) + throw new Error("Diagnostic coordinate exceeds editor limits"); + return { line: Number(line), character: Number(character) }; + }; + let range: SourceDiagnostic["range"] = null; + if (diagnostic.range !== null) { + const value = object(diagnostic.range); + range = { start: position(value.start), end: position(value.end) }; + if ( + range.start.line > range.end.line || + (range.start.line === range.end.line && range.start.character > range.end.character) + ) + throw new Error("Reversed diagnostic range"); + } + return { severity: diagnostic.severity, message, range }; +} + export type Reply = + | { kind: "source_checked"; data: { diagnostics: SourceDiagnostic[] } } | { kind: "actor_behavior"; data: { source: string; source_id: string } } | { kind: "logs"; data: { entries: LogEntry[]; next_sequence: bigint; has_more: boolean } } | { kind: "actors"; data: ActorSnapshot[] } @@ -362,9 +401,19 @@ export function decodeResponse(line: string, requestId: string, command: Command ) { throw new Error("Unknown protocol error code"); } - throw new ProtocolError(code, text(error.message)); + throw new ProtocolError( + code, + text(error.message), + error.diagnostics === undefined ? [] : array(error.diagnostics).map(decodeSourceDiagnostic), + ); } const reply = object(result.Ok); + if (command.operation === "check_source" && reply.kind === "source_checked") { + return { + kind: "source_checked", + data: { diagnostics: array(object(reply.data).diagnostics).map(decodeSourceDiagnostic) }, + }; + } if ( (command.operation === "actor_behavior" || command.operation === "write_actor_behavior") && reply.kind === "actor_behavior" diff --git a/vscode/test/protocol.test.ts b/vscode/test/protocol.test.ts index f3a7222..1eae020 100644 --- a/vscode/test/protocol.test.ts +++ b/vscode/test/protocol.test.ts @@ -176,6 +176,72 @@ test("server errors are explicit, typed, and not treated as empty actor lists", ); }); +test("structured diagnostics validate severity and exact editor coordinates on replies and errors", () => { + const command: Command = { + operation: "check_source", + parameters: { source: "pub fn main() { missing }" }, + }; + const diagnostic = { + severity: "error", + message: "Missing variable", + range: { start: { line: 0, character: 16 }, end: { line: 0, character: 23 } }, + }; + const reply = { kind: "source_checked", data: { diagnostics: [diagnostic] } }; + assert.deepEqual(decodeResponse(envelope(reply), "test", command), reply); + for (const change of [ + { severity: "unknown" }, + { message: "" }, + { range: { start: { line: -1, character: 0 }, end: { line: 0, character: 0 } } }, + { range: { start: { line: 0, character: 8 }, end: { line: 0, character: 2 } } }, + { + range: { + start: { line: 9007199254740992n, character: 0 }, + end: { line: 9007199254740992n, character: 1 }, + }, + }, + ]) { + assert.throws(() => + decodeResponse( + envelope({ kind: "source_checked", data: { diagnostics: [{ ...diagnostic, ...change }] } }), + "test", + command, + ), + ); + } + assert.deepEqual( + decodeResponse( + envelope({ + kind: "source_checked", + data: { diagnostics: [{ ...diagnostic, range: null, severity: "warning" }] }, + }), + "test", + command, + ), + { + kind: "source_checked", + data: { diagnostics: [{ ...diagnostic, range: null, severity: "warning" }] }, + }, + ); + assert.throws( + () => + decodeResponse( + json({ + version: 5, + request_id: "test", + result: { + Err: { code: "runtime_failure", message: "Compile failed", diagnostics: [diagnostic] }, + }, + }), + "test", + command, + ), + (error) => + error instanceof ProtocolError && + error.diagnostics.length === 1 && + error.diagnostics[0]?.range?.end.character === 23, + ); +}); + test("mutation replies must match the requested operation and carry validated data", () => { const spawn: Command = { operation: "spawn",