From 61e1fb525da09af8d15215bd8448b1729d52007b Mon Sep 17 00:00:00 2001 From: Teakowa <27560638+Teakowa@users.noreply.github.com> Date: Thu, 29 Oct 2026 11:46:05 +0800 Subject: [PATCH 1/2] perf: bound serve freshness to provider-declared compile inputs (#1005) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Entry-based provider sessions now negotiate LPP 1.7 and carry the provider's `compileInputs` declaration — every path its compile observed — onto the resolved input. The freshness check hashes exactly that observation set: file contents, directory entry lists, missing paths, and symlink targets. A provider without the capability or a check-only load keeps the member-list and whole-tree fingerprints, so undeclaring providers behave exactly as before. Artifact-format negotiation now accepts any session at LPP 1.4 or later so 1.7 sessions keep mapped-text artifacts. LppSourceProvider moves to source_provider/lpp.rs to keep both files under the source-size limit. Fixes #1005 --- crates/wright-driver/src/connect.rs | 4 + crates/wright-driver/src/diag.rs | 1 + crates/wright-driver/src/input.rs | 135 ++++++++++ crates/wright-driver/src/select.rs | 1 + crates/wright-driver/src/service/state.rs | 38 +-- crates/wright-driver/src/session/provider.rs | 15 +- crates/wright-driver/src/source_provider.rs | 215 +--------------- .../wright-driver/src/source_provider/lpp.rs | 243 ++++++++++++++++++ crates/wright-driver/tests/check_failures.rs | 1 + .../tests/source_provider_freshness.rs | 110 +++++++- .../tests/source_provider_lpp.rs | 176 ++++++++++++- crates/wright-lpp/src/lib.rs | 21 ++ crates/wright-lpp/src/provider.rs | 12 +- crates/wright-lpp/src/types.rs | 15 +- crates/wright-lpp/tests/unit.rs | 7 +- docs/agent-contract.md | 3 +- docs/cli/integration.md | 8 +- 17 files changed, 753 insertions(+), 252 deletions(-) create mode 100644 crates/wright-driver/src/source_provider/lpp.rs diff --git a/crates/wright-driver/src/connect.rs b/crates/wright-driver/src/connect.rs index 2348a719..4c6cd24d 100644 --- a/crates/wright-driver/src/connect.rs +++ b/crates/wright-driver/src/connect.rs @@ -186,6 +186,10 @@ mod tests { mutual_protocol_version(&supported(&["1.6"]), Some(ConnectFloor::Entry)), None ); + assert_eq!( + mutual_protocol_version(&supported(&["1.5", "1.7"]), Some(ConnectFloor::Entry)), + Some("1.7") + ); assert_eq!( mutual_protocol_version(&supported(&["1.0"]), None), Some("1.0") diff --git a/crates/wright-driver/src/diag.rs b/crates/wright-driver/src/diag.rs index ab0c6c5d..a0a4f72c 100644 --- a/crates/wright-driver/src/diag.rs +++ b/crates/wright-driver/src/diag.rs @@ -326,6 +326,7 @@ mod tests { kind: "workshop".to_string(), locale: Some("en-US".to_string()), }, + compile_inputs: None, }; let locale = workshop_rs::catalog::Locale::new("en-US"); for kind in OWNER_KINDS { diff --git a/crates/wright-driver/src/input.rs b/crates/wright-driver/src/input.rs index 52f15a68..96337fe8 100644 --- a/crates/wright-driver/src/input.rs +++ b/crates/wright-driver/src/input.rs @@ -20,6 +20,11 @@ pub struct ResolvedInput { pub display: String, pub identity: String, pub origin: Origin, + /// The complete disk-observation set the provider declared for the + /// load (LPP `compileInputs`, #1005) — like `identity`, set after + /// resolution by the provider path. `None` when the provider does not + /// advertise compile-input completeness, or for a non-provider load. + pub compile_inputs: Option>, } pub fn sha256_hex(bytes: &[u8]) -> String { @@ -108,6 +113,73 @@ pub(crate) fn digest_files(files: Vec) -> DiskFingerprint { DiskFingerprint::Digest(sha256_hex(&hasher.finalize())) } +/// The fingerprint of a provider-declared compile-input set (LPP +/// `compileInputs`, #1005). Each declared path contributes the observation +/// the declaration records — existence and kind, a symlink's target, a +/// file's contents, or a listed directory's entry names and kinds — so any +/// change the compile could observe invalidates the snapshot while files +/// outside the declared set stay unread. Callers trust the declaration +/// only when the provider advertised `compileInputs`. +pub(crate) fn digest_declared_inputs(inputs: &[PathBuf]) -> DiskFingerprint { + let mut inputs = inputs.to_vec(); + inputs.sort(); + inputs.dedup(); + let mut hasher = Sha256::new(); + for path in &inputs { + hasher.update(path.as_os_str().as_encoded_bytes()); + hasher.update([0]); + match std::fs::symlink_metadata(path) { + Ok(meta) if meta.file_type().is_symlink() => { + hasher.update(b"link"); + if let Ok(target) = std::fs::read_link(path) { + hasher.update(target.as_os_str().as_encoded_bytes()); + } + } + Ok(meta) if meta.is_dir() => { + hasher.update(b"dir"); + let mut entries: Vec> = std::fs::read_dir(path) + .map(|read| { + read.flatten() + .map(|entry| { + let kind = entry + .file_type() + .map(|kind| { + if kind.is_dir() { + b'd' + } else if kind.is_symlink() { + b'l' + } else { + b'f' + } + }) + .unwrap_or(b'?'); + let mut bytes = entry.file_name().into_encoded_bytes(); + bytes.push(kind); + bytes + }) + .collect() + }) + .unwrap_or_default(); + entries.sort(); + for entry in entries { + hasher.update(&entry); + hasher.update([0]); + } + } + Ok(_) => { + hasher.update(b"file"); + match std::fs::read(path) { + Ok(bytes) => hasher.update(&bytes), + Err(_) => hasher.update([0xff]), + } + } + Err(_) => hasher.update(b"missing"), + } + hasher.update([0]); + } + DiskFingerprint::Digest(sha256_hex(&hasher.finalize())) +} + pub fn resolve(config: &SessionConfig) -> Result { match &config.input { InputSpec::Path(path) => resolve_path(path, config), @@ -187,6 +259,7 @@ fn resolve_file( display, identity: sha256_hex(&bytes), origin, + compile_inputs: None, }) } @@ -233,6 +306,7 @@ fn resolve_directory( display, identity: String::new(), origin, + compile_inputs: None, }) } @@ -414,6 +488,7 @@ fn resolve_text( display, identity: sha256_hex(text.as_bytes()), origin, + compile_inputs: None, }) } @@ -445,6 +520,7 @@ fn resolve_stdin(config: &SessionConfig) -> Result { display: "".to_string(), identity: sha256_hex(&bytes), origin, + compile_inputs: None, }) } @@ -613,6 +689,65 @@ mod tests { std::fs::remove_dir_all(directory).expect("remove test directory"); } + #[test] + #[cfg(unix)] + fn digest_declared_inputs_observes_each_declared_kind() { + let directory = temp_dir("declared"); + let member = directory.join("main.opy"); + let missing = directory.join("missing.opy"); + let undeclared = directory.join("notes.md"); + std::fs::write(&member, "v1\n").expect("write member"); + std::fs::write(&undeclared, "v1\n").expect("write undeclared"); + let inputs = vec![directory.clone(), member.clone(), missing.clone()]; + let baseline = digest_declared_inputs(&inputs); + + std::fs::write(&undeclared, "v2\n").expect("edit undeclared"); + assert_eq!( + baseline, + digest_declared_inputs(&inputs), + "a path outside the declared set is not observed" + ); + + std::fs::write(&member, "v2\n").expect("edit member"); + assert_ne!( + baseline, + digest_declared_inputs(&inputs), + "a declared file's contents are observed" + ); + std::fs::write(&member, "v1\n").expect("restore member"); + assert_eq!(baseline, digest_declared_inputs(&inputs)); + + std::fs::write(directory.join("added.opy"), "n\n").expect("add member"); + assert_ne!( + baseline, + digest_declared_inputs(&inputs), + "a declared directory's entries are observed" + ); + std::fs::remove_file(directory.join("added.opy")).expect("remove member"); + + std::fs::write(&missing, "m\n").expect("create missing"); + assert_ne!( + baseline, + digest_declared_inputs(&inputs), + "a declared missing path's existence is observed" + ); + std::fs::remove_file(&missing).expect("restore missing"); + assert_eq!(baseline, digest_declared_inputs(&inputs)); + + let link = directory.join("link.opy"); + std::os::unix::fs::symlink(&member, &link).expect("create symlink"); + let linked = digest_declared_inputs(std::slice::from_ref(&link)); + std::fs::remove_file(&link).expect("unlink"); + std::os::unix::fs::symlink(&undeclared, &link).expect("relink"); + assert_ne!( + linked, + digest_declared_inputs(&[link]), + "a declared symlink's target is observed" + ); + + std::fs::remove_dir_all(directory).expect("remove test directory"); + } + #[test] fn directory_detection_reports_workshop_and_opy_as_ambiguous() { static COUNTER: AtomicUsize = AtomicUsize::new(0); diff --git a/crates/wright-driver/src/select.rs b/crates/wright-driver/src/select.rs index 4926ec68..725980b3 100644 --- a/crates/wright-driver/src/select.rs +++ b/crates/wright-driver/src/select.rs @@ -457,6 +457,7 @@ mod tests { kind: "workshop".to_string(), locale: None, }, + compile_inputs: None, }; let items = vec![finding("a", "warning", Some("sub/f.ws"))]; diff --git a/crates/wright-driver/src/service/state.rs b/crates/wright-driver/src/service/state.rs index 473798cb..0074baf5 100644 --- a/crates/wright-driver/src/service/state.rs +++ b/crates/wright-driver/src/service/state.rs @@ -12,26 +12,30 @@ use crate::diag::Diagnostic; use crate::input; use crate::{CompilerSession, Loaded, Provenance}; -/// The snapshot's disk-observation set (#1005): where the load's declared -/// member list covers every disk input the snapshot depends on, freshness -/// hashes exactly those files instead of walking the whole input tree. +/// The snapshot's disk-observation set (#1005): where the load declares +/// every disk input the snapshot depends on, freshness hashes exactly +/// those observations instead of walking the whole input tree. /// -/// `Provenance::Source` names the single parsed file, so its member list -/// is complete when the input spec targets that file — members are -/// display paths resolved against `cwd`, the same base the provider -/// document set uses for non-mapped members. A directory spec still -/// observes membership changes (an added `.ws` file flips a Workshop -/// directory load to `input-kind-ambiguous`), so it keeps the directory -/// fingerprint even though the resolved member list has one entry. -/// A mapped provider artifact's `files` table is not complete — opy-rs -/// omits compile-time `.js` script inputs (`#!postCompileHook`, -/// `__script__`) whose edits change the artifact (opy-rs#654) — so -/// mapped loads keep the whole-tree fingerprint the provider contract -/// implies until the member list covers them. Unmapped artifacts, -/// check-only loads, and caller-held text inputs declare no member list -/// and keep the previous fingerprint (or `Detached`). +/// A provider that advertised `compileInputs` (LPP 1.7) declares the +/// complete observation set of its compile — files read, directories +/// listed, missing paths, symlinks and their targets — and the +/// fingerprint hashes those paths. `Provenance::Source` names the single +/// parsed file, so its member list is complete when the input spec +/// targets that file — members are display paths resolved against `cwd`, +/// the same base the provider document set uses for non-mapped members. +/// A directory spec still observes membership changes (an added `.ws` +/// file flips a Workshop directory load to `input-kind-ambiguous`), so +/// it keeps the directory fingerprint even though the resolved member +/// list has one entry. A provider without `compileInputs` declares no +/// complete member list — a mapped artifact's `files` table omits +/// compile-time `.js` inputs (opy-rs#654) — so its loads keep the +/// whole-tree fingerprint, as do unmapped artifacts, check-only loads, +/// and caller-held text inputs. fn loaded_fingerprint(config: &crate::SessionConfig, loaded: &Loaded) -> input::DiskFingerprint { let resolved = &loaded.input; + if let Some(inputs) = &resolved.compile_inputs { + return input::digest_declared_inputs(inputs); + } let file_input = match &config.input { InputSpec::Path(path) => !resolved.cwd.join(path).is_dir(), _ => false, diff --git a/crates/wright-driver/src/session/provider.rs b/crates/wright-driver/src/session/provider.rs index fdae686b..11af747d 100644 --- a/crates/wright-driver/src/session/provider.rs +++ b/crates/wright-driver/src/session/provider.rs @@ -45,15 +45,16 @@ impl CompilerSession { #[cfg(not(target_arch = "wasm32"))] if self.source_provider.is_none() { // The shared connect path (#1004): entry requests prefer the - // 1.4 artifact-negotiation session, and the target's kind sets - // the retry floor — entry requests need 1.1 project loading, - // and a directory target cannot drop below the 1.3 + // highest session the client implements so compile answers + // can carry `compileInputs` (LPP 1.7, #1005), and the target's + // kind sets the retry floor — entry requests need 1.1 project + // loading, and a directory target cannot drop below the 1.3 // source-identity contract it loads under (#638). let connected = self .connect_provider( opy_provider::OPY_LANGUAGE_ID, &lpp_client_info(), - wright_lpp::LPP_ARTIFACT_NEGOTIATION_PROTOCOL_VERSION, + wright_lpp::LPP_COMPILE_INPUTS_PROTOCOL_VERSION, Some(resolved.target.into()), ) .map_err(|error| { @@ -124,6 +125,12 @@ impl CompilerSession { }, ProviderOperation::Compile => provider.compile(&target).map_err(|e| e.diagnostic())?, }; + // The provider's declared compile inputs ride the resolved input + // into `Loaded`, where the freshness check hashes exactly that + // observation set instead of the whole entry tree (#1005). `None` + // — a provider without the `compileInputs` capability, or a + // check-only answer — keeps the wider fingerprint. + resolved.compile_inputs = provider.compile_inputs(); if (operation == ProviderOperation::Compile || compilation.workshop_text.is_some()) && resolved.target == InputTarget::Directory { diff --git a/crates/wright-driver/src/source_provider.rs b/crates/wright-driver/src/source_provider.rs index 10852da9..5519f422 100644 --- a/crates/wright-driver/src/source_provider.rs +++ b/crates/wright-driver/src/source_provider.rs @@ -1,17 +1,13 @@ use std::fmt; use std::path::{Path, PathBuf}; -#[cfg(not(target_arch = "wasm32"))] -use workshop_rs::program::{MAPPED_TEXT_V1, TEXT_V1}; #[cfg(not(target_arch = "wasm32"))] mod columns; #[cfg(not(target_arch = "wasm32"))] -use workshop_rs::MappedText; +mod lpp; use workshop_rs::SourceMap; use crate::diag::{Diagnostic, Stage}; -#[cfg(not(target_arch = "wasm32"))] -use crate::diag::{Origin, Severity, SourceSpan}; use crate::input::InputTarget; #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -188,158 +184,20 @@ pub trait SourceProvider { self.compile(target) } fn compile(&mut self, target: &SourceTarget) -> Result; -} - -#[cfg(not(target_arch = "wasm32"))] -pub struct LppSourceProvider { - provider: Box, - locale: Option, -} - -#[cfg(not(target_arch = "wasm32"))] -impl LppSourceProvider { - pub fn new(provider: Box, locale: Option) -> Self { - Self { provider, locale } - } - - fn entry( - &self, - target: &SourceTarget, - ) -> Result { - let uri = url::Url::from_file_path(target.entry_path()) - .map_err(|()| SourceProviderError::Failed { - code: "provider-invalid-entry".to_string(), - stage: Stage::Internal, - class: crate::diag::ExitClass::Internal, - message: format!( - "cannot convert OPY entry '{}' to an absolute file URI", - target.entry_path().display() - ), - })? - .to_string(); - Ok(wright_lpp::ProjectEntry { - uri, - language_id: SourceLanguage::Opy.as_str().to_string(), - version: 1, - kind: if target.is_directory() { - wright_lpp::ProjectTargetKind::Directory - } else { - wright_lpp::ProjectTargetKind::File - }, - }) - } - - fn project_root_uri(target: &SourceTarget) -> Option { - target - .project_root - .as_deref() - .and_then(provider_project_root) - } -} - -#[cfg(not(target_arch = "wasm32"))] -impl Drop for LppSourceProvider { - fn drop(&mut self) { - let _ = self.provider.shutdown(); - } -} - -#[cfg(not(target_arch = "wasm32"))] -impl SourceProvider for LppSourceProvider { - fn language(&self) -> SourceLanguage { - SourceLanguage::Opy - } - fn check(&mut self, target: &SourceTarget) -> Result { - let entry = self.entry(target)?; - let root_uri = Self::project_root_uri(target); - let res = if target.is_directory() { - self.provider - .check_target(&entry, root_uri.as_deref(), self.locale.as_deref()) - } else { - self.provider - .check_entry(&entry, root_uri.as_deref(), self.locale.as_deref()) - } - .map_err(|error| provider_error(error, &request_context("check", target)))?; - Ok(SourceCompilation { - workshop_text: None, - locale: self.locale.clone(), - provenance: SourceProvenance::Unmapped, - diagnostics: provider_diagnostics(res.documents, self.locale.as_deref()), - source_identity: None, - }) - } - - fn compile(&mut self, target: &SourceTarget) -> Result { - let entry = self.entry(target)?; - let root_uri = Self::project_root_uri(target); - let negotiates_artifacts = self - .provider - .capabilities() - .map_err(|error| provider_error(error, &request_context("compile", target)))? - .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 { - self.provider - .compile_entry(&entry, root_uri.as_deref(), self.locale.as_deref()) - } - .map_err(|error| provider_error(error, &request_context("compile", target)))?; - 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(), - stage: Stage::Internal, - class: crate::diag::ExitClass::Internal, - 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(), - stage: Stage::Internal, - class: crate::diag::ExitClass::Internal, - message: format!( - "the source provider returned unsupported artifact format '{}', expected '{TEXT_V1}'", - a.format - ), - }); - } - None => (None, SourceProvenance::Unmapped), - }; - Ok(SourceCompilation { - workshop_text, - locale: self.locale.clone(), - provenance, - diagnostics: provider_diagnostics(res.diagnostics, self.locale.as_deref()), - source_identity: res.source_identity, - }) + /// The filesystem inputs the provider declared complete for its most + /// recent entry-based compile (LPP `compileInputs`, #1005): `Some` + /// when the provider advertises compile-input completeness and the + /// compile answered with the full observation set, `None` when the + /// provider declares none — the caller then keeps the wider + /// fingerprint. + fn compile_inputs(&self) -> Option> { + None } } #[cfg(not(target_arch = "wasm32"))] -fn request_context(operation: &str, target: &SourceTarget) -> String { - format!( - "the source provider {operation} request for '{}' failed", - target.entry_path().display() - ) -} +pub use lpp::LppSourceProvider; #[cfg(not(target_arch = "wasm32"))] pub fn provider_failure_code(error: &wright_lpp::ProviderError) -> String { @@ -406,55 +264,6 @@ pub(crate) fn provider_error( } } -#[cfg(not(target_arch = "wasm32"))] -fn provider_diagnostics( - documents: Vec, - locale: Option<&str>, -) -> Vec { - let mut arena: Vec = documents - .iter() - .map(|doc| provider_uri_path(&doc.uri)) - .collect(); - arena.sort(); - arena.dedup(); - documents - .into_iter() - .flat_map(|doc| { - let path = provider_uri_path(&doc.uri); - let file = arena.iter().position(|entry| *entry == path).unwrap_or(0); - let lines: Vec = std::fs::read_to_string(&path) - .map(|text| text.split('\n').map(str::to_owned).collect()) - .unwrap_or_default(); - doc.diagnostics.into_iter().map(move |d| { - let severity = match d.severity { - wright_lpp::DiagnosticSeverity::Error => Severity::Error, - wright_lpp::DiagnosticSeverity::Warning => Severity::Warning, - _ => Severity::Info, - }; - Diagnostic { - code: d.code.unwrap_or_else(|| "provider-diagnostic".to_string()), - stage: Stage::Frontend, - severity, - class: crate::diag::class_for_stage(Stage::Frontend), - message: d.message.clone(), - status: None, - span: Some(SourceSpan { - file, - path: path.clone(), - start: columns::provider_position(d.range.start, &lines), - end: columns::provider_position(d.range.end, &lines), - }), - source: Some(Origin { - kind: SourceLanguage::Opy.as_str().to_string(), - locale: locale.map(str::to_owned), - }), - fields: Default::default(), - } - }) - }) - .collect() -} - #[cfg(not(target_arch = "wasm32"))] pub(crate) fn provider_uri_path(uri: &str) -> String { url::Url::parse(uri) @@ -624,7 +433,7 @@ mod tests { message: "diag".to_string(), }], }; - let diagnostics = provider_diagnostics( + let diagnostics = lpp::provider_diagnostics( vec![doc("file:///project/b.opy"), doc("file:///project/a.opy")], None, ); @@ -672,7 +481,7 @@ mod tests { }], }; - let diagnostics = provider_diagnostics( + let diagnostics = lpp::provider_diagnostics( vec![ doc(uri, 4), doc("file:///definitely/not/on-disk.opy".to_string(), 4), diff --git a/crates/wright-driver/src/source_provider/lpp.rs b/crates/wright-driver/src/source_provider/lpp.rs new file mode 100644 index 00000000..d36fcea5 --- /dev/null +++ b/crates/wright-driver/src/source_provider/lpp.rs @@ -0,0 +1,243 @@ +//! The LPP-backed `SourceProvider` adapter (#142): it speaks the wire +//! contract through `wright_lpp` and folds each answer into a +//! `SourceCompilation`. Split out of `source_provider.rs` under the +//! file-size budget; the domain types stay in the parent module. + +use std::path::PathBuf; + +use workshop_rs::MappedText; +use workshop_rs::program::{MAPPED_TEXT_V1, TEXT_V1}; + +use crate::diag::{Diagnostic, Origin, Severity, SourceSpan, Stage}; + +use super::columns; +use super::{ + SourceCompilation, SourceLanguage, SourceProvenance, SourceProvider, SourceProviderError, + SourceTarget, provider_error, provider_uri_path, +}; + +pub struct LppSourceProvider { + provider: Box, + locale: Option, + /// The declared input set of the most recent provider answer: `Some` + /// when the session negotiated `compileInputs` and the compile + /// answered with the list, `None` otherwise. + compile_inputs: Option>, +} + +impl LppSourceProvider { + pub fn new(provider: Box, locale: Option) -> Self { + Self { + provider, + locale, + compile_inputs: None, + } + } + + fn entry( + &self, + target: &SourceTarget, + ) -> Result { + let uri = url::Url::from_file_path(target.entry_path()) + .map_err(|()| SourceProviderError::Failed { + code: "provider-invalid-entry".to_string(), + stage: Stage::Internal, + class: crate::diag::ExitClass::Internal, + message: format!( + "cannot convert OPY entry '{}' to an absolute file URI", + target.entry_path().display() + ), + })? + .to_string(); + Ok(wright_lpp::ProjectEntry { + uri, + language_id: SourceLanguage::Opy.as_str().to_string(), + version: 1, + kind: if target.is_directory() { + wright_lpp::ProjectTargetKind::Directory + } else { + wright_lpp::ProjectTargetKind::File + }, + }) + } + + fn project_root_uri(target: &SourceTarget) -> Option { + target + .project_root + .as_deref() + .and_then(super::provider_project_root) + } +} + +impl Drop for LppSourceProvider { + fn drop(&mut self) { + let _ = self.provider.shutdown(); + } +} + +impl SourceProvider for LppSourceProvider { + fn language(&self) -> SourceLanguage { + SourceLanguage::Opy + } + + fn check(&mut self, target: &SourceTarget) -> Result { + self.compile_inputs = None; + let entry = self.entry(target)?; + let root_uri = Self::project_root_uri(target); + let res = if target.is_directory() { + self.provider + .check_target(&entry, root_uri.as_deref(), self.locale.as_deref()) + } else { + self.provider + .check_entry(&entry, root_uri.as_deref(), self.locale.as_deref()) + } + .map_err(|error| provider_error(error, &request_context("check", target)))?; + Ok(SourceCompilation { + workshop_text: None, + locale: self.locale.clone(), + provenance: SourceProvenance::Unmapped, + diagnostics: provider_diagnostics(res.documents, self.locale.as_deref()), + source_identity: None, + }) + } + + fn compile(&mut self, target: &SourceTarget) -> Result { + self.compile_inputs = None; + let entry = self.entry(target)?; + let root_uri = Self::project_root_uri(target); + let (negotiates_artifacts, declares_inputs) = { + let negotiated = self + .provider + .capabilities() + .map_err(|error| provider_error(error, &request_context("compile", target)))?; + ( + wright_lpp::protocol_version_at_least( + &negotiated.protocol_version, + wright_lpp::LPP_ARTIFACT_NEGOTIATION_PROTOCOL_VERSION, + ), + negotiated.capabilities.compile_inputs, + ) + }; + 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 { + self.provider + .compile_entry(&entry, root_uri.as_deref(), self.locale.as_deref()) + } + .map_err(|error| provider_error(error, &request_context("compile", target)))?; + 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(), + stage: Stage::Internal, + class: crate::diag::ExitClass::Internal, + 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(), + stage: Stage::Internal, + class: crate::diag::ExitClass::Internal, + message: format!( + "the source provider returned unsupported artifact format '{}', expected '{TEXT_V1}'", + a.format + ), + }); + } + None => (None, SourceProvenance::Unmapped), + }; + // The declaration is trusted only when the provider advertised + // `compileInputs` for this session — an unadvertised field is + // ignored, so the freshness path keeps the wider fingerprint. + self.compile_inputs = declares_inputs + .then_some(res.compile_inputs) + .flatten() + .map(|uris| { + uris.iter() + .map(|uri| PathBuf::from(provider_uri_path(uri))) + .collect() + }); + Ok(SourceCompilation { + workshop_text, + locale: self.locale.clone(), + provenance, + diagnostics: provider_diagnostics(res.diagnostics, self.locale.as_deref()), + source_identity: res.source_identity, + }) + } + + fn compile_inputs(&self) -> Option> { + self.compile_inputs.clone() + } +} + +fn request_context(operation: &str, target: &SourceTarget) -> String { + format!( + "the source provider {operation} request for '{}' failed", + target.entry_path().display() + ) +} + +pub(super) fn provider_diagnostics( + documents: Vec, + locale: Option<&str>, +) -> Vec { + let mut arena: Vec = documents + .iter() + .map(|doc| provider_uri_path(&doc.uri)) + .collect(); + arena.sort(); + arena.dedup(); + documents + .into_iter() + .flat_map(|doc| { + let path = provider_uri_path(&doc.uri); + let file = arena.iter().position(|entry| *entry == path).unwrap_or(0); + let lines: Vec = std::fs::read_to_string(&path) + .map(|text| text.split('\n').map(str::to_owned).collect()) + .unwrap_or_default(); + doc.diagnostics.into_iter().map(move |d| { + let severity = match d.severity { + wright_lpp::DiagnosticSeverity::Error => Severity::Error, + wright_lpp::DiagnosticSeverity::Warning => Severity::Warning, + _ => Severity::Info, + }; + Diagnostic { + code: d.code.unwrap_or_else(|| "provider-diagnostic".to_string()), + stage: Stage::Frontend, + severity, + class: crate::diag::class_for_stage(Stage::Frontend), + message: d.message.clone(), + status: None, + span: Some(SourceSpan { + file, + path: path.clone(), + start: columns::provider_position(d.range.start, &lines), + end: columns::provider_position(d.range.end, &lines), + }), + source: Some(Origin { + kind: SourceLanguage::Opy.as_str().to_string(), + locale: locale.map(str::to_owned), + }), + fields: Default::default(), + } + }) + }) + .collect() +} diff --git a/crates/wright-driver/tests/check_failures.rs b/crates/wright-driver/tests/check_failures.rs index 9b90923d..37f395ab 100644 --- a/crates/wright-driver/tests/check_failures.rs +++ b/crates/wright-driver/tests/check_failures.rs @@ -80,6 +80,7 @@ fn negotiated() -> wright_lpp::NegotiatedCapabilities { source_identity: false, lookup: false, resolve_ids: false, + compile_inputs: false, other: Default::default(), }, } diff --git a/crates/wright-driver/tests/source_provider_freshness.rs b/crates/wright-driver/tests/source_provider_freshness.rs index 7b340830..052fae63 100644 --- a/crates/wright-driver/tests/source_provider_freshness.rs +++ b/crates/wright-driver/tests/source_provider_freshness.rs @@ -1,8 +1,9 @@ //! The provider-backed request-count and freshness contracts (#1005): -//! `check` issues one provider compile per run, and a mapped snapshot's -//! freshness observes the whole input tree — including the compile-time -//! `.js` inputs the mapped `files` table does not yet declare -//! (opy-rs#654) — so a non-source edit still invalidates it. +//! `check` issues one provider compile per run, and a snapshot's +//! freshness observes exactly the provider's declared compile inputs +//! when the session negotiated `compileInputs` (LPP 1.7) — the whole +//! input tree otherwise, since an undeclaring provider's member list can +//! omit compile-time `.js` inputs (opy-rs#654). use std::path::PathBuf; use std::sync::{Arc, Mutex}; @@ -42,10 +43,12 @@ fn cleanup(dir: PathBuf) { /// A provider that serves the same compilation on every call — the /// request-count and freshness tests need a repeatable compile across -/// reloads. +/// reloads. `compile_inputs` models the LPP `compileInputs` declaration: +/// `Some` is the complete observation set of the staged compile. struct CloningProvider { operations: Arc>>, compilation: SourceCompilation, + compile_inputs: Option>, } impl SourceProvider for CloningProvider { @@ -67,6 +70,10 @@ impl SourceProvider for CloningProvider { .push("compile"); Ok(self.compilation.clone()) } + + fn compile_inputs(&self) -> Option> { + self.compile_inputs.clone() + } } fn mapped_provenance(text: &str, authored: &std::path::Path) -> wright_driver::SourceProvenance { @@ -91,6 +98,7 @@ fn provider_backend_check_issues_one_compile_per_run() { let provider = CloningProvider { operations: Arc::clone(&operations), compilation: SourceCompilation::success(workshop_fixture("synthetic/basic-rule")), + compile_inputs: None, }; let config = SessionConfig { input: InputSpec::Path(entry), @@ -116,6 +124,7 @@ fn provider_backend_check_serves_compile_from_its_own_snapshot() { let provider = CloningProvider { operations: Arc::clone(&operations), compilation: SourceCompilation::success(workshop_fixture("synthetic/basic-rule")), + compile_inputs: None, }; let config = SessionConfig { input: InputSpec::Path(entry), @@ -135,12 +144,12 @@ fn provider_backend_check_serves_compile_from_its_own_snapshot() { } #[test] -fn a_mapped_project_still_observes_files_the_member_list_omits() { +fn an_undeclaring_provider_keeps_the_whole_tree_fingerprint() { let (dir, entry) = temp_entry(); - // The mapped `files` table lists source members only; a `.js` hook - // consumed at compile time is an input the member list omits until - // opy-rs registers it (opy-rs#654), so the snapshot keeps the - // whole-tree fingerprint and a script edit still invalidates it. + // The provider declares no `compileInputs` set, so the snapshot + // cannot bound its observation set by the mapped `files` table — it + // omits compile-time `.js` inputs (opy-rs#654) — and keeps the + // whole-tree fingerprint: a script edit still invalidates it. let script = dir.join("hook.js"); std::fs::write(&script, "// postCompileHook input").expect("script input"); let text = workshop_fixture("synthetic/control-flow"); @@ -155,6 +164,7 @@ fn a_mapped_project_still_observes_files_the_member_list_omits() { diagnostics: Vec::new(), source_identity: None, }, + compile_inputs: None, }; let config = SessionConfig { input: InputSpec::Path(entry), @@ -197,3 +207,83 @@ fn a_mapped_project_still_observes_files_the_member_list_omits() { ); cleanup(dir); } + +#[test] +fn a_declared_compile_input_set_bounds_the_fingerprint() { + let (dir, entry) = temp_entry(); + // The provider advertises a complete compile-input set (LPP 1.7): + // the entry, the source member, and the compile-time `.js` script. + // Freshness hashes exactly that observation set — an edit to a file + // outside it is never read, while an edit to a declared input still + // invalidates the snapshot (#1005). + let member = dir.join("main.opy"); + let script = dir.join("hook.js"); + let unrelated = dir.join("notes.md"); + let text = workshop_fixture("synthetic/control-flow"); + std::fs::write(&member, &text).expect("member source"); + std::fs::write(&script, "// postCompileHook input").expect("script input"); + std::fs::write(&unrelated, "# project notes").expect("unrelated file"); + let provenance = mapped_provenance(&text, &member); + let provider = CloningProvider { + operations: Arc::new(Mutex::new(Vec::new())), + compilation: SourceCompilation { + workshop_text: Some(text), + locale: None, + provenance, + diagnostics: Vec::new(), + source_identity: None, + }, + compile_inputs: Some(vec![entry.clone(), member.clone(), script.clone()]), + }; + let config = SessionConfig { + input: InputSpec::Path(entry), + kind: SourceKind::Opy, + ..SessionConfig::default() + }; + let mut session = CompilerSession::with_source_provider(config, Box::new(provider)) + .expect("provider session"); + assert!( + session.compile().ok, + "the mapped provider load establishes a snapshot" + ); + let mut service = ToolService::new(&mut session).expect("service loads"); + + for _ in 0..3 { + let ToolResponse::Ok { .. } = service.handle(&ToolRequest::Project) else { + panic!("project reads the established snapshot") + }; + } + assert_eq!(service.reload_count(), 0); + + std::fs::write(&unrelated, "# edited project notes").expect("edit unrelated"); + std::fs::write(dir.join("added.txt"), "a new undeclared file").expect("add file"); + let ToolResponse::Ok { .. } = service.handle(&ToolRequest::Project) else { + panic!("undeclared files leave the snapshot fresh") + }; + assert_eq!( + service.reload_count(), + 0, + "files outside the declared input set are not observed (#1005)" + ); + + std::fs::write(&script, "// an edited compile-time input").expect("edit script"); + let ToolResponse::Ok { .. } = service.handle(&ToolRequest::Project) else { + panic!("a declared input edit reloads into a fresh snapshot") + }; + assert_eq!( + service.reload_count(), + 1, + "a declared compile-time input change invalidates the snapshot once" + ); + + std::fs::write(&member, "// an edited project member").expect("edit member"); + let ToolResponse::Ok { .. } = service.handle(&ToolRequest::Project) else { + panic!("a declared member edit reloads into a fresh snapshot") + }; + assert_eq!( + service.reload_count(), + 2, + "a declared member change invalidates the snapshot exactly once" + ); + cleanup(dir); +} diff --git a/crates/wright-driver/tests/source_provider_lpp.rs b/crates/wright-driver/tests/source_provider_lpp.rs index 573d8d88..c077ceb8 100644 --- a/crates/wright-driver/tests/source_provider_lpp.rs +++ b/crates/wright-driver/tests/source_provider_lpp.rs @@ -4,6 +4,7 @@ use std::path::PathBuf; +use wright_driver::service::{ToolRequest, ToolResponse, ToolService}; use wright_driver::source_provider::{SourceLanguage, SourceProvider, SourceTarget}; use wright_driver::{CompilerSession, InputSpec, SessionConfig, SourceBackend, SourceKind, Stage}; @@ -355,7 +356,7 @@ for line in sys.stdin: assert_eq!( attempts, vec![ - serde_json::json!({"version": "1.4", "client": "wright"}), + serde_json::json!({"version": "1.7", "client": "wright"}), serde_json::json!({"version": "1.3", "client": "wright"}), ], "the retry asks for the highest mutual version, not the next fixed rung" @@ -363,6 +364,179 @@ for line in sys.stdin: cleanup(dir); } +#[cfg(unix)] +#[test] +fn a_compile_inputs_declaration_bounds_the_snapshot_fingerprint() { + use std::os::unix::fs::PermissionsExt; + + const PROVIDER: &str = r#"#!/usr/bin/env python3 +import json, sys, os, pathlib +base = pathlib.Path(os.path.abspath(__file__)).parent +log = base.joinpath("init-log") +artifact = open(base.joinpath("artifact.ws")).read() +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) +for line in sys.stdin: + request = json.loads(line) + id, method = request["id"], request["method"] + if method == "lpp/initialize": + with open(log, "a") as f: + f.write(json.dumps({"version": request["params"]["protocolVersion"]}) + "\n") + reply(id, {"protocolVersion": request["params"]["protocolVersion"], + "serverInfo": {"name": "fake", "version": "0"}, + "languages": [{"id": "opy", "extensions": ["opy"]}], + "capabilities": {"check": True, "compile": True, "projectLoading": True, + "compileInputs": True}}) + elif method == "lpp/compile": + reply(id, {"diagnostics": [], + "compileInputs": [base.joinpath("main.opy").as_uri(), + base.joinpath("hook.js").as_uri()], + "artifact": {"format": "workshop-rs/text-v1", "content": artifact}}) + else: + reply(id, {}) +"#; + + let (dir, entry) = temp_entry(); + let script = dir.join("hook.js"); + let unrelated = dir.join("notes.md"); + std::fs::write(&script, "// postCompileHook input").expect("script input"); + std::fs::write(&unrelated, "# project notes").expect("unrelated file"); + let provider_script = dir.join("fake-provider"); + std::fs::write(&provider_script, PROVIDER).expect("provider script"); + std::fs::set_permissions(&provider_script, std::fs::Permissions::from_mode(0o755)) + .expect("chmod"); + std::fs::write( + dir.join("artifact.ws"), + workshop_fixture("synthetic/basic-rule"), + ) + .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(provider_script), + ..SessionConfig::default() + }) + .expect("session"); + + let compile = session.compile(); + assert!( + compile.ok, + "the declaring provider compiles: {:?}", + compile.diagnostics + ); + let attempts: Vec = std::fs::read_to_string(dir.join("init-log")) + .expect("init log") + .lines() + .map(|line| serde_json::from_str(line).expect("log line")) + .collect(); + assert_eq!( + attempts, + vec![serde_json::json!({"version": "1.7"})], + "the compile-input session negotiates LPP 1.7" + ); + let mut service = ToolService::new(&mut session).expect("service loads"); + let ToolResponse::Ok { .. } = service.handle(&ToolRequest::Project) else { + panic!("project reads the established snapshot") + }; + assert_eq!(service.reload_count(), 0); + + std::fs::write(&unrelated, "# edited project notes").expect("edit unrelated"); + let ToolResponse::Ok { .. } = service.handle(&ToolRequest::Project) else { + panic!("an undeclared file leaves the snapshot fresh") + }; + assert_eq!( + service.reload_count(), + 0, + "a file outside the wire-declared input set is not observed (#1005)" + ); + + std::fs::write(&script, "// an edited compile-time input").expect("edit script"); + let ToolResponse::Ok { .. } = service.handle(&ToolRequest::Project) else { + panic!("a declared input edit reloads into a fresh snapshot") + }; + assert_eq!( + service.reload_count(), + 1, + "a declared compile-time input change invalidates the snapshot once" + ); + cleanup(dir); +} + +#[cfg(unix)] +#[test] +fn an_unadvertised_compile_inputs_field_keeps_the_wide_fingerprint() { + use std::os::unix::fs::PermissionsExt; + + const PROVIDER: &str = r#"#!/usr/bin/env python3 +import json, sys, os, pathlib +base = pathlib.Path(os.path.abspath(__file__)).parent +artifact = open(base.joinpath("artifact.ws")).read() +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) +for line in sys.stdin: + request = json.loads(line) + id, method = request["id"], request["method"] + if method == "lpp/initialize": + reply(id, {"protocolVersion": request["params"]["protocolVersion"], + "serverInfo": {"name": "fake", "version": "0"}, + "languages": [{"id": "opy", "extensions": ["opy"]}], + "capabilities": {"check": True, "compile": True, "projectLoading": True}}) + elif method == "lpp/compile": + reply(id, {"diagnostics": [], + "compileInputs": [base.joinpath("main.opy").as_uri()], + "artifact": {"format": "workshop-rs/text-v1", "content": artifact}}) + else: + reply(id, {}) +"#; + + let (dir, entry) = temp_entry(); + let unrelated = dir.join("notes.md"); + std::fs::write(&unrelated, "# project notes").expect("unrelated file"); + let provider_script = dir.join("fake-provider"); + std::fs::write(&provider_script, PROVIDER).expect("provider script"); + std::fs::set_permissions(&provider_script, std::fs::Permissions::from_mode(0o755)) + .expect("chmod"); + std::fs::write( + dir.join("artifact.ws"), + workshop_fixture("synthetic/basic-rule"), + ) + .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(provider_script), + ..SessionConfig::default() + }) + .expect("session"); + + // The provider answers `compileInputs` without advertising the + // capability: the declaration is not trusted, so freshness keeps the + // whole-tree fingerprint and an undeclared file still invalidates. + let compile = session.compile(); + assert!(compile.ok, "compile: {:?}", compile.diagnostics); + let mut service = ToolService::new(&mut session).expect("service loads"); + let ToolResponse::Ok { .. } = service.handle(&ToolRequest::Project) else { + panic!("project reads the established snapshot") + }; + + std::fs::write(&unrelated, "# edited project notes").expect("edit unrelated"); + let ToolResponse::Ok { .. } = service.handle(&ToolRequest::Project) else { + panic!("an undeclared-file edit reloads under the wide fingerprint") + }; + assert_eq!( + service.reload_count(), + 1, + "an unadvertised declaration is ignored (#1005)" + ); + cleanup(dir); +} + #[cfg(unix)] #[test] fn a_directory_target_requires_the_source_identity_capability() { diff --git a/crates/wright-lpp/src/lib.rs b/crates/wright-lpp/src/lib.rs index 1dd1e30b..7fe8ecc6 100644 --- a/crates/wright-lpp/src/lib.rs +++ b/crates/wright-lpp/src/lib.rs @@ -26,6 +26,10 @@ pub const LPP_ARTIFACT_NEGOTIATION_PROTOCOL_VERSION: &str = "1.4"; /// The LPP 1.5 version that adds name and signature lookup on `lpp/lookup`. pub const LPP_LOOKUP_PROTOCOL_VERSION: &str = "1.5"; +/// The LPP 1.7 version that adds the `compileInputs` compile-input +/// declaration on `lpp/compile`. +pub const LPP_COMPILE_INPUTS_PROTOCOL_VERSION: &str = "1.7"; + /// Every protocol version this client implements, in ascending order — /// the client's side of the highest-mutual-version computation LPP §19.2 /// requires after a `protocolVersionMismatch` (#638). 1.6 is excluded: @@ -37,8 +41,25 @@ pub const LPP_SUPPORTED_PROTOCOL_VERSIONS: &[&str] = &[ LPP_SOURCE_IDENTITY_PROTOCOL_VERSION, LPP_ARTIFACT_NEGOTIATION_PROTOCOL_VERSION, LPP_LOOKUP_PROTOCOL_VERSION, + LPP_COMPILE_INPUTS_PROTOCOL_VERSION, ]; +/// Whether `version` is at least `minimum` in `major.minor` order. LPP +/// protocol versions are cumulative: a contract element defined in 1.4 is +/// valid in every later negotiated session. +pub fn protocol_version_at_least(version: &str, minimum: &str) -> bool { + fn tuple(version: &str) -> Option<(u32, u32)> { + let mut parts = version.split('.'); + let major = parts.next()?.parse().ok()?; + let minor = parts.next()?.parse().ok()?; + Some((major, minor)) + } + match (tuple(version), tuple(minimum)) { + (Some(version), Some(minimum)) => version >= minimum, + _ => false, + } +} + /// 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 b9a49f38..43484de2 100644 --- a/crates/wright-lpp/src/provider.rs +++ b/crates/wright-lpp/src/provider.rs @@ -16,7 +16,7 @@ use crate::types::{ }; use crate::{ LPP_ARTIFACT_NEGOTIATION_PROTOCOL_VERSION, LPP_DIRECTORY_TARGET_PROTOCOL_VERSION, - LPP_LOOKUP_PROTOCOL_VERSION, LPP_PROTOCOL_VERSION, + LPP_LOOKUP_PROTOCOL_VERSION, LPP_PROTOCOL_VERSION, protocol_version_at_least, }; /// The negotiated result of a successful `lpp/initialize`. @@ -205,8 +205,8 @@ pub trait LanguageProvider { capability_unavailable("projectLoading", "lpp/compile") } - /// `lpp/compile` over a file or directory target in an LPP 1.4 session, - /// stating the artifact formats the client accepts, most preferred first. + /// `lpp/compile` over a file or directory target, most-preferred + /// artifact format first (LPP 1.4+). fn compile_target_accepting( &mut self, _target: &ProjectEntry, @@ -538,10 +538,10 @@ impl LanguageProvider for StdioLanguageProvider { ) -> Result { self.require_capability(Capability::ProjectLoading)?; let negotiated = self.capabilities()?.protocol_version.clone(); - if negotiated != LPP_ARTIFACT_NEGOTIATION_PROTOCOL_VERSION { + if !protocol_version_at_least(&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(), + message: "acceptedArtifactFormats requires LPP 1.4 or later".to_string(), }); } let mut params = entry_params(target, project_root, locale); @@ -1009,7 +1009,7 @@ mod tests { } #[test] - fn accepted_artifact_formats_are_sent_only_in_an_lpp_14_session() { + fn accepted_artifact_formats_need_lpp_14_or_later() { let target = ProjectEntry { uri: "file:///project/main.opy".to_string(), language_id: "opy".to_string(), diff --git a/crates/wright-lpp/src/types.rs b/crates/wright-lpp/src/types.rs index 1bb0ec1d..8c432dc8 100644 --- a/crates/wright-lpp/src/types.rs +++ b/crates/wright-lpp/src/types.rs @@ -147,6 +147,15 @@ pub struct CompileResult { skip_serializing_if = "Option::is_none" )] pub source_identity: Option, + /// The complete set of filesystem inputs an entry-based compile read + /// (LPP 1.7 `compileInputs`): absolute `file:` URIs, present only when + /// the provider advertised the capability. + #[serde( + rename = "compileInputs", + default, + skip_serializing_if = "Option::is_none" + )] + pub compile_inputs: Option>, pub artifact: Option, } @@ -309,7 +318,7 @@ impl Capability { /// means not offered, so every field defaults rather than failing /// `provider-malformed` (#638). Ids newer than this client knows are /// retained in `other` instead of being silently dropped. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct Capabilities { #[serde(default)] pub check: bool, @@ -340,6 +349,10 @@ pub struct Capabilities { /// pre-1.6 sessions. #[serde(rename = "resolveIds", default)] pub resolve_ids: bool, + /// `compileInputs` entry-based compile results (LPP 1.7). Absent in + /// pre-1.7 sessions. + #[serde(rename = "compileInputs", default)] + pub compile_inputs: bool, /// Capability ids this client does not model — retained rather than /// silently dropped so negotiation diagnostics can see what the /// provider offered. diff --git a/crates/wright-lpp/tests/unit.rs b/crates/wright-lpp/tests/unit.rs index 5c9247b3..af45e1da 100644 --- a/crates/wright-lpp/tests/unit.rs +++ b/crates/wright-lpp/tests/unit.rs @@ -720,18 +720,13 @@ fn malformed_initialize_response_fails_the_initialize_only() { fn capability_require_refuses_unnegotiated_capabilities() { let capabilities = Capabilities { check: true, - compile: false, - project_loading: false, reconstruct: true, symbols: true, definition: true, references: true, rename: true, edit_validation: true, - source_identity: false, - lookup: false, - resolve_ids: false, - other: std::collections::BTreeMap::new(), + ..Default::default() }; assert!(capabilities.require(Capability::Check).is_ok()); let error = capabilities diff --git a/docs/agent-contract.md b/docs/agent-contract.md index 952c9e02..f4f79d72 100644 --- a/docs/agent-contract.md +++ b/docs/agent-contract.md @@ -242,7 +242,8 @@ A request that consults the program needs a valid current snapshot at request time. When none exists, the request performs the deferred initial load; when one exists, the service fingerprints the input's observable file set: a file input's own content, a directory input's resolved members, or -every file under a source-language project's entry directory. It reloads +a source-language project's declared compile inputs (LPP 1.7 +`compileInputs`), or every file under its entry directory. It reloads when the set changes, so an edit, an added file, or a removed file is reflected in the next `symbols`, `references`, `check`, `lint`, or other program-reading request without restarting the session. An unchanged input diff --git a/docs/cli/integration.md b/docs/cli/integration.md index d3c6c530..fdaead72 100644 --- a/docs/cli/integration.md +++ b/docs/cli/integration.md @@ -6,9 +6,11 @@ `.opy` `check`, `compile`, `lint`, `analyze`, and `inspect` inputs use the owner-backed `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. +Wright first negotiates LPP 1.7; a provider without it falls back to the +highest mutual version — LPP 1.4 sessions keep artifact negotiation, and +older providers keep 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`, `analyze`, `inspect`, and the `inspect` query subcommands parse the provider's canonical Workshop artifact through `workshop-rs` and reuse the existing Wright analyzer and lint registry, so they require the provider's From 280d6795beaebb8b16d6e34b934fe16ea1d3a2a3 Mon Sep 17 00:00:00 2001 From: Teakowa <27560638+Teakowa@users.noreply.github.com> Date: Thu, 29 Oct 2026 12:49:55 +0800 Subject: [PATCH 2/2] fix: keep serve freshness safe on malformed and non-file compile inputs (#1005) An advertised-but-empty `compileInputs` declaration degenerated the snapshot fingerprint to a constant, so no later disk change could ever invalidate it; treat an empty declaration as undeclared so the load falls back to the whole-tree fingerprint. A declared path that is not a regular file (FIFO, socket, device) could block `std::fs::read` forever inside the freshness check; hash a kind marker for non-file kinds instead of reading contents. --- crates/wright-driver/src/input.rs | 27 ++++++++-- crates/wright-driver/src/service/state.rs | 8 ++- .../tests/source_provider_freshness.rs | 53 +++++++++++++++++++ 3 files changed, 81 insertions(+), 7 deletions(-) diff --git a/crates/wright-driver/src/input.rs b/crates/wright-driver/src/input.rs index 96337fe8..330df4ce 100644 --- a/crates/wright-driver/src/input.rs +++ b/crates/wright-driver/src/input.rs @@ -116,10 +116,11 @@ pub(crate) fn digest_files(files: Vec) -> DiskFingerprint { /// The fingerprint of a provider-declared compile-input set (LPP /// `compileInputs`, #1005). Each declared path contributes the observation /// the declaration records — existence and kind, a symlink's target, a -/// file's contents, or a listed directory's entry names and kinds — so any -/// change the compile could observe invalidates the snapshot while files -/// outside the declared set stay unread. Callers trust the declaration -/// only when the provider advertised `compileInputs`. +/// file's contents, a listed directory's entry names and kinds, or a +/// marker for any other kind — so any change the compile could observe +/// invalidates the snapshot while files outside the declared set stay +/// unread and the read never blocks on a non-file kind. Callers trust the +/// declaration only when the provider advertised `compileInputs`. pub(crate) fn digest_declared_inputs(inputs: &[PathBuf]) -> DiskFingerprint { let mut inputs = inputs.to_vec(); inputs.sort(); @@ -166,13 +167,14 @@ pub(crate) fn digest_declared_inputs(inputs: &[PathBuf]) -> DiskFingerprint { hasher.update([0]); } } - Ok(_) => { + Ok(meta) if meta.is_file() => { hasher.update(b"file"); match std::fs::read(path) { Ok(bytes) => hasher.update(&bytes), Err(_) => hasher.update([0xff]), } } + Ok(_) => hasher.update(b"other"), Err(_) => hasher.update(b"missing"), } hasher.update([0]); @@ -748,6 +750,21 @@ mod tests { std::fs::remove_dir_all(directory).expect("remove test directory"); } + #[test] + #[cfg(unix)] + fn digest_declared_inputs_does_not_block_on_a_fifo() { + let directory = temp_dir("fifo"); + let member = directory.join("member.opy"); + let mkfifo = std::process::Command::new("mkfifo").arg(&member).status(); + assert!(mkfifo.expect("spawn mkfifo").success()); + let (sender, receiver) = std::sync::mpsc::channel(); + std::thread::spawn(move || drop(sender.send(digest_declared_inputs(&[member])))); + receiver + .recv_timeout(std::time::Duration::from_secs(15)) + .expect("a declared FIFO must not block the freshness check"); + std::fs::remove_dir_all(directory).expect("remove test directory"); + } + #[test] fn directory_detection_reports_workshop_and_opy_as_ambiguous() { static COUNTER: AtomicUsize = AtomicUsize::new(0); diff --git a/crates/wright-driver/src/service/state.rs b/crates/wright-driver/src/service/state.rs index 0074baf5..5c9c8245 100644 --- a/crates/wright-driver/src/service/state.rs +++ b/crates/wright-driver/src/service/state.rs @@ -30,10 +30,14 @@ use crate::{CompilerSession, Loaded, Provenance}; /// complete member list — a mapped artifact's `files` table omits /// compile-time `.js` inputs (opy-rs#654) — so its loads keep the /// whole-tree fingerprint, as do unmapped artifacts, check-only loads, -/// and caller-held text inputs. +/// and caller-held text inputs. An advertised-but-empty declaration is +/// malformed — a compile necessarily observed at least its entry — so it +/// falls back like an undeclared set. fn loaded_fingerprint(config: &crate::SessionConfig, loaded: &Loaded) -> input::DiskFingerprint { let resolved = &loaded.input; - if let Some(inputs) = &resolved.compile_inputs { + if let Some(inputs) = &resolved.compile_inputs + && !inputs.is_empty() + { return input::digest_declared_inputs(inputs); } let file_input = match &config.input { diff --git a/crates/wright-driver/tests/source_provider_freshness.rs b/crates/wright-driver/tests/source_provider_freshness.rs index 052fae63..4c68c9a3 100644 --- a/crates/wright-driver/tests/source_provider_freshness.rs +++ b/crates/wright-driver/tests/source_provider_freshness.rs @@ -287,3 +287,56 @@ fn a_declared_compile_input_set_bounds_the_fingerprint() { ); cleanup(dir); } + +#[test] +fn an_empty_compile_input_declaration_keeps_staleness_detection() { + let (dir, entry) = temp_entry(); + // An advertised-but-empty `compileInputs` declaration is malformed — + // an entry-based compile necessarily observed at least the entry — so + // freshness falls back to the whole-tree fingerprint instead of + // freezing on the empty set's constant digest (#1005). + let text = workshop_fixture("synthetic/control-flow"); + std::fs::write(dir.join("main.opy"), &text).expect("member source"); + let provenance = mapped_provenance(&text, &dir.join("main.opy")); + let provider = CloningProvider { + operations: Arc::new(Mutex::new(Vec::new())), + compilation: SourceCompilation { + workshop_text: Some(text), + locale: None, + provenance, + diagnostics: Vec::new(), + source_identity: None, + }, + compile_inputs: Some(Vec::new()), + }; + let config = SessionConfig { + input: InputSpec::Path(entry), + kind: SourceKind::Opy, + ..SessionConfig::default() + }; + let mut session = CompilerSession::with_source_provider(config, Box::new(provider)) + .expect("provider session"); + assert!( + session.compile().ok, + "the mapped provider load establishes a snapshot" + ); + let mut service = ToolService::new(&mut session).expect("service loads"); + + for _ in 0..3 { + let ToolResponse::Ok { .. } = service.handle(&ToolRequest::Project) else { + panic!("project reads the established snapshot") + }; + } + assert_eq!(service.reload_count(), 0); + + std::fs::write(dir.join("hook.js"), "// an edited compile-time input").expect("edit input"); + let ToolResponse::Ok { .. } = service.handle(&ToolRequest::Project) else { + panic!("a disk change reloads into a fresh snapshot") + }; + assert_eq!( + service.reload_count(), + 1, + "an empty declaration falls back to the whole-tree fingerprint (#1005)" + ); + cleanup(dir); +}