fix(pi): bind native publication to durable ownership

This commit is contained in:
SaladDay
2026-08-03 05:17:33 +00:00
parent 0b609158d7
commit a221e086a3
5 changed files with 778 additions and 276 deletions
+108 -96
View File
@@ -379,7 +379,7 @@ fn apply_pi_provider_patch_checked(
expected: Option<&IndexMap<String, Option<Value>>>,
expected_fingerprints: Option<&IndexMap<String, String>>,
patch: &IndexMap<String, Option<Value>>,
) -> Result<bool, AppError> {
) -> Result<PiDocumentCommit, AppError> {
let lock = path_lock(path)?;
let _guard = lock_path(&lock)?;
if let Some(parent) = path.parent() {
@@ -395,7 +395,7 @@ fn apply_pi_provider_patch_checked(
ensure_provider_fingerprints_match(path, before.as_deref(), expected_fingerprints)?;
}
if before.is_none() && patch.values().all(Option::is_none) {
return Ok(false);
return Ok(PiDocumentCommit { bytes: None });
}
let serialized = serialize_models_mutation(path, before.as_deref(), &|document| {
let providers = document
@@ -416,7 +416,7 @@ fn apply_pi_provider_patch_checked(
Ok(())
})?;
if before.as_deref() == Some(serialized.as_slice()) {
return Ok(false);
return Ok(PiDocumentCommit { bytes: before });
}
match compare_exchange_shared_file_bytes(
@@ -427,7 +427,11 @@ fn apply_pi_provider_patch_checked(
None,
"Pi models file",
) {
Ok(_) => return Ok(true),
Ok(_) => {
return Ok(PiDocumentCommit {
bytes: Some(serialized),
})
}
Err(AppError::Conflict(_)) => continue,
Err(error) => return Err(error),
}
@@ -439,6 +443,10 @@ fn apply_pi_provider_patch_checked(
)))
}
struct PiDocumentCommit {
bytes: Option<Vec<u8>>,
}
fn provider_values_from_bytes<'a>(
path: &Path,
bytes: Option<&[u8]>,
@@ -503,6 +511,25 @@ fn ensure_provider_fingerprints_match(
Ok(())
}
fn provider_fingerprints_from_bytes<'a>(
path: &Path,
bytes: Option<&[u8]>,
provider_keys: impl IntoIterator<Item = &'a String>,
) -> Result<IndexMap<String, String>, AppError> {
let document = parse_pi_models_document(path, bytes)?;
Ok(provider_keys
.into_iter()
.filter_map(|provider_key| {
document.providers().get(provider_key).map(|entry| {
(
provider_key.clone(),
pi_raw_provider_fingerprint(&entry.raw_source),
)
})
})
.collect())
}
fn provider_value_label(value: &Option<Value>) -> &'static str {
if value.is_some() {
"present"
@@ -522,6 +549,7 @@ pub(crate) struct PiProviderPatchReceipt {
path: PathBuf,
before: PiProviderValuesSnapshot,
attempted: IndexMap<String, Option<Value>>,
attempted_fingerprints: IndexMap<String, String>,
attempted_file_existed: bool,
}
@@ -545,7 +573,7 @@ impl PiProviderPatchReceipt {
if let Err(error) = apply_pi_provider_patch_checked(
&self.path,
Some(&self.attempted),
None,
Some(&self.attempted_fingerprints),
&self.before.values,
) {
let observed =
@@ -654,61 +682,6 @@ fn attempted_provider_values(
.collect()
}
fn failed_publish_after_conditional_compensation(
path: &Path,
before: &PiProviderValuesSnapshot,
attempted: &IndexMap<String, Option<Value>>,
publish_error: AppError,
) -> AppError {
let retryable_conflict = matches!(&publish_error, AppError::Conflict(_));
let observed = match snapshot_pi_provider_values(path, before.values.keys().cloned()) {
Ok(observed) => observed,
Err(observe_error) => {
return AppError::Config(format!(
"Pi provider publication failed ({publish_error}) and its commit state could not \
be inspected for compensation: {observe_error}"
));
}
};
if observed.values == before.values {
if !retryable_conflict {
if let Err(sync_error) = sync_shared_file_parent(path) {
return AppError::Config(format!(
"Pi provider publication failed ({publish_error}); the previous exact-key state \
is visible but directory durability could not be confirmed: {sync_error}"
));
}
}
return publish_error;
}
if observed.values != *attempted {
if retryable_conflict {
// A normal compare/exchange conflict must remain retryable. Its
// canonical value belongs to the concurrent writer; any displaced
// recovery artifact is already named in the lower-level error.
return publish_error;
}
return AppError::Config(format!(
"Pi provider publication failed ({publish_error}); the live exact-key state was \
superseded before conditional compensation and was preserved"
));
}
let receipt = PiProviderPatchReceipt {
path: path.to_path_buf(),
before: before.clone(),
attempted: attempted.clone(),
attempted_file_existed: observed.file_existed,
};
match receipt.rollback() {
Ok(()) => publish_error,
Err(rollback_error) => AppError::Config(format!(
"Pi provider publication failed ({publish_error}) after the attempted exact-key state \
became live; conditional compensation also failed: {rollback_error}"
)),
}
}
/// Publish an exact-key patch only if every preflighted provider value still
/// matches. Whole-file CAS retries preserve unrelated edits, but re-check the
/// provider precondition before every retry.
@@ -737,24 +710,16 @@ pub(crate) fn apply_pi_provider_patch_with_receipt_and_fingerprints(
));
}
let attempted = attempted_provider_values(before, patch);
let changed = match apply_pi_provider_patch_checked(
path,
Some(&before.values),
expected_fingerprints,
patch,
) {
Ok(changed) => changed,
Err(error) => {
return Err(failed_publish_after_conditional_compensation(
path, before, &attempted, error,
));
}
};
let commit =
apply_pi_provider_patch_checked(path, Some(&before.values), expected_fingerprints, patch)?;
let attempted_fingerprints =
provider_fingerprints_from_bytes(path, commit.bytes.as_deref(), attempted.keys())?;
Ok(PiProviderPatchReceipt {
path: path.to_path_buf(),
before: before.clone(),
attempted,
attempted_file_existed: before.file_existed || changed,
attempted_fingerprints,
attempted_file_existed: commit.bytes.is_some(),
})
}
@@ -899,11 +864,10 @@ mod tests {
&IndexMap::from([("managed".to_string(), Some(serde_json::json!({"api": "x"})))]),
)
.expect("publish");
fs::write(
&path,
"{\n \"providers\": {\"managed\": {\"api\": \"x\"}},\n \"external\": true\n}\n",
)
.expect("external root update");
let mut external = fs::read_to_string(&path).expect("published document");
let root_end = external.rfind("\n}").expect("root closing brace");
external.insert_str(root_end, ",\n \"external\": true");
fs::write(&path, external).expect("external root-only update");
receipt.rollback().expect("restore managed key");
let restored: Value =
serde_json::from_slice(&fs::read(&path).expect("external file retained"))
@@ -1056,7 +1020,7 @@ mod tests {
}
#[test]
fn failed_conflict_recovery_after_replace_compensates_attempted_provider_state() {
fn transient_conflict_recovery_failure_still_restores_external_provider_state() {
let temp = tempfile::tempdir().expect("tempdir");
let path = temp.path().join("models.json");
fs::write(&path, br#"{"providers":{"managed":{"api":"before"}}}"#).expect("seed");
@@ -1077,18 +1041,11 @@ mod tests {
.expect_err("an uncertain conflict must remain an error");
assert!(matches!(error, AppError::Conflict(_)));
let restored = read_pi_models_document(&path).expect("restored document");
assert_eq!(restored.providers()["managed"].value["api"], "before");
assert!(
fs::read_dir(temp.path())
.expect("recovery directory")
.filter_map(Result::ok)
.any(|entry| fs::read(entry.path()).ok().as_deref() == Some(external)),
"the displaced external version must remain recoverable"
);
assert_eq!(restored.providers()["managed"].value["api"], "external");
}
#[test]
fn failed_conflict_recovery_after_delete_compensates_attempted_absence() {
fn transient_delete_recovery_failure_still_restores_external_provider_state() {
let temp = tempfile::tempdir().expect("tempdir");
let path = temp.path().join("models.json");
fs::write(&path, br#"{"providers":{"managed":{"api":"before"}}}"#).expect("seed");
@@ -1106,16 +1063,71 @@ mod tests {
.expect_err("an uncertain delete conflict must remain an error");
assert!(matches!(error, AppError::Conflict(_)));
let restored = read_pi_models_document(&path).expect("restored document");
assert_eq!(restored.providers()["managed"].value["api"], "before");
assert!(
fs::read_dir(temp.path())
.expect("recovery directory")
.filter_map(Result::ok)
.any(|entry| fs::read(entry.path()).ok().as_deref() == Some(external)),
"the quarantined external version must remain recoverable"
assert_eq!(restored.providers()["managed"].value["api"], "external");
}
#[test]
fn precondition_conflict_never_compensates_an_external_same_value_writer() {
let temp = tempfile::tempdir().expect("tempdir");
let path = temp.path().join("models.json");
let before =
snapshot_pi_provider_values(&path, ["managed".to_string()]).expect("preflight");
let external = br#"{
"providers": {
// external ownership and formatting must survive
"managed": {"api": "attempted"}
}
}"#;
crate::pi_config::shared_file::replace_before_next_compare_exchange(&path, external);
let error = apply_pi_provider_patch_with_receipt(
&path,
&before,
&IndexMap::from([(
"managed".to_string(),
Some(serde_json::json!({"api": "attempted"})),
)]),
)
.expect_err("the external create must own the conflict");
assert!(matches!(error, AppError::Conflict(_)));
assert_eq!(
fs::read(&path).expect("external document preserved"),
external,
"semantic equality must never be used as writer identity"
);
}
#[test]
fn receipt_rollback_rejects_same_value_entry_with_new_raw_ownership() {
let temp = tempfile::tempdir().expect("tempdir");
let path = temp.path().join("models.json");
fs::write(&path, br#"{"providers":{"managed":{"api":"before"}}}"#).expect("seed");
let before =
snapshot_pi_provider_values(&path, ["managed".to_string()]).expect("preflight");
let receipt = apply_pi_provider_patch_with_receipt(
&path,
&before,
&IndexMap::from([(
"managed".to_string(),
Some(serde_json::json!({"api": "attempted"})),
)]),
)
.expect("publish");
let external = br#"{
"providers": {
// same value, independently published raw entry
"managed": { "api": "attempted" }
}
}"#;
fs::write(&path, external).expect("external same-value rewrite");
let error = receipt
.rollback()
.expect_err("raw ownership change must stop compensation");
assert!(matches!(error, AppError::Conflict(_)));
assert_eq!(fs::read(&path).expect("external retained"), external);
}
#[test]
fn external_rename_during_patch_is_reparsed_before_owned_fields_change() {
let temp = tempfile::tempdir().expect("tempdir");
+47 -9
View File
@@ -206,7 +206,7 @@ impl fmt::Debug for CandidateHeaderPlan {
formatter
.debug_struct("CandidateHeaderPlan")
.field("family", &self.family)
.field("endpoint", &self.endpoint)
.field("endpoint", &"<redacted>")
.field("credential", &"<redacted>")
.field("auth_header", &self.auth_header)
.field("provider_headers", &self.provider_headers)
@@ -242,7 +242,7 @@ impl fmt::Debug for MaterializedCandidate {
let protocol_header_names = self.protocol_headers.keys().collect::<Vec<_>>();
formatter
.debug_struct("MaterializedCandidate")
.field("endpoint", &self.endpoint)
.field("endpoint", &"<redacted>")
.field("header_names", &header_names)
.field("family", &self.family)
.field("protocol_header_names", &protocol_header_names)
@@ -282,11 +282,7 @@ impl CandidateHeaderPlan {
return Err(reasons);
};
let endpoint = match Url::parse(&model.base_url) {
Ok(endpoint)
if matches!(endpoint.scheme(), "http" | "https") && endpoint.host().is_some() =>
{
endpoint
}
Ok(endpoint) if valid_gateway_endpoint(&endpoint) => endpoint,
_ => {
reasons.push(PiGatewayReason {
code: PiGatewayReasonCode::InvalidEndpoint,
@@ -381,7 +377,7 @@ impl CandidateHeaderPlan {
code: PiGatewayReasonCode::InvalidEndpoint,
json_pointer: "/customEndpoints".to_string(),
})?;
if !matches!(endpoint.scheme(), "http" | "https") || endpoint.host().is_none() {
if !valid_gateway_endpoint(&endpoint) {
return Err(PiGatewayReason {
code: PiGatewayReasonCode::InvalidEndpoint,
json_pointer: "/customEndpoints".to_string(),
@@ -784,6 +780,13 @@ fn parse_transport_header_value(value: &str) -> Option<HeaderValue> {
HeaderValue::from_str(value).ok()
}
fn valid_gateway_endpoint(endpoint: &Url) -> bool {
matches!(endpoint.scheme(), "http" | "https")
&& endpoint.host().is_some()
&& endpoint.username().is_empty()
&& endpoint.password().is_none()
}
#[cfg(test)]
mod tests {
use super::*;
@@ -820,9 +823,10 @@ mod tests {
fn sensitive_gateway_debug_output_redacts_credentials_and_header_values() {
let credential = "sk-debug-credential-never-log";
let header_secret = "debug-header-value-never-log";
let query_secret = "debug-query-never-log";
let composition = composed(json!({
"api": "openai-responses",
"baseUrl": "https://example.test/v1",
"baseUrl": format!("https://example.test/v1?token={query_secret}"),
"apiKey": credential,
"headers": {"x-private": header_secret},
"models": [{"id": "m"}]
@@ -832,6 +836,7 @@ mod tests {
let assessment_debug = format!("{assessment:?}");
assert!(!assessment_debug.contains(credential));
assert!(!assessment_debug.contains(header_secret));
assert!(!assessment_debug.contains(query_secret));
let materialized = assessment.plans[0]
.materialize(&|_expression: &str| None)
@@ -839,10 +844,43 @@ mod tests {
let materialized_debug = format!("{materialized:?}");
assert!(!materialized_debug.contains(credential));
assert!(!materialized_debug.contains(header_secret));
assert!(!materialized_debug.contains(query_secret));
assert!(materialized_debug.contains("authorization"));
assert!(materialized_debug.contains("x-private"));
}
#[test]
fn gateway_rejects_endpoint_userinfo_and_never_debugs_it() {
let userinfo_secret = "userinfo-secret-never-log";
let composition = composed(json!({
"api": "openai-responses",
"baseUrl": format!("https://user:{userinfo_secret}@example.test/v1"),
"apiKey": "credential",
"models": [{"id": "m"}]
}));
let assessment = assess_composition(&composition);
assert_eq!(assessment.capability, PiGatewayCapability::DirectOnly);
assert_eq!(
assessment.reasons[0].code,
PiGatewayReasonCode::InvalidEndpoint
);
assert!(!format!("{assessment:?}").contains(userinfo_secret));
let safe = composed(json!({
"api": "openai-responses",
"baseUrl": "https://example.test/v1",
"apiKey": "credential",
"models": [{"id": "m"}]
}));
let plan = assess_composition(&safe).plans.remove(0);
let error = plan
.with_endpoint(&format!(
"https://user:{userinfo_secret}@failover.example/v1"
))
.expect_err("custom endpoint userinfo must be rejected");
assert_eq!(error.code, PiGatewayReasonCode::InvalidEndpoint);
}
#[test]
fn unknown_api_is_composed_but_direct_only() {
let composition = composed(json!({
+428 -110
View File
@@ -33,6 +33,12 @@ static FAIL_NEXT_ROLLBACK_RESTORE: LazyLock<Mutex<HashMap<PathBuf, usize>>> =
static FAIL_NEXT_PARENT_SYNC: LazyLock<Mutex<HashMap<PathBuf, usize>>> =
LazyLock::new(|| Mutex::new(HashMap::new()));
#[derive(Debug)]
struct StagedReplacement {
path: PathBuf,
identity: Metadata,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub(crate) struct SharedFileSnapshot {
pub revision: String,
@@ -183,14 +189,54 @@ fn compare_exchange_under_lock(
.transpose()?;
run_before_compare_exchange_hook(path)?;
let result = match (expected, replacement, staged.as_deref()) {
(None, Some(bytes), Some(staged)) => match rename_noreplace(staged, path) {
let result = match (expected, replacement, staged.as_ref()) {
(None, Some(bytes), Some(staged)) => match rename_noreplace(&staged.path, path) {
Ok(()) => {
sync_parent(parent)?;
Ok(snapshot(Some(bytes)))
match sync_parent(parent) {
Ok(()) => Ok(snapshot(Some(bytes))),
Err(publish_error) => {
match rollback_installed_file(
path,
&staged.identity,
bytes,
None,
parent,
max_bytes,
label,
) {
Ok(InstalledRollback::Restored) => Err(publish_error),
Ok(InstalledRollback::Superseded) => Err(AppError::Config(format!(
"{label} create lost its durability barrier ({publish_error}); \
a concurrent external state won and was preserved"
))),
Err(rollback_error) => {
if path_is_installed(
path,
&staged.identity,
bytes,
max_bytes,
label,
) && sync_parent(parent).is_ok()
{
// A rollback can fail for reasons unrelated to
// the now-visible canonical file. A successful
// second durability barrier makes the only
// honest outcome a committed success.
Ok(snapshot(Some(bytes)))
} else {
Err(ambiguous_publication(
label,
&publish_error,
&rollback_error,
))
}
}
}
}
}
}
Err(error) if is_destination_exists(&error) => Err(concurrent_change(path, label)),
Err(error) => Err(rename_error("create", staged, path, error)),
Err(error) => Err(rename_error("create", &staged.path, path, error)),
},
(Some(expected), Some(replacement), Some(staged)) => {
replace_existing_if_equal(path, staged, expected, replacement, max_bytes, label)
@@ -203,32 +249,27 @@ fn compare_exchange_under_lock(
};
if let Some(staged) = staged {
// Success consumes or removes the staged path. On a failed rollback it
// intentionally remains as a recovery artifact containing data that
// must not be discarded, so only remove a file still containing our
// proposed replacement.
if replacement.is_some_and(|bytes| {
read_regular_bytes(&staged, max_bytes, label)
.ok()
.flatten()
.as_deref()
== Some(bytes)
}) {
let _ = fs::remove_file(&staged);
}
// Never use byte equality as writer identity: an external writer may
// independently publish the same bytes with different ownership.
// Staging cleanup is safe only while the original staged inode/file-id
// remains at this private path.
let _ = remove_file_if_identity(&staged.path, &staged.identity);
}
result
}
fn replace_existing_if_equal(
path: &Path,
staged: &Path,
staged: &StagedReplacement,
expected: &[u8],
replacement: &[u8],
max_bytes: u64,
label: &str,
) -> Result<SharedFileSnapshot, AppError> {
let displaced = match install_over_existing(staged, path) {
let parent = path
.parent()
.expect("a path accepted by compare/exchange has a parent");
let displaced = match install_over_existing(&staged.path, path) {
Ok(displaced) => displaced,
Err(error)
if matches!(
@@ -238,58 +279,74 @@ fn replace_existing_if_equal(
{
return Err(concurrent_change(path, label));
}
Err(error) => return Err(rename_error("exchange", staged, path, error)),
Err(error) => return Err(rename_error("exchange", &staged.path, path, error)),
};
let displaced_bytes = read_regular_bytes(&displaced, max_bytes, label);
if matches!(
displaced_bytes,
Ok(ref bytes) if bytes.as_deref() == Some(expected)
) {
fs::remove_file(&displaced).map_err(|error| AppError::io(&displaced, error))?;
sync_parent(
path.parent()
.expect("a path accepted by compare/exchange has a parent"),
)?;
return Ok(snapshot(Some(replacement)));
return match sync_parent(parent) {
Ok(()) => {
finalize_recovery_artifact(&displaced, parent, label);
Ok(snapshot(Some(replacement)))
}
Err(publish_error) => match rollback_installed_file(
path,
&staged.identity,
replacement,
Some(&displaced),
parent,
max_bytes,
label,
) {
Ok(InstalledRollback::Restored) => Err(publish_error),
Ok(InstalledRollback::Superseded) => Err(AppError::Config(format!(
"{label} replacement lost its durability barrier ({publish_error}); \
a concurrent external state won and was preserved"
))),
Err(rollback_error) => {
if path_is_installed(path, &staged.identity, replacement, max_bytes, label)
&& sync_parent(parent).is_ok()
{
finalize_recovery_artifact(&displaced, parent, label);
Ok(snapshot(Some(replacement)))
} else {
Err(ambiguous_publication(
label,
&publish_error,
&rollback_error,
))
}
}
},
};
}
run_before_rollback_exchange_hook(path)?;
let proposed_or_raced = run_before_rollback_restore_hook(path)
.and_then(|()| restore_displaced(&displaced, path))
.map_err(|error| {
AppError::Conflict(format!(
"{label} changed during atomic replacement and could not be restored; \
match rollback_installed_file(
path,
&staged.identity,
replacement,
Some(&displaced),
parent,
max_bytes,
label,
) {
Ok(InstalledRollback::Restored) => match displaced_bytes {
Ok(_) => Err(concurrent_change(path, label)),
Err(error) => Err(AppError::Conflict(format!(
"{label} became unsafe during atomic replacement and was restored: {error}"
))),
},
Ok(InstalledRollback::Superseded) => Err(AppError::Config(format!(
"{label} changed again during rollback; all external bytes were preserved \
and require explicit recovery/reconciliation"
))),
Err(error) => Err(AppError::Config(format!(
"{label} changed during atomic replacement and could not be restored safely; \
the displaced bytes remain at {}: {error}",
displaced.display()
))
})?;
let proposed_bytes = read_regular_bytes(&proposed_or_raced, max_bytes, label)?;
if proposed_bytes.as_deref() == Some(replacement) {
fs::remove_file(&proposed_or_raced)
.map_err(|error| AppError::io(&proposed_or_raced, error))?;
} else {
// A second external replacement won the namespace after we detected
// the first conflict. The rollback necessarily moved those newer
// bytes into the recovery path. Do not let higher layers classify
// this as an ordinary retryable conflict and silently continue from
// the older canonical file.
sync_parent(
path.parent()
.expect("a path accepted by compare/exchange has a parent"),
)?;
return Err(AppError::Config(format!(
"{label} changed again during rollback; newer external bytes are preserved at {} and require explicit recovery",
proposed_or_raced.display()
)));
}
sync_parent(
path.parent()
.expect("a path accepted by compare/exchange has a parent"),
)?;
match displaced_bytes {
Ok(_) => Err(concurrent_change(path, label)),
Err(error) => Err(AppError::Conflict(format!(
"{label} became unsafe during atomic replacement and was restored: {error}"
displaced.display()
))),
}
}
@@ -308,41 +365,270 @@ fn delete_existing_if_equal(
quarantined,
Ok(ref bytes) if bytes.as_deref() == Some(expected)
) {
fs::remove_file(&quarantine).map_err(|error| AppError::io(&quarantine, error))?;
sync_parent(
path.parent()
.expect("a path accepted by compare/exchange has a parent"),
)?;
return Ok(snapshot(None));
let parent = path
.parent()
.expect("a path accepted by compare/exchange has a parent");
return match sync_parent(parent) {
Ok(()) => {
finalize_recovery_artifact(&quarantine, parent, label);
Ok(snapshot(None))
}
Err(publish_error) => {
match restore_quarantined_file(&quarantine, path, parent, label) {
Ok(InstalledRollback::Restored) => Err(publish_error),
Ok(InstalledRollback::Superseded) => Err(AppError::Config(format!(
"{label} delete lost its durability barrier ({publish_error}); \
a concurrent external state won and was preserved"
))),
Err(rollback_error) => {
if path_is_missing(path) && sync_parent(parent).is_ok() {
finalize_recovery_artifact(&quarantine, parent, label);
Ok(snapshot(None))
} else {
Err(ambiguous_publication(
label,
&publish_error,
&rollback_error,
))
}
}
}
}
};
}
match run_before_rollback_restore_hook(path).and_then(|()| rename_noreplace(&quarantine, path))
{
Ok(()) => {
sync_parent(
path.parent()
.expect("a path accepted by compare/exchange has a parent"),
)?;
match quarantined {
Ok(_) => Err(concurrent_change(path, label)),
Err(error) => Err(AppError::Conflict(format!(
"{label} became unsafe during delete and was restored: {error}"
))),
}
}
Err(error) => Err(AppError::Conflict(format!(
let parent = path
.parent()
.expect("a path accepted by compare/exchange has a parent");
match restore_quarantined_file(&quarantine, path, parent, label) {
Ok(InstalledRollback::Restored) => match quarantined {
Ok(_) => Err(concurrent_change(path, label)),
Err(error) => Err(AppError::Conflict(format!(
"{label} became unsafe during delete and was restored: {error}"
))),
},
Ok(InstalledRollback::Superseded) => Err(AppError::Config(format!(
"{label} changed again while a delete was being restored; the newer \
canonical state and displaced bytes at {} were both preserved",
quarantine.display()
))),
Err(error) => Err(AppError::Config(format!(
"{label} changed during delete and the displaced bytes remain at {}: {error}",
quarantine.display()
))),
}
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
enum InstalledRollback {
Restored,
Superseded,
}
/// Remove an installed replacement without ever identifying it by content alone.
///
/// The canonical path is first moved to a private recovery name and its
/// inode/file-id and exact bytes are compared with the staged witness. If an
/// external writer won between the failed durability barrier and this rollback,
/// that writer is restored (or retained at a recovery path) instead of being
/// overwritten.
fn rollback_installed_file(
path: &Path,
installed_identity: &Metadata,
installed_bytes: &[u8],
displaced: Option<&Path>,
parent: &Path,
max_bytes: u64,
label: &str,
) -> Result<InstalledRollback, AppError> {
if let Err(error) = run_before_rollback_restore_hook(path) {
// The hook models a transient namespace rollback failure. Retrying the
// actual no-replace operation is part of the production guarantee.
log::warn!("retrying {label} rollback after transient failure: {error}");
}
let rejected = sibling_temp_path(path, "rejected");
match rename_noreplace(path, &rejected) {
Ok(()) => {}
Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
return restore_displaced_without_overwrite(displaced, path, parent, label);
}
Err(error) => {
return Err(rename_error(
"isolate failed publication",
path,
&rejected,
error,
));
}
}
let rejected_metadata =
fs::symlink_metadata(&rejected).map_err(|error| AppError::io(&rejected, error))?;
let rejected_is_installed = same_file(installed_identity, &rejected_metadata)
&& read_regular_bytes(&rejected, max_bytes, label)
.ok()
.flatten()
.as_deref()
== Some(installed_bytes);
if !rejected_is_installed {
let restored = match rename_noreplace(&rejected, path) {
Ok(()) => InstalledRollback::Superseded,
Err(error) if is_destination_exists(&error) => InstalledRollback::Superseded,
Err(error) => {
return Err(rename_error(
"restore external winner",
&rejected,
path,
error,
));
}
};
sync_parent(parent)?;
return Ok(restored);
}
let restored = restore_displaced_without_overwrite(displaced, path, parent, label)?;
remove_file_if_identity(&rejected, installed_identity)?;
sync_parent(parent)?;
Ok(restored)
}
fn restore_displaced_without_overwrite(
displaced: Option<&Path>,
path: &Path,
parent: &Path,
label: &str,
) -> Result<InstalledRollback, AppError> {
let Some(displaced) = displaced else {
sync_parent(parent)?;
return Ok(InstalledRollback::Restored);
};
match rename_noreplace(displaced, path) {
Ok(()) => {
sync_parent(parent)?;
Ok(InstalledRollback::Restored)
}
Err(error) if is_destination_exists(&error) => {
sync_parent(parent)?;
Ok(InstalledRollback::Superseded)
}
Err(error) => Err(rename_error(
&format!("restore displaced {label}"),
displaced,
path,
error,
)),
}
}
fn restore_quarantined_file(
quarantine: &Path,
path: &Path,
parent: &Path,
label: &str,
) -> Result<InstalledRollback, AppError> {
if let Err(error) = run_before_rollback_restore_hook(path) {
log::warn!("retrying {label} delete rollback after transient failure: {error}");
}
match rename_noreplace(quarantine, path) {
Ok(()) => {
sync_parent(parent)?;
Ok(InstalledRollback::Restored)
}
Err(error) if is_destination_exists(&error) => {
sync_parent(parent)?;
Ok(InstalledRollback::Superseded)
}
Err(error) => Err(rename_error(
"restore quarantined file",
quarantine,
path,
error,
)),
}
}
fn path_is_installed(
path: &Path,
expected_identity: &Metadata,
expected_bytes: &[u8],
max_bytes: u64,
label: &str,
) -> bool {
fs::symlink_metadata(path).ok().is_some_and(|actual| {
actual.file_type().is_file()
&& same_file(expected_identity, &actual)
&& read_regular_bytes(path, max_bytes, label)
.ok()
.flatten()
.as_deref()
== Some(expected_bytes)
})
}
fn path_is_missing(path: &Path) -> bool {
matches!(
fs::symlink_metadata(path),
Err(error) if error.kind() == std::io::ErrorKind::NotFound
)
}
fn remove_file_if_identity(path: &Path, expected: &Metadata) -> Result<bool, AppError> {
let actual = match fs::symlink_metadata(path) {
Ok(actual) => actual,
Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(false),
Err(error) => return Err(AppError::io(path, error)),
};
if !actual.file_type().is_file() || !same_file(expected, &actual) {
return Ok(false);
}
fs::remove_file(path).map_err(|error| AppError::io(path, error))?;
Ok(true)
}
fn finalize_recovery_artifact(path: &Path, parent: &Path, label: &str) {
match fs::remove_file(path) {
Ok(()) => {
if sync_parent(parent).is_err() {
if let Err(error) = sync_parent(parent) {
// The canonical mutation already passed its own barrier.
// This cleanup barrier only governs whether a private
// recovery name can reappear after a crash, so it must not
// turn a committed operation into a false failure.
log::warn!(
"{label} committed, but recovery-artifact cleanup durability is uncertain at {}: {error}",
path.display()
);
}
}
}
Err(error) if error.kind() == std::io::ErrorKind::NotFound => {}
Err(error) => {
log::warn!(
"{label} committed, but its private recovery artifact remains at {}: {error}",
path.display()
);
}
}
}
fn ambiguous_publication(
label: &str,
publish_error: &AppError,
rollback_error: &AppError,
) -> AppError {
AppError::Config(format!(
"{label} lost its durability barrier ({publish_error}) and neither conditional rollback \
nor a second durability barrier established a safe outcome: {rollback_error}"
))
}
fn stage_replacement(
path: &Path,
bytes: &[u8],
preserve_mode: bool,
new_file_mode: Option<u32>,
) -> Result<PathBuf, AppError> {
) -> Result<StagedReplacement, AppError> {
let staged = sibling_temp_path(path, "cas");
let mut options = OpenOptions::new();
options.create_new(true).write(true);
@@ -379,7 +665,14 @@ fn stage_replacement(
file.flush()
.and_then(|_| file.sync_all())
.map_err(|error| AppError::io(&staged, error))?;
Ok(staged)
let identity = file
.metadata()
.map_err(|error| AppError::io(&staged, error))?;
drop(file);
Ok(StagedReplacement {
path: staged,
identity,
})
}
fn sibling_temp_path(path: &Path, purpose: &str) -> PathBuf {
@@ -585,19 +878,6 @@ fn install_over_existing(staged: &Path, path: &Path) -> std::io::Result<PathBuf>
Ok(backup)
}
#[cfg(any(target_os = "linux", target_os = "macos"))]
fn restore_displaced(displaced: &Path, path: &Path) -> std::io::Result<PathBuf> {
exchange_paths(displaced, path)?;
Ok(displaced.to_path_buf())
}
#[cfg(windows)]
fn restore_displaced(displaced: &Path, path: &Path) -> std::io::Result<PathBuf> {
let backup = sibling_temp_path(path, "rejected");
replace_file_with_backup(path, displaced, &backup)?;
Ok(backup)
}
#[cfg(not(any(target_os = "linux", target_os = "macos", windows)))]
fn rename_noreplace(_source: &Path, _destination: &Path) -> std::io::Result<()> {
Err(std::io::Error::new(
@@ -614,14 +894,6 @@ fn install_over_existing(_staged: &Path, _path: &Path) -> std::io::Result<PathBu
))
}
#[cfg(not(any(target_os = "linux", target_os = "macos", windows)))]
fn restore_displaced(_displaced: &Path, _path: &Path) -> std::io::Result<PathBuf> {
Err(std::io::Error::new(
std::io::ErrorKind::Unsupported,
"atomic file exchange is unsupported on this platform",
))
}
fn is_destination_exists(error: &std::io::Error) -> bool {
error.kind() == std::io::ErrorKind::AlreadyExists
|| matches!(error.raw_os_error(), Some(libc::EEXIST))
@@ -963,18 +1235,64 @@ mod tests {
);
assert!(message.contains("explicit recovery"));
assert_eq!(
fs::read(&path).expect("first external version restored"),
b"external-a"
fs::read(&path).expect("newest external version restored"),
b"external-b"
);
assert!(
fs::read_dir(temp.path())
.expect("recovery directory")
.filter_map(Result::ok)
.any(|entry| fs::read(entry.path()).ok().as_deref() == Some(b"external-b")),
"the newest external bytes must remain in a named recovery artifact"
.any(|entry| fs::read(entry.path()).ok().as_deref() == Some(b"external-a")),
"the displaced external bytes must remain in a named recovery artifact"
);
}
#[cfg(unix)]
#[test]
fn create_sync_failure_returns_error_only_after_removing_its_file_identity() {
let temp = tempfile::tempdir().expect("tempdir");
let path = temp.path().join("shared.md");
fail_next_parent_sync_for_test(&path);
let error = replace_shared_file(&path, "missing", b"created", 1024, None, "test")
.expect_err("the injected durability failure remains visible");
assert!(error.to_string().contains("injected"));
assert!(
!path.exists(),
"Err must not leave the attempted create live"
);
}
#[cfg(unix)]
#[test]
fn replace_sync_failure_returns_error_only_after_restoring_before_image() {
let temp = tempfile::tempdir().expect("tempdir");
let path = temp.path().join("shared.md");
fs::write(&path, b"before").expect("seed");
let before = read_shared_file(&path, 1024, "test").expect("snapshot");
fail_next_parent_sync_for_test(&path);
let error = replace_shared_file(&path, &before.revision, b"after", 1024, None, "test")
.expect_err("the injected durability failure remains visible");
assert!(error.to_string().contains("injected"));
assert_eq!(fs::read(&path).expect("before restored"), b"before");
}
#[cfg(unix)]
#[test]
fn delete_sync_failure_returns_error_only_after_restoring_before_image() {
let temp = tempfile::tempdir().expect("tempdir");
let path = temp.path().join("shared.md");
fs::write(&path, b"before").expect("seed");
let before = read_shared_file(&path, 1024, "test").expect("snapshot");
fail_next_parent_sync_for_test(&path);
let error = delete_shared_file(&path, &before.revision, 1024, "test")
.expect_err("the injected durability failure remains visible");
assert!(error.to_string().contains("injected"));
assert_eq!(fs::read(&path).expect("before restored"), b"before");
}
#[cfg(unix)]
#[test]
fn replacement_preserves_exact_existing_permissions_despite_umask() {
+53
View File
@@ -346,6 +346,59 @@ mod tests {
assert!(validate_direct_instruction_content("# Explicit override").is_ok());
}
#[cfg(unix)]
#[test]
fn direct_instruction_entry_never_reports_failure_with_its_attempt_live() {
let temp = tempfile::tempdir().expect("tempdir");
let path = temp
.path()
.join(PiPromptFileKind::SystemOverride.filename());
crate::pi_config::shared_file::fail_next_parent_sync_for_test(&path);
PiPromptFileService::replace_at(
temp.path(),
PiPromptFileKind::SystemOverride,
"missing",
"created",
)
.expect_err("failed create must be compensated");
assert!(!path.exists());
let before = PiPromptFileService::replace_at(
temp.path(),
PiPromptFileKind::SystemOverride,
"missing",
"before",
)
.expect("seed");
crate::pi_config::shared_file::fail_next_parent_sync_for_test(&path);
PiPromptFileService::replace_at(
temp.path(),
PiPromptFileKind::SystemOverride,
&before.revision,
"after",
)
.expect_err("failed replace must restore its before-image");
assert_eq!(
fs::read_to_string(&path).expect("before restored"),
"before"
);
let before = PiPromptFileService::read_at(temp.path(), PiPromptFileKind::SystemOverride)
.expect("snapshot");
crate::pi_config::shared_file::fail_next_parent_sync_for_test(&path);
PiPromptFileService::delete_at(
temp.path(),
PiPromptFileKind::SystemOverride,
&before.revision,
)
.expect_err("failed delete must restore its before-image");
assert_eq!(
fs::read_to_string(&path).expect("before restored"),
"before"
);
}
#[test]
fn templates_reject_ambiguous_or_traversing_slugs() {
for slug in [
+142 -61
View File
@@ -222,7 +222,23 @@ impl PromptService {
if !snapshot.exists {
return Err(AppError::Message("Pi AGENTS.md does not exist".to_string()));
}
return Self::import_pi_snapshot(state, snapshot);
let before = state.db.get_prompts(AppType::Pi.as_str())?;
let timestamp = get_unix_timestamp()?;
let id = format!("imported-{timestamp}");
let prompt = Prompt {
id: id.clone(),
name: format!(
"导入的提示词 {}",
chrono::Local::now().format("%Y-%m-%d %H:%M")
),
content: snapshot.content.clone(),
description: Some("从 Pi AGENTS.md 导入".to_string()),
enabled: true,
created_at: Some(timestamp),
updated_at: Some(timestamp),
};
Self::import_pi_snapshot(state, &guard, &snapshot, &before, prompt)?;
return Ok(id);
}
let file_path = prompt_file_path(&app)?;
@@ -291,26 +307,20 @@ impl PromptService {
return Ok(0);
}
let timestamp = get_unix_timestamp()?;
let mut after = existing.clone();
let id = format!("auto-imported-{timestamp}");
after.insert(
id.clone(),
Prompt {
id,
name: format!(
"Auto-imported Prompt {}",
chrono::Local::now().format("%Y-%m-%d %H:%M")
),
content: snapshot.content,
description: Some("Automatically imported on first launch".to_string()),
enabled: true,
created_at: Some(timestamp),
updated_at: Some(timestamp),
},
);
state
.db
.compare_exchange_prompt_selection(app.as_str(), &existing, &after)?;
let prompt = Prompt {
id: id.clone(),
name: format!(
"Auto-imported Prompt {}",
chrono::Local::now().format("%Y-%m-%d %H:%M")
),
content: snapshot.content.clone(),
description: Some("Automatically imported on first launch".to_string()),
enabled: true,
created_at: Some(timestamp),
updated_at: Some(timestamp),
};
Self::import_pi_snapshot(state, &guard, &snapshot, &existing, prompt)?;
return Ok(1);
}
@@ -402,20 +412,22 @@ impl PromptService {
db.compare_exchange_prompt_selection(AppType::Pi.as_str(), &original, &prompts)?;
#[cfg(test)]
replace_pi_agents_after_reconcile_save_for_test(db, &snapshot.path)?;
apply_pi_native_binding_after_save_hooks_for_test(db, &snapshot.path)?;
let verified =
match PiPromptFileService::read_under_guard(guard, PiPromptFileKind::GlobalContext)
{
Ok(verified) => verified,
Err(error) => {
restore_pi_library_after_failed_reconcile(db, &original, &prompts, &error)?;
restore_pi_library_after_failed_native_binding(
db, &original, &prompts, &error,
)?;
return Err(error);
}
};
if verified.revision == snapshot.revision {
return Ok(());
}
restore_pi_library_after_failed_reconcile(
restore_pi_library_after_failed_native_binding(
db,
&original,
&prompts,
@@ -582,38 +594,56 @@ impl PromptService {
fn import_pi_snapshot(
state: &AppState,
snapshot: PiPromptFileSnapshot,
) -> Result<String, AppError> {
let timestamp = get_unix_timestamp()?;
let id = format!("imported-{timestamp}");
let before = state.db.get_prompts(AppType::Pi.as_str())?;
guard: &PiInstructionFileGuard,
snapshot: &PiPromptFileSnapshot,
before: &IndexMap<String, Prompt>,
prompt: Prompt,
) -> Result<(), AppError> {
let mut prompts = before.clone();
for prompt in prompts.values_mut() {
prompt.enabled = false;
}
prompts.insert(
id.clone(),
Prompt {
id: id.clone(),
name: format!(
"导入的提示词 {}",
chrono::Local::now().format("%Y-%m-%d %H:%M")
),
content: snapshot.content,
description: Some("Pi AGENTS.md 导入".to_string()),
// Import is an explicit reconciliation action. The native file
// is already active by presence, so its exact DB counterpart
// must become the sole enabled library entry without
// rewriting the user-owned file.
enabled: true,
created_at: Some(timestamp),
updated_at: Some(timestamp),
},
);
// Import is an explicit reconciliation action. The native file is
// already active by presence, so its exact DB counterpart becomes the
// sole enabled library entry without rewriting the user-owned file.
prompts.insert(prompt.id.clone(), prompt);
let precommit =
PiPromptFileService::read_under_guard(guard, PiPromptFileKind::GlobalContext)?;
if precommit.revision != snapshot.revision {
return Err(AppError::Conflict(
"Pi AGENTS.md changed before prompt import publication".to_string(),
));
}
state
.db
.compare_exchange_prompt_selection(AppType::Pi.as_str(), &before, &prompts)?;
Ok(id)
.compare_exchange_prompt_selection(AppType::Pi.as_str(), before, &prompts)?;
#[cfg(test)]
apply_pi_native_binding_after_save_hooks_for_test(state.db.as_ref(), &snapshot.path)?;
let verified =
match PiPromptFileService::read_under_guard(guard, PiPromptFileKind::GlobalContext) {
Ok(verified) => verified,
Err(error) => {
restore_pi_library_after_failed_native_binding(
state.db.as_ref(),
before,
&prompts,
&error,
)?;
return Err(error);
}
};
if verified.revision != snapshot.revision {
let error = AppError::Conflict("Pi AGENTS.md changed during prompt import".to_string());
restore_pi_library_after_failed_native_binding(
state.db.as_ref(),
before,
&prompts,
&error,
)?;
return Err(error);
}
Ok(())
}
}
@@ -720,7 +750,7 @@ fn build_pi_reconciled_library(
prompts
}
fn restore_pi_library_after_failed_reconcile(
fn restore_pi_library_after_failed_native_binding(
db: &Database,
original: &IndexMap<String, Prompt>,
attempted: &IndexMap<String, Prompt>,
@@ -729,7 +759,7 @@ fn restore_pi_library_after_failed_reconcile(
db.restore_prompt_selection_if_attempted(AppType::Pi.as_str(), attempted, original)
.map_err(|restore_error| {
AppError::Config(format!(
"Pi prompt reconciliation lost its native revision ({cause}) and failed to \
"Pi prompt-library publication lost its native revision ({cause}) and failed to \
restore the previous portable library without overwriting a newer database \
revision: {restore_error}"
))
@@ -737,23 +767,23 @@ fn restore_pi_library_after_failed_reconcile(
}
#[cfg(test)]
static PI_RECONCILE_AFTER_SAVE_REPLACEMENTS: std::sync::LazyLock<
static PI_NATIVE_BINDING_AFTER_SAVE_REPLACEMENTS: std::sync::LazyLock<
std::sync::Mutex<std::collections::VecDeque<String>>,
> = std::sync::LazyLock::new(|| std::sync::Mutex::new(std::collections::VecDeque::new()));
#[cfg(test)]
fn replace_pi_agents_after_reconcile_save_for_test(
fn apply_pi_native_binding_after_save_hooks_for_test(
db: &Database,
path: &str,
) -> Result<(), AppError> {
if let Some(replacement) = PI_RECONCILE_AFTER_SAVE_DB_REPLACEMENTS
if let Some(replacement) = PI_NATIVE_BINDING_AFTER_SAVE_DB_REPLACEMENTS
.lock()
.map_err(|error| AppError::Lock(error.to_string()))?
.pop_front()
{
db.save_prompt_selection(AppType::Pi.as_str(), &replacement)?;
}
let replacement = PI_RECONCILE_AFTER_SAVE_REPLACEMENTS
let replacement = PI_NATIVE_BINDING_AFTER_SAVE_REPLACEMENTS
.lock()
.map_err(|error| AppError::Lock(error.to_string()))?
.pop_front();
@@ -764,23 +794,23 @@ fn replace_pi_agents_after_reconcile_save_for_test(
}
#[cfg(test)]
static PI_RECONCILE_AFTER_SAVE_DB_REPLACEMENTS: std::sync::LazyLock<
static PI_NATIVE_BINDING_AFTER_SAVE_DB_REPLACEMENTS: std::sync::LazyLock<
std::sync::Mutex<std::collections::VecDeque<IndexMap<String, Prompt>>>,
> = std::sync::LazyLock::new(|| std::sync::Mutex::new(std::collections::VecDeque::new()));
#[cfg(test)]
fn replace_pi_library_after_next_reconcile_save_for_test(replacement: IndexMap<String, Prompt>) {
PI_RECONCILE_AFTER_SAVE_DB_REPLACEMENTS
PI_NATIVE_BINDING_AFTER_SAVE_DB_REPLACEMENTS
.lock()
.expect("Pi reconcile DB hook lock")
.push_back(replacement);
}
#[cfg(test)]
fn replace_pi_agents_after_each_reconcile_save_for_test(
fn replace_pi_agents_after_each_native_binding_save_for_test(
replacements: impl IntoIterator<Item = &'static str>,
) {
*PI_RECONCILE_AFTER_SAVE_REPLACEMENTS
*PI_NATIVE_BINDING_AFTER_SAVE_REPLACEMENTS
.lock()
.expect("Pi reconcile hook lock") =
replacements.into_iter().map(ToOwned::to_owned).collect();
@@ -895,6 +925,55 @@ mod tests {
);
}
#[test]
#[serial]
fn public_pi_import_rolls_back_portable_state_if_native_revision_changes() {
let temp = tempfile::tempdir().expect("tempdir");
let _restore = EnvRestore::set("PI_CODING_AGENT_DIR", temp.path());
std::fs::write(temp.path().join("AGENTS.md"), "native-before").expect("seed AGENTS.md");
let state = AppState::new(Arc::new(Database::memory().expect("database")));
replace_pi_agents_after_each_native_binding_save_for_test(["native-after"]);
let error = PromptService::import_from_file(&state, AppType::Pi)
.expect_err("a stale native snapshot must not become active portable state");
assert!(matches!(error, AppError::Conflict(_)));
assert!(
state
.db
.get_prompts(AppType::Pi.as_str())
.expect("restored portable library")
.is_empty(),
"failed import must conditionally restore its exact DB before-image"
);
assert_eq!(
std::fs::read_to_string(temp.path().join("AGENTS.md")).expect("external native edit"),
"native-after"
);
}
#[test]
#[serial]
fn first_launch_pi_import_rolls_back_if_native_revision_changes() {
let temp = tempfile::tempdir().expect("tempdir");
let _restore = EnvRestore::set("PI_CODING_AGENT_DIR", temp.path());
std::fs::write(temp.path().join("AGENTS.md"), "native-before").expect("seed AGENTS.md");
let state = AppState::new(Arc::new(Database::memory().expect("database")));
replace_pi_agents_after_each_native_binding_save_for_test(["native-after"]);
let error = PromptService::import_from_file_on_first_launch(&state, AppType::Pi)
.expect_err("first-launch import must bind its DB row to one native revision");
assert!(matches!(error, AppError::Conflict(_)));
assert!(state
.db
.get_prompts(AppType::Pi.as_str())
.expect("restored portable library")
.is_empty());
assert_eq!(
std::fs::read_to_string(temp.path().join("AGENTS.md")).expect("external native edit"),
"native-after"
);
}
#[test]
fn unowned_empty_agents_file_is_not_treated_as_absent() {
let snapshot = PiPromptFileSnapshot {
@@ -1248,7 +1327,9 @@ mod tests {
.expect("before prompts"),
)
.expect("serialize before");
replace_pi_agents_after_each_reconcile_save_for_test(["native-1", "native-2", "native-3"]);
replace_pi_agents_after_each_native_binding_save_for_test([
"native-1", "native-2", "native-3",
]);
assert!(matches!(
PromptService::reconcile_pi_library(&state),
@@ -1291,7 +1372,7 @@ mod tests {
prompt("restored-import", "portable-restored", true, 2),
)]);
replace_pi_library_after_next_reconcile_save_for_test(imported.clone());
replace_pi_agents_after_each_reconcile_save_for_test(["native-1"]);
replace_pi_agents_after_each_native_binding_save_for_test(["native-1"]);
let error = PromptService::reconcile_pi_library(&state)
.expect_err("stale compensation must not overwrite a concurrent import");