Skip to content

Commit 893d7fe

Browse files
fix(native-sidecar): keep guest writes to API-seeded files (#2070)
Python and WASM/shell writes go only to the kernel VFS, so the VM root shadow kept the bytes from filesystem.writeFile. The next shadow-to-kernel sync saw the two sides differ and copied the stale shadow over the guest edit, and treated a guest rm/recreate as a host deletion. The shadow sync inventory now records a stamp for both sides at each reconciliation. A shadow change reaches the kernel only when the kernel node is unchanged since then, so every guest writer wins without a per-runtime mirror. Seeded bootstrap content still imports as before. Fixes #2022 and #1901. The Python test helper is adapted from #2023 by ankssjain. Co-authored-by: ankssjain <308424510+ankssjain@users.noreply.github.com>
1 parent 86a0d36 commit 893d7fe

5 files changed

Lines changed: 382 additions & 42 deletions

File tree

‎crates/native-sidecar/src/execution/launch.rs‎

Lines changed: 117 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -813,24 +813,96 @@ fn sync_vm_shadow_root_to_kernel(vm: &mut VmState) -> Result<(), SidecarError> {
813813
}
814814
}
815815

816-
let failed_replacements = propagate_shadow_deletions_to_kernel(vm, &mut snapshot.entries);
816+
let kernel_owned = kernel_owned_shadow_paths(vm, &snapshot);
817+
let failed_replacements = propagate_shadow_deletions_to_kernel(vm, &mut snapshot);
817818
let mut synced_file_times = BTreeMap::new();
818819
sync_host_directory_tree_to_kernel_inner(
819820
vm,
820821
&shadow_root,
821822
&shadow_root,
822823
"/",
823824
&mut synced_file_times,
824-
Some(&snapshot.entries),
825-
Some(&failed_replacements),
825+
Some(&ShadowSyncPlan {
826+
inventory: &snapshot.entries,
827+
failed_replacements: &failed_replacements,
828+
kernel_owned: &kernel_owned,
829+
}),
826830
)?;
831+
for (path, entry) in &mut snapshot.entries {
832+
if entry.deletion_pending {
833+
continue;
834+
}
835+
if let Some(shadow) = snapshot.shadow_stamps.get(path) {
836+
entry.baseline = Some(ShadowSyncBaseline {
837+
shadow: Some(*shadow),
838+
kernel: kernel_shadow_sync_stamp(vm, path),
839+
});
840+
}
841+
}
827842
vm.shadow_sync_inventory = snapshot.entries;
828843
Ok(())
829844
}
830845

846+
/// Every guest runtime writes the kernel VFS directly, and most of those
847+
/// writes never reach the shadow. A shadow change therefore applies to the
848+
/// kernel only when the kernel node is unchanged since the last
849+
/// reconciliation; otherwise the kernel holds the newer guest edit.
850+
fn shadow_change_applies_to_kernel(
851+
previous: Option<&ShadowSyncInventoryEntry>,
852+
shadow: Option<ShadowSyncStamp>,
853+
kernel: Option<ShadowSyncStamp>,
854+
) -> bool {
855+
match previous.map(|entry| entry.baseline) {
856+
// A shadow node that was never inventoried may only fill a kernel gap.
857+
None => kernel.is_none(),
858+
// Seeded shadow content that was never reconciled is authoritative.
859+
Some(None) => true,
860+
Some(Some(baseline)) => baseline.shadow != shadow && baseline.kernel == kernel,
861+
}
862+
}
863+
864+
/// Shadow paths whose kernel node must not be overwritten by this walk.
865+
fn kernel_owned_shadow_paths(vm: &mut VmState, snapshot: &ShadowSyncSnapshot) -> BTreeSet<String> {
866+
let mut kernel_owned = BTreeSet::new();
867+
for path in snapshot.entries.keys() {
868+
let kernel = kernel_shadow_sync_stamp(vm, path);
869+
if !shadow_change_applies_to_kernel(
870+
vm.shadow_sync_inventory.get(path),
871+
snapshot.shadow_stamps.get(path).copied(),
872+
kernel,
873+
) {
874+
kernel_owned.insert(path.clone());
875+
}
876+
}
877+
kernel_owned
878+
}
879+
880+
/// Trusted bookkeeping read of a kernel node. It must not require the guest
881+
/// `fs.stat` permission that a `KernelVm::lstat` would check.
882+
fn kernel_shadow_sync_stamp(vm: &mut VmState, guest_path: &str) -> Option<ShadowSyncStamp> {
883+
use agentos_kernel::vfs::VirtualFileSystem;
884+
vm.kernel
885+
.filesystem_mut()
886+
.inner_mut()
887+
.lstat(guest_path)
888+
.ok()
889+
.map(|stat| ShadowSyncStamp::kernel(&stat))
890+
}
891+
892+
/// Paths the additive shadow walk must leave alone.
893+
struct ShadowSyncPlan<'a> {
894+
/// The entry appeared after the inventory pass when it is missing here.
895+
inventory: &'a BTreeMap<String, ShadowSyncInventoryEntry>,
896+
/// Stale kernel objects whose removal failed.
897+
failed_replacements: &'a BTreeSet<String>,
898+
/// Kernel nodes that changed since the last reconciliation.
899+
kernel_owned: &'a BTreeSet<String>,
900+
}
901+
831902
#[derive(Default)]
832903
struct ShadowSyncSnapshot {
833904
entries: BTreeMap<String, ShadowSyncInventoryEntry>,
905+
shadow_stamps: BTreeMap<String, ShadowSyncStamp>,
834906
incomplete_subtrees: BTreeSet<String>,
835907
}
836908

@@ -922,10 +994,23 @@ fn collect_shadow_sync_snapshot_inner(
922994
} else {
923995
continue;
924996
};
997+
let metadata = match entry.metadata() {
998+
Ok(metadata) => metadata,
999+
Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue,
1000+
Err(error) => {
1001+
return Err(SidecarError::Io(format!(
1002+
"failed to inventory host shadow entry {}: {error}",
1003+
host_path.display()
1004+
)));
1005+
}
1006+
};
9251007
snapshot.entries.insert(
9261008
guest_path.clone(),
9271009
ShadowSyncInventoryEntry::present(node_type),
9281010
);
1011+
snapshot
1012+
.shadow_stamps
1013+
.insert(guest_path.clone(), ShadowSyncStamp::host(&metadata));
9291014
if node_type == ShadowNodeType::Directory {
9301015
collect_shadow_sync_snapshot_inner(shadow_root, &host_path, &guest_path, snapshot)?;
9311016
}
@@ -942,8 +1027,9 @@ fn collect_shadow_sync_snapshot_inner(
9421027
/// leaves before the directories that contain them.
9431028
fn propagate_shadow_deletions_to_kernel(
9441029
vm: &mut VmState,
945-
current: &mut BTreeMap<String, ShadowSyncInventoryEntry>,
1030+
snapshot: &mut ShadowSyncSnapshot,
9461031
) -> BTreeSet<String> {
1032+
let current = &mut snapshot.entries;
9471033
let stale = vm
9481034
.shadow_sync_inventory
9491035
.iter()
@@ -962,6 +1048,16 @@ fn propagate_shadow_deletions_to_kernel(
9621048
{
9631049
continue;
9641050
}
1051+
let kernel = kernel_shadow_sync_stamp(vm, &path);
1052+
if !shadow_change_applies_to_kernel(
1053+
Some(&previous),
1054+
snapshot.shadow_stamps.get(&path).copied(),
1055+
kernel,
1056+
) {
1057+
// The kernel node was replaced or edited after the shadow copy
1058+
// went stale (for example `rm f && echo new > f` in a guest).
1059+
continue;
1060+
}
9651061
let stat = match vm.kernel.lstat(&path) {
9661062
Ok(stat) => stat,
9671063
Err(error) if error.code() == "ENOENT" => continue,
@@ -1003,10 +1099,13 @@ fn retain_shadow_deletion_tombstone(
10031099
) {
10041100
current
10051101
.entry(path.to_owned())
1006-
.and_modify(|entry| entry.deletion_pending = true)
1102+
.and_modify(|entry| {
1103+
entry.deletion_pending = true;
1104+
entry.baseline = previous.baseline;
1105+
})
10071106
.or_insert(ShadowSyncInventoryEntry {
1008-
node_type: previous.node_type,
10091107
deletion_pending: true,
1108+
..previous
10101109
});
10111110
}
10121111

@@ -1115,7 +1214,6 @@ fn sync_host_directory_tree_to_kernel(
11151214
&normalized_guest_root,
11161215
&mut synced_file_times,
11171216
None,
1118-
None,
11191217
)
11201218
}
11211219

@@ -1134,8 +1232,7 @@ fn sync_host_directory_tree_to_kernel_inner(
11341232
current_host_dir: &Path,
11351233
guest_root: &str,
11361234
synced_file_times: &mut BTreeMap<(u64, u64), (u64, u64)>,
1137-
expected_inventory: Option<&BTreeMap<String, ShadowSyncInventoryEntry>>,
1138-
failed_replacements: Option<&BTreeSet<String>>,
1235+
plan: Option<&ShadowSyncPlan<'_>>,
11391236
) -> Result<(), SidecarError> {
11401237
let entries = match fs::read_dir(current_host_dir) {
11411238
Ok(entries) => entries,
@@ -1196,26 +1293,29 @@ fn sync_host_directory_tree_to_kernel_inner(
11961293
if should_skip_shadow_sync_path(vm, &guest_path) {
11971294
continue;
11981295
}
1199-
if expected_inventory.is_some_and(|inventory| !inventory.contains_key(&guest_path)) {
1296+
if plan.is_some_and(|plan| !plan.inventory.contains_key(&guest_path)) {
12001297
// The entry appeared after the inventory pass. Pick it up on the
12011298
// next sync rather than mixing two different shadow snapshots.
12021299
continue;
12031300
}
1204-
if failed_replacements.is_some_and(|failed| failed.contains(&guest_path)) {
1301+
if plan.is_some_and(|plan| plan.failed_replacements.contains(&guest_path)) {
12051302
// Never write through a stale object whose removal failed. The
12061303
// retained tombstone will retry before a future additive write.
12071304
continue;
12081305
}
1306+
let kernel_owned = plan.is_some_and(|plan| plan.kernel_owned.contains(&guest_path));
12091307

12101308
if file_type.is_dir() {
1211-
ensure_kernel_shadow_node_type(vm, &guest_path, ShadowNodeType::Directory)?;
1309+
if !kernel_owned {
1310+
ensure_kernel_shadow_node_type(vm, &guest_path, ShadowNodeType::Directory)?;
1311+
}
12121312
let metadata = entry.metadata().map_err(|error| {
12131313
SidecarError::Io(format!(
12141314
"failed to read host shadow metadata {}: {error}",
12151315
host_path.display()
12161316
))
12171317
})?;
1218-
if !is_shadow_bootstrap_dir(&guest_path) {
1318+
if !kernel_owned && !is_shadow_bootstrap_dir(&guest_path) {
12191319
if !vm.kernel.exists(&guest_path).unwrap_or(false) {
12201320
vm.kernel.mkdir(&guest_path, true).map_err(|error| {
12211321
SidecarError::InvalidState(format!(
@@ -1243,11 +1343,13 @@ fn sync_host_directory_tree_to_kernel_inner(
12431343
&host_path,
12441344
guest_root,
12451345
synced_file_times,
1246-
expected_inventory,
1247-
failed_replacements,
1346+
plan,
12481347
)?;
12491348
continue;
12501349
}
1350+
if kernel_owned {
1351+
continue;
1352+
}
12511353

12521354
if file_type.is_file() {
12531355
ensure_kernel_shadow_node_type(vm, &guest_path, ShadowNodeType::File)?;

‎crates/native-sidecar/src/execution/mod.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,9 @@ use crate::state::{
153153
PendingTcpSocket, PendingUnixConnectionGuard, PendingUnixSocket, PlainSocketWritePayload,
154154
ProcNetEntry, ProcessEventEnvelope, PythonHostSocket, PythonSocketConnectCompletion,
155155
PythonTcpReadBuffer, QueuedHttp2Command, QueuedHttp2Event, ReactorIoLimits,
156-
ResolvedChildProcessExecution, ResolvedTcpConnectAddr, ShadowNodeType,
157-
ShadowSyncInventoryEntry, SharedBridge, SharedSidecarRequestClient, SidecarKernel,
158-
SocketDescriptionLease, SocketQueryKind, SocketReadinessRegistration,
156+
ResolvedChildProcessExecution, ResolvedTcpConnectAddr, ShadowNodeType, ShadowSyncBaseline,
157+
ShadowSyncInventoryEntry, ShadowSyncStamp, SharedBridge, SharedSidecarRequestClient,
158+
SidecarKernel, SocketDescriptionLease, SocketQueryKind, SocketReadinessRegistration,
159159
SocketReadinessSubscribers, TlsWritePayload, VmDnsConfig, VmFetchBodyMode, VmFetchStreamState,
160160
VmHandle, VmListenPolicy, VmPendingByteBudget, VmState, DEFAULT_JAVASCRIPT_NET_BACKLOG,
161161
EXECUTION_DRIVER_NAME, EXECUTION_SANDBOX_ROOT_ENV, HOST_FUNCTION_DRIVER_NAME,

‎crates/native-sidecar/src/filesystem.rs‎

Lines changed: 48 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,9 @@ use crate::service::{
1717
python_error,
1818
};
1919
use crate::state::{
20-
ActiveExecutionEvent, ActiveProcess, BridgeError, ShadowNodeType, ShadowSyncInventoryEntry,
21-
SharedBridge, SidecarKernel, VmHandle, VmState, EXECUTION_DRIVER_NAME,
22-
PYTHON_VFS_RPC_GUEST_ROOT,
20+
ActiveExecutionEvent, ActiveProcess, BridgeError, ShadowNodeType, ShadowSyncBaseline,
21+
ShadowSyncInventoryEntry, ShadowSyncStamp, SharedBridge, SidecarKernel, VmHandle, VmState,
22+
EXECUTION_DRIVER_NAME, PYTHON_VFS_RPC_GUEST_ROOT,
2323
};
2424
use crate::{DispatchResult, NativeSidecar, NativeSidecarBridge, SidecarError};
2525

@@ -736,23 +736,20 @@ fn refresh_shadow_inventory_path(vm: &mut VmState, guest_path: &str) -> Result<(
736736
}
737737
let guest_path = normalize_path(guest_path);
738738
let mut updates = collect_shadow_inventory_ancestors(vm, &guest_path)?;
739-
let Some(node_type) = shadow_inventory_kernel_node_type(vm, &guest_path)? else {
739+
let Some(entry) = shadow_inventory_entry(vm, &guest_path)? else {
740740
forget_shadow_inventory_path(vm, &guest_path);
741741
return Ok(());
742742
};
743-
updates.insert(
744-
guest_path.clone(),
745-
ShadowSyncInventoryEntry::present(node_type),
746-
);
747-
if node_type == ShadowNodeType::Directory {
743+
updates.insert(guest_path.clone(), entry);
744+
if entry.node_type == ShadowNodeType::Directory {
748745
let entries = vm
749746
.kernel
750747
.read_dir_recursive(&guest_path, None)
751748
.map_err(kernel_error)?;
752749
for entry in entries {
753750
let path = normalize_path(&entry.path);
754-
if let Some(node_type) = shadow_inventory_kernel_node_type(vm, &path)? {
755-
updates.insert(path, ShadowSyncInventoryEntry::present(node_type));
751+
if let Some(entry) = shadow_inventory_entry(vm, &path)? {
752+
updates.insert(path, entry);
756753
}
757754
}
758755
}
@@ -772,11 +769,8 @@ fn refresh_shadow_inventory_path(vm: &mut VmState, guest_path: &str) -> Result<(
772769
fn refresh_shadow_inventory_node(vm: &mut VmState, guest_path: &str) -> Result<(), SidecarError> {
773770
let guest_path = normalize_path(guest_path);
774771
let mut updates = collect_shadow_inventory_ancestors(vm, &guest_path)?;
775-
if let Some(node_type) = shadow_inventory_kernel_node_type(vm, &guest_path)? {
776-
updates.insert(
777-
guest_path.clone(),
778-
ShadowSyncInventoryEntry::present(node_type),
779-
);
772+
if let Some(entry) = shadow_inventory_entry(vm, &guest_path)? {
773+
updates.insert(guest_path.clone(), entry);
780774
}
781775
vm.shadow_sync_inventory.extend(updates);
782776
Ok(())
@@ -801,20 +795,21 @@ fn collect_shadow_inventory_ancestors(
801795
}
802796
let mut updates = BTreeMap::new();
803797
for ancestor in ancestors.into_iter().rev() {
804-
if shadow_inventory_kernel_node_type(vm, &ancestor)? == Some(ShadowNodeType::Directory) {
805-
updates.insert(
806-
ancestor,
807-
ShadowSyncInventoryEntry::present(ShadowNodeType::Directory),
808-
);
798+
if let Some(entry) = shadow_inventory_entry(vm, &ancestor)? {
799+
if entry.node_type == ShadowNodeType::Directory {
800+
updates.insert(ancestor, entry);
801+
}
809802
}
810803
}
811804
Ok(updates)
812805
}
813806

814-
fn shadow_inventory_kernel_node_type(
807+
/// Inventory a path whose kernel state the sidecar just mirrored into the
808+
/// shadow, recording both sides as reconciled.
809+
fn shadow_inventory_entry(
815810
vm: &mut VmState,
816811
guest_path: &str,
817-
) -> Result<Option<ShadowNodeType>, SidecarError> {
812+
) -> Result<Option<ShadowSyncInventoryEntry>, SidecarError> {
818813
// This inventory is trusted sidecar bookkeeping after the guest mutation has
819814
// already passed its operation-specific permission check. Re-entering through
820815
// `KernelVm::lstat` would incorrectly require a separate guest `fs.stat`
@@ -831,7 +826,24 @@ fn shadow_inventory_kernel_node_type(
831826
} else {
832827
ShadowNodeType::File
833828
};
834-
Ok(Some(node_type))
829+
let shadow_path = shadow_host_path_for_guest(&vm.cwd, guest_path);
830+
let shadow = match fs::symlink_metadata(&shadow_path) {
831+
Ok(metadata) => Some(ShadowSyncStamp::host(&metadata)),
832+
Err(error) if error.kind() == std::io::ErrorKind::NotFound => None,
833+
Err(error) => {
834+
return Err(SidecarError::Io(format!(
835+
"failed to inspect shadow path {}: {error}",
836+
shadow_path.display()
837+
)));
838+
}
839+
};
840+
Ok(Some(ShadowSyncInventoryEntry::reconciled(
841+
node_type,
842+
ShadowSyncBaseline {
843+
shadow,
844+
kernel: Some(ShadowSyncStamp::kernel(&stat)),
845+
},
846+
)))
835847
}
836848

837849
fn forget_shadow_inventory_path(vm: &mut VmState, guest_path: &str) {
@@ -5377,8 +5389,20 @@ fn sync_active_shadow_path_to_kernel(
53775389
if host_paths.is_empty() && !vm.kernel.exists(&guest_path).unwrap_or(false) {
53785390
host_paths.push(shadow_host_path_for_guest(&vm.cwd, &guest_path));
53795391
}
5392+
// The root walk above already decided which side wins for a reconciled
5393+
// shadow path; importing it again here would undo a newer kernel edit.
5394+
let reconciled_in_root_shadow = vm
5395+
.shadow_sync_inventory
5396+
.get(&guest_path)
5397+
.is_some_and(|entry| entry.baseline.is_some());
5398+
let vm_shadow_root = normalize_host_path(&vm.cwd);
53805399

53815400
for host_path in host_paths {
5401+
if reconciled_in_root_shadow
5402+
&& path_is_within_root(&normalize_host_path(&host_path), &vm_shadow_root)
5403+
{
5404+
continue;
5405+
}
53825406
let metadata = match fs::symlink_metadata(&host_path) {
53835407
Ok(metadata) => metadata,
53845408
Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue,

0 commit comments

Comments
 (0)