From 37a13e356620af5f8ae143f472023335f8bc59a3 Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 9 Oct 2026 02:22:18 +0900 Subject: [PATCH 1/4] fix: resolve target-qualified ancestor interfaces through live publications Add a provider-owned routing hint without granting authority. Fetch checks provider ownership and re-resolves one coherent target/scope publication; default self/shared scope behavior remains unchanged. Validation: 7 ancestor regression tests, 901 Worker unit and 110 integration tests, root cargo check, formatting and diff checks passed. --- crates/worker/src/wip.rs | 30 ++++++++- crates/worker/src/wip/ancestor_tests.rs | 81 +++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 1 deletion(-) diff --git a/crates/worker/src/wip.rs b/crates/worker/src/wip.rs index 3a3687f90..eaeeece80 100644 --- a/crates/worker/src/wip.rs +++ b/crates/worker/src/wip.rs @@ -288,6 +288,13 @@ pub trait WipSubtreeProvider: Send + Sync { /// looking up a mutable ancestor after resolving the target. Self-scoped /// publishers can use `WipPublication::self_scoped` on their pinned snapshot. async fn publication(&self, path: &str) -> Result, ProtocolError>; + /// Routing hint for target-qualified local Interface names in an ancestor + /// scope. This grants no authority: Host re-resolves one coherent current + /// publication, checks the exact reference, and forbids escaping this owner. + /// Self/shared-scope providers retain the ordinary scope Object route. + fn interface_target(&self, reference: &InterfaceReference) -> Option { + Some(reference.scope.clone()) + } async fn children(&self, path: &str) -> Result, ProtocolError>; } @@ -1248,8 +1255,29 @@ impl WipHost { reference .validate() .map_err(|e| protocol_error(ProtocolErrorCode::InvalidRequest, e.to_string()))?; + let target = if let Some(mount) = self.subtree(&reference.scope) { + let target = mount.provider.interface_target(reference).ok_or_else(|| { + protocol_error( + ProtocolErrorCode::InterfaceNotFound, + "interface is not published", + ) + })?; + if wip_protocol::validate_path(&target).is_err() + || !self + .subtree(&target) + .is_some_and(|owner| owner.root == mount.root) + { + return Err(protocol_error( + ProtocolErrorCode::InterfaceNotFound, + "interface target is outside its provider", + )); + } + target + } else { + reference.scope.clone() + }; if let Some(publication) = self - .publication_for_interface_live(&reference.scope, reference) + .publication_for_interface_live(&target, reference) .await? && publication.projection.interface == *reference && let Some(scope) = publication.scope diff --git a/crates/worker/src/wip/ancestor_tests.rs b/crates/worker/src/wip/ancestor_tests.rs index ab1ac929a..d645a3436 100644 --- a/crates/worker/src/wip/ancestor_tests.rs +++ b/crates/worker/src/wip/ancestor_tests.rs @@ -538,3 +538,84 @@ async fn same_name_scope_replacement_or_loss_of_ref_rejects_old_metadata_without ); } } + +struct TargetQualifiedProvider { + inner: Arc, + target_hint: Mutex>, +} +#[async_trait] +impl WipSubtreeProvider for TargetQualifiedProvider { + fn interface_target(&self, _: &InterfaceReference) -> Option { + self.target_hint.lock().unwrap().clone() + } + async fn publication(&self, path: &str) -> Result, ProtocolError> { + self.inner.publication(path).await + } + async fn children(&self, path: &str) -> Result, ProtocolError> { + self.inner.children(path).await + } +} + +#[tokio::test] +async fn ancestor_interface_target_hint_fetches_one_current_publication_and_never_grants_authority() +{ + let (_, inner) = fixture("/github", true, false); + let provider = Arc::new(TargetQualifiedProvider { + inner: inner.clone(), + target_hint: Mutex::new(Some(inner.target_path.clone())), + }); + let mut mounts = WipMountRegistry::new(); + mounts.allocate_namespace("scope", "github").unwrap(); + mounts + .mount(projection( + "/github", + "/github", + "read", + inner.calls.clone(), + )) + .unwrap(); + mounts + .mount_subtree(WipSubtreeMount { + root: "/github".into(), + provider: provider.clone(), + }) + .unwrap(); + let host = WipHost::new(mounts); + let selected = reference("/github", "read"); + let fetched = host.fetch_interface_live(&selected).await.unwrap(); + assert_eq!(fetched.scope_ref, Some("live-scope".into())); + assert_eq!( + inner.state.lock().unwrap().lookups, + [inner.target_path.clone()] + ); + // A hint must not fabricate a Descriptor or restore a deleted scope lifetime. + inner.state.lock().unwrap().visible = false; + assert_eq!( + host.fetch_interface_live(&selected).await.unwrap_err().code, + ProtocolErrorCode::InterfaceNotFound + ); + inner.state.lock().unwrap().visible = true; + let forged = reference("/github", "not-published"); + assert_eq!( + host.fetch_interface_live(&forged).await.unwrap_err().code, + ProtocolErrorCode::InterfaceNotFound + ); + for hint in [ + None, + Some("/outside"), + Some("/github-extra/item"), + Some("/github/../item"), + Some("/github/missing"), + ] { + *provider.target_hint.lock().unwrap() = hint.map(str::to_owned); + inner.state.lock().unwrap().lookups.clear(); + assert_eq!( + host.fetch_interface_live(&selected).await.unwrap_err().code, + ProtocolErrorCode::InterfaceNotFound + ); + if hint != Some("/github/missing") { + assert!(inner.state.lock().unwrap().lookups.is_empty()); + } + } + assert_eq!(inner.calls.load(Ordering::SeqCst), 0); +} From 64459991204722f64b246a28d8f7781444a4fb76 Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 9 Oct 2026 02:22:18 +0900 Subject: [PATCH 2/4] fix: migrate dependent server WIP tests to public object tools Dependent test compilation exposed removed discover, opaque Interface strings and private call usage. Select structured references from actual path Inspect and invoke through the public WIP 0.2.0 API. Validation: complete Workspace Server suite (691 unit + 11 CLI), Server API 83 tests including generated artifact freshness, root cargo check and SDK resolution verification passed. --- Cargo.lock | 1 + crates/workspace-server/Cargo.toml | 1 + .../src/server_workspace_config_wip_tests.rs | 84 +++++-------------- 3 files changed, 22 insertions(+), 64 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 649153ef5..f64b67e06 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6766,6 +6766,7 @@ dependencies = [ "uuid", "webauthn-authenticator-rs", "webauthn-rs", + "wip-protocol", "workdir", "worker", "worker-runtime", diff --git a/crates/workspace-server/Cargo.toml b/crates/workspace-server/Cargo.toml index 1f5f4f66a..7b50a34cf 100644 --- a/crates/workspace-server/Cargo.toml +++ b/crates/workspace-server/Cargo.toml @@ -56,6 +56,7 @@ zeroize.workspace = true webauthn-rs = { workspace = true } [dev-dependencies] +wip-protocol.workspace = true agen.workspace = true tempfile.workspace = true tools.workspace = true diff --git a/crates/workspace-server/src/server_workspace_config_wip_tests.rs b/crates/workspace-server/src/server_workspace_config_wip_tests.rs index c0df1d1a5..104cc61b4 100644 --- a/crates/workspace-server/src/server_workspace_config_wip_tests.rs +++ b/crates/workspace-server/src/server_workspace_config_wip_tests.rs @@ -139,73 +139,29 @@ impl worker::WorkspaceClient for ConfigRouterClient { Ok(response) } } -fn config_interface(path: &str) -> String { - // Use the published contextual interface identity, not a model-managed validator. - let hex = path - .bytes() - .map(|byte| format!("{byte:02x}")) - .collect::(); - format!("yoi.workspace-config/node/v1/@/{hex}") +async fn inspected_interface(runtime: &worker::wip::WipRuntime, path: &str, refresh: bool) -> wip_protocol::InterfaceReference { + let inspected = runtime.inspect(path.into(),refresh).await.unwrap(); + let value: Value = serde_json::from_str(inspected.content.as_deref().unwrap()).unwrap(); + assert_eq!(value["path"], path); + let reference = &value["interfaces"][0]["reference"]; + wip_protocol::InterfaceReference { scope:reference["scope"].as_str().unwrap().into(), name:reference["name"].as_str().unwrap().into() } } async fn config_discover(runtime: &worker::wip::WipRuntime, path: &str) { - let observed = runtime - .discover(path.into(), 0, true) - .await - .unwrap() - .content - .unwrap(); - assert!( - observed.contains(&config_interface(path)), - "interface must come from live discovery: {observed}" - ); - runtime.inspect(config_interface(path), true).await.unwrap(); + runtime.tree(path.into(),0,true).await.unwrap(); + let reference = inspected_interface(runtime,path,true).await; + assert_eq!(reference.scope,path); + assert_eq!(reference.name,"yoi.workspace-config/node/v1"); } -async fn config_call( - runtime: &worker::wip::WipRuntime, - path: &str, - operation: &str, - args: Value, -) -> std::result::Result { - runtime - .call( - path.into(), - config_interface(path), - operation.into(), - args, - agen::tool::ToolExecutionContext::direct(), - ) - .await +async fn config_call(runtime: &worker::wip::WipRuntime, path: &str, operation: &str, args: Value) -> std::result::Result { + let interface = inspected_interface(runtime,path,false).await; + runtime.invoke(path.into(),interface,operation.into(),args,agen::tool::ToolExecutionContext::direct()).await } - -async fn catalog_call( - runtime: &worker::wip::WipRuntime, - path: &str, - base: &str, - operation: &str, -) -> Value { - let hex = path - .bytes() - .map(|byte| format!("{byte:02x}")) - .collect::(); - let interface = format!("{base}/@/{hex}"); - let discovered = runtime - .discover(path.into(), 0, true) - .await - .unwrap() - .content - .unwrap(); - assert!(discovered.contains(&interface), "{discovered}"); - runtime.inspect(interface.clone(), true).await.unwrap(); - let output = runtime - .call( - path.into(), - interface, - operation.into(), - json!({}), - agen::tool::ToolExecutionContext::direct(), - ) - .await - .unwrap(); +async fn catalog_call(runtime: &worker::wip::WipRuntime, path: &str, base: &str, operation: &str) -> Value { + runtime.tree(path.into(),0,true).await.unwrap(); + let interface = inspected_interface(runtime,path,true).await; + assert_eq!(interface.scope,path); + assert_eq!(interface.name,base); + let output = runtime.invoke(path.into(),interface,operation.into(),json!({}),agen::tool::ToolExecutionContext::direct()).await.unwrap(); serde_json::from_str(output.content.as_deref().unwrap()).unwrap() } @@ -635,7 +591,7 @@ async fn assert_config_production_http_adapter( assert_config_catalog(&wip, &attached).await; config_discover(&wip, "/workspace-config/main.dcdl").await; let main_interface = wip - .inspect(config_interface("/workspace-config/main.dcdl"), true) + .inspect("/workspace-config/main.dcdl".into(), true) .await .unwrap() .content From af655ae632fb4a51dd593b7fddc028dcc890603f Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 9 Oct 2026 02:22:52 +0900 Subject: [PATCH 3/4] feat: limit checkout indexing to entrances and expose bounded typed discovery Content directories always have no indexable children; direct authorized paths remain resolvable. Add List using provider Read/enumeration authority and separate List permission, dirs-first lexical live continuation, bounded retention/path bytes, scoped result coordinates and External DTO validation. List/Glob/Grep return Descriptor-typed entries without eager acquisition. Bind content Interfaces coherently to the live entrance scope. Validation: root cargo check; FS 58 unit+6 pagination, Workdir 125+5 (default and http-client), Tools 84 unit+39 integration+1 doctest, Worker 901 unit+110 integration, Server 691 unit+11 CLI, Server API 83 tests; exact crates.io WIP 0.2.0 resolution; format/diff checks. Focused model/local/remote/provider/race/scope checks passed. Commands and semantics in docs/development/wip-integration-validation.md. Publication/review paused: continuation ShowTicket returned HTTP 401. No Backend bypass, source push, MR approval, target integration or live environment update is claimed. --- crates/fs-operation/src/lib.rs | 2 + crates/fs-operation/src/local.rs | 134 ++++- crates/fs-operation/src/operation.rs | 21 + crates/fs-operation/tests/list_pagination.rs | 248 +++++++++ crates/tools/README.md | 17 + crates/tools/src/checkout.rs | 126 ++++- crates/tools/src/checkout_tests.rs | 497 ++++++++++++++++++ crates/tools/src/edit.rs | 1 + crates/tools/src/glob.rs | 1 + crates/tools/src/grep.rs | 1 + crates/tools/src/read.rs | 1 + crates/tools/src/write.rs | 1 + crates/workdir/CHECKOUT_PROVIDER.md | 34 +- crates/workdir/src/checkout.rs | 14 + crates/workdir/src/checkout_search_tests.rs | 10 + crates/workdir/src/external.rs | 31 +- crates/workdir/src/lib.rs | 6 +- crates/workdir/src/local.rs | 2 + crates/workdir/src/scope.rs | 6 + crates/workdir/tests/list_pagination.rs | 267 ++++++++++ crates/worker/src/checkout.rs | 376 ++++++++----- crates/worker/src/checkout_http_tests.rs | 88 +++- crates/worker/src/checkout_race_tests.rs | 58 +- crates/worker/src/checkout_wip_tests.rs | 345 +++++++++++- crates/worker/src/fs_view.rs | 2 + crates/workspace-server/src/server.rs | 1 + docs/design/checkout-wip-projection.md | 114 +++- .../development/wip-integration-validation.md | 74 ++- docs/report/t-714-ticket-read-auth-blocker.md | 29 + 29 files changed, 2297 insertions(+), 210 deletions(-) create mode 100644 crates/fs-operation/tests/list_pagination.rs create mode 100644 crates/workdir/tests/list_pagination.rs create mode 100644 docs/report/t-714-ticket-read-auth-blocker.md diff --git a/crates/fs-operation/src/lib.rs b/crates/fs-operation/src/lib.rs index 63436ed05..791aeae7b 100644 --- a/crates/fs-operation/src/lib.rs +++ b/crates/fs-operation/src/lib.rs @@ -941,6 +941,7 @@ mod tests { let list_error = run_list( &root, ListRequest { + after: None, path: FsPath::new("selected-dir").unwrap(), limit: 10, }, @@ -1008,6 +1009,7 @@ mod tests { let listed = run_list( &root, ListRequest { + after: None, path: FsPath::new("notes").unwrap(), limit: 10, }, diff --git a/crates/fs-operation/src/local.rs b/crates/fs-operation/src/local.rs index 887a09248..8885b5587 100644 --- a/crates/fs-operation/src/local.rs +++ b/crates/fs-operation/src/local.rs @@ -1,3 +1,5 @@ +use std::cmp::Ordering; +use std::collections::BinaryHeap; use std::ffi::OsString; use std::fs; use std::io::{BufReader, Read}; @@ -7,9 +9,9 @@ use sha2::{Digest, Sha256}; use crate::{ AtomicWriteMode, BoundedReadLimits, ContentHash, EditRequest, EditResult, EntryKind, - FsAccessPolicy, FsError, FsPath, ListEntry, ListRequest, ListResult, ReadBytesRequest, - ReadBytesResult, ReadRequest, ReadResult, StatRequest, StatResult, WriteRequest, WriteResult, - direct_symlink, + FsAccessPolicy, FsError, FsPath, ListCursor, ListEntry, ListRequest, ListResult, + ReadBytesRequest, ReadBytesResult, ReadRequest, ReadResult, StatRequest, StatResult, + WriteRequest, WriteResult, direct_symlink, }; const READ_BUFFER_BYTES: usize = 16 * 1024; @@ -437,12 +439,55 @@ pub fn run_edit( }) } +fn list_order( + left_kind: EntryKind, + left: &FsPath, + right_kind: EntryKind, + right: &FsPath, +) -> Ordering { + (left_kind != EntryKind::Directory) + .cmp(&(right_kind != EntryKind::Directory)) + .then_with(|| left.as_str().cmp(right.as_str())) +} + +struct RetainedListEntry(ListEntry); + +impl PartialEq for RetainedListEntry { + fn eq(&self, other: &Self) -> bool { + self.cmp(other) == Ordering::Equal + } +} +impl Eq for RetainedListEntry {} +impl Ord for RetainedListEntry { + fn cmp(&self, other: &Self) -> Ordering { + list_order(self.0.kind, &self.0.path, other.0.kind, &other.0.path) + } +} +impl PartialOrd for RetainedListEntry { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +fn list_entry_bytes(entry: &ListEntry) -> usize { + entry.path.as_str().len().saturating_add(64) +} + pub fn run_list( root: &Path, request: ListRequest, access: &dyn FsAccessPolicy, ) -> Result { let logical = request.path; + if let Some(after) = &request.after { + if after.path.is_root() + || Path::new(after.path.as_str()).parent() != Some(Path::new(logical.as_str())) + { + return Err(FsError::InvalidArgument( + "List cursor must name a direct child of the listed directory".into(), + )); + } + } access .check_cancelled() .map_err(|error| map_io(&logical, error))?; @@ -462,11 +507,15 @@ pub fn run_list( total_entries: 0, total_bytes: 0, truncated: false, + next_after: None, }); } - let mut entries = Vec::new(); + let mut entries: BinaryHeap = BinaryHeap::new(); + let mut retained_limit = request.limit.saturating_add(1); let mut retained_path_bytes = 0_usize; - let mut provider_truncated = false; + let mut total_entries = 0usize; + let mut total_bytes = 0u64; + let mut remaining_entries = 0usize; let read_dir = access .open_read_dir(&logical_base, &path) .map_err(|error| map_io(&logical, error))?; @@ -513,34 +562,77 @@ pub fn run_list( let Ok(result_path) = FsPath::new(relative) else { continue; }; - let retained = result_path.as_str().len().saturating_add(64); - if retained_path_bytes.saturating_add(retained) > crate::MAX_RESULT_PATH_BYTES { - provider_truncated = true; - break; + total_entries += 1; + total_bytes = total_bytes.saturating_add(metadata.len()); + if request.after.as_ref().is_some_and(|after| { + list_order(kind, &result_path, after.kind, &after.path) != Ordering::Greater + }) { + continue; } - retained_path_bytes = retained_path_bytes.saturating_add(retained); - entries.push(ListEntry { + remaining_entries += 1; + let candidate = RetainedListEntry(ListEntry { path: result_path, kind, size: metadata.len(), }); + if entries.len() == retained_limit { + if candidate >= *entries.peek().expect("nonempty List retention") { + continue; + } + retained_path_bytes -= list_entry_bytes(&entries.pop().unwrap().0); + } + retained_path_bytes = retained_path_bytes.saturating_add(list_entry_bytes(&candidate.0)); + entries.push(candidate); + // Evict greatest keys, never stop scanning: enumeration order is + // unspecified and a later entry may precede every retained key. + while retained_path_bytes > crate::MAX_RESULT_PATH_BYTES { + retained_path_bytes -= list_entry_bytes(&entries.pop().unwrap().0); + // Never grow cardinality again after a byte-bound eviction: a + // later short path must not leap over a discarded earlier key. + retained_limit = entries.len(); + } + if retained_limit == 0 { + return Err(FsError::InvalidArgument( + "List entry exceeds provider response bound".into(), + )); + } } - entries.sort_by(|left, right| { - let left_dir = left.kind == EntryKind::Directory; - let right_dir = right.kind == EntryKind::Directory; - right_dir - .cmp(&left_dir) - .then_with(|| left.path.as_str().cmp(right.path.as_str())) - }); - let total_entries = entries.len(); - let total_bytes = entries.iter().map(|entry| entry.size).sum(); - let truncated = provider_truncated || entries.len() > request.limit; + let mut entries: Vec<_> = entries + .into_sorted_vec() + .into_iter() + .map(|entry| entry.0) + .collect(); entries.truncate(request.limit); + // Continuation duplicates the last path on the wire; include it in the + // aggregate provider response bound too. + let mut response_bytes: usize = entries.iter().map(list_entry_bytes).sum(); + while remaining_entries > entries.len() + && entries.last().is_some_and(|last| { + response_bytes.saturating_add(list_entry_bytes(last)) > crate::MAX_RESULT_PATH_BYTES + }) + { + response_bytes -= list_entry_bytes(&entries.pop().unwrap()); + } + if request.limit > 0 && remaining_entries > 0 && entries.is_empty() { + return Err(FsError::InvalidArgument( + "List entry exceeds provider response bound".into(), + )); + } + let truncated = remaining_entries > entries.len(); + let next_after = if truncated { + entries.last().map(|entry| ListCursor { + kind: entry.kind, + path: entry.path.clone(), + }) + } else { + None + }; Ok(ListResult { entries, total_entries, total_bytes, truncated, + next_after, }) } diff --git a/crates/fs-operation/src/operation.rs b/crates/fs-operation/src/operation.rs index 3d6e612f3..a2dc6d18f 100644 --- a/crates/fs-operation/src/operation.rs +++ b/crates/fs-operation/src/operation.rs @@ -278,6 +278,22 @@ pub struct ListRequest { pub path: FsPath, #[schemars(range(min = 0, max = 9_007_199_254_740_991_usize))] pub limit: usize, + /// Exclusive live continuation in the same coordinates as returned entries. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub after: Option, +} + +/// A List ordering key: directories first, then lexical path (all other kinds +/// share one ordering group). The cursor need not still exist. Each page scans +/// the live directory: insertions before the key are not revisited, insertions +/// after it may appear, and removals disappear. Renames or kind changes can move +/// an entry across the key and cause omission or repetition; no snapshot or +/// cross-page consistency is promised. Scoped wrappers translate coordinates. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(deny_unknown_fields)] +pub struct ListCursor { + pub kind: EntryKind, + pub path: FsPath, } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] @@ -293,6 +309,11 @@ pub struct ListEntry { #[serde(deny_unknown_fields)] pub struct ListResult { pub entries: Vec, + /// Last returned key when more entries follow. A zero-limit request has no + /// continuation even when truncated. Totals describe all visible entries + /// in this scan, including those at or before `after`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub next_after: Option, #[schemars(range(min = 0, max = 9_007_199_254_740_991_usize))] pub total_entries: usize, #[schemars(range(min = 0, max = 9_007_199_254_740_991_u64))] diff --git a/crates/fs-operation/tests/list_pagination.rs b/crates/fs-operation/tests/list_pagination.rs new file mode 100644 index 000000000..cb3d14f78 --- /dev/null +++ b/crates/fs-operation/tests/list_pagination.rs @@ -0,0 +1,248 @@ +use fs_operation::{ + EntryKind, FsAccessPolicy, FsError, FsPath, ListCursor, ListRequest, ListResult, + MAX_RESULT_PATH_BYTES, run_list, +}; +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicUsize, Ordering}; + +struct Access { + root: PathBuf, + checks: AtomicUsize, + cancel_at: usize, + enumerate: bool, +} +impl Access { + fn new(root: &Path) -> Self { + Self { + root: root.to_path_buf(), + checks: AtomicUsize::new(0), + cancel_at: usize::MAX, + enumerate: true, + } + } +} +impl FsAccessPolicy for Access { + fn is_readable(&self, path: &Path) -> bool { + path.starts_with(&self.root) && path != self.root.join("denied") + } + fn is_writable(&self, _: &Path) -> bool { + false + } + fn can_enumerate_directory(&self, _: &Path, _: &Path) -> bool { + self.enumerate + } + fn check_cancelled(&self) -> std::io::Result<()> { + if self.checks.fetch_add(1, Ordering::Relaxed) >= self.cancel_at { + return Err(std::io::Error::new( + std::io::ErrorKind::Interrupted, + "cancelled", + )); + } + Ok(()) + } +} +fn path(value: &str) -> FsPath { + FsPath::new(value).unwrap() +} +fn page(root: &Path, limit: usize, after: Option) -> ListResult { + run_list( + root, + ListRequest { + path: FsPath::root(), + limit, + after, + }, + &Access::new(root), + ) + .unwrap() +} +fn names(result: &ListResult) -> Vec<&str> { + result + .entries + .iter() + .map(|entry| entry.path.as_str()) + .collect() +} + +#[test] +fn list_pages_order_directories_first_then_paths_without_duplicates() { + let root = tempfile::tempdir().unwrap(); + for name in ["z", "b", "a"] { + std::fs::write(root.path().join(name), b"xx").unwrap(); + } + for name in ["z-dir", "a-dir"] { + std::fs::create_dir(root.path().join(name)).unwrap(); + } + let full = page(root.path(), usize::MAX, None); + assert_eq!(names(&full), ["a-dir", "z-dir", "a", "b", "z"]); + let mut after = None; + let mut entries = Vec::new(); + loop { + let result = page(root.path(), 2, after); + assert!(result.entries.len() <= 2); + assert_eq!(result.total_entries, 5); + assert_eq!(result.total_bytes, full.total_bytes); + assert_eq!(result.truncated, result.next_after.is_some()); + after = result.next_after; + entries.extend(result.entries); + if after.is_none() { + break; + } + } + assert_eq!(entries, full.entries); + let zero = page(root.path(), 0, None); + assert!(zero.entries.is_empty()); + assert!(zero.truncated); + assert!(zero.next_after.is_none()); + assert_eq!(zero.total_entries, 5); + let exhausted = page( + root.path(), + 2, + Some(ListCursor { + kind: EntryKind::File, + path: path("z"), + }), + ); + assert!(exhausted.entries.is_empty()); + assert!(!exhausted.truncated); + assert!(exhausted.next_after.is_none()); + assert_eq!(exhausted.total_entries, 5); +} + +#[test] +fn list_live_cursor_survives_removal_and_only_observes_new_keys_after_it() { + let root = tempfile::tempdir().unwrap(); + for name in ["b", "d"] { + std::fs::write(root.path().join(name), b"x").unwrap(); + } + let first = page(root.path(), 1, None); + assert_eq!(names(&first), ["b"]); + std::fs::remove_file(root.path().join("b")).unwrap(); + for name in ["a", "c"] { + std::fs::write(root.path().join(name), b"xx").unwrap(); + } + // All directories sort before a file cursor, even when created later. + std::fs::create_dir(root.path().join("z-directory")).unwrap(); + let second = page(root.path(), 10, first.next_after); + assert_eq!(names(&second), ["c", "d"]); + assert_eq!(second.total_entries, 4); + assert!(!second.truncated); + assert!(second.next_after.is_none()); + std::fs::remove_file(root.path().join("d")).unwrap(); + std::fs::rename(root.path().join("a"), root.path().join("e")).unwrap(); + let moved = page( + root.path(), + 10, + Some(ListCursor { + kind: EntryKind::File, + path: path("d"), + }), + ); + assert_eq!(names(&moved), ["e"]); +} + +#[test] +fn list_path_byte_bound_returns_a_continuation_instead_of_losing_the_tail() { + let root = tempfile::tempdir().unwrap(); + let long_count = MAX_RESULT_PATH_BYTES / (240 + 64) + 10; + // A shorter late key must not leap over a longer evicted earlier key. + let count = long_count + 50; + let expected: Vec<_> = (0..count) + .map(|n| format!("{n:05}{}", "x".repeat(if n < long_count { 235 } else { 1 }))) + .collect(); + for name in expected.iter().rev() { + std::fs::write(root.path().join(name), b"x").unwrap(); + } + let first = page(root.path(), usize::MAX, None); + assert!(first.truncated); + assert_eq!(first.total_entries, count); + assert_eq!(first.total_bytes, count as u64); + let path_bytes: usize = first + .entries + .iter() + .map(|entry| entry.path.as_str().len() + 64) + .sum::() + + first.next_after.as_ref().unwrap().path.as_str().len() + + 64; + assert!(path_bytes <= MAX_RESULT_PATH_BYTES); + let next = page(root.path(), usize::MAX, first.next_after.clone()); + assert!(!next.truncated); + let got: Vec<_> = names(&first).into_iter().chain(names(&next)).collect(); + assert_eq!(got, expected.iter().map(String::as_str).collect::>()); + let small = page(root.path(), 1, None); + assert_eq!(names(&small), [expected[0].as_str()]); + assert_eq!(small.total_entries, count); +} + +#[test] +fn list_keeps_ignored_and_hidden_entries_but_respects_read_and_enumeration_authority() { + let root = tempfile::tempdir().unwrap(); + std::fs::write(root.path().join(".gitignore"), "ignored\n").unwrap(); + for name in ["ignored", ".hidden", "denied"] { + std::fs::write(root.path().join(name), b"x").unwrap(); + } + let first = page(root.path(), 2, None); + assert_eq!(names(&first), [".gitignore", ".hidden"]); + let second = page(root.path(), 2, first.next_after); + assert_eq!(names(&second), ["ignored"]); + assert_eq!(second.total_entries, 3); + let mut access = Access::new(root.path()); + access.enumerate = false; + let result = run_list( + root.path(), + ListRequest { + path: FsPath::root(), + limit: 1, + after: None, + }, + &access, + ) + .unwrap(); + assert!(result.entries.is_empty()); + assert!(!result.truncated); + assert!(result.next_after.is_none()); +} + +#[test] +fn list_cancellation_is_checked_even_after_the_page_is_full() { + let root = tempfile::tempdir().unwrap(); + for name in ["a", "b", "c"] { + std::fs::write(root.path().join(name), b"x").unwrap(); + } + let mut access = Access::new(root.path()); + access.cancel_at = 3; + let error = run_list( + root.path(), + ListRequest { + path: FsPath::root(), + limit: 1, + after: None, + }, + &access, + ) + .unwrap_err(); + assert!( + matches!(error, FsError::Io { source, .. } if source.kind() == std::io::ErrorKind::Interrupted) + ); +} + +#[test] +fn list_rejects_cursor_outside_the_requested_directory() { + let root = tempfile::tempdir().unwrap(); + for value in ["", "other/a"] { + let error = run_list( + root.path(), + ListRequest { + path: FsPath::root(), + limit: 1, + after: Some(ListCursor { + kind: EntryKind::File, + path: path(value), + }), + }, + &Access::new(root.path()), + ) + .unwrap_err(); + assert!(matches!(error, FsError::InvalidArgument(_))); + } +} diff --git a/crates/tools/README.md b/crates/tools/README.md index d8f72b3fb..dd046a3e0 100644 --- a/crates/tools/README.md +++ b/crates/tools/README.md @@ -38,6 +38,7 @@ capabilities remain authoritative. `Create` uses the `Write` capability/identity | Edit | `old_string`, `new_string`, `replace_all?` | Existing file; prior read/hash and provider validator required | | Write | `content` | Existing file only; prior read/hash and provider validator required | | Create | `path`, `content` | Bound parent directory; relative destination strictly below it; create-new, missing parents allowed | +| List | `limit?` (1..1000, default 100), `after?` (`{kind, path}`) | Bound directory; Read capability; all typed `EntryKind` cursor values accepted, path is a direct child in checkout-root-relative provider coordinates | | Glob | `pattern`, `path?` | Bound directory; optional path relative beneath it | | Grep | `pattern`, `path?`, `glob?`, `type?`, `case_insensitive?`, `-B?`, `-A?`, `-C?`, `multiline?`, `output_mode?`, `head_limit?`, `offset?` | Bound directory; same provider search/options/rendering as Tool Grep | @@ -55,6 +56,22 @@ The constructor's empty scope layers/root output are refined by provider wrapper Tools does not synthesize authority or parse/rebase rendered filenames. Ordinary Tool dispatch remains `session.glob`/`session.grep`, with its existing error mapping. +List dispatches one bounded `checkout_search` page without Bash or entry prefetch. +`CheckoutToolOutput.listing` preserves the typed `ListResult`, including +`next_after`, for Host projection into canonical Worldspace entries and cursors. +Its normal Tool output is a summary only, and `paths` is empty; every other +Operation returns `listing: None`. The Host decodes canonical cursor paths before +calling Tools, mapping the public `{kind, entry}` cursor into `{kind, path}`. +Both returned entries and the continuation must stay directly inside the bound +directory. Cursor kinds retain all typed values (`directory`, `file`, `symlink`, +`other`); ordering groups directories first and all non-directories together, +then compares paths. A truncated page must supply `next_after` matching its last +entry's ordering key; an untruncated page has no continuation. Special typed +listing entries are path coordinates, not checkout observations: direct checkout +observation remains provider File/Directory-only. Pagination is live, not a +cross-page snapshot; the same before/after bound-directory validator checks +used for search apply to List. + `CheckoutToolOutput.paths` contains typed checkout-root-relative search paths (from `GlobResult.paths` / `GrepResult.paths`) and the created path for Create. Rendered Grep text is never parsed to construct links. The optional validator is diff --git a/crates/tools/src/checkout.rs b/crates/tools/src/checkout.rs index 73e59e5e5..be2fdf19f 100644 --- a/crates/tools/src/checkout.rs +++ b/crates/tools/src/checkout.rs @@ -18,6 +18,10 @@ use crate::{ToolsError, Tracker, file_target::FileTarget}; pub struct CheckoutToolOutput { pub output: ToolOutput, pub paths: Vec, + /// Typed directory page, including its provider-coordinate continuation. + /// Special entries are path coordinates, not checked checkout observations. + /// The Host owns projection into canonical Worldspace entries and cursors. + pub listing: Option, pub validator: Option>, } @@ -47,6 +51,7 @@ pub async fn execute_checkout_tool( .unwrap_or_default(); let (capability, allowed): (_, &[&str]) = match tool_name { "Read" => (WorkdirSessionCapability::Read, &file_fields), + "List" => (WorkdirSessionCapability::Read, &["limit", "after"]), "Edit" => (WorkdirSessionCapability::Edit, &file_fields), "Write" => (WorkdirSessionCapability::Write, &file_fields), "Create" => (WorkdirSessionCapability::Write, &["path", "content"]), @@ -132,7 +137,7 @@ pub async fn execute_checkout_tool( crate::write::execute_write(target, tracker, params.content, Some(destination), ctx) .await } - "Glob" | "Grep" => { + "List" | "Glob" | "Grep" => { let before = selected .session .checkout_observe(path.clone()) @@ -141,7 +146,10 @@ pub async fn execute_checkout_tool( if before.validator != validator { return Err(stale_search()); } - let mut result = if tool_name == "Glob" { + let mut result = if tool_name == "List" { + let params: ListParams = decode(Value::Object(arguments), tool_name)?; + execute_list(selected.session.clone(), path.clone(), params).await? + } else if tool_name == "Glob" { let params: crate::glob::GlobParams = decode(Value::Object(arguments), tool_name)?; check_pattern(¶ms.pattern)?; let search_path = @@ -191,6 +199,120 @@ pub async fn execute_checkout_tool( } } +/// Native List has no path override. `after.path` is already decoded by the Host +/// into checkout-root-relative provider coordinates, not directory-relative. +#[derive(serde::Deserialize)] +#[serde(deny_unknown_fields)] +struct ListParams { + #[serde(default = "default_list_limit")] + limit: usize, + #[serde(default)] + after: Option, +} + +fn default_list_limit() -> usize { + 100 +} + +async fn execute_list( + session: workdir::WorkdirSessionHandle, + path: WorkdirPath, + params: ListParams, +) -> Result { + if !(1..=1000).contains(¶ms.limit) { + return Err(ToolError::InvalidArgument( + "List limit must be between 1 and 1000".into(), + )); + } + if let Some(after) = ¶ms.after { + if !is_direct_child(&path, &after.path) { + return Err(ToolError::InvalidArgument( + "List after must name a direct child of the bound directory".into(), + )); + } + } + let result = session + .checkout_search(workdir::CheckoutSearchRequest::new( + workdir::CheckoutSearchOperation::List(workdir::ListRequest { + path: path.clone(), + limit: params.limit, + after: params.after, + }), + )) + .await + .map_err(crate::file_target::checked_error)?; + let listing = match result { + workdir::CheckoutSearchResult::List(listing) => listing, + _ => return Err(crate::file_target::readonly_result_error("List")), + }; + // Fail closed before the Host can publish typed links to provider results. + if listing.entries.len() > params.limit + || listing + .entries + .iter() + .any(|entry| !is_direct_child(&path, &entry.path)) + || listing + .next_after + .as_ref() + .is_some_and(|after| !is_direct_child(&path, &after.path)) + { + return Err(crate::file_target::readonly_result_error( + "List page escaped bound directory or exceeded its limit", + )); + } + // The provider ordering key is directory/non-directory group plus path. + // Preserve the typed kind; special entries remain continuable without + // acquiring a File/Directory-only checkout observation for them. + let valid_continuation = match (&listing.next_after, listing.entries.last()) { + (Some(after), Some(last)) if listing.truncated => { + after.path == last.path + && (after.kind == workdir::EntryKind::Directory) + == (last.kind == workdir::EntryKind::Directory) + } + (None, _) => !listing.truncated, + _ => false, + }; + if !valid_continuation { + return Err(crate::file_target::readonly_result_error( + "List continuation does not match the truncated page's last ordering key", + )); + } + let summary = format!( + "Listed {} of {} entries in {}{}", + listing.entries.len(), + listing.total_entries, + path, + if listing.truncated { + " (truncated)" + } else { + "" + }, + ); + Ok(CheckoutToolOutput { + output: ToolOutput { + summary, + content: None, + attachments: Vec::new(), + }, + paths: Vec::new(), + listing: Some(listing), + validator: None, + }) +} + +fn is_direct_child(base: &WorkdirPath, path: &WorkdirPath) -> bool { + let Ok(logical) = WorkdirPath::new(path.as_str()) else { + return false; + }; + let parent = std::path::Path::new(logical.as_str()).parent(); + let expected = if base == &WorkdirPath::root() { + std::path::Path::new("") + } else { + std::path::Path::new(base.as_str()) + }; + logical != WorkdirPath::root() && parent == Some(expected) +} + fn decode(arguments: Value, name: &str) -> Result { crate::error::decode_file_value(arguments, name) } diff --git a/crates/tools/src/checkout_tests.rs b/crates/tools/src/checkout_tests.rs index d11203ed3..d8ec7b74e 100644 --- a/crates/tools/src/checkout_tests.rs +++ b/crates/tools/src/checkout_tests.rs @@ -85,6 +85,326 @@ impl Fixture { } } +#[derive(Debug)] +struct ListProvider { + workdir: workdir::Workdir, + capabilities: workdir::WorkdirSessionCapabilities, + result: workdir::CheckoutSearchResult, + change_validator: bool, + expected_after: Option, + searched: std::sync::atomic::AtomicBool, +} + +// This boundary adapter exposes only checked metadata/search. Any file content +// read, ordinary list fallback, or command execution is a test failure. +#[async_trait::async_trait] +impl workdir::WorkdirSession for ListProvider { + fn workdir(&self) -> &workdir::Workdir { + &self.workdir + } + fn capabilities(&self) -> workdir::WorkdirSessionCapabilities { + self.capabilities + } + async fn checkout_observe( + &self, + path: WorkdirPath, + ) -> Result { + assert_eq!( + path.as_str(), + "src", + "List observes only its bound directory" + ); + Ok(workdir::CheckoutObservation { + path, + kind: workdir::EntryKind::Directory, + size: 0, + validator: vec![u8::from( + self.change_validator && self.searched.load(std::sync::atomic::Ordering::SeqCst), + )], + capabilities: self.capabilities, + }) + } + async fn checkout_search( + &self, + request: workdir::CheckoutSearchRequest, + ) -> Result { + let workdir::CheckoutSearchOperation::List(list) = request.operation else { + panic!("native List must dispatch a typed List request"); + }; + assert_eq!(list.after, self.expected_after); + assert!( + !self + .searched + .swap(true, std::sync::atomic::Ordering::SeqCst), + "List must not prefetch another page" + ); + Ok(self.result.clone()) + } + async fn stat( + &self, + _: workdir::StatRequest, + ) -> Result { + panic!("native List must not call stat"); + } + async fn read( + &self, + _: workdir::ReadRequest, + ) -> Result { + panic!("native List must not read file content"); + } + async fn write( + &self, + _: workdir::WriteRequest, + ) -> Result { + panic!("native List must not write"); + } + async fn edit( + &self, + _: workdir::EditRequest, + ) -> Result { + panic!("native List must not edit"); + } + async fn list( + &self, + _: workdir::ListRequest, + ) -> Result { + panic!("native List must use checkout_search, not ordinary list"); + } + async fn glob( + &self, + _: workdir::GlobRequest, + ) -> Result { + panic!("native List must not glob"); + } + async fn grep( + &self, + _: workdir::GrepRequest, + ) -> Result { + panic!("native List must not grep"); + } + async fn start_command( + &self, + _: workdir::CommandRequest, + ) -> Result { + panic!("native List must not execute commands"); + } + async fn command_status( + &self, + _: workdir::CommandHandle, + ) -> Result { + panic!("native List must not use commands"); + } + async fn command_output( + &self, + _: workdir::CommandOutputRequest, + ) -> Result { + panic!("native List must not use commands"); + } + async fn cancel_command(&self, _: workdir::CommandHandle) -> Result<(), workdir::WorkdirError> { + panic!("native List must not use commands"); + } + async fn close(&self) -> Result<(), workdir::WorkdirError> { + Ok(()) + } +} + +async fn list_with_provider( + capabilities: workdir::WorkdirSessionCapabilities, + result: workdir::CheckoutSearchResult, + change_validator: bool, +) -> Result { + list_with_provider_after(capabilities, result, change_validator, None).await +} + +async fn list_with_provider_after( + capabilities: workdir::WorkdirSessionCapabilities, + result: workdir::CheckoutSearchResult, + change_validator: bool, + after: Option, +) -> Result { + let router = Arc::new(WorkdirSessionRouter::new()); + router + .attach( + WorkdirAttachmentAlias::new("selected").unwrap(), + Arc::new(ListProvider { + workdir: workdir::Workdir::new("list-test"), + capabilities, + result, + change_validator, + expected_after: after.clone(), + searched: std::sync::atomic::AtomicBool::new(false), + }), + ) + .unwrap(); + let generation = router.resolve(Some("selected")).unwrap().generation; + execute_checkout_tool( + router, + Tracker::new(), + "selected", + generation, + WorkdirPath::new("src").unwrap(), + vec![0], + "List", + json!({"limit": 1, "after": after}), + Default::default(), + ) + .await +} + +fn list_page() -> workdir::ListResult { + workdir::ListResult { + entries: vec![workdir::ListEntry { + path: WorkdirPath::new("src/a.txt").unwrap(), + kind: workdir::EntryKind::File, + size: 5, + }], + total_entries: 2, + total_bytes: 10, + truncated: true, + next_after: Some(fs_operation::ListCursor { + kind: workdir::EntryKind::File, + path: WorkdirPath::new("src/a.txt").unwrap(), + }), + } +} + +#[tokio::test] +async fn checkout_list_uses_read_capability_and_checked_search_without_prefetch() { + let read_only = workdir::WorkdirSessionCapabilities::EMPTY.with(WorkdirSessionCapability::Read); + let page = list_page(); + let result = list_with_provider( + read_only, + workdir::CheckoutSearchResult::List(page.clone()), + false, + ) + .await + .unwrap(); + assert_eq!(result.listing, Some(page)); + assert_eq!(result.validator, Some(vec![0])); + let no_read = workdir::WorkdirSessionCapabilities::EMPTY.with(WorkdirSessionCapability::Glob); + let error = list_with_provider( + no_read, + workdir::CheckoutSearchResult::List(list_page()), + false, + ) + .await + .err() + .unwrap(); + assert!(matches!(error, ToolError::InvalidArgument(_))); + assert!(error.to_string().contains("Read")); +} + +#[tokio::test] +async fn checkout_list_rejects_changed_post_search_directory_validator() { + let error = list_with_provider( + workdir::WorkdirSessionCapabilities::READ_ONLY, + workdir::CheckoutSearchResult::List(list_page()), + true, + ) + .await + .err() + .unwrap(); + assert_checkout_stale(error); +} + +#[tokio::test] +async fn checkout_list_rejects_provider_pages_that_escape_or_exceed_bounds() { + let mut outside = list_page(); + outside.entries[0].path = WorkdirPath::new("outside.txt").unwrap(); + let mut nested = list_page(); + nested.entries[0].path = WorkdirPath::new("src/nested/a.txt").unwrap(); + let mut cursor_outside = list_page(); + cursor_outside.next_after.as_mut().unwrap().path = WorkdirPath::new("outside.txt").unwrap(); + let mut cursor_kind = list_page(); + cursor_kind.next_after.as_mut().unwrap().kind = workdir::EntryKind::Directory; + let mut oversized = list_page(); + oversized.entries.push(oversized.entries[0].clone()); + for result in [ + workdir::CheckoutSearchResult::List(outside), + workdir::CheckoutSearchResult::List(nested), + workdir::CheckoutSearchResult::List(cursor_outside), + workdir::CheckoutSearchResult::List(cursor_kind), + workdir::CheckoutSearchResult::List(oversized), + workdir::CheckoutSearchResult::Glob(workdir::GlobResult { + paths: vec![], + truncated: false, + }), + ] { + let error = list_with_provider( + workdir::WorkdirSessionCapabilities::READ_ONLY, + result, + false, + ) + .await + .err() + .unwrap(); + assert_checkout_code(error, "checkout_unavailable"); + } +} + +#[tokio::test] +async fn checkout_list_special_entry_cursors_preserve_kind_and_continue_the_page() { + // Provider-only special entries stay path coordinates: the adapter forbids + // content reads and does not offer direct observations for these entries. + for kind in [workdir::EntryKind::Other, workdir::EntryKind::Symlink] { + let mut first_page = list_page(); + first_page.entries[0].kind = kind; + first_page.next_after.as_mut().unwrap().kind = kind; + let first = list_with_provider( + workdir::WorkdirSessionCapabilities::READ_ONLY, + workdir::CheckoutSearchResult::List(first_page), + false, + ) + .await + .unwrap() + .listing + .unwrap(); + assert_eq!(first.entries[0].kind, kind); + let after = first.next_after.unwrap(); + assert_eq!(after.kind, kind); + let mut second_page = list_page(); + second_page.entries[0].path = WorkdirPath::new("src/z.txt").unwrap(); + second_page.truncated = false; + second_page.next_after = None; + let second = list_with_provider_after( + workdir::WorkdirSessionCapabilities::READ_ONLY, + workdir::CheckoutSearchResult::List(second_page.clone()), + false, + Some(after), + ) + .await + .unwrap() + .listing + .unwrap(); + assert_eq!(second, second_page); + } +} + +#[tokio::test] +async fn checkout_list_rejects_missing_or_mismatched_truncated_continuations() { + let mut missing = list_page(); + missing.next_after = None; + let mut empty = list_page(); + empty.entries.clear(); + let mut wrong_path = list_page(); + wrong_path.next_after.as_mut().unwrap().path = WorkdirPath::new("src/z.txt").unwrap(); + let mut wrong_group = list_page(); + wrong_group.next_after.as_mut().unwrap().kind = workdir::EntryKind::Directory; + let mut untruncated = list_page(); + untruncated.truncated = false; + for page in [missing, empty, wrong_path, wrong_group, untruncated] { + let error = list_with_provider( + workdir::WorkdirSessionCapabilities::READ_ONLY, + workdir::CheckoutSearchResult::List(page), + false, + ) + .await + .err() + .unwrap(); + assert_checkout_code(error, "checkout_unavailable"); + } +} + fn assert_checkout_stale(error: ToolError) { assert_checkout_code(error, "checkout_stale"); } @@ -557,6 +877,183 @@ async fn checkout_search_typed_paths_and_normal_rendering() { let normal = fixture.normal("Grep", json!({"target_workdir": "main", "path": "src", "pattern": "hit", "output_mode": mode, "-C": 1})).await.unwrap(); assert_eq!(grep.output.summary, normal.summary); assert_eq!(grep.output.content, normal.content); + assert!(grep.listing.is_none()); assert!(!grep.output.content.unwrap().contains("outside.txt")); } } + +#[tokio::test] +async fn checkout_list_returns_one_typed_page_and_provider_coordinate_continuation() { + let fixture = Fixture::new(); + std::fs::create_dir_all(fixture.dir.path().join("src/nested")).unwrap(); + std::fs::write(fixture.dir.path().join("src/a.txt"), "alpha").unwrap(); + std::fs::write(fixture.dir.path().join("src/z.txt"), "z").unwrap(); + std::fs::write(fixture.dir.path().join("outside.txt"), "outside").unwrap(); + let (_, validator) = fixture.observation("main", "src").await; + let first = fixture + .native("main", "src", "List", json!({"limit": 1})) + .await + .unwrap(); + assert!(first.paths.is_empty()); + assert!(first.output.content.is_none()); + assert!(first.output.summary.contains("Listed 1 of 3 entries")); + assert!(first.output.summary.contains("truncated")); + assert_eq!(first.validator, Some(validator)); + let page = first.listing.unwrap(); + assert_eq!(page.entries.len(), 1); + assert_eq!(page.entries[0].path.as_str(), "src/nested"); + assert_eq!(page.entries[0].kind, workdir::EntryKind::Directory); + assert!(page.truncated); + let total_bytes = page.total_bytes; + assert!(total_bytes >= 6); + let cursor = page.next_after.unwrap(); + assert_eq!(cursor.path.as_str(), "src/nested"); + let second = fixture + .native("main", "src", "List", json!({"after": cursor})) + .await + .unwrap(); + let page = second.listing.unwrap(); + assert_eq!( + page.entries + .iter() + .map(|entry| entry.path.as_str()) + .collect::>(), + ["src/a.txt", "src/z.txt"] + ); + assert_eq!(page.total_entries, 3); + assert_eq!(page.total_bytes, total_bytes); + assert!(!page.truncated); + assert!(page.next_after.is_none()); + assert_eq!(fixture.tracker.change_stat(), crate::ChangeStat::default()); +} + +#[tokio::test] +async fn checkout_list_root_file_cursor_stays_on_selected_attachment() { + let fixture = Fixture::new(); + std::fs::write(fixture.dir.path().join("a.txt"), "a").unwrap(); + std::fs::write(fixture.dir.path().join("z.txt"), "z").unwrap(); + let other = TempDir::new().unwrap(); + std::fs::write(other.path().join("wrong.txt"), "wrong").unwrap(); + fixture + .router + .attach( + WorkdirAttachmentAlias::new("other").unwrap(), + Arc::new(LocalWorkdirSession::new( + Scope::writable(other.path()).unwrap(), + other.path().to_path_buf(), + )), + ) + .unwrap(); + let first = fixture + .native("main", ".", "List", json!({"limit": 1})) + .await + .unwrap() + .listing + .unwrap(); + let cursor = first.next_after.unwrap(); + assert_eq!(cursor.kind, workdir::EntryKind::File); + assert_eq!(cursor.path.as_str(), "a.txt"); + let second = fixture + .native("main", ".", "List", json!({"after": cursor})) + .await + .unwrap() + .listing + .unwrap(); + assert_eq!(second.entries.len(), 1); + assert_eq!(second.entries[0].path.as_str(), "z.txt"); + let other_page = fixture + .native("other", ".", "List", json!({})) + .await + .unwrap() + .listing + .unwrap(); + assert_eq!(other_page.entries.len(), 1); + assert_eq!(other_page.entries[0].path.as_str(), "wrong.txt"); +} + +#[tokio::test] +async fn checkout_list_defaults_to_100_and_accepts_both_limit_bounds() { + let fixture = Fixture::new(); + for index in 0..101 { + std::fs::write(fixture.dir.path().join(format!("{index:03}.txt")), "").unwrap(); + } + for (arguments, count, truncated) in [ + (json!({}), 100, true), + (json!({"limit": 1}), 1, true), + (json!({"limit": 1000}), 101, false), + ] { + let result = fixture + .native("main", ".", "List", arguments) + .await + .unwrap(); + let page = result.listing.unwrap(); + assert_eq!(page.entries.len(), count); + assert_eq!(page.truncated, truncated); + assert_eq!(page.next_after.is_some(), truncated); + } +} + +#[tokio::test] +async fn checkout_list_rejects_route_overrides_malformed_limits_and_nonchild_cursors() { + let fixture = Fixture::new(); + std::fs::create_dir(fixture.dir.path().join("src")).unwrap(); + for arguments in [ + json!({"path": "."}), + json!({"target_workdir": "other"}), + json!({"generation": 1}), + json!({"validator": []}), + json!({"limit": 0}), + json!({"limit": 1001}), + json!({"limit": -1}), + json!({"limit": 1.5}), + json!({"limit": "1"}), + json!({"limit": null}), + json!({"after": "src/a.txt"}), + json!({"after": {"path": "src/a.txt"}}), + json!({"after": {"kind": "file", "path": "src/a.txt", "extra": 1}}), + json!({"after": {"kind": "bogus", "path": "src/a.txt"}}), + json!({"after": {"kind": "file", "path": "src"}}), + json!({"after": {"kind": "directory", "path": "src/nested/deep"}}), + json!({"after": {"kind": "file", "path": "src-other/a.txt"}}), + json!({"after": {"kind": "file", "path": "a.txt"}}), + json!({"after": {"kind": "file", "path": "/src/a.txt"}}), + json!({"after": {"kind": "file", "path": "src/../a.txt"}}), + json!({"after": {"kind": "file", "path": "."}}), + ] { + let error = fixture + .native("main", "src", "List", arguments.clone()) + .await + .err() + .unwrap(); + assert!( + matches!(error, ToolError::InvalidArgument(_)), + "{arguments}: {error}" + ); + } +} + +#[tokio::test] +async fn checkout_list_requires_current_generation_and_directory_validator() { + let fixture = Fixture::new(); + let (generation, validator) = fixture.observation("main", ".").await; + // No timing dependency: make the supplied observation explicitly stale. + let mut stale_validator = validator.clone(); + stale_validator.push(0); + for (generation, validator) in [(generation, stale_validator), (generation + 1, validator)] { + let error = execute_checkout_tool( + fixture.router.clone(), + fixture.tracker.clone(), + "main", + generation, + WorkdirPath::root(), + validator, + "List", + json!({}), + Default::default(), + ) + .await + .err() + .unwrap(); + assert_checkout_stale(error); + } +} diff --git a/crates/tools/src/edit.rs b/crates/tools/src/edit.rs index 805a83087..8f3e8e947 100644 --- a/crates/tools/src/edit.rs +++ b/crates/tools/src/edit.rs @@ -110,6 +110,7 @@ pub(crate) async fn execute_edit( attachments: Vec::new(), }, paths: Vec::new(), + listing: None, validator, }) } diff --git a/crates/tools/src/glob.rs b/crates/tools/src/glob.rs index fe65fc36a..509d5f3ea 100644 --- a/crates/tools/src/glob.rs +++ b/crates/tools/src/glob.rs @@ -93,6 +93,7 @@ pub(crate) async fn execute_glob( attachments: Vec::new(), }, paths: result.paths, + listing: None, validator: None, }) } diff --git a/crates/tools/src/grep.rs b/crates/tools/src/grep.rs index 85257a8cf..1fa48343a 100644 --- a/crates/tools/src/grep.rs +++ b/crates/tools/src/grep.rs @@ -147,6 +147,7 @@ pub(crate) async fn execute_grep( attachments: Vec::new(), }, paths: result.paths, + listing: None, validator: None, }) } diff --git a/crates/tools/src/read.rs b/crates/tools/src/read.rs index f9d49ab1f..e32d2389a 100644 --- a/crates/tools/src/read.rs +++ b/crates/tools/src/read.rs @@ -108,6 +108,7 @@ pub(crate) async fn execute_read( attachments: Vec::new(), }, paths: Vec::new(), + listing: None, validator, }) } diff --git a/crates/tools/src/write.rs b/crates/tools/src/write.rs index c6f083cba..a79b01713 100644 --- a/crates/tools/src/write.rs +++ b/crates/tools/src/write.rs @@ -125,6 +125,7 @@ pub(crate) async fn execute_write( attachments: Vec::new(), }, paths: create_path.into_iter().collect(), + listing: None, validator, }) } diff --git a/crates/workdir/CHECKOUT_PROVIDER.md b/crates/workdir/CHECKOUT_PROVIDER.md index 0e7a9827b..119702002 100644 --- a/crates/workdir/CHECKOUT_PROVIDER.md +++ b/crates/workdir/CHECKOUT_PROVIDER.md @@ -101,6 +101,37 @@ Host generation reuse, native permission identity, read-history updates, model Operation publication, transport hosting (Runtime/Backend/External), and durable Ticket/MR review remain the parent integration's responsibilities. +## Bounded live List pagination + +`ListRequest { path, limit, after: Option }` accepts an exclusive +ordering key `ListCursor { kind: EntryKind, path: FsPath }`. Directories sort +first, then paths sort lexically within the directory/non-directory groups; +File, Symlink and Other share the non-directory group. `ListResult.next_after` +is the last returned key when another page remains. Reuse that key unchanged +with the same directory and output root. Cursor paths must name a direct child +in the result coordinate frame; they are not statted and need not still exist. +The optional fields default to absent and are omitted from JSON when absent. + +The shared provider engine retains at most `limit + 1` candidates, additionally +bounded by the 1 MiB aggregate path-response ceiling (including continuation), +while scanning the directory once. It never stops merely because the page is +full: enumeration order cannot determine the retained prefix. `total_entries` +and `total_bytes` describe all visible entries in the current scan, including +keys at/before `after`, not only the page or remaining suffix. A byte-bounded +page may contain fewer than `limit` entries and still provide continuation. +A zero-limit request returns totals and truncation but no continuation. +List uses Read capability and read/enumeration scope, and **does not ignore-filter** +`.gitignore`/`.ignore` entries or hide dotfiles; Glob/Grep keep their own policy. +Cancellation and the 100,000-entry traversal ceiling apply to the complete scan, +including excluded entries. External DTOs retain their existing item/path bounds +and also validate cursor paths and continuation response bytes. + +Each page observes a live directory, not a snapshot, and no cross-page +consistency is promised. Insertions after the cursor may appear; earlier keys +are not revisited. Removed entries disappear, including the cursor entry. +Renames or kind changes can move an entry across the cursor and cause omission +or repetition. Even one scan is not transactional against external editors. + ## Scoped checkout search boundary `CheckoutSearchRequest::new(CheckoutSearchOperation::{List, Glob, Grep}(request))` @@ -110,7 +141,8 @@ are `scope_layers: Vec>` and `output_root: WorkdirPath Rules union within each layer; layers intersect with each other, the current provider authority, and the output-root boundary. Wrappers contribute their own scope; caller-supplied layers can only narrow, never replace it. Each wrapper -translates all incoming coordinates once and forwards the composed output root. +translates incoming provider coordinates once and forwards the composed output root. +List cursors remain output-root-relative, just like result paths. Provider-rendered Grep text and every typed result path are already relative to that final root; wrappers must not prefix cwd onto returned paths again. diff --git a/crates/workdir/src/checkout.rs b/crates/workdir/src/checkout.rs index f524ae61c..d6592df2b 100644 --- a/crates/workdir/src/checkout.rs +++ b/crates/workdir/src/checkout.rs @@ -167,6 +167,19 @@ impl CheckoutSearchRequest { } let valid = match &self.operation { CheckoutSearchOperation::List(r) => { + if let Some(after) = &r.after { + validate_path(&after.path)?; + if after.path.is_root() + || std::path::Path::new(after.path.as_str()).parent() + != std::path::Path::new(r.path.as_str()) + .strip_prefix(self.output_root.as_str()) + .ok() + { + return Err(WorkdirError::InvalidArgument( + "List cursor must name a direct child of the listed directory".into(), + )); + } + } r.limit <= crate::external::MAX_EXTERNAL_RESULT_ITEMS } CheckoutSearchOperation::Glob(r) => { @@ -793,6 +806,7 @@ mod tests { for provider in [&local as &dyn WorkdirSession, &external] { let listed = provider .list(ListRequest { + after: None, path: WorkdirPath::root(), limit: 100, }) diff --git a/crates/workdir/src/checkout_search_tests.rs b/crates/workdir/src/checkout_search_tests.rs index 0de5ab0db..44c81488e 100644 --- a/crates/workdir/src/checkout_search_tests.rs +++ b/crates/workdir/src/checkout_search_tests.rs @@ -60,6 +60,7 @@ fn fixture() -> tempfile::TempDir { async fn verify_sparse(session: &dyn WorkdirSession) { let listed = session .list(ListRequest { + after: None, path: WorkdirPath::root(), limit: 100, }) @@ -75,6 +76,7 @@ async fn verify_sparse(session: &dyn WorkdirSession) { ); let empty = session .list(ListRequest { + after: None, path: path("denied"), limit: 100, }) @@ -108,6 +110,7 @@ async fn verify_sparse(session: &dyn WorkdirSession) { let CheckoutSearchResult::List(native) = session .checkout_search(CheckoutSearchRequest::new(CheckoutSearchOperation::List( ListRequest { + after: None, path: WorkdirPath::root(), limit: 100, }, @@ -161,6 +164,7 @@ async fn checkout_search_nested_sparse_nonrecursive_scopes_rebase_and_exclude_co .tool_session() .checkout_search(CheckoutSearchRequest::new(CheckoutSearchOperation::List( ListRequest { + after: None, path: WorkdirPath::root(), limit: 100 } @@ -196,6 +200,7 @@ async fn checkout_search_composes_actual_nested_wrappers_and_output_root() { let CheckoutSearchResult::List(result) = stacked .checkout_search(CheckoutSearchRequest::new(CheckoutSearchOperation::List( ListRequest { + after: None, path: path("keep"), limit: 100, }, @@ -229,6 +234,7 @@ async fn checkout_search_composes_actual_nested_wrappers_and_output_root() { let entries = second .tool_session() .list(ListRequest { + after: None, path: WorkdirPath::root(), limit: 100, }) @@ -246,6 +252,7 @@ async fn checkout_search_composes_actual_nested_wrappers_and_output_root() { second .tool_session() .list(ListRequest { + after: None, path: path("deep"), limit: 100 }) @@ -292,6 +299,7 @@ async fn checkout_search_scope_layers_intersect_before_list_and_grep() { root.path().to_path_buf(), ); let mut request = CheckoutSearchRequest::new(CheckoutSearchOperation::List(ListRequest { + after: None, path: path("child"), limit: 100, })); @@ -325,6 +333,7 @@ async fn checkout_search_wire_dto_bounds_and_provider_capabilities() { root.path().to_path_buf(), ); let request = CheckoutSearchRequest::new(CheckoutSearchOperation::List(ListRequest { + after: None, path: path("child/keep"), limit: 100, })); @@ -795,6 +804,7 @@ async fn checkout_search_preserves_current_provider_scope_and_rejects_symlink_es let CheckoutSearchResult::List(result) = source .checkout_search(CheckoutSearchRequest { operation: CheckoutSearchOperation::List(ListRequest { + after: None, path: path("child/denied"), limit: 100, }), diff --git a/crates/workdir/src/external.rs b/crates/workdir/src/external.rs index 1b37cda23..1d91c20a3 100644 --- a/crates/workdir/src/external.rs +++ b/crates/workdir/src/external.rs @@ -155,7 +155,11 @@ fn validate_external_operation(operation: &WorkdirSessionOperation) -> Result<() <= MAX_EXTERNAL_SCOPE_RULES && validate_external_operation(&match &request.operation { crate::CheckoutSearchOperation::List(r) => { - WorkdirSessionOperation::List(r.clone()) + // request.validate() checks the output_root-relative + // cursor; validate the provider-coordinate path here. + let mut list = r.clone(); + list.after = None; + WorkdirSessionOperation::List(list) } crate::CheckoutSearchOperation::Glob(r) => { WorkdirSessionOperation::Glob(r.clone()) @@ -212,7 +216,14 @@ fn validate_external_operation(operation: &WorkdirSessionOperation) -> Result<() .contains(&request.max_bytes) } WorkdirSessionOperation::List(request) => { - is_root_relative(&request.path) && request.limit <= MAX_EXTERNAL_RESULT_ITEMS + is_root_relative(&request.path) + && request.limit <= MAX_EXTERNAL_RESULT_ITEMS + && request.after.as_ref().is_none_or(|after| { + is_root_relative(&after.path) + && !after.path.is_root() + && Path::new(after.path.as_str()).parent() + == Some(Path::new(request.path.as_str())) + }) } WorkdirSessionOperation::Glob(request) => { is_root_relative(&request.path) @@ -395,7 +406,20 @@ fn validate_external_result(result: &WorkdirSessionOperationResult) -> Result<() .entries .iter() .all(|entry| is_root_relative(&entry.path)) - && result_paths_fit(result.entries.iter().map(|entry| &entry.path)) + && result.next_after.as_ref().is_none_or(|after| { + is_root_relative(&after.path) + && result.truncated + && result.entries.last().is_some_and(|entry| { + entry.kind == after.kind && entry.path == after.path + }) + }) + && result_paths_fit( + result + .entries + .iter() + .map(|entry| &entry.path) + .chain(result.next_after.iter().map(|after| &after.path)), + ) } WorkdirSessionOperationResult::Glob(result) => { result.paths.len() <= MAX_EXTERNAL_RESULT_ITEMS @@ -917,6 +941,7 @@ mod tests { assert!(serde_json::from_value::(nested).is_err()); let unbounded = WorkdirSessionOperation::List(ListRequest { + after: None, path: WorkdirPath::root(), limit: MAX_EXTERNAL_RESULT_ITEMS + 1, }); diff --git a/crates/workdir/src/lib.rs b/crates/workdir/src/lib.rs index 29f265784..f46126384 100644 --- a/crates/workdir/src/lib.rs +++ b/crates/workdir/src/lib.rs @@ -33,9 +33,9 @@ use tokio::sync::broadcast; pub use fs_operation::{ BoundedReadLimits, ContentHash, EditRequest, EditResult, EntryKind, FsPath as WorkdirPath, - GlobRequest, GlobResult, GrepOutputMode, GrepRequest, GrepResult, ListEntry, ListRequest, - ListResult, ReadBytesRequest, ReadBytesResult, ReadRequest, ReadResult, StatRequest, - StatResult, WriteRequest, WriteResult, + GlobRequest, GlobResult, GrepOutputMode, GrepRequest, GrepResult, ListCursor, ListEntry, + ListRequest, ListResult, ReadBytesRequest, ReadBytesResult, ReadRequest, ReadResult, + StatRequest, StatResult, WriteRequest, WriteResult, }; pub use http::dispatch_workdir_session_operation; pub use local::{ diff --git a/crates/workdir/src/local.rs b/crates/workdir/src/local.rs index 417f6fe88..2d619655a 100644 --- a/crates/workdir/src/local.rs +++ b/crates/workdir/src/local.rs @@ -2695,6 +2695,7 @@ mod tests { let listed = WorkdirSession::list( &workdir, ListRequest { + after: None, path: WorkdirPath::new("notes").unwrap(), limit: 10, }, @@ -3385,6 +3386,7 @@ mod tests { let list = WorkdirSession::list( &workdir, ListRequest { + after: None, path: WorkdirPath::new("yoi.local").unwrap(), limit: 10, }, diff --git a/crates/workdir/src/scope.rs b/crates/workdir/src/scope.rs index 2f798d4b5..e36e3cd88 100644 --- a/crates/workdir/src/scope.rs +++ b/crates/workdir/src/scope.rs @@ -1005,6 +1005,9 @@ impl WorkdirSession for ScopedWorkdirSession { .await?; self.ensure_read(&path, request.operation.capability())?; *request.operation.path_mut() = path; + // List cursors stay output_root-relative, just like source results. + // Composing output_root below handles their coordinates without stat + // of a key that may have disappeared between live pages. request.output_root = self.resolve_path(&request.output_root)?; for layer in &mut request.scope_layers { for rule in layer { @@ -1873,6 +1876,7 @@ mod tests { assert_eq!( parent .list(ListRequest { + after: None, path: fs_path("child"), limit: 100 }) @@ -1890,6 +1894,7 @@ mod tests { child .tool_session() .list(ListRequest { + after: None, path: FsPath::root(), limit: 100 }) @@ -1901,6 +1906,7 @@ mod tests { .tool_session() .checkout_search(crate::CheckoutSearchRequest::new( crate::CheckoutSearchOperation::List(ListRequest { + after: None, path: FsPath::root(), limit: 100 }) diff --git a/crates/workdir/tests/list_pagination.rs b/crates/workdir/tests/list_pagination.rs new file mode 100644 index 000000000..8d168b028 --- /dev/null +++ b/crates/workdir/tests/list_pagination.rs @@ -0,0 +1,267 @@ +#![cfg(target_os = "linux")] +use std::sync::Arc; +use workdir::external::{ + ExternalWorkdirOperation, ExternalWorkdirOperationResult, MAX_EXTERNAL_RESULT_ITEMS, +}; +use workdir::http::{WorkdirSessionOperation as Op, WorkdirSessionOperationResult as Output}; +use workdir::*; + +fn path(value: &str) -> WorkdirPath { + WorkdirPath::new(value).unwrap() +} +fn rule(value: &str, recursive: bool) -> WorkdirToolScopeRule { + WorkdirToolScopeRule { + target: path(value), + recursive, + permission: WorkdirToolScopePermission::Read, + symlink_policy: manifest::SymlinkPolicy::Logical, + } +} +fn scope(rules: Vec, cwd: &str) -> WorkdirToolScope { + WorkdirToolScope { + rules, + cwd: path(cwd), + command: false, + } +} +fn request(value: &str, limit: usize, after: Option) -> ListRequest { + ListRequest { + path: path(value), + limit, + after, + } +} +fn names(result: &ListResult) -> Vec<&str> { + result + .entries + .iter() + .map(|entry| entry.path.as_str()) + .collect() +} + +#[tokio::test] +async fn list_scoped_external_pages_rebase_cursor_and_preserve_sparse_authority() { + let root = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(root.path().join("child/deep")).unwrap(); + for name in ["child/a", "child/z", "child/deep/secret", "outside"] { + std::fs::write(root.path().join(name), b"x").unwrap(); + } + let source = LocalWorkdirSession::external_read_only( + Workdir::new("pagination"), + root.path(), + BoundedReadLimits::EXTERNAL_DEFAULT, + ) + .unwrap(); + let broker = WorkdirToolBroker::new(Arc::new(source)); + let lease = broker + .scope(scope(vec![rule("child", false)], "child")) + .await + .unwrap(); + // Two actual wrappers must not duplicate child/ in the continuation. + let stacked = WorkdirToolBroker::new(lease.tool_session()); + let first = stacked.list(request("", 1, None)).await.unwrap(); + assert_eq!(names(&first), ["deep"]); + assert_eq!(first.next_after.as_ref().unwrap().path, path("deep")); + assert_eq!( + first.next_after.as_ref().unwrap().kind, + EntryKind::Directory + ); + assert_eq!(first.total_entries, 3); + assert!( + stacked + .list(request("deep", 1, None)) + .await + .unwrap() + .entries + .is_empty() + ); + let second = stacked + .list(request("", 1, first.next_after)) + .await + .unwrap(); + assert_eq!(names(&second), ["a"]); + std::fs::remove_file(root.path().join("child/a")).unwrap(); + std::fs::write(root.path().join("child/b"), b"xx").unwrap(); + let third = stacked + .list(request("", 10, second.next_after)) + .await + .unwrap(); + assert_eq!(names(&third), ["b", "z"]); + assert!(!third.truncated); + assert!(third.next_after.is_none()); + assert_eq!(third.total_entries, 3); + lease.close().await.unwrap(); + assert!(matches!( + stacked.list(request("", 1, None)).await, + Err(WorkdirError::SessionClosed) + )); +} + +#[tokio::test] +async fn list_checkout_search_cursor_uses_output_root_coordinates_across_nested_wrappers() { + let root = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(root.path().join("child/keep")).unwrap(); + for name in ["a", "b", "c"] { + std::fs::write(root.path().join("child/keep").join(name), b"x").unwrap(); + } + let source = LocalWorkdirSession::external_read_only( + Workdir::new("rebased-pagination"), + root.path(), + BoundedReadLimits::EXTERNAL_DEFAULT, + ) + .unwrap(); + let broker = WorkdirToolBroker::new(Arc::new(source)); + let lease = broker + .scope(scope(vec![rule("child", true)], "child")) + .await + .unwrap(); + let stacked = WorkdirToolBroker::new(lease.tool_session()); + let mut search = + CheckoutSearchRequest::new(CheckoutSearchOperation::List(request("keep", 1, None))); + search.output_root = path("keep"); + let CheckoutSearchResult::List(first) = stacked.checkout_search(search.clone()).await.unwrap() + else { + panic!() + }; + assert_eq!(names(&first), ["a"]); + if let CheckoutSearchOperation::List(list) = &mut search.operation { + list.after = first.next_after; + } + let CheckoutSearchResult::List(second) = stacked.checkout_search(search).await.unwrap() else { + panic!() + }; + assert_eq!(names(&second), ["b"]); + assert_eq!(second.next_after.unwrap().path, path("b")); +} + +#[tokio::test] +async fn list_read_capability_suffices_without_glob_and_missing_read_is_denied() { + let root = tempfile::tempdir().unwrap(); + std::fs::write(root.path().join("a"), b"x").unwrap(); + let source = LocalWorkdirSession::materialized( + root.path().to_path_buf(), + root.path().to_path_buf(), + manifest::SharedScope::new(manifest::Scope::writable(root.path()).unwrap()), + WorkdirSessionCapabilities::from_capabilities([WorkdirSessionCapability::Read]), + ); + let broker = WorkdirToolBroker::new(Arc::new(source)); + let lease = broker.scope(scope(vec![rule("", true)], "")).await.unwrap(); + assert_eq!( + names( + &lease + .tool_session() + .list(request("", 1, None)) + .await + .unwrap() + ), + ["a"] + ); + let denied = LocalWorkdirSession::external_with_capabilities_pinned( + Workdir::new("no-read"), + ExternalWorkdirRoot::pin(root.path()).unwrap(), + BoundedReadLimits::EXTERNAL_DEFAULT, + WorkdirSessionCapabilities::COMMAND_ONLY, + ) + .unwrap(); + assert!(matches!( + denied.list(request("", 1, None)).await, + Err(WorkdirError::Unsupported(WorkdirSessionCapability::Read)) + )); +} + +#[tokio::test] +async fn list_external_wire_preserves_continuation_and_rejects_unbounded_cursor() { + let root = tempfile::tempdir().unwrap(); + std::fs::create_dir(root.path().join("child")).unwrap(); + for name in ["a", "b"] { + std::fs::write(root.path().join("child").join(name), b"x").unwrap(); + } + let source = LocalWorkdirSession::external_read_only( + Workdir::new("wire-list"), + root.path(), + BoundedReadLimits::EXTERNAL_DEFAULT, + ) + .unwrap(); + let first = source.list(request("child", 1, None)).await.unwrap(); + let wire = serde_json::to_value( + ExternalWorkdirOperationResult::try_from(Output::List(first.clone())).unwrap(), + ) + .unwrap(); + assert_eq!(wire["result"]["next_after"]["path"], "child/a"); + let decoded: ExternalWorkdirOperationResult = serde_json::from_value(wire.clone()).unwrap(); + assert_eq!(decoded.into_inner(), Output::List(first.clone())); + let operation = + ExternalWorkdirOperation::try_from(Op::List(request("child", 1, first.next_after))) + .unwrap(); + let decoded: ExternalWorkdirOperation = + serde_json::from_value(serde_json::to_value(operation).unwrap()).unwrap(); + let Output::List(next) = dispatch_workdir_session_operation(&source, decoded.into_inner()) + .await + .unwrap() + else { + panic!() + }; + assert_eq!(names(&next), ["child/b"]); + assert!(next.next_after.is_none()); + for bad_path in ["/host".to_owned(), "x".repeat(4097), "outside/a".to_owned()] { + let mut bad = serde_json::to_value(Op::List(request("child", 1, None))).unwrap(); + bad["request"]["after"] = serde_json::json!({"kind": "file", "path": bad_path}); + assert!(serde_json::from_value::(bad).is_err()); + let mut bad_result = wire.clone(); + bad_result["result"]["next_after"]["path"] = serde_json::json!(bad_path); + assert!(serde_json::from_value::(bad_result).is_err()); + } + assert!( + ExternalWorkdirOperation::try_from(Op::List(request( + "child", + MAX_EXTERNAL_RESULT_ITEMS + 1, + None + ))) + .is_err() + ); + // Search cursors use result coordinates, not provider path coordinates. + let mut search = CheckoutSearchRequest::new(CheckoutSearchOperation::List(request( + "child", + 1, + Some(ListCursor { + kind: EntryKind::File, + path: path("a"), + }), + ))); + search.output_root = path("child"); + let bounded = ExternalWorkdirOperation::try_from(Op::CheckoutSearch(search)).unwrap(); + let Output::CheckoutSearch(CheckoutSearchResult::List(next)) = + dispatch_workdir_session_operation(&source, bounded.into_inner()) + .await + .unwrap() + else { + panic!() + }; + assert_eq!(names(&next), ["b"]); +} + +#[test] +fn list_external_result_counts_cursor_toward_aggregate_path_bound() { + let entries: Vec<_> = (0..252) + .map(|n| ListEntry { + path: path(&format!("{n:04}{}", "x".repeat(4092))), + kind: EntryKind::File, + size: 1, + }) + .collect(); + let after = ListCursor { + kind: EntryKind::File, + path: entries.last().unwrap().path.clone(), + }; + let result = ListResult { + entries, + total_entries: 253, + total_bytes: 253, + truncated: true, + next_after: None, + }; + let mut wire = serde_json::to_value(Output::List(result)).unwrap(); + assert!(serde_json::from_value::(wire.clone()).is_ok()); + wire["result"]["next_after"] = serde_json::to_value(after).unwrap(); + assert!(serde_json::from_value::(wire).is_err()); +} diff --git a/crates/worker/src/checkout.rs b/crates/worker/src/checkout.rs index 127cbd163..7035e782e 100644 --- a/crates/worker/src/checkout.rs +++ b/crates/worker/src/checkout.rs @@ -11,11 +11,12 @@ use manifest::{ToolPermissionAction, ToolPermissionConfig}; use serde_json::{Value as Json, json}; use sha2::{Digest, Sha256}; use wip_protocol::{ - Documentation, INTERFACE_FORMAT_V1, InterfaceDescriptor, Object, OperationDeclaration, - ParameterDeclaration, ProtocolError, ProtocolErrorCode, ReturnDeclaration, TypeExpr, Value, + Documentation, FieldDeclaration, INTERFACE_FORMAT_V1, InterfaceDescriptor, Object, + OperationDeclaration, ParameterDeclaration, ProtocolError, ProtocolErrorCode, + ReturnDeclaration, TypeExpr, Value, }; use workdir::{ - CheckoutObservation, EntryKind, ListRequest, WorkdirPath, WorkdirSessionCapability as Cap, + CheckoutObservation, EntryKind, WorkdirPath, WorkdirSessionCapability as Cap, WorkdirSessionRouter, }; @@ -126,11 +127,15 @@ impl Provider { } self.project( ROOT, - descriptor(vec![operation( - "list", - "List currently accessible checkouts", - vec![], - )]), + descriptor(vec![{ + let mut op = operation( + "list", + "List accessible checkout entrances (not filesystem contents)", + vec![], + ); + op.returns.r#type = TypeExpr::Json; + op + }]), state.finalize().to_vec(), "collection", Arc::new(Collection { @@ -169,8 +174,14 @@ impl Provider { } } async fn node(self: &Arc, path: &str) -> Result, ProtocolError> { + Ok(self.publication_inner(path).await?.map(|p| p.projection)) + } + async fn publication_inner( + self: &Arc, + path: &str, + ) -> Result, ProtocolError> { if path == ROOT { - return Ok(Some(self.collection())); + return WipPublication::self_scoped(self.collection()).map(Some); } let Some((alias, target)) = self.parse(path) else { return Ok(None); @@ -202,39 +213,86 @@ impl Provider { if observation.kind == EntryKind::File && !self.allowed("Read", &permission_input) { return Ok(None); } + let workdir = selected.session.workdir().id().as_str().to_owned(); + let root = checkout_root(&alias); + // The router guard pins this connection across both observations. Root + // lifetime is attachment lifetime, not directory-content validator state. + let scope = if target.is_root() { + None + } else { + let root_observation = + match selected.session.checkout_observe(WorkdirPath::root()).await { + Ok(o) => o, + Err(_) => return Ok(None), + }; + self.entry_projection( + &root, + &alias, + selected.generation, + &workdir, + WorkdirPath::root(), + root_observation, + ) + }; + let Some(projection) = self.entry_projection( + path, + &alias, + selected.generation, + &workdir, + target, + observation, + ) else { + return Ok(None); + }; + if path == root { + return WipPublication::self_scoped(projection).map(Some); + } + let Some(scope) = scope else { + return Ok(None); + }; + Ok(Some(WipPublication { + projection, + scope: Some(scope.object), + })) + } + fn entry_projection( + self: &Arc, + path: &str, + alias: &str, + generation: u64, + workdir: &str, + target: WorkdirPath, + observation: CheckoutObservation, + ) -> Option { let ops = self.operations(&observation); if ops.is_empty() { - return Ok(None); + return None; } let kind = if observation.kind == EntryKind::File { "file" } else { "directory" }; - let workdir = selected.session.workdir().id().as_str().to_owned(); - let validator = self.wrap_validator( - &alias, - selected.generation, - &workdir, - &observation.validator, - ); + let validator = self.wrap_validator(alias, generation, workdir, &observation.validator); let handler = Arc::new(FileHandler { provider: self.clone(), - alias, - generation: selected.generation, - workdir, + alias: alias.into(), + generation, + workdir: workdir.into(), target, observation, }); - let family = format!("{kind}-g{}", selected.generation); - Ok(Some(self.project( - path, - descriptor(ops), - validator, - &family, - handler, - ))) + let family = format!("{kind}-g{generation}"); + let mut projection = self.project(path, descriptor(ops), validator, &family, handler); + // Keep path-qualified local names (permissions/capabilities may differ + // per target) while binding every content Interface to its live entrance. + projection.interface.scope = checkout_root(alias); + projection.interface.name.push_str("#target/"); + projection.interface.name.push_str(&encode_identity(path)); + projection.object.interfaces = vec![projection.interface.clone()]; + Some(projection) } + fn operations(&self, o: &CheckoutObservation) -> Vec { let mut ops = Vec::new(); let has = |cap, name| o.capabilities.supports(cap) && self.may_allow(name); @@ -261,6 +319,13 @@ impl Provider { )); } } else { + if has(Cap::Read, "List") { + ops.push(operation( + "list", + "List direct contents, directories first then lexical path. Bounded live pages, not a snapshot; pass returned after to continue. Limit defaults to 100, maximum 1000. List permission and provider Read/enumeration authority apply. Includes hidden/ignored names per provider List policy, unlike Glob/Grep.", + vec![parameter("limit", false, TypeExpr::Integer), parameter("after", false, cursor_type())], + )); + } if has(Cap::Glob, "Glob") { ops.push(operation( "glob", @@ -288,6 +353,15 @@ impl Provider { #[async_trait] impl WipSubtreeProvider for Arc { + fn interface_target(&self, reference: &wip_protocol::InterfaceReference) -> Option { + if reference.scope == ROOT { + return Some(ROOT.into()); + } + let (_, encoded) = reference.name.rsplit_once("#target/")?; + let path = decode_identity(encoded)?; + let (alias, _) = self.parse(&path)?; + (reference.scope == checkout_root(&alias)).then_some(path) + } async fn publication(&self, path: &str) -> Result, ProtocolError> { let _permit = tokio::time::timeout(self.deadline, self.permits.acquire()) .await @@ -298,7 +372,7 @@ impl WipSubtreeProvider for Arc { ) })? .map_err(|_| error(ProtocolErrorCode::Internal, "checkout unavailable"))?; - tokio::time::timeout(self.deadline, self.node(path)) + tokio::time::timeout(self.deadline, self.publication_inner(path)) .await .map_err(|_| { error( @@ -306,7 +380,6 @@ impl WipSubtreeProvider for Arc { "checkout observation deadline", ) })? - .and_then(|projection| projection.map(WipPublication::self_scoped).transpose()) } async fn children(&self, path: &str) -> Result, ProtocolError> { let _permit = tokio::time::timeout(self.deadline, self.permits.acquire()) @@ -354,70 +427,9 @@ impl Provider { } return Ok(paths); } - let Some((alias, target)) = self.parse(path) else { - return Err(error(ProtocolErrorCode::NotFound, "checkout not published")); - }; - let selected = self - .router - .resolve(Some(&alias)) - .map_err(|_| error(ProtocolErrorCode::NotFound, "attachment expired"))?; - let o = selected - .session - .checkout_observe(target.clone()) - .await - .map_err(|_| error(ProtocolErrorCode::NotFound, "checkout not published"))?; - if o.kind != EntryKind::Directory { - return Ok(Vec::new()); - } - if !self.allowed( - "Glob", - &json!({"target_workdir":alias,"path":target.as_str(),"pattern":"*"}), - ) && !self.allowed( - "Grep", - &json!({"target_workdir":alias,"path":target.as_str(),"pattern":""}), - ) { - return Ok(Vec::new()); - } - let listing = selected - .session - .checkout_search(workdir::CheckoutSearchRequest::new( - workdir::CheckoutSearchOperation::List(ListRequest { - path: target, - limit: MAX_NODES + 1, - }), - )) - .await - .map_err(|_| error(ProtocolErrorCode::Internal, "checkout listing unavailable"))?; - let workdir::CheckoutSearchResult::List(listing) = listing else { - return Err(error( - ProtocolErrorCode::Internal, - "mismatched checkout listing", - )); - }; - if listing.truncated || listing.entries.len() > MAX_NODES { - return Err(error( - ProtocolErrorCode::ResourceLimitExceeded, - "directory observation exceeds node bound", - )); - } - let mut paths = Vec::new(); - for entry in listing.entries { - let p = object_path(&alias, &entry.path); - if self.node(&p).await?.is_some() { - paths.push(p); - } - } - if !self - .router - .resolve(Some(&alias)) - .is_ok_and(|current| current.generation == selected.generation) - { - return Err(error( - ProtocolErrorCode::ValidatorMismatch, - "attachment changed", - )); - } - Ok(paths) + // Contents are addressable, but never indexable. Host observe already + // resolves the requested Object; no provider content List is needed. + Ok(Vec::new()) } } @@ -501,6 +513,7 @@ impl WipOperationHandler for FileHandler { context: WipCallContext, ) -> Result { let tool = match operation { + "list" => "List", "read" => "Read", "edit" => "Edit", "write" => "Write", @@ -528,7 +541,34 @@ impl WipOperationHandler for FileHandler { } let mut permission_input = args.clone(); permission_input.insert("target_workdir".into(), json!(self.alias)); - if tool == "Glob" || tool == "Grep" { + if tool == "List" { + permission_input.insert("path".into(), json!(self.target.as_str())); + if let Some(after) = args.get_mut("after") { + let entry = after.get("entry").and_then(Json::as_str).ok_or_else(|| { + protocol( + ProtocolErrorCode::InvalidArguments, + "after requires an entry", + ) + })?; + let Some((alias, path)) = self.provider.parse(entry) else { + return Err(protocol( + ProtocolErrorCode::InvalidArguments, + "invalid continuation entry", + )); + }; + if alias != self.alias || object_path(&alias, &path) != entry { + return Err(protocol( + ProtocolErrorCode::InvalidArguments, + "continuation must belong to this checkout", + )); + } + let fields = after.as_object_mut().ok_or_else(|| { + protocol(ProtocolErrorCode::InvalidArguments, "invalid continuation") + })?; + fields.remove("entry"); + fields.insert("path".into(), json!(path.as_str())); + } + } else if tool == "Glob" || tool == "Grep" { let relative = args.get("path").and_then(Json::as_str).unwrap_or(""); permission_input.insert("path".into(), json!(join_under(&self.target, relative)?)); } else if tool == "Create" { @@ -588,49 +628,49 @@ impl WipOperationHandler for FileHandler { } })? .map_err(map_tool_error)?; - let links = tokio::time::timeout_at(deadline, async { - let mut links = Vec::new(); - self.check_connection()?; - for path in &result.paths { - let route = object_path(&self.alias, path); - // References are validated only against the captured live connection. - if self - .provider - .node(&route) - .await - .map_err(WipOperationError::Protocol)? - .is_some() - { - links.push(json!({"path":route})); - } - self.check_connection()?; + // Coordinates are not observations. Never prefetch unselected results; + // a vanished entry cannot fail a successful discovery Operation. + self.check_connection()?; + let mut fields = BTreeMap::from([("summary".into(), Value::String(result.output.summary))]); + if let Some(content) = result.output.content { + fields.insert("content".into(), Value::String(content)); + } + let items = if let Some(listing) = result.listing { + fields.insert("truncated".into(), Value::Boolean(listing.truncated)); + if let Some(after) = listing.next_after { + fields.insert("after".into(), cursor_value(&self.alias, &after)); } - Ok::<_, WipOperationError>(links) - }) - .await - .map_err(|_| { - if matches!(tool, "Write" | "Edit" | "Create") { - WipOperationError::OutcomeUnknown("completed operation response deadline".into()) - } else { - protocol( - ProtocolErrorCode::ResourceLimitExceeded, - "checkout result deadline", - ) - } - })? - .map_err(|e| { - if matches!(tool, "Write" | "Edit" | "Create") { - WipOperationError::OutcomeUnknown("completed operation response unavailable".into()) - } else { - e - } - })?; - let value = json_to_wip( - &json!({"summary":result.output.summary,"content":result.output.content,"items":links}), - ) - .map_err(|_| { - WipOperationError::OutcomeUnknown("invalid completed file operation result".into()) - })?; + listing + .entries + .into_iter() + .map(|item| { + Value::Record(BTreeMap::from([ + ( + "entry".into(), + Value::String(object_path(&self.alias, &item.path)), + ), + ("kind".into(), Value::String(kind_name(item.kind).into())), + ( + "size".into(), + Value::Integer(i64::try_from(item.size).unwrap_or(i64::MAX)), + ), + ])) + }) + .collect() + } else { + result + .paths + .iter() + .map(|path| { + Value::Record(BTreeMap::from([( + "entry".into(), + Value::String(object_path(&self.alias, path)), + )])) + }) + .collect() + }; + fields.insert("items".into(), Value::List(items)); + let value = Value::Record(fields); if let Some(raw) = result.validator { Ok(WipOperationOutput::native_with_validator( value, @@ -693,10 +733,70 @@ fn operation( parameters, returns: ReturnDeclaration { documentation: None, - r#type: TypeExpr::Json, + r#type: result_type(name == "list"), }, } } +fn field(name: &str, required: bool, r#type: TypeExpr) -> FieldDeclaration { + FieldDeclaration { + name: name.into(), + required, + documentation: None, + r#type, + } +} +fn cursor_type() -> TypeExpr { + TypeExpr::Record { + fields: vec![ + field("kind", true, TypeExpr::String), + field("entry", true, TypeExpr::Entry), + ], + } +} +fn result_type(list: bool) -> TypeExpr { + let mut item = vec![field("entry", true, TypeExpr::Entry)]; + if list { + item.extend([ + field("kind", true, TypeExpr::String), + field("size", true, TypeExpr::Integer), + ]); + } + let mut fields = vec![ + field("summary", true, TypeExpr::String), + field("content", false, TypeExpr::String), + field( + "items", + true, + TypeExpr::List { + items: Box::new(TypeExpr::Record { fields: item }), + }, + ), + ]; + if list { + fields.extend([ + field("truncated", true, TypeExpr::Boolean), + field("after", false, cursor_type()), + ]); + } + TypeExpr::Record { fields } +} +fn kind_name(kind: EntryKind) -> &'static str { + match kind { + EntryKind::Directory => "directory", + EntryKind::File => "file", + EntryKind::Symlink => "symlink", + EntryKind::Other => "other", + } +} +fn cursor_value(alias: &str, cursor: &workdir::ListCursor) -> Value { + Value::Record(BTreeMap::from([ + ("kind".into(), Value::String(kind_name(cursor.kind).into())), + ( + "entry".into(), + Value::String(object_path(alias, &cursor.path)), + ), + ])) +} fn descriptor(operations: Vec) -> InterfaceDescriptor { InterfaceDescriptor{format:INTERFACE_FORMAT_V1.into(),documentation:Some(Documentation{summary:"AI-oriented attached Workdir operations".into(),details:Some("The route binds attachment and target. Line offsets are zero-based; text output numbers lines from one. Read-before-write and provider validators are both required. Paths are Worldspace references, never host paths.".into())}),types:Vec::new(),operations} } diff --git a/crates/worker/src/checkout_http_tests.rs b/crates/worker/src/checkout_http_tests.rs index 455971e7a..b69c99d5b 100644 --- a/crates/worker/src/checkout_http_tests.rs +++ b/crates/worker/src/checkout_http_tests.rs @@ -379,7 +379,7 @@ fn first_link(value: &Value) -> &str { panic!("expected typed link List") }; assert_eq!(items.len(), 1); - let Value::String(path) = field(&items[0], "path") else { + let Value::String(path) = field(&items[0], "entry") else { panic!("expected typed link path") }; path @@ -669,3 +669,89 @@ async fn checkout_http_malformed_response_after_write_is_unknown_without_retry() async fn checkout_http_lost_response_after_write_is_unknown_without_retry() { watchdog(committed_write_unknown(WriteResponseFault::LostBody)).await; } + +#[tokio::test] +async fn remote_checkout_list_pages_and_direct_inspect_read_use_provider_without_tree_enumeration() +{ + use super::checkout_wip_tests::{json_content, model_inspect, model_invoke, model_operation}; + let fixture = HttpFixture::new(WorkdirSessionCapabilities::READ_ONLY).await; + std::fs::create_dir(fixture.dir.path().join("empty")).unwrap(); + let remote = fixture.open(TOKEN).await.unwrap(); + let (runtime, _) = native_runtime(Arc::new(remote)); + let runtime = Arc::new(runtime); + let tree = json_content( + model_operation( + runtime.clone(), + "Tree", + json!({"path":"/checkouts", "depth":8}), + ) + .await + .unwrap(), + ); + assert_eq!(tree["tree"]["children"][0]["children"], json!([])); + assert!( + !fixture + .state + .operations + .lock() + .unwrap() + .iter() + .any(|op| matches!( + op, + WorkdirSessionOperation::CheckoutSearch(_) | WorkdirSessionOperation::List(_) + )) + ); + let entrance = model_inspect(runtime.clone(), "/checkouts/main").await; + let first = json_content( + model_invoke(runtime.clone(), &entrance, "list", json!({"limit":1})) + .await + .unwrap(), + ); + assert_eq!(first["items"][0]["entry"], "/checkouts/main/empty"); + assert_eq!(first["items"][0]["kind"], "directory"); + assert_eq!(first["truncated"], true); + let next = json_content( + model_invoke( + runtime.clone(), + &entrance, + "list", + json!({"limit":1,"after":first["after"]}), + ) + .await + .unwrap(), + ); + assert_eq!(next["items"][0]["entry"], "/checkouts/main/src"); + assert_eq!(next["truncated"], false); + let deep = model_inspect(runtime.clone(), "/checkouts/main/src/deep").await; + for (operation, args) in [ + ("list", json!({})), + ("glob", json!({"pattern":"*.txt"})), + ( + "grep", + json!({"pattern":"needle","output_mode":"files_with_matches"}), + ), + ] { + let found = json_content( + model_invoke(runtime.clone(), &deep, operation, args) + .await + .unwrap(), + ); + let file = found["items"][0]["entry"].as_str().unwrap(); + assert_eq!(file, "/checkouts/main/src/deep/a.txt"); + let inspected = model_inspect(runtime.clone(), file).await; + let read = json_content( + model_invoke(runtime.clone(), &inspected, "read", json!({})) + .await + .unwrap(), + ); + assert!(read["content"].as_str().unwrap().contains("needle")); + fixture.public_output(&found.to_string()); + } + let operations = fixture.state.operations.lock().unwrap(); + assert!(operations.iter().any(|op| matches!(op, WorkdirSessionOperation::CheckoutSearch(r) if matches!(&r.operation, CheckoutSearchOperation::List(l) if l.after.is_some())))); + assert!( + !operations + .iter() + .any(|op| matches!(op, WorkdirSessionOperation::List(_))) + ); +} diff --git a/crates/worker/src/checkout_race_tests.rs b/crates/worker/src/checkout_race_tests.rs index 954ce0977..15e1c6ade 100644 --- a/crates/worker/src/checkout_race_tests.rs +++ b/crates/worker/src/checkout_race_tests.rs @@ -13,9 +13,9 @@ use tempfile::TempDir; use workdir::{ CheckoutRequest, CheckoutResult, CommandHandle, CommandOutput, CommandOutputRequest, CommandRequest, CommandStatus, EditRequest, EditResult, GlobRequest, GlobResult, GrepRequest, - GrepResult, ListResult, LocalWorkdirSession, ReadBytesRequest, ReadBytesResult, ReadRequest, - ReadResult, StatRequest, StatResult, Workdir, WorkdirAttachmentAlias, WorkdirError, - WorkdirScopeAuthorizationRequest, WorkdirScopeOverlapRequest, WorkdirSession, + GrepResult, ListRequest, ListResult, LocalWorkdirSession, ReadBytesRequest, ReadBytesResult, + ReadRequest, ReadResult, StatRequest, StatResult, Workdir, WorkdirAttachmentAlias, + WorkdirError, WorkdirScopeAuthorizationRequest, WorkdirScopeOverlapRequest, WorkdirSession, WorkdirSessionCapabilities, WorkdirSessionHandle, WriteRequest, WriteResult, }; @@ -28,6 +28,7 @@ struct HookedSession { observe_hook: Mutex>, stall_path: Mutex>, stalled_observations: AtomicUsize, + searches: AtomicUsize, } impl std::fmt::Debug for HookedSession { @@ -52,6 +53,7 @@ impl HookedSession { observe_hook: Mutex::new(None), stall_path: Mutex::new(None), stalled_observations: AtomicUsize::new(0), + searches: AtomicUsize::new(0), }) } @@ -123,6 +125,7 @@ impl WorkdirSession for HookedSession { &self, request: workdir::CheckoutSearchRequest, ) -> Result { + self.searches.fetch_add(1, Ordering::SeqCst); self.local.checkout_search(request).await } @@ -484,7 +487,7 @@ async fn collection_list_and_root_enumeration_bound_stalled_observation_and_rele } #[tokio::test] -async fn grep_post_search_link_observation_is_deadline_bounded_and_releases_permit() { +async fn grep_returns_typed_entry_without_observing_stalled_result() { let dir = TempDir::new().unwrap(); std::fs::write(dir.path().join("a.txt"), "needle\n").unwrap(); let session = HookedSession::new(&dir); @@ -499,8 +502,8 @@ async fn grep_post_search_link_observation_is_deadline_bounded_and_releases_perm let root = checkout_root("main"); let projection = p.node(&root).await.unwrap().unwrap(); - // Directory observations and the normal local grep/checkout execution can - // complete. Only publishing the typed search result path gets stuck. + // Directory/search can complete. The result path must never be observed + // during result publication, even when it would block indefinitely. session.stall_observation(Some(WorkdirPath::new("a.txt").unwrap())); let arguments = BTreeMap::from([ ("pattern".into(), Value::String("needle".into())), @@ -514,13 +517,17 @@ async fn grep_post_search_link_observation_is_deadline_bounded_and_releases_perm projection.handler.call("grep", &arguments, context()), ) .await - .expect("post-search link observations must share the operation deadline"); - assert_deadline(result, "checkout result deadline"); - assert_eq!(session.stalled_observations.load(Ordering::SeqCst), 1); + .expect("completed search must return without acquiring its entries"); + let output = result.unwrap_or_else(|_| panic!("successful search must not observe each entry")); + assert_eq!( + wip_to_json(&output.value).unwrap()["items"], + json!([{"entry": format!("{root}/a.txt")}]) + ); + assert_eq!(session.stalled_observations.load(Ordering::SeqCst), 0); assert_permit_released(&p); - // A successful retry both checks cancellation cleanup and proves that the - // delegated search produces the typed path consumed by post-processing. + // A second explicit search produces the same typed coordinate, without + // acquiring it even when it could resolve successfully. session.stall_observation(None); let result = tokio::time::timeout( Duration::from_secs(2), @@ -533,6 +540,33 @@ async fn grep_post_search_link_observation_is_deadline_bounded_and_releases_perm Err(_) => panic!("grep must succeed once the link observation is unstalled"), }; let value = wip_to_json(&output.value).unwrap(); - assert_eq!(value["items"], json!([{"path": format!("{root}/a.txt")}])); + assert_eq!(value["items"], json!([{"entry": format!("{root}/a.txt")}])); assert_permit_released(&p); } + +#[tokio::test] +async fn checkout_content_children_never_call_provider_list_even_for_deep_paths() { + let dir = TempDir::new().unwrap(); + std::fs::create_dir_all(dir.path().join("nested/deep")).unwrap(); + for n in 0..1100 { + std::fs::write(dir.path().join(format!("file-{n}")), "fixture").unwrap(); + } + let session = HookedSession::new(&dir); + let router = Arc::new(WorkdirSessionRouter::new()); + router + .attach( + WorkdirAttachmentAlias::new("main").unwrap(), + session.clone(), + ) + .unwrap(); + let subtree = Arc::new(provider(router)); + assert_eq!( + subtree.children(ROOT).await.unwrap(), + vec![checkout_root("main")] + ); + for path in ["/checkouts/main", "/checkouts/main/nested/deep"] { + assert!(subtree.publication(path).await.unwrap().is_some()); + assert!(subtree.children(path).await.unwrap().is_empty()); + } + assert_eq!(session.searches.load(Ordering::SeqCst), 0); +} diff --git a/crates/worker/src/checkout_wip_tests.rs b/crates/worker/src/checkout_wip_tests.rs index 09b4c4df0..89972eefe 100644 --- a/crates/worker/src/checkout_wip_tests.rs +++ b/crates/worker/src/checkout_wip_tests.rs @@ -113,7 +113,7 @@ async fn checkout_wip_search_preserves_root_and_nested_ignore_rules_and_links() .as_array() .unwrap() .iter() - .map(|item| item["path"].as_str().unwrap().to_string()) + .map(|item| item["entry"].as_str().unwrap().to_string()) .collect::>(); // Grep retains traversal order; Glob sorts its result paths. paths.sort(); @@ -159,8 +159,8 @@ async fn checkout_wip_native_roundtrip_search_read_edit_write_create() { .await .unwrap(); let grep_result: Json = serde_json::from_str(grep.content.as_deref().unwrap()).unwrap(); - assert_eq!(grep_result["items"][0]["path"], file); - let followed = grep_result["items"][0]["path"].as_str().unwrap(); + assert_eq!(grep_result["items"][0]["entry"], file); + let followed = grep_result["items"][0]["entry"].as_str().unwrap(); assert_eq!(followed, file); assert!( call( @@ -588,8 +588,9 @@ async fn checkout_wip_scoped_child_search_and_revoke_cannot_expose_siblings() { .observe_live("/checkouts/child", 2) .await .unwrap(); - assert_eq!(observed.children.unwrap().len(), 1); + assert!(observed.children.unwrap().is_empty()); for (op, args) in [ + ("list", json!({"limit":1})), ("glob", json!({"pattern":"**/*"})), ( "grep", @@ -654,3 +655,339 @@ async fn checkout_wip_scoped_child_search_and_revoke_cannot_expose_siblings() { "changed" ); } + +pub(super) fn json_content(output: ToolOutput) -> Json { + serde_json::from_str(output.content.as_deref().unwrap()).unwrap() +} +pub(super) async fn model_operation( + r: Arc, + name: &str, + args: Json, +) -> Result { + let (_, tool) = wip_tool_definitions(r) + .into_iter() + .map(|definition| definition()) + .find(|(meta, _)| meta.name == name) + .unwrap(); + tool.execute(&args.to_string(), Default::default()).await +} +pub(super) async fn model_inspect(r: Arc, path: &str) -> Json { + json_content( + model_operation(r, "Inspect", json!({"path":path})) + .await + .unwrap(), + ) +} +pub(super) async fn model_invoke( + r: Arc, + inspected: &Json, + operation: &str, + arguments: Json, +) -> Result { + model_operation(r, "Invoke", json!({"path":inspected["path"], "interface":inspected["interfaces"][0]["reference"], "operation":operation, "arguments":arguments})).await +} + +#[tokio::test] +async fn checkout_depth_boundaries_are_independent_of_empty_small_or_large_contents() { + for count in [0, 2, 1100] { + let dir = TempDir::new().unwrap(); + std::fs::create_dir_all(dir.path().join("nested/deep")).unwrap(); + for n in 0..count { + std::fs::write(dir.path().join(format!("file-{n}")), "fixture").unwrap(); + } + let router = Arc::new(WorkdirSessionRouter::new()); + router + .attach( + WorkdirAttachmentAlias::new("main").unwrap(), + session(&dir, true), + ) + .unwrap(); + let r = runtime(router, None); + let one = r.host.observe_live("/checkouts", 1).await.unwrap(); + assert!(one.children.unwrap()[0].children.is_none()); + for depth in [2, 32] { + let observation = r.host.observe_live("/checkouts", depth).await.unwrap(); + let entrances = observation.children.unwrap(); + assert_eq!(entrances.len(), 1); + assert_eq!(entrances[0].children, Some(vec![])); + } + let world = r.host.observe_live("/", 32).await.unwrap(); + assert_eq!( + world.children.unwrap()[0].children.as_ref().unwrap()[0].children, + Some(vec![]) + ); + for path in ["/checkouts/main", "/checkouts/main/nested/deep"] { + assert!( + r.host + .observe_live(path, 0) + .await + .unwrap() + .children + .is_none() + ); + assert_eq!( + r.host.observe_live(path, 1).await.unwrap().children, + Some(vec![]) + ); + } + } +} + +#[tokio::test] +async fn checkout_model_list_glob_grep_entries_are_lazy_and_never_become_tree_edges() { + let dir = TempDir::new().unwrap(); + std::fs::create_dir_all(dir.path().join("empty")).unwrap(); + std::fs::create_dir_all(dir.path().join("nested/deep")).unwrap(); + std::fs::write(dir.path().join("nested/deep/a.txt"), "needle\n").unwrap(); + std::fs::write(dir.path().join("z.txt"), "needle\n").unwrap(); + let router = Arc::new(WorkdirSessionRouter::new()); + router + .attach( + WorkdirAttachmentAlias::new("main").unwrap(), + session(&dir, true), + ) + .unwrap(); + let r = Arc::new(runtime(router, None)); + let entrance = model_inspect(r.clone(), "/checkouts/main").await; + assert_eq!(entrance["path"], "/checkouts/main"); + assert_eq!( + entrance["interfaces"][0]["reference"]["scope"], + "/checkouts/main" + ); + let projection = r + .host + .projection_live("/checkouts/main") + .await + .unwrap() + .unwrap(); + assert_eq!( + entrance["object_signature"], + wip_text_view::render_object(&projection.object).unwrap() + ); + assert_eq!( + entrance["interfaces"][0]["signature"], + wip_text_view::render_interface(&wip_text_view::Interface { + reference: &projection.interface, + descriptor: &projection.descriptor + }) + .unwrap() + ); + assert!( + entrance["interfaces"][0]["signature"] + .as_str() + .unwrap() + .contains("entry") + ); + let mut after = None; + let mut items = vec![]; + loop { + let mut args = json!({"limit":1}); + if let Some(cursor) = after { + args["after"] = cursor; + } + let page = json_content( + model_invoke(r.clone(), &entrance, "list", args) + .await + .unwrap(), + ); + assert_eq!(page["items"].as_array().unwrap().len(), 1); + items.push(page["items"][0].clone()); + if page["truncated"] == false { + assert!(page.get("after").is_none()); + break; + } + after = Some(page["after"].clone()); + } + assert_eq!( + items + .iter() + .map(|item| item["entry"].clone()) + .collect::>(), + vec![ + json!("/checkouts/main/empty"), + json!("/checkouts/main/nested"), + json!("/checkouts/main/z.txt") + ] + ); + assert_eq!(items[0]["kind"], "directory"); + assert_eq!(items[2]["kind"], "file"); + let empty = model_inspect(r.clone(), items[0]["entry"].as_str().unwrap()).await; + assert_eq!( + json_content( + model_invoke(r.clone(), &empty, "list", json!({})) + .await + .unwrap() + )["items"], + json!([]) + ); + let deep = model_inspect(r.clone(), "/checkouts/main/nested/deep").await; + assert_eq!( + deep["interfaces"][0]["reference"]["scope"], + "/checkouts/main" + ); + for (operation, args) in [ + ("list", json!({})), + ("glob", json!({"pattern":"*.txt"})), + ( + "grep", + json!({"pattern":"needle", "output_mode":"files_with_matches"}), + ), + ] { + let found = json_content( + model_invoke(r.clone(), &deep, operation, args) + .await + .unwrap(), + ); + let path = found["items"][0]["entry"].as_str().unwrap(); + assert_eq!(path, "/checkouts/main/nested/deep/a.txt"); + // No result-wide eager acquisition by Host or Client. + if operation == "list" { + let state = r.state.lock().unwrap(); + assert!(state.client.object(&state.session, path).is_none()); + } + let inspected = model_inspect(r.clone(), path).await; + let read = json_content( + model_invoke(r.clone(), &inspected, "read", json!({})) + .await + .unwrap(), + ); + assert!(read["content"].as_str().unwrap().contains("needle")); + } + let tree = json_content( + model_operation(r.clone(), "Tree", json!({"path":"/checkouts", "depth":8})) + .await + .unwrap(), + ); + assert_eq!(tree["tree"]["children"][0]["children"], json!([])); + std::fs::remove_file(dir.path().join("nested/deep/a.txt")).unwrap(); + assert!( + model_operation( + r, + "Inspect", + json!({"path":"/checkouts/main/nested/deep/a.txt", "refresh":true}) + ) + .await + .is_err() + ); +} + +#[tokio::test] +async fn checkout_list_permission_cannot_be_bypassed_with_glob_or_command_identity() { + let dir = TempDir::new().unwrap(); + std::fs::write(dir.path().join("a.txt"), "fixture").unwrap(); + let router = Arc::new(WorkdirSessionRouter::new()); + router + .attach( + WorkdirAttachmentAlias::new("main").unwrap(), + session(&dir, true), + ) + .unwrap(); + let r = runtime( + router, + Some(ToolPermissionConfig { + default_action: ToolPermissionAction::Allow, + rules: vec![manifest::ToolPermissionRule { + tool: "List".into(), + pattern: "*".into(), + action: ToolPermissionAction::Deny, + }], + }), + ); + let p = r + .host + .projection_live("/checkouts/main") + .await + .unwrap() + .unwrap(); + assert!(!p.descriptor.operations.iter().any(|op| op.name == "list")); + assert!(p.descriptor.operations.iter().any(|op| op.name == "glob")); + let denied = p + .handler + .call( + "list", + &BTreeMap::new(), + WipCallContext { + execution: Default::default(), + security_context: "fixture".into(), + }, + ) + .await; + assert!( + matches!(denied, Err(WipOperationError::Protocol(e)) if e.code == ProtocolErrorCode::PermissionDenied) + ); +} + +#[tokio::test] +async fn checkout_ancestor_scope_rejects_siblings_prefixes_refs_and_old_connection_before_read() { + let dir = TempDir::new().unwrap(); + std::fs::write(dir.path().join("a.txt"), "same\n").unwrap(); + let router = Arc::new(WorkdirSessionRouter::new()); + let main = WorkdirAttachmentAlias::new("main").unwrap(); + router.attach(main.clone(), session(&dir, true)).unwrap(); + router + .attach( + WorkdirAttachmentAlias::new("main-extra").unwrap(), + session(&dir, true), + ) + .unwrap(); + let r = runtime(router.clone(), None); + let path = "/checkouts/main/a.txt"; + let p = r.host.projection_live(path).await.unwrap().unwrap(); + let fetched = r.host.fetch_interface_live(&p.interface).await.unwrap(); + assert!( + fetched.scope_ref.is_none(), + "ref-less publication is legitimate" + ); + // Ordinary content changes do not terminate the entrance's Interface lifetime. + std::fs::write(dir.path().join("new.txt"), "fixture").unwrap(); + assert_eq!( + r.host + .fetch_interface_live(&p.interface) + .await + .unwrap() + .interface, + p.interface + ); + let request = |reference, scope_ref| CallOperationRequest { + target: wip_protocol::Target { + path: path.into(), + validator: None, + }, + interface: wip_protocol::InterfaceTarget { + reference, + scope_ref, + validator: None, + }, + operation: "read".into(), + arguments: BTreeMap::new(), + }; + for scope in ["/checkouts/main-extra", "/checkouts/main/a"] { + let mut wrong = p.interface.clone(); + wrong.scope = scope.into(); + assert!(r.host.fetch_interface_live(&wrong).await.is_err()); + let forged = request(wrong, None); + let result = r + .host + .call( + forged, + WipCallContext { + execution: Default::default(), + security_context: "fixture".into(), + }, + ) + .await; + assert!( + matches!(result,Err(WipOperationError::Protocol(e)) if e.code == ProtocolErrorCode::InterfaceMismatch) + ); + } + let forged = request(p.interface.clone(), Some("old".into())); + assert!( + matches!(r.host.call(forged, WipCallContext { execution:Default::default(),security_context:"fixture".into() }).await, Err(WipOperationError::Protocol(e)) if e.code == ProtocolErrorCode::InterfaceMismatch) + ); + router.detach(&main).await.unwrap(); + router.attach(main, session(&dir, true)).unwrap(); + let forged = request(p.interface.clone(), None); + assert!( + matches!(r.host.call(forged, WipCallContext { execution:Default::default(),security_context:"fixture".into() }).await, Err(WipOperationError::Protocol(e)) if e.code == ProtocolErrorCode::InterfaceMismatch) + ); +} diff --git a/crates/worker/src/fs_view.rs b/crates/worker/src/fs_view.rs index e9230c00b..5508718c2 100644 --- a/crates/worker/src/fs_view.rs +++ b/crates/worker/src/fs_view.rs @@ -143,6 +143,7 @@ impl WorkerFsView { .list(ListRequest { path: logical.clone(), limit: DIR_FILE_REF_ENTRY_LIMIT, + after: None, }) .await .map_err(ToolsError::from) @@ -222,6 +223,7 @@ impl WorkerFsView { .list(ListRequest { path: parent, limit: COMPLETION_LIMIT, + after: None, }) .await else { diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index e3a574509..12fb280e0 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -54114,6 +54114,7 @@ mod tests { WorkdirSessionOperation::List(workdir::ListRequest { path: workdir::WorkdirPath::root(), limit: 10, + after: None, }), ) .await diff --git a/docs/design/checkout-wip-projection.md b/docs/design/checkout-wip-projection.md index 203a9101e..51e28bfda 100644 --- a/docs/design/checkout-wip-projection.md +++ b/docs/design/checkout-wip-projection.md @@ -14,9 +14,11 @@ is created. ├── workdirs identity/lifecycle ├── workdir-attachments Backend connection lifetimes └── checkouts current accessible content entrances; list - └── directory; glob, grep, create_file - ├── README.md file; read, edit, write - └── src directory; glob, grep, create_file + └── directory; list, glob, grep, create_file + +# Not indexable; resolve known paths directly or discover via Operations: +/checkouts//README.md file; read, edit, write +/checkouts//src directory; list, glob, grep, create_file ``` Operations are Interface declarations, not children or `/features/...` routes. @@ -42,7 +44,7 @@ validates every resolved projection's path/owner/descriptor and every enumerated child's immediate-parent relationship. Static and dynamic/subtree overlaps are rejected. Observe resolves only the requested depth; successful observations are complete through that depth, not partial trees masked as success. Directory -listing and native search use the shared List/Glob/Grep engines through typed +listing Operations and native search use the shared List/Glob/Grep engines through typed `CheckoutSearchRequest` forwarding. Scoped sessions append intersecting rule layers, translate their cwd, and require provider-side visibility checks before entry descent/content reads. Results retain session-logical coordinates, so a @@ -51,6 +53,21 @@ providers fail closed rather than falling back to a broader source search. Direct resolution also uses current provider authority, not possession of a cached Interface. +## Indexable boundary (T-714) + +Only `/checkouts` and accessible immediate attachment entrances are indexable. +No directory content List runs during tree/observe, even at deep depth or when a +known deep directory is the observation root. Empty, small and large directories +all have the same boundary. Positive-depth content observations return +`children: []` (no **indexable** children), not an omitted field and not +an assertion that the directory is empty. At depth zero children are omitted; +`observe("/checkouts", 1)` omits entrance children, depth 2 returns [] there. +The Client retains discovered/inspected paths without adding indexable edges. + +Known authorized paths remain directly resolvable and operable. Use List/Glob/Grep +for discovery, then Inspect only selected entry coordinates; no successful search +is undone when an entry disappears before Inspect. Bash/COMMAND is unnecessary. + ## Names and connection isolation Aliases use T-697's canonical encoding: nonempty ASCII letters/digits/`-`/`_` are @@ -66,18 +83,27 @@ existence alone does not expose its content. Following a reference still require current scope/capability and an accessible filesystem projection. Objects have no shared inode `ref`: hard-linked names and different attachments -must not be mistaken for one path-independent representation. Interface references -are opaque, path-qualified and include a fresh projection incarnation and live -attachment generation. Validators combine that namespace with the provider's +must not be mistaken for one path-independent representation. Interface +references are structured `(scope, name)`. Each entrance is its own +scope; content Interfaces use that entrance as ancestor scope, irrespective of +indexability. Local names bind target coordinates, projection incarnation and +live attachment generation. The provider's target-qualified-name routing hint +only selects a target; fetch re-resolves one coherent current publication and +checks the exact Interface, Descriptor, validator and scope under the pinned +router connection. It never turns a cached registration or name into authority. +Validators combine that namespace with the provider's opaque identity/state validator. Restoring a Worker creates a fresh incarnation; detach/alias reuse changes the router generation. An old Interface, validator or read-history entry cannot retarget a new connection. No separate connection ledger is added. The existing router's active-operation exclusion and begin/finish/cancel detach paths remain authoritative; detach does not remove the Workdir/files. Collection enumeration rejects connection-set changes between sampling and -publication instead of pairing old items with a new validator. Search-link -publication checks the captured attachment generation, not merely a reused -alias; response-side provider awaits are covered by the operation deadline. +publication instead of pairing old items with a new validator. Result publication +checks the captured attachment generation, not merely a +reused alias. It maps typed provider paths without observing/statting every +result. Provider awaits are covered by the operation deadline. Objects publish +no scope_ref here; legitimate ref-less binding still checks membership, +validators, current provider authority and attachment generation. ## AI Operation mapping @@ -89,15 +115,20 @@ below are relative to the **bound directory**, not the Worldspace or endpoint. | Object | Operation | Arguments | Result | | --- | --- | --- | --- | | `/checkouts` | `list` | none | bounded `items` containing alias/slug/Workdir/generation/path | -| directory | `glob` | `pattern`, optional `path` | normal bounded Glob text/summary plus same-checkout Object links | -| directory | `grep` | `pattern`; optional `path`, `glob`, `type`, `case_insensitive`, `-A`, `-B`, `-C`, `multiline`, `output_mode`, `head_limit`, `offset` | normal Grep grouped/context/count text and typed provider search targets mapped to Object links | +| directory | `list` | optional `limit` (default 100, 1..1000), `after` (returned `{kind, entry}`) | direct-content records `{kind, size, entry}`, `truncated`, optional `after` | +| directory | `glob` | `pattern`, optional `path` | normal bounded Glob text/summary plus Descriptor-typed same-checkout `entry` coordinates | +| directory | `grep` | `pattern`; optional `path`, `glob`, `type`, `case_insensitive`, `-A`, `-B`, `-C`, `multiline`, `output_mode`, `head_limit`, `offset` | normal Grep grouped/context/count text and typed provider search targets mapped to Descriptor-typed `entry` coordinates | | file | `read` | optional line `offset`, `limit` | line-numbered text and normal Read summary | | file | `edit` | `old_string`, `new_string`, optional `replace_all` | replacement count/summary and preview | | file | `write` | `content` | existing-file save summary; never creation | | directory | `create_file` | `path`, `content` | create-new summary and created-file link; permitted missing parents are created | -Native operation results use a JSON record with `summary`, nullable `content` and -`items` (`path` is an absolute WIP Object navigation coordinate, not an OS path). +Native operation results declare a typed record with `summary`, optional `content` +and `items`. Each item contains `entry: entry` in its Descriptor (an absolute +Worldspace coordinate, encoded as a string in WIP Value), never a guessed ordinary +string or an OS path. List additionally declares `kind`, byte `size`, `truncated` +and optional `after: {kind: string, entry: entry}`. File operations have empty +items except Create, which returns the new coordinate. Search links are constructed from **typed provider paths**, not by parsing Grep's rendered filenames/line delimiters. Provider scope remains authoritative for search output as well as links. Glob/Grep share normal provider ignore/pattern, @@ -129,12 +160,47 @@ must not record history; successful mutations update hash/change statistics only after provider success. Create is strictly beneath the bound parent, creates only an absent file, and must not overwrite an existing destination or symlink. -Manifest permission identities remain Read/Edit/Write/Glob/Grep; create_file uses -Write. The Host reconstructs the same route-bound input for policy evaluation; +Manifest permission identities are Read/Edit/Write/Glob/Grep plus **List** for +native directory listing; create_file uses Write. List publication uses provider +Read capability and potential List allow policy; execution reconstructs the bound +directory's logical path for the exact List policy. Allowing Glob or COMMAND does +not override a List deny/ask rule, and Interface scope validity grants no file +access. Normal Tools mode is unchanged (no new top-level List tool). The Host reconstructs the same route-bound input for policy evaluation; provider read/write scope, capabilities and delegated leases are checked again. `ask` fails closed. Native replacement claims remove `/tools/Read`, `/tools/Edit`, `/tools/Write`, `/tools/Glob`, `/tools/Grep`, not unrelated Tools. +## Directory List pagination + +List uses the existing provider List engine, **not** glob("*") or shell ls. It +includes direct visible empty directories, hidden and ignored names: unlike +Glob/Grep, provider List does not apply search ignore files. Existing Read scope, +readability and can_enumerate_directory checks still gate names; no ancestor or +nonrecursive enumeration permission is inferred from a known descendant. + +Order is directories first, then lexical canonical provider path, with every +other kind in the second group. after is the last returned exclusive ordering +key, rebased to a canonical Worldspace entry by the Host. Pass the returned record +unchanged to continue on the same directory; a foreign checkout, non-direct child, +absolute provider path or unknown field is rejected. No cursor stat is required: +the key may have disappeared. This is **live pagination, not a snapshot**. New +keys before the cursor are not revisited; later keys may appear, removals vanish, +and renames/kind changes may omit or repeat an entry. Directory validators fence +each call, not all pages; refresh Inspect after a stale rejection before issuing +a new explicit call. Detach/reuse expires the old Interface and validator. + +Each page scans the live directory under provider cancellation/deadline and the +existing traversal-entry ceiling, retaining at most limit+1 ordering candidates +and the aggregate path-byte budget. It never retains the whole unbounded listing. +The next ordering key is explicit whenever truncated; no next key means complete. +Provider resource failures stay explicit failures, not silent truncation or retries. + +UTF-8 canonical names are retained verbatim; unrepresentable names are skipped by +the existing provider. Native List may reject visible symlinks under the checked +provider's no-follow policy. If a provider returns symlink/other kinds they remain +classified path coordinates, not a promise of inspectable/operable Objects: +direct checkout resolution only publishes authorized regular files/directories. + ## Consistency, safety and resource policy The provider resolves/pins the entry/parent, compares its opaque validator and @@ -172,7 +238,7 @@ rediscover/reread. Do not report partially created parents as unchanged. Host observations are bounded to depth 32 and 1024 nodes with a 30-second total cooperative deadline. The checkout adapter permits at most 16 concurrent requests -and bounds admission waiting and the complete operation (including result-link +and bounds admission waiting and the complete operation (including bounded result publication) to 30 seconds each; provider path/depth, source/response, mutation/search and transport bounds apply too. No initial context expands an entire tree. Resource-limit errors do not turn incomplete observation into a @@ -182,8 +248,9 @@ truncation under the existing search contract. ## Typical sequence 1. Use Tree on `/checkouts` with depth 1; Inspect `/checkouts` directly and Invoke `list`. -2. Inspect an entrance or a known deep directory directly; Invoke `glob`/`grep`. -3. Follow a returned file `path`, Inspect that path and Invoke line `read`. +2. Inspect an entrance or known deep directory directly; Invoke `list`/`glob`/`grep`. + For List, pass returned `after` with the next limit to obtain another live page. +3. Follow a returned file `entry`, Inspect that path and Invoke line `read`. 4. Call `edit` or `write`; Client-managed validators protect the exact observed object while the shared tracker enforces the prior Read. 5. Rediscover after external changes; a stale rejection is not permission to skip @@ -203,9 +270,14 @@ outside this feature. Run from the main repository, on Linux with `openat2` and `/proc/self/fd`: ```sh -cargo check --workspace --all-targets +cargo test -p worker --lib checkout_wip +cargo test -p workdir --test list_pagination +cargo check cargo test -p fs-operation -p workdir -p tools -p worker -cargo test -p worker-runtime -p yoi-workspace-server +# Workdir External/HTTP features and dependent server fixture: +cargo test -p workdir --features http-client +cargo test -p yoi-workspace-server --lib backend_workdir_session_proxy_executes_typed_operations +node scripts/verify-wip-dependencies.mjs cargo fmt --all -- --check git diff --check ``` diff --git a/docs/development/wip-integration-validation.md b/docs/development/wip-integration-validation.md index 577ca28e7..5dfb7ce5e 100644 --- a/docs/development/wip-integration-validation.md +++ b/docs/development/wip-integration-validation.md @@ -115,10 +115,76 @@ permission identity and ToolOutput/attachments. - `WipMountRegistry::mount_interface` adds a separate static Interface namespace without replacing Object ownership or flattening same-named operations. It does not overlay stale registration metadata onto a live subtree provider. -- FS indexable boundaries and directory.list remain provider policy. This change - deliberately does not implement the separate T-714 boundary/list migration. - Relevant transport/commit tests are `checkout_wip_tests.rs`, - `checkout_http_tests.rs`, and `checkout_race_tests.rs`. +- FS indexable boundaries and directory.list are provider policy. T-714 now + implements entrance-only trees and bounded typed discovery (details below). + Transport/commit tests are `checkout_wip_tests.rs`, `checkout_http_tests.rs`, + and `checkout_race_tests.rs`. - Worldspace state is runtime-only. Historical opaque references in append-only session/tool history are not migrated into cache or parsed for a guessed scope; a restored Client starts with fresh observations and direct path Inspect. + +## T-714 checkout discovery validation + +Based on integrated T-713 at `f9d4c97f`, still using the exact released crates.io +0.2.0 SDK and official renderer. No alternate wire, renderer or upstream version +was introduced. The checkout projection design documents provider live pagination, +its ordering/cursor/byte ceilings, special-entry policy and the distinction between +indexable emptiness and empty filesystem directories. + +Focused regressions: + +```sh +cargo test -p worker --lib checkout_wip +cargo test -p worker --lib checkout::race_tests +cargo test -p worker --lib checkout_http_tests +cargo test -p worker --lib ancestor_tests +cargo test -p tools --lib checkout_list +cargo test -p fs-operation --test list_pagination +cargo test -p workdir --test list_pagination +``` + +Host depth-zero omission / positive-depth `children: []`, empty/small/1100-entry +folders, direct deep roots and zero provider content-List calls during observation +are tested independently of Client output. Registered model tools exercise entrance +self-scope and deep target ancestor-scope, paginated List/Glob/Grep typed entries, +lazy selected Inspect with official signatures/path context, and Read without +COMMAND. Acquired paths never become tree edges; a stalled result observation is +never awaited and disappearance only fails a later explicit Inspect. Remote HTTP +checks verify List continuation on the wire with no ordinary List fallback or +response credentials/host-path leakage. Provider tests cover scoped External and +nested cwd/output_root rebasing, nonrecursive enumeration/read authority, ignore +policy, live mutations, byte bounds, cancellation and malicious cursor/result DTOs. + +`WipSubtreeProvider::interface_target` is a routing hint for target-qualified local +names under ancestor scopes. The default retains self/shared-scope behavior. Host +checks provider ownership and resolves **one** coherent current target/scope +publication; it never treats the hint as authorization or fetches a scope in an +independent mutable lookup. Focused Host tests reject forged/outside/prefix/missing +hints, ref-less deletion and absent membership while preserving existing ancestor +ref/validator and in-flight snapshots. Actual checkout tests reject sibling/prefix +scopes, supplied refs when current publication has none and stale alias generations; +ordinary content updates do not terminate the entrance Interface lifetime. + +Completion validation: + +- Root `cargo check` passed. +- `cargo test -p fs-operation -p workdir -p tools -p worker` passed: FS **58 unit + + 6 pagination**; Workdir **125 unit + 5 pagination**; Tools **84 unit + 39 + integration + 1 doctest**; Worker **901 unit + 110 integration**. +- `cargo test -p workdir --features http-client` passed (**125 + 5**). +- `cargo test -p yoi-workspace-server` passed (**691 unit + 11 CLI**). +- `cargo test -p server-api` passed (**83**, including checked-in OpenAPI/TypeScript + freshness). No generated API artifact changes were needed. +- `node scripts/verify-wip-dependencies.mjs` confirmed one crates.io 0.2.0 Protocol + source and all four exact SDK packages. `cargo fmt --all -- --check` and + `git diff --check HEAD` passed. + +The first dependent Server test build exposed predecessor fixtures still using +removed `discover`/opaque strings/private `call`. Those tests now select structured +references from actual path Inspect and use public Tree/Inspect/Invoke; the complete +Server suite above includes configuration integration and the typed Workdir proxy. +Its dev-only Protocol dependency reuses the same locked SDK source; the resolved +package/version set is unchanged. + +No full workspace test, full Nix/image build, deployment, target integration or +live dogfood update is claimed. Target movement remains Orchestrator authority. diff --git a/docs/report/t-714-ticket-read-auth-blocker.md b/docs/report/t-714-ticket-read-auth-blocker.md new file mode 100644 index 000000000..9c9bc0062 --- /dev/null +++ b/docs/report/t-714-ticket-read-auth-blocker.md @@ -0,0 +1,29 @@ +# T-714: Ticket authority read rejected after implementation validation + +## Observed boundary + +On the continuation turn, `ShowTicket({ id: "T-714", event_limit: 5 })` +returned `Workspace Ticket API request failed with HTTP status 401`. +The same assigned Coder session had successfully read the Ticket and recorded +one shared-seam coordination decision earlier. This report does not diagnose +whether the rejection is authentication expiry, capability invalidation, or +another Backend condition. + +## Consequence + +The local checkout implementation and validation were complete. Git retained the +assigned `work/t-714-checkout-discovery` branch and all changes; coherent local +commits were authorized by the user. Publication/MR creation and review were +paused because a fresh authoritative Ticket/MR read could not be obtained. +No alternate actor, CLI, Backend storage access or replacement control-plane +path was attempted. No target integration or live dogfood update occurred. + +## Improvement / recovery + +Provide an actionable typed error distinction between authentication failure and +capability rejection, and a host-owned way to restore valid transport authority +without changing actor identity or bypassing the rejected capability. After that, +reread the Ticket and any linked MR, normally publish only its original source +selector, verify exact provider HEAD, and request an independently bound review. +The local implementation/validation evidence is in commits and +`docs/development/wip-integration-validation.md`; no approval is claimed here. From b8faa83c471ca7c94e8340830d41d42addccac3c Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 9 Oct 2026 15:30:21 +0900 Subject: [PATCH 4/4] fix: preserve unknown mutation outcomes after connection loss Address POST_COMMIT_CONNECTION_FENCE from independent review of af655ae6. Post-success publication must not claim pre-effect rejection if Write/Edit/Create may have committed before attachment loss. Keep read-only validator mismatch and lazy entry mapping unchanged. Add deterministic real checked provider/Tools commit -> detach -> production publication -> WIP Invoke/Client tests for all three mutations, asserting committed effects, OutcomeUnknown audit and exactly one dispatch. Narrow race9, Worker903+110, root check, dependent Server config19/proxy1, SDK resolution and fmt/diff checks passed. --- crates/worker/src/checkout.rs | 123 ++++++----- crates/worker/src/checkout_race_tests.rs | 197 ++++++++++++++++++ .../development/wip-integration-validation.md | 22 ++ 3 files changed, 290 insertions(+), 52 deletions(-) diff --git a/crates/worker/src/checkout.rs b/crates/worker/src/checkout.rs index 7035e782e..adba8a547 100644 --- a/crates/worker/src/checkout.rs +++ b/crates/worker/src/checkout.rs @@ -503,6 +503,76 @@ impl FileHandler { )) } } + /// Classify post-success publication failures separately from pre-effect + /// provider errors; result coordinates remain lazy for all operations. + fn publish_result( + &self, + tool: &str, + result: tools::CheckoutToolOutput, + ) -> Result { + // Coordinates are not observations. Never prefetch unselected results; + // a vanished entry cannot fail a successful discovery Operation. + self.check_connection().map_err(|error| { + if matches!(tool, "Write" | "Edit" | "Create") { + // The provider has already succeeded: connection loss while + // publishing cannot prove that a committed effect was rejected. + WipOperationError::OutcomeUnknown( + "completed file operation lost its attachment before result publication".into(), + ) + } else { + error + } + })?; + let mut fields = BTreeMap::from([("summary".into(), Value::String(result.output.summary))]); + if let Some(content) = result.output.content { + fields.insert("content".into(), Value::String(content)); + } + let items = if let Some(listing) = result.listing { + fields.insert("truncated".into(), Value::Boolean(listing.truncated)); + if let Some(after) = listing.next_after { + fields.insert("after".into(), cursor_value(&self.alias, &after)); + } + listing + .entries + .into_iter() + .map(|item| { + Value::Record(BTreeMap::from([ + ( + "entry".into(), + Value::String(object_path(&self.alias, &item.path)), + ), + ("kind".into(), Value::String(kind_name(item.kind).into())), + ( + "size".into(), + Value::Integer(i64::try_from(item.size).unwrap_or(i64::MAX)), + ), + ])) + }) + .collect() + } else { + result + .paths + .iter() + .map(|path| { + Value::Record(BTreeMap::from([( + "entry".into(), + Value::String(object_path(&self.alias, path)), + )])) + }) + .collect() + }; + fields.insert("items".into(), Value::List(items)); + let value = Value::Record(fields); + if let Some(raw) = result.validator { + Ok(WipOperationOutput::native_with_validator( + value, + self.provider + .wrap_validator(&self.alias, self.generation, &self.workdir, &raw), + )) + } else { + Ok(WipOperationOutput::native(value)) + } + } } #[async_trait] impl WipOperationHandler for FileHandler { @@ -628,58 +698,7 @@ impl WipOperationHandler for FileHandler { } })? .map_err(map_tool_error)?; - // Coordinates are not observations. Never prefetch unselected results; - // a vanished entry cannot fail a successful discovery Operation. - self.check_connection()?; - let mut fields = BTreeMap::from([("summary".into(), Value::String(result.output.summary))]); - if let Some(content) = result.output.content { - fields.insert("content".into(), Value::String(content)); - } - let items = if let Some(listing) = result.listing { - fields.insert("truncated".into(), Value::Boolean(listing.truncated)); - if let Some(after) = listing.next_after { - fields.insert("after".into(), cursor_value(&self.alias, &after)); - } - listing - .entries - .into_iter() - .map(|item| { - Value::Record(BTreeMap::from([ - ( - "entry".into(), - Value::String(object_path(&self.alias, &item.path)), - ), - ("kind".into(), Value::String(kind_name(item.kind).into())), - ( - "size".into(), - Value::Integer(i64::try_from(item.size).unwrap_or(i64::MAX)), - ), - ])) - }) - .collect() - } else { - result - .paths - .iter() - .map(|path| { - Value::Record(BTreeMap::from([( - "entry".into(), - Value::String(object_path(&self.alias, path)), - )])) - }) - .collect() - }; - fields.insert("items".into(), Value::List(items)); - let value = Value::Record(fields); - if let Some(raw) = result.validator { - Ok(WipOperationOutput::native_with_validator( - value, - self.provider - .wrap_validator(&self.alias, self.generation, &self.workdir, &raw), - )) - } else { - Ok(WipOperationOutput::native(value)) - } + self.publish_result(tool, result) } } diff --git a/crates/worker/src/checkout_race_tests.rs b/crates/worker/src/checkout_race_tests.rs index 15e1c6ade..da58fc64c 100644 --- a/crates/worker/src/checkout_race_tests.rs +++ b/crates/worker/src/checkout_race_tests.rs @@ -570,3 +570,200 @@ async fn checkout_content_children_never_call_provider_list_even_for_deep_paths( } assert_eq!(session.searches.load(Ordering::SeqCst), 0); } + +/// Deterministic adapter fixture for the production post-success publication +/// boundary. Execute through the real provider/Tools, then detach after the +/// routed operation guard has finished, before publishing the committed result. +/// No race timing or production-only test hook is needed. +struct DetachBeforePublication { + file: FileHandler, + calls: Arc, +} +#[async_trait] +impl WipOperationHandler for DetachBeforePublication { + async fn call( + &self, + operation: &str, + arguments: &BTreeMap, + context: WipCallContext, + ) -> Result { + self.calls.fetch_add(1, Ordering::SeqCst); + let tool = match operation { + "write" => "Write", + "edit" => "Edit", + "create_file" => "Create", + _ => panic!("mutation fixture only"), + }; + let result = tools::execute_checkout_tool( + self.file.provider.router.clone(), + self.file.provider.tracker.clone(), + &self.file.alias, + self.file.generation, + self.file.target.clone(), + self.file.observation.validator.clone(), + tool, + wip_to_json(&Value::Record(arguments.clone())).unwrap(), + context.execution, + ) + .await + .map_err(map_tool_error)?; + self.file + .provider + .router + .detach(&WorkdirAttachmentAlias::new(&self.file.alias).unwrap()) + .await + .unwrap(); + self.file.publish_result(tool, result) + } +} + +#[tokio::test] +async fn committed_checkout_mutation_losing_connection_is_unknown_not_rejected_or_replayed() { + use crate::wip::{WipAuditOutcome, WipRuntime}; + for (operation, arguments, target, result_path, expected) in [ + ( + "write", + json!({"content":"saved\n"}), + "a.txt", + "a.txt", + "saved\n", + ), + ( + "edit", + json!({"old_string":"old", "new_string":"changed"}), + "a.txt", + "a.txt", + "changed\n", + ), + ( + "create_file", + json!({"path":"new.txt", "content":"created\n"}), + "dir", + "dir/new.txt", + "created\n", + ), + ] { + let dir = TempDir::new().unwrap(); + std::fs::create_dir(dir.path().join("dir")).unwrap(); + std::fs::write(dir.path().join("a.txt"), "old\n").unwrap(); + let router = Arc::new(WorkdirSessionRouter::new()); + router + .attach( + WorkdirAttachmentAlias::new("main").unwrap(), + HookedSession::new(&dir), + ) + .unwrap(); + let p = provider(router.clone()); + if operation != "create_file" { + p.node("/checkouts/main/a.txt") + .await + .unwrap() + .unwrap() + .handler + .call("read", &BTreeMap::new(), context()) + .await + .unwrap_or_else(|_| panic!("prior Read failed")); + } + let selected = router.resolve(Some("main")).unwrap(); + let target = WorkdirPath::new(target).unwrap(); + let observation = selected + .session + .checkout_observe(target.clone()) + .await + .unwrap(); + let file = FileHandler { + provider: p.clone(), + alias: "main".into(), + generation: selected.generation, + workdir: selected.session.workdir().id().as_str().into(), + target, + observation, + }; + drop(selected); + let calls = Arc::new(AtomicUsize::new(0)); + let route = object_path("main", &file.target); + let mut projection = p.node(&route).await.unwrap().unwrap(); + let interface = projection.interface.clone(); + projection.handler = Arc::new(DetachBeforePublication { + file, + calls: calls.clone(), + }); + let mut registry = WipMountRegistry::new(); + registry + .allocate_namespace("checkout", "checkouts") + .unwrap(); + // Static observation metadata isolates the response boundary; real + // execution still uses checked provider authority and prior Read. + registry + .mount(p.node("/checkouts/main").await.unwrap().unwrap()) + .unwrap(); + registry.mount(projection).unwrap(); + let runtime = WipRuntime::from_mounts(registry, "post-commit-test".into()).unwrap(); + let error = runtime + .invoke( + route, + interface, + operation.into(), + arguments, + Default::default(), + ) + .await + .unwrap_err(); + assert!( + error.to_string().contains("outcome unknown"), + "{operation}: {error}" + ); + assert_eq!( + runtime.audit().last().unwrap().outcome, + WipAuditOutcome::OutcomeUnknown + ); + assert_eq!( + std::fs::read_to_string(dir.path().join(result_path)).unwrap(), + expected + ); + assert_eq!( + calls.load(Ordering::SeqCst), + 1, + "completed mutation must never replay" + ); + assert_eq!(runtime.metrics().operation_round_trips, 1); + } +} + +#[tokio::test] +async fn readonly_checkout_result_losing_connection_keeps_validator_mismatch() { + let dir = TempDir::new().unwrap(); + let router = Arc::new(WorkdirSessionRouter::new()); + let alias = WorkdirAttachmentAlias::new("main").unwrap(); + router + .attach(alias.clone(), HookedSession::new(&dir)) + .unwrap(); + let selected = router.resolve(Some("main")).unwrap(); + let file = FileHandler { + provider: provider(router.clone()), + alias: "main".into(), + generation: selected.generation, + workdir: selected.session.workdir().id().as_str().into(), + target: WorkdirPath::root(), + observation: selected + .session + .checkout_observe(WorkdirPath::root()) + .await + .unwrap(), + }; + drop(selected); + router.detach(&alias).await.unwrap(); + for tool in ["Read", "List", "Glob", "Grep"] { + let result = tools::CheckoutToolOutput { + output: agen::tool::ToolOutput { + summary: "finished".into(), + content: None, + attachments: vec![], + }, + paths: vec![], + listing: None, + validator: None, + }; + assert_validator_mismatch(file.publish_result(tool, result)); + } +} diff --git a/docs/development/wip-integration-validation.md b/docs/development/wip-integration-validation.md index 5dfb7ce5e..7b55c86e6 100644 --- a/docs/development/wip-integration-validation.md +++ b/docs/development/wip-integration-validation.md @@ -188,3 +188,25 @@ package/version set is unchanged. No full workspace test, full Nix/image build, deployment, target integration or live dogfood update is claimed. Target movement remains Orchestrator authority. + +### Post-success connection-loss review regression + +The first independent source review identified `POST_COMMIT_CONNECTION_FENCE`: +lazy result publication had propagated a connection mismatch as a pre-effect +rejection even after successful Write/Edit/Create. The production result +publication boundary now maps post-success connection loss for mutations to +`OutcomeUnknown`, while Read/List/Glob/Grep keep their validator mismatch. Lazy +entry mapping is unchanged; there is still no result-wide acquisition. + +`committed_checkout_mutation_losing_connection_is_unknown_not_rejected_or_replayed` +executes real checked Tools/provider mutations, deterministically detaches after +the routed execution guard finishes but before the same production publication +boundary, then passes the error through WIP Invoke/Client. It checks committed +file contents, unknown audit (not rejection), and exactly one dispatch for each +Write/Edit/Create. `readonly_checkout_result_losing_connection_keeps_validator_mismatch` +protects the non-mutating error policy. No sleep or production test hook is used. + +After this fix, all **9** checkout race tests and the complete Worker suite +(**903 unit + 110 integration**) passed, as did root Cargo check, formatting, +diff checks and released SDK resolution verification. The earlier **901** Worker +count above describes the pre-review combined-crate run.