fix: restore usage statistics after core upgrades
- Persist explicit opt-out in the app's config.toml - Remove usage_statistics_disabled when users re-enable statistics - Enable statistics on core startup unless explicitly disabled - Add regression tests for v7/v8 compatibility and opt-out persistence
This commit is contained in:
1 parent
c24bb1556b
commit
dccc45f57d
6 files changed
+60
-5
No files matched your search
@@ -284,7 +284,7 @@ pub(crate) fn save_core_logging_settings(
|
||||
next.redis_usage_queue_retention_seconds = settings.redis_usage_queue_retention_seconds;
|
||||
|
||||
patch_core_logging_settings(&next)?;
|
||||
let config = match gui_config_state.sync_core_settings(&next) {
|
||||
let config = match gui_config_state.sync_core_logging_settings(&next) {
|
||||
Ok(config) => config,
|
||||
Err(error) => {
|
||||
let rollback_error = patch_core_logging_settings(&previous).err();
|
||||
|
||||
@@ -302,7 +302,11 @@ pub(crate) fn merge_core_config_yaml(
|
||||
}
|
||||
None => merge_core_config_fields(template, None)?,
|
||||
};
|
||||
apply_gui_managed_settings(&base, config)
|
||||
// Old mirrored values may be stale after a kernel upgrade. Only an explicit
|
||||
// software-side opt-out should disable statistics when starting the kernel.
|
||||
let mut startup_config = config.clone();
|
||||
startup_config.usage_statistics_enabled = config.usage_statistics_disabled != Some(true);
|
||||
apply_gui_managed_settings(&base, &startup_config)
|
||||
}
|
||||
|
||||
pub(crate) fn merge_core_config_fields(
|
||||
@@ -1928,6 +1932,11 @@ pub(crate) fn write_gui_config_to_path(
|
||||
.and_then(|content| content.parse::<Document>().ok())
|
||||
.unwrap_or_default();
|
||||
let root = document.as_table_mut();
|
||||
if config.usage_statistics_disabled == Some(true) {
|
||||
set_codex_table_item(root, "usage_statistics_disabled", value(true));
|
||||
} else {
|
||||
root.remove("usage_statistics_disabled");
|
||||
}
|
||||
for (key, item) in [
|
||||
("locale", value(config.locale.as_str())),
|
||||
("port", value(i64::from(config.port))),
|
||||
|
||||
+14
-3
@@ -615,6 +615,8 @@ struct GuiConfigFile {
|
||||
logs_max_total_size_mb: u32,
|
||||
error_logs_max_files: u32,
|
||||
usage_statistics_enabled: bool,
|
||||
#[serde(rename = "usage_statistics_disabled", skip_serializing_if = "Option::is_none")]
|
||||
usage_statistics_disabled: Option<bool>,
|
||||
redis_usage_queue_retention_seconds: u32,
|
||||
request_log: bool,
|
||||
plugins_enabled: bool,
|
||||
@@ -921,6 +923,7 @@ impl Default for GuiConfigFile {
|
||||
logs_max_total_size_mb: DEFAULT_LOGS_MAX_TOTAL_SIZE_MB,
|
||||
error_logs_max_files: DEFAULT_ERROR_LOGS_MAX_FILES,
|
||||
usage_statistics_enabled: true,
|
||||
usage_statistics_disabled: None,
|
||||
redis_usage_queue_retention_seconds: DEFAULT_REDIS_USAGE_QUEUE_RETENTION_SECONDS,
|
||||
request_log: false,
|
||||
plugins_enabled: false,
|
||||
@@ -2165,14 +2168,18 @@ impl GuiConfigState {
|
||||
}
|
||||
|
||||
fn sync_core_settings(&self, settings: &CoreConfigSettings) -> Result<GuiConfigFile, String> {
|
||||
self.sync_core_settings_internal(settings, None, false)
|
||||
self.sync_core_settings_internal(settings, None, false, false)
|
||||
}
|
||||
|
||||
fn sync_core_logging_settings(&self, settings: &CoreConfigSettings) -> Result<GuiConfigFile, String> {
|
||||
self.sync_core_settings_internal(settings, None, false, true)
|
||||
}
|
||||
|
||||
fn sync_core_settings_external(
|
||||
&self,
|
||||
settings: &CoreConfigSettings,
|
||||
) -> Result<GuiConfigFile, String> {
|
||||
self.sync_core_settings_internal(settings, None, true)
|
||||
self.sync_core_settings_internal(settings, None, true, false)
|
||||
}
|
||||
|
||||
fn sync_core_settings_with_api_key(
|
||||
@@ -2180,7 +2187,7 @@ impl GuiConfigState {
|
||||
settings: &CoreConfigSettings,
|
||||
added_api_key: Option<GuiApiKeyEntry>,
|
||||
) -> Result<GuiConfigFile, String> {
|
||||
self.sync_core_settings_internal(settings, added_api_key, false)
|
||||
self.sync_core_settings_internal(settings, added_api_key, false, false)
|
||||
}
|
||||
|
||||
fn sync_core_settings_internal(
|
||||
@@ -2188,6 +2195,7 @@ impl GuiConfigState {
|
||||
settings: &CoreConfigSettings,
|
||||
added_api_key: Option<GuiApiKeyEntry>,
|
||||
apply_external_proxy: bool,
|
||||
save_usage_preference: bool,
|
||||
) -> Result<GuiConfigFile, String> {
|
||||
self.update(|config| {
|
||||
config.api_keys = merge_core_api_keys_with_gui_metadata(
|
||||
@@ -2205,6 +2213,9 @@ impl GuiConfigState {
|
||||
config.logs_max_total_size_mb = settings.logs_max_total_size_mb;
|
||||
config.error_logs_max_files = settings.error_logs_max_files;
|
||||
config.usage_statistics_enabled = settings.usage_statistics_enabled;
|
||||
if save_usage_preference {
|
||||
config.usage_statistics_disabled = (!settings.usage_statistics_enabled).then_some(true);
|
||||
}
|
||||
config.redis_usage_queue_retention_seconds =
|
||||
settings.redis_usage_queue_retention_seconds;
|
||||
config.request_log = settings.request_log;
|
||||
|
||||
@@ -1,6 +1,39 @@
|
||||
use super::support::*;
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn usage_statistics_opt_out_is_removed_when_reenabled() {
|
||||
let home = agent_test_home("usage-opt-out");
|
||||
let path = home.join("config.toml");
|
||||
let mut config = GuiConfigFile::default();
|
||||
config.usage_statistics_disabled = Some(true);
|
||||
write_gui_config_to_path(&config, &path).unwrap();
|
||||
let content = fs::read_to_string(&path).unwrap();
|
||||
assert!(content.contains("usage_statistics_disabled = true"));
|
||||
let restored: GuiConfigFile = toml::from_str(&content).unwrap();
|
||||
assert_eq!(restored.usage_statistics_disabled, Some(true));
|
||||
config.usage_statistics_disabled = None;
|
||||
write_gui_config_to_path(&config, &path).unwrap();
|
||||
assert!(!fs::read_to_string(&path).unwrap().contains("usage_statistics_disabled"));
|
||||
fs::remove_dir_all(home).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn usage_statistics_startup_repairs_stale_values_unless_explicitly_disabled() {
|
||||
for preference in ["", "usage_statistics_disabled = false\n", "usage_statistics_disabled = true\n"] {
|
||||
let config: GuiConfigFile = toml::from_str(&format!("usage-statistics-enabled = false\n{preference}")).unwrap();
|
||||
for template in ["usage-statistics-enabled: false\n", "config-version: 8\nobservability: {usage: {usage-statistics-enabled: false}}\n"] {
|
||||
for current in [None, Some("usage-statistics-enabled: false\n"), Some(template)] {
|
||||
let merged = merge_core_config_yaml(template, current, &config).unwrap();
|
||||
let document = serde_norway::from_str(&merged).unwrap();
|
||||
let settings = core_config_settings_from_value(&document).unwrap();
|
||||
assert_eq!(settings.usage_statistics_enabled, config.usage_statistics_disabled != Some(true));
|
||||
assert!(!merged.contains("usage_statistics_disabled"));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn gui_field_edit_preserves_comments_and_unknown_configuration() {
|
||||
let home = agent_test_home("gui-field-edit");
|
||||
|
||||
@@ -1967,6 +1967,7 @@ fn startup_preserves_all_user_owned_yaml_and_only_applies_gui_managed_values() {
|
||||
error_logs_max_files: 25,
|
||||
usage_statistics_enabled: false,
|
||||
redis_usage_queue_retention_seconds: 180,
|
||||
usage_statistics_disabled: Some(true),
|
||||
request_log: true,
|
||||
plugins_enabled: true,
|
||||
routing_strategy: "fill-first".to_string(),
|
||||
|
||||
@@ -5322,6 +5322,7 @@ mod tests {
|
||||
logs_max_total_size_mb: crate::DEFAULT_LOGS_MAX_TOTAL_SIZE_MB,
|
||||
error_logs_max_files: crate::DEFAULT_ERROR_LOGS_MAX_FILES,
|
||||
usage_statistics_enabled: true,
|
||||
usage_statistics_disabled: None,
|
||||
redis_usage_queue_retention_seconds: crate::DEFAULT_REDIS_USAGE_QUEUE_RETENTION_SECONDS,
|
||||
request_log: false,
|
||||
plugins_enabled: false,
|
||||
|
||||
Reference in new issue
Block a user