diff --git a/server/src/config/mod.rs b/server/src/config/mod.rs index 18c53879..6fbf209b 100644 --- a/server/src/config/mod.rs +++ b/server/src/config/mod.rs @@ -506,7 +506,7 @@ impl ProjectConfig { Err(_) => { // New format failed — try legacy let legacy: LegacyProjectConfig = - toml::from_str(content).map_err(|e| format!("Parse config: {e}"))?; + toml::from_str(content).map_err(|e| format_parse_error(&e))?; if let Some(agent) = legacy.agent { slog!( "[config] Warning: [agent] table is deprecated. \ @@ -659,6 +659,26 @@ impl ProjectConfig { } } +/// Format a `toml::de::Error` for display, appending a hint when the error is +/// an "unknown field" rejection (raised by `ComponentConfig`'s +/// `deny_unknown_fields`). Such errors most commonly happen when a top-level +/// setting is placed after the last `[[component]]` header — TOML attaches +/// trailing bare keys to the table above them, so the setting silently lands +/// inside `ComponentConfig` instead of `ProjectConfig`. The same hint is also +/// shown for a genuine typo'd field inside `[[component]]`, since the error +/// text can't distinguish the two cases. +fn format_parse_error(e: &toml::de::Error) -> String { + let mut msg = format!("Parse config: {e}"); + if e.to_string().contains("unknown field") { + msg.push_str( + "\n\nhint: top-level settings must appear before the first `[[component]]` \ + section in project.toml — keys placed after a `[[component]]` table are \ + attached to that component instead of the project.", + ); + } + msg +} + /// Validate agent configs: no duplicate names, no empty names, positive budgets/turns. fn validate_agents(agents: &[AgentConfig]) -> Result<(), String> { let mut names = HashSet::new(); diff --git a/server/src/config/tests.rs b/server/src/config/tests.rs index 53d62f5f..e1050ea8 100644 --- a/server/src/config/tests.rs +++ b/server/src/config/tests.rs @@ -563,6 +563,86 @@ base_branch = "main" err.contains("base_branch"), "expected error to mention the unrecognized key, got: {err}" ); + assert!( + err.contains("before the first `[[component]]`"), + "expected error to include the misplaced-top-level-key hint, got: {err}" + ); +} + +#[test] +fn unrecognized_key_error_includes_toml_line_and_column() { + // Story 1183: the hinted error must still carry the original toml + // parse error, including its line/column location. + let toml_str = r#" +[[component]] +name = "server" +path = "." +base_branch = "main" +"#; + + let err = ProjectConfig::parse(toml_str).unwrap_err(); + assert!( + err.contains("line") && err.contains("column"), + "expected error to retain toml's line/column info, got: {err}" + ); +} + +#[test] +fn typo_field_in_component_table_gets_same_hint() { + // Story 1183 AC3: a genuine typo'd field (not a misplaced top-level + // setting) inside [[component]] still errors, and reusing the same + // hint text is acceptable since the error can't tell the two apart. + let toml_str = r#" +[[component]] +nam = "server" +path = "." +"#; + + let err = ProjectConfig::parse(toml_str).unwrap_err(); + assert!( + err.contains("before the first `[[component]]`"), + "expected typo'd field error to include the same hint, got: {err}" + ); +} + +#[test] +fn valid_config_with_component_still_parses_unchanged() { + // Story 1183 AC3: valid configs must keep parsing unchanged. + let toml_str = r#" +base_branch = "main" + +[[component]] +name = "server" +path = "." +setup = ["cargo check"] + +[[agent]] +name = "coder" +"#; + + let config = ProjectConfig::parse(toml_str).unwrap(); + assert_eq!(config.base_branch, Some("main".to_string())); + assert_eq!(config.component.len(), 1); + assert_eq!(config.component[0].name, "server"); +} + +#[test] +fn agent_config_does_not_use_strict_parsing_yet() { + // Story 1183 AC4: AgentConfig has no `deny_unknown_fields`, so a + // top-level setting placed after the last [[agent]] section is + // silently absorbed rather than erroring. Per AC4's own conditional + // ("if agents use strict parsing"), no hint is required for this case + // today — this test documents the current (non-strict) behavior. + let toml_str = r#" +[[agent]] +name = "coder" + +max_retries = 5 +"#; + + let config = ProjectConfig::parse(toml_str).unwrap(); + assert_eq!(config.agent.len(), 1); + assert_eq!(config.agent[0].name, "coder"); } // ── base_branch config ──────────────────────────────────────────────────