From 4e5c8ca60a8ce38ec1114d2066e9434820eacc1a Mon Sep 17 00:00:00 2001 From: stefanskoricdev Date: Tue, 21 Jul 2026 13:45:46 +0200 Subject: [PATCH 1/2] observability: Replace file sink config with log directory routing Replace log_file/log_file_mode with log_dir/SCE_LOG_DIR across config schema, resolution, and config rendering. Route enabled log records to append-only date/session-specific files under the configured directory while preserving stderr emission and fail-open file write diagnostics. Thread optional session IDs through logger trait methods for per-session file partitioning. Co-authored-by: SCE --- .sce/config.json | 3 +- .../config/schema/sce-config.schema.json | 16 +- cli/src/app.rs | 2 + cli/src/services/app_support.rs | 7 +- cli/src/services/config/render.rs | 13 +- cli/src/services/config/resolver.rs | 58 +-- cli/src/services/config/schema.rs | 31 +- cli/src/services/config/types.rs | 39 +- cli/src/services/default_paths.rs | 5 - cli/src/services/hooks/mod.rs | 9 +- cli/src/services/observability.rs | 364 +++++++++--------- cli/src/services/observability/traits.rs | 116 +++++- cli/src/services/parse/command_runtime.rs | 1 + config/pkl/base/sce-config-schema.pkl | 9 +- config/schema/sce-config.schema.json | 16 +- context/architecture.md | 4 +- context/cli/config-precedence-contract.md | 16 +- context/context-map.md | 4 +- context/glossary.md | 13 +- context/overview.md | 6 +- context/patterns.md | 2 +- context/sce/cli-observability-contract.md | 34 +- 22 files changed, 372 insertions(+), 396 deletions(-) diff --git a/.sce/config.json b/.sce/config.json index a756fd03..33ab585b 100644 --- a/.sce/config.json +++ b/.sce/config.json @@ -7,8 +7,7 @@ "pi" ] }, - "log_file": "context/tmp/sce.log", - "log_file_mode": "append", + "log_dir": "context/tmp", "log_level": "error", "policies": { "attribution_hooks": { diff --git a/cli/assets/generated/config/schema/sce-config.schema.json b/cli/assets/generated/config/schema/sce-config.schema.json index dfd6eff1..45ccb6ee 100644 --- a/cli/assets/generated/config/schema/sce-config.schema.json +++ b/cli/assets/generated/config/schema/sce-config.schema.json @@ -25,17 +25,10 @@ "json" ] }, - "log_file": { + "log_dir": { "type": "string", "minLength": 1 }, - "log_file_mode": { - "type": "string", - "enum": [ - "truncate", - "append" - ] - }, "timeout_ms": { "type": "integer", "minimum": 0 @@ -358,10 +351,5 @@ "additionalProperties": false } }, - "additionalProperties": false, - "dependentRequired": { - "log_file_mode": [ - "log_file" - ] - } + "additionalProperties": false } diff --git a/cli/src/app.rs b/cli/src/app.rs index 19997da4..aacbe1fa 100644 --- a/cli/src/app.rs +++ b/cli/src/app.rs @@ -356,6 +356,7 @@ where "sce.app.start", "Starting command dispatch", &[("component", services::observability::NAME)], + None, ); let Some(command_args) = args.take() else { return Err(ClassifiedError::runtime(REPEATED_COMMAND_DISPATCH_ERROR)); @@ -380,6 +381,7 @@ where "sce.command.parsed", "Command parsed", &[("command", command.name().as_ref())], + None, ); Ok(command) } diff --git a/cli/src/services/app_support.rs b/cli/src/services/app_support.rs index 3243ab4f..fa5eef73 100644 --- a/cli/src/services/app_support.rs +++ b/cli/src/services/app_support.rs @@ -88,6 +88,7 @@ pub(crate) fn log_startup_configuration( ("path", loaded_path.path.to_string_lossy().as_ref()), ("source", loaded_path.source.as_str()), ], + None, ); } for validation_error in &observability_config.validation_errors { @@ -95,6 +96,7 @@ pub(crate) fn log_startup_configuration( INVALID_CONFIG_WARNING_EVENT_ID, "Invalid discovered config skipped; using degraded defaults", &[("error", validation_error.as_str())], + None, ); } } @@ -112,6 +114,7 @@ where "sce.command.dispatch_start", "Dispatching command", &[("command", command_name.as_ref())], + None, ); let dispatch_result = command.execute(context); if dispatch_result.is_ok() { @@ -119,6 +122,7 @@ where "sce.command.dispatch_end", "Command dispatch completed", &[("command", command_name.as_ref())], + None, ); } dispatch_result.inspect(|_payload| { @@ -126,6 +130,7 @@ where "sce.command.completed", "Command completed", &[("command", command_name.as_ref())], + None, ); }) } @@ -136,7 +141,7 @@ where W: Write, { if let Some(log) = logger { - log.log_classified_error(error); + log.log_classified_error(error, None); } write_error_diagnostic(stderr, error); ExitCode::from(error.class().exit_code()) diff --git a/cli/src/services/config/render.rs b/cli/src/services/config/render.rs index 720907dd..3a6a3995 100644 --- a/cli/src/services/config/render.rs +++ b/cli/src/services/config/render.rs @@ -57,11 +57,7 @@ pub(super) fn format_show_output(runtime: &RuntimeConfig, report_format: ReportF runtime.log_format.value.as_str(), runtime.log_format.source, ), - "log_file": format_optional_resolved_value_json(&runtime.log_file), - "log_file_mode": format_resolved_value_json( - runtime.log_file_mode.value.as_str(), - runtime.log_file_mode.source, - ), + "log_dir": format_optional_resolved_value_json(&runtime.log_dir), "timeout_ms": { "value": runtime.timeout_ms.value, "source": runtime.timeout_ms.source.as_str(), @@ -204,12 +200,7 @@ fn format_observability_text_lines(runtime: &RuntimeConfig) -> Vec { runtime.log_format.value.as_str(), runtime.log_format.source, ), - format_optional_resolved_value_text("log_file", &runtime.log_file), - format_resolved_value_text( - "log_file_mode", - runtime.log_file_mode.value.as_str(), - runtime.log_file_mode.source, - ), + format_optional_resolved_value_text("log_dir", &runtime.log_dir), ] } diff --git a/cli/src/services/config/resolver.rs b/cli/src/services/config/resolver.rs index b370ca64..fee70c34 100644 --- a/cli/src/services/config/resolver.rs +++ b/cli/src/services/config/resolver.rs @@ -14,10 +14,9 @@ use super::policy::{build_validation_warnings, resolve_bash_policy_config, BashP use super::schema; use super::types::{ parse_bool_value_from, ConfigPathSource, ConfigRequest, DatabaseRetryConfig, LoadedConfigPath, - LogFileMode, LogFormat, LogLevel, ReportFormat, ResolvedAuthRuntimeConfig, - ResolvedHookRuntimeConfig, ResolvedObservabilityRuntimeConfig, ResolvedOptionalValue, - ResolvedValue, ValueSource, ENV_ATTRIBUTION_HOOKS_DISABLED, ENV_LOG_FILE, ENV_LOG_FILE_MODE, - ENV_LOG_FORMAT, ENV_LOG_LEVEL, + LogFormat, LogLevel, ReportFormat, ResolvedAuthRuntimeConfig, ResolvedHookRuntimeConfig, + ResolvedObservabilityRuntimeConfig, ResolvedOptionalValue, ResolvedValue, ValueSource, + ENV_ATTRIBUTION_HOOKS_DISABLED, ENV_LOG_DIR, ENV_LOG_FORMAT, ENV_LOG_LEVEL, }; const DEFAULT_TIMEOUT_MS: u64 = 30000; @@ -58,8 +57,7 @@ pub(super) struct RuntimeConfig { pub(super) loaded_config_paths: Vec, pub(super) log_level: ResolvedValue, pub(super) log_format: ResolvedValue, - pub(super) log_file: ResolvedOptionalValue, - pub(super) log_file_mode: ResolvedValue, + pub(super) log_dir: ResolvedOptionalValue, pub(super) timeout_ms: ResolvedValue, pub(super) attribution_hooks_enabled: ResolvedValue, pub(super) workos_client_id: ResolvedOptionalValue, @@ -191,8 +189,7 @@ where Ok(ResolvedObservabilityRuntimeConfig { log_level: runtime.log_level.value, log_format: runtime.log_format.value, - log_file: runtime.log_file.value, - log_file_mode: runtime.log_file_mode.value, + log_dir: runtime.log_dir.value, loaded_config_paths: runtime.loaded_config_paths, validation_errors: runtime.validation_errors, }) @@ -268,8 +265,7 @@ where let mut file_config = schema::FileConfig { log_level: None, log_format: None, - log_file: None, - log_file_mode: None, + log_dir: None, timeout_ms: None, attribution_hooks_enabled: None, workos_client_id: None, @@ -295,11 +291,8 @@ where if let Some(log_format) = layer.log_format { file_config.log_format = Some(log_format); } - if let Some(log_file) = layer.log_file { - file_config.log_file = Some(log_file); - } - if let Some(log_file_mode) = layer.log_file_mode { - file_config.log_file_mode = Some(log_file_mode); + if let Some(log_dir) = layer.log_dir { + file_config.log_dir = Some(log_dir); } if let Some(timeout_ms) = layer.timeout_ms { file_config.timeout_ms = Some(timeout_ms); @@ -364,45 +357,23 @@ where }; } - let mut resolved_log_file = ResolvedOptionalValue { + let mut resolved_log_dir = ResolvedOptionalValue { value: file_config - .log_file + .log_dir .as_ref() .map(|value| value.value.clone()), source: file_config - .log_file + .log_dir .as_ref() .map(|value| ValueSource::ConfigFile(value.source)), }; - if let Some(raw) = env_lookup(ENV_LOG_FILE) { - resolved_log_file = ResolvedOptionalValue { + if let Some(raw) = env_lookup(ENV_LOG_DIR) { + resolved_log_dir = ResolvedOptionalValue { value: Some(raw), source: Some(ValueSource::Env), }; } - let mut resolved_log_file_mode = ResolvedValue { - value: LogFileMode::Truncate, - source: ValueSource::Default, - }; - if let Some(value) = file_config.log_file_mode { - resolved_log_file_mode = ResolvedValue { - value: value.value, - source: ValueSource::ConfigFile(value.source), - }; - } - if let Some(raw) = env_lookup(ENV_LOG_FILE_MODE) { - resolved_log_file_mode = ResolvedValue { - value: LogFileMode::parse(&raw, ENV_LOG_FILE_MODE)?, - source: ValueSource::Env, - }; - } - if resolved_log_file.value.is_none() && resolved_log_file_mode.source != ValueSource::Default { - bail!( - "{ENV_LOG_FILE_MODE} requires {ENV_LOG_FILE}. Try: set {ENV_LOG_FILE} to a file path or unset {ENV_LOG_FILE_MODE}." - ); - } - let mut resolved_timeout_ms = ResolvedValue { value: DEFAULT_TIMEOUT_MS, source: ValueSource::Default, @@ -468,8 +439,7 @@ where loaded_config_paths, log_level: resolved_log_level, log_format: resolved_log_format, - log_file: resolved_log_file, - log_file_mode: resolved_log_file_mode, + log_dir: resolved_log_dir, timeout_ms: resolved_timeout_ms, attribution_hooks_enabled: resolved_attribution_hooks_enabled, workos_client_id: resolved_workos_client_id, diff --git a/cli/src/services/config/schema.rs b/cli/src/services/config/schema.rs index c5944b8d..83992e34 100644 --- a/cli/src/services/config/schema.rs +++ b/cli/src/services/config/schema.rs @@ -18,8 +18,8 @@ use serde_json::Value; use super::policy::{parse_bash_policy_presets, parse_custom_bash_policies, CustomBashPolicyEntry}; use super::types::{ - ConfigPathSource, DatabaseRetryConfig, IntegrationTargetId, IntegrationsConfig, LogFileMode, - LogFormat, LogLevel, + ConfigPathSource, DatabaseRetryConfig, IntegrationTargetId, IntegrationsConfig, LogFormat, + LogLevel, }; use crate::services::resilience::RetryPolicy; @@ -32,8 +32,7 @@ pub(crate) const TOP_LEVEL_CONFIG_KEYS: &[&str] = &[ CONFIG_SCHEMA_DECLARATION_KEY, "log_level", "log_format", - "log_file", - "log_file_mode", + "log_dir", "timeout_ms", super::resolver::WORKOS_CLIENT_ID_KEY.config_key, "policies", @@ -41,7 +40,7 @@ pub(crate) const TOP_LEVEL_CONFIG_KEYS: &[&str] = &[ ]; pub(crate) const TOP_LEVEL_CONFIG_KEYS_DESCRIPTION: &str = - "$schema, log_level, log_format, log_file, log_file_mode, timeout_ms, workos_client_id, policies, integrations"; + "$schema, log_level, log_format, log_dir, timeout_ms, workos_client_id, policies, integrations"; static CONFIG_SCHEMA_VALIDATOR: OnceLock = OnceLock::new(); @@ -67,8 +66,7 @@ pub(crate) struct ParsedFileConfigDocument { pub(crate) _schema: Option, pub(crate) log_level: Option, pub(crate) log_format: Option, - pub(crate) log_file: Option, - pub(crate) log_file_mode: Option, + pub(crate) log_dir: Option, pub(crate) timeout_ms: Option, pub(crate) workos_client_id: Option, pub(crate) policies: Option, @@ -143,8 +141,7 @@ pub(crate) struct FileConfigValue { pub(crate) struct FileConfig { pub(crate) log_level: Option>, pub(crate) log_format: Option>, - pub(crate) log_file: Option>, - pub(crate) log_file_mode: Option>, + pub(crate) log_dir: Option>, pub(crate) timeout_ms: Option>, pub(crate) attribution_hooks_enabled: Option>, pub(crate) workos_client_id: Option>, @@ -276,18 +273,7 @@ pub(crate) fn parse_file_config( }) }) .transpose()?; - let log_file = typed - .log_file - .map(|value| FileConfigValue { value, source }); - let log_file_mode = typed - .log_file_mode - .map(|raw| -> Result> { - Ok(FileConfigValue { - value: LogFileMode::parse(&raw, &format!("config file '{}'", path.display()))?, - source, - }) - }) - .transpose()?; + let log_dir = typed.log_dir.map(|value| FileConfigValue { value, source }); let timeout_ms = typed .timeout_ms .map(|value| FileConfigValue { value, source }); @@ -301,8 +287,7 @@ pub(crate) fn parse_file_config( Ok(FileConfig { log_level, log_format, - log_file, - log_file_mode, + log_dir, timeout_ms, attribution_hooks_enabled, workos_client_id, diff --git a/cli/src/services/config/types.rs b/cli/src/services/config/types.rs index be3bdc55..79dea06f 100644 --- a/cli/src/services/config/types.rs +++ b/cli/src/services/config/types.rs @@ -15,8 +15,7 @@ pub const NAME: &str = "config"; pub(crate) const ENV_LOG_LEVEL: &str = "SCE_LOG_LEVEL"; pub(crate) const ENV_LOG_FORMAT: &str = "SCE_LOG_FORMAT"; -pub(crate) const ENV_LOG_FILE: &str = "SCE_LOG_FILE"; -pub(crate) const ENV_LOG_FILE_MODE: &str = "SCE_LOG_FILE_MODE"; +pub(crate) const ENV_LOG_DIR: &str = "SCE_LOG_DIR"; pub(crate) const ENV_ATTRIBUTION_HOOKS_DISABLED: &str = "SCE_ATTRIBUTION_HOOKS_DISABLED"; pub type ReportFormat = OutputFormat; @@ -104,39 +103,6 @@ impl LogFormat { } } -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub enum LogFileMode { - Truncate, - Append, -} - -impl LogFileMode { - pub(crate) fn parse(raw: &str, source: &str) -> anyhow::Result { - match raw { - "truncate" => Ok(Self::Truncate), - "append" => Ok(Self::Append), - _ => anyhow::bail!( - "Invalid log file mode '{raw}' from {source}. Valid values: truncate, append." - ), - } - } - - pub(crate) fn parse_env(raw: &str, key: &str) -> anyhow::Result { - match raw { - "truncate" => Ok(Self::Truncate), - "append" => Ok(Self::Append), - _ => anyhow::bail!("Invalid {key} '{raw}'. Valid values: truncate, append."), - } - } - - pub(crate) fn as_str(self) -> &'static str { - match self { - Self::Truncate => "truncate", - Self::Append => "append", - } - } -} - #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub(crate) enum ValueSource { Flag, @@ -230,8 +196,7 @@ pub(crate) struct ResolvedAuthRuntimeConfig { pub(crate) struct ResolvedObservabilityRuntimeConfig { pub(crate) log_level: LogLevel, pub(crate) log_format: LogFormat, - pub(crate) log_file: Option, - pub(crate) log_file_mode: LogFileMode, + pub(crate) log_dir: Option, pub(crate) loaded_config_paths: Vec, pub(crate) validation_errors: Vec, } diff --git a/cli/src/services/default_paths.rs b/cli/src/services/default_paths.rs index d7c03ac3..375c378c 100644 --- a/cli/src/services/default_paths.rs +++ b/cli/src/services/default_paths.rs @@ -343,7 +343,6 @@ pub(crate) mod repo_dir { #[allow(dead_code)] pub(crate) mod repo_file { pub const SCE_CONFIG: &str = "config.json"; - pub const SCE_LOG: &str = "sce.log"; pub const OPENCODE_MANIFEST: &str = "opencode.json"; pub const GIT_COMMIT_EDITMSG: &str = "COMMIT_EDITMSG"; } @@ -435,10 +434,6 @@ impl RepoPaths { self.sce_dir().join(repo_file::SCE_CONFIG) } - pub(crate) fn sce_log_file(&self) -> PathBuf { - self.sce_dir().join(repo_file::SCE_LOG) - } - pub(crate) fn opencode_dir(&self) -> PathBuf { self.root.join(repo_dir::OPENCODE) } diff --git a/cli/src/services/hooks/mod.rs b/cli/src/services/hooks/mod.rs index 3e9700eb..5ea24535 100644 --- a/cli/src/services/hooks/mod.rs +++ b/cli/src/services/hooks/mod.rs @@ -235,6 +235,7 @@ fn log_conversation_trace_fail_open(error: &anyhow::Error, logger: Option<&dyn L "sce.hooks.conversation_trace.error", &error.to_string(), &[], + None, ); } @@ -401,6 +402,7 @@ fn log_skipped_conversation_trace_payloads( ("event_type", event_type), ("payload_index", index.as_str()), ], + None, ); } } @@ -417,6 +419,7 @@ fn log_conversation_trace_batch_insert_failure( "sce.hooks.conversation_trace.agent_trace_db_batch_failed", &error.to_string(), &[("event_type", event_type), ("valid_count", count.as_str())], + None, ); } } @@ -705,7 +708,7 @@ fn run_diff_trace_subcommand_from_payload( fn log_diff_trace_fail_open(error: &anyhow::Error, logger: Option<&dyn Logger>) -> String { if let Some(log) = logger { - log.error("sce.hooks.diff_trace.error", &error.to_string(), &[]); + log.error("sce.hooks.diff_trace.error", &error.to_string(), &[], None); } String::from("diff-trace hook intake failed open; error logged.") @@ -722,6 +725,7 @@ fn run_diff_trace_subcommand_from_payload_with( "sce.hooks.diff_trace.agent_trace_db_time_invalid", &error.to_string(), &[], + None, ); } } @@ -739,6 +743,7 @@ fn run_diff_trace_subcommand_from_payload_with( "sce.hooks.diff_trace.agent_trace_db_write_failed", &error.to_string(), &[], + None, ); } false @@ -1410,6 +1415,7 @@ fn staged_diff_has_ai_overlap( "sce.hooks.commit_msg.ai_overlap_error", &format!("Staged AI-overlap evidence check failed: {error}."), &[], + None, ); } return StagedDiffAiOverlapResult::Error; @@ -1429,6 +1435,7 @@ fn staged_diff_has_ai_overlap( "sce.hooks.commit_msg.ai_overlap_error", "Staged AI-overlap evidence check failed: error during staged-diff or trace query.", &[], + None, ); } } diff --git a/cli/src/services/observability.rs b/cli/src/services/observability.rs index 73f6c95b..40c466d1 100644 --- a/cli/src/services/observability.rs +++ b/cli/src/services/observability.rs @@ -1,29 +1,29 @@ -use std::fs::{File, OpenOptions}; -use std::io::Write; -#[cfg(unix)] -use std::os::unix::fs::{MetadataExt, OpenOptionsExt, PermissionsExt}; -#[cfg(unix)] -use std::path::Path; -use std::path::PathBuf; -use std::sync::{Arc, Mutex}; +use std::{ + collections::BTreeMap, + fmt::Write as FmtWrite, + fs::{self, OpenOptions}, + io::{self, Write}, + path::{Path, PathBuf}, + sync::{Arc, Mutex, OnceLock}, +}; -use anyhow::{anyhow, bail, Result}; -use chrono::Utc; +use anyhow::{bail, Context, Result}; +use chrono::{Local, NaiveDate, Utc}; use serde_json::json; use tracing::Level; use crate::services::config::{ - self, LogFileMode, LogFormat, LogLevel, ENV_LOG_FILE, ENV_LOG_FILE_MODE, ENV_LOG_FORMAT, - ENV_LOG_LEVEL, + self, LogFormat, LogLevel, ENV_LOG_DIR, ENV_LOG_FORMAT, ENV_LOG_LEVEL, }; -use crate::services::default_paths::{repo_dir, repo_file}; use crate::services::error::ClassifiedError; use crate::services::security::redact_sensitive_text; -use crate::services::style::{error_text, heading}; pub mod traits; pub const NAME: &str = "observability"; +const LOG_FILE_PREFIX: &str = "sce"; +const LOG_FILE_EXTENSION: &str = "log"; +const EMPTY_SESSION_ID_TOKEN: &str = "%EMPTY"; #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub struct ObservabilityConfig { @@ -43,105 +43,23 @@ impl Default for ObservabilityConfig { #[derive(Clone, Debug)] pub struct Logger { config: ObservabilityConfig, - file_sink: Option, -} - -#[derive(Clone, Debug)] -struct LogFileSink { - path: PathBuf, - writer: Arc>, -} - -impl LogFileSink { - fn open(path: PathBuf, mode: LogFileMode) -> Result { - if path.as_os_str().is_empty() { - bail!( - "Invalid {ENV_LOG_FILE} ''. Try: set it to an absolute or relative file path, for example {}.", - default_repo_log_file_example() - ); - } - - if let Some(parent) = path.parent() { - if !parent.as_os_str().is_empty() { - std::fs::create_dir_all(parent).map_err(|error| { - anyhow!( - "Failed to prepare log directory '{}': {}", - parent.display(), - error - ) - })?; - } - } - - let mut options = OpenOptions::new(); - options.create(true).write(true); - match mode { - LogFileMode::Truncate => { - options.truncate(true); - } - LogFileMode::Append => { - options.append(true); - } - } - - #[cfg(unix)] - { - options.mode(0o600); - } - - let file = options.open(&path).map_err(|error| { - anyhow!( - "Failed to open {} '{}': {}. Try: verify the path is writable or unset {}.", - ENV_LOG_FILE, - path.display(), - error, - ENV_LOG_FILE - ) - })?; - - #[cfg(unix)] - enforce_unix_log_file_permissions(&path)?; - - Ok(Self { - path, - writer: Arc::new(Mutex::new(file)), - }) - } - - fn write_line(&self, line: &str) -> Result<()> { - let mut writer = self - .writer - .lock() - .map_err(|_| anyhow!("Log file writer lock poisoned"))?; - writer.write_all(line.as_bytes())?; - writer.write_all(b"\n")?; - writer.flush()?; - Ok(()) - } -} - -fn default_repo_log_file_example() -> String { - format!("{}/{}", repo_dir::SCE, repo_file::SCE_LOG) + log_dir: Option, } impl Logger { pub fn from_resolved_config( config: &config::ResolvedObservabilityRuntimeConfig, ) -> Result { - let file_sink = match config.log_file.as_deref() { - Some(path) => Some(LogFileSink::open( - PathBuf::from(path), - config.log_file_mode, - )?), - None => None, - }; + if let Some(log_dir) = config.log_dir.as_deref() { + validate_log_dir(log_dir)?; + } Ok(Self { config: ObservabilityConfig { level: config.log_level, format: config.log_format, }, - file_sink, + log_dir: config.log_dir.as_deref().map(PathBuf::from), }) } @@ -151,9 +69,6 @@ impl Logger { F: Fn(&str) -> Option, { let mut config = ObservabilityConfig::default(); - let mut file_path = None; - let mut file_mode_raw_seen = false; - let mut file_mode = LogFileMode::Truncate; if let Some(raw) = lookup(ENV_LOG_LEVEL) { config.level = LogLevel::parse_env(&raw, ENV_LOG_LEVEL)?; @@ -163,47 +78,57 @@ impl Logger { config.format = LogFormat::parse_env(&raw, ENV_LOG_FORMAT)?; } - if let Some(raw) = lookup(ENV_LOG_FILE) { - file_path = Some(PathBuf::from(raw)); - } - - if let Some(raw) = lookup(ENV_LOG_FILE_MODE) { - file_mode_raw_seen = true; - file_mode = LogFileMode::parse_env(&raw, ENV_LOG_FILE_MODE)?; + let mut log_dir = None; + if let Some(raw) = lookup(ENV_LOG_DIR) { + validate_log_dir(&raw)?; + log_dir = Some(PathBuf::from(raw)); } - if file_path.is_none() && file_mode_raw_seen { - bail!( - "{ENV_LOG_FILE_MODE} requires {ENV_LOG_FILE}. Try: set {ENV_LOG_FILE} to a file path or unset {ENV_LOG_FILE_MODE}." - ); - } - - let file_sink = match file_path { - Some(path) => Some(LogFileSink::open(path, file_mode)?), - None => None, - }; - - Ok(Self { config, file_sink }) + Ok(Self { config, log_dir }) } - pub fn info(&self, event_id: &str, message: &str, fields: &[(&str, &str)]) { - self.log(LogLevel::Info, event_id, message, fields); + pub fn info( + &self, + event_id: &str, + message: &str, + fields: &[(&str, &str)], + session_id: Option<&str>, + ) { + self.log(LogLevel::Info, event_id, message, fields, session_id); } - pub fn debug(&self, event_id: &str, message: &str, fields: &[(&str, &str)]) { - self.log(LogLevel::Debug, event_id, message, fields); + pub fn debug( + &self, + event_id: &str, + message: &str, + fields: &[(&str, &str)], + session_id: Option<&str>, + ) { + self.log(LogLevel::Debug, event_id, message, fields, session_id); } - pub fn warn(&self, event_id: &str, message: &str, fields: &[(&str, &str)]) { - self.log_forced(LogLevel::Warn, event_id, message, fields); + pub fn warn( + &self, + event_id: &str, + message: &str, + fields: &[(&str, &str)], + session_id: Option<&str>, + ) { + self.log_forced(LogLevel::Warn, event_id, message, fields, session_id); } #[cfg_attr(not(test), allow(dead_code))] - pub fn error(&self, event_id: &str, message: &str, fields: &[(&str, &str)]) { - self.log(LogLevel::Error, event_id, message, fields); + pub fn error( + &self, + event_id: &str, + message: &str, + fields: &[(&str, &str)], + session_id: Option<&str>, + ) { + self.log(LogLevel::Error, event_id, message, fields, session_id); } - pub fn log_classified_error(&self, error: &ClassifiedError) { + pub fn log_classified_error(&self, error: &ClassifiedError, session_id: Option<&str>) { let event_id = format!("sce.error.{}", error.code()); self.log( LogLevel::Error, @@ -213,37 +138,56 @@ impl Logger { ("error_code", error.code()), ("error_class", error.class().as_str()), ], + session_id, ); } - fn log(&self, level: LogLevel, event_id: &str, message: &str, fields: &[(&str, &str)]) { + fn log( + &self, + level: LogLevel, + event_id: &str, + message: &str, + fields: &[(&str, &str)], + session_id: Option<&str>, + ) { if !self.enabled(level) { return; } - self.log_forced(level, event_id, message, fields); + self.log_forced(level, event_id, message, fields, session_id); } - fn log_forced(&self, level: LogLevel, event_id: &str, message: &str, fields: &[(&str, &str)]) { + fn log_forced( + &self, + level: LogLevel, + event_id: &str, + message: &str, + fields: &[(&str, &str)], + session_id: Option<&str>, + ) { emit_tracing_event(level, event_id, message, fields); let line = self.render_line(level, event_id, message, fields); let redacted_line = redact_sensitive_text(&line); - eprintln!("{redacted_line}"); - - if let Some(file_sink) = &self.file_sink { - if let Err(error) = file_sink.write_line(&redacted_line) { - let diagnostic = redact_sensitive_text(&format!( - "Failed to write log file '{}': {}. Try: verify the file is writable or unset {}.", - file_sink.path.display(), - error, - ENV_LOG_FILE - )); - eprintln!("{}: {}", heading("Error"), error_text(&diagnostic)); - } + emit_stderr_line(&redacted_line); + + if let Err(error) = self.write_log_line(&redacted_line, session_id) { + let diagnostic = redact_sensitive_text(&format!( + "Failed to write SCE log file: {error}. Logging continues on stderr." + )); + emit_stderr_line(&diagnostic); } } + fn write_log_line(&self, redacted_line: &str, session_id: Option<&str>) -> Result<()> { + let Some(log_dir) = self.log_dir.as_deref() else { + return Ok(()); + }; + + let path = current_log_path(log_dir, session_id); + append_log_line(&path, redacted_line) + } + fn enabled(&self, level: LogLevel) -> bool { level.severity() <= self.config.level.severity() } @@ -301,6 +245,104 @@ impl Logger { } } +fn validate_log_dir(value: &str) -> Result<()> { + if value.is_empty() { + bail!("Invalid {ENV_LOG_DIR} ''. Try: set it to a directory path or unset {ENV_LOG_DIR}."); + } + + Ok(()) +} + +fn current_log_path(log_dir: &Path, session_id: Option<&str>) -> PathBuf { + log_path_for_date(log_dir, Local::now().date_naive(), session_id) +} + +fn log_path_for_date(log_dir: &Path, date: NaiveDate, session_id: Option<&str>) -> PathBuf { + log_dir.join(log_name_for_date(date, session_id)) +} + +fn log_name_for_date(date: NaiveDate, session_id: Option<&str>) -> String { + let date = date.format("%d_%m_%Y"); + match session_id { + Some(session_id) => format!( + "{LOG_FILE_PREFIX}-{date}-{}.{LOG_FILE_EXTENSION}", + sanitize_session_id_for_filename(session_id) + ), + None => format!("{LOG_FILE_PREFIX}-{date}.{LOG_FILE_EXTENSION}"), + } +} + +fn sanitize_session_id_for_filename(session_id: &str) -> String { + if session_id.is_empty() { + return EMPTY_SESSION_ID_TOKEN.to_string(); + } + + let mut sanitized = String::with_capacity(session_id.len()); + for byte in session_id.as_bytes() { + match byte { + b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' => { + sanitized.push(char::from(*byte)); + } + _ => { + let _ = write!(&mut sanitized, "%{byte:02X}"); + } + } + } + sanitized +} + +fn append_log_line(path: &Path, redacted_line: &str) -> Result<()> { + if let Some(parent) = path.parent() { + fs::create_dir_all(parent) + .with_context(|| format!("failed to create log directory '{}'", parent.display()))?; + } + + let lock = file_log_lock(path); + let _guard = lock.lock().map_err(|error| { + anyhow::anyhow!("failed to lock log file '{}': {error}", path.display()) + })?; + + let mut options = OpenOptions::new(); + options.create(true).append(true); + configure_owner_only_file_permissions(&mut options); + let mut file = options + .open(path) + .with_context(|| format!("failed to open log file '{}' for append", path.display()))?; + writeln!(file, "{redacted_line}") + .with_context(|| format!("failed to append log line to '{}'", path.display()))?; + file.flush() + .with_context(|| format!("failed to flush log file '{}'", path.display())) +} + +fn file_log_lock(path: &Path) -> Arc> { + static FILE_LOG_LOCKS: OnceLock>>>> = OnceLock::new(); + let locks = FILE_LOG_LOCKS.get_or_init(|| Mutex::new(BTreeMap::new())); + let mut locks = locks + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + Arc::clone( + locks + .entry(path.to_path_buf()) + .or_insert_with(|| Arc::new(Mutex::new(()))), + ) +} + +#[cfg(unix)] +fn configure_owner_only_file_permissions(options: &mut OpenOptions) { + use std::os::unix::fs::OpenOptionsExt; + + options.mode(0o600); +} + +#[cfg(not(unix))] +fn configure_owner_only_file_permissions(_options: &mut OpenOptions) {} + +fn emit_stderr_line(line: &str) { + let mut stderr = io::stderr().lock(); + let _ = writeln!(stderr, "{line}"); + let _ = stderr.flush(); +} + fn emit_tracing_event(level: LogLevel, event_id: &str, message: &str, fields: &[(&str, &str)]) { emit_tracing_event_with_fields_json(level, event_id, message, || tracing_fields_json(fields)); } @@ -368,31 +410,3 @@ fn emit_tracing_event_with_fields_json( ), } } - -#[cfg(unix)] -fn enforce_unix_log_file_permissions(path: &Path) -> Result<()> { - let metadata = std::fs::metadata(path).map_err(|error| { - anyhow!( - "Failed to inspect permissions for log file '{}': {}", - path.display(), - error - ) - })?; - - let mode = metadata.mode() & 0o777; - if mode & 0o077 != 0 { - std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600)).map_err( - |error| { - anyhow!( - "Failed to secure permissions for {} '{}': {}. Try: run 'chmod 600 {}' and retry.", - ENV_LOG_FILE, - path.display(), - error, - path.display() - ) - }, - )?; - } - - Ok(()) -} diff --git a/cli/src/services/observability/traits.rs b/cli/src/services/observability/traits.rs index 18e7154c..d715f1f3 100644 --- a/cli/src/services/observability/traits.rs +++ b/cli/src/services/observability/traits.rs @@ -1,15 +1,39 @@ use crate::services::error::ClassifiedError; pub trait Logger: Send + Sync { - fn info(&self, event_id: &str, message: &str, fields: &[(&str, &str)]); + fn info( + &self, + event_id: &str, + message: &str, + fields: &[(&str, &str)], + session_id: Option<&str>, + ); - fn debug(&self, event_id: &str, message: &str, fields: &[(&str, &str)]); + fn debug( + &self, + event_id: &str, + message: &str, + fields: &[(&str, &str)], + session_id: Option<&str>, + ); - fn warn(&self, event_id: &str, message: &str, fields: &[(&str, &str)]); + fn warn( + &self, + event_id: &str, + message: &str, + fields: &[(&str, &str)], + session_id: Option<&str>, + ); - fn error(&self, event_id: &str, message: &str, fields: &[(&str, &str)]); + fn error( + &self, + event_id: &str, + message: &str, + fields: &[(&str, &str)], + session_id: Option<&str>, + ); - fn log_classified_error(&self, error: &ClassifiedError); + fn log_classified_error(&self, error: &ClassifiedError, session_id: Option<&str>); } pub trait Telemetry: Send + Sync { @@ -24,36 +48,88 @@ pub trait Telemetry: Send + Sync { pub struct NoopLogger; impl Logger for NoopLogger { - fn info(&self, _event_id: &str, _message: &str, _fields: &[(&str, &str)]) {} + fn info( + &self, + _event_id: &str, + _message: &str, + _fields: &[(&str, &str)], + _session_id: Option<&str>, + ) { + } - fn debug(&self, _event_id: &str, _message: &str, _fields: &[(&str, &str)]) {} + fn debug( + &self, + _event_id: &str, + _message: &str, + _fields: &[(&str, &str)], + _session_id: Option<&str>, + ) { + } - fn warn(&self, _event_id: &str, _message: &str, _fields: &[(&str, &str)]) {} + fn warn( + &self, + _event_id: &str, + _message: &str, + _fields: &[(&str, &str)], + _session_id: Option<&str>, + ) { + } - fn error(&self, _event_id: &str, _message: &str, _fields: &[(&str, &str)]) {} + fn error( + &self, + _event_id: &str, + _message: &str, + _fields: &[(&str, &str)], + _session_id: Option<&str>, + ) { + } - fn log_classified_error(&self, _error: &ClassifiedError) {} + fn log_classified_error(&self, _error: &ClassifiedError, _session_id: Option<&str>) {} } impl Logger for super::Logger { - fn info(&self, event_id: &str, message: &str, fields: &[(&str, &str)]) { - super::Logger::info(self, event_id, message, fields); + fn info( + &self, + event_id: &str, + message: &str, + fields: &[(&str, &str)], + session_id: Option<&str>, + ) { + super::Logger::info(self, event_id, message, fields, session_id); } - fn debug(&self, event_id: &str, message: &str, fields: &[(&str, &str)]) { - super::Logger::debug(self, event_id, message, fields); + fn debug( + &self, + event_id: &str, + message: &str, + fields: &[(&str, &str)], + session_id: Option<&str>, + ) { + super::Logger::debug(self, event_id, message, fields, session_id); } - fn warn(&self, event_id: &str, message: &str, fields: &[(&str, &str)]) { - super::Logger::warn(self, event_id, message, fields); + fn warn( + &self, + event_id: &str, + message: &str, + fields: &[(&str, &str)], + session_id: Option<&str>, + ) { + super::Logger::warn(self, event_id, message, fields, session_id); } - fn error(&self, event_id: &str, message: &str, fields: &[(&str, &str)]) { - super::Logger::error(self, event_id, message, fields); + fn error( + &self, + event_id: &str, + message: &str, + fields: &[(&str, &str)], + session_id: Option<&str>, + ) { + super::Logger::error(self, event_id, message, fields, session_id); } - fn log_classified_error(&self, error: &ClassifiedError) { - super::Logger::log_classified_error(self, error); + fn log_classified_error(&self, error: &ClassifiedError, session_id: Option<&str>) { + super::Logger::log_classified_error(self, error, session_id); } } diff --git a/cli/src/services/parse/command_runtime.rs b/cli/src/services/parse/command_runtime.rs index 6fe07f9c..1433cbd8 100644 --- a/cli/src/services/parse/command_runtime.rs +++ b/cli/src/services/parse/command_runtime.rs @@ -25,6 +25,7 @@ where "sce.command.raw_args", "Parsing command arguments", &[("args_summary", &args_summary)], + None, ); } diff --git a/config/pkl/base/sce-config-schema.pkl b/config/pkl/base/sce-config-schema.pkl index e8ec8440..4d179c65 100644 --- a/config/pkl/base/sce-config-schema.pkl +++ b/config/pkl/base/sce-config-schema.pkl @@ -70,14 +70,10 @@ local sceConfigSchema = new JsonSchema { type = "string" enum = new { "text"; "json" } } - ["log_file"] = new JsonSchema { + ["log_dir"] = new JsonSchema { type = "string" minLength = 1 } - ["log_file_mode"] = new JsonSchema { - type = "string" - enum = new { "truncate"; "append" } - } ["timeout_ms"] = new JsonSchema { type = "integer" minimum = 0 @@ -178,9 +174,6 @@ local sceConfigSchema = new JsonSchema { } } } - dependentRequired { - ["log_file_mode"] = new { "log_file" } - } } local jsonRenderer = new JsonRenderer { diff --git a/config/schema/sce-config.schema.json b/config/schema/sce-config.schema.json index dfd6eff1..45ccb6ee 100644 --- a/config/schema/sce-config.schema.json +++ b/config/schema/sce-config.schema.json @@ -25,17 +25,10 @@ "json" ] }, - "log_file": { + "log_dir": { "type": "string", "minLength": 1 }, - "log_file_mode": { - "type": "string", - "enum": [ - "truncate", - "append" - ] - }, "timeout_ms": { "type": "integer", "minimum": 0 @@ -358,10 +351,5 @@ "additionalProperties": false } }, - "additionalProperties": false, - "dependentRequired": { - "log_file_mode": [ - "log_file" - ] - } + "additionalProperties": false } diff --git a/context/architecture.md b/context/architecture.md index 6d7d2089..634cc3b6 100644 --- a/context/architecture.md +++ b/context/architecture.md @@ -106,14 +106,14 @@ The repository includes a new placeholder Rust binary crate at `cli/`. - `AppContext` is the CLI's borrowed dependency view in `cli/src/app.rs`: it is generic over logger, telemetry, filesystem, and git capability implementations and stores references plus an optional `repo_root: Option` instead of owning `Arc` trait objects. Because it borrows from `AppRuntime`, `AppContext` is a lightweight, short-lived view and must not be stored long-term (e.g., in structs or across await points). Startup creates a context view over `AppRuntime`'s concrete production dependencies with `repo_root` set to `None`; command paths can derive repo-root-scoped context views through the `ContextWithRepoRoot` accessor trait / `AppContext::with_repo_root(...)`, which reuses the same borrowed dependencies while attaching the resolved root. Narrow accessor traits expose associated concrete capability types for logger, telemetry, fs, and git (`&Self::...`) plus repo-root access, so call sites can express capability requirements without erasing the borrowed dependencies back to trait objects; lifecycle providers consume the repo-root accessor rather than the full context type. - Command parse-time conversion and run-time handling are separated by an internal static `RuntimeCommand` seam. `cli/src/services/command_registry.rs` defines the `RuntimeCommand` enum with variants for help/help-text, version, completion, auth, config, setup, doctor, hooks, policy, and trace, plus a deterministic `CommandRegistry` name catalog populated by `build_default_registry()`. `parse_command_phase` in `cli/src/app.rs` delegates clap-output conversion to `cli/src/services/parse/command_runtime.rs`, which owns clap error classification, help rendering bridges, and parsed-request-to-enum conversion while returning concrete enum values. Service-owned `command.rs` modules define command payload structs and generic execution methods with narrow context requirements: context-free commands accept any context, hooks requires logger access, setup/doctor require repo-root scoping, and central dispatch requires the union of logger plus repo-root-scoping capabilities. `services::app_support::execute_command_phase` emits lifecycle logs around `RuntimeCommand::execute(...)`; the enum performs the only central dispatch match and delegates business behavior to the service-owned command structs. - Startup observability bootstrapping in `cli/src/app.rs` still tolerates invalid default-discovered config files by continuing with degraded defaults plus `sce.config.invalid_config` warn-level logs, but the warning/logging work is now isolated behind the startup-context and runtime-initialization phases rather than one inline startup function. -- `cli/src/services/observability.rs` provides deterministic runtime observability controls and rendering for app lifecycle logs, including shared config-resolved threshold/format and file-sink inputs with precedence `env > config file > defaults` for non-flag observability keys, optional file sink controls (`SCE_LOG_FILE`, `SCE_LOG_FILE_MODE` with deterministic truncate-or-append policy), stable event identifiers, severity filtering, the forced-emission warning path used for invalid discovered config startup diagnostics, stderr-only primary emission with optional mirrored file writes, and redaction-safe emission through the shared security helper. Its `observability::traits` submodule exposes the current `Logger` and object-safe `Telemetry` trait boundaries plus `NoopLogger`; the concrete observability logger and telemetry runtime still own behavior and implement those traits without changing runtime behavior. `services::app_support::render_run_outcome` consumes the logger through that trait boundary when logging classified errors and stdout-write failures. +- `cli/src/services/observability.rs` provides deterministic runtime observability controls and rendering for app lifecycle logs, including shared config-resolved threshold/format and `log_dir` inputs with precedence `env > config file > defaults` for non-flag observability keys, stable event identifiers, severity filtering, the forced-emission warning path used for invalid discovered config startup diagnostics, stderr primary emission, redaction-safe emission through the shared security helper, and optional append-only log-directory writes. When `log_dir` is configured, each enabled or forced log operation selects `/sce-.log` or `/sce--.log` using the machine-local date and optional logger session context, with deterministic percent-encoding for unsafe session filename bytes. Its `observability::traits` submodule exposes the current `Logger` API with `Option<&str>` session context plus object-safe `Telemetry` trait boundaries and `NoopLogger`; the concrete observability logger and telemetry runtime still own behavior and implement those traits. `services::app_support::render_run_outcome` consumes the logger through that trait boundary when logging classified errors and stdout-write failures. - `cli/src/services/observability.rs` no longer owns duplicate log enums or parsing helpers; it consumes the canonical primitive seam from `cli/src/services/config/mod.rs` and stays focused on logger and telemetry runtime behavior. - `cli/src/cli_schema.rs` is now the canonical owner for top-level command metadata for the real clap-backed command set (`auth`, `config`, `setup`, `doctor`, `hooks`, `version`, `completion`), including the slim top-level help purpose text and per-command visibility on `sce`, `sce help`, and `sce --help`; `cli/src/command_surface.rs` remains the custom top-level help renderer and known-command classifier, adding the synthetic `help` row plus the ASCII banner while consuming that shared metadata instead of maintaining a parallel command catalog. - `cli/src/services/default_paths.rs` is the canonical production path catalog for the CLI: it resolves config/state/cache roots with platform-aware XDG or `dirs` fallbacks through an internal `roots` seam, exposes named default paths for current persisted artifacts and database files (global config, auth tokens, auth DB, local DB, legacy/global agent trace DB fallback, and per-checkout agent trace DB files), and owns canonical repo-relative, embedded-asset, install, hook, and context-path accessors so non-test production path definitions have one shared owner. Current production consumers such as config discovery, doctor reporting, setup/install flows, database adapters, checkout identity, and local hook runtime path resolution consume this shared catalog rather than defining owned path literals in their own modules. - `cli/src/services/agent_trace.rs` is the Rust CLI owner for the SCE web base URL (`SCE_WEB_BASE_URL`) and exposes helpers for SCE-owned URL construction: Agent Trace conversation lookup URLs, persisted Agent Trace trace URLs, Agent Trace session URLs, and setup-created config schema URLs. Production Rust code should consume those helpers instead of repeating `sce.crocoder.dev` literals. - `cli/src/services/config/mod.rs` is the config service facade and `sce config` orchestration surface (`show`, `validate`, `--help`), with bare `sce config` routed by `cli/src/app.rs` to the same help payload as `sce config --help`. Focused submodules own the implementation slices: `types.rs` owns shared config/runtime primitives, `schema.rs` owns generated schema embedding plus typed file parsing, `policy.rs` owns bash-policy semantic validation plus policy-specific formatting and runtime preset-catalog access for the Rust evaluator, `resolver.rs` owns deterministic config-file discovery, file-layer merging, explicit value precedence (`flags > env > config file > defaults` where flag-backed), shared auth-key resolution, observability-runtime resolution, attribution-hooks runtime gate resolution, database-retry config resolution and `DATABASE_RETRY_CONFIG` `OnceLock` initialization, default-discovered invalid-file degradation, and explicit-path fatal errors for `--config` / `SCE_CONFIG_FILE`, and private `render.rs` owns `sce config show` / `sce config validate` text and JSON output construction plus rendering-specific display-value helpers. The facade preserves existing `services::config` imports for startup/auth/hooks callers while delegating command execution to resolution plus rendering submodules. - `cli/src/services/output_format.rs` defines the canonical shared CLI output-format contract (`OutputFormat`) for supporting commands, with deterministic `text|json` parsing and command-scoped actionable invalid-value guidance. -- `cli/src/services/config/types.rs` is the canonical owner for the shared runtime/config primitive seam used by the CLI: `LogLevel`, `LogFormat`, `LogFileMode`, the observability env-key constants, and the shared bool parsing helpers used by both config resolution and observability bootstrap; `cli/src/services/config/mod.rs` re-exports those primitives through the facade. +- `cli/src/services/config/types.rs` is the canonical owner for the shared runtime/config primitive seam used by the CLI: `LogLevel`, `LogFormat`, `SCE_LOG_LEVEL`, `SCE_LOG_FORMAT`, `SCE_LOG_DIR`, and the shared bool parsing helpers used by both config resolution and observability bootstrap; `cli/src/services/config/mod.rs` re-exports those primitives through the facade. - `cli/src/services/capabilities.rs` defines the current broad CLI capability traits consumed by the borrowed, compile-time-typed `AppContext`: `FsOps` with `StdFsOps` for filesystem operations and `GitOps` with `ProcessGitOps` for git command execution plus repository-root/hooks-directory resolution. Existing service internals do not consume these traits directly yet; command execution uses narrow accessors and repo-root-scoped context derivation. - `cli/src/services/lifecycle.rs` defines the current compile-safe lifecycle seam. `ServiceLifecycle` has default no-op generic `diagnose`, `fix`, and `setup` methods over `C: HasRepoRoot`, with lifecycle-owned health, fix, and setup result types so the trait contract is not publicly anchored to doctor/setup module types or the full `AppContext` shape. The same module owns the static `LifecycleProvider` enum and shared `lifecycle_providers(include_hooks)` catalog/factory, returning providers in deterministic order (config → local_db → auth_db → agent_trace_db → hooks when requested); enum dispatch calls each concrete provider through generic context methods without boxed lifecycle-provider allocation or repo-root trait-object context erasure. Hooks exposes a `HooksLifecycle` provider in `cli/src/services/hooks/lifecycle.rs` for hook rollout diagnosis/fix/setup using lifecycle-owned health records plus the canonical required-hook installer. Config exposes a `ConfigLifecycle` provider in `cli/src/services/config/lifecycle.rs` for global/repo-local config validation and repo-local `.sce/config.json` bootstrap. local_db exposes a `LocalDbLifecycle` provider in `cli/src/services/local_db/lifecycle.rs` for canonical local DB path health, parent-directory readiness/bootstrap, and `LocalDb::new()` setup. auth_db exposes an `AuthDbLifecycle` provider in `cli/src/services/auth_db/lifecycle.rs` for canonical auth DB path health, parent-directory readiness/bootstrap, and `AuthDb::new()` setup. agent_trace_db exposes an `AgentTraceDbLifecycle` provider in `cli/src/services/agent_trace_db/lifecycle.rs` for setup-time checkout identity creation when a repo root is available, per-checkout Agent Trace DB path health/fix when an ID exists, and legacy global DB fallback outside checkout context. Doctor runtime aggregates the full provider catalog for `diagnose` and `fix` and adapts lifecycle records into doctor report/fix records at the orchestration boundary; setup command aggregates the shared catalog for `setup` with hooks included only when requested and adapts hook setup outcomes before rendering setup-owned messages. - Agent Trace lifecycle setup uses checkout identity to resolve the per-checkout DB path and initializes it with `AgentTraceDb::open_at(path)`; hook runtime keeps lazy per-checkout DB initialization/upgrade only as the fallback for setup-not-run or incomplete-schema cases. diff --git a/context/cli/config-precedence-contract.md b/context/cli/config-precedence-contract.md index 495797d5..40f48778 100644 --- a/context/cli/config-precedence-contract.md +++ b/context/cli/config-precedence-contract.md @@ -27,9 +27,9 @@ Repo-configured bash-tool policy values are config-file only in this task slice: Resolved observability values that currently have no CLI flag layer follow the same lower-precedence chain without a flag step: -1. environment values (`SCE_LOG_FORMAT`, `SCE_LOG_FILE`, `SCE_LOG_FILE_MODE`) -2. config file values (`log_format`, `log_file`, `log_file_mode`) -3. defaults where defined (`log_format=text`, `log_file_mode=truncate`); `log_file` remains unset when no env/config value is present +1. environment values (`SCE_LOG_FORMAT`, `SCE_LOG_DIR`) +2. config file values (`log_format`, `log_dir`) +3. defaults where defined (`log_format=text`); `log_dir` remains unset when no env/config value is present Supported auth-adjacent runtime keys can participate in one shared key-declared precedence path without defining CLI flags. Each key declares its config-file name, environment variable name, and whether a baked default is allowed. The shared resolver supports keys that allow a baked default and keys that intentionally omit one. The first implemented migrated key is `workos_client_id`, which resolves as: @@ -61,13 +61,11 @@ When a default-discovered global or repo-local config file exists but fails JSON - Startup/runtime config resolution now degrades gracefully only for default-discovered files: invalid discovered files are skipped and reported via collected `validation_errors`, while explicit `--config` / `SCE_CONFIG_FILE` targets still fail immediately on the same parse or validation errors. - Config file content must be valid JSON with a top-level object. -- Allowed keys: `$schema`, `log_level`, `log_format`, `log_file`, `log_file_mode`, `timeout_ms`, `workos_client_id`, `policies`, `integrations`. +- Allowed keys: `$schema`, `log_level`, `log_format`, `log_dir`, `timeout_ms`, `workos_client_id`, `policies`, `integrations`. - Unknown keys fail validation. - `log_level` must be one of `error|warn|info|debug`. - `log_format` must be `text` or `json` when present. -- `log_file` must be a non-empty string when present. -- `log_file_mode` must be `truncate` or `append` when present. -- `log_file_mode` requires `log_file`. +- `log_dir` must be a non-empty string when present. - `timeout_ms` must be an unsigned integer. - `workos_client_id` must be a string when present. @@ -94,13 +92,13 @@ When a default-discovered global or repo-local config file exists but fails JSON - `show` reports discovered config files as `config_paths` (JSON) / `Config files:` (text). - Resolved values in `show` continue to report `source`; when source is `config_file`, output also reports a deterministic `config_source` value (`flag`, `env`, `default_discovered_global`, `default_discovered_local`). - `show` includes migrated supported auth keys in `result.resolved`. -- `show` includes resolved observability values directly in `result.resolved`, preserving flat logging keys (`log_level`, `log_format`, `log_file`, `log_file_mode`). +- `show` includes resolved observability values directly in `result.resolved`, preserving flat logging keys (`log_level`, `log_format`, `log_dir`). - `validate` text output is limited to `SCE config validation`, `Validation issues`, and `Validation warnings` lines. - `validate` JSON output is limited to `result.command`, `result.valid`, `result.issues`, and `result.warnings`. - `show` includes resolved bash-tool policies under `result.resolved.policies.bash`. - Bash-policy output includes resolved preset IDs, expanded custom entries (`id`, `match.argv_prefix`, `message`), and config-file source metadata when present. - `show` text output renders `policies.bash` as a single deterministic line and reports `(unset)` when no policy config resolves. -- `show` text output renders observability values as deterministic per-key lines, reporting `(unset)` for `log_file` when no value resolves. +- `show` text output renders observability values as deterministic per-key lines, reporting `(unset)` for `log_dir` when no value resolves. - `show` and `validate` both include `warnings`; this list is empty for normal valid config and carries deterministic redundancy messaging for valid-but-overlapping preset combinations such as `forbid-git-all` plus `forbid-git-commit`. - `validate` reports skipped invalid discovered config files through `result.valid = false` plus `result.issues`, using the collected `validation_errors` verbatim in both text and JSON output rather than hard-failing before render. - `validate` reaches its normal renderer for invalid discovered config; invalid discovered config is reported as a validation result rather than causing a pre-render startup failure. diff --git a/context/context-map.md b/context/context-map.md index d81a35c5..669d45a3 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -16,10 +16,10 @@ Feature/domain context: - `context/cli/structured-patch-service.md` (Claude structured editor-hook derivation in `cli/src/services/structured_patch.rs`, including `Write` structured-update hunks, `Write` `tool_input.content` create fallback, `Edit` structured patches, deterministic skip reasons, `ParsedPatch` output semantics, and Rust golden fixture coverage) - `context/cli/styling-service.md` (CLI text-mode output styling with `owo-colors` and `comfy-table`, TTY/`NO_COLOR` policy, shared helper API for human-facing surfaces, and per-column right-to-left RGB gradient banner rendering) - `context/cli/trace-command.md` (`sce trace` command group: discovery of per-checkout `agent-trace-*.db` files under `/sce/` with mtime-desc + checkout-id tiebreak alias assignment and six-required-table readiness probing, implemented `sce trace db shell ` wiring that resolves aliases/checkout IDs and opens the embedded in-process SQL shell without external `turso`, including `.tables` table-name listing for visible/internal tables, implemented `sce trace db list` text + JSON rendering using `services::style::heading`, implemented `sce trace status` per-checkout rendering with `StatusError::{NotInGitRepo, NoCheckoutId, DbMissing}` mapped to validation-class exits and skipped-DB pass-through, implemented `sce trace status --all` aggregation across every discovered DB, and the completed removal of `sce doctor dbs` whose discovery scan/rendering moved into `services::trace`) -- `context/cli/config-precedence-contract.md` (implemented `sce config` show/validate command contract, deterministic `flags > env > config file > defaults` resolution order, focused `config/resolver.rs` ownership for config discovery/merge/runtime precedence plus default-discovered invalid-file degradation, focused `config/render.rs` ownership for `show`/`validate` text+JSON output construction, canonical `$schema` acceptance for startup-loaded `sce/config.json` files, shared auth-key env/config/optional baked-default support starting with `workos_client_id`, shared runtime resolution for flat logging observability keys, canonical Pkl-generated `sce/config.json` schema ownership plus CLI embedding/reuse contract including `policies.attribution_hooks.enabled` default-true/explicit-false opt-out metadata, config-file selection order, `show` provenance output, and trimmed `validate` output contract) +- `context/cli/config-precedence-contract.md` (implemented `sce config` show/validate command contract, deterministic `flags > env > config file > defaults` resolution order, focused `config/resolver.rs` ownership for config discovery/merge/runtime precedence plus default-discovered invalid-file degradation, focused `config/render.rs` ownership for `show`/`validate` text+JSON output construction, canonical `$schema` acceptance for startup-loaded `sce/config.json` files, shared auth-key env/config/optional baked-default support starting with `workos_client_id`, shared runtime resolution for flat logging observability keys including `log_dir` / `SCE_LOG_DIR`, canonical Pkl-generated `sce/config.json` schema ownership plus CLI embedding/reuse contract including `policies.attribution_hooks.enabled` default-true/explicit-false opt-out metadata, config-file selection order, `show` provenance output, and trimmed `validate` output contract) - `context/cli/capability-traits.md` (current broad CLI capability seam in `cli/src/services/capabilities.rs`, including `FsOps`/`StdFsOps`, `GitOps`/`ProcessGitOps`, git root/hooks resolution behavior, compile-time-typed borrowed AppContext wiring with associated-type narrow capability accessors plus `ContextWithRepoRoot` repo-root-scoped context derivation, generic command execution bounds, and test-only unimplemented stubs; current service internals do not consume fs/git traits until later lifecycle migration tasks) - `context/cli/service-lifecycle.md` (current compile-safe lifecycle seam in `cli/src/services/lifecycle.rs`, including default no-op `ServiceLifecycle` diagnose/fix/setup methods against narrow `HasRepoRoot`, lifecycle-owned health/fix/setup result types with generic setup messages, doctor/setup adapter boundaries, the static `LifecycleProvider` enum catalog/dispatcher, hook/config/local_db/auth_db/agent_trace_db lifecycle providers including setup-time checkout identity registration plus per-checkout Agent Trace DB initialization, implemented doctor aggregation over diagnose/fix providers, and implemented setup aggregation over `setup` providers in order config → local_db → auth_db → agent_trace_db → hooks when requested) -- `context/sce/cli-observability-contract.md` (implemented config-backed runtime observability contract for the flat logging config-file shape with env-over-config fallback, concrete logger/telemetry runtime behavior plus logger and object-safe telemetry trait boundaries, AppContext observability wiring, generic `RunOutcome` final rendering, runtime-classified repeated telemetry action protection, operator-facing `sce config show` observability reporting, and the trimmed `sce config validate` status-only validation surface) +- `context/sce/cli-observability-contract.md` (implemented config-backed runtime observability contract for the flat logging config-file shape with `log_dir` / `SCE_LOG_DIR` env-over-config fallback, append-only local-date/session log file routing, deterministic session filename sanitization, concrete logger/telemetry runtime behavior plus logger and object-safe telemetry trait boundaries, AppContext observability wiring, generic `RunOutcome` final rendering, runtime-classified repeated telemetry action protection, operator-facing `sce config show` observability reporting, and the trimmed `sce config validate` status-only validation surface) - `context/sce/shared-context-code-workflow.md` - `context/sce/shared-context-plan-workflow.md` (canonical `/change-to-plan` workflow, clarification/readiness gate contract, and one-task/one-atomic-commit task-slicing policy) - `context/sce/plan-code-overlap-map.md` (T01 overlap matrix for Shared Context Plan/Code, related commands, and core skill ownership/dedup targets) diff --git a/context/glossary.md b/context/glossary.md index 18b60e17..d894b724 100644 --- a/context/glossary.md +++ b/context/glossary.md @@ -76,14 +76,13 @@ - `sce exit-code class contract`: Stable top-level process exit-code mapping owned by `cli/src/app.rs` (`0` success, `2` parse failure, `3` validation failure, `4` runtime failure, `5` dependency failure) so automation can branch on failure class without parsing text errors. - `sce stderr error-code taxonomy`: Stable user-facing diagnostic code classes emitted by `cli/src/app.rs` (`SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, `SCE-ERR-DEPENDENCY`) via `Error []: ...` stderr formatting. - `class-default Try guidance injection`: `cli/src/app.rs` diagnostic behavior that appends `Try:` remediation text by failure class only when an error message does not already include `Try:` guidance. -- `sce observability baseline`: App-runtime logging contract in `cli/src/services/observability.rs` and `cli/src/app.rs` with config-resolved observability inputs, deterministic env-over-config-file-over-default precedence for non-flag logging keys, optional file sink controls (`SCE_LOG_FILE`, `SCE_LOG_FILE_MODE`), stable lifecycle `event_id` values, and stderr-only primary emission. +- `sce observability baseline`: App-runtime logging contract in `cli/src/services/observability.rs` and `cli/src/app.rs` with config-resolved observability inputs, deterministic env-over-config-file-over-default precedence for non-flag logging keys, optional append-only `log_dir` / `SCE_LOG_DIR` file routing by machine-local date plus optional session filename partitioning, stable lifecycle `event_id` values, and stderr primary emission. - `sce stdout/stderr contract`: App-level stream routing contract in `cli/src/app.rs` where command success payloads are emitted on stdout only, while redacted user-facing diagnostics are emitted on stderr. - `SCE_LOG_LEVEL`: Optional runtime env key for `sce` observability threshold; allowed values are `error`, `warn`, `info`, and `debug`, defaulting to `error` when unset. - `SCE_LOG_FORMAT`: Optional runtime env key for `sce` observability record format; allowed values are `text` and `json`, defaulting to `text` when unset. -- `SCE_LOG_FILE`: Optional runtime env key for `sce` observability file sink path; when set, rendered observability lines are mirrored to this file path with parent-directory auto-create behavior. -- `SCE_LOG_FILE_MODE`: Optional runtime env key controlling `SCE_LOG_FILE` write policy; allowed values are `truncate` and `append`, defaults to `truncate`, and requires `SCE_LOG_FILE`. +- `SCE_LOG_DIR`: Optional runtime env key for `sce` observability log-directory configuration; when set, it overrides config-file `log_dir` and must be non-empty. -- `logger trait boundary`: `services::observability::traits::Logger` mirrors the current observability logger API (`info`, `debug`, `warn`, `error`, `log_classified_error`) for generic command/runtime bounds and tests, with the concrete `services::observability::Logger` implementing it and `NoopLogger` available for side-effect-free tests. +- `logger trait boundary`: `services::observability::traits::Logger` mirrors the current observability logger API (`info`, `debug`, `warn`, `error`, `log_classified_error`) for generic command/runtime bounds and tests, with each method accepting `Option<&str>` session context for optional file routing; the concrete `services::observability::Logger` implements it and `NoopLogger` remains available for side-effect-free tests. - `telemetry trait boundary`: `services::observability::traits::Telemetry` mirrors the current telemetry subscriber API (`with_default_subscriber`) for generic app-runtime bounds and tests, with the concrete `services::observability::TelemetryRuntime` implementing it by delegating to the existing inherent method. - `app startup phases`: Current `cli/src/app.rs` execution model that separates dependency checking, startup-context construction, runtime initialization, command parse/execute, and output rendering into named helpers while preserving the CLI's existing exit-code, stderr-diagnostic, and degraded-startup behavior; output rendering and execution-phase logging helpers live in `cli/src/services/app_support.rs`. - `RunOutcome`: Generic final render payload in `cli/src/services/app_support.rs` (`RunOutcome`) carrying a command result, optional startup diagnostic, and optional logger implementing the logger trait boundary. Production construction in `cli/src/app.rs` uses the concrete observability logger, while rendering is not hardcoded to that production type. @@ -103,13 +102,13 @@ - `FsOps`: Filesystem capability trait in `cli/src/services/capabilities.rs` with `read_file`, `write_file`, `metadata`, and `exists`, implemented in production by `StdFsOps`. - `GitOps`: Git capability trait in `cli/src/services/capabilities.rs` with `run_command`, `resolve_repository_root`, `resolve_hooks_directory`, and `is_available`, implemented in production by `ProcessGitOps`. - `SCE default path policy seam`: Canonical path resolver in `cli/src/services/default_paths.rs` that owns config/state/cache root resolution through an internal `roots` helper seam, named default paths, and an explicit inventory for the current default persisted artifacts (`global config`, `auth tokens`); named DB paths include `auth DB`, `local DB`, and `Agent Trace DB`. On Linux those defaults resolve to `$XDG_CONFIG_HOME/sce/config.json`, `$XDG_STATE_HOME/sce/auth/tokens.json`, `$XDG_STATE_HOME/sce/auth.db`, `$XDG_STATE_HOME/sce/local.db`, and `$XDG_STATE_HOME/sce/agent-trace.db` with platform-equivalent `dirs` fallbacks elsewhere. The same module is also the canonical owner for broader production CLI path definitions, including repo-local `context/tmp/` scratch/session access, and is protected by a regression test that fails when new non-test production path literals are introduced outside `default_paths.rs`. -- `cli config precedence contract`: Deterministic runtime value resolution in `cli/src/services/config/resolver.rs` with precedence `flags > env > config file > defaults` for flag-backed keys (`log_level`, `timeout_ms`) plus shared app-runtime observability keys (`log_format`, `log_file`, `log_file_mode`) consumed by `cli/src/app.rs`; config discovery order is `--config`, `SCE_CONFIG_FILE`, then default discovered global+local paths (`${config_root}/sce/config.json` merged before `.sce/config.json`, with local overriding per key, where `config_root` comes from the shared default path policy seam and resolves to `$XDG_CONFIG_HOME` / `dirs::config_dir()` semantics with platform fallback behavior rather than the old state/data-root default). Runtime startup config loading permits the canonical top-level `"$schema": "https://sce.crocoder.dev/config.json"` declaration anywhere those config files are parsed (parsing delegated to `schema.rs`). -- `shared runtime/config primitives seam`: Canonical ownership in `cli/src/services/config/types.rs` for the CLI's shared observability/config enums (`LogLevel`, `LogFormat`, `LogFileMode`), request/response primitives (`ConfigSubcommand`, `ConfigRequest`, `ReportFormat`), source metadata types (`ValueSource`, `ConfigPathSource`, `LoadedConfigPath`, `ResolvedValue`, `ResolvedOptionalValue`), resolved runtime config types (`ResolvedAuthRuntimeConfig`, `ResolvedObservabilityRuntimeConfig`, `ResolvedHookRuntimeConfig`), the `NAME` constant, observability env-key constants, and shared bool parsing helpers; re-exported through `cli/src/services/config/mod.rs` via `pub use types::*` so downstream modules continue importing through `services::config` unchanged. +- `cli config precedence contract`: Deterministic runtime value resolution in `cli/src/services/config/resolver.rs` with precedence `flags > env > config file > defaults` for flag-backed keys (`log_level`, `timeout_ms`) plus shared app-runtime observability keys (`log_format`, `log_dir`) consumed by `cli/src/app.rs`; config discovery order is `--config`, `SCE_CONFIG_FILE`, then default discovered global+local paths (`${config_root}/sce/config.json` merged before `.sce/config.json`, with local overriding per key, where `config_root` comes from the shared default path policy seam and resolves to `$XDG_CONFIG_HOME` / `dirs::config_dir()` semantics with platform fallback behavior rather than the old state/data-root default). Runtime startup config loading permits the canonical top-level `"$schema": "https://sce.crocoder.dev/config.json"` declaration anywhere those config files are parsed (parsing delegated to `schema.rs`). +- `shared runtime/config primitives seam`: Canonical ownership in `cli/src/services/config/types.rs` for the CLI's shared observability/config enums (`LogLevel`, `LogFormat`), request/response primitives (`ConfigSubcommand`, `ConfigRequest`, `ReportFormat`), source metadata types (`ValueSource`, `ConfigPathSource`, `LoadedConfigPath`, `ResolvedValue`, `ResolvedOptionalValue`), resolved runtime config types (`ResolvedAuthRuntimeConfig`, `ResolvedObservabilityRuntimeConfig`, `ResolvedHookRuntimeConfig`), the `NAME` constant, observability env-key constants, and shared bool parsing helpers; re-exported through `cli/src/services/config/mod.rs` via `pub use types::*` so downstream modules continue importing through `services::config` unchanged. - `config schema and file parsing seam`: Canonical ownership in `cli/src/services/config/schema.rs` for the CLI's JSON Schema embedding (`SCE_CONFIG_SCHEMA_JSON`), `OnceLock` validator (`CONFIG_SCHEMA_VALIDATOR`, `config_schema_validator()`), top-level allowed-key validation (`TOP_LEVEL_CONFIG_KEYS`, `validate_object_keys`), serde DTO definitions (`ParsedFileConfigDocument`, `ParsedPoliciesConfigDocument`, `ParsedBashPolicyConfigDocument`, `ParsedAttributionHooksConfigDocument`, `ParsedCustomBashPolicyEntryDocument`, `ParsedCustomBashPolicyMatchDocument`), file config value wrapper (`FileConfigValue`) and aggregate (`FileConfig`), type aliases (`ParsedBashPolicyConfig`, `ParsedFilePolicies`), and config-file load/parse helpers (`validate_config_file`, `parse_file_config`, `deserialize_typed_config`, `map_policies_config`, `map_attribution_hooks_config`, `map_bash_policy_config`); `validate_config_file` is re-exported `pub(crate)` through `mod.rs` for `lifecycle.rs` and `doctor` consumers. Policy parsing helpers (`parse_bash_policy_presets`, `parse_custom_bash_policies`) and `CustomBashPolicyEntry` are imported from `super::policy` rather than the parent module. - `config policy semantic validation seam`: Canonical ownership in `cli/src/services/config/policy.rs` for the CLI's bash-policy and attribution-hooks semantic validation, merge helpers, and policy rendering: built-in/custom bash-policy catalog types and OnceLock (`BuiltinBashPolicyCatalog`, `BuiltinBashPolicyPreset`, `BuiltinBashPolicyMatcher`, `BuiltinBashPolicyRedundancyWarning`, `BUILTIN_BASH_POLICY_CATALOG`, `BASH_POLICY_PRESET_CATALOG_JSON`), policy config types (`BashPolicyConfig`, `CustomBashPolicyEntry`), catalog accessors (`builtin_bash_policy_catalog`, `builtin_bash_policy_preset_ids`, `is_builtin_bash_policy_preset_id`), policy parsing and validation (`parse_bash_policy_presets`, `parse_custom_bash_policies`, `parse_custom_bash_policy_entry`, `parse_custom_bash_policy_match`, `parse_custom_bash_policy_argv_prefix`), policy resolution (`resolve_bash_policy_config`, `build_validation_warnings`), and policy rendering (`format_bash_policies_text`, `format_bash_policies_json`); `mod.rs` imports `BashPolicyConfig`, `build_validation_warnings`, `format_bash_policies_json`, `format_bash_policies_text`, and `resolve_bash_policy_config` from `policy` for resolution and rendering consumers. - `config runtime resolver seam`: Canonical ownership in `cli/src/services/config/resolver.rs` for config-file discovery, file-layer merging, env/flag/default precedence resolution, shared auth-key resolution (`workos_client_id`), observability runtime resolution, attribution-hooks gate resolution, and default-discovered invalid-file degradation; `cli/src/services/config/mod.rs` delegates `sce config show|validate` runtime resolution to this seam while facade re-exports preserve startup/auth/hooks callers through `services::config`. - `config render seam`: Canonical ownership in `cli/src/services/config/render.rs` for `sce config show` and `sce config validate` text/JSON output construction, including rendering-specific config-path formatting, resolved-value formatting, validation issue/warning rendering, and auth display-value redaction/abbreviation helpers; `cli/src/services/config/mod.rs` delegates rendering to this private submodule after resolver-owned runtime config resolution. -- `sce config schema artifact`: Canonical JSON Schema for global and repo-local `sce/config.json` files, authored in `config/pkl/base/sce-config-schema.pkl`, generated to `config/schema/sce-config.schema.json`, and embedded by `cli/src/services/config/schema.rs` for shared `sce config validate` and doctor config validation. The current schema accepts the canonical `$schema` declaration, flat logging keys (`log_level`, `log_format`, `log_file`, `log_file_mode`), existing auth/config keys, and enforces the schema-level dependency that `log_file_mode` requires `log_file`. +- `sce config schema artifact`: Canonical JSON Schema for global and repo-local `sce/config.json` files, authored in `config/pkl/base/sce-config-schema.pkl`, generated to `config/schema/sce-config.schema.json`, and embedded by `cli/src/services/config/schema.rs` for shared `sce config validate` and doctor config validation. The current schema accepts the canonical `$schema` declaration, flat logging keys (`log_level`, `log_format`, `log_dir`), and existing auth/config keys. - `bash tool policy config surface`: Nested repo config namespace under `.sce/config.json` at `policies.bash`, currently supporting unique built-in `presets` plus repo-owned `custom` argv-prefix rules with deterministic validation, merged global/local resolution, and first-class `sce config show|validate` reporting. - `attribution hooks gate`: Enabled-by-default local hook runtime gate resolved through shared config precedence in `cli/src/services/config/mod.rs` (with parsing in `schema.rs`): opt-out env `SCE_ATTRIBUTION_HOOKS_DISABLED` overrides repo/global config key `policies.attribution_hooks.enabled` with inverted semantics, and the current enabled path activates commit-msg-only attribution gated by the staged-diff AI-overlap preflight. - `StagedDiffAiOverlapResult`: Three-valued enum in `cli/src/services/hooks/mod.rs` returned by the staged-diff AI-overlap evidence check: `Overlap` (staged diff overlaps with at least one recent AI/editor diff trace), `NoOverlap` (no overlap found; staged diff and recent traces were both available but share no touched lines, or staged patch has no touched lines), `Error` (DB open failure, schema not ready, query error, staged diff read failure, or clock failure). Both `NoOverlap` and `Error` map to `ai_contribution_present = false` at the commit-msg policy seam; `Error` additionally triggers `sce.hooks.commit_msg.ai_overlap_error` logging. diff --git a/context/overview.md b/context/overview.md index 2329cce5..ea33430c 100644 --- a/context/overview.md +++ b/context/overview.md @@ -9,7 +9,7 @@ It also includes a Rust CLI (`sce`) for Shared Context Engineering workflows: au - **Exit codes:** `2` parse, `3` validation, `4` runtime, `5` dependency failure (see `context/sce/cli-exit-code-contract.md`). - **Stderr diagnostics:** stable `SCE-ERR-{PARSE,VALIDATION,RUNTIME,DEPENDENCY}` codes with class-default `Try:` remediation (see `context/sce/cli-error-code-taxonomy.md`). - **Stdout/stderr:** command payloads on stdout only; redacted diagnostics on stderr (see `context/sce/cli-stdout-stderr-contract.md`). -- **Observability:** config-resolved logging to stderr, optional `SCE_LOG_FILE` mirroring (see `context/sce/cli-observability-contract.md`). +- **Observability:** config-resolved logging to stderr with optional `SCE_LOG_DIR` / `log_dir` resolution (see `context/sce/cli-observability-contract.md`). - **Config precedence:** `flags > env > config file > defaults` (see `context/cli/config-precedence-contract.md`). - **Attribution hooks:** enabled by default, gated by staged-diff AI-overlap preflight; `SCE_ATTRIBUTION_HOOKS_DISABLED` opt-out (see `context/sce/agent-trace-commit-msg-coauthor-policy.md`). - **Install channels:** repo-flake Nix, Cargo, npm, and source-built Flatpak (`dev.crocoder.sce`); Homebrew deferred (see `context/sce/cli-first-install-channels-contract.md`). @@ -18,7 +18,7 @@ The CLI crate currently depends on `anyhow`, `chrono`, `clap`, `clap_complete`, Its command loop is implemented with `clap` derive-based argument parsing and `anyhow` error handling. Top-level help now displays an ASCII art "SCE" banner with a per-column right-to-left color gradient (cyan→magenta when color is enabled, plain ASCII when disabled) above a slim command list without implemented/placeholder labels, and hides `auth` and `hooks` from `sce`, `sce help`, and `sce --help`, while those commands remain directly invocable. The real top-level command catalog/help-visibility contract is now centralized in `cli/src/cli_schema.rs` and consumed by `cli/src/command_surface.rs` for custom banner/help rendering plus known-command classification. The runtime includes implemented auth flows (`auth login|logout|status`) plus auth-local guidance for bare `sce auth` / `sce auth --help`, implemented config inspection/validation (`config show`/`config validate`) with bare `sce config` routing to the same help payload as `sce config --help`, real setup orchestration, implemented `doctor` diagnosis-vs-fix CLI surface and stable output-shape scaffolding (`sce doctor`, `sce doctor --fix`, `--format text|json`) plus current installed-CLI/global-state diagnostics for state-root resolution, global config validation, local DB and Agent Trace DB path + health, writable DB-parent-path checks, git availability/repository targeting, bare-repo refusal, effective hook-path source detection, an intentionally empty repo-scoped SCE database section for the active repository, required-hook presence/executable/content-drift checks against canonical embedded SCE-managed hook assets, repair-mode reuse of canonical setup hook installation for missing/stale/non-executable required hooks and missing hooks directories, and doctor-owned bootstrap repair for missing canonical DB parent directories, implemented attribution-only `hooks` subcommand routing/validation entrypoints with commit-msg-only behavior behind an enabled-by-default gate with explicit opt-out controls, implemented machine-readable runtime identification (`version`), implemented shell completion script generation via `clap_complete` (`completion --shell `), and placeholder dispatch for deferred commands (`sync`) through explicit service contracts. Parse-time command conversion plus run-time command handling now flow through an internal `RuntimeCommand` seam in `cli/src/app.rs`, so top-level app orchestration no longer owns one monolithic dispatch `match` for every command. The command loop now enforces a stable exit-code contract in `cli/src/app.rs`: `2` parse failures, `3` invocation validation failures, `4` runtime failures, and `5` dependency startup failures. The same runtime also emits stable user-facing stderr error classes (`SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, `SCE-ERR-DEPENDENCY`) using deterministic `Error []: ...` diagnostics with class-default `Try:` remediation appended when missing. -The app runtime now also includes a structured observability baseline in `cli/src/services/observability.rs`: deterministic env-controlled log threshold/format (`SCE_LOG_LEVEL` defaults to `error`; `SCE_LOG_FORMAT` defaults to `text`), optional file sink controls (`SCE_LOG_FILE`, `SCE_LOG_FILE_MODE` with deterministic `truncate` default), stable lifecycle event IDs, stderr-only primary emission so stdout command payloads remain pipe-safe, and `observability::traits` boundaries for logger and telemetry behavior. +The app runtime now also includes a structured observability baseline in `cli/src/services/observability.rs`: deterministic env-controlled log threshold/format (`SCE_LOG_LEVEL` defaults to `error`; `SCE_LOG_FORMAT` defaults to `text`), optional append-only log-directory routing (`SCE_LOG_DIR` / `log_dir`, unset by default) with per-operation machine-local dated file selection and optional session filename partitioning, stable lifecycle event IDs, stderr primary emission so stdout command payloads remain pipe-safe, and `observability::traits` boundaries for logger and telemetry behavior. The app command dispatcher now enforces a centralized stdout/stderr stream contract in `cli/src/app.rs`: command success payloads are emitted on stdout only, while redacted user-facing diagnostics are emitted on stderr. `cli/src/app.rs` also now runs through explicit startup phases — dependency check, observability config resolution, runtime initialization, command parse/execute, and output rendering — with the app runtime carrying logger/telemetry plus static command-catalog state across those phases while preserving the existing exit-code and degraded-startup contracts. Within that lifecycle, `parse_command_phase` delegates clap-to-runtime conversion to `cli/src/services/parse/command_runtime.rs`, which returns a static `RuntimeCommand` enum, `services::app_support::execute_command_phase` logs around enum-owned `execute(...)` dispatch, and generic `RunOutcome` rendering logs classified errors through the logger trait boundary without coupling render support to the production logger type. Command payload structs for `help`, `version`, `completion`, `auth`, `config`, `setup`, `doctor`, and `hooks` live in service-owned `command.rs` files; `cli/src/services/command_registry.rs` owns the deterministic static command-name catalog and enum variants instead of boxed command trait objects. The CLI now also enforces a shared output-format parser contract in `cli/src/services/output_format.rs`, with canonical `--format ` parsing and command-specific actionable invalid-value guidance reused by `config` and `version` services. A compile-safe service lifecycle seam also exists in `cli/src/services/lifecycle.rs`: `ServiceLifecycle` exposes default no-op `diagnose`, `fix`, and `setup` methods against the narrow `HasRepoRoot` accessor, uses lifecycle-owned health/fix/setup result types, and owns the shared static `LifecycleProvider` enum catalog/factory with deterministic config → local_db → auth_db → agent_trace_db → hooks ordering and no boxed provider aggregation. Hooks has a `services/hooks/lifecycle.rs` provider for hook rollout diagnosis/fix/setup, config has a `services/config/lifecycle.rs` provider for global/repo-local config validation plus repo-local config bootstrap, local_db has a `services/local_db/lifecycle.rs` provider for canonical local DB path health, parent-directory readiness/bootstrap, and `LocalDb::new()` setup, auth_db has a `services/auth_db/lifecycle.rs` provider for canonical auth DB path health, parent-directory readiness/bootstrap, and `AuthDb::new()` setup, and agent_trace_db has a `services/agent_trace_db/lifecycle.rs` provider for checkout identity setup, per-checkout Agent Trace DB path health/parent readiness when an ID exists, and legacy global fallback outside checkout context. Doctor runtime aggregates the full shared provider catalog for `diagnose` and `fix` and adapts lifecycle records into doctor-owned output records; setup command aggregates the shared provider catalog for `setup` with hooks included only when requested and adapts lifecycle setup outcomes before rendering setup-owned messages. Agent Trace lifecycle setup now also initializes the per-checkout Agent Trace DB via `AgentTraceDb::open_at(path)`; hook runtime lazy initialization remains a fallback when setup has not prepared the DB or schema metadata is incomplete. @@ -27,7 +27,7 @@ The `setup` command includes an `inquire`-backed target-selection flow: default The CLI now compiles an embedded setup asset manifest from `config/.opencode/**`, `config/.claude/**`, `config/.pi/**`, and `cli/assets/hooks/**` via `cli/build.rs`; `cli/src/services/setup/mod.rs` exposes deterministic normalized relative paths plus file bytes and target-scoped iteration without runtime reads from `config/`. The same build script also discovers `cli/migrations//*.sql` at compile time and writes `cli/src/generated_migrations.rs` constants sorted by numeric filename prefix for database migration consumers. The setup service also provides repository-root install orchestration: it resolves the repository root, derives a repo-root-scoped `AppContext` from the runtime command context, aggregates `ServiceLifecycle::setup` calls across lifecycle providers (config → local_db → auth_db → agent_trace_db → hooks when requested), handles interactive or flag-based target selection for config asset installation, and reports deterministic completion details (selected target(s) and installed file counts). Setup uses a unified remove-and-replace policy for all write flows — it removes existing targets before swapping staged content and returns deterministic recovery guidance (recover from version control) on swap failure, without creating backup artifacts. The setup command gates all modes on an existing git repository before any writes. Internally, `cli/src/services/setup/mod.rs` now separates install-flow logic from interactive prompt logic through focused support seams. The CLI now also applies baseline security hardening for reliability-driven automation: diagnostics/logging paths use deterministic secret redaction, `sce setup --hooks --repo ` canonicalizes and validates repository paths before execution, and setup write flows run explicit directory write-permission probes before staging/swap operations. -The config service now provides deterministic runtime config resolution with explicit precedence (`flags > env > config file > defaults`), strict config-file validation (`$schema`, `log_level`, `log_format`, `log_file`, `log_file_mode`, `timeout_ms`, `workos_client_id`, and nested `policies.bash`, `policies.attribution_hooks.enabled`, plus `policies.database_retry` with per-DB `connection_open`/`query` retry policy specs), deterministic default discovery/merge of global+local config files (`${config_root}/sce/config.json` then `.sce/config.json` with local override, where `config_root` comes from the shared default-path seam with XDG/`dirs::config_dir()` config-root resolution), defaults for the resolved observability value set (`log_level=error`, `log_format=text`, `log_file_mode=truncate`), shared auth-key resolution with optional baked defaults starting at `workos_client_id`, first-class bash-policy preset/custom parsing with deterministic conflict and duplicate-prefix validation, and a canonical Pkl-authored `sce/config.json` JSON Schema generated to `config/schema/sce-config.schema.json` and embedded by `cli/src/services/config/mod.rs` for both `sce config validate` and doctor-time config checks. Runtime startup config loading now keeps parity with that schema by accepting the canonical `"$schema": "https://sce.crocoder.dev/config.json"` declaration in repo-local and global config files, so startup commands such as `sce version` no longer fail before dispatch on that field. App-runtime observability now consumes flat logging keys through the shared resolver, so env values still override config-file values while config files provide deterministic fallback for file logging; `sce config show` reports resolved observability/auth/policy values with provenance, while `sce config validate` is now a trimmed validation surface that reports only pass/fail plus validation errors or warnings in text and JSON modes. The canonical preset catalog and matching contract live in `config/pkl/data/bash-policy-presets.json` and `context/sce/bash-tool-policy-enforcement-contract.md`. +The config service now provides deterministic runtime config resolution with explicit precedence (`flags > env > config file > defaults`), strict config-file validation (`$schema`, `log_level`, `log_format`, `log_dir`, `timeout_ms`, `workos_client_id`, and nested `policies.bash`, `policies.attribution_hooks.enabled`, plus `policies.database_retry` with per-DB `connection_open`/`query` retry policy specs), deterministic default discovery/merge of global+local config files (`${config_root}/sce/config.json` then `.sce/config.json` with local override, where `config_root` comes from the shared default-path seam with XDG/`dirs::config_dir()` config-root resolution), defaults for the resolved observability value set (`log_level=error`, `log_format=text`, `log_dir` unset), shared auth-key resolution with optional baked defaults starting at `workos_client_id`, first-class bash-policy preset/custom parsing with deterministic conflict and duplicate-prefix validation, and a canonical Pkl-authored `sce/config.json` JSON Schema generated to `config/schema/sce-config.schema.json` and embedded by `cli/src/services/config/mod.rs` for both `sce config validate` and doctor-time config checks. Runtime startup config loading now keeps parity with that schema by accepting the canonical `"$schema": "https://sce.crocoder.dev/config.json"` declaration in repo-local and global config files, so startup commands such as `sce version` no longer fail before dispatch on that field. App-runtime observability now consumes flat logging keys through the shared resolver, so env values still override config-file values while config files provide deterministic fallback for `log_dir`; `sce config show` reports resolved observability/auth/policy values with provenance, while `sce config validate` is now a trimmed validation surface that reports only pass/fail plus validation errors or warnings in text and JSON modes. The canonical preset catalog and matching contract live in `config/pkl/data/bash-policy-presets.json` and `context/sce/bash-tool-policy-enforcement-contract.md`. Invalid default-discovered config files now also degrade gracefully at startup: `sce` keeps running with degraded observability defaults, logs `sce.config.invalid_config` warnings, and reserves hard failures for explicit `--config` / `SCE_CONFIG_FILE` targets or other truly invalid runtime observability inputs. `cli/src/services/config/mod.rs` is now a module facade that declares focused config submodules (`types`, `schema`, `policy`, `resolver`, private `render`, `command`, and `lifecycle`), re-exporting `pub use types::*` and `pub(crate) use schema::validate_config_file`. Shared config primitive ownership is delegated to `cli/src/services/config/types.rs`; schema loading and file parsing to `cli/src/services/config/schema.rs`; bash-policy semantic validation and policy-specific formatting to `cli/src/services/config/policy.rs`; runtime discovery/precedence to `cli/src/services/config/resolver.rs`; and `sce config show` / `sce config validate` text+JSON output construction to `cli/src/services/config/render.rs`. Downstream modules continue importing through `services::config` unchanged. The CLI now has a generic borrowed `AppContext` dependency view in `cli/src/app.rs`; `AppRuntime` owns concrete production logger/telemetry/fs/git dependencies, and command execution receives context views that borrow those dependencies plus an optional `repo_root: Option`. `AppContext::with_repo_root(...)` / `ContextWithRepoRoot` derives repo-root-scoped views while preserving the borrowed runtime dependencies, and command execution is generic over associated-type narrow accessor traits where practical. The broad capability seam lives in `cli/src/services/capabilities.rs`, where `FsOps`/`StdFsOps` wrap filesystem operations and `GitOps`/`ProcessGitOps` wrap git process execution plus repository-root/hooks-directory resolution. The shared default path service in `cli/src/services/default_paths.rs` is now the canonical owner for production CLI path definitions. It resolves per-user config/state/cache roots through a dedicated internal `roots` seam, exposes the current persisted-artifact inventory (global config and auth tokens), and also defines named DB paths (auth DB, local DB, Agent Trace DB) plus the repo-relative, embedded-asset, install, hook, and context-path accessors consumed across current CLI production code. Non-test production modules should consume this shared catalog instead of hardcoding owned path literals. No default cache-backed persisted artifact currently exists, so cache-root resolution remains available without speculative cache-path features and no legacy default-path fallback is supported. diff --git a/context/patterns.md b/context/patterns.md index 085538ec..d54c47ff 100644 --- a/context/patterns.md +++ b/context/patterns.md @@ -103,7 +103,7 @@ - Emit user-facing CLI diagnostics with stable class-based error IDs (`SCE-ERR-PARSE`, `SCE-ERR-VALIDATION`, `SCE-ERR-RUNTIME`, `SCE-ERR-DEPENDENCY`) using deterministic `Error []: ...` stderr formatting, and auto-append class-default `Try:` remediation only when the message does not already provide one. - Keep CLI observability separate from command payloads: emit deterministic lifecycle logs to `stderr` only with stable `event_id` values, and preserve `stdout` for command result payloads. - For baseline runtime observability controls, resolve logging settings through the shared config resolver first, preserving deterministic precedence (`flags > env > config file > defaults`) and fail-fast validation on invalid env/config inputs. -- For optional observability file sinks, gate enablement behind explicit `SCE_LOG_FILE`, require `SCE_LOG_FILE_MODE` only when file sink is set, default write policy to deterministic `truncate`, and enforce owner-only file permissions (`0600`) on Unix. +- For optional observability log-directory configuration, gate enablement behind explicit `SCE_LOG_DIR` / `log_dir`; when configured, select append-only log files per emission from the machine-local date and optional logger session context, and keep session IDs out of rendered log schemas unless a caller explicitly passes them as normal fields. - For runtime CLI configuration, keep precedence deterministic and explicit (`flags > env > config file > defaults`) and expose inspect/validate command entrypoints with stable text/JSON outputs. - For commands that support text/JSON dual output, centralize `--format ` parsing in one shared contract and pass command-specific `--help` guidance into invalid-value errors instead of duplicating parser logic per command. diff --git a/context/sce/cli-observability-contract.md b/context/sce/cli-observability-contract.md index 7984a6ef..af951d83 100644 --- a/context/sce/cli-observability-contract.md +++ b/context/sce/cli-observability-contract.md @@ -3,7 +3,7 @@ ## Scope This document defines the implemented structured observability baseline for `sce` runtime execution. -It covers deterministic stderr logger controls, the current logger and telemetry trait boundaries, optional OpenTelemetry export bootstrap, config-backed runtime resolution, startup degradation behavior for invalid discovered config, and event emission boundaries in `cli/src/services/observability.rs`, `cli/src/services/config/mod.rs`, and `cli/src/app.rs`. +It covers deterministic stderr logger controls, optional append-only log-directory routing, the current logger and telemetry trait boundaries, config-backed runtime resolution, startup degradation behavior for invalid discovered config, and event emission boundaries in `cli/src/services/observability.rs`, `cli/src/services/config/mod.rs`, and `cli/src/app.rs`. Runtime observability now consumes the shared resolved observability config from `cli/src/services/config/mod.rs`: env values still win, config-file values act as fallback, and defaults apply when both are absent. When default-discovered config files are invalid JSON, fail schema validation, or are not top-level JSON objects, observability resolution now skips those files, collects the failure text in `validation_errors`, and continues with defaults; explicit `--config` / `SCE_CONFIG_FILE` selections remain fatal. Startup therefore keeps running with degraded observability defaults instead of turning discovered invalid config into a startup failure. Those resolved values are surfaced to operators through `sce config show`; `sce config validate` uses the same validation path but now reports only validation status plus any errors or warnings. @@ -11,24 +11,25 @@ Runtime observability now consumes the shared resolved observability config from - `SCE_LOG_LEVEL` selects log threshold with allowed values `error`, `warn`, `info`, `debug`. - `SCE_LOG_FORMAT` selects log format with allowed values `text`, `json`. -- `SCE_LOG_FILE` optionally enables a file log sink at the provided file path. -- `SCE_LOG_FILE_MODE` controls file-write policy with allowed values `truncate` and `append`. -- `SCE_LOG_FILE_MODE` requires `SCE_LOG_FILE`. +- `SCE_LOG_DIR` configures the optional log-directory value used by the logger configuration surface. - Defaults are deterministic: `log_level=error` and `log_format=text` when higher-precedence env/config inputs are unset. -- When file logging is enabled and `SCE_LOG_FILE_MODE` is unset, default policy is `truncate`. +- `log_dir` remains unset when no env/config value is present. - Invalid observability env values still fail invocation validation with actionable error text. - Invalid default-discovered observability config files no longer block runtime config resolution by themselves; they are skipped and resolution falls back to defaults. - After degraded observability config is constructed, startup emits one `warn`-level log per skipped discovered-file failure before command dispatch continues. ## Repository-local default in this repo - This repository now ships a repo-local config at `.sce/config.json`. -- The local config sets `log_level=debug`, `log_file=context/tmp/sce.log`, and `log_file_mode=append`. -- Running `sce` commands from this repository therefore mirrors lifecycle logs into `context/tmp/sce.log` unless higher-precedence flag or env inputs override those values. +- The local config sets `log_level=error` and `log_dir=context/tmp`. +- Running `sce` commands from this repository resolves `context/tmp` as the configured log directory unless `SCE_LOG_DIR` overrides it. ## Emission contract -- Log output is emitted to `stderr` only; command result payloads remain on `stdout`. -- When `SCE_LOG_FILE` is set, the same rendered log lines are also mirrored to the configured file sink. +- Log output is always emitted to `stderr`; command result payloads remain on `stdout`. +- When `log_dir` is configured, each enabled or forced log operation also appends the redacted rendered record to a file selected at emit time from the machine-local date and optional caller-provided session ID. +- Sessionless file logs route to `/sce-.log`; session-aware file logs route to `/sce--.log`. +- Session filename sanitization preserves ASCII letters, digits, `-`, and `_`; percent-encodes every other UTF-8 byte as uppercase `%HH`; and represents an explicitly empty `Some("")` session ID with the reserved `%EMPTY` token. +- File routing is append-only, creates the configured directory when needed, uses owner-only create permissions on Unix, serializes writes per path, and fails open by keeping stderr logging active when file creation/write/flush fails. - Each emitted record includes a stable `event_id`. - Current app-level event identifiers: - `sce.app.start` @@ -53,11 +54,11 @@ Runtime observability now consumes the shared resolved observability config from - Timestamps are UTC ISO8601 with millisecond precision (e.g., `2026-03-20T14:30:00.123Z`) generated via `chrono::Utc::now()`. - Logger threshold behavior is deterministic and severity-based (`error < warn < info < debug`). - Startup invalid-config diagnostics use an explicit warn-emission path so the warning is still rendered even when degraded defaults resolve to `log_level=error`. -- File sink writes are deterministic line-based writes with immediate flush after each record. +- Rendered records remain deterministic line-based strings on `stderr`; optional log-directory files contain the same redacted rendered lines and do not add session IDs to the record schema automatically. ## Observability trait boundaries -- `cli/src/services/observability/traits.rs` exposes the `services::observability::traits::Logger` trait with the current logging API: `info`, `debug`, `warn`, `error`, and `log_classified_error`. +- `cli/src/services/observability/traits.rs` exposes the `services::observability::traits::Logger` trait with the current logging API: `info`, `debug`, `warn`, `error`, and `log_classified_error`, each accepting `Option<&str>` session context used only for optional file routing. - The concrete `services::observability::Logger` implements the trait while retaining the existing inherent methods and behavior. - `NoopLogger` is available from the same traits module for tests and future dependency-injected services that need a logger without side effects. - The same traits module exposes object-safe `services::observability::traits::Telemetry` with the current app subscriber boundary: `with_default_subscriber` for command-lifecycle execution. @@ -66,16 +67,15 @@ Runtime observability now consumes the shared resolved observability config from - Final stream rendering uses `RunOutcome` in `cli/src/services/app_support.rs`, so classified-error and stdout-write-failure logging depends on the logger trait boundary rather than the concrete production logger type. - `run_command_lifecycle` expects the telemetry subscriber action to execute command dispatch at most once; if a telemetry implementation invokes the action again, the app returns a `SCE-ERR-RUNTIME` classified error rather than panicking or reparsing consumed arguments. -## File sink safety contract +## Log directory config safety contract -- On file-sink initialization, parent directories are created when missing. -- On Unix, log file permissions are tightened to owner-only (`0600`) when group/other bits are present. -- File open failures include actionable remediation guidance (verify writable path or unset `SCE_LOG_FILE`). -- File write failures are reported to `stderr` as diagnostics and do not alter command `stdout` payload contracts. +- `log_dir` config-file values are schema-validated as non-empty strings. +- `SCE_LOG_DIR` env values are resolved with env-over-config precedence and rejected when explicitly empty. +- Logger construction validates configured `log_dir` as non-empty without opening files; directory creation, per-operation local-date file selection, append-only writes, and Unix owner-only create permissions happen at log emission time. ## Ownership and verification - `cli/src/services/config/mod.rs` owns shared observability value resolution, config-file discovery/merge, and env-over-config precedence for runtime inputs. -- `cli/src/services/observability.rs` owns runtime logger construction from resolved values, level filtering, tracing-event enablement checks, record rendering, and optional file sink lifecycle/permission enforcement; `cli/src/services/observability/traits.rs` owns the logger and telemetry trait boundaries plus the no-op logger implementation. +- `cli/src/services/observability.rs` owns runtime logger construction from resolved values, `log_dir` non-empty validation, level filtering, tracing-event enablement checks, record rendering, local-date/session file-name selection, session filename sanitization, and append-only file writes; `cli/src/services/observability/traits.rs` owns the logger and telemetry trait boundaries plus the no-op logger implementation. - `cli/src/app.rs` owns lifecycle event emission around parse/dispatch success and failure paths, resolves observability config before command dispatch, emits startup invalid-config warning events for skipped discovered config files, wraps dispatch inside the observability subscriber context, and guards the single-use command-dispatch action against repeated telemetry invocation with a runtime-classified error. `cli/src/services/app_support.rs` owns final stdout/stderr rendering and generic logger-backed classified-error logging. - Contract behavior is exercised by app command tests and the root flake check suite. From 6c56e6081aabea2d9514b7a5a465fa7e64e75a32 Mon Sep 17 00:00:00 2001 From: stefanskoricdev Date: Tue, 21 Jul 2026 18:31:02 +0200 Subject: [PATCH 2/2] hooks: Route trace diagnostics by producer session Propagate producer-native session IDs through diff-trace and conversation-trace logging. Emit dedicated database-open errors without duplicate generic diagnostics while preserving fail-open behavior. Co-authored-by: SCE --- cli/src/services/hooks/mod.rs | 161 ++++++++++++++---- context/context-map.md | 4 +- context/patterns.md | 4 +- context/sce/agent-trace-db.md | 4 +- .../sce/agent-trace-hooks-command-routing.md | 9 +- context/sce/cli-observability-contract.md | 1 + 6 files changed, 143 insertions(+), 40 deletions(-) diff --git a/cli/src/services/hooks/mod.rs b/cli/src/services/hooks/mod.rs index 5ea24535..ff3614ab 100644 --- a/cli/src/services/hooks/mod.rs +++ b/cli/src/services/hooks/mod.rs @@ -138,6 +138,7 @@ pub struct ConversationTracePartBatch { pub struct SkippedConversationTracePayload { pub index: usize, pub reason: String, + pub session_id: Option, } #[derive(Clone, Debug, Eq, PartialEq)] @@ -211,12 +212,18 @@ fn run_conversation_trace_subcommand( ) -> String { let stdin_payload = match read_hook_stdin() { Ok(payload) => payload, - Err(error) => return log_conversation_trace_fail_open(&error, logger), + Err(error) => return log_conversation_trace_fail_open(&error, logger, None), }; + let session_id = conversation_trace_fail_open_session_id(&stdin_payload); - match run_conversation_trace_subcommand_from_payload(repository_root, &stdin_payload, logger) { + match run_conversation_trace_subcommand_from_payload( + repository_root, + &stdin_payload, + logger, + session_id.as_deref(), + ) { Ok(output) => output, - Err(error) => log_conversation_trace_fail_open(&error, logger), + Err(error) => log_conversation_trace_fail_open(&error, logger, session_id.as_deref()), } } @@ -224,18 +231,28 @@ fn run_conversation_trace_subcommand_from_payload( repository_root: &Path, stdin_payload: &str, logger: Option<&dyn Logger>, + session_id: Option<&str>, ) -> Result { let payload = parse_conversation_trace_payload(stdin_payload)?; - persist_conversation_trace_payload_to_agent_trace_db(repository_root, payload, logger) + Ok(persist_conversation_trace_payload_to_agent_trace_db( + repository_root, + payload, + logger, + session_id, + )) } -fn log_conversation_trace_fail_open(error: &anyhow::Error, logger: Option<&dyn Logger>) -> String { +fn log_conversation_trace_fail_open( + error: &anyhow::Error, + logger: Option<&dyn Logger>, + session_id: Option<&str>, +) -> String { if let Some(log) = logger { log.error( "sce.hooks.conversation_trace.error", &error.to_string(), &[], - None, + session_id, ); } @@ -246,11 +263,26 @@ fn persist_conversation_trace_payload_to_agent_trace_db( repository_root: &Path, payload: ConversationTracePayload, logger: Option<&dyn Logger>, -) -> Result { - let db = open_agent_trace_db_for_hook_runtime( + session_id: Option<&str>, +) -> String { + let db = match open_agent_trace_db_for_hook_runtime( repository_root, "Failed to open Agent Trace DB for conversation-trace persistence.", - )?; + ) { + Ok(db) => db, + Err(error) => { + if let Some(log) = logger { + log.error( + "sce.hooks.conversation_trace.agent_trace_db_open_failed", + &error.to_string(), + &[], + session_id, + ); + } + + return String::from("conversation-trace hook intake failed open; error logged."); + } + }; let summary = persist_conversation_trace_payload_to_agent_trace_db_with( payload, @@ -259,9 +291,8 @@ fn persist_conversation_trace_payload_to_agent_trace_db( |inserts| db.insert_parts(inserts), ); - Ok(summary.render()) + summary.render() } - fn persist_conversation_trace_payload_to_agent_trace_db_with( payload: ConversationTracePayload, logger: Option<&dyn Logger>, @@ -323,6 +354,10 @@ where log_skipped_conversation_trace_payloads(logger, EVENT_TYPE, &batch.skipped); let valid_count = batch.inserts.len(); + let session_id = batch + .inserts + .first() + .map(|insert| insert.session_id.clone()); let persisted = if valid_count == 0 { 0 } else { @@ -337,6 +372,7 @@ where EVENT_TYPE, valid_count, &error, + session_id.as_deref(), ); 0 } @@ -361,6 +397,10 @@ where log_skipped_conversation_trace_payloads(logger, EVENT_TYPE, &batch.skipped); let valid_count = batch.inserts.len(); + let session_id = batch + .inserts + .first() + .map(|insert| insert.session_id.clone()); let persisted = if valid_count == 0 { 0 } else { @@ -375,6 +415,7 @@ where EVENT_TYPE, valid_count, &error, + session_id.as_deref(), ); 0 } @@ -402,7 +443,7 @@ fn log_skipped_conversation_trace_payloads( ("event_type", event_type), ("payload_index", index.as_str()), ], - None, + skipped.session_id.as_deref(), ); } } @@ -412,6 +453,7 @@ fn log_conversation_trace_batch_insert_failure( event_type: &str, valid_count: usize, error: &anyhow::Error, + session_id: Option<&str>, ) { if let Some(log) = logger { let count = valid_count.to_string(); @@ -419,7 +461,7 @@ fn log_conversation_trace_batch_insert_failure( "sce.hooks.conversation_trace.agent_trace_db_batch_failed", &error.to_string(), &[("event_type", event_type), ("valid_count", count.as_str())], - None, + session_id, ); } } @@ -475,6 +517,7 @@ fn parse_conversation_trace_payloads(payloads: &[Value]) -> ConversationTracePay let mut skipped = Vec::new(); for (index, item) in payloads.iter().enumerate() { + let session_id = non_empty_string(item.get("session_id")).map(str::to_owned); let Some(item) = conversation_trace_payload_item(item, index, &mut skipped) else { continue; }; @@ -486,6 +529,7 @@ fn parse_conversation_trace_payloads(payloads: &[Value]) -> ConversationTracePay skipped.push(SkippedConversationTracePayload { index, reason: error.to_string(), + session_id: session_id.clone(), }); continue; } @@ -497,6 +541,7 @@ fn parse_conversation_trace_payloads(payloads: &[Value]) -> ConversationTracePay Err(error) => message_skipped.push(SkippedConversationTracePayload { index, reason: error.to_string(), + session_id: session_id.clone(), }), }, CONVERSATION_TRACE_MESSAGE_PART_UPDATED => { @@ -505,6 +550,7 @@ fn parse_conversation_trace_payloads(payloads: &[Value]) -> ConversationTracePay Err(error) => part_skipped.push(SkippedConversationTracePayload { index, reason: error.to_string(), + session_id: session_id.clone(), }), } } @@ -513,6 +559,7 @@ fn parse_conversation_trace_payloads(payloads: &[Value]) -> ConversationTracePay reason: conversation_trace_validation_error( "field 'type' must be one of 'message' or 'message.part'", ), + session_id, }), } } @@ -542,6 +589,7 @@ fn conversation_trace_payload_item<'a>( reason: conversation_trace_validation_error(&format!( "payloads[{index}] must be an object" )), + session_id: None, }); return None; }; @@ -677,15 +725,44 @@ fn conversation_trace_validation_error(detail: &str) -> String { format!("Invalid conversation-trace payload from STDIN: {detail}.") } +fn conversation_trace_fail_open_session_id(stdin_payload: &str) -> Option { + let payload: Value = serde_json::from_str(stdin_payload).ok()?; + let payload = payload.as_object()?; + + if payload.contains_key("hook_event_name") { + return non_empty_string(payload.get("session_id")).map(str::to_owned); + } + + let first_payload = payload.get("payloads")?.as_array()?.first()?; + non_empty_string(first_payload.get("session_id")).map(str::to_owned) +} + +fn diff_trace_fail_open_session_id(stdin_payload: &str) -> Option { + let payload: Value = serde_json::from_str(stdin_payload).ok()?; + let payload = payload.as_object()?; + let field_name = if payload.contains_key("hook_event_name") { + "session_id" + } else { + "sessionID" + }; + + non_empty_string(payload.get(field_name)).map(str::to_owned) +} + +fn non_empty_string(value: Option<&Value>) -> Option<&str> { + value?.as_str().filter(|value| !value.trim().is_empty()) +} + fn run_diff_trace_subcommand(repository_root: &Path, logger: Option<&dyn Logger>) -> String { let stdin_payload = match read_hook_stdin() { Ok(payload) => payload, - Err(error) => return log_diff_trace_fail_open(&error, logger), + Err(error) => return log_diff_trace_fail_open(&error, logger, None), }; + let session_id = diff_trace_fail_open_session_id(&stdin_payload); match run_diff_trace_subcommand_from_payload(repository_root, &stdin_payload, logger) { Ok(output) => output, - Err(error) => log_diff_trace_fail_open(&error, logger), + Err(error) => log_diff_trace_fail_open(&error, logger, session_id.as_deref()), } } @@ -706,9 +783,18 @@ fn run_diff_trace_subcommand_from_payload( )) } -fn log_diff_trace_fail_open(error: &anyhow::Error, logger: Option<&dyn Logger>) -> String { +fn log_diff_trace_fail_open( + error: &anyhow::Error, + logger: Option<&dyn Logger>, + session_id: Option<&str>, +) -> String { if let Some(log) = logger { - log.error("sce.hooks.diff_trace.error", &error.to_string(), &[], None); + log.error( + "sce.hooks.diff_trace.error", + &error.to_string(), + &[], + session_id, + ); } String::from("diff-trace hook intake failed open; error logged.") @@ -725,25 +811,25 @@ fn run_diff_trace_subcommand_from_payload_with( "sce.hooks.diff_trace.agent_trace_db_time_invalid", &error.to_string(), &[], - None, + Some(&payload.session_id), ); } } - let agent_trace_db_result = persist_diff_trace_payload_to_agent_trace_db( + let agent_trace_db_persisted = match persist_diff_trace_payload_to_agent_trace_db( repository_root, payload, payload.model_id.as_deref(), payload.tool_version.as_deref(), - ); - let agent_trace_db_persisted = match agent_trace_db_result { - Ok(()) => true, + logger, + ) { + Ok(persisted) => persisted, Err(error) => { if let Some(log) = logger { log.warn( "sce.hooks.diff_trace.agent_trace_db_write_failed", &error.to_string(), &[], - None, + Some(&payload.session_id), ); } false @@ -1097,27 +1183,42 @@ fn persist_diff_trace_payload_to_agent_trace_db( payload: &DiffTracePayload, model_id: Option<&str>, tool_version: Option<&str>, -) -> Result<()> { + logger: Option<&dyn Logger>, +) -> Result { persist_diff_trace_payload_to_agent_trace_db_with(payload, model_id, tool_version, |input| { - let db = open_agent_trace_db_for_hook_runtime( + let db = match open_agent_trace_db_for_hook_runtime( repository_root, "Failed to open Agent Trace DB for diff-trace persistence.", - )?; + ) { + Ok(db) => db, + Err(error) => { + if let Some(log) = logger { + log.error( + "sce.hooks.diff_trace.agent_trace_db_open_failed", + &error.to_string(), + &[], + Some(&payload.session_id), + ); + } + + return Ok(false); + } + }; db.insert_diff_trace(input) .context("Failed to persist diff-trace payload to Agent Trace DB.")?; - Ok(()) + Ok(true) }) } -fn persist_diff_trace_payload_to_agent_trace_db_with( +fn persist_diff_trace_payload_to_agent_trace_db_with( payload: &DiffTracePayload, model_id: Option<&str>, tool_version: Option<&str>, insert_fn: F, -) -> Result<()> +) -> Result where - F: FnOnce(DiffTraceInsert<'_>) -> Result<()>, + F: FnOnce(DiffTraceInsert<'_>) -> Result, { let time_ms = diff_trace_db_time_ms(payload.time)?; let session_id = prefixed_diff_trace_session_id(&payload.tool_name, &payload.session_id); diff --git a/context/context-map.md b/context/context-map.md index 669d45a3..42f5ec68 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -19,7 +19,7 @@ Feature/domain context: - `context/cli/config-precedence-contract.md` (implemented `sce config` show/validate command contract, deterministic `flags > env > config file > defaults` resolution order, focused `config/resolver.rs` ownership for config discovery/merge/runtime precedence plus default-discovered invalid-file degradation, focused `config/render.rs` ownership for `show`/`validate` text+JSON output construction, canonical `$schema` acceptance for startup-loaded `sce/config.json` files, shared auth-key env/config/optional baked-default support starting with `workos_client_id`, shared runtime resolution for flat logging observability keys including `log_dir` / `SCE_LOG_DIR`, canonical Pkl-generated `sce/config.json` schema ownership plus CLI embedding/reuse contract including `policies.attribution_hooks.enabled` default-true/explicit-false opt-out metadata, config-file selection order, `show` provenance output, and trimmed `validate` output contract) - `context/cli/capability-traits.md` (current broad CLI capability seam in `cli/src/services/capabilities.rs`, including `FsOps`/`StdFsOps`, `GitOps`/`ProcessGitOps`, git root/hooks resolution behavior, compile-time-typed borrowed AppContext wiring with associated-type narrow capability accessors plus `ContextWithRepoRoot` repo-root-scoped context derivation, generic command execution bounds, and test-only unimplemented stubs; current service internals do not consume fs/git traits until later lifecycle migration tasks) - `context/cli/service-lifecycle.md` (current compile-safe lifecycle seam in `cli/src/services/lifecycle.rs`, including default no-op `ServiceLifecycle` diagnose/fix/setup methods against narrow `HasRepoRoot`, lifecycle-owned health/fix/setup result types with generic setup messages, doctor/setup adapter boundaries, the static `LifecycleProvider` enum catalog/dispatcher, hook/config/local_db/auth_db/agent_trace_db lifecycle providers including setup-time checkout identity registration plus per-checkout Agent Trace DB initialization, implemented doctor aggregation over diagnose/fix providers, and implemented setup aggregation over `setup` providers in order config → local_db → auth_db → agent_trace_db → hooks when requested) -- `context/sce/cli-observability-contract.md` (implemented config-backed runtime observability contract for the flat logging config-file shape with `log_dir` / `SCE_LOG_DIR` env-over-config fallback, append-only local-date/session log file routing, deterministic session filename sanitization, concrete logger/telemetry runtime behavior plus logger and object-safe telemetry trait boundaries, AppContext observability wiring, generic `RunOutcome` final rendering, runtime-classified repeated telemetry action protection, operator-facing `sce config show` observability reporting, and the trimmed `sce config validate` status-only validation surface) +- `context/sce/cli-observability-contract.md` (implemented config-backed runtime observability contract for the flat logging config-file shape with `log_dir` / `SCE_LOG_DIR` env-over-config fallback, append-only local-date/session log file routing including reliable producer-native diff-trace/conversation-trace session routing and hook-specific non-duplicated Agent Trace DB-open error events, deterministic session filename sanitization, concrete logger/telemetry runtime behavior plus logger and object-safe telemetry trait boundaries, AppContext observability wiring, generic `RunOutcome` final rendering, runtime-classified repeated telemetry action protection, operator-facing `sce config show` observability reporting, and the trimmed `sce config validate` status-only validation surface) - `context/sce/shared-context-code-workflow.md` - `context/sce/shared-context-plan-workflow.md` (canonical `/change-to-plan` workflow, clarification/readiness gate contract, and one-task/one-atomic-commit task-slicing policy) - `context/sce/plan-code-overlap-map.md` (T01 overlap matrix for Shared Context Plan/Code, related commands, and core skill ownership/dedup targets) @@ -52,7 +52,7 @@ Feature/domain context: - `context/sce/agent-trace-retry-queue-observability.md` (inactive local-hook retry path plus historical retry/metrics reference) - `context/sce/agent-trace-local-hooks-mvp-contract-gap-matrix.md` (T01 Local Hooks MVP production contract freeze and deterministic gap matrix for `agent-trace-local-hooks-production-mvp`) - `context/sce/agent-trace-minimal-generator.md` (implemented a library minimal Agent Trace generator seam at `cli/src/services/agent_trace.rs`, used by the active post-commit hook flow to produce strict `0.1.0` JSON payloads with top-level `version`, UUIDv7 `id` derived from commit-time metadata, caller-provided commit-time `timestamp`, optional top-level `vcs` metadata emitted when present (`type` from enum `git|jj|hg|svn`, `revision` from metadata input; current post-commit flow provides `git`), optional top-level `tool` metadata (`name`/`version`) sourced from builder metadata inputs when overlapping AI content exists, and always-emitted `metadata.sce.version` sourced from the compiled `sce` CLI package version, plus per-file trace data from patch inputs via `intersect_patches(constructed_patch, post_commit_patch)` then `post_commit_patch`-anchored hunk classification into `ai`/`mixed`/`unknown` contributor categories, serialized per conversation with a required lookup `url` derived from top-level `AgentTrace.id`, nested `contributor.type` with optional `contributor.model_id` omitted when provenance is missing, one derived `ranges[{start_line,end_line,content_hash}]` entry per post-commit or embedded-patch hunk, and range `content_hash` values that hash touched-line kind/content independent of positions and metadata) -- `context/sce/agent-trace-hooks-command-routing.md` (implemented `sce hooks` command routing plus current runtime behavior: enabled-by-default commit-msg attribution with explicit opt-out controls, no-op `pre-commit`/`post-rewrite` entrypoints, active `post-commit` intersection and Agent Trace DB persistence, DB-only `diff-trace` STDIN intake with OpenCode normalized payloads and Claude structured `PostToolUse` payload classification, tool-prefixed stored `diff_traces.session_id` values (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi), direct nullable `model_id`/`tool_version` persistence without session fallback, Claude direct model metadata extraction from top-level or nested `model` fields with `claude/` prefix normalization, no parsed-payload artifact persistence under `context/tmp`, `session-model` removed from the supported hook surface, and `conversation-trace` STDIN intake for normalized batches plus supported raw Claude events; this document also owns the current `diff-trace` and `conversation-trace` fail-open intake contracts.) +- `context/sce/agent-trace-hooks-command-routing.md` (implemented `sce hooks` command routing plus current runtime behavior: enabled-by-default commit-msg attribution with explicit opt-out controls, no-op `pre-commit`/`post-rewrite` entrypoints, active `post-commit` intersection and Agent Trace DB persistence, DB-only `diff-trace` STDIN intake with OpenCode normalized payloads and Claude structured `PostToolUse` payload classification, tool-prefixed stored `diff_traces.session_id` values (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi) while hook diagnostics route with unprefixed producer-native sessions when available, direct nullable `model_id`/`tool_version` persistence without session fallback, Claude direct model metadata extraction from top-level or nested `model` fields with `claude/` prefix normalization, no parsed-payload artifact persistence under `context/tmp`, `session-model` removed from the supported hook surface, and `conversation-trace` STDIN intake for normalized batches plus supported raw Claude events with per-item and first-valid-insert batch diagnostic routing; this document also owns the current `diff-trace` and `conversation-trace` fail-open intake contracts.) - `context/sce/automated-profile-contract.md` (deterministic gate policy for automated OpenCode profile, including 10 gate categories, permission mappings, automated `/commit` single-commit execution behavior, and automated profile constraints) - `context/sce/bash-tool-policy-enforcement-contract.md` (approved bash-tool blocking contract plus current Rust evaluator seam and OpenCode/Claude delegation references, including config schema, argv-prefix matching, shell/nix unwrapping, fixed preset catalog/messages, and precedence rules) - `context/sce/generated-opencode-plugin-registration.md` (current generated OpenCode plugin-registration contract, canonical Pkl ownership, generated manifest/plugin paths including `sce-bash-policy` + `sce-agent-trace`, TypeScript source ownership, and Claude generated settings boundary including Agent Trace hooks plus `PreToolUse` Bash policy hook registration through the missing-CLI install-guidance helper) diff --git a/context/patterns.md b/context/patterns.md index d54c47ff..faf7a278 100644 --- a/context/patterns.md +++ b/context/patterns.md @@ -134,8 +134,8 @@ - For future CLI domains, define trait-first service contracts with request/plan models in `cli/src/services/*` and keep placeholder implementations explicitly non-runnable until production behavior is approved. - Model deferred integration boundaries with concrete event/capability data structures (for example hook-runtime attribution snapshots/policies and cloud-sync checkpoints) so later tasks can implement behavior without reshaping public seams. - For the current local-hook baseline, keep `pre-commit` and `post-rewrite` as deterministic no-op entrypoints; keep `post-commit` as the active bounded recent-diff-trace intersection entrypoint with validated `--remote-url` plumbed through Agent Trace flow and any direct diagnostics printed to stderr; keep `diff-trace` as an explicit STDIN intake path with deterministic required-field validation for `sessionID`, `diff`, `time`, `tool_name`, optional `model_id` (absent/`null` → `None`), and `tool_version` (present and either `null` or non-empty string), same-tool-idempotent stored `diff_traces.session_id` prefixing (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi), non-lossy AgentTraceDb `time_ms` conversion, and AgentTraceDb insertion whose failure is logged and reflected in deterministic success text without creating a `context/tmp` artifact fallback; keep `conversation-trace` as the active message/part intake path. `session-model` is no longer a supported hook intake path. -- For `diff-trace` hook intake, keep producer-facing failure behavior fail-open: STDIN read, parse/validation, and setup/persistence failures are logged with `sce.hooks.diff_trace.error` and converted into command success; preserve the existing valid-payload success text and the AgentTraceDb write-warning success path. -- For `conversation-trace` hook intake, keep producer-facing failure behavior fail-open: STDIN read, top-level parse/validation, unsupported raw Claude hook events, and AgentTraceDb setup/persistence failures are logged with `sce.hooks.conversation_trace.error` and converted into command success; preserve valid-payload mixed-batch accounting, skipped-item logging, and batch-insert warning behavior. +- For `diff-trace` hook intake, keep producer-facing failure behavior fail-open: STDIN read and parse/validation failures use `sce.hooks.diff_trace.error`; AgentTraceDb open failures use the error-level `sce.hooks.diff_trace.agent_trace_db_open_failed`; later conversion/insert failures retain `sce.hooks.diff_trace.agent_trace_db_write_failed`. Preserve existing output text and emit only the most specific persistence diagnostic for one failure. +- For `conversation-trace` hook intake, keep producer-facing failure behavior fail-open: STDIN read, top-level parse/validation, and unsupported raw Claude hook events use `sce.hooks.conversation_trace.error`; AgentTraceDb open failures use the error-level `sce.hooks.conversation_trace.agent_trace_db_open_failed`; later batch inserts retain their existing warning event. Preserve valid-payload mixed-batch accounting, skipped-item logging, output text, and one diagnostic per DB-open failure. - For diff-trace attribution persistence, persist direct payload `model_id` and `tool_version` values as-is; missing attribution fields are stored as `NULL` in `diff_traces`. The former `session_models` fallback lookup was removed. - For commit-msg co-author policy seams, gate canonical trailer insertion on runtime controls (`SCE_DISABLED` plus the shared attribution-hooks enablement gate) plus the staged-diff AI-overlap evidence gate (`StagedDiffAiOverlapResult::Overlap` maps to `ai_contribution_present = true`; `NoOverlap` and `Error` both map to `false`), and enforce idempotent dedupe so allowed cases end with exactly one `Co-authored-by: SCE ` trailer. - For local hook attribution flows, resolve the top-level enablement gate through the shared config precedence model (`SCE_ATTRIBUTION_HOOKS_DISABLED` opt-out env over `policies.attribution_hooks.enabled`, default `true`) so commit-msg attribution is enabled by default while explicit config `enabled = false` and truthy env opt-out still suppress it without adding hook-specific config parsing. diff --git a/context/sce/agent-trace-db.md b/context/sce/agent-trace-db.md index 157ab772..a206f5aa 100644 --- a/context/sce/agent-trace-db.md +++ b/context/sce/agent-trace-db.md @@ -177,7 +177,7 @@ Both triggers compare `OLD.*` vs `NEW.*` for all mutable columns (excluding `upd - Direct payload `model_id` and `tool_version` values pass into `DiffTraceInsert` as-is. The stored `session_id` is tool-prefixed before insert construction: `opencode` payloads store `oc_`, `claude` structured payloads store `cc_`, `pi` normalized payloads store `pi_`, and same-tool-prefixed values are not double-prefixed. The `payload_type` field is set to `PAYLOAD_TYPE_PATCH` for `OpenCode` normalized diff-trace payloads and `PAYLOAD_TYPE_STRUCTURED` for Claude structured `PostToolUse` payloads. Claude structured intake best-effort extracts direct `model`/`model_id`/`modelId` metadata, including nested `model.id` / `model.model` / `model.name`, normalizes values with the `claude/` prefix when present, and leaves `model_id` nullable when metadata is absent. - `time` is accepted as a `u64` Unix epoch millisecond input and must fit the signed `i64` `time_ms` column before any persistence starts. - The hook inserts the parsed payload fields plus nullable direct attribution through `AgentTraceDb::insert_diff_trace()` without writing a parsed-payload artifact under `context/tmp`. -- AgentTraceDb open/insert failures are logged and reflected in deterministic success text as failed DB persistence; no artifact fallback is created. +- AgentTraceDb open failures are logged at error level through `sce.hooks.diff_trace.agent_trace_db_open_failed`; later conversion/insert failures retain `sce.hooks.diff_trace.agent_trace_db_write_failed`. Both failure classes preserve deterministic failed-persistence success text and create no artifact fallback. Open failures use the producer-native unprefixed session and do not also emit the write-failure event. - Existing artifact files are not backfilled into the database. Post-commit intersection rows are written by the active `post-commit` hook flow through per-checkout lazy AgentTraceDb access, and the same flow now also inserts built Agent Trace payloads into `agent_traces` via `AgentTraceDb::insert_agent_trace()` (see [agent-trace-hooks-command-routing.md](agent-trace-hooks-command-routing.md)). The persisted `trace_json` is the schema-validated `build_agent_trace(...)` output and includes top-level `metadata.sce.version` from the compiled `sce` CLI package version plus `content_hash` on every emitted range. Range `content_hash` values are computed from the touched-line kind/content of the post-commit hunk that produced the persisted range, not from DB IDs, paths, line positions, or runtime metadata. @@ -188,7 +188,7 @@ Post-commit intersection rows are written by the active `post-commit` hook flow - `message` items validate and map payloads without message-level `text`, `agent`, or `summary_diffs` to `InsertMessageInsert`; valid rows are inserted through at most one multi-row `AgentTraceDb::insert_messages(...)` call per invocation so repeated `(session_id, message_id)` events are ignored without failing. - `message.part` items validate and map payloads with required part `text` to `InsertPartInsert`; valid rows are inserted through at most one multi-row `AgentTraceDb::insert_parts(...)` call per invocation so parts remain append-only and do not require a pre-existing message row. - Unsupported item types, missing/non-string item types, non-object items, and event-specific parser validation failures are retained as skipped-item diagnostics, logged, and counted as skipped while valid sibling items remain eligible for persistence. -- The hook opens one per-checkout `AgentTraceDb` per invocation through lazy checkout DB resolution before insertion; DB open/initialization failures are logged through `sce.hooks.conversation_trace.error` and returned as hook success because conversation-trace intake is fail-open to producers. +- The hook opens one per-checkout `AgentTraceDb` per invocation through lazy checkout DB resolution before insertion; DB open/initialization failures are logged at error level through `sce.hooks.conversation_trace.agent_trace_db_open_failed` and returned as hook success because conversation-trace intake is fail-open to producers. The event uses the existing best-effort producer-native session route and does not also emit `sce.hooks.conversation_trace.error` for the same open failure. - Multi-row insert failures are logged once and count the whole valid-item batch as skipped without failing the command; the hook does not fall back to row-by-row insertion after a batch failure. Successful inserts contribute to deterministic success output counts (`attempted`, `persisted_messages`, `persisted_parts`, `skipped`). Duplicate parent message inserts preserve the existing `ON CONFLICT DO NOTHING` affected-row semantics. - No `context/tmp` artifact is written for conversation traces. - The generated OpenCode agent-trace plugin sends mixed-batch envelopes for conversation traces: regular `message` and `message.part` events each carry one per-item `type`, while diff-backed `message` events send one envelope containing the synthetic parent message item plus patch part items. diff --git a/context/sce/agent-trace-hooks-command-routing.md b/context/sce/agent-trace-hooks-command-routing.md index 9d4a6a27..31a69224 100644 --- a/context/sce/agent-trace-hooks-command-routing.md +++ b/context/sce/agent-trace-hooks-command-routing.md @@ -73,8 +73,8 @@ - OpenCode forwards user-message `message` diffs with `tool_name="opencode"`, always including `model_id`, and nullable OpenCode client-version metadata. - Claude generated settings no longer register `SessionStart`; supported `PostToolUse` `Write|Edit|MultiEdit|NotebookEdit` events are routed directly to `sce hooks diff-trace`. Runtime persistence currently derives structured diff traces for `Write` create and `Edit` structured-patch payloads; unsupported Claude payload shapes are no-ops. - Neither TypeScript runtime writes `context/tmp/*-diff-trace.json` artifacts or AgentTraceDb rows directly. -- `diff-trace` command success reports AgentTraceDb persistence only. AgentTraceDb open/insert failures are logged through `sce.hooks.diff_trace.agent_trace_db_write_failed` and reflected in deterministic success text as failed DB persistence; no parsed-payload artifact fallback is created. -- `diff-trace` producer-facing intake failures are logged through `sce.hooks.diff_trace.error` and returned as hook success; the valid-payload path is DB-only and does not write parsed-payload artifacts. +- `diff-trace` command success reports AgentTraceDb persistence only. AgentTraceDb open failures are logged at error level through `sce.hooks.diff_trace.agent_trace_db_open_failed`; later conversion/insert failures retain the warn-level `sce.hooks.diff_trace.agent_trace_db_write_failed` event. Both paths keep the deterministic failed-persistence success text and create no parsed-payload artifact fallback. Diagnostics route through the logger's optional session argument with the original producer-provided session ID, never the AgentTraceDb-only tool-prefixed value. A DB-open failure emits only the open-specific event, not the broader write-failure event. +- `diff-trace` producer-facing intake failures are logged through `sce.hooks.diff_trace.error` and returned as hook success. Fail-open routing checks only the expected non-empty top-level session field: `session_id` when `hook_event_name` identifies a Claude raw event, otherwise `sessionID`; malformed JSON, wrong-shaped input, or a missing/empty expected field remains sessionless. The valid-payload path is DB-only and does not write parsed-payload artifacts. - `conversation-trace` is a recognized hook subcommand routed through `HookSubcommand::ConversationTrace`. Rust intake classifies incoming STDIN JSON by the presence of a top-level `hook_event_name` field: raw Claude hook events are routed through `transform_claude_user_prompt_submit`, `transform_claude_stop`, or `transform_claude_post_tool_use` depending on the event name, while payloads without `hook_event_name` follow the existing mixed-batch `{ payloads: [...] }` path. - **Raw Claude `UserPromptSubmit` events** (detected by `hook_event_name = "UserPromptSubmit"`): the raw event payload is validated and transformed by `transform_claude_user_prompt_submit` before being forwarded to `parse_conversation_trace_payloads`. - Validates that `hook_event_name` is exactly `"UserPromptSubmit"` and the required `session_id` and `prompt` fields are present and non-empty. @@ -103,10 +103,11 @@ - `part_type: "text"` and `part_type: "reasoning"` store the raw `text` string unchanged. - `part_type: "patch"` first tries `load_patch_from_json` — if `payload.text` is already a valid JSON-serialized `ParsedPatch`, stores the raw text unchanged (no re-parse, no re-serialize). If JSON loading fails, falls back to the shared unified-diff parser (`parse_patch_from_text`) and stores JSON-serialized `ParsedPatch` in `parts.text`. If both fail, the item is skipped with a validation error mentioning both patch and patch-JSON formats. - `part_type: "question"` requires `text` to be a JSON string whose parsed value is an array of objects with string `question` and `answer` fields; valid question text is stored unchanged, while invalid question text is skipped through the same per-item validation path as malformed patch items. - - Unsupported item `type` values, missing/non-string item `type`, non-object items, and event-specific item validation failures are recorded as skipped-item diagnostics (`index`, `reason`) while valid sibling items remain eligible for persistence; skipped validation items are logged through `sce.hooks.conversation_trace.payload_skipped`. Top-level JSON/object/`payloads` shape failures produce `Invalid conversation-trace payload from STDIN: ...` diagnostics internally, then fail open through `sce.hooks.conversation_trace.error` with hook success text. + - Unsupported item `type` values, missing/non-string item `type`, non-object items, and event-specific item validation failures are recorded as skipped-item diagnostics (`index`, `reason`, optional producer session) while valid sibling items remain eligible for persistence; skipped validation items are logged through `sce.hooks.conversation_trace.payload_skipped` using that item's usable `session_id` for logger file routing only. Top-level JSON/object/`payloads` shape failures produce `Invalid conversation-trace payload from STDIN: ...` diagnostics internally, then fail open through `sce.hooks.conversation_trace.error` with hook success text. - Shared persistence (both classification paths converge before DB writes): - Current persistence opens one per-checkout `AgentTraceDb` per hook invocation through lazy checkout DB resolution, then inserts the non-empty valid `message` batch through at most one multi-row `AgentTraceDb::insert_messages(...)` call and the non-empty valid `message.part` batch through at most one multi-row `AgentTraceDb::insert_parts(...)` call. - - DB open/initialization failures fail open through `sce.hooks.conversation_trace.error` with hook success text; valid-item multi-row insert failures are logged once through `sce.hooks.conversation_trace.agent_trace_db_batch_failed`, count the whole valid-item batch as skipped, and do not fail the command. The hook does not fall back to row-by-row insertion after a multi-row insert failure. + - DB open/initialization failures log at error level through `sce.hooks.conversation_trace.agent_trace_db_open_failed` and return the existing hook success text without also emitting the broader `sce.hooks.conversation_trace.error` event. Session routing checks only top-level `session_id` for a raw Claude event identified by `hook_event_name`; otherwise it checks only `payloads[0].session_id`. Malformed JSON, wrong-shaped input, or a missing/empty expected field remains sessionless; later mixed-batch items are not inspected for this fail-open route. + - Valid-item multi-row insert failures are logged once through `sce.hooks.conversation_trace.agent_trace_db_batch_failed`, count the whole valid-item batch as skipped, and do not fail the command. The batch warning routes with the first valid insert's `session_id`; an empty batch remains sessionless. The diagnostic is emitted once and the hook does not fall back to row-by-row insertion after a multi-row insert failure. - Current valid-payload success output reports deterministic mixed-batch accounting: `conversation-trace hook persisted mixed payload batch to AgentTraceDb: attempted=, persisted_messages=, persisted_parts=, skipped=.` The hook does not persist `context/tmp` artifacts. - Fail-open output for conversation-trace intake failures is `conversation-trace hook intake failed open; error logged.` so hook callers do not receive app-level classified errors or non-zero exits for intake failures. - The generated OpenCode agent-trace plugin emits this mixed-batch shape for conversation-trace handoff: ordinary message/part events produce one-item mixed envelopes, completed question-tool parts produce `message.part` items with `part_type: "question"`, and diff-backed message events produce one envelope containing the synthetic parent `message` item plus patch `message.part` items. diff --git a/context/sce/cli-observability-contract.md b/context/sce/cli-observability-contract.md index af951d83..16f68731 100644 --- a/context/sce/cli-observability-contract.md +++ b/context/sce/cli-observability-contract.md @@ -29,6 +29,7 @@ Runtime observability now consumes the shared resolved observability config from - When `log_dir` is configured, each enabled or forced log operation also appends the redacted rendered record to a file selected at emit time from the machine-local date and optional caller-provided session ID. - Sessionless file logs route to `/sce-.log`; session-aware file logs route to `/sce--.log`. - Session filename sanitization preserves ASCII letters, digits, `-`, and `_`; percent-encodes every other UTF-8 byte as uppercase `%HH`; and represents an explicitly empty `Some("")` session ID with the reserved `%EMPTY` token. +- `sce hooks diff-trace` and `conversation-trace` pass producer-native session context into this existing routing argument when available. Diff-trace logging never uses the AgentTraceDb-only `oc_`/`cc_`/`pi_` prefix; skipped conversation items use their own session; batch-wide conversation insert failures use the first valid insert's session. Agent Trace DB open failures use hook-specific error events (`sce.hooks.diff_trace.agent_trace_db_open_failed` and `sce.hooks.conversation_trace.agent_trace_db_open_failed`) and do not also emit their broader write/intake events for the same failure. Session IDs remain absent from rendered record fields unless separately supplied as fields. - File routing is append-only, creates the configured directory when needed, uses owner-only create permissions on Unix, serializes writes per path, and fails open by keeping stderr logging active when file creation/write/flush fails. - Each emitted record includes a stable `event_id`. - Current app-level event identifiers: