Step 3: Config — file I/O and serialization#3
Conversation
Rust concepts: std::fs, serde deserialization, Default trait, Path/PathBuf Source files: src/core/config.ts (182 lines) This PR is documentation only — no Rust code to review. Read the lesson, implement the task, make the tests pass.
Reviewer's GuideAdds a new lesson on config loading and implements Rust-side configuration loading with defaults, JSON parsing, and file discovery, wiring it into the main module and types. Sequence diagram for the new load_config processsequenceDiagram
actor User
participant Main as Main_binary
participant Config as Config_module
participant Proc as Process_module
participant FS as File_system
participant Serde as Serde_JSON
User->>Main: start wt-cli
Main->>Config: load_config(cwd)
Config->>Proc: run_process("git", ["rev-parse","--show-toplevel"], cwd)
Proc-->>Config: Ok(git_root_path) or Err
alt git root found
Config->>FS: read_to_string(git_root_path + "/wt.config.json")
alt wt.config.json exists
FS-->>Config: Ok(json_string)
Config->>Serde: from_str(json_string) as WtConfig
Serde-->>Config: Ok(parsed_config) or Err
else wt.config.json missing
FS-->>Config: Err
Config->>FS: read_to_string(git_root_path + "/package.json")
alt package.json exists
FS-->>Config: Ok(pkg_json_string)
Config->>Serde: from_str(pkg_json_string) as Value
Serde-->>Config: Ok(pkg_value) or Err
else package.json missing
FS-->>Config: Err
end
end
else git root not found
Proc-->>Config: Err
end
alt any JSON parse error
Serde-->>Config: Err
Config-->>Main: Err(String)
else no config files or only partial config
Config->>Config: defaults = get_default_config()
Config->>Config: merged = overlay(parsed_fields, defaults)
Config-->>Main: Ok(LoadedConfig{config: merged, source, root_path})
end
Main-->>User: continue with loaded configuration
Class diagram for Rust configuration types and loader moduleclassDiagram
class WtConfig {
+String worktree_dir
+String main_branch
+String default_base
+String remote
+bool auto_setup
+u32 stale_days
+SetupConfig setup
+LifecycleScripts lifecycle_scripts
}
class SetupConfig {
+Vec~String~ steps
}
class LifecycleScripts {
+Option~String~ postsetup
+Option~String~ preclean
}
class LoadedConfig {
+WtConfig config
+String source
+String root_path
}
class ConfigModule {
+get_default_config() WtConfig
+load_config(cwd: &str) Result~LoadedConfig, String~
+read_json_file(path: &str) Result~serde_json::Value, String~
}
WtConfig --> SetupConfig
WtConfig --> LifecycleScripts
LoadedConfig --> WtConfig
ConfigModule ..> WtConfig
ConfigModule ..> LoadedConfig
ConfigModule ..> SetupConfig
ConfigModule ..> LifecycleScripts
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 57 minutes and 37 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds Step 3 lesson documentation for implementing config discovery, JSON file I/O, and serde-based deserialization with defaults in the Rust rewrite of wt-cli.
Changes:
- Add a new lesson document describing config loading behavior (git root discovery, config file search, JSON parsing, defaults).
- Provide an implementation outline and test skeleton for
src/config.rs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| **Update `src/types.rs`** — add `#[serde(default)]` annotations to WtConfig fields that have defaults: | ||
| - `worktree_dir` defaults to `"tree"` | ||
| - `main_branch` defaults to `"main"` | ||
| - `default_base` defaults to `"main"` | ||
| - `remote` defaults to `"origin"` | ||
| - `auto_setup` defaults to `true` | ||
| - `stale_days` defaults to `30` | ||
| - `setup` defaults to `SetupConfig { steps: vec![] }` | ||
| - `lifecycle_scripts` defaults to `LifecycleScripts { postsetup: None, preclean: None }` |
There was a problem hiding this comment.
The instruction to "add #[serde(default)] annotations" is insufficient for these fields: #[serde(default)] uses Default::default() for the field type (e.g., String -> "", bool -> false, u32 -> 0), so it won’t produce defaults like "tree", true, or 30. Consider updating the lesson to recommend #[serde(default = "...")] per field, or implementing Default for WtConfig and adding #[serde(default)] at the struct level so missing fields come from WtConfig::default().
| #[test] | ||
| fn parse_partial_json_overrides_defaults() { | ||
| // Create JSON: {"worktreeDir": "custom-tree", "staleDays": 14} | ||
| // Parse into WtConfig | ||
| // Assert worktree_dir is "custom-tree" but main_branch is still "main" (default) | ||
| } |
There was a problem hiding this comment.
The lesson (and test skeleton) uses camelCase JSON keys like worktreeDir / staleDays, but the Rust example uses snake_case fields (worktree_dir, stale_days). Unless you mention adding #[serde(rename_all = "camelCase")] (or per-field #[serde(rename = "...")]) on WtConfig and nested structs, serde_json deserialization will fail to find those fields.
| Checking if a file exists: | ||
| ```rust | ||
| let path = Path::new("wt.config.json"); | ||
| if path.exists() { | ||
| let content = fs::read_to_string(path)?; | ||
| } | ||
| ``` | ||
|
|
||
| **vs TypeScript:** `fs.readFileSync(path, "utf8")` — similar, but Rust makes the error handling explicit instead of throwing. | ||
|
|
||
| **Common gotcha:** `Path::new("file")` creates a path reference but doesn't check if the file exists. `.exists()` does a syscall. Don't check-then-read (race condition) — just read and handle the error. But for this module, we check because we want to try multiple candidate files. | ||
|
|
There was a problem hiding this comment.
This section warns against check-then-read due to TOCTOU, but the example and later guidance still encourages path.exists() followed by read_to_string(). For the “multiple candidate files” case, it’s safer and simpler to attempt the read directly and, on io::ErrorKind::NotFound, fall back to the next candidate (avoids the race and an extra syscall).
| ## Your Task | ||
|
|
||
| **File:** `src/config.rs` (create this file) | ||
|
|
||
| **Update `src/types.rs`** — add `#[serde(default)]` annotations to WtConfig fields that have defaults: | ||
| - `worktree_dir` defaults to `"tree"` | ||
| - `main_branch` defaults to `"main"` | ||
| - `default_base` defaults to `"main"` | ||
| - `remote` defaults to `"origin"` | ||
| - `auto_setup` defaults to `true` | ||
| - `stale_days` defaults to `30` | ||
| - `setup` defaults to `SetupConfig { steps: vec![] }` | ||
| - `lifecycle_scripts` defaults to `LifecycleScripts { postsetup: None, preclean: None }` | ||
|
|
||
| Implement `src/config.rs` with these functions: | ||
|
|
||
| **`pub fn get_default_config() -> WtConfig`** | ||
| - Returns a WtConfig with all default values (matching the TypeScript `getDefaultConfig()`) | ||
|
|
||
| **`pub fn load_config(cwd: &str) -> Result<LoadedConfig, String>`** | ||
| - First, find the git root by running `git rev-parse --show-toplevel` (use your `process::run_process`) | ||
| - Then try reading `wt.config.json` from that root — if it exists, parse it as WtConfig |
There was a problem hiding this comment.
The task refers to src/types.rs, src/process.rs, and process::run_process, but this repository currently only has src/main.rs. If this step assumes earlier lessons/PRs created those modules, it would help to explicitly say so (or link to the relevant steps); otherwise the instructions should include creating those files so the step is runnable.
Added the full original config.ts and a line-by-line translation walkthrough explaining zod→serde, async readFile→sync std::fs, candidate loop→match-on-Result, and PathBuf for path operations.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6f9d0eb07
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - **Zod → serde.** Both do the same job: parse + validate + apply defaults. Zod is a runtime library. Serde is a compile-time derive macro — it generates the parsing code at build time. | ||
| - `z.string().default("tree")` → `#[serde(default = "default_worktree_dir")]`. You write a function that returns the default value. If the JSON field is missing, serde calls it. | ||
| - `z.string().optional()` → `Option<String>` with `#[serde(default)]`. `Option<T>` already defaults to `None`, so `#[serde(default)]` is enough — no custom function needed. | ||
| - `z.number().int().positive()` → `u32`. Rust's type system enforces "positive integer" at compile time. No runtime validation needed for that constraint. |
There was a problem hiding this comment.
Preserve strict positivity when translating
staleDays
The walkthrough says z.number().int().positive() can be represented by u32 without extra validation, but u32 permits 0 whereas Zod positive() requires > 0. If learners implement this as written, {"staleDays": 0} becomes valid in Rust even though it is invalid in the TypeScript source, changing config semantics and potentially altering stale-cleanup behavior; the lesson should require a non-zero type or explicit runtime check.
Useful? React with 👍 / 👎.
| .map_err(|e| format!("Invalid config in {}: {}", source, e))?; | ||
| return Ok(LoadedConfig { config, source: source.to_string(), root_path }); | ||
| } | ||
| Err(e) if e.contains("No such file") => continue, // file doesn't exist, try next |
There was a problem hiding this comment.
Replace string-matching for missing-file detection
The sample load_config loop classifies missing files using e.contains("No such file"), which depends on localized OS error text. On platforms/locales where the message differs (for example Windows), absent config files can be treated as hard failures instead of falling through to the next candidate/defaults. The lesson should demonstrate checking ErrorKind::NotFound before stringifying errors.
Useful? React with 👍 / 👎.
Learning step — doc only. Read
lessons/03-config-file-io-serde.md, implement the task insrc/config.rs, make the tests pass.Rust concepts: std::fs, serde deserialization, Default trait, Path/PathBuf
Source: src/core/config.ts (182 lines)
What you'll build: Config loading with JSON parsing, defaults, and file discovery
When tests pass, merge this PR.
Summary by Sourcery
Documentation: