From 702c7c89f7a91c494adb0e33f3096d5bc27b8a58 Mon Sep 17 00:00:00 2001 From: altair823 Date: Sun, 3 May 2026 04:23:16 +0000 Subject: [PATCH] =?UTF-8?q?review(p9-fb-05):=20=ED=9A=8C=EC=B0=A8=201=20ni?= =?UTF-8?q?t=20=EB=B0=98=EC=98=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - `Config.source_dir` 를 `pub(crate)` 로 좁힘. invariant ("from_file / load 만이 정당한 setter") 가 외부 mutation 으로 깨지지 않도록. 대신 `pub fn source_dir(&self) -> Option<&Path>` (read-only) + `pub fn with_source_dir(self, dir) -> Self` (builder) 노출 — 테스트 / 프로그래마틱 사용은 builder 통과. - `resolve_workspace_root` 의 `current_dir()` 실패 fallback 에 `tracing::warn!` 추가. chroot / deleted-cwd / permission 문제로 cwd 가 안 잡힐 때 silently `./root` 로 떨어지지 않고 로그가 남음. `tracing` 을 kebab-config 의 deps 에 추가 (workspace dep). 테스트 27 통과 + 워크스페이스 clippy clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- Cargo.lock | 1 + crates/kebab-config/Cargo.toml | 3 +++ crates/kebab-config/src/lib.rs | 41 +++++++++++++++++++++++++++++----- 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index afd79f2..9045280 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3571,6 +3571,7 @@ dependencies = [ "serde", "serde_json", "toml", + "tracing", ] [[package]] diff --git a/crates/kebab-config/Cargo.toml b/crates/kebab-config/Cargo.toml index 4b40b01..9cf48ee 100644 --- a/crates/kebab-config/Cargo.toml +++ b/crates/kebab-config/Cargo.toml @@ -15,3 +15,6 @@ serde = { workspace = true } serde_json = { workspace = true } toml = "0.8" dirs = "5" +# p9-fb-05: warn-log when current_dir() fails (chroot, deleted cwd) +# during workspace.root resolution. +tracing = { workspace = true } diff --git a/crates/kebab-config/src/lib.rs b/crates/kebab-config/src/lib.rs index 264aa13..e0e11d4 100644 --- a/crates/kebab-config/src/lib.rs +++ b/crates/kebab-config/src/lib.rs @@ -39,8 +39,13 @@ pub struct Config { /// against the config file's location instead of the user's /// `cwd` (so `--config /tmp/cfg.toml` + `root = "kb"` reads /// `/tmp/kb` no matter where the user invoked from). + /// + /// `pub(crate)` so external callers can't break the + /// "stamped only by from_file/load" invariant by hand. Use + /// [`Config::with_source_dir`] for tests / programmatic + /// construction that need a specific `source_dir`. #[serde(skip)] - pub source_dir: Option, + pub(crate) source_dir: Option, } #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] @@ -308,6 +313,21 @@ impl Config { } } + /// p9-fb-05: read-only accessor for the source-file directory + /// (where `from_file` / `load` stamped it). Returns `None` for + /// `Config::defaults()` and other in-memory constructions. + pub fn source_dir(&self) -> Option<&Path> { + self.source_dir.as_deref() + } + + /// p9-fb-05: builder for tests / programmatic callers that need + /// to pin `source_dir` without going through `from_file`. Returns + /// `self` so it chains: `Config::defaults().with_source_dir(p)`. + pub fn with_source_dir(mut self, dir: PathBuf) -> Self { + self.source_dir = Some(dir); + self + } + /// p9-fb-05: resolve `workspace.root` to an absolute `PathBuf`. /// Order: /// 1. tilde / env / `${VAR}` substitutions per [`expand_path`]. @@ -321,10 +341,21 @@ impl Config { /// arise when the user is using defaults AND has a relative /// root, which is rare (defaults ship `~/KnowledgeBase`). pub fn resolve_workspace_root(&self) -> PathBuf { - let base = self - .source_dir - .clone() - .unwrap_or_else(|| std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."))); + let base = self.source_dir.clone().unwrap_or_else(|| { + std::env::current_dir().unwrap_or_else(|e| { + // chroot / deleted-cwd / permission failure: log so a + // user with an environment problem doesn't silently + // wonder why their workspace.root resolved to "./root" + // (which then fails at `create_dir_all` time with a + // less obvious error). + tracing::warn!( + target: "kebab-config", + error = %e, + "current_dir() failed; falling back to '.' for workspace.root resolution" + ); + PathBuf::from(".") + }) + }); paths::expand_path_with_base(&self.workspace.root, "", &base) }