1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
//! Unrecognised-key reporting for the HOST-level config files (#5207).
//!
//! Why: the owner ruling's complaint is that a misspelled config key currently
//! no-ops in silence. The obvious fix — `#[serde(deny_unknown_fields)]` on
//! [`crate::core::config::MpmConfig`] and
//! [`crate::core::trusty_tools_config::TrustyToolsConfig`] — is a REGRESSION on
//! those two, because both loaders answer a parse failure by returning
//! `Default`. Denying unknown fields there would upgrade "one key is ignored"
//! into "the entire file is ignored": a single typo would silently drop every
//! model override, every section, the whole config, behind one `warn` line. The
//! failure it introduces is strictly worse than the one it fixes.
//!
//! So strictness is applied where it is safe and total — the NEW committed
//! surface, [`crate::core::project_config::ProjectLevelConfig`], which has no
//! legacy corpus and is reviewed in a PR — and the host files get this instead:
//! the lenient parse they already had, plus a loud, precise report of every key
//! that parse threw away. The typo stops being silent; the other keys keep
//! working.
//!
//! What: [`unknown_key_paths`] diffs the raw document against a round-trip of
//! the parsed struct. Anything present in the input but absent from the
//! round-trip was dropped by serde, i.e. the schema does not define it. This
//! needs no hand-maintained key list, so it cannot drift as sections are added,
//! and it descends into nested tables. [`report_unknown_keys`] logs the result.
//! Test: `unknown_top_level_key_is_reported`, `unknown_nested_key_is_reported`,
//! `known_keys_are_not_reported`, `map_valued_sections_are_not_reported`,
//! `explicit_null_is_not_reported` in `config_keys_tests.rs`.
//!
//! [`unknown_key_paths`]: crate::core::config_keys::unknown_key_paths
//! [`report_unknown_keys`]: crate::core::config_keys::report_unknown_keys
use std::collections::HashSet;
use std::sync::{Mutex, OnceLock, PoisonError};
use serde::Serialize;
use serde_json::Value;
/// Host-config keys a reader OUTSIDE the parsed struct owns (#9097).
///
/// Why: `tm fleet init --session` writes `[supervisor] session` into
/// `~/.trusty-mpm/config.toml` and reads it back through `toml_edit`
/// (`bin/tm/commands/fleet/session_name.rs`), so `MpmConfig` never declares it.
/// A key the product itself wrote is not a typo and must not be reported as one.
/// Test: `a_product_written_supervisor_session_is_not_reported`,
/// `the_recorded_session_key_is_exempt_from_the_unknown_key_report`.
pub const KEYS_READ_ELSEWHERE: &[&str] = &["supervisor.session"];
/// Convert a TOML document to the common `serde_json::Value` tree.
///
/// Why: the two host config files are TOML and YAML; normalising both into one
/// tree type lets a single diff serve them. Returns `None` for input that does
/// not parse — the caller has already reported that as a malformed file.
/// Test: `unknown_top_level_key_is_reported`.
pub fn toml_document(raw: &str) -> Option<Value> {
toml::from_str::<Value>(raw).ok()
}
/// Convert a YAML document to the common `serde_json::Value` tree.
///
/// Why: see [`toml_document`]. A YAML file whose root is not a mapping (empty
/// file, bare scalar) yields `None`, since there are no keys to check.
/// Test: `explicit_null_is_not_reported`.
pub fn yaml_document(raw: &str) -> Option<Value> {
serde_yaml::from_str::<Value>(raw)
.ok()
.filter(Value::is_object)
}
/// Dotted paths of every key in `raw` that `parsed`'s schema does not define.
///
/// Why: this is the whole detection mechanism, and it is deliberately derived
/// rather than declared — round-tripping the PARSED value re-emits exactly the
/// keys serde understood, so the difference is exactly what serde discarded. A
/// hand-written list of valid keys would have to be updated every time a section
/// is added, and would silently stop catching typos the day someone forgot.
/// What: returns dotted paths (`"models.tiers.hiaku"`) in document order.
/// Descends into any table present on both sides. Two deliberate exclusions:
/// a key whose input value is `null` is skipped, because a field with
/// `skip_serializing_if = "Option::is_none"` legitimately round-trips to
/// nothing; and a table that round-trips as a table is compared key-by-key,
/// which means a `HashMap`-valued section (`[models.agents]`, whose keys are
/// arbitrary agent names) reports nothing, since every key survives.
/// Test: `unknown_top_level_key_is_reported`, `unknown_nested_key_is_reported`,
/// `known_keys_are_not_reported`, `map_valued_sections_are_not_reported`.
pub fn unknown_key_paths<T: Serialize>(raw: &Value, parsed: &T) -> Vec<String> {
let Ok(known) = serde_json::to_value(parsed) else {
return Vec::new();
};
let mut out = Vec::new();
diff_into(raw, &known, "", &mut out);
// #8261: `[builders]` also carries the build-lease keys, read by a separate
// struct so `BuildersConfig` keeps its published shape.
out.retain(|path| !crate::core::build_lease::config::LEASE_KEYS.contains(&path.as_str()));
// #9097: `supervisor.session` is written and read by `tm fleet`, not serde.
out.retain(|path| !KEYS_READ_ELSEWHERE.contains(&path.as_str()));
// #9091: `[accounts]` is read strictly by `core::gh_org_accounts`, not by
// `MpmConfig`, so a broken table is an error there instead of a reset here.
out.retain(|path| path != "accounts");
out
}
/// Recursive worker for [`unknown_key_paths`].
///
/// What: walks `raw`'s object keys; a key missing from `known` is recorded, a
/// key present on both recurses. Non-object nodes terminate the walk.
/// Test: via [`unknown_key_paths`]'s tests.
fn diff_into(raw: &Value, known: &Value, prefix: &str, out: &mut Vec<String>) {
let (Value::Object(raw_map), Value::Object(known_map)) = (raw, known) else {
return;
};
for (key, raw_value) in raw_map {
let path = if prefix.is_empty() {
key.clone()
} else {
format!("{prefix}.{key}")
};
match known_map.get(key) {
Some(known_value) => diff_into(raw_value, known_value, &path, out),
// An explicit `null` legitimately round-trips to nothing via
// `skip_serializing_if = "Option::is_none"`, so it is not a typo.
None if raw_value.is_null() => {}
None => out.push(path),
}
}
}
/// Log every unrecognised key in a host config file.
///
/// Why: the whole point of the module — an ignored key must produce a signal an
/// operator can act on, naming the file and the exact path so the fix is
/// mechanical. `warn` rather than `error`: the file still applied, and every key
/// the schema does define took effect.
/// What: one warning listing all unrecognised paths; silent when there are none
/// (the overwhelmingly common case). A given file and key set warns once per
/// process (#9097): `tm doctor` loads the same config about twenty times.
/// Test: `report_is_silent_for_a_clean_document`,
/// `an_unknown_key_warns_once_per_process_across_repeated_loads`.
pub fn report_unknown_keys<T: Serialize>(file: &str, raw: &Value, parsed: &T) {
let unknown = unknown_key_paths(raw, parsed);
if unknown.is_empty() {
return;
}
// #9097: once per (file, key set) per process, not once per load. A new
// typo in the same file is a new key set, so it still warns.
static REPORTED: OnceLock<Mutex<HashSet<String>>> = OnceLock::new();
let first = REPORTED
.get_or_init(Mutex::default)
.lock()
.unwrap_or_else(PoisonError::into_inner)
.insert(format!("{file}\0{}", unknown.join(",")));
if !first {
return;
}
tracing::warn!(
"{file}: ignoring {} unrecognised key(s): {}. \
These had no effect — check for a typo against the documented schema.",
unknown.len(),
unknown.join(", ")
);
}
#[cfg(test)]
#[path = "config_keys_tests.rs"]
mod tests;