charles/peon
1
0
Fork 0
Code Issues 12 Pull requests Projects Releases Packages Wiki Activity Actions

feat(config): implement TOML loading & defaults #22

Merged
claude-desktop merged 4 commits from peon/3 into main 2026-04-17 07:03:13 +00:00
Conversation 1 Commits 4 Files changed 3 +362 -2
claude-desktop commented 2026-04-16 20:44:46 +00:00
Collaborator
Copy link

Summary

  • Implements AppConfig with GeneralConfig, ProvidersConfig, TerminalConfig, GitConfig, and a keybindings HashMap
  • TerminalMode enum (Embedded / Kitty / Foot / Auto) with serde rename_all = lowercase
  • load_config() reads ~/.config/forge-agent/config.toml, creating it from the embedded default_config.toml if absent
  • Empty provider path strings are normalised to None (auto-detection signal for callers)
  • GeneralConfig::resolved_data_dir() expands a leading ~ to the actual home directory
  • 8 unit tests: defaults, missing fields, invalid TOML, tilde expansion, path preservation

Closes #3

## Summary - Implements `AppConfig` with `GeneralConfig`, `ProvidersConfig`, `TerminalConfig`, `GitConfig`, and a `keybindings` `HashMap` - `TerminalMode` enum (`Embedded` / `Kitty` / `Foot` / `Auto`) with `serde rename_all = lowercase` - `load_config()` reads `~/.config/forge-agent/config.toml`, creating it from the embedded `default_config.toml` if absent - Empty provider path strings are normalised to `None` (auto-detection signal for callers) - `GeneralConfig::resolved_data_dir()` expands a leading `~` to the actual home directory - 8 unit tests: defaults, missing fields, invalid TOML, tilde expansion, path preservation Closes #3
feat(config): implement TOML loading & defaults
All checks were successful
qa / qa (pull_request) Successful in 10m24s
Details
9d989e2825
 
- AppConfig with GeneralConfig, ProvidersConfig, TerminalConfig, GitConfig,
  and keybindings HashMap
- TerminalMode enum (Embedded/Kitty/Foot/Auto) with serde rename_all=lowercase
- load_config() reads ~/.config/forge-agent/config.toml, creates it from
  the embedded default_config.toml if absent
- Empty provider path strings normalised to None (auto-detection signal)
- GeneralConfig::resolved_data_dir() expands leading ~ to home dir
- 8 unit tests covering defaults, missing fields, invalid TOML, tilde expansion

Closes #3
Ghost requested changes 2026-04-16 20:46:33 +00:00
Ghost left a comment
Copy link

All acceptance criteria from #3 are implemented and the structure is clean. One real bug to fix before merging.

All acceptance criteria from #3 are implemented and the structure is clean. One real bug to fix before merging.
crates/backend/src/config.rs
@ -2,0 +144,4 @@
#[derive(Debug, Error)]
pub enum ConfigError {
#[error("cannot determine config directory")]
NoConfigDir,
Ghost commented 2026-04-16 20:46:33 +00:00
Copy link

Bug: ConfigError::NoHomeDir is dead code — never returned.

expand_tilde silently returns the unexpanded path when dirs::home_dir() is None, so this variant is never constructed anywhere. That's misleading to callers inspecting the public ConfigError type, and it will cause the resolved_data_dir_is_absolute test below to silently fail in environments without $HOME (the fallback produces a relative PathBuf::from("~/.local/share/forge-agent")), making the assert!(resolved.is_absolute()) assertion blow up.

Pick one of the two clean fixes:

Option A — keep silent-fallback, remove the dead variant:

// Remove NoHomeDir from ConfigError.
// Document expand_tilde's silent-fallback contract.
/// Returns the path unchanged if home dir cannot be determined.
fn expand_tilde(path: &str) -> PathBuf { … }

And weaken the test:

#[test]
fn resolved_data_dir_expands_tilde() {
    let cfg = parse_config(DEFAULT_CONFIG).unwrap();
    let resolved = cfg.general.resolved_data_dir();
    // Only assert absolute when $HOME is actually set.
    if dirs::home_dir().is_some() {
        assert!(resolved.is_absolute());
    }
}

Option B — wire up the variant properly:

fn expand_tilde(path: &str) -> Result<PathBuf, ConfigError> {
    if let Some(rest) = path.strip_prefix("~/") {
        let home = dirs::home_dir().ok_or(ConfigError::NoHomeDir)?;
        return Ok(home.join(rest));
    }
    if path == "~" {
        return dirs::home_dir().ok_or(ConfigError::NoHomeDir);
    }
    Ok(PathBuf::from(path))
}

Then resolved_data_dir returns Result<PathBuf, ConfigError> and load_config propagates it.

**Bug: `ConfigError::NoHomeDir` is dead code — never returned.** `expand_tilde` silently returns the unexpanded path when `dirs::home_dir()` is `None`, so this variant is never constructed anywhere. That's misleading to callers inspecting the public `ConfigError` type, and it will cause the `resolved_data_dir_is_absolute` test below to silently fail in environments without `$HOME` (the fallback produces a relative `PathBuf::from("~/.local/share/forge-agent")`), making the `assert!(resolved.is_absolute())` assertion blow up. Pick one of the two clean fixes: **Option A — keep silent-fallback, remove the dead variant:** ```rust // Remove NoHomeDir from ConfigError. // Document expand_tilde's silent-fallback contract. /// Returns the path unchanged if home dir cannot be determined. fn expand_tilde(path: &str) -> PathBuf { … } ``` And weaken the test: ```rust #[test] fn resolved_data_dir_expands_tilde() { let cfg = parse_config(DEFAULT_CONFIG).unwrap(); let resolved = cfg.general.resolved_data_dir(); // Only assert absolute when $HOME is actually set. if dirs::home_dir().is_some() { assert!(resolved.is_absolute()); } } ``` **Option B — wire up the variant properly:** ```rust fn expand_tilde(path: &str) -> Result<PathBuf, ConfigError> { if let Some(rest) = path.strip_prefix("~/") { let home = dirs::home_dir().ok_or(ConfigError::NoHomeDir)?; return Ok(home.join(rest)); } if path == "~" { return dirs::home_dir().ok_or(ConfigError::NoHomeDir); } Ok(PathBuf::from(path)) } ``` Then `resolved_data_dir` returns `Result<PathBuf, ConfigError>` and `load_config` propagates it.
fix(config): wire up NoHomeDir error in expand_tilde
Some checks failed
qa / qa (pull_request) Has been cancelled
Details
0f33a68fd1
 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fix(config): correct misleading doc comment on parse_config
All checks were successful
qa / qa (pull_request) Successful in 12m7s
Details
17275048ba
 
The doc comments on load_config and parse_config claimed tilde expansion
happens during parsing, but it actually happens lazily when
resolved_data_dir() is called. Also change DEFAULT_CONFIG from static to
const, which is more conventional for compile-time string literals.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fix(ci): add libsqlite3-dev to system dependencies
All checks were successful
qa / qa (pull_request) Successful in 1m53s
Details
d0888391ae
 
Required by libsqlite3-sys for sqlx compilation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Sign in to join this conversation.
Reviewers
No reviewers
reviewer
Ghost
Labels
Clear labels   
area:config
Configuration loading, TOML, settings

  
area:contracts
Shared types crate (contracts)

  
area:engine
OrchestrationEngine, reactors, dispatch loop

  
area:eventsourcing
Event sourcing core — decider, projector, commands, events

  
area:frontend
GTK4/RELM4 frontend components

  
area:git
Git integration — GitCore, checkpoints, branches

  
area:ipc
IPC channels, AppCommand/AppEvent, GTK↔tokio bridge

  
area:persistence
SQLite persistence, EventStore, migrations

  
area:provider
Provider adapters (Claude, Codex), JSON-RPC stdio

  
area:scaffold
Project scaffolding, workspace setup, CI

  
area:terminal
Terminal manager, PTY, VTE4, Kitty/Foot

  
type:user-story
User story for v0.1.0 implementation

No labels
area:config
area:contracts
area:engine
area:eventsourcing
area:frontend
area:git
area:ipc
area:persistence
area:provider
area:scaffold
area:terminal
type:user-story
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
dev
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
charles/peon!22
No description provided.