From f48594ccd838d0734846a2a7f72737be4c0e81c3 Mon Sep 17 00:00:00 2001 From: Teakowa <27560638+Teakowa@users.noreply.github.com> Date: Fri, 25 Sep 2026 21:07:13 +0800 Subject: [PATCH 1/3] feat: map provider-backed Wright findings through SourceMap Negotiate LPP 1.4 with the OPY provider and list workshop-rs/mapped-text-v1 in acceptedArtifactFormats on lpp/compile. When a mapped artifact is returned, apply its SourceMap to the re-parsed Program before validation and transforms so lint, analyze, and validation diagnostics resolve to authored source paths. Nodes without an authored origin keep no span; a shape mismatch falls back to unmapped findings with a source-map-mismatch warning. Providers without LPP 1.4 keep the existing unmapped behavior. Bump workshop-rs to v0.6.4 for the public SourceMap API. Fixes #408 --- Cargo.lock | 4 +- Cargo.toml | 4 +- crates/wright-driver/src/session.rs | 99 +++++++-- crates/wright-driver/src/source_provider.rs | 47 ++++- crates/wright-driver/tests/source_provider.rs | 189 ++++++++++++++++++ crates/wright-lpp/src/lib.rs | 3 + crates/wright-lpp/src/provider.rs | 124 +++++++++++- docs/cli/integration.md | 28 ++- 8 files changed, 461 insertions(+), 37 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index afa92dd9..78a27c0b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2082,8 +2082,8 @@ checksum = "f17a85883d4e6d00e8a97c586de764dabcc06133f7f1d55dce5cdc070ad7fe59" [[package]] name = "workshop-rs" -version = "0.5.0" -source = "git+https://github.com/wrightkit/workshop-rs.git?rev=ac5a6a4cf15bfccc5597cfd6ccb7b5028dfd5053#ac5a6a4cf15bfccc5597cfd6ccb7b5028dfd5053" +version = "0.6.4" +source = "git+https://github.com/wrightkit/workshop-rs.git?rev=712c839f09f9c4daaf65aa7ae619f4fca065e34d#712c839f09f9c4daaf65aa7ae619f4fca065e34d" dependencies = [ "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 32ee5586..0647e860 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,8 +10,8 @@ license = "AGPL-3.0-or-later" repository = "https://github.com/wrightkit/wright" [workspace.dependencies] -# Canonical Workshop core; pinned to the workshop-rs 1.0 release candidate (workshop-rs#268). -workshop-rs = { git = "https://github.com/wrightkit/workshop-rs.git", rev = "ac5a6a4cf15bfccc5597cfd6ccb7b5028dfd5053" } +# Canonical Workshop core; pinned to the workshop-rs 1.0 release candidate (workshop-rs v0.6.4). +workshop-rs = { git = "https://github.com/wrightkit/workshop-rs.git", rev = "712c839f09f9c4daaf65aa7ae619f4fca065e34d" } libc = "0.2" serde = "1" serde_json = "1" diff --git a/crates/wright-driver/src/session.rs b/crates/wright-driver/src/session.rs index a121504c..32f65f6d 100644 --- a/crates/wright-driver/src/session.rs +++ b/crates/wright-driver/src/session.rs @@ -24,7 +24,7 @@ use crate::result::{ }; use crate::source_provider::{ SourceBackend, SourceLanguage, SourceProvenance, SourceProvider, SourceProviderError, - SourceTarget, + SourceTarget, provider_uri_path, }; fn error_kind(error: &opy_provider::OpyProviderError) -> wright_lpp::LocalProviderErrorKind { @@ -64,6 +64,9 @@ pub enum Provenance { Source, /// The program came from an unmapped provider-returned canonical artifact. Unmapped, + /// The program came from a provider-returned canonical artifact whose + /// source map was applied; nodes without an authored origin stay unmapped. + Mapped, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -308,9 +311,20 @@ impl CompilerSession { name: wright_lpp::LPP_CLIENT_NAME.to_string(), version: crate::result::DRIVER_VERSION.to_string(), }; - let initialize = match resolved.target { - InputTarget::File => provider.initialize_project_loading(Some(&client_info)), - InputTarget::Directory => provider.initialize_project_target(Some(&client_info)), + // LPP 1.4 lets the provider return a source-mapped artifact; a + // provider without it keeps the unmapped LPP 1.1/1.2 session. + let initialize = match provider.initialize_artifact_negotiation(Some(&client_info)) { + Err(error) if error.code() == "protocol-version-mismatch" => { + match resolved.target { + InputTarget::File => { + provider.initialize_project_loading(Some(&client_info)) + } + InputTarget::Directory => { + provider.initialize_project_target(Some(&client_info)) + } + } + } + other => other, }; initialize.map_err(|error| { SourceProviderError::Failed { @@ -371,8 +385,10 @@ impl CompilerSession { return Err(first); } self.diagnostics.extend(provider_diagnostics); - let SourceProvenance::Unmapped = compilation.provenance; - let provenance = Provenance::Unmapped; + let source_map = match compilation.provenance { + SourceProvenance::Unmapped => None, + SourceProvenance::Mapped(map) => Some(map), + }; let locale_name = compilation .locale .or_else(|| resolved.origin.locale.clone()) @@ -383,7 +399,7 @@ impl CompilerSession { program: Arc::new(Program::default()), origin: resolved.origin.clone(), input: resolved.clone(), - provenance, + provenance: Provenance::Unmapped, source_files: Arc::new(vec![resolved.display.clone()]), }); } @@ -401,11 +417,36 @@ impl CompilerSession { &locale, &self.catalog, ) - .map_err(|error| workshop_diag_for_unmapped_provider_artifact(error, resolved))?; + .map_err(|error| workshop_diag_for_provider_artifact(error, resolved, &[]))?; + let mut provenance = Provenance::Unmapped; + let mut source_files = vec![resolved.display.clone()]; + if let Some(map) = &source_map { + match map.apply(&mut program) { + Ok(()) => { + provenance = Provenance::Mapped; + source_files = map.files().iter().map(|f| provider_uri_path(f)).collect(); + } + Err(error) => self.diagnostics.push(Diagnostic::warning( + "source-map-mismatch", + Stage::Frontend, + format!( + "the provider source map does not match its Workshop artifact ({error}); findings are reported as unmapped" + ), + )), + } + } self.progress(ProgressEvent::new(ProgressPhase::Validation)); - program - .validate() - .map_err(|error| workshop_diag_for_unmapped_provider_artifact(error, resolved))?; + program.validate().map_err(|error| { + workshop_diag_for_provider_artifact( + error, + resolved, + if provenance == Provenance::Mapped { + &source_files + } else { + &[] + }, + ) + })?; if self.config.profile != wright_transform::Profile::Off { self.progress(ProgressEvent::new(ProgressPhase::Lowering)); wright_transform::run_canonical(&mut program, self.config.profile).map_err( @@ -424,7 +465,7 @@ impl CompilerSession { origin: resolved.origin.clone(), input: resolved.clone(), provenance, - source_files: Arc::new(vec![resolved.display.clone()]), + source_files: Arc::new(source_files), }; self.loaded = Some(loaded.clone()); self.loaded_operation = Some(operation); @@ -1011,6 +1052,14 @@ pub(crate) fn resolve_finding_span_paths(findings: &mut serde_json::Value, loade } let path = if loaded.provenance == Provenance::Unmapped { "".to_string() + } else if loaded.provenance == Provenance::Mapped { + // Every mapped file is an authored source; file 0 is not the input. + let file = span.get("file").and_then(serde_json::Value::as_u64); + match file.and_then(|file| loaded.source_files.get(file as usize)) { + Some(source) => root_relative(Some(Path::new(source)), &loaded.input.root) + .unwrap_or_else(|| source.clone()), + None => "".to_string(), + } } else if let Some(file) = span.get("file").and_then(serde_json::Value::as_u64) { if let Some(source) = loaded.source_files.get(file as usize) { let p = if file == 0 { @@ -1119,15 +1168,33 @@ fn provider_artifact_origin(resolved: &ResolvedInput) -> Origin { } } -fn workshop_diag_for_unmapped_provider_artifact( +/// Map a Workshop error on a provider artifact to a driver diagnostic. +/// +/// `mapped_files` is the applied source map's file table: a span into it is an +/// authored location. Without one, the span points into the provider artifact. +fn workshop_diag_for_provider_artifact( error: workshop_rs::WorkshopError, resolved: &ResolvedInput, + mapped_files: &[String], ) -> Diagnostic { let mut diagnostic = workshop_diag(error, resolved); - if let Some(span) = &mut diagnostic.span { - span.path = "".to_string(); + if mapped_files.is_empty() { + if let Some(span) = &mut diagnostic.span { + span.path = "".to_string(); + } + diagnostic.source = Some(provider_artifact_origin(resolved)); + } else if let Some(span) = &mut diagnostic.span { + match mapped_files.get(span.file) { + Some(path) => { + span.path = root_relative(Some(Path::new(path)), &resolved.root) + .unwrap_or_else(|| path.clone()); + } + None => diagnostic.span = None, + } + } + if diagnostic.span.is_none() { + diagnostic.source = Some(provider_artifact_origin(resolved)); } - diagnostic.source = Some(provider_artifact_origin(resolved)); diagnostic } diff --git a/crates/wright-driver/src/source_provider.rs b/crates/wright-driver/src/source_provider.rs index 28aec224..26c565be 100644 --- a/crates/wright-driver/src/source_provider.rs +++ b/crates/wright-driver/src/source_provider.rs @@ -1,6 +1,11 @@ use std::fmt; use std::path::{Path, PathBuf}; +use workshop_rs::{MappedText, SourceMap}; + +const TEXT_V1: &str = "workshop-rs/text-v1"; +const MAPPED_TEXT_V1: &str = "workshop-rs/mapped-text-v1"; + use crate::diag::{Diagnostic, Origin, Position, Severity, SourceSpan, Stage}; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -39,9 +44,14 @@ pub struct SourceTarget { pub project_root: Option, } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +/// How a compiled Workshop artifact relates to the authored source. +#[derive(Debug, Clone, PartialEq, Eq)] pub enum SourceProvenance { + /// The artifact carries no mapping to the authored source. Unmapped, + /// The provider returned `workshop-rs/mapped-text-v1`; the map applies to + /// the program parsed from the artifact's Workshop text. + Mapped(SourceMap), } impl SourceTarget { @@ -237,7 +247,17 @@ impl SourceProvider for LppSourceProvider { fn compile(&mut self, target: &SourceTarget) -> Result { let entry = self.entry(target)?; let root_uri = Self::project_root_uri(target); - let res = if target.is_directory() { + let negotiates_artifacts = self.provider.capabilities().is_ok_and(|c| { + c.protocol_version == wright_lpp::LPP_ARTIFACT_NEGOTIATION_PROTOCOL_VERSION + }); + let res = if negotiates_artifacts { + self.provider.compile_target_accepting( + &entry, + root_uri.as_deref(), + self.locale.as_deref(), + &[MAPPED_TEXT_V1, TEXT_V1], + ) + } else if target.is_directory() { self.provider .compile_target(&entry, root_uri.as_deref(), self.locale.as_deref()) } else { @@ -245,23 +265,34 @@ impl SourceProvider for LppSourceProvider { .compile_entry(&entry, root_uri.as_deref(), self.locale.as_deref()) } .map_err(provider_error)?; - let workshop_text = match res.artifact { - Some(a) if a.format == "workshop-rs/text-v1" => Some(a.content), + let (workshop_text, provenance) = match res.artifact { + Some(a) if a.format == TEXT_V1 => (Some(a.content), SourceProvenance::Unmapped), + Some(a) if negotiates_artifacts && a.format == MAPPED_TEXT_V1 => { + let mapped = MappedText::from_json(&a.content).map_err(|error| { + SourceProviderError::Failed { + code: "provider-artifact-format".to_string(), + message: format!( + "the source provider returned an invalid '{MAPPED_TEXT_V1}' artifact: {error}" + ), + } + })?; + (Some(mapped.text), SourceProvenance::Mapped(mapped.map)) + } Some(a) => { return Err(SourceProviderError::Failed { code: "provider-artifact-format".to_string(), message: format!( - "the source provider returned unsupported artifact format '{}', expected 'workshop-rs/text-v1'", + "the source provider returned unsupported artifact format '{}', expected '{TEXT_V1}'", a.format ), }); } - None => None, + None => (None, SourceProvenance::Unmapped), }; Ok(SourceCompilation { workshop_text, locale: self.locale.clone(), - provenance: SourceProvenance::Unmapped, + provenance, diagnostics: provider_diagnostics(res.diagnostics, self.locale.as_deref()), source_identity: res.source_identity, }) @@ -312,7 +343,7 @@ fn provider_diagnostics( .collect() } -fn provider_uri_path(uri: &str) -> String { +pub(crate) fn provider_uri_path(uri: &str) -> String { url::Url::parse(uri) .ok() .and_then(|u| u.to_file_path().ok()) diff --git a/crates/wright-driver/tests/source_provider.rs b/crates/wright-driver/tests/source_provider.rs index ca3a835f..2d569936 100644 --- a/crates/wright-driver/tests/source_provider.rs +++ b/crates/wright-driver/tests/source_provider.rs @@ -322,6 +322,195 @@ fn provider_backend_lint_and_analyze_use_the_canonical_artifact_without_opy_span cleanup(dir); } +/// A source map for `text` whose spans point into `authored`, edited by +/// `edit` before decoding, as a provider would return it. +fn mapped_provenance( + text: &str, + authored: &Path, + edit: impl FnOnce(&mut serde_json::Value), +) -> wright_driver::SourceProvenance { + let catalog = workshop_rs::catalog::Catalog::builtin().expect("catalog"); + let locale = workshop_rs::catalog::Locale::new("en-US"); + let program = workshop_rs::parser::parse_with_context(text, &catalog, &locale, &catalog) + .expect("fixture parses"); + let json = workshop_rs::MappedText { + text: text.to_string(), + map: workshop_rs::SourceMap::extract(&program), + } + .to_json(); + let mut artifact: serde_json::Value = serde_json::from_str(&json).expect("mapped JSON"); + artifact["files"] = serde_json::json!([{ + "path": url::Url::from_file_path(authored).expect("file URI").to_string(), + }]); + edit(&mut artifact); + let mapped = workshop_rs::MappedText::from_json(&artifact.to_string()).expect("mapped text"); + wright_driver::SourceProvenance::Mapped(mapped.map) +} + +fn mapped_lint_session( + dir: &Path, + entry: PathBuf, + edit: impl FnOnce(&mut serde_json::Value), +) -> CompilerSession { + let text = workshop_fixture("synthetic/control-flow"); + let provenance = mapped_provenance(&text, &dir.join("main.opy"), edit); + let provider = RecordingProvider { + target: Arc::new(Mutex::new(None)), + operations: Arc::new(Mutex::new(Vec::new())), + check_compilation: None, + compilation: Some(SourceCompilation { + workshop_text: Some(text), + locale: None, + provenance, + diagnostics: Vec::new(), + source_identity: None, + }), + failure: None, + }; + let config = SessionConfig { + input: InputSpec::Path(entry), + kind: SourceKind::Opy, + ..SessionConfig::default() + }; + CompilerSession::with_source_provider(config, Box::new(provider)).expect("provider session") +} + +#[test] +fn mapped_provider_artifact_attributes_findings_to_authored_source() { + let (dir, entry) = temp_entry(); + let mut session = mapped_lint_session(&dir, entry, |_| {}); + + let lint = session.lint(); + assert!(lint.ok, "mapped lint: {:?}", lint.diagnostics); + assert_eq!( + session.load().expect("loaded").provenance, + wright_driver::Provenance::Mapped + ); + assert_eq!(lint.result.program["origin"]["kind"], "opy"); + let findings = lint.result.findings.as_array().expect("finding array"); + assert!(!findings.is_empty(), "fixture supplies a lint finding"); + assert!(findings.iter().all(|finding| { + finding.pointer("/span/path") == Some(&serde_json::Value::String("main.opy".to_string())) + && finding + .pointer("/span/start/line") + .and_then(serde_json::Value::as_u64) + .is_some_and(|line| line > 0) + })); + cleanup(dir); +} + +#[test] +fn nodes_without_an_authored_origin_stay_explicitly_unmapped() { + let (dir, entry) = temp_entry(); + let mut session = mapped_lint_session(&dir, entry, |artifact| { + artifact["spans"] + .as_array_mut() + .expect("span list") + .retain(|node| node["node"] != "action"); + }); + + let lint = session.lint(); + assert!(lint.ok, "mapped lint: {:?}", lint.diagnostics); + let findings = lint.result.findings.as_array().expect("finding array"); + assert!(!findings.is_empty(), "fixture supplies a lint finding"); + assert!( + findings + .iter() + .all(|finding| finding.get("span") == Some(&serde_json::Value::Null)), + "action findings lose their span instead of borrowing another location: {findings:?}" + ); + cleanup(dir); +} + +#[test] +fn shape_mismatch_falls_back_to_unmapped_findings_with_a_diagnostic() { + let (dir, entry) = temp_entry(); + let mut session = mapped_lint_session(&dir, entry, |artifact| { + artifact["shape"]["rules"] + .as_array_mut() + .expect("rule shapes") + .push(serde_json::json!({ "conditions": 0, "actions": 0 })); + }); + + let lint = session.lint(); + assert!( + lint.ok, + "mismatched map must not fail lint: {:?}", + lint.diagnostics + ); + assert_eq!( + session.load().expect("loaded").provenance, + wright_driver::Provenance::Unmapped + ); + let mismatch = lint + .diagnostics + .iter() + .find(|diagnostic| diagnostic.code == "source-map-mismatch") + .expect("mismatch diagnostic"); + assert_eq!(mismatch.severity, wright_driver::Severity::Warning); + let findings = lint.result.findings.as_array().expect("finding array"); + assert!(!findings.is_empty(), "fixture supplies a lint finding"); + assert!(findings.iter().all(|finding| finding.pointer("/span/path") + == Some(&serde_json::Value::String( + "".to_string() + )))); + cleanup(dir); +} + +/// Set `WRIGHT_BASTION_MAIN` to `src/main.opy` of OWBastion/Bastion at revision +/// c010e1a2d468ec7140f474e334067e5ab8d02d89 and `WRIGHT_OPY_PROVIDER` to an +/// `opy-provider` that emits `workshop-rs/mapped-text-v1` to run the pinned +/// real-project attribution check. +#[test] +fn pinned_bastion_findings_resolve_to_authored_opy_locations() { + let (Ok(main), Ok(provider)) = ( + std::env::var("WRIGHT_BASTION_MAIN"), + std::env::var("WRIGHT_OPY_PROVIDER"), + ) else { + eprintln!("SKIPPED: WRIGHT_BASTION_MAIN and WRIGHT_OPY_PROVIDER are not set"); + return; + }; + let main = PathBuf::from(main); + let root = main.parent().expect("project source root").to_path_buf(); + let mut session = CompilerSession::new(SessionConfig { + input: InputSpec::Path(main), + kind: SourceKind::Opy, + source_backend: SourceBackend::Provider, + opy_provider: wright_driver::OpyProviderConfig::with_executable(PathBuf::from(provider)), + ..SessionConfig::default() + }) + .expect("session"); + + let lint = session.lint(); + assert!(lint.ok, "Bastion lint: {:?}", lint.diagnostics); + assert_eq!( + session.load().expect("loaded").provenance, + wright_driver::Provenance::Mapped, + "the provider must hand off a source map" + ); + let findings = lint.result.findings.as_array().expect("finding array"); + let mut mapped = 0; + for finding in findings { + let span = &finding["span"]; + if span.is_null() { + continue; + } + let path = span["path"].as_str().expect("span path"); + assert!(path.ends_with(".opy"), "not an authored path: {path}"); + let source = std::fs::read_to_string(root.join(path)).expect("authored file"); + let line = span["start"]["line"].as_u64().expect("span line") as usize; + assert!( + (1..=source.lines().count()).contains(&line), + "{path}:{line} is outside the authored file" + ); + mapped += 1; + } + assert!( + mapped > 0, + "no Bastion finding resolved to an authored location" + ); +} + #[test] fn provider_backend_rejects_stdin_without_fabricating_an_entry() { let config = SessionConfig { diff --git a/crates/wright-lpp/src/lib.rs b/crates/wright-lpp/src/lib.rs index 26ae42bb..75d49b04 100644 --- a/crates/wright-lpp/src/lib.rs +++ b/crates/wright-lpp/src/lib.rs @@ -44,6 +44,9 @@ pub const LPP_PROJECT_LOADING_PROTOCOL_VERSION: &str = "1.1"; /// The LPP 1.2 version that adds provider-owned directory targets. pub const LPP_DIRECTORY_TARGET_PROTOCOL_VERSION: &str = "1.2"; +/// The LPP 1.4 version that adds artifact format negotiation on `lpp/compile`. +pub const LPP_ARTIFACT_NEGOTIATION_PROTOCOL_VERSION: &str = "1.4"; + /// The client name reported in `lpp/initialize` `clientInfo`. pub const LPP_CLIENT_NAME: &str = "wright"; diff --git a/crates/wright-lpp/src/provider.rs b/crates/wright-lpp/src/provider.rs index 97509d30..da6c9b35 100644 --- a/crates/wright-lpp/src/provider.rs +++ b/crates/wright-lpp/src/provider.rs @@ -23,7 +23,10 @@ use crate::types::{ InitializeResult, LocationsResult, Position, ProjectEntry, ReconstructResult, RenameResult, SymbolsResult, TextEdit, ValidateEditsResult, WorkshopArtifact, }; -use crate::{LPP_DIRECTORY_TARGET_PROTOCOL_VERSION, LPP_PROTOCOL_VERSION}; +use crate::{ + LPP_ARTIFACT_NEGOTIATION_PROTOCOL_VERSION, LPP_DIRECTORY_TARGET_PROTOCOL_VERSION, + LPP_PROTOCOL_VERSION, +}; /// The negotiated result of a successful `lpp/initialize`. #[derive(Debug, Clone)] @@ -98,6 +101,19 @@ pub trait LanguageProvider { }) } + /// Initialize with LPP 1.4 for `lpp/compile` artifact format negotiation. + fn initialize_artifact_negotiation( + &mut self, + client_info: Option<&ClientInfo>, + ) -> Result { + let _ = client_info; + Err(ProviderError::ProtocolVersionMismatch { + supported: Vec::new(), + message: "the provider client does not support LPP 1.4 artifact negotiation" + .to_string(), + }) + } + /// The negotiated capabilities, after a successful initialize. fn capabilities(&self) -> Result<&NegotiatedCapabilities, ProviderError>; @@ -176,6 +192,23 @@ pub trait LanguageProvider { )) } + /// `lpp/compile` over a file or directory target in an LPP 1.4 session, + /// stating the artifact formats the client accepts, most preferred first. + fn compile_target_accepting( + &mut self, + target: &ProjectEntry, + project_root: Option<&str>, + locale: Option<&str>, + accepted_artifact_formats: &[&str], + ) -> Result { + let _ = (target, project_root, locale, accepted_artifact_formats); + Err(ProviderError::lpp( + crate::error::LppErrorKind::CapabilityUnavailable, + json!({ "capability": "projectLoading", "method": "lpp/compile" }), + "capability 'projectLoading' is not available in this provider client", + )) + } + /// `lpp/reconstruct`: reconstruct source from a provider-owned artifact. fn reconstruct( &mut self, @@ -398,6 +431,13 @@ impl LanguageProvider for StdioLanguageProvider { self.initialize_with_version(LPP_DIRECTORY_TARGET_PROTOCOL_VERSION, client_info) } + fn initialize_artifact_negotiation( + &mut self, + client_info: Option<&ClientInfo>, + ) -> Result { + self.initialize_with_version(LPP_ARTIFACT_NEGOTIATION_PROTOCOL_VERSION, client_info) + } + fn capabilities(&self) -> Result<&NegotiatedCapabilities, ProviderError> { self.negotiated .as_ref() @@ -476,6 +516,26 @@ impl LanguageProvider for StdioLanguageProvider { self.compile_entry(target, project_root, locale) } + fn compile_target_accepting( + &mut self, + target: &ProjectEntry, + project_root: Option<&str>, + locale: Option<&str>, + accepted_artifact_formats: &[&str], + ) -> Result { + self.require_capability(Capability::ProjectLoading)?; + let negotiated = self.capabilities()?.protocol_version.clone(); + if negotiated != LPP_ARTIFACT_NEGOTIATION_PROTOCOL_VERSION { + return Err(ProviderError::ProtocolVersionMismatch { + supported: vec![negotiated], + message: "acceptedArtifactFormats is valid only in an LPP 1.4 session".to_string(), + }); + } + let mut params = entry_params(target, project_root, locale); + params["acceptedArtifactFormats"] = json!(accepted_artifact_formats); + self.call(Capability::Compile, "lpp/compile", params) + } + fn reconstruct( &mut self, artifact: &WorkshopArtifact, @@ -751,6 +811,12 @@ mod tests { result } + fn init_result_artifact_negotiation_json() -> Value { + let mut result = init_result_project_loading_json(); + result["protocolVersion"] = json!("1.4"); + result + } + fn ok_response(result: Value) -> Value { json!({ "jsonrpc": "2.0", "id": 0, "result": result }) } @@ -928,4 +994,60 @@ mod tests { assert_eq!(check["params"]["entry"]["kind"], "directory"); fake.assert_only_requests(1); } + + #[test] + fn accepted_artifact_formats_are_sent_only_in_an_lpp_14_session() { + let target = ProjectEntry { + uri: "file:///project/main.opy".to_string(), + language_id: "opy".to_string(), + version: 1, + kind: crate::types::ProjectTargetKind::File, + }; + let (mut provider, fake) = Fake::spawn(vec![ + FakeStep::Respond(ok_response(init_result_artifact_negotiation_json())), + FakeStep::Respond(ok_response(json!({ "diagnostics": [], "artifact": null }))), + ]); + provider + .initialize_artifact_negotiation(None) + .expect("LPP 1.4 initialize"); + provider + .compile_target_accepting( + &target, + None, + None, + &["workshop-rs/mapped-text-v1", "workshop-rs/text-v1"], + ) + .expect("negotiated compile"); + let initialize: Value = serde_json::from_str( + &fake + .requests + .recv_timeout(Duration::from_millis(250)) + .expect("initialize request"), + ) + .expect("initialize JSON"); + assert_eq!(initialize["params"]["protocolVersion"], "1.4"); + let compile: Value = serde_json::from_str( + &fake + .requests + .recv_timeout(Duration::from_millis(250)) + .expect("compile request"), + ) + .expect("compile JSON"); + assert_eq!( + compile["params"]["acceptedArtifactFormats"], + json!(["workshop-rs/mapped-text-v1", "workshop-rs/text-v1"]) + ); + + let (mut older, fake) = Fake::spawn(vec![FakeStep::Respond(ok_response( + init_result_directory_target_json(), + ))]); + older + .initialize_project_target(None) + .expect("LPP 1.2 initialize"); + let error = older + .compile_target_accepting(&target, None, None, &["workshop-rs/text-v1"]) + .expect_err("pre-1.4 session"); + assert_eq!(error.supported_protocol_versions(), vec!["1.2"]); + fake.assert_only_requests(1); + } } diff --git a/docs/cli/integration.md b/docs/cli/integration.md index cc0ad2c4..784ef1a8 100644 --- a/docs/cli/integration.md +++ b/docs/cli/integration.md @@ -5,15 +5,27 @@ ## The `.opy` source implementation `.opy` `check`, `compile`, `lint`, and `analyze` inputs use the owner-backed -`opy-rs` implementation through Wright's narrow LPP adapter. File targets use -LPP 1.1; directory targets use LPP 1.2 so `opy-rs` owns project entry -discovery. No Node or OverPy is involved, and provider failures -never fall back to the native frontend. `lint` and `analyze` parse the +`opy-rs` implementation through Wright's narrow LPP adapter. Wright first +negotiates LPP 1.4; a provider without it keeps the LPP 1.1 file-target or +LPP 1.2 directory-target session. No Node or OverPy is involved, and provider +failures never fall back to the native frontend. `lint` and `analyze` parse the provider's canonical Workshop artifact through `workshop-rs` and reuse the -existing Wright analyzer and lint registry. Provider artifacts have no OPY -span map, so their analysis origin and lint spans are explicitly -`provider-artifact` / `` rather than fabricated OPY -locations. An entry path is required for provider-backed workflows, so stdin +existing Wright analyzer and lint registry. + +In an LPP 1.4 session Wright lists `workshop-rs/mapped-text-v1` and +`workshop-rs/text-v1` in `acceptedArtifactFormats` on `lpp/compile`. When the +provider returns the mapped format, Wright applies its `SourceMap` to the +re-parsed program before validation and transforms, so findings and validation +diagnostics resolve to authored source paths (root-relative when under the +project root). Nodes without an authored origin, and nodes a transform +restructures, carry no span: their findings have `span: null` rather than a +fabricated location. If the map does not match the artifact's shape, Wright +reports a `source-map-mismatch` warning and treats the whole artifact as +unmapped. Without the mapped format, the analysis origin and lint spans are +explicitly `provider-artifact` / `` rather than fabricated +OPY locations. + +An entry path is required for provider-backed workflows, so stdin `.opy` is rejected; `--root` supplies the project root for file and directory inputs. The provider executable is resolved by the #244 bootstrap path and can be From 7d41b81ceb49ec3ed55d42339227263db66a6be0 Mon Sep 17 00:00:00 2001 From: Teakowa <27560638+Teakowa@users.noreply.github.com> Date: Fri, 25 Sep 2026 21:24:42 +0800 Subject: [PATCH 2/3] fix: restart provider before LPP fallback and map analyze spans Respawn the provider process before the pre-1.4 fallback, since LPP allows one initialize per session. Resolve mapped file paths for analyze facts. Use the workshop-rs artifact format constants. --- crates/wright-driver/src/session.rs | 144 +++++++++++------- crates/wright-driver/src/source_provider.rs | 4 +- crates/wright-driver/tests/source_provider.rs | 92 +++++++++++ 3 files changed, 182 insertions(+), 58 deletions(-) diff --git a/crates/wright-driver/src/session.rs b/crates/wright-driver/src/session.rs index 32f65f6d..48bdb326 100644 --- a/crates/wright-driver/src/session.rs +++ b/crates/wright-driver/src/session.rs @@ -298,34 +298,40 @@ impl CompilerSession { } .with_project_root(resolved.root.clone()); if self.source_provider.is_none() { - let mut provider = self - .language_provider(opy_provider::OPY_LANGUAGE_ID) - .map_err(|error| { - SourceProviderError::Failed { - code: error.code().to_string(), - message: error.to_string(), - } - .diagnostic() - })?; + let spawn = |session: &Self| { + session + .language_provider(opy_provider::OPY_LANGUAGE_ID) + .map_err(|error| { + SourceProviderError::Failed { + code: error.code().to_string(), + message: error.to_string(), + } + .diagnostic() + }) + }; + let mut provider = spawn(self)?; let client_info = wright_lpp::ClientInfo { name: wright_lpp::LPP_CLIENT_NAME.to_string(), version: crate::result::DRIVER_VERSION.to_string(), }; - // LPP 1.4 lets the provider return a source-mapped artifact; a - // provider without it keeps the unmapped LPP 1.1/1.2 session. - let initialize = match provider.initialize_artifact_negotiation(Some(&client_info)) { - Err(error) if error.code() == "protocol-version-mismatch" => { - match resolved.target { - InputTarget::File => { - provider.initialize_project_loading(Some(&client_info)) - } - InputTarget::Directory => { - provider.initialize_project_target(Some(&client_info)) - } + // LPP 1.4 lets the provider return a source-mapped artifact. A + // provider without it keeps the unmapped LPP 1.1/1.2 session on a + // restarted process, as the protocol requires after a version + // mismatch. + let mut initialize = provider.initialize_artifact_negotiation(Some(&client_info)); + if initialize + .as_ref() + .is_err_and(|error| error.code() == "protocol-version-mismatch") + { + let _ = provider.shutdown(); + provider = spawn(self)?; + initialize = match resolved.target { + InputTarget::File => provider.initialize_project_loading(Some(&client_info)), + InputTarget::Directory => { + provider.initialize_project_target(Some(&client_info)) } - } - other => other, - }; + }; + } initialize.map_err(|error| { SourceProviderError::Failed { code: error.code().to_string(), @@ -635,7 +641,10 @@ impl CompilerSession { if let serde_json::Value::Object(object) = &mut program { object.remove("findings"); } - let facts = semantic_facts(&service); + let mut facts = semantic_facts(&service); + if loaded.provenance == Provenance::Mapped { + resolve_nested_span_paths(&mut facts, &loaded); + } self.finish("analyze", AnalyzeResult { program, facts }) } @@ -1047,41 +1056,66 @@ pub(crate) fn resolve_finding_span_paths(findings: &mut serde_json::Value, loade let Some(span) = finding.get_mut("span") else { continue; }; - if !span.is_object() { - continue; + if span.is_object() { + span["path"] = serde_json::Value::String(span_path(span, loaded)); } - let path = if loaded.provenance == Provenance::Unmapped { - "".to_string() - } else if loaded.provenance == Provenance::Mapped { - // Every mapped file is an authored source; file 0 is not the input. - let file = span.get("file").and_then(serde_json::Value::as_u64); - match file.and_then(|file| loaded.source_files.get(file as usize)) { - Some(source) => root_relative(Some(Path::new(source)), &loaded.input.root) - .unwrap_or_else(|| source.clone()), - None => "".to_string(), - } - } else if let Some(file) = span.get("file").and_then(serde_json::Value::as_u64) { - if let Some(source) = loaded.source_files.get(file as usize) { - let p = if file == 0 { - loaded - .input - .path - .as_deref() - .or_else(|| Some(Path::new(source))) - } else if Path::new(source).is_absolute() { - Some(Path::new(source)) - } else { - None - }; - p.and_then(|path| root_relative(Some(path), &loaded.input.root)) - .unwrap_or_else(|| source.clone()) + } +} + +/// Add the resolved `path` to every span object nested anywhere in `value`. +fn resolve_nested_span_paths(value: &mut serde_json::Value, loaded: &Loaded) { + match value { + serde_json::Value::Object(object) => { + if ["file", "start", "end"] + .iter() + .all(|key| object.contains_key(*key)) + { + let path = span_path(&serde_json::Value::Object(object.clone()), loaded); + object.insert("path".to_string(), serde_json::Value::String(path)); } else { - format!("") + object + .values_mut() + .for_each(|v| resolve_nested_span_paths(v, loaded)); } + } + serde_json::Value::Array(items) => items + .iter_mut() + .for_each(|v| resolve_nested_span_paths(v, loaded)), + _ => {} + } +} + +fn span_path(span: &serde_json::Value, loaded: &Loaded) -> String { + if loaded.provenance == Provenance::Unmapped { + "".to_string() + } else if loaded.provenance == Provenance::Mapped { + // Every mapped file is an authored source; file 0 is not the input. + let file = span.get("file").and_then(serde_json::Value::as_u64); + match file.and_then(|file| loaded.source_files.get(file as usize)) { + Some(source) => root_relative(Some(Path::new(source)), &loaded.input.root) + .unwrap_or_else(|| source.clone()), + None => "".to_string(), + } + } else if let Some(file) = span.get("file").and_then(serde_json::Value::as_u64) { + if let Some(source) = loaded.source_files.get(file as usize) { + let p = if file == 0 { + loaded + .input + .path + .as_deref() + .or_else(|| Some(Path::new(source))) + } else if Path::new(source).is_absolute() { + Some(Path::new(source)) + } else { + None + }; + p.and_then(|path| root_relative(Some(path), &loaded.input.root)) + .unwrap_or_else(|| source.clone()) } else { - loaded.input.display.clone() - }; - span["path"] = serde_json::Value::String(path); + format!("") + } + } else { + loaded.input.display.clone() } } diff --git a/crates/wright-driver/src/source_provider.rs b/crates/wright-driver/src/source_provider.rs index 26c565be..883522f0 100644 --- a/crates/wright-driver/src/source_provider.rs +++ b/crates/wright-driver/src/source_provider.rs @@ -1,11 +1,9 @@ use std::fmt; use std::path::{Path, PathBuf}; +use workshop_rs::program::{MAPPED_TEXT_V1, TEXT_V1}; use workshop_rs::{MappedText, SourceMap}; -const TEXT_V1: &str = "workshop-rs/text-v1"; -const MAPPED_TEXT_V1: &str = "workshop-rs/mapped-text-v1"; - use crate::diag::{Diagnostic, Origin, Position, Severity, SourceSpan, Stage}; #[derive(Debug, Clone, Copy, PartialEq, Eq)] diff --git a/crates/wright-driver/tests/source_provider.rs b/crates/wright-driver/tests/source_provider.rs index 2d569936..c5e1c8f9 100644 --- a/crates/wright-driver/tests/source_provider.rs +++ b/crates/wright-driver/tests/source_provider.rs @@ -399,6 +399,98 @@ fn mapped_provider_artifact_attributes_findings_to_authored_source() { cleanup(dir); } +#[test] +fn mapped_analyze_locations_resolve_to_authored_source() { + fn collect(value: &serde_json::Value, spans: &mut Vec) { + match value { + serde_json::Value::Object(object) => { + if object.contains_key("file") && object.contains_key("start") { + spans.push(value.clone()); + } else { + object.values().for_each(|v| collect(v, spans)); + } + } + serde_json::Value::Array(items) => items.iter().for_each(|v| collect(v, spans)), + _ => {} + } + } + + let (dir, entry) = temp_entry(); + let mut session = mapped_lint_session(&dir, entry, |_| {}); + + let analyze = session.analyze(); + assert!(analyze.ok, "mapped analyze: {:?}", analyze.diagnostics); + let mut spans = Vec::new(); + collect(&analyze.result.facts, &mut spans); + assert!(!spans.is_empty(), "analyze facts carry spans"); + assert!(spans.iter().all(|span| span["path"] == "main.opy")); + cleanup(dir); +} + +/// A conforming pre-1.4 provider allows one `lpp/initialize` per process, so +/// the fallback after a refused 1.4 negotiation must use a restarted process. +#[cfg(unix)] +#[test] +fn pre_lpp_14_provider_is_restarted_before_the_unmapped_fallback() { + use std::os::unix::fs::PermissionsExt; + + const PROVIDER: &str = r#"#!/usr/bin/env python3 +import json, os, sys +artifact = open(os.path.join(os.path.dirname(os.path.abspath(__file__)), "artifact.ws")).read() +initialized = False +def reply(id, result=None, error=None): + message = {"jsonrpc": "2.0", "id": id} + message.update({"error": error} if error else {"result": result}) + print(json.dumps(message), flush=True) +def lpp_error(id, kind, details, text): + reply(id, error={"code": -32000, "message": text, "data": {"lpp": {"kind": kind, "details": details}}}) +for line in sys.stdin: + request = json.loads(line) + id, method = request["id"], request["method"] + if method == "lpp/initialize": + first = not initialized + initialized = True + if not first: + lpp_error(id, "alreadyInitialized", {}, "already initialized") + elif request["params"]["protocolVersion"] != "1.1": + lpp_error(id, "protocolVersionMismatch", {"supportedProtocolVersions": ["1.1"]}, "unsupported") + else: + reply(id, {"protocolVersion": "1.1", "serverInfo": {"name": "fake", "version": "0"}, + "languages": [{"id": "opy", "extensions": ["opy"]}], + "capabilities": {"check": True, "compile": True, "reconstruct": False, "symbols": False, "definition": False, "references": False, "rename": False, "editValidation": False, "projectLoading": True}}) + elif method == "lpp/compile": + reply(id, {"diagnostics": [], "artifact": {"format": "workshop-rs/text-v1", "content": artifact}}) + else: + reply(id, {}) +"#; + + let (dir, entry) = temp_entry(); + let script = dir.join("fake-provider"); + std::fs::write(&script, PROVIDER).expect("provider script"); + std::fs::set_permissions(&script, std::fs::Permissions::from_mode(0o755)).expect("chmod"); + std::fs::write( + dir.join("artifact.ws"), + workshop_fixture("synthetic/control-flow"), + ) + .expect("artifact"); + let mut session = CompilerSession::new(SessionConfig { + input: InputSpec::Path(entry), + kind: SourceKind::Opy, + source_backend: SourceBackend::Provider, + opy_provider: wright_driver::OpyProviderConfig::with_executable(script), + ..SessionConfig::default() + }) + .expect("session"); + + let lint = session.lint(); + assert!(lint.ok, "fallback lint: {:?}", lint.diagnostics); + assert_eq!( + session.load().expect("loaded").provenance, + wright_driver::Provenance::Unmapped + ); + cleanup(dir); +} + #[test] fn nodes_without_an_authored_origin_stay_explicitly_unmapped() { let (dir, entry) = temp_entry(); From cfc664b8559e020cfeb772532668c8e3cd3eec20 Mon Sep 17 00:00:00 2001 From: Teakowa <27560638+Teakowa@users.noreply.github.com> Date: Fri, 25 Sep 2026 21:37:44 +0800 Subject: [PATCH 3/3] test: check analyze locations in the pinned Bastion regression --- crates/wright-driver/tests/source_provider.rs | 58 ++++++++++--------- 1 file changed, 31 insertions(+), 27 deletions(-) diff --git a/crates/wright-driver/tests/source_provider.rs b/crates/wright-driver/tests/source_provider.rs index c5e1c8f9..94764caa 100644 --- a/crates/wright-driver/tests/source_provider.rs +++ b/crates/wright-driver/tests/source_provider.rs @@ -347,6 +347,21 @@ fn mapped_provenance( wright_driver::SourceProvenance::Mapped(mapped.map) } +/// Every span object (`file`, `start`, `end`) nested anywhere in `value`. +fn collect_spans(value: &serde_json::Value, spans: &mut Vec) { + match value { + serde_json::Value::Object(object) => { + if object.contains_key("file") && object.contains_key("start") { + spans.push(value.clone()); + } else { + object.values().for_each(|v| collect_spans(v, spans)); + } + } + serde_json::Value::Array(items) => items.iter().for_each(|v| collect_spans(v, spans)), + _ => {} + } +} + fn mapped_lint_session( dir: &Path, entry: PathBuf, @@ -401,27 +416,13 @@ fn mapped_provider_artifact_attributes_findings_to_authored_source() { #[test] fn mapped_analyze_locations_resolve_to_authored_source() { - fn collect(value: &serde_json::Value, spans: &mut Vec) { - match value { - serde_json::Value::Object(object) => { - if object.contains_key("file") && object.contains_key("start") { - spans.push(value.clone()); - } else { - object.values().for_each(|v| collect(v, spans)); - } - } - serde_json::Value::Array(items) => items.iter().for_each(|v| collect(v, spans)), - _ => {} - } - } - let (dir, entry) = temp_entry(); let mut session = mapped_lint_session(&dir, entry, |_| {}); let analyze = session.analyze(); assert!(analyze.ok, "mapped analyze: {:?}", analyze.diagnostics); let mut spans = Vec::new(); - collect(&analyze.result.facts, &mut spans); + collect_spans(&analyze.result.facts, &mut spans); assert!(!spans.is_empty(), "analyze facts carry spans"); assert!(spans.iter().all(|span| span["path"] == "main.opy")); cleanup(dir); @@ -580,13 +581,21 @@ fn pinned_bastion_findings_resolve_to_authored_opy_locations() { wright_driver::Provenance::Mapped, "the provider must hand off a source map" ); - let findings = lint.result.findings.as_array().expect("finding array"); - let mut mapped = 0; - for finding in findings { - let span = &finding["span"]; - if span.is_null() { - continue; - } + let mut spans = Vec::new(); + collect_spans(&lint.result.findings, &mut spans); + let lint_spans = spans.len(); + let analyze = session.analyze(); + assert!(analyze.ok, "Bastion analyze: {:?}", analyze.diagnostics); + collect_spans(&analyze.result.facts, &mut spans); + assert!( + lint_spans > 0, + "no Bastion finding resolved to an authored location" + ); + assert!( + spans.len() > lint_spans, + "no Bastion analyze location resolved to an authored location" + ); + for span in &spans { let path = span["path"].as_str().expect("span path"); assert!(path.ends_with(".opy"), "not an authored path: {path}"); let source = std::fs::read_to_string(root.join(path)).expect("authored file"); @@ -595,12 +604,7 @@ fn pinned_bastion_findings_resolve_to_authored_opy_locations() { (1..=source.lines().count()).contains(&line), "{path}:{line} is outside the authored file" ); - mapped += 1; } - assert!( - mapped > 0, - "no Bastion finding resolved to an authored location" - ); } #[test]