From 5c6904b0e0b6a36d12ecf35c2be2a2175fd29cf1 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 17 Sep 2026 01:34:01 +0000 Subject: [PATCH] fix(boundary): validate at edges and stop re-parsing admitted values Keep hook stdin policy, confirmation, and duration parsing at the shell. Stop treating unregistered retention JSON and transport serializers as application contracts. Co-authored-by: Zack Jackson --- .../src/hooks/claude.rs | 12 +-- .../tracedecay-agent-hosts/src/hooks/mod.rs | 66 +++++++++++++--- .../src/hooks/wire_stdin_bound_tests.rs | 21 ++++- crates/tracedecay-api/src/configuration.rs | 74 +++++++++++++---- .../src/analytics_bridge/diagnostics.rs | 4 +- .../src/observability.rs | 79 ++++--------------- .../src/observability/costs.rs | 21 +---- crates/tracedecay-cli/src/agent_cmd.rs | 45 ++++++++--- crates/tracedecay-cli/src/cli.rs | 5 +- crates/tracedecay-cli/src/cli/help.rs | 2 +- .../src/config/model.rs | 39 +++------ .../src/config/model/tests.rs | 51 +++++++++++- .../src/configuration/user_settings.rs | 16 ++-- crates/tracedecay-configuration/src/lib.rs | 8 +- .../src/settings_api.rs | 12 +-- .../src/storage_telemetry_api.rs | 28 ++++--- .../tracedecay-mcp/src/handlers/analytics.rs | 6 +- 17 files changed, 298 insertions(+), 191 deletions(-) diff --git a/crates/tracedecay-agent-hosts/src/hooks/claude.rs b/crates/tracedecay-agent-hosts/src/hooks/claude.rs index c995c5ce4b..cb47837d63 100644 --- a/crates/tracedecay-agent-hosts/src/hooks/claude.rs +++ b/crates/tracedecay-agent-hosts/src/hooks/claude.rs @@ -239,17 +239,7 @@ async fn claude_post_tool_use_response( #[hotpath::measure(future = true, label = "hosts.hooks.claude.stop")] pub async fn hook_stop(runtime: &HookRuntimeV1) -> i32 { let started = Instant::now(); - let event = match super::read_stdin_bounded() { - Ok(super::HookStdinRead::Event(event)) => event, - Ok(super::HookStdinRead::Oversized) => { - eprintln!( - "tracedecay hook: stdin exceeds wire message bound ({})", - tracedecay_framing::WIRE_RECORD_TOO_LARGE - ); - return 1; - } - Err(_) => String::new(), - }; + let event = read_hook_event!(); let (root, output) = claude_stop_response_for_event(runtime, &event, started).await; if !super::write_hook_output( root.as_deref(), diff --git a/crates/tracedecay-agent-hosts/src/hooks/mod.rs b/crates/tracedecay-agent-hosts/src/hooks/mod.rs index 06996e73a2..75422076c4 100644 --- a/crates/tracedecay-agent-hosts/src/hooks/mod.rs +++ b/crates/tracedecay-agent-hosts/src/hooks/mod.rs @@ -366,19 +366,9 @@ fn hook_output_owner_event_id( macro_rules! read_hook_event { () => {{ - match $crate::hooks::read_stdin_bounded() { - Ok($crate::hooks::HookStdinRead::Event(event)) => event, - Ok($crate::hooks::HookStdinRead::Oversized) => { - eprintln!( - "tracedecay hook: stdin exceeds wire message bound ({})", - ::tracedecay_framing::WIRE_RECORD_TOO_LARGE - ); - return 0; - } - Err(e) => { - eprintln!("tracedecay hook: failed to read stdin: {e}"); - return 1; - } + match $crate::hooks::take_hook_event($crate::hooks::read_stdin_bounded()) { + Ok(event) => event, + Err(code) => return code, } }}; } @@ -1209,6 +1199,56 @@ pub(crate) enum HookStdinRead { Oversized, } +/// Shared stdin admission for every host hook. Policy lives here so a shell +/// cannot invent a second exit code or continue on an unread event. +pub(crate) enum HookStdinAdmission { + Event(String), + Oversized, + ReadFailed(std::io::Error), +} + +pub(crate) fn classify_hook_stdin(read: std::io::Result) -> HookStdinAdmission { + match read { + Ok(HookStdinRead::Event(event)) => HookStdinAdmission::Event(event), + Ok(HookStdinRead::Oversized) => HookStdinAdmission::Oversized, + Err(error) => HookStdinAdmission::ReadFailed(error), + } +} + +/// Exit code for a rejected stdin read. `None` means the event was admitted. +pub(crate) fn hook_stdin_exit_code(admission: &HookStdinAdmission) -> Option { + match admission { + HookStdinAdmission::Event(_) => None, + HookStdinAdmission::Oversized => Some(0), + HookStdinAdmission::ReadFailed(_) => Some(1), + } +} + +fn report_rejected_hook_stdin(admission: &HookStdinAdmission) { + match admission { + HookStdinAdmission::Oversized => { + eprintln!( + "tracedecay hook: stdin exceeds wire message bound ({})", + tracedecay_framing::WIRE_RECORD_TOO_LARGE + ); + } + HookStdinAdmission::ReadFailed(error) => { + eprintln!("tracedecay hook: failed to read stdin: {error}"); + } + HookStdinAdmission::Event(_) => {} + } +} + +pub(crate) fn take_hook_event(read: std::io::Result) -> Result { + match classify_hook_stdin(read) { + HookStdinAdmission::Event(event) => Ok(event), + admission => { + report_rejected_hook_stdin(&admission); + Err(hook_stdin_exit_code(&admission).unwrap_or(1)) + } + } +} + /// Read host-hook stdin with the host wire message byte cap enforced before /// whole-body materialization. pub(crate) fn read_stdin_bounded() -> std::io::Result { diff --git a/crates/tracedecay-agent-hosts/src/hooks/wire_stdin_bound_tests.rs b/crates/tracedecay-agent-hosts/src/hooks/wire_stdin_bound_tests.rs index 1d8fa9a412..1509befef0 100644 --- a/crates/tracedecay-agent-hosts/src/hooks/wire_stdin_bound_tests.rs +++ b/crates/tracedecay-agent-hosts/src/hooks/wire_stdin_bound_tests.rs @@ -1,5 +1,9 @@ -use super::{HookStdinRead, read_stdin_bounded_from}; -use std::io::{self, Read}; +use std::io::{self, ErrorKind, Read}; + +use super::{ + HookStdinAdmission, HookStdinRead, classify_hook_stdin, hook_stdin_exit_code, + read_stdin_bounded_from, +}; use tracedecay_framing::MAX_WIRE_MESSAGE_BYTES; struct ChunkedHostileReader { @@ -36,6 +40,19 @@ fn hook_stdin_streams_hostile_input_and_returns_oversized_without_payload() { assert!(hostile.remaining < MAX_WIRE_MESSAGE_BYTES + 512 * 1024); } +#[test] +fn hook_stdin_admission_is_one_policy() { + let event = classify_hook_stdin(Ok(HookStdinRead::Event("{\"stop\":true}".to_owned()))); + assert!(matches!(event, HookStdinAdmission::Event(_))); + assert_eq!(hook_stdin_exit_code(&event), None); + + let oversized = classify_hook_stdin(Ok(HookStdinRead::Oversized)); + assert_eq!(hook_stdin_exit_code(&oversized), Some(0)); + + let failed = classify_hook_stdin(Err(io::Error::new(ErrorKind::BrokenPipe, "closed"))); + assert_eq!(hook_stdin_exit_code(&failed), Some(1)); +} + #[test] fn hook_stdin_accepts_exact_wire_cap() { let body = vec![b'a'; MAX_WIRE_MESSAGE_BYTES]; diff --git a/crates/tracedecay-api/src/configuration.rs b/crates/tracedecay-api/src/configuration.rs index 8fef15bfa9..4db256c463 100644 --- a/crates/tracedecay-api/src/configuration.rs +++ b/crates/tracedecay-api/src/configuration.rs @@ -115,21 +115,37 @@ pub fn parse_code_index_worker_settings_patch( .map_err(|error| patch_shape_error("code index worker settings", &error)) } -/// Validate the transport-owned user patch invariants. The executable supplies -/// the duration parser because profile configuration remains its authority. -pub fn validate_user_settings_patch( +/// User-settings fields after the HTTP boundary has parsed display strings. +/// +/// `watcher_debounce_ms` is already a positive millisecond count. Planners +/// write that unsigned value and do not parse the wire string again. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AdmittedUserSettingsPatch { + pub upload_enabled: Option, + pub watcher_debounce_ms: Option, + pub extraction_timeout_secs: Option, +} + +/// Parse a user settings patch once. The executable supplies the duration +/// parser because profile configuration remains its authority. +pub fn admit_user_settings_patch( patch: &UserSettingsPatch, - duration_is_valid: impl Fn(&str) -> bool, -) -> Result<(), DashboardConfigurationRouteErrorV1> { + parse_debounce_millis: impl Fn(&str) -> Option, +) -> Result { let mut errors = Vec::new(); - if let Some(debounce) = &patch.watcher_debounce - && !duration_is_valid(debounce) - { - errors.push(validation_error( - "watcher_debounce", - "watcher_debounce must be a duration like \"2s\", \"15s\", or \"1m\"", - )); - } + let watcher_debounce_ms = match patch.watcher_debounce.as_deref() { + Some(debounce) => match parse_debounce_millis(debounce) { + Some(millis) => Some(millis), + None => { + errors.push(validation_error( + "watcher_debounce", + "watcher_debounce must be a duration like \"2s\", \"15s\", or \"1m\"", + )); + None + } + }, + None => None, + }; if patch.extraction_timeout_secs == Some(0) { errors.push(validation_error( "extraction_timeout_secs", @@ -137,7 +153,11 @@ pub fn validate_user_settings_patch( )); } if errors.is_empty() { - Ok(()) + Ok(AdmittedUserSettingsPatch { + upload_enabled: patch.upload_enabled, + watcher_debounce_ms, + extraction_timeout_secs: patch.extraction_timeout_secs, + }) } else { Err(settings_validation_error(errors)) } @@ -289,6 +309,32 @@ mod tests { assert_eq!(body["validation_errors"][0]["field"], "code_index_workers"); } + #[test] + fn user_settings_patch_parses_debounce_once() { + let patch = parse_user_settings_patch(json!({ + "expected_revision_id": "configuration.revision.fixture", + "idempotency_key": "configuration.idempotency.fixture", + "watcher_debounce": "15s", + "extraction_timeout_secs": 60 + })) + .unwrap(); + let admitted = + admit_user_settings_patch(&patch, |value| (value == "15s").then_some(15_000)).unwrap(); + assert_eq!(admitted.watcher_debounce_ms, Some(15_000)); + assert_eq!(admitted.extraction_timeout_secs, Some(60)); + + let invalid = parse_user_settings_patch(json!({ + "expected_revision_id": "configuration.revision.fixture", + "idempotency_key": "configuration.idempotency.fixture", + "watcher_debounce": "0s" + })) + .unwrap(); + let (status, Json(body)) = admit_user_settings_patch(&invalid, |_| None) + .expect_err("zero duration must be denied"); + assert_eq!(status, StatusCode::BAD_REQUEST); + assert_eq!(body["validation_errors"][0]["field"], "watcher_debounce"); + } + #[test] fn project_backed_user_patch_rejects_mixed_profile_worker_mutation() { let result = parse_user_settings_patch(json!({ diff --git a/crates/tracedecay-application/src/analytics_bridge/diagnostics.rs b/crates/tracedecay-application/src/analytics_bridge/diagnostics.rs index fef211713e..670ed3996a 100644 --- a/crates/tracedecay-application/src/analytics_bridge/diagnostics.rs +++ b/crates/tracedecay-application/src/analytics_bridge/diagnostics.rs @@ -81,7 +81,7 @@ pub async fn analytics_diagnostics_with_db( let observatory = hotpath::measure_block!("analytics.observatory", { let observatory = crate::observability::observatory_read_model(gdb, project_filter.as_deref(), 0).await; - crate::observability::observatory_cli_value(&observatory).map_err(cli_error)? + serde_json::to_value(&observatory).map_err(cli_error)? }); let provider_scope = if all_projects { None @@ -105,7 +105,7 @@ pub async fn analytics_diagnostics_with_db( 0, ) .await; - crate::observability::costs_cli_value(&costs).map_err(cli_error)? + serde_json::to_value(&costs).map_err(cli_error)? }); let event_rows: Vec = events.iter().map(durable_analytics_event_row).collect(); diff --git a/crates/tracedecay-application/src/observability.rs b/crates/tracedecay-application/src/observability.rs index a9ea5375bd..c19df7b2bf 100644 --- a/crates/tracedecay-application/src/observability.rs +++ b/crates/tracedecay-application/src/observability.rs @@ -27,9 +27,8 @@ mod workflow_emit; pub use cost_latency::{provider_latency_read_model, unavailable_provider_latency}; pub use costs::{ - costs_cli_value, costs_export_bytes, costs_mcp_value, costs_read_model, - costs_read_model_with_provider_usage, costs_read_model_with_provider_usage_and_observability, - costs_unavailable_read_model, + costs_read_model, costs_read_model_with_provider_usage, + costs_read_model_with_provider_usage_and_observability, costs_unavailable_read_model, }; pub use delivery_recorder::{ BoundedDeliverySettlementRecorderV1, DeliverySettlementRecordOutcomeV1, @@ -101,40 +100,6 @@ const ANALYTICS_DESCRIPTOR: &str = "analytics-events.v1"; pub(super) const COST_DESCRIPTOR: &str = "provider-costs.v1"; const FEEDBACK_DESCRIPTOR: &str = "feedback-system-quality.v1"; -/// Canonical wire projection used by every dashboard wire surface. Adapters may wrap the -/// value in their transport framing but may not recompute metrics or coverage. -fn canonical_observatory_value( - model: &ObservatoryReadModelV1, -) -> Result { - serde_json::to_value(model) -} - -pub fn observatory_cli_value( - model: &ObservatoryReadModelV1, -) -> Result { - canonical_observatory_value(model) -} - -pub fn observatory_mcp_value( - model: &ObservatoryReadModelV1, -) -> Result { - canonical_observatory_value(model) -} - -pub fn observatory_http_value( - model: &ObservatoryReadModelV1, -) -> Result { - canonical_observatory_value(model) -} - -/// Bounded public JSON export. It is the same canonical model as interactive -/// surfaces, including absent values, exact denominator, and coverage state. -pub fn observatory_export_bytes( - model: &ObservatoryReadModelV1, -) -> Result, serde_json::Error> { - serde_json::to_vec(model) -} - fn coverage( eligible: Option, observed: u64, @@ -529,46 +494,36 @@ mod tests { 10, "fixture_source_unavailable", ); - let cli = observatory_cli_value(&observatory).expect("CLI JSON"); - let mcp = observatory_mcp_value(&observatory).expect("MCP JSON"); - let http = observatory_http_value(&observatory).expect("HTTP JSON"); - let dashboard = serde_json::to_value(&observatory).expect("dashboard payload"); + let wire = serde_json::to_value(&observatory).expect("observatory wire"); let export: serde_json::Value = - serde_json::from_slice(&observatory_export_bytes(&observatory).expect("export JSON")) - .expect("decode export JSON"); - assert_eq!(cli, mcp); - assert_eq!(cli, http); - assert_eq!(cli, dashboard); - assert_eq!(cli, export); - assert_eq!(cli["metrics"][0]["value"], serde_json::Value::Null); + serde_json::from_slice(&serde_json::to_vec(&observatory).expect("observatory export")) + .expect("decode observatory export"); + assert_eq!(wire, export); + assert_eq!(wire["metrics"][0]["value"], serde_json::Value::Null); assert_eq!( - cli["metrics"][0]["denominator_value"], + wire["metrics"][0]["denominator_value"], serde_json::Value::Null ); - assert_eq!(cli["metrics"][0]["coverage"]["state"], "unknown"); + assert_eq!(wire["metrics"][0]["coverage"]["state"], "unknown"); assert_eq!( - cli["metrics"][0]["unavailable_reason"], + wire["metrics"][0]["unavailable_reason"], "fixture_source_unavailable" ); let costs = costs_unavailable_read_model(Some("scope:parity"), 10, "fixture_cost_unavailable"); - let cli = costs_cli_value(&costs).expect("CLI costs JSON"); - let mcp = costs_mcp_value(&costs).expect("MCP costs JSON"); - let dashboard = serde_json::to_value(&costs).expect("dashboard costs payload"); + let wire = serde_json::to_value(&costs).expect("costs wire"); let export: serde_json::Value = - serde_json::from_slice(&costs_export_bytes(&costs).expect("costs export JSON")) - .expect("decode costs export JSON"); - assert_eq!(cli, mcp); - assert_eq!(cli, dashboard); - assert_eq!(cli, export); - assert_eq!(cli["usage"][0]["coverage"]["state"], "unknown"); + serde_json::from_slice(&serde_json::to_vec(&costs).expect("costs export")) + .expect("decode costs export"); + assert_eq!(wire, export); + assert_eq!(wire["usage"][0]["coverage"]["state"], "unknown"); assert_eq!( - cli["usage"][0]["denominator_value"], + wire["usage"][0]["denominator_value"], serde_json::Value::Null ); assert_eq!( - cli["usage"][0]["unavailable_reason"], + wire["usage"][0]["unavailable_reason"], "fixture_cost_unavailable" ); } diff --git a/crates/tracedecay-application/src/observability/costs.rs b/crates/tracedecay-application/src/observability/costs.rs index 155fdc4a6e..0fdf017c60 100644 --- a/crates/tracedecay-application/src/observability/costs.rs +++ b/crates/tracedecay-application/src/observability/costs.rs @@ -1,4 +1,7 @@ -//! Canonical Costs read-model composition and wire serialization. +//! Canonical Costs read-model composition. +//! +//! Transport adapters serialize [`CostsReadModelV1`] themselves. This module +//! does not name a CLI, MCP, or HTTP wire. use tracedecay_contracts::{CostsReadModelV1, MetricCoverageV1, MetricSourceV1, now_micros}; use tracedecay_domain::{CoverageStateV1, ObservationScopeV1}; @@ -15,22 +18,6 @@ use tracedecay_session_memory::provider_usage::{ price_provider_usage, provider_usage_aggregate, }; -fn canonical_costs_value(model: &CostsReadModelV1) -> Result { - serde_json::to_value(model) -} - -pub fn costs_cli_value(model: &CostsReadModelV1) -> Result { - canonical_costs_value(model) -} - -pub fn costs_mcp_value(model: &CostsReadModelV1) -> Result { - canonical_costs_value(model) -} - -pub fn costs_export_bytes(model: &CostsReadModelV1) -> Result, serde_json::Error> { - serde_json::to_vec(model) -} - pub fn costs_unavailable_read_model( scope_ref: Option<&str>, since_seconds: i64, diff --git a/crates/tracedecay-cli/src/agent_cmd.rs b/crates/tracedecay-cli/src/agent_cmd.rs index 2bcd832e36..b3b5829666 100644 --- a/crates/tracedecay-cli/src/agent_cmd.rs +++ b/crates/tracedecay-cli/src/agent_cmd.rs @@ -265,12 +265,21 @@ fn apply_host_bundle_artifact_action_at( message: "artifact backup/restore has no dry-run mode".to_string(), }); } - // A backup only ever writes a new snapshot; nothing deployed changes, so - // it needs no confirmation. A restore overwrites deployed bytes and keeps - // requiring `--yes`. - if matches!(action, crate::cli::HostBundleAction::ArtifactRestore { .. }) && !options.yes { + // The writer refuses either operation without explicit confirmation. + // Restore overwrites deployed bytes; backup publishes a receipt. The shell + // must not advertise a weaker policy than that contract. + if !options.yes { + let message = match &action { + crate::cli::HostBundleAction::ArtifactBackup { .. } => "artifact backup requires --yes", + crate::cli::HostBundleAction::ArtifactRestore { .. } => { + "artifact restore requires --yes" + } + crate::cli::HostBundleAction::Status | crate::cli::HostBundleAction::Recover { .. } => { + "status and recovery are not artifact backup/restore operations" + } + }; return Err(tracedecay_domain::errors::TraceDecayError::Config { - message: "artifact restore requires --yes".to_string(), + message: message.to_string(), }); } let component = @@ -1755,10 +1764,10 @@ mod tests { use super::{ AgentReinstallOutcome, CatalogHostComponentRegistrationAuthority, ComponentSetApplyContext, HostBundleCliOperation, apply_canonical_component_set, - apply_default_canonical_component_set, broker_codex_daemon_automation_project, - canonical_host_component_set, canonical_host_component_set_with_tracedecay_bin, - component_is_not_applicable, component_set_request, - reinstall_agent_integrations_with_persisted_dashboard_policies, + apply_default_canonical_component_set, apply_host_bundle_artifact_action_at, + broker_codex_daemon_automation_project, canonical_host_component_set, + canonical_host_component_set_with_tracedecay_bin, component_is_not_applicable, + component_set_request, reinstall_agent_integrations_with_persisted_dashboard_policies, }; use tracedecay_agent_hosts::agents::host_bundle::{ CompetingHostExtensionClaimV1, HostBundleError, HostComponentSetExecutionRequestV1, @@ -2570,6 +2579,24 @@ mod tests { ); } + #[test] + fn artifact_backup_requires_the_writer_confirmation() { + let error = apply_host_bundle_artifact_action_at( + crate::cli::HostBundleAction::ArtifactBackup { + agent: "opencode".to_string(), + }, + crate::cli::HostBundleCliOptions::default(), + Path::new("/tmp"), + Path::new("/tmp"), + 0, + ) + .unwrap_err(); + assert!( + error.to_string().contains("artifact backup requires --yes"), + "{error}" + ); + } + #[test] fn artifact_command_route_backs_up_and_restores_managed_files() { let _profile = pinned_host_profile(); diff --git a/crates/tracedecay-cli/src/cli.rs b/crates/tracedecay-cli/src/cli.rs index b8c784340f..55874a367d 100644 --- a/crates/tracedecay-cli/src/cli.rs +++ b/crates/tracedecay-cli/src/cli.rs @@ -132,7 +132,10 @@ pub enum HostBundleAction { #[arg(long)] quarantine: bool, }, - /// Snapshot one installed component's managed artifact files + /// Snapshot one installed component's managed artifact files. + /// + /// Requires the global `--yes` confirmation. The backup writer refuses an + /// unconfirmed receipt even though deployed files are not overwritten. ArtifactBackup { /// Agent whose selected component owns the managed artifacts #[arg(long, value_parser = agent_value_parser())] diff --git a/crates/tracedecay-cli/src/cli/help.rs b/crates/tracedecay-cli/src/cli/help.rs index 43a022ef51..1c4cbbbefa 100644 --- a/crates/tracedecay-cli/src/cli/help.rs +++ b/crates/tracedecay-cli/src/cli/help.rs @@ -250,7 +250,7 @@ Examples: tracedecay host-bundle recover --dry-run tracedecay host-bundle recover --agent opencode --yes tracedecay host-bundle recover --agent opencode --quarantine --yes - tracedecay host-bundle artifact-backup --agent opencode --component agent + tracedecay host-bundle artifact-backup --agent opencode --component agent --yes tracedecay host-bundle artifact-restore --agent opencode --component agent --backup-id <32-hex-id> --yes Quarantine moves the journal aside into the lifecycle control directory and diff --git a/crates/tracedecay-configuration/src/config/model.rs b/crates/tracedecay-configuration/src/config/model.rs index 42e82de216..beb1fdbd4a 100644 --- a/crates/tracedecay-configuration/src/config/model.rs +++ b/crates/tracedecay-configuration/src/config/model.rs @@ -31,25 +31,13 @@ use tracedecay_runtime_core::config::{ is_generated_dir_segment, }; -use super::{ - PinnedRuntimeConfiguration, optional_text_setting, required_bool, required_unsigned, - required_usize, -}; +use super::{PinnedRuntimeConfiguration, required_bool, required_unsigned, required_usize}; /// Name of the legacy configuration migration input stored inside the data /// directory. It is not a runtime authority and production code must never /// rewrite it. pub const CONFIG_FILENAME: &str = "config.json"; -/// Atomic daemon retention/compaction policy tree. -/// -/// The value is canonical JSON for [`RetentionConfig`]. Keeping the session -/// (LCM), observation-evidence, orphan-store, debris, and compaction windows -/// under one setting keeps the retention engines threaded as a single -/// versioned unit the daemon backstop reads, mirroring the semantic key. Absent -/// or unset resolves to [`RetentionConfig::default`]'s bounded safe policy. -pub const SYNC_RETENTION_SETTING_KEY: &str = "sync.retention.v1"; - /// Returns `true` if any component of `path` is a generated/vendored /// directory segment, or `path` itself carries a minified-asset suffix /// (`app.min.js`, `app.min.css`, ...) — mirrors the `**/*.min.*` default @@ -576,9 +564,12 @@ impl TraceDecayConfig { /// Layers the daemon-only policy over the shared runtime settings of an /// already validated pin. The shared settings are copied from the pin, so /// they agree with every other consumer by construction; only the - /// daemon-only sync, retention, and legacy metadata fields are decoded - /// here, from the same snapshot, without defaults, file reads, or - /// environment reads. + /// daemon-only sync and legacy metadata fields are decoded here, from the + /// same snapshot, without defaults, file reads, or environment reads. + /// + /// Retention is not a registered configuration setting. An unregistered + /// text blob is not parsed into policy; the admitted value is + /// [`RetentionConfig::default`]. #[hotpath::measure(label = "daemon.config.parse")] pub fn from_runtime(runtime: &PinnedRuntimeConfiguration) -> Result { let shared = runtime.config(); @@ -633,7 +624,7 @@ impl TraceDecayConfig { auto_init: required_bool(snapshot, SYNC_AUTO_INIT_SETTING_KEY)?, auto_track_pr_branches: shared.sync.auto_track_pr_branches, auto_track_pr_poll_secs: shared.sync.auto_track_pr_poll_secs, - retention: retention_config_from_snapshot(snapshot)?, + retention: admitted_retention_config(snapshot), }, telemetry: TelemetryConfig { timings: shared.telemetry.timings, @@ -642,15 +633,10 @@ impl TraceDecayConfig { } } -fn retention_config_from_snapshot(snapshot: &ConfigurationSnapshotV1) -> Result { - let retention = match optional_text_setting(snapshot, SYNC_RETENTION_SETTING_KEY)? { - None => RetentionConfig::default(), - Some(value) => serde_json::from_str(value).map_err(|error| { - config_error(format!("resolved retention setting is invalid: {error}")) - })?, - }; - retention.validate()?; - Ok(retention) +/// Retention is not in the closed configuration key inventory. A snapshot may +/// still carry an invented text key; that private JSON is not policy. +fn admitted_retention_config(_snapshot: &ConfigurationSnapshotV1) -> RetentionConfig { + RetentionConfig::default() } fn config_error(message: impl Into) -> TraceDecayError { @@ -707,6 +693,7 @@ pub fn load_config_from_path(project_root: &Path, config_path: &Path) -> Result< e ), })?; + config.sync.retention.validate()?; Ok(config) } diff --git a/crates/tracedecay-configuration/src/config/model/tests.rs b/crates/tracedecay-configuration/src/config/model/tests.rs index 5646af820b..95547e967c 100644 --- a/crates/tracedecay-configuration/src/config/model/tests.rs +++ b/crates/tracedecay-configuration/src/config/model/tests.rs @@ -552,8 +552,57 @@ fn default_excludes_still_catch_target_and_worktrees() { #[cfg(test)] #[allow(clippy::unwrap_used)] mod retention_config_tests { - use crate::{RetentionConfig, SyncConfig}; + use std::collections::BTreeMap; + + use crate::{RetentionConfig, SyncConfig, TraceDecayConfig}; use tracedecay_contracts::storage::compaction::CompactionThresholdConfig; + use tracedecay_domain::configuration::{ + CandidateDispositionV1, ConfigurationCandidateV1, ConfigurationLayerIdV1, + ConfigurationRevisionId, ConfigurationSnapshotV1, ConfigurationValueV1, SettingKey, + }; + + #[test] + fn unregistered_retention_text_is_not_policy() { + let key = SettingKey::new("sync.retention.v1").unwrap(); + let mut values = BTreeMap::new(); + values.insert( + key.clone(), + ConfigurationValueV1::Text(r#"{"orphan_store_gc_days":7}"#.to_owned()), + ); + let mut provenance = BTreeMap::new(); + provenance.insert( + key, + vec![ConfigurationCandidateV1 { + layer: ConfigurationLayerIdV1::Default, + revision_id: ConfigurationRevisionId::new("configuration.revision.fixture") + .unwrap(), + disposition: CandidateDispositionV1::Winning, + safe_reason: None, + }], + ); + let snapshot = ConfigurationSnapshotV1::new(values, provenance).unwrap(); + let retention = super::super::admitted_retention_config(&snapshot); + assert_eq!(retention, RetentionConfig::default()); + assert_ne!(retention.orphan_store_gc_days, Some(7)); + } + + #[test] + fn legacy_config_file_rejects_zero_retention_window() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.json"); + let mut config = TraceDecayConfig::default(); + config.sync.retention.orphan_store_gc_days = Some(0); + std::fs::write( + &path, + serde_json::to_string(&config).expect("serialize legacy config"), + ) + .unwrap(); + let error = super::super::load_config_from_path(dir.path(), &path).unwrap_err(); + assert!( + error.to_string().contains("orphan_store_gc_days"), + "{error}" + ); + } #[test] fn default_retention_runs_only_safe_bounded_maintenance() { diff --git a/crates/tracedecay-configuration/src/configuration/user_settings.rs b/crates/tracedecay-configuration/src/configuration/user_settings.rs index fbb89d627e..be6f184d6b 100644 --- a/crates/tracedecay-configuration/src/configuration/user_settings.rs +++ b/crates/tracedecay-configuration/src/configuration/user_settings.rs @@ -41,7 +41,8 @@ pub struct UserSettingsSnapshotV1 { #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct UserSettingsMutationV1 { pub upload_enabled: Option, - pub watcher_debounce: Option, + /// Positive milliseconds already parsed at the settings boundary. + pub watcher_debounce_ms: Option, pub extraction_timeout_secs: Option, } @@ -204,9 +205,7 @@ pub fn plan_user_settings_mutation( ConfigurationValueV1::Boolean(upload_enabled), )?); } - if let Some(watcher_debounce) = mutation.watcher_debounce { - let watcher_debounce_ms = parse_duration_millis(&watcher_debounce) - .ok_or_else(|| unavailable("canonical watcher debounce duration"))?; + if let Some(watcher_debounce_ms) = mutation.watcher_debounce_ms { mutations.push(set( layer.clone(), USER_WATCHER_DEBOUNCE_MS_SETTING_KEY, @@ -328,7 +327,7 @@ mod tests { profile_id.clone(), UserSettingsMutationV1 { upload_enabled: Some(true), - watcher_debounce: Some("15s".to_owned()), + watcher_debounce_ms: Some(15_000), extraction_timeout_secs: Some(60), }, ) @@ -336,6 +335,13 @@ mod tests { assert_eq!(plan.mutations.len(), 3); assert!(plan.restart_recommended); + assert!(plan.mutations.iter().any(|mutation| matches!( + mutation, + DirectConfigurationMutation::Set { + value, + .. + } if matches!(value.as_ref(), ConfigurationValueV1::Unsigned(15_000)) + ))); assert!(plan.mutations.iter().all(|mutation| { matches!( mutation.target_layer(), diff --git a/crates/tracedecay-configuration/src/lib.rs b/crates/tracedecay-configuration/src/lib.rs index d65ba7e375..23b62fa17d 100644 --- a/crates/tracedecay-configuration/src/lib.rs +++ b/crates/tracedecay-configuration/src/lib.rs @@ -8,10 +8,10 @@ pub mod config; pub mod configuration; pub use config::model::{ - CONFIG_FILENAME, MIN_AUTO_TRACK_PR_POLL_SECS, RetentionConfig, SYNC_RETENTION_SETTING_KEY, - SyncConfig, TelemetryConfig, TraceDecayConfig, brand_env, get_config_path, is_excluded, - is_excluded_dir, is_generated_path_segment, is_in_gitignore, is_included, load_config, - load_config_from_path, resolve_path, resolve_path_with_discovery, save_config_to_path, + CONFIG_FILENAME, MIN_AUTO_TRACK_PR_POLL_SECS, RetentionConfig, SyncConfig, TelemetryConfig, + TraceDecayConfig, brand_env, get_config_path, is_excluded, is_excluded_dir, + is_generated_path_segment, is_in_gitignore, is_included, load_config, load_config_from_path, + resolve_path, resolve_path_with_discovery, save_config_to_path, }; pub use config::{ OpenedRuntimeConfiguration, PinnedRuntimeConfiguration, PinnedRuntimeConfigurationCachePort, diff --git a/crates/tracedecay-dashboard-api/src/settings_api.rs b/crates/tracedecay-dashboard-api/src/settings_api.rs index 05197a0a7b..b96c91ad78 100644 --- a/crates/tracedecay-dashboard-api/src/settings_api.rs +++ b/crates/tracedecay-dashboard-api/src/settings_api.rs @@ -12,11 +12,11 @@ use serde::Serialize; use serde_json::{Value, json}; use tracedecay_api::configuration::{ DashboardConfigurationRouteErrorV1, PROJECT_SETTINGS_APPLY_OPERATION, - SETTINGS_REFRESH_OPERATION, configuration_application_problem_error, + SETTINGS_REFRESH_OPERATION, admit_user_settings_patch, configuration_application_problem_error, configuration_authority_unavailable_error, configuration_revision_conflict_error, parse_code_index_worker_settings_patch, parse_project_settings_patch, parse_user_settings_patch, settings_validation_error, - validate_code_index_worker_settings_patch, validate_user_settings_patch, + validate_code_index_worker_settings_patch, }; use tracedecay_contracts::{ ApplicationOutcome, ApplicationProblemEnvelope, ApplicationProblemKind, @@ -426,7 +426,7 @@ pub async fn patch_user_settings( ) -> ApiResult { let patch = parse_user_settings_patch(patch)?; let pr_autotrack = pr_autotrack_payload(&state)?; - validate_user_settings_patch(&patch, |value| parse_duration_millis(value).is_some())?; + let admitted = admit_user_settings_patch(&patch, parse_duration_millis)?; let idempotency_key = ConfigurationIdempotencyKey::new(patch.idempotency_key.clone()).map_err(|_| { settings_validation_error(json!([{ @@ -458,9 +458,9 @@ pub async fn patch_user_settings( ¤t, profile_id, UserSettingsMutationV1 { - upload_enabled: patch.upload_enabled, - watcher_debounce: patch.watcher_debounce, - extraction_timeout_secs: patch.extraction_timeout_secs, + upload_enabled: admitted.upload_enabled, + watcher_debounce_ms: admitted.watcher_debounce_ms, + extraction_timeout_secs: admitted.extraction_timeout_secs, }, ) .map_err(|_| configuration_authority_unavailable_error())?; diff --git a/crates/tracedecay-dashboard-api/src/storage_telemetry_api.rs b/crates/tracedecay-dashboard-api/src/storage_telemetry_api.rs index 3140eb977a..7d9ea50873 100644 --- a/crates/tracedecay-dashboard-api/src/storage_telemetry_api.rs +++ b/crates/tracedecay-dashboard-api/src/storage_telemetry_api.rs @@ -7,13 +7,12 @@ //! result. //! //! Both typed dimensions now have a real server-side source: -//! - **budget**: the owner-configurable soft budgets live in the configuration -//! control plane under [`tracedecay_configuration::SYNC_RETENTION_SETTING_KEY`] -//! (`sync.retention.v1` → `store_soft_budgets_bytes`, keyed by store key). -//! A configured budget is evaluated against the live sample; a store with no -//! entry reports `unset` — *the owner has not configured a budget*, which is -//! deliberately distinct from "the server cannot evaluate budgets". A config -//! or sample the dashboard cannot read reports `unknown`, never a fabricated +//! - **budget**: soft budgets are a field of [`tracedecay_configuration::RetentionConfig`] +//! (`store_soft_budgets_bytes`). That tree is not a registered configuration +//! setting, so a control-plane snapshot cannot admit one. A store with no +//! entry reports `unset` — *no budget is configured*, which is deliberately +//! distinct from "the server cannot evaluate budgets". A config or sample +//! the dashboard cannot read reports `unknown`, never a fabricated //! "within budget". //! - **growth**: this route never establishes watermarks. Until a daemon-owned //! execution sampler supplies a bounded history, store growth is typed @@ -137,12 +136,15 @@ pub struct StorageTelemetryPayloadV1 { pub table_growth_coverage: DashboardCoverageV1, } -/// The owner setting path that configures a store's soft byte budget. -pub const BUDGET_SETTING_KEY: &str = "sync.retention.v1 store_soft_budgets_bytes"; -const BUDGET_UNSET_REASON: &str = "no soft size budget is configured by the owner for this store (set \ - sync.retention.v1 store_soft_budgets_bytes for the store key to configure one)"; -const BUDGET_NOTE: &str = "budgets are owner configuration: sync.retention.v1 store_soft_budgets_bytes, keyed by store \ - key; a store with no entry reports unset (no budget configured), never a fabricated pass"; +/// Internal retention field that would carry a soft byte budget. +/// +/// No registered configuration setting writes this field. Unset means no +/// budget is configured, not that an unregistered text key was parsed. +pub const BUDGET_SETTING_KEY: &str = "store_soft_budgets_bytes"; +const BUDGET_UNSET_REASON: &str = "no registered configuration setting configures a soft size budget for this store \ + (store_soft_budgets_bytes)"; +const BUDGET_NOTE: &str = "soft size budgets are not a registered configuration setting; a store with no entry reports \ + unset (store_soft_budgets_bytes), never a fabricated pass"; const GROWTH_NOTE: &str = "store growth requires bounded execution-owned watermarks; dashboard reads observe current size but \ never establish a baseline or a historical series"; const GROWTH_UNKNOWN_REASON: &str = diff --git a/crates/tracedecay-mcp/src/handlers/analytics.rs b/crates/tracedecay-mcp/src/handlers/analytics.rs index 0dec94da69..1ff48a33b0 100644 --- a/crates/tracedecay-mcp/src/handlers/analytics.rs +++ b/crates/tracedecay-mcp/src/handlers/analytics.rs @@ -396,8 +396,7 @@ async fn observatory_and_costs_sections( label = "mcp.analytics.report.observatory" ) .await; - let observatory = tracedecay_application::observability::observatory_mcp_value(&observatory) - .map_err(config_error)?; + let observatory = serde_json::to_value(&observatory).map_err(config_error)?; let provider_scope = if all_projects { None } else { @@ -426,8 +425,7 @@ async fn observatory_and_costs_sections( label = "mcp.analytics.report.costs" ) .await; - let costs = - tracedecay_application::observability::costs_mcp_value(&costs).map_err(config_error)?; + let costs = serde_json::to_value(&costs).map_err(config_error)?; let object = value .as_object_mut() .ok_or_else(|| config_error("analytics response must be a JSON object"))?;