huskies: merge 1183 story Config parse error hints at misplaced top-level keys after [[component]]
This commit is contained in:
@@ -506,7 +506,7 @@ impl ProjectConfig {
|
|||||||
Err(_) => {
|
Err(_) => {
|
||||||
// New format failed — try legacy
|
// New format failed — try legacy
|
||||||
let legacy: LegacyProjectConfig =
|
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 {
|
if let Some(agent) = legacy.agent {
|
||||||
slog!(
|
slog!(
|
||||||
"[config] Warning: [agent] table is deprecated. \
|
"[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.
|
/// Validate agent configs: no duplicate names, no empty names, positive budgets/turns.
|
||||||
fn validate_agents(agents: &[AgentConfig]) -> Result<(), String> {
|
fn validate_agents(agents: &[AgentConfig]) -> Result<(), String> {
|
||||||
let mut names = HashSet::new();
|
let mut names = HashSet::new();
|
||||||
|
|||||||
@@ -563,6 +563,86 @@ base_branch = "main"
|
|||||||
err.contains("base_branch"),
|
err.contains("base_branch"),
|
||||||
"expected error to mention the unrecognized key, got: {err}"
|
"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 ──────────────────────────────────────────────────
|
// ── base_branch config ──────────────────────────────────────────────────
|
||||||
|
|||||||
Reference in New Issue
Block a user