Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ directly.
- **Before every commit**, run CI-parity checks. Any manual edit after fmt must be re-checked.
- Every released version must map to `docs/releases/vX.Y.Z.md` with process log and detail links.
- Local agent debug context for a release should be recorded in `.docs/releases/vX.Y.Z-debug.md`.
- Public-repo issues, PRs, and public-doc wording should stay LoongClaw-centric; keep detailed external project comparisons in `loongclaw-ai/knowledge-base` unless naming an external project is strictly necessary.

## 5. Verification Gates

Expand Down
1 change: 1 addition & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ directly.
- **Before every commit**, run CI-parity checks. Any manual edit after fmt must be re-checked.
- Every released version must map to `docs/releases/vX.Y.Z.md` with process log and detail links.
- Local agent debug context for a release should be recorded in `.docs/releases/vX.Y.Z-debug.md`.
- Public-repo issues, PRs, and public-doc wording should stay LoongClaw-centric; keep detailed external project comparisons in `loongclaw-ai/knowledge-base` unless naming an external project is strictly necessary.

## 5. Verification Gates

Expand Down
2 changes: 2 additions & 0 deletions crates/app/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ pub use provider::{
ProviderTransportPolicy, ProviderTransportReadiness, ProviderTransportReadinessLevel,
ProviderWireApi, ReasoningEffort, parse_provider_kind_id,
};
#[cfg(test)]
pub(crate) use runtime::inject_test_config_write_failure;
#[allow(unused_imports)]
pub use runtime::{
AcpBackendProfilesConfig, AcpConfig, AcpConversationRoutingMode, AcpDispatchConfig,
Expand Down
47 changes: 47 additions & 0 deletions crates/app/src/config/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ use std::{
path::PathBuf,
};

#[cfg(test)]
use std::cell::Cell;

use serde::{Deserialize, Serialize};

use crate::CliResult;
Expand Down Expand Up @@ -43,6 +46,27 @@ use super::{
};
use crate::secrets::{canonicalize_env_secret_reference, secret_ref_env_name};

#[cfg(test)]
thread_local! {
static TEST_CONFIG_WRITE_FAILURE: Cell<bool> = const { Cell::new(false) };
}

#[cfg(test)]
pub struct ScopedTestConfigWriteFailure;

#[cfg(test)]
impl Drop for ScopedTestConfigWriteFailure {
fn drop(&mut self) {
TEST_CONFIG_WRITE_FAILURE.with(|flag| flag.set(false));
}
}

#[cfg(test)]
pub fn inject_test_config_write_failure() -> ScopedTestConfigWriteFailure {
TEST_CONFIG_WRITE_FAILURE.with(|flag| flag.set(true));
ScopedTestConfigWriteFailure
}

#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)]
pub struct ConfigValidationDiagnostic {
pub severity: String,
Expand Down Expand Up @@ -2099,6 +2123,13 @@ pub fn write(path: Option<&str>, config: &LoongClawConfig, force: bool) -> CliRe
}

let encoded = encode_toml_config(config)?;
#[cfg(any(test, debug_assertions))]
if config_write_failure_injected(&output_path) {
return Err(format!(
"failed to write config file {}: injected test failure",
output_path.display()
));
}
fs::write(&output_path, encoded).map_err(|error| {
format!(
"failed to write config file {}: {error}",
Expand All @@ -2120,6 +2151,22 @@ pub fn default_loongclaw_home() -> PathBuf {
shared_default_loongclaw_home()
}

#[cfg(any(test, debug_assertions))]
fn config_write_failure_injected(output_path: &Path) -> bool {
#[cfg(test)]
if TEST_CONFIG_WRITE_FAILURE.with(Cell::get) {
return true;
}

let configured_path = std::env::var_os("LOONGCLAW_TEST_FAIL_CONFIG_WRITE_PATH");
let Some(configured_path) = configured_path else {
return false;
};

let configured_path = PathBuf::from(configured_path);
configured_path == output_path
}

#[cfg(feature = "config-toml")]
fn parse_toml_config(raw: &str) -> CliResult<LoongClawConfig> {
let (config, selection_report) = parse_toml_config_components(raw)?;
Expand Down
2 changes: 1 addition & 1 deletion crates/app/src/migration/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,7 @@ fn external_skill_artifact_label(artifact: &ExternalSkillArtifact) -> String {
.to_owned()
}

fn merge_profile_note_addendum(existing: Option<&str>, addendum: &str) -> Option<String> {
pub fn merge_profile_note_addendum(existing: Option<&str>, addendum: &str) -> Option<String> {
let trimmed_addendum = addendum.trim();
if trimmed_addendum.is_empty() {
return None;
Expand Down
63 changes: 36 additions & 27 deletions crates/app/src/migration/orchestrator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -777,6 +777,9 @@ fn restore_output_from_backup(
output_preexisted: bool,
) -> CliResult<()> {
if output_preexisted {
if output_path.exists() {
remove_config_output_path(output_path)?;
}
fs::copy(backup_path, output_path).map_err(|error| {
format!(
"failed to restore config {} from backup {}: {error}",
Expand All @@ -785,16 +788,32 @@ fn restore_output_from_backup(
)
})?;
} else if output_path.exists() {
fs::remove_file(output_path).map_err(|error| {
format!(
"failed to remove partial config {} after rollback: {error}",
output_path.display()
)
})?;
remove_config_output_path(output_path)?;
}
Ok(())
}

fn remove_config_output_path(output_path: &Path) -> CliResult<()> {
let metadata = fs::symlink_metadata(output_path).map_err(|error| {
format!(
"failed to inspect partial config {} during rollback: {error}",
output_path.display()
)
})?;
let file_type = metadata.file_type();
let removal_result = if file_type.is_dir() {
fs::remove_dir_all(output_path)
} else {
fs::remove_file(output_path)
};
removal_result.map_err(|error| {
format!(
"failed to remove partial config {} after rollback: {error}",
output_path.display()
)
})
}

fn finalize_apply_import_selection_failure(
error: String,
config: &crate::config::LoongClawConfig,
Expand Down Expand Up @@ -1226,8 +1245,6 @@ fn import_session_id() -> String {
#[cfg(test)]
mod tests {
use super::*;
#[cfg(unix)]
use std::os::unix::fs::PermissionsExt;
use std::{
fs,
path::{Path, PathBuf},
Expand Down Expand Up @@ -1905,7 +1922,6 @@ mod tests {
fs::remove_dir_all(&root).ok();
}

#[cfg(unix)]
#[test]
fn apply_import_selection_rolls_back_bridged_external_skills_when_config_write_fails() {
let root = unique_temp_dir("loongclaw-import-apply-managed-external-skills-rollback");
Expand Down Expand Up @@ -1933,14 +1949,9 @@ mod tests {
let mut baseline = crate::config::LoongClawConfig::default();
baseline.external_skills.install_root =
Some(root.join("managed-skills").display().to_string());
let rendered = crate::config::render(&baseline).expect("render baseline config");
fs::write(&output_path, rendered).expect("write baseline config");
let mut readonly_permissions = fs::metadata(&output_path)
.expect("read output metadata")
.permissions();
readonly_permissions.set_mode(0o400);
fs::set_permissions(&output_path, readonly_permissions)
.expect("make output file read only");
let baseline_body = crate::config::render(&baseline).expect("render baseline config");
fs::write(&output_path, &baseline_body).expect("write baseline config");
let _write_failure = crate::config::inject_test_config_write_failure();

let discovery = discover_import_sources(&root, DiscoveryOptions::default())
.expect("discovery should succeed");
Expand All @@ -1953,23 +1964,21 @@ mod tests {
apply_external_skills_plan: true,
external_skills_input_path: Some(root.clone()),
})
.expect_err("read-only config path should fail to persist");
.expect_err("injected config write failure should abort apply");

assert!(
error.contains("failed to write config file"),
"expected config write failure, got: {error}"
);
assert!(
!root.join("managed-skills").join("release-guard").exists(),
"rollback should remove bridged managed installs after config persistence failure"
output_path.is_file(),
"rollback should restore the original config file after config persistence failure"
);
assert_eq!(
fs::read_to_string(&output_path).expect("read restored baseline config"),
baseline_body,
"rollback should restore the original config body after config persistence failure"
);

let mut cleanup_permissions = fs::metadata(&output_path)
.expect("read output metadata for cleanup")
.permissions();
cleanup_permissions.set_mode(0o600);
fs::set_permissions(&output_path, cleanup_permissions)
.expect("restore output permissions for cleanup");
fs::remove_dir_all(&root).ok();
}

Expand Down
16 changes: 11 additions & 5 deletions crates/app/src/tools/config_import.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::{
ffi::OsString,
fs,
path::{Path, PathBuf},
};

Expand Down Expand Up @@ -543,8 +542,10 @@ fn resolve_safe_path_with_config(

fn canonicalize_or_fallback(path: PathBuf) -> Result<PathBuf, String> {
if path.exists() {
return fs::canonicalize(&path)
let canonical = dunce::canonicalize(&path)
.map_err(|error| format!("failed to canonicalize {}: {error}", path.display()));
let canonical = canonical.map(|resolved| dunce::simplified(&resolved).to_path_buf())?;
return Ok(canonical);
}
Ok(super::normalize_without_fs(&path))
}
Expand All @@ -553,23 +554,25 @@ fn resolve_path_within_root(root: &Path, normalized: &Path) -> Result<PathBuf, S
ensure_path_within_root(root, normalized)?;

if normalized.exists() {
let canonical = fs::canonicalize(normalized).map_err(|error| {
let canonical = dunce::canonicalize(normalized).map_err(|error| {
format!(
"failed to canonicalize target path {}: {error}",
normalized.display()
)
})?;
let canonical = dunce::simplified(&canonical).to_path_buf();
ensure_path_within_root(root, &canonical)?;
return Ok(canonical);
}

let (ancestor, suffix) = split_existing_ancestor(normalized)?;
let canonical_ancestor = fs::canonicalize(&ancestor).map_err(|error| {
let canonical_ancestor = dunce::canonicalize(&ancestor).map_err(|error| {
format!(
"failed to canonicalize ancestor {}: {error}",
ancestor.display()
)
})?;
let canonical_ancestor = dunce::simplified(&canonical_ancestor).to_path_buf();
ensure_path_within_root(root, &canonical_ancestor)?;

let mut reconstructed = canonical_ancestor;
Expand All @@ -581,7 +584,9 @@ fn resolve_path_within_root(root: &Path, normalized: &Path) -> Result<PathBuf, S
}

fn ensure_path_within_root(root: &Path, path: &Path) -> Result<(), String> {
if path.starts_with(root) {
let normalized_root = dunce::simplified(root);
let normalized_path = dunce::simplified(path);
if normalized_path.starts_with(normalized_root) {
return Ok(());
}
Err(format!(
Expand Down Expand Up @@ -620,6 +625,7 @@ fn split_existing_ancestor(path: &Path) -> Result<(PathBuf, Vec<OsString>), Stri

#[cfg(test)]
mod tests {
use std::fs;
use std::time::{SystemTime, UNIX_EPOCH};

use super::*;
Expand Down
Loading
Loading