diff --git a/cli/src/cli_schema.rs b/cli/src/cli_schema.rs index a9537c56..1700ea4b 100644 --- a/cli/src/cli_schema.rs +++ b/cli/src/cli_schema.rs @@ -253,6 +253,14 @@ pub enum TraceSubcommand { #[arg(long, value_enum, default_value_t = OutputFormat::Text)] format: OutputFormat, }, + + #[command( + about = "Synchronize the current repository's Agent Trace database with the control plane" + )] + Sync { + #[arg(long, value_enum, default_value_t = OutputFormat::Text)] + format: OutputFormat, + }, } #[derive(Subcommand, Debug, Clone, PartialEq, Eq)] diff --git a/cli/src/services/agent_trace_export/mod.rs b/cli/src/services/agent_trace_export/mod.rs index 7995c70a..b3ee1450 100644 --- a/cli/src/services/agent_trace_export/mod.rs +++ b/cli/src/services/agent_trace_export/mod.rs @@ -712,7 +712,13 @@ mod tests { let db_path = unique_test_db_path("messages-limit"); let db = RepositoryAgentTraceDb::new_at(&db_path).expect("test DB should open"); for id in 1..=10 { - insert_message_row_with_id(&db, id, &format!("msg-{id}")); + db.insert_message(InsertMessageInsert { + session_id: "sess-1".to_string(), + message_id: format!("msg-{id}"), + role: MessageRole::Assistant, + generated_at_unix_ms: 1_000 + id, + }) + .expect("seed message should insert"); } let reader = AgentTraceExportReader::new(&db); diff --git a/cli/src/services/agent_trace_sync/control_plane.rs b/cli/src/services/agent_trace_sync/control_plane.rs new file mode 100644 index 00000000..362131ab --- /dev/null +++ b/cli/src/services/agent_trace_sync/control_plane.rs @@ -0,0 +1,1225 @@ +//! Wire-contract DTOs for the control-plane Agent Trace ingestion API. +//! +//! These types define the request/response shapes for +//! `POST /agent-trace/ingestion/state` and `POST /agent-trace/ingestion/batch`. +//! They perform no HTTP I/O and hold no cursor state themselves. + +use std::fmt; +use std::sync::Arc; + +use anyhow::anyhow; +use reqwest::StatusCode; +use serde::{Deserialize, Serialize}; + +use crate::services::agent_trace_export::{ + self, AgentTraceAgentTraceExportRow, AgentTraceDiffTraceExportRow, AgentTraceMessageExportRow, + AgentTracePartExportRow, +}; +use crate::services::auth::{self, AuthError, TokenResponse}; +use crate::services::resilience::{run_with_retry, RetryPolicy}; +use crate::services::token_storage::{self, StoredTokens, TokenStorageError}; + +/// One of the four independent Agent Trace capture streams, identified by its +/// literal wire value. These values are always the `snake_case` stream +/// identifiers (`messages`, `parts`, `diff_traces`, `agent_traces`), distinct +/// from the `camelCase` field names (`diffTraces`, `agentTraces`) used in +/// [`AgentTraceCursors`]. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum IngestionStream { + Messages, + Parts, + DiffTraces, + AgentTraces, +} + +/// Request body for `POST /agent-trace/ingestion/state`. +#[derive(Clone, Debug, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct AgentTraceIngestionStateRequest { + pub repository_id: String, + pub source_instance_id: String, +} + +/// Authoritative server-side cursor for each of the four capture streams, as +/// returned by `/state`. Each cursor is the last `source_row_id` the control +/// plane has accepted for that stream. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct AgentTraceCursors { + pub messages: i64, + pub parts: i64, + pub diff_traces: i64, + pub agent_traces: i64, +} + +impl AgentTraceCursors { + /// Rejects any field outside `0..=JS_MAX_SAFE_INTEGER`, the range an + /// exportable cursor must stay within to survive JSON round-trip without + /// truncation or casting. The control plane is the authoritative source + /// of cursor progress, but a syntactically valid `/state` response can + /// still carry a value the wire contract cannot represent; this rejects + /// it before it reaches any export reader or the sync engine. + fn validate(&self) -> Result<(), ControlPlaneError> { + for (name, value) in [ + ("messages", self.messages), + ("parts", self.parts), + ("diffTraces", self.diff_traces), + ("agentTraces", self.agent_traces), + ] { + agent_trace_export::validate_js_safe_integer(value).map_err(|error| { + ControlPlaneError::InvalidResponse(format!("cursors.{name} is invalid: {error}")) + })?; + } + + Ok(()) + } +} + +/// Response body for `POST /agent-trace/ingestion/state`. +#[derive(Clone, Debug, PartialEq, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct AgentTraceIngestionStateResponse { + pub cursors: AgentTraceCursors, +} + +/// Request body for `POST /agent-trace/ingestion/batch`, generic over the +/// PR #198 export row type carried by the stream being uploaded. +#[derive(Clone, Debug, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct AgentTraceIngestionBatchRequest { + pub repository_id: String, + pub source_instance_id: String, + pub stream: IngestionStream, + pub expected_cursor: i64, + pub rows: Vec, +} + +/// Response body for `POST /agent-trace/ingestion/batch`. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct AgentTraceIngestionBatchResponse { + pub accepted: usize, + pub cursor: i64, +} + +const STATE_PATH: &str = "agent-trace/ingestion/state"; +const BATCH_PATH: &str = "agent-trace/ingestion/batch"; + +const STATE_RETRY_MAX_ATTEMPTS: u32 = 3; +const STATE_RETRY_TIMEOUT_MS: u64 = 10_000; +const STATE_RETRY_INITIAL_BACKOFF_MS: u64 = 250; +const STATE_RETRY_MAX_BACKOFF_MS: u64 = 2_000; + +/// Typed failure classification for control-plane HTTP interactions, kept +/// separate from `ClassifiedError` so the sync engine and CLI wiring can +/// react to each case before deciding how to surface it. +#[derive(Debug)] +pub enum ControlPlaneError { + MissingCredentials, + AuthenticationFailed(String), + Transport(String), + BadRequest(String), + Forbidden(String), + Conflict(String), + ServerError(String), + InvalidResponse(String), + Storage(String), + /// A terminal protocol/API mismatch during a control-plane request (e.g. + /// `404`/`405`/`415`/`422`), distinct from `InvalidResponse` which is + /// reserved for a syntactically successful (`2xx`) but undecodable body. + Protocol { + status: StatusCode, + message: String, + }, +} + +impl fmt::Display for ControlPlaneError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::MissingCredentials => write!( + f, + "No stored WorkOS credentials were found. Try: run 'sce auth login' before running 'sce trace sync'." + ), + Self::AuthenticationFailed(reason) => write!( + f, + "Control-plane authentication failed: {reason}. Try: run 'sce auth login' to re-authenticate." + ), + Self::Transport(reason) => write!(f, "Control-plane request failed: {reason}"), + Self::BadRequest(reason) => { + write!(f, "Control-plane rejected the request as invalid: {reason}") + } + Self::Forbidden(reason) => write!(f, "Control-plane denied the request: {reason}"), + Self::Conflict(reason) => { + write!(f, "Control-plane reported a cursor conflict: {reason}") + } + Self::ServerError(reason) => { + write!(f, "Control-plane request failed with a server error: {reason}") + } + Self::InvalidResponse(reason) => { + write!(f, "Control-plane returned an unexpected response: {reason}") + } + Self::Storage(reason) => write!(f, "Local credential storage error: {reason}"), + Self::Protocol { status, message } => write!( + f, + "Control-plane rejected the request ({status}): {message}" + ), + } + } +} + +impl std::error::Error for ControlPlaneError {} + +impl From for ControlPlaneError { + fn from(value: TokenStorageError) -> Self { + Self::Storage(value.to_string()) + } +} + +impl From for ControlPlaneError { + fn from(value: AuthError) -> Self { + match value { + AuthError::Unauthorized(reason) => Self::AuthenticationFailed(reason), + AuthError::RequestFailed(error) => Self::Transport(error.to_string()), + AuthError::Storage(error) => Self::Storage(error.to_string()), + other => Self::AuthenticationFailed(other.to_string()), + } + } +} + +fn is_transient(error: &ControlPlaneError) -> bool { + matches!( + error, + ControlPlaneError::Transport(_) | ControlPlaneError::ServerError(_) + ) +} + +enum StateAttempt { + Done(AgentTraceIngestionStateResponse), + Terminal(ControlPlaneError), +} + +/// Seam over `token_storage::{load_tokens, save_tokens}` so tests can assert +/// exactly when a token is (or is not) saved without touching the real, +/// process-wide encrypted auth database. +pub trait CredentialStore: Send + Sync { + fn load(&self) -> Result, ControlPlaneError>; + fn save(&self, token: &TokenResponse) -> Result; +} + +/// Production `CredentialStore` backed by the real encrypted auth database. +pub struct SystemCredentialStore; + +impl CredentialStore for SystemCredentialStore { + fn load(&self) -> Result, ControlPlaneError> { + Ok(token_storage::load_tokens()?) + } + + fn save(&self, token: &TokenResponse) -> Result { + Ok(token_storage::save_tokens(token)?) + } +} + +/// Authenticated HTTP client for the control-plane Agent Trace ingestion API. +/// +/// Loads/refreshes stored `WorkOS` credentials through the existing +/// `auth`/`token_storage` primitives, injects the `Authorization: Bearer` +/// header, and retries exactly once on an unexpected `401`. +pub struct AuthenticatedControlPlaneClient { + http: reqwest::Client, + base_url: String, + workos_api_base_url: String, + workos_client_id: String, + credential_store: Arc, +} + +impl AuthenticatedControlPlaneClient { + pub fn new( + http: reqwest::Client, + base_url: impl Into, + workos_api_base_url: impl Into, + workos_client_id: impl Into, + ) -> Self { + Self::with_credential_store( + http, + base_url, + workos_api_base_url, + workos_client_id, + Box::new(SystemCredentialStore), + ) + } + + pub fn with_credential_store( + http: reqwest::Client, + base_url: impl Into, + workos_api_base_url: impl Into, + workos_client_id: impl Into, + credential_store: Box, + ) -> Self { + Self { + http, + base_url: base_url.into(), + workos_api_base_url: workos_api_base_url.into(), + workos_client_id: workos_client_id.into(), + credential_store: Arc::from(credential_store), + } + } + + /// Calls `POST /agent-trace/ingestion/state`, retrying transient + /// `500`/`503`/transport failures via the existing sync resilience retry + /// policy. `400`/`403`/post-refresh-`401` are terminal and never retried. + pub async fn ingestion_state( + &self, + request: &AgentTraceIngestionStateRequest, + ) -> Result { + let url = self.endpoint(STATE_PATH); + let policy = RetryPolicy { + max_attempts: STATE_RETRY_MAX_ATTEMPTS, + timeout_ms: STATE_RETRY_TIMEOUT_MS, + initial_backoff_ms: STATE_RETRY_INITIAL_BACKOFF_MS, + max_backoff_ms: STATE_RETRY_MAX_BACKOFF_MS, + }; + + let outcome = run_with_retry( + policy, + "agent_trace_sync.ingestion_state", + "check network connectivity and control-plane availability, then rerun 'sce trace sync'", + |_attempt| { + let url = url.clone(); + async move { + match self.send_state_request(&url, request).await { + Ok(response) => Ok(StateAttempt::Done(response)), + Err(error) if is_transient(&error) => Err(anyhow!(error)), + Err(error) => Ok(StateAttempt::Terminal(error)), + } + } + }, + ) + .await + .map_err(|error| ControlPlaneError::ServerError(error.to_string()))?; + + match outcome { + StateAttempt::Done(response) => Ok(response), + StateAttempt::Terminal(error) => Err(error), + } + } + + pub async fn ingest_messages( + &self, + request: &AgentTraceIngestionBatchRequest, + ) -> Result { + self.post_batch(request).await + } + + pub async fn ingest_parts( + &self, + request: &AgentTraceIngestionBatchRequest, + ) -> Result { + self.post_batch(request).await + } + + pub async fn ingest_diff_traces( + &self, + request: &AgentTraceIngestionBatchRequest, + ) -> Result { + self.post_batch(request).await + } + + pub async fn ingest_agent_traces( + &self, + request: &AgentTraceIngestionBatchRequest, + ) -> Result { + self.post_batch(request).await + } + + async fn post_batch( + &self, + request: &AgentTraceIngestionBatchRequest, + ) -> Result + where + T: Serialize, + { + let url = self.endpoint(BATCH_PATH); + let response = self + .execute_authenticated(|token| self.http.post(&url).bearer_auth(token).json(request)) + .await?; + classify_response(response).await + } + + async fn send_state_request( + &self, + url: &str, + request: &AgentTraceIngestionStateRequest, + ) -> Result { + let response = self + .execute_authenticated(|token| self.http.post(url).bearer_auth(token).json(request)) + .await?; + let state: AgentTraceIngestionStateResponse = classify_response(response).await?; + state.cursors.validate()?; + Ok(state) + } + + /// Sends one authenticated request, retrying exactly once on an + /// unexpected `401` (refresh, save, retry). A `401` after the retry is + /// terminal. + async fn execute_authenticated( + &self, + build: F, + ) -> Result + where + F: Fn(&str) -> reqwest::RequestBuilder, + { + let token = self.resolve_access_token().await?; + let response = build(&token) + .send() + .await + .map_err(|error| ControlPlaneError::Transport(error.to_string()))?; + + if response.status() != StatusCode::UNAUTHORIZED { + return Ok(response); + } + + let refreshed_token = self.force_refresh_access_token().await?; + let retried = build(&refreshed_token) + .send() + .await + .map_err(|error| ControlPlaneError::Transport(error.to_string()))?; + + if retried.status() == StatusCode::UNAUTHORIZED { + return Err(ControlPlaneError::AuthenticationFailed( + "control-plane returned 401 after a refreshed token was retried".to_string(), + )); + } + + Ok(retried) + } + + /// Loads the stored token, reusing it as-is when still valid and + /// refreshing (and saving) it only when expired. Makes exactly one + /// expiry decision, so a token cannot be refreshed without also being + /// persisted. + async fn resolve_access_token(&self) -> Result { + let stored = self + .load_credentials() + .await? + .ok_or(ControlPlaneError::MissingCredentials)?; + + if !auth::is_stored_token_expired(&stored)? { + return Ok(stored.access_token); + } + + let token = auth::renew_stored_token_from_refresh_token( + &self.http, + &self.workos_api_base_url, + &self.workos_client_id, + &stored.refresh_token, + ) + .await?; + self.save_credentials(&token).await?; + + Ok(token.access_token) + } + + /// Unconditionally refreshes the stored token via its refresh token and + /// saves the result, used for the unexpected-`401` retry path. + async fn force_refresh_access_token(&self) -> Result { + let stored = self + .load_credentials() + .await? + .ok_or(ControlPlaneError::MissingCredentials)?; + let token = auth::renew_stored_token_from_refresh_token( + &self.http, + &self.workos_api_base_url, + &self.workos_client_id, + &stored.refresh_token, + ) + .await?; + self.save_credentials(&token).await?; + Ok(token.access_token) + } + + async fn load_credentials(&self) -> Result, ControlPlaneError> { + let credential_store = Arc::clone(&self.credential_store); + tokio::task::spawn_blocking(move || credential_store.load()) + .await + .map_err(|error| { + ControlPlaneError::Storage(format!( + "credential store worker failed while loading credentials: {error}" + )) + })? + } + + async fn save_credentials( + &self, + token: &TokenResponse, + ) -> Result { + let credential_store = Arc::clone(&self.credential_store); + let token = token.clone(); + tokio::task::spawn_blocking(move || credential_store.save(&token)) + .await + .map_err(|error| { + ControlPlaneError::Storage(format!( + "credential store worker failed while saving credentials: {error}" + )) + })? + } + + fn endpoint(&self, path: &str) -> String { + format!("{}/{}", self.base_url.trim_end_matches('/'), path) + } +} + +/// Maximum accepted length, in bytes, of a `message`/`error` field extracted +/// from a control-plane error body. Anything longer is treated as +/// untrustworthy and discarded in favor of a generic fallback. +const MAX_SAFE_ERROR_MESSAGE_LEN: usize = 500; + +/// Extracts a narrow, safe error message from a raw HTTP error response body. +/// +/// Accepts only a top-level JSON object with a `message` or `error` string +/// field (checked in that order) no longer than +/// [`MAX_SAFE_ERROR_MESSAGE_LEN`]. Returns `None` for malformed JSON, HTML, +/// non-object top-level values, a missing/non-string field, or a field that +/// exceeds the length bound — so an arbitrary server-side implementation +/// detail (a SQL error, a stack trace, an HTML error page) can never reach +/// the CLI's user-visible error text. +fn extract_safe_error_message(body: &str) -> Option { + let value: serde_json::Value = serde_json::from_str(body).ok()?; + let object = value.as_object()?; + let field = object + .get("message") + .or_else(|| object.get("error"))? + .as_str()?; + + if field.is_empty() || field.len() > MAX_SAFE_ERROR_MESSAGE_LEN { + return None; + } + + Some(field.to_string()) +} + +fn safe_error_message(body: &str, generic: &str) -> String { + extract_safe_error_message(body).unwrap_or_else(|| generic.to_string()) +} + +async fn classify_response(response: reqwest::Response) -> Result +where + T: serde::de::DeserializeOwned, +{ + let status = response.status(); + if status.is_success() { + return response + .json::() + .await + .map_err(|error| ControlPlaneError::InvalidResponse(error.to_string())); + } + + let body = response + .text() + .await + .unwrap_or_else(|error| format!("")); + + match status { + StatusCode::BAD_REQUEST => Err(ControlPlaneError::BadRequest(safe_error_message( + &body, + "control plane rejected the Agent Trace request", + ))), + StatusCode::FORBIDDEN => Err(ControlPlaneError::Forbidden(safe_error_message( + &body, + "Agent Trace source cannot be synchronized by the current authenticated user", + ))), + StatusCode::CONFLICT => Err(ControlPlaneError::Conflict(safe_error_message( + &body, + "Agent Trace cursor conflict", + ))), + StatusCode::SERVICE_UNAVAILABLE => Err(ControlPlaneError::ServerError(safe_error_message( + &body, + "control-plane Agent Trace storage is unavailable", + ))), + status if status.is_server_error() => Err(ControlPlaneError::ServerError( + safe_error_message(&body, "control plane encountered an internal error"), + )), + status if status.is_client_error() => Err(ControlPlaneError::Protocol { + status, + message: safe_error_message( + &body, + "control plane rejected the Agent Trace request as unsupported", + ), + }), + status => Err(ControlPlaneError::InvalidResponse(format!( + "unexpected status {status}" + ))), + } +} + +#[cfg(test)] +mod tests { + use std::sync::{Arc, Mutex}; + use std::time::{SystemTime, UNIX_EPOCH}; + + use super::*; + use crate::services::agent_trace_db::MessageRole; + use crate::services::agent_trace_export::{ + AgentTraceAgentTraceExportRow, AgentTraceDiffTraceExportRow, AgentTraceMessageExportRow, + AgentTracePartExportRow, + }; + use crate::services::agent_trace_sync::test_http_server::{CannedResponse, TestHttpServer}; + use serde_json::json; + + #[test] + fn ingestion_stream_serializes_to_exact_snake_case_wire_values() { + assert_eq!( + serde_json::to_value(IngestionStream::Messages).unwrap(), + json!("messages") + ); + assert_eq!( + serde_json::to_value(IngestionStream::Parts).unwrap(), + json!("parts") + ); + assert_eq!( + serde_json::to_value(IngestionStream::DiffTraces).unwrap(), + json!("diff_traces") + ); + assert_eq!( + serde_json::to_value(IngestionStream::AgentTraces).unwrap(), + json!("agent_traces") + ); + } + + #[test] + fn state_request_serializes_to_camel_case_fields() { + let request = AgentTraceIngestionStateRequest { + repository_id: "acme-monorepo".to_string(), + source_instance_id: "src-123".to_string(), + }; + + assert_eq!( + serde_json::to_value(&request).unwrap(), + json!({ + "repositoryId": "acme-monorepo", + "sourceInstanceId": "src-123", + }) + ); + } + + #[test] + fn state_response_deserializes_camel_case_cursor_fields() { + let response: AgentTraceIngestionStateResponse = serde_json::from_value(json!({ + "cursors": { + "messages": 10, + "parts": 20, + "diffTraces": 30, + "agentTraces": 40, + } + })) + .unwrap(); + + assert_eq!( + response.cursors, + AgentTraceCursors { + messages: 10, + parts: 20, + diff_traces: 30, + agent_traces: 40, + } + ); + } + + #[test] + fn agent_trace_cursors_accept_boundary_values() { + for boundary in [0_i64, 1_i64, agent_trace_export::JS_MAX_SAFE_INTEGER] { + let cursors = AgentTraceCursors { + messages: boundary, + parts: boundary, + diff_traces: boundary, + agent_traces: boundary, + }; + assert!( + cursors.validate().is_ok(), + "boundary {boundary} should be valid" + ); + } + } + + #[test] + fn agent_trace_cursors_reject_out_of_range_value_in_each_field() { + let base = AgentTraceCursors { + messages: 0, + parts: 0, + diff_traces: 0, + agent_traces: 0, + }; + + for invalid in [-1_i64, agent_trace_export::JS_MAX_SAFE_INTEGER + 1] { + let cases = [ + AgentTraceCursors { + messages: invalid, + ..base + }, + AgentTraceCursors { + parts: invalid, + ..base + }, + AgentTraceCursors { + diff_traces: invalid, + ..base + }, + AgentTraceCursors { + agent_traces: invalid, + ..base + }, + ]; + + for cursors in cases { + let error = cursors + .validate() + .expect_err(&format!("{invalid} should be rejected in {cursors:?}")); + assert!(matches!(error, ControlPlaneError::InvalidResponse(_))); + } + } + } + + #[test] + fn batch_response_deserializes_accepted_and_cursor() { + let response: AgentTraceIngestionBatchResponse = serde_json::from_value(json!({ + "accepted": 3, + "cursor": 13, + })) + .unwrap(); + + assert_eq!( + response, + AgentTraceIngestionBatchResponse { + accepted: 3, + cursor: 13, + } + ); + } + + #[test] + fn batch_request_composes_with_message_export_rows() { + let request = AgentTraceIngestionBatchRequest { + repository_id: "acme-monorepo".to_string(), + source_instance_id: "src-123".to_string(), + stream: IngestionStream::Messages, + expected_cursor: 10, + rows: vec![AgentTraceMessageExportRow { + source_row_id: 11, + session_id: "session-1".to_string(), + message_id: "message-1".to_string(), + role: MessageRole::User, + generated_at_unix_ms: 1_700_000_000_000, + }], + }; + + let value = serde_json::to_value(&request).unwrap(); + assert_eq!(value["repositoryId"], json!("acme-monorepo")); + assert_eq!(value["sourceInstanceId"], json!("src-123")); + assert_eq!(value["stream"], json!("messages")); + assert_eq!(value["expectedCursor"], json!(10)); + assert_eq!(value["rows"][0]["sourceRowId"], json!(11)); + } + + #[test] + fn batch_request_composes_with_part_export_rows() { + let request = AgentTraceIngestionBatchRequest { + repository_id: "acme-monorepo".to_string(), + source_instance_id: "src-123".to_string(), + stream: IngestionStream::Parts, + expected_cursor: 0, + rows: vec![AgentTracePartExportRow { + source_row_id: 1, + session_id: "session-1".to_string(), + message_id: "message-1".to_string(), + part_type: "text".to_string(), + text: "hello".to_string(), + generated_at_unix_ms: 1_700_000_000_000, + }], + }; + + let value = serde_json::to_value(&request).unwrap(); + assert_eq!(value["stream"], json!("parts")); + assert_eq!(value["rows"][0]["type"], json!("text")); + } + + #[test] + fn batch_request_composes_with_diff_trace_export_rows() { + let request = AgentTraceIngestionBatchRequest { + repository_id: "acme-monorepo".to_string(), + source_instance_id: "src-123".to_string(), + stream: IngestionStream::DiffTraces, + expected_cursor: 0, + rows: vec![AgentTraceDiffTraceExportRow { + source_row_id: 1, + session_id: "session-1".to_string(), + time_ms: 1_700_000_000_000, + patch: "diff --git a b".to_string(), + model_id: None, + tool_name: None, + tool_version: None, + payload_type: "patch".to_string(), + }], + }; + + let value = serde_json::to_value(&request).unwrap(); + assert_eq!(value["stream"], json!("diff_traces")); + assert_eq!(value["rows"][0]["payloadType"], json!("patch")); + } + + #[test] + fn batch_request_composes_with_agent_trace_export_rows() { + let request = AgentTraceIngestionBatchRequest { + repository_id: "acme-monorepo".to_string(), + source_instance_id: "src-123".to_string(), + stream: IngestionStream::AgentTraces, + expected_cursor: 0, + rows: vec![AgentTraceAgentTraceExportRow { + source_row_id: 1, + agent_trace_id: "agent-trace-1".to_string(), + commit_id: "abc123".to_string(), + commit_time_ms: 1_700_000_000_000, + trace_json: "{}".to_string(), + url: "https://example.com/trace".to_string(), + remote_url: None, + }], + }; + + let value = serde_json::to_value(&request).unwrap(); + assert_eq!(value["stream"], json!("agent_traces")); + assert_eq!(value["rows"][0]["agentTraceId"], json!("agent-trace-1")); + } + + fn block_on(future: F) -> F::Output { + tokio::runtime::Builder::new_current_thread() + .enable_io() + .enable_time() + .build() + .expect("build test tokio runtime") + .block_on(future) + } + + fn now_unix_seconds() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("system clock before unix epoch") + .as_secs() + } + + fn valid_stored_tokens(access_token: &str) -> StoredTokens { + StoredTokens { + access_token: access_token.to_string(), + token_type: "Bearer".to_string(), + expires_in: 3_600, + refresh_token: "refresh-token-1".to_string(), + scope: None, + stored_at_unix_seconds: now_unix_seconds(), + } + } + + fn expired_stored_tokens(access_token: &str) -> StoredTokens { + StoredTokens { + access_token: access_token.to_string(), + token_type: "Bearer".to_string(), + expires_in: 1, + refresh_token: "refresh-token-1".to_string(), + scope: None, + stored_at_unix_seconds: 0, + } + } + + fn token_response_json(access_token: &str) -> serde_json::Value { + json!({ + "access_token": access_token, + "token_type": "Bearer", + "expires_in": 3_600, + "refresh_token": "refresh-token-2", + }) + } + + fn state_response_json() -> serde_json::Value { + json!({ + "cursors": { + "messages": 0, + "parts": 0, + "diffTraces": 0, + "agentTraces": 0, + } + }) + } + + fn sample_state_request() -> AgentTraceIngestionStateRequest { + AgentTraceIngestionStateRequest { + repository_id: "acme-monorepo".to_string(), + source_instance_id: "src-123".to_string(), + } + } + + fn sample_batch_request() -> AgentTraceIngestionBatchRequest { + AgentTraceIngestionBatchRequest { + repository_id: "acme-monorepo".to_string(), + source_instance_id: "src-123".to_string(), + stream: IngestionStream::Messages, + expected_cursor: 0, + rows: vec![AgentTraceMessageExportRow { + source_row_id: 1, + session_id: "session-1".to_string(), + message_id: "message-1".to_string(), + role: MessageRole::User, + generated_at_unix_ms: 1_700_000_000_000, + }], + } + } + + #[derive(Clone, Default)] + struct FakeCredentialStore { + tokens: Arc>>, + save_calls: Arc>>, + } + + impl FakeCredentialStore { + fn empty() -> Self { + Self::default() + } + + fn with_tokens(tokens: StoredTokens) -> Self { + let store = Self::default(); + *store.tokens.lock().unwrap() = Some(tokens); + store + } + + fn save_call_count(&self) -> usize { + self.save_calls.lock().unwrap().len() + } + } + + impl CredentialStore for FakeCredentialStore { + fn load(&self) -> Result, ControlPlaneError> { + Ok(self.tokens.lock().unwrap().clone()) + } + + fn save(&self, token: &TokenResponse) -> Result { + self.save_calls.lock().unwrap().push(token.clone()); + let stored = StoredTokens { + access_token: token.access_token.clone(), + token_type: token.token_type.clone(), + expires_in: token.expires_in, + refresh_token: token.refresh_token.clone(), + scope: token.scope.clone(), + stored_at_unix_seconds: now_unix_seconds(), + }; + *self.tokens.lock().unwrap() = Some(stored.clone()); + Ok(stored) + } + } + + struct RuntimeCheckingCredentialStore; + + impl CredentialStore for RuntimeCheckingCredentialStore { + fn load(&self) -> Result, ControlPlaneError> { + // This would panic if the blocking credential operation ran on a + // Tokio runtime thread. + let runtime = tokio::runtime::Builder::new_current_thread() + .build() + .expect("build nested test runtime"); + runtime.block_on(async {}); + Ok(Some(valid_stored_tokens("valid-access-token"))) + } + + fn save(&self, _token: &TokenResponse) -> Result { + panic!("no token refresh expected in this test"); + } + } + + #[test] + fn credential_store_operations_run_outside_the_async_runtime() { + let client = AuthenticatedControlPlaneClient::with_credential_store( + test_http_client(), + "http://127.0.0.1:1", + "http://127.0.0.1:1", + "test-client-id", + Box::new(RuntimeCheckingCredentialStore), + ); + + let access_token = block_on(client.resolve_access_token()).expect("token should load"); + + assert_eq!(access_token, "valid-access-token"); + } + + /// A plain-`http://` loopback test double never needs TLS root + /// certificates. Skip platform certificate verification so the test + /// suite also passes in sandboxes with no system CA store. + fn test_http_client() -> reqwest::Client { + reqwest::Client::builder() + .danger_accept_invalid_certs(true) + .build() + .expect("build test reqwest client") + } + + fn client_with( + server: &TestHttpServer, + credential_store: FakeCredentialStore, + ) -> AuthenticatedControlPlaneClient { + AuthenticatedControlPlaneClient::with_credential_store( + test_http_client(), + server.base_url.clone(), + server.base_url.clone(), + "test-client-id", + Box::new(credential_store), + ) + } + + #[test] + fn missing_credentials_fail_before_any_http_call() { + let server = TestHttpServer::start(); + let client = client_with(&server, FakeCredentialStore::empty()); + + let error = block_on(client.ingestion_state(&sample_state_request())).unwrap_err(); + + assert!(matches!(error, ControlPlaneError::MissingCredentials)); + assert!(error.to_string().contains("sce auth login")); + assert_eq!(server.call_count(), 0); + } + + #[test] + fn valid_token_is_reused_without_resave() { + let server = TestHttpServer::start(); + server.queue_response(CannedResponse::json(200, &state_response_json())); + let store = FakeCredentialStore::with_tokens(valid_stored_tokens("valid-access-token")); + let client = client_with(&server, store); + + let response = block_on(client.ingestion_state(&sample_state_request())).unwrap(); + + assert_eq!(response.cursors.messages, 0); + assert_eq!(server.call_count(), 1); + let requests = server.captured_requests(); + assert_eq!( + requests[0].headers.get("authorization").map(String::as_str), + Some("Bearer valid-access-token") + ); + } + + #[test] + fn valid_token_is_not_resaved() { + let server = TestHttpServer::start(); + server.queue_response(CannedResponse::json(200, &state_response_json())); + let store = FakeCredentialStore::with_tokens(valid_stored_tokens("valid-access-token")); + let store_handle = store.clone(); + let client = client_with(&server, store); + + block_on(client.ingestion_state(&sample_state_request())).unwrap(); + + assert_eq!(store_handle.save_call_count(), 0); + } + + #[test] + fn expired_token_is_refreshed_and_saved() { + let server = TestHttpServer::start(); + server.queue_response(CannedResponse::json( + 200, + &token_response_json("refreshed-token"), + )); + server.queue_response(CannedResponse::json(200, &state_response_json())); + let store = FakeCredentialStore::with_tokens(expired_stored_tokens("stale-access-token")); + let store_handle = store.clone(); + let client = client_with(&server, store); + + let response = block_on(client.ingestion_state(&sample_state_request())).unwrap(); + + assert_eq!(response.cursors.messages, 0); + assert_eq!(server.call_count(), 2); + assert_eq!(store_handle.save_call_count(), 1); + let requests = server.captured_requests(); + assert_eq!( + requests[1].headers.get("authorization").map(String::as_str), + Some("Bearer refreshed-token") + ); + } + + #[test] + fn unexpected_401_refreshes_once_and_retries_once_on_success() { + let server = TestHttpServer::start(); + server.queue_response(CannedResponse::json(401, &json!({"error": "unauthorized"}))); + server.queue_response(CannedResponse::json( + 200, + &token_response_json("refreshed-token"), + )); + server.queue_response(CannedResponse::json(200, &state_response_json())); + let store = FakeCredentialStore::with_tokens(valid_stored_tokens("stale-but-unexpired")); + let store_handle = store.clone(); + let client = client_with(&server, store); + + let response = block_on(client.ingestion_state(&sample_state_request())).unwrap(); + + assert_eq!(response.cursors.messages, 0); + assert_eq!(server.call_count(), 3); + assert_eq!(store_handle.save_call_count(), 1); + let requests = server.captured_requests(); + assert_eq!( + requests[2].headers.get("authorization").map(String::as_str), + Some("Bearer refreshed-token") + ); + } + + #[test] + fn unexpected_401_twice_fails_without_a_third_attempt() { + let server = TestHttpServer::start(); + server.queue_response(CannedResponse::json(401, &json!({"error": "unauthorized"}))); + server.queue_response(CannedResponse::json( + 200, + &token_response_json("refreshed-token"), + )); + server.queue_response(CannedResponse::json(401, &json!({"error": "unauthorized"}))); + let store = FakeCredentialStore::with_tokens(valid_stored_tokens("stale-but-unexpired")); + let client = client_with(&server, store); + + let error = block_on(client.ingestion_state(&sample_state_request())).unwrap_err(); + + assert!(matches!(error, ControlPlaneError::AuthenticationFailed(_))); + assert_eq!(server.call_count(), 3); + } + + #[test] + fn state_request_body_has_exact_shape_and_call_count() { + let server = TestHttpServer::start(); + server.queue_response(CannedResponse::json(200, &state_response_json())); + let store = FakeCredentialStore::with_tokens(valid_stored_tokens("valid-access-token")); + let client = client_with(&server, store); + + block_on(client.ingestion_state(&sample_state_request())).unwrap(); + + assert_eq!(server.call_count(), 1); + let requests = server.captured_requests(); + assert_eq!(requests[0].path, "/agent-trace/ingestion/state"); + let body: serde_json::Value = serde_json::from_str(&requests[0].body).unwrap(); + assert_eq!( + body, + json!({ + "repositoryId": "acme-monorepo", + "sourceInstanceId": "src-123", + }) + ); + } + + #[test] + fn batch_classifies_400_as_bad_request() { + let server = TestHttpServer::start(); + server.queue_response(CannedResponse::json(400, &json!({"error": "invalid"}))); + let store = FakeCredentialStore::with_tokens(valid_stored_tokens("valid-access-token")); + let client = client_with(&server, store); + + let error = block_on(client.ingest_messages(&sample_batch_request())).unwrap_err(); + + assert!(matches!(error, ControlPlaneError::BadRequest(_))); + } + + #[test] + fn batch_classifies_403_as_forbidden() { + let server = TestHttpServer::start(); + server.queue_response(CannedResponse::json(403, &json!({"error": "forbidden"}))); + let store = FakeCredentialStore::with_tokens(valid_stored_tokens("valid-access-token")); + let client = client_with(&server, store); + + let error = block_on(client.ingest_messages(&sample_batch_request())).unwrap_err(); + + assert!(matches!(error, ControlPlaneError::Forbidden(_))); + } + + #[test] + fn batch_classifies_409_as_conflict() { + let server = TestHttpServer::start(); + server.queue_response(CannedResponse::json(409, &json!({"error": "conflict"}))); + let store = FakeCredentialStore::with_tokens(valid_stored_tokens("valid-access-token")); + let client = client_with(&server, store); + + let error = block_on(client.ingest_messages(&sample_batch_request())).unwrap_err(); + + assert!(matches!(error, ControlPlaneError::Conflict(_))); + } + + #[test] + fn batch_classifies_5xx_as_server_error() { + let server = TestHttpServer::start(); + server.queue_response(CannedResponse::json(500, &json!({"error": "boom"}))); + let store = FakeCredentialStore::with_tokens(valid_stored_tokens("valid-access-token")); + let client = client_with(&server, store); + + let error = block_on(client.ingest_messages(&sample_batch_request())).unwrap_err(); + + assert!(matches!(error, ControlPlaneError::ServerError(_))); + } + + #[test] + fn server_error_body_is_not_leaked() { + let server = TestHttpServer::start(); + server.queue_response(CannedResponse::text( + 500, + "SQLITE_ERROR: no such table: agent_trace_messages at /var/lib/sce/db.sqlite3", + )); + let store = FakeCredentialStore::with_tokens(valid_stored_tokens("valid-access-token")); + let client = client_with(&server, store); + + let error = block_on(client.ingest_messages(&sample_batch_request())).unwrap_err(); + + let rendered = error.to_string(); + assert!(matches!(error, ControlPlaneError::ServerError(_))); + assert!(!rendered.contains("SQLITE_ERROR")); + assert!(!rendered.contains("agent_trace_messages")); + assert!(!rendered.contains("/var/lib/sce")); + assert!(rendered.contains("control plane encountered an internal error")); + } + + #[test] + fn known_safe_error_payload_is_surfaced() { + let server = TestHttpServer::start(); + server.queue_response(CannedResponse::json( + 400, + &json!({"message": "repositoryId is required"}), + )); + let store = FakeCredentialStore::with_tokens(valid_stored_tokens("valid-access-token")); + let client = client_with(&server, store); + + let error = block_on(client.ingest_messages(&sample_batch_request())).unwrap_err(); + + match error { + ControlPlaneError::BadRequest(message) => { + assert_eq!(message, "repositoryId is required"); + } + other => panic!("expected BadRequest, got {other:?}"), + } + } + + #[test] + fn html_error_body_falls_back_to_generic_message() { + let server = TestHttpServer::start(); + server.queue_response(CannedResponse::text( + 500, + "

500 Internal Server Error

", + )); + let store = FakeCredentialStore::with_tokens(valid_stored_tokens("valid-access-token")); + let client = client_with(&server, store); + + let error = block_on(client.ingest_messages(&sample_batch_request())).unwrap_err(); + + let rendered = error.to_string(); + assert!(!rendered.contains("")); + assert!(rendered.contains("control plane encountered an internal error")); + } + + #[test] + fn batch_classifies_404_as_protocol_error() { + let server = TestHttpServer::start(); + server.queue_response(CannedResponse::json(404, &json!({"error": "not found"}))); + let store = FakeCredentialStore::with_tokens(valid_stored_tokens("valid-access-token")); + let client = client_with(&server, store); + + let error = block_on(client.ingest_messages(&sample_batch_request())).unwrap_err(); + + match error { + ControlPlaneError::Protocol { status, message } => { + assert_eq!(status, StatusCode::NOT_FOUND); + assert_eq!(message, "not found"); + } + other => panic!("expected Protocol, got {other:?}"), + } + } +} diff --git a/cli/src/services/agent_trace_sync/mod.rs b/cli/src/services/agent_trace_sync/mod.rs new file mode 100644 index 00000000..bd89534c --- /dev/null +++ b/cli/src/services/agent_trace_sync/mod.rs @@ -0,0 +1,486 @@ +//! Synchronization of a repository's local Agent Trace capture database with +//! the control-plane Agent Trace ingestion API. + +pub mod control_plane; + +#[cfg(test)] +pub(crate) mod test_http_server; + +use std::fmt; + +use crate::services::agent_trace_export::{ + AgentTraceAgentTraceExportRow, AgentTraceDiffTraceExportRow, AgentTraceMessageExportRow, + AgentTracePartExportRow, +}; + +/// Bound on consecutive `409`/ambiguous-batch-failure reconciliation attempts +/// for one stream, matching the order of magnitude of existing retry +/// constants (`TOKEN_REFRESH_MAX_ATTEMPTS = 3` in `auth.rs`). Exhausting it +/// fails the stream rather than looping unboundedly. +pub const RECONCILIATION_MAX_ATTEMPTS: u32 = 5; + +/// Exposes the `source_row_id` each of the four PR #198 export row types +/// carries, so the sync engine can validate and advance cursors generically +/// without a per-stream copy of the same logic. +pub trait AgentTraceExportRow { + fn source_row_id(&self) -> i64; +} + +impl AgentTraceExportRow for AgentTraceMessageExportRow { + fn source_row_id(&self) -> i64 { + self.source_row_id + } +} + +impl AgentTraceExportRow for AgentTracePartExportRow { + fn source_row_id(&self) -> i64 { + self.source_row_id + } +} + +impl AgentTraceExportRow for AgentTraceDiffTraceExportRow { + fn source_row_id(&self) -> i64 { + self.source_row_id + } +} + +impl AgentTraceExportRow for AgentTraceAgentTraceExportRow { + fn source_row_id(&self) -> i64 { + self.source_row_id + } +} + +/// Result of one batch-ingest attempt, as classified by the caller-supplied +/// ingest closure. `Conflict` and `Ambiguous` carry no data: reconciliation +/// always re-derives truth from a fresh `/state` call rather than trusting +/// anything about the failed attempt itself. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BatchAttemptOutcome { + /// The batch was accepted. `accepted` and `cursor` are the server + /// response's own fields, validated by the engine before the stream + /// cursor advances. + Accepted { accepted: usize, cursor: i64 }, + /// The server rejected the batch with a cursor conflict (`409`). + Conflict, + /// The batch outcome could not be determined (`5xx`, a transport + /// failure, or an invalid response). + Ambiguous, +} + +/// Terminal failure of [`sync_stream`]. +#[derive(Debug)] +pub enum StreamSyncError { + /// The local-row reader closure failed. + Read(String), + /// The `/state`-refresh closure failed. + Refresh(String), + /// A syntactically successful batch response did not match the rows + /// that were sent (`accepted != rows.len()` or + /// `cursor != rows.last().source_row_id()`). + InvalidResponse(String), + /// The reconciliation loop exceeded [`RECONCILIATION_MAX_ATTEMPTS`] + /// without converging. + DidNotConverge, +} + +impl fmt::Display for StreamSyncError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Read(reason) => write!(f, "failed to read local rows: {reason}"), + Self::Refresh(reason) => write!(f, "failed to refresh authoritative cursor: {reason}"), + Self::InvalidResponse(reason) => { + write!(f, "control-plane batch response did not match the sent rows: {reason}") + } + Self::DidNotConverge => write!( + f, + "stream did not converge after {RECONCILIATION_MAX_ATTEMPTS} reconciliation attempts" + ), + } + } +} + +impl std::error::Error for StreamSyncError {} + +/// Outcome of a fully converged [`sync_stream`] run for one stream. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct StreamSyncOutcome { + pub uploaded: usize, + pub initial_cursor: i64, + pub final_cursor: i64, + pub batches: usize, +} + +/// Synchronizes one Agent Trace capture stream: reads local rows after +/// `cursor` via `read_after`, uploads them in batches bounded by +/// `batch_limit` via `ingest_batch`, and advances the cursor only from a +/// validated server response. Never infers the next cursor from +/// `cursor + rows.len()`; it always uses the server-reported `cursor` +/// (or, on reconciliation, the freshly fetched `/state` cursor). +/// +/// On `Conflict` or `Ambiguous`, calls `refresh_cursor` and resumes from the +/// refreshed value: if it advanced, the next read naturally skips the +/// already-accepted rows; if unchanged, the same rows are re-read and +/// resent. Both cases share one bounded reconciliation counter. +pub fn sync_stream( + initial_cursor: i64, + batch_limit: usize, + mut read_after: ReadFn, + mut ingest_batch: IngestFn, + mut refresh_cursor: RefreshFn, +) -> Result +where + T: AgentTraceExportRow, + ReadFn: FnMut(i64, usize) -> Result, StreamSyncError>, + IngestFn: FnMut(i64, &[T]) -> BatchAttemptOutcome, + RefreshFn: FnMut() -> Result, +{ + let mut cursor = initial_cursor; + let mut uploaded = 0usize; + let mut batches = 0usize; + let mut reconciliation_attempts = 0u32; + + loop { + let rows = read_after(cursor, batch_limit)?; + if rows.is_empty() { + break; + } + + match ingest_batch(cursor, &rows) { + BatchAttemptOutcome::Accepted { + accepted, + cursor: reported_cursor, + } => { + let last_row_id = rows + .last() + .expect("rows checked non-empty above") + .source_row_id(); + + if accepted != rows.len() || reported_cursor != last_row_id { + return Err(StreamSyncError::InvalidResponse(format!( + "sent {} rows up to source_row_id {last_row_id}, server reported accepted={accepted} cursor={reported_cursor}", + rows.len() + ))); + } + + cursor = reported_cursor; + uploaded += rows.len(); + batches += 1; + reconciliation_attempts = 0; + } + BatchAttemptOutcome::Conflict | BatchAttemptOutcome::Ambiguous => { + reconciliation_attempts += 1; + if reconciliation_attempts > RECONCILIATION_MAX_ATTEMPTS { + return Err(StreamSyncError::DidNotConverge); + } + + cursor = refresh_cursor()?; + } + } + } + + Ok(StreamSyncOutcome { + uploaded, + initial_cursor, + final_cursor: cursor, + batches, + }) +} + +#[cfg(test)] +mod tests { + use std::cell::RefCell; + + use super::*; + use crate::services::agent_trace_db::MessageRole; + + fn row(source_row_id: i64) -> AgentTraceMessageExportRow { + AgentTraceMessageExportRow { + source_row_id, + session_id: "session-1".to_string(), + message_id: format!("message-{source_row_id}"), + role: MessageRole::User, + generated_at_unix_ms: 1_700_000_000_000, + } + } + + /// In-memory local store: rows never disappear, only the caller-visible + /// server-accepted cursor advances, mirroring how the real reader and + /// control plane are independent sources of truth. + struct FakeLocalRows { + rows: Vec, + } + + impl FakeLocalRows { + fn new(count: i64) -> Self { + Self { + rows: (1..=count).map(row).collect(), + } + } + + fn after(&self, cursor: i64, limit: usize) -> Vec { + self.rows + .iter() + .filter(|row| row.source_row_id > cursor) + .take(limit) + .cloned() + .collect() + } + } + + #[test] + fn empty_database_makes_no_batch_calls() { + let local = FakeLocalRows::new(0); + let batch_calls = RefCell::new(0usize); + + let outcome = sync_stream( + 0, + 500, + |cursor, limit| Ok(local.after(cursor, limit)), + |_cursor, rows: &[AgentTraceMessageExportRow]| { + *batch_calls.borrow_mut() += 1; + BatchAttemptOutcome::Accepted { + accepted: rows.len(), + cursor: rows.last().map_or(0, |row| row.source_row_id), + } + }, + || panic!("refresh should not be called when there is nothing to reconcile"), + ) + .unwrap(); + + assert_eq!(*batch_calls.borrow(), 0); + assert_eq!( + outcome, + StreamSyncOutcome { + uploaded: 0, + initial_cursor: 0, + final_cursor: 0, + batches: 0, + } + ); + } + + #[test] + fn one_batch_uploads_all_rows_and_advances_cursor() { + let local = FakeLocalRows::new(3); + let batch_calls = RefCell::new(0usize); + + let outcome = sync_stream( + 0, + 500, + |cursor, limit| Ok(local.after(cursor, limit)), + |_cursor, rows: &[AgentTraceMessageExportRow]| { + *batch_calls.borrow_mut() += 1; + BatchAttemptOutcome::Accepted { + accepted: rows.len(), + cursor: rows.last().unwrap().source_row_id, + } + }, + || panic!("refresh should not be called on an all-success run"), + ) + .unwrap(); + + assert_eq!(*batch_calls.borrow(), 1); + assert_eq!( + outcome, + StreamSyncOutcome { + uploaded: 3, + initial_cursor: 0, + final_cursor: 3, + batches: 1, + } + ); + } + + #[test] + fn more_than_five_hundred_rows_span_multiple_bounded_batches() { + let local = FakeLocalRows::new(1_100); + let batch_sizes = RefCell::new(Vec::new()); + + let outcome = sync_stream( + 0, + 500, + |cursor, limit| { + assert!(limit <= 500); + Ok(local.after(cursor, limit)) + }, + |_cursor, rows: &[AgentTraceMessageExportRow]| { + batch_sizes.borrow_mut().push(rows.len()); + BatchAttemptOutcome::Accepted { + accepted: rows.len(), + cursor: rows.last().unwrap().source_row_id, + } + }, + || panic!("refresh should not be called on an all-success run"), + ) + .unwrap(); + + assert_eq!(*batch_sizes.borrow(), vec![500, 500, 100]); + assert_eq!(outcome.uploaded, 1_100); + assert_eq!(outcome.final_cursor, 1_100); + assert_eq!(outcome.batches, 3); + } + + #[test] + fn gapped_source_ids_advance_cursor_from_last_id_not_row_count() { + let local = FakeLocalRows { + rows: vec![row(2), row(5), row(9)], + }; + + let outcome = sync_stream( + 0, + 500, + |cursor, limit| Ok(local.after(cursor, limit)), + |_cursor, rows: &[AgentTraceMessageExportRow]| BatchAttemptOutcome::Accepted { + accepted: rows.len(), + cursor: rows.last().unwrap().source_row_id, + }, + || panic!("refresh should not be called on an all-success run"), + ) + .unwrap(); + + assert_eq!(outcome.uploaded, 3); + assert_eq!(outcome.final_cursor, 9); + let uploaded_as_cursor_delta = + i64::try_from(outcome.uploaded).expect("uploaded count fits in i64 for this test"); + assert_ne!( + outcome.final_cursor, + outcome.initial_cursor + uploaded_as_cursor_delta + ); + } + + #[test] + fn conflict_resends_only_the_unsent_tail() { + // Mirrors the plan's worked example: state=10, local rows 11-13, the + // first batch 409s, refreshed state=12, only row 13 is resent with + // expected_cursor=12. + let local = FakeLocalRows { + rows: vec![row(11), row(12), row(13)], + }; + let attempts: RefCell)>> = RefCell::new(Vec::new()); + + let outcome = sync_stream( + 10, + 500, + |cursor, limit| Ok(local.after(cursor, limit)), + |cursor, rows: &[AgentTraceMessageExportRow]| { + let ids: Vec = rows.iter().map(|row| row.source_row_id).collect(); + attempts.borrow_mut().push((cursor, ids)); + if attempts.borrow().len() == 1 { + BatchAttemptOutcome::Conflict + } else { + BatchAttemptOutcome::Accepted { + accepted: rows.len(), + cursor: rows.last().unwrap().source_row_id, + } + } + }, + || Ok(12), + ) + .unwrap(); + + let attempts = attempts.into_inner(); + assert_eq!(attempts, vec![(10, vec![11, 12, 13]), (12, vec![13])]); + assert_eq!(outcome.uploaded, 1); + assert_eq!(outcome.final_cursor, 13); + } + + #[test] + fn ambiguous_failure_with_advanced_refresh_does_not_resend() { + let local = FakeLocalRows::new(3); + let attempts: RefCell>> = RefCell::new(Vec::new()); + + let outcome = sync_stream( + 0, + 500, + |cursor, limit| Ok(local.after(cursor, limit)), + |_cursor, rows: &[AgentTraceMessageExportRow]| { + let ids: Vec = rows.iter().map(|row| row.source_row_id).collect(); + let is_first = attempts.borrow().is_empty(); + attempts.borrow_mut().push(ids); + if is_first { + BatchAttemptOutcome::Ambiguous + } else { + BatchAttemptOutcome::Accepted { + accepted: rows.len(), + cursor: rows.last().unwrap().source_row_id, + } + } + }, + // The control plane had already committed the batch server-side + // despite the ambiguous response; /state now reflects it. + || Ok(3), + ) + .unwrap(); + + let attempts = attempts.into_inner(); + assert_eq!(attempts, vec![vec![1, 2, 3]]); + assert_eq!(outcome.uploaded, 0); + assert_eq!(outcome.final_cursor, 3); + } + + #[test] + fn ambiguous_failure_with_unchanged_refresh_resends_once() { + let local = FakeLocalRows::new(3); + let attempts: RefCell>> = RefCell::new(Vec::new()); + + let outcome = sync_stream( + 0, + 500, + |cursor, limit| Ok(local.after(cursor, limit)), + |_cursor, rows: &[AgentTraceMessageExportRow]| { + let ids: Vec = rows.iter().map(|row| row.source_row_id).collect(); + let is_first = attempts.borrow().is_empty(); + attempts.borrow_mut().push(ids); + if is_first { + BatchAttemptOutcome::Ambiguous + } else { + BatchAttemptOutcome::Accepted { + accepted: rows.len(), + cursor: rows.last().unwrap().source_row_id, + } + } + }, + // /state confirms nothing was actually committed. + || Ok(0), + ) + .unwrap(); + + let attempts = attempts.into_inner(); + assert_eq!(attempts, vec![vec![1, 2, 3], vec![1, 2, 3]]); + assert_eq!(outcome.uploaded, 3); + assert_eq!(outcome.final_cursor, 3); + } + + #[test] + fn reconciliation_bound_fails_with_did_not_converge() { + let local = FakeLocalRows::new(3); + + let result = sync_stream( + 0, + 500, + |cursor, limit| Ok(local.after(cursor, limit)), + |_cursor, _rows: &[AgentTraceMessageExportRow]| BatchAttemptOutcome::Ambiguous, + || Ok(0), + ); + + assert!(matches!(result, Err(StreamSyncError::DidNotConverge))); + } + + #[test] + fn invalid_response_rejects_mismatched_accepted_and_cursor() { + let local = FakeLocalRows::new(3); + + let result = sync_stream( + 0, + 500, + |cursor, limit| Ok(local.after(cursor, limit)), + |_cursor, _rows: &[AgentTraceMessageExportRow]| BatchAttemptOutcome::Accepted { + accepted: 2, + cursor: 99, + }, + || panic!("refresh should not be called on an invalid response"), + ); + + assert!(matches!(result, Err(StreamSyncError::InvalidResponse(_)))); + } +} diff --git a/cli/src/services/agent_trace_sync/test_http_server.rs b/cli/src/services/agent_trace_sync/test_http_server.rs new file mode 100644 index 00000000..b2a96b09 --- /dev/null +++ b/cli/src/services/agent_trace_sync/test_http_server.rs @@ -0,0 +1,185 @@ +//! A bespoke, in-repo, test-only HTTP/1.1 server used to exercise +//! `AuthenticatedControlPlaneClient` (and later the sync engine and CLI +//! wiring) against canned responses and captured requests, without adding a +//! `wiremock`/`httptest`-style dev-dependency to a crate that currently +//! declares none. + +use std::collections::{HashMap, VecDeque}; +use std::io::{BufRead, BufReader, Read, Write}; +use std::net::TcpListener; +use std::sync::{Arc, Mutex}; +use std::thread; + +#[derive(Clone, Debug)] +pub struct CapturedRequest { + pub method: String, + pub path: String, + pub headers: HashMap, + pub body: String, +} + +#[derive(Clone, Debug)] +pub struct CannedResponse { + pub status: u16, + pub body: String, +} + +impl CannedResponse { + pub fn json(status: u16, body: &serde_json::Value) -> Self { + Self { + status, + body: body.to_string(), + } + } + + pub fn text(status: u16, body: impl Into) -> Self { + Self { + status, + body: body.into(), + } + } +} + +/// A single-threaded, sequential test HTTP server: each accepted connection +/// is answered with the next queued [`CannedResponse`] (or a `500` marker +/// response when the queue is empty), and every parsed request is recorded +/// for assertions. +pub struct TestHttpServer { + pub base_url: String, + requests: Arc>>, + responses: Arc>>, +} + +impl TestHttpServer { + pub fn start() -> Self { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind test http server"); + let addr = listener.local_addr().expect("read test http server addr"); + + let requests: Arc>> = Arc::new(Mutex::new(Vec::new())); + let responses: Arc>> = Arc::new(Mutex::new(VecDeque::new())); + + let thread_requests = Arc::clone(&requests); + let thread_responses = Arc::clone(&responses); + thread::spawn(move || { + for stream in listener.incoming() { + let Ok(stream) = stream else { + continue; + }; + handle_connection(stream, &thread_requests, &thread_responses); + } + }); + + Self { + base_url: format!("http://{addr}"), + requests, + responses, + } + } + + pub fn queue_response(&self, response: CannedResponse) { + self.responses + .lock() + .expect("test http server responses lock") + .push_back(response); + } + + pub fn captured_requests(&self) -> Vec { + self.requests + .lock() + .expect("test http server requests lock") + .clone() + } + + pub fn call_count(&self) -> usize { + self.requests + .lock() + .expect("test http server requests lock") + .len() + } +} + +fn handle_connection( + stream: std::net::TcpStream, + requests: &Arc>>, + responses: &Arc>>, +) { + let mut reader = BufReader::new(stream.try_clone().expect("clone test http stream")); + + let mut request_line = String::new(); + if reader.read_line(&mut request_line).unwrap_or(0) == 0 { + return; + } + let mut parts = request_line.split_whitespace(); + let method = parts.next().unwrap_or_default().to_string(); + let path = parts.next().unwrap_or_default().to_string(); + + let mut headers = HashMap::new(); + let mut content_length = 0usize; + loop { + let mut line = String::new(); + if reader.read_line(&mut line).unwrap_or(0) == 0 { + break; + } + let trimmed = line.trim_end_matches(['\r', '\n']); + if trimmed.is_empty() { + break; + } + if let Some((name, value)) = trimmed.split_once(':') { + let name = name.trim().to_ascii_lowercase(); + let value = value.trim().to_string(); + if name == "content-length" { + content_length = value.parse().unwrap_or(0); + } + headers.insert(name, value); + } + } + + let mut body_bytes = vec![0u8; content_length]; + if content_length > 0 && reader.read_exact(&mut body_bytes).is_err() { + return; + } + let body = String::from_utf8_lossy(&body_bytes).to_string(); + + requests + .lock() + .expect("test http server requests lock") + .push(CapturedRequest { + method, + path, + headers, + body, + }); + + let canned = responses + .lock() + .expect("test http server responses lock") + .pop_front() + .unwrap_or_else(|| CannedResponse { + status: 500, + body: r#"{"error":"no canned response queued"}"#.to_string(), + }); + + let mut stream = stream; + let response = format!( + "HTTP/1.1 {} {}\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", + canned.status, + status_reason(canned.status), + canned.body.len(), + canned.body, + ); + let _ = stream.write_all(response.as_bytes()); + let _ = stream.flush(); +} + +fn status_reason(status: u16) -> &'static str { + match status { + 200 => "OK", + 400 => "Bad Request", + 401 => "Unauthorized", + 403 => "Forbidden", + 409 => "Conflict", + 500 => "Internal Server Error", + 503 => "Service Unavailable", + _ => "Unknown", + } +} diff --git a/cli/src/services/config/render.rs b/cli/src/services/config/render.rs index c947074d..a9757683 100644 --- a/cli/src/services/config/render.rs +++ b/cli/src/services/config/render.rs @@ -4,7 +4,8 @@ use crate::services::style; use super::policy::{format_bash_policies_json, format_bash_policies_text}; use super::resolver::{ - AuthConfigKeySpec, RuntimeConfig, PRECEDENCE_DESCRIPTION, WORKOS_CLIENT_ID_KEY, + AuthConfigKeySpec, RuntimeConfig, CONTROL_PLANE_BASE_URL_KEY, PRECEDENCE_DESCRIPTION, + WORKOS_CLIENT_ID_KEY, }; use super::types::DatabaseRetryConfig; use super::{ConfigPathSource, ReportFormat, ResolvedOptionalValue, ValueSource}; @@ -34,6 +35,10 @@ pub(super) fn format_show_output(runtime: &RuntimeConfig, report_format: ReportF WORKOS_CLIENT_ID_KEY, &runtime.workos_client_id, ), + format_optional_auth_resolved_value_text( + CONTROL_PLANE_BASE_URL_KEY, + &runtime.control_plane_base_url, + ), format_optional_resolved_value_text( "agent_trace.repository_id", &runtime.agent_trace_repository_id, @@ -77,6 +82,7 @@ pub(super) fn format_show_output(runtime: &RuntimeConfig, report_format: ReportF "config_source": runtime.timeout_ms.source.config_source().map(ConfigPathSource::as_str), }, "workos_client_id": format_optional_auth_resolved_value_json(WORKOS_CLIENT_ID_KEY, &runtime.workos_client_id), + "control_plane_base_url": format_optional_auth_resolved_value_json(CONTROL_PLANE_BASE_URL_KEY, &runtime.control_plane_base_url), "agent_trace": { "repository_id": format_optional_resolved_value_json(&runtime.agent_trace_repository_id), "repository_remote": format_resolved_value_json( diff --git a/cli/src/services/config/resolver.rs b/cli/src/services/config/resolver.rs index f7b0c2e9..05c07b57 100644 --- a/cli/src/services/config/resolver.rs +++ b/cli/src/services/config/resolver.rs @@ -34,6 +34,15 @@ pub(crate) const WORKOS_CLIENT_ID_KEY: AuthConfigKeySpec = AuthConfigKeySpec { baked_default: Some(WORKOS_CLIENT_ID_BAKED_DEFAULT), }; +const CONTROL_PLANE_BASE_URL_ENV: &str = "SCE_CONTROL_PLANE_BASE_URL"; +const CONTROL_PLANE_BASE_URL_BAKED_DEFAULT: &str = "https://sce.crocoder.dev"; + +pub(crate) const CONTROL_PLANE_BASE_URL_KEY: AuthConfigKeySpec = AuthConfigKeySpec { + config_key: "control_plane_base_url", + env_key: CONTROL_PLANE_BASE_URL_ENV, + baked_default: Some(CONTROL_PLANE_BASE_URL_BAKED_DEFAULT), +}; + #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub(crate) struct AuthConfigKeySpec { pub(crate) config_key: &'static str, @@ -66,6 +75,7 @@ pub(super) struct RuntimeConfig { pub(super) timeout_ms: ResolvedValue, pub(super) attribution_hooks_enabled: ResolvedValue, pub(super) workos_client_id: ResolvedOptionalValue, + pub(super) control_plane_base_url: ResolvedOptionalValue, pub(super) agent_trace_repository_id: ResolvedOptionalValue, pub(super) agent_trace_repository_remote: ResolvedValue, pub(super) bash_policies: ResolvedOptionalValue, @@ -190,6 +200,7 @@ where Ok(ResolvedAuthRuntimeConfig { workos_client_id: runtime.workos_client_id, + control_plane_base_url: runtime.control_plane_base_url, }) } @@ -304,6 +315,7 @@ where timeout_ms: None, attribution_hooks_enabled: None, workos_client_id: None, + control_plane_base_url: None, agent_trace_repository_id: None, agent_trace_repository_remote: None, bash_policy_presets: None, @@ -343,6 +355,9 @@ where if let Some(workos_client_id) = layer.workos_client_id { file_config.workos_client_id = Some(workos_client_id); } + if let Some(control_plane_base_url) = layer.control_plane_base_url { + file_config.control_plane_base_url = Some(control_plane_base_url); + } if let Some(agent_trace_repository_id) = layer.agent_trace_repository_id { file_config.agent_trace_repository_id = Some(agent_trace_repository_id); } @@ -479,6 +494,11 @@ where file_config.workos_client_id, &env_lookup, ); + let resolved_control_plane_base_url = resolve_optional_auth_config_value( + CONTROL_PLANE_BASE_URL_KEY, + file_config.control_plane_base_url, + &env_lookup, + ); let resolved_agent_trace_repository_id = ResolvedOptionalValue { value: file_config @@ -520,6 +540,7 @@ where timeout_ms: resolved_timeout_ms, attribution_hooks_enabled: resolved_attribution_hooks_enabled, workos_client_id: resolved_workos_client_id, + control_plane_base_url: resolved_control_plane_base_url, agent_trace_repository_id: resolved_agent_trace_repository_id, agent_trace_repository_remote: resolved_agent_trace_repository_remote, bash_policies: resolved_bash_policies, @@ -748,6 +769,13 @@ mod tests { } fn resolve_runtime_with_config(config: Option<&'static str>) -> Result { + resolve_runtime_with_env_and_config(None, config) + } + + fn resolve_runtime_with_env_and_config( + env: Option<(&'static str, &'static str)>, + config: Option<&'static str>, + ) -> Result { let request = if config.is_some() { explicit_config_request() } else { @@ -762,7 +790,7 @@ mod tests { resolve_runtime_config_with( &request, Path::new("/tmp/repo"), - |_| None, + |key| env.and_then(|(env_key, value)| (key == env_key).then_some(value.to_string())), |_| Ok(config.unwrap_or("{}").to_string()), path_exists_fn, || Ok(PathBuf::from("/tmp/missing-global-sce-config.json")), @@ -875,4 +903,53 @@ mod tests { assert!(!resolved.attribution_hooks_enabled); } + + #[test] + fn control_plane_base_url_resolves_to_baked_default() { + let runtime = resolve_runtime_with_config(None).unwrap(); + + assert_eq!( + runtime.control_plane_base_url.value.as_deref(), + Some(CONTROL_PLANE_BASE_URL_BAKED_DEFAULT) + ); + assert_eq!( + runtime.control_plane_base_url.source, + Some(ValueSource::Default) + ); + } + + #[test] + fn control_plane_base_url_resolves_from_config_file_over_default() { + let runtime = resolve_runtime_with_config(Some( + r#"{"control_plane_base_url":"https://control-plane.example.test"}"#, + )) + .unwrap(); + + assert_eq!( + runtime.control_plane_base_url.value.as_deref(), + Some("https://control-plane.example.test") + ); + assert_eq!( + runtime.control_plane_base_url.source, + Some(ValueSource::ConfigFile(ConfigPathSource::Flag)) + ); + } + + #[test] + fn control_plane_base_url_env_overrides_config_file_and_default() { + let runtime = resolve_runtime_with_env_and_config( + Some((CONTROL_PLANE_BASE_URL_ENV, "https://control-plane.env.test")), + Some(r#"{"control_plane_base_url":"https://control-plane.example.test"}"#), + ) + .unwrap(); + + assert_eq!( + runtime.control_plane_base_url.value.as_deref(), + Some("https://control-plane.env.test") + ); + assert_eq!( + runtime.control_plane_base_url.source, + Some(ValueSource::Env) + ); + } } diff --git a/cli/src/services/config/schema.rs b/cli/src/services/config/schema.rs index 0db3f1da..4483bbda 100644 --- a/cli/src/services/config/schema.rs +++ b/cli/src/services/config/schema.rs @@ -38,13 +38,14 @@ pub(crate) const TOP_LEVEL_CONFIG_KEYS: &[&str] = &[ "log_file_retention_limit", "timeout_ms", super::resolver::WORKOS_CLIENT_ID_KEY.config_key, + super::resolver::CONTROL_PLANE_BASE_URL_KEY.config_key, "agent_trace", "policies", "integrations", ]; pub(crate) const TOP_LEVEL_CONFIG_KEYS_DESCRIPTION: &str = - "$schema, log_level, log_format, timeout_ms, workos_client_id, agent_trace, policies, integrations, log_dir, log_file_retention_limit"; + "$schema, log_level, log_format, timeout_ms, workos_client_id, control_plane_base_url, agent_trace, policies, integrations, log_dir, log_file_retention_limit"; static CONFIG_SCHEMA_VALIDATOR: OnceLock = OnceLock::new(); @@ -74,6 +75,7 @@ pub(crate) struct ParsedFileConfigDocument { pub(crate) log_file_retention_limit: Option, pub(crate) timeout_ms: Option, pub(crate) workos_client_id: Option, + pub(crate) control_plane_base_url: Option, pub(crate) agent_trace: Option, pub(crate) policies: Option, pub(crate) integrations: Option, @@ -160,6 +162,7 @@ pub(crate) struct FileConfig { pub(crate) timeout_ms: Option>, pub(crate) attribution_hooks_enabled: Option>, pub(crate) workos_client_id: Option>, + pub(crate) control_plane_base_url: Option>, pub(crate) agent_trace_repository_id: Option>, pub(crate) agent_trace_repository_remote: Option>, pub(crate) bash_policy_presets: Option>>, @@ -307,6 +310,9 @@ pub(crate) fn parse_file_config( let workos_client_id = typed .workos_client_id .map(|value| FileConfigValue { value, source }); + let control_plane_base_url = typed + .control_plane_base_url + .map(|value| FileConfigValue { value, source }); let (agent_trace_repository_id, agent_trace_repository_remote) = map_agent_trace_config(typed.agent_trace.as_ref(), object, path, source)?; let (attribution_hooks_enabled, bash_policy_presets, bash_policy_custom, database_retry) = @@ -321,6 +327,7 @@ pub(crate) fn parse_file_config( timeout_ms, attribution_hooks_enabled, workos_client_id, + control_plane_base_url, agent_trace_repository_id, agent_trace_repository_remote, bash_policy_presets, diff --git a/cli/src/services/config/types.rs b/cli/src/services/config/types.rs index 4ab802fa..867c63ad 100644 --- a/cli/src/services/config/types.rs +++ b/cli/src/services/config/types.rs @@ -191,6 +191,7 @@ pub(crate) struct LoadedConfigPath { #[derive(Clone, Debug, Eq, PartialEq)] pub(crate) struct ResolvedAuthRuntimeConfig { pub(crate) workos_client_id: ResolvedOptionalValue, + pub(crate) control_plane_base_url: ResolvedOptionalValue, } #[derive(Clone, Debug, Eq, PartialEq)] diff --git a/cli/src/services/db/mod.rs b/cli/src/services/db/mod.rs index 6561629f..9f08cdee 100644 --- a/cli/src/services/db/mod.rs +++ b/cli/src/services/db/mod.rs @@ -179,6 +179,29 @@ fn build_current_thread_runtime(db_name: &str) -> Result(runtime: &tokio::runtime::Runtime, fut: F) -> T +where + F: std::future::Future + Send, + T: Send, +{ + if tokio::runtime::Handle::try_current().is_ok() { + std::thread::scope(|scope| scope.spawn(|| runtime.block_on(fut)).join()) + .unwrap_or_else(|payload| std::panic::resume_unwind(payload)) + } else { + runtime.block_on(fut) + } +} + fn run_embedded_migrations( conn: &turso::Connection, runtime: &tokio::runtime::Runtime, @@ -203,7 +226,7 @@ fn ensure_migrations_table( runtime: &tokio::runtime::Runtime, db_name: &str, ) -> Result<()> { - runtime.block_on(async { + block_on_isolated(runtime, async { conn.execute(MIGRATIONS_TABLE_SQL, ()) .await .map_err(|e| anyhow::anyhow!("{db_name} migration metadata setup failed: {e}")) @@ -218,7 +241,7 @@ fn is_migration_applied( db_name: &str, id: &str, ) -> Result { - runtime.block_on(async { + block_on_isolated(runtime, async { let mut rows = conn.query(SELECT_MIGRATION_SQL, (id,)).await.map_err(|e| { anyhow::anyhow!("{db_name} migration metadata query failed for {id}: {e}") })?; @@ -236,7 +259,7 @@ fn apply_migration( id: &str, sql: &str, ) -> Result<()> { - runtime.block_on(async { + block_on_isolated(runtime, async { // Migration files may contain multiple statements (the repository // Agent Trace baseline is one multi-statement schema file), so batch // execution is required; `execute` would stop after the first @@ -396,7 +419,7 @@ impl TursoDb { &operation_name, CONNECTION_OPEN_RETRY_HINT, |_| { - runtime.block_on(async { + block_on_isolated(&runtime, async { let path_str = db_path.to_str().ok_or_else(|| { anyhow::anyhow!("invalid UTF-8 in database path: {}", db_path.display()) })?; @@ -441,7 +464,7 @@ impl TursoDb { &operation_name, QUERY_RETRY_HINT, |_| { - self.core.runtime.block_on(async { + block_on_isolated(&self.core.runtime, async { self.core .conn .execute(sql, params.clone()) @@ -472,7 +495,7 @@ impl TursoDb { &operation_name, QUERY_RETRY_HINT, |_| { - self.core.runtime.block_on(async { + block_on_isolated(&self.core.runtime, async { self.core .conn .query(sql, params.clone()) @@ -500,7 +523,7 @@ impl TursoDb { &operation_name, QUERY_RETRY_HINT, |_| { - self.core.runtime.block_on(async { + block_on_isolated(&self.core.runtime, async { let mut rows = self.core .conn @@ -554,7 +577,7 @@ impl TursoDb { &operation_name, QUERY_RETRY_HINT, |_| { - self.core.runtime.block_on(async { + block_on_isolated(&self.core.runtime, async { let mut rows = self.core .conn @@ -701,7 +724,7 @@ impl EncryptedTursoDb { &operation_name, CONNECTION_OPEN_RETRY_HINT, |_| { - runtime.block_on(async { + block_on_isolated(&runtime, async { let path_str = db_path.to_str().ok_or_else(|| { anyhow::anyhow!("invalid UTF-8 in database path: {}", db_path.display()) })?; @@ -759,7 +782,7 @@ impl EncryptedTursoDb { &operation_name, QUERY_RETRY_HINT, |_| { - self.core.runtime.block_on(async { + block_on_isolated(&self.core.runtime, async { self.core .conn .execute(sql, params.clone()) @@ -790,7 +813,7 @@ impl EncryptedTursoDb { &operation_name, QUERY_RETRY_HINT, |_| { - self.core.runtime.block_on(async { + block_on_isolated(&self.core.runtime, async { self.core .conn .query(sql, params.clone()) @@ -821,7 +844,7 @@ impl EncryptedTursoDb { &operation_name, QUERY_RETRY_HINT, |_| { - self.core.runtime.block_on(async { + block_on_isolated(&self.core.runtime, async { let mut rows = self.core .conn diff --git a/cli/src/services/mod.rs b/cli/src/services/mod.rs index 3d814bd3..731a351e 100644 --- a/cli/src/services/mod.rs +++ b/cli/src/services/mod.rs @@ -4,6 +4,8 @@ pub mod agent_trace_db; pub mod agent_trace_export; #[allow(dead_code)] pub mod agent_trace_storage; +#[allow(dead_code)] +pub mod agent_trace_sync; pub mod app_support; pub mod auth; pub mod auth_command; diff --git a/cli/src/services/parse/command_runtime.rs b/cli/src/services/parse/command_runtime.rs index 27ac0353..45496476 100644 --- a/cli/src/services/parse/command_runtime.rs +++ b/cli/src/services/parse/command_runtime.rs @@ -268,6 +268,9 @@ fn convert_trace_subcommand(subcommand: cli_schema::TraceSubcommand) -> RuntimeC cli_schema::TraceSubcommand::Status { all, format } => services::trace::TraceRequest { subcommand: services::trace::TraceSubcommandRequest::Status { all, format }, }, + cli_schema::TraceSubcommand::Sync { format } => services::trace::TraceRequest { + subcommand: services::trace::TraceSubcommandRequest::Sync { format }, + }, }; RuntimeCommand::Trace(services::trace::command::TraceCommand { request }) @@ -511,6 +514,38 @@ mod tests { ); } + #[test] + fn trace_sync_parses_to_trace_sync_request_with_default_text_format() { + let command = parse(&["sce", "trace", "sync"]); + + let RuntimeCommand::Trace(command) = command else { + panic!("expected trace command"); + }; + + assert_eq!( + command.request.subcommand, + services::trace::TraceSubcommandRequest::Sync { + format: services::output_format::OutputFormat::Text, + } + ); + } + + #[test] + fn trace_sync_json_format_parses_to_trace_sync_request() { + let command = parse(&["sce", "trace", "sync", "--format", "json"]); + + let RuntimeCommand::Trace(command) = command else { + panic!("expected trace command"); + }; + + assert_eq!( + command.request.subcommand, + services::trace::TraceSubcommandRequest::Sync { + format: services::output_format::OutputFormat::Json, + } + ); + } + #[test] fn trace_db_help_lists_shell_subcommand() { let command = parse(&["sce", "trace", "db", "--help"]); diff --git a/cli/src/services/trace/command.rs b/cli/src/services/trace/command.rs index 87c97f48..ef0aa378 100644 --- a/cli/src/services/trace/command.rs +++ b/cli/src/services/trace/command.rs @@ -4,9 +4,11 @@ use crate::services::trace::discovery::discover_agent_trace_dbs; use crate::services::trace::render_list; use crate::services::trace::render_status; use crate::services::trace::render_status_all; +use crate::services::trace::render_sync; use crate::services::trace::shell::{run_agent_trace_db_shell, ShellTarget}; use crate::services::trace::status::{resolve_current_status, StatusErrorOrRuntime}; use crate::services::trace::status_all::aggregate_current_status_all; +use crate::services::trace::sync::{run_current_sync, TraceSyncError}; use crate::services::trace::{ resolve_agent_trace_db_identifier, TraceRequest, TraceSubcommandRequest, }; @@ -36,6 +38,11 @@ fn classify_status_error(err: StatusErrorOrRuntime) -> ClassifiedError { } } +#[allow(clippy::needless_pass_by_value)] +fn classify_sync_error(err: TraceSyncError) -> ClassifiedError { + ClassifiedError::runtime(format!("{err}")) +} + impl TraceCommand { pub fn execute(&self, context: &C) -> Result where @@ -94,6 +101,14 @@ impl TraceCommand { render_status::render(&report, *format) .map_err(|error| ClassifiedError::runtime(format!("{error:#}"))) } + TraceSubcommandRequest::Sync { format } => { + let repo_root = current_repo_root(context)?; + + let report = run_current_sync(&repo_root).map_err(classify_sync_error)?; + + render_sync::render(&report, *format) + .map_err(|error| ClassifiedError::runtime(format!("{error:#}"))) + } } } } diff --git a/cli/src/services/trace/mod.rs b/cli/src/services/trace/mod.rs index 9f1b4dfe..babc9fcc 100644 --- a/cli/src/services/trace/mod.rs +++ b/cli/src/services/trace/mod.rs @@ -5,10 +5,12 @@ pub mod discovery; pub mod render_list; pub mod render_status; pub mod render_status_all; +pub mod render_sync; pub mod shell; pub mod stats; pub mod status; pub mod status_all; +pub mod sync; pub const NAME: &str = "trace"; @@ -25,6 +27,7 @@ pub enum TraceSubcommandRequest { DbList { format: OutputFormat }, DbShell { identifier: Option }, Status { all: bool, format: OutputFormat }, + Sync { format: OutputFormat }, } #[derive(Clone, Debug, PartialEq, Eq)] diff --git a/cli/src/services/trace/render_sync.rs b/cli/src/services/trace/render_sync.rs new file mode 100644 index 00000000..398d51c5 --- /dev/null +++ b/cli/src/services/trace/render_sync.rs @@ -0,0 +1,195 @@ +//! Renderers for `sce trace sync` (text and JSON). + +use anyhow::{Context, Result}; +use serde_json::json; + +use crate::services::output_format::OutputFormat; +use crate::services::style; +use crate::services::trace::sync::{AgentTraceSyncReport, StreamSyncReport}; +use crate::services::trace::NAME; + +const HEADING: &str = "Agent Trace sync complete."; + +const COL_STREAM: &str = "Stream"; +const COL_UPLOADED: &str = "Uploaded"; +const COL_FINAL_CURSOR: &str = "Final cursor"; + +pub fn render(report: &AgentTraceSyncReport, format: OutputFormat) -> Result { + match format { + OutputFormat::Text => Ok(render_text(report)), + OutputFormat::Json => render_json(report), + } +} + +fn render_text(report: &AgentTraceSyncReport) -> String { + let mut lines = vec![style::heading(HEADING)]; + lines.push(format!("Repository ID: {}", report.repository_id)); + lines.push(format!("Source instance ID: {}", report.source_instance_id)); + lines.push(String::new()); + + let headers = [COL_STREAM, COL_UPLOADED, COL_FINAL_CURSOR]; + let rows: [[String; 3]; 4] = [ + format_row("messages", &report.streams.messages), + format_row("parts", &report.streams.parts), + format_row("diff_traces", &report.streams.diff_traces), + format_row("agent_traces", &report.streams.agent_traces), + ]; + + let widths: Vec = (0..headers.len()) + .map(|col| { + rows.iter() + .map(|row| row[col].len()) + .max() + .unwrap_or(0) + .max(headers[col].len()) + }) + .collect(); + + lines.push(join_row(&headers.map(str::to_string), &widths)); + for row in &rows { + lines.push(join_row(row, &widths)); + } + + lines.join("\n") +} + +fn join_row(cells: &[String; N], widths: &[usize]) -> String { + cells + .iter() + .enumerate() + .map(|(i, cell)| format!("{cell:>() + .join(" ") + .trim_end() + .to_string() +} + +fn format_row(name: &str, stream: &StreamSyncReport) -> [String; 3] { + [ + name.to_string(), + stream.uploaded.to_string(), + stream.final_cursor.to_string(), + ] +} + +fn render_json(report: &AgentTraceSyncReport) -> Result { + let payload = json!({ + "status": "ok", + "command": NAME, + "subcommand": "sync", + "repositoryId": report.repository_id, + "sourceInstanceId": report.source_instance_id, + "streams": { + "messages": stream_json(&report.streams.messages), + "parts": stream_json(&report.streams.parts), + "diffTraces": stream_json(&report.streams.diff_traces), + "agentTraces": stream_json(&report.streams.agent_traces), + }, + }); + + serde_json::to_string_pretty(&payload).context("failed to serialize trace sync report to JSON") +} + +fn stream_json(stream: &StreamSyncReport) -> serde_json::Value { + json!({ + "uploaded": stream.uploaded, + "initialCursor": stream.initial_cursor, + "finalCursor": stream.final_cursor, + "batches": stream.batches, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn sample_report() -> AgentTraceSyncReport { + AgentTraceSyncReport { + repository_id: "repo-123".to_string(), + source_instance_id: "source-abc".to_string(), + streams: crate::services::trace::sync::StreamSyncReports { + messages: StreamSyncReport { + uploaded: 3, + initial_cursor: 10, + final_cursor: 13, + batches: 1, + }, + parts: StreamSyncReport { + uploaded: 5, + initial_cursor: 20, + final_cursor: 25, + batches: 1, + }, + diff_traces: StreamSyncReport { + uploaded: 0, + initial_cursor: 7, + final_cursor: 7, + batches: 0, + }, + agent_traces: StreamSyncReport { + uploaded: 2, + initial_cursor: 1, + final_cursor: 3, + batches: 1, + }, + }, + } + } + + #[test] + fn text_renders_concise_per_stream_layout_without_batches() { + let rendered = render_text(&sample_report()); + assert!(rendered.contains("Agent Trace sync complete.")); + assert!(rendered.contains("Repository ID: repo-123")); + assert!(rendered.contains("Source instance ID: source-abc")); + assert!(rendered.contains("messages")); + assert!(rendered.contains("parts")); + assert!(rendered.contains("diff_traces")); + assert!(rendered.contains("agent_traces")); + // Concise: no per-batch or per-row detail is printed. + assert!(!rendered.contains("batch")); + } + + #[test] + fn text_row_values_match_uploaded_and_final_cursor() { + let rendered = render_text(&sample_report()); + let messages_line = rendered + .lines() + .find(|line| line.trim_start().starts_with("messages")) + .expect("messages row present"); + assert!(messages_line.contains('3')); + assert!(messages_line.contains("13")); + } + + #[test] + fn json_shape_matches_contract() { + let payload = render_json(&sample_report()).expect("json render"); + let value: serde_json::Value = serde_json::from_str(&payload).expect("valid json"); + assert_eq!(value["status"], "ok"); + assert_eq!(value["command"], "trace"); + assert_eq!(value["subcommand"], "sync"); + assert_eq!(value["repositoryId"], "repo-123"); + assert_eq!(value["sourceInstanceId"], "source-abc"); + + let messages = &value["streams"]["messages"]; + assert_eq!(messages["uploaded"], 3); + assert_eq!(messages["initialCursor"], 10); + assert_eq!(messages["finalCursor"], 13); + assert_eq!(messages["batches"], 1); + + let diff_traces = &value["streams"]["diffTraces"]; + assert_eq!(diff_traces["uploaded"], 0); + assert_eq!(diff_traces["initialCursor"], 7); + assert_eq!(diff_traces["finalCursor"], 7); + assert_eq!(diff_traces["batches"], 0); + + let agent_traces = &value["streams"]["agentTraces"]; + assert_eq!(agent_traces["uploaded"], 2); + assert_eq!(agent_traces["initialCursor"], 1); + assert_eq!(agent_traces["finalCursor"], 3); + assert_eq!(agent_traces["batches"], 1); + + assert!(value.get("diff_traces").is_none()); + assert!(value.get("agent_traces").is_none()); + } +} diff --git a/cli/src/services/trace/stats.rs b/cli/src/services/trace/stats.rs index 5b42c37a..2a975034 100644 --- a/cli/src/services/trace/stats.rs +++ b/cli/src/services/trace/stats.rs @@ -221,18 +221,16 @@ mod tests { generated_at_unix_ms: 1_100, }) .expect("message 2"); - let parts = ["p1", "p2", "p3"] - .iter() - .enumerate() - .map(|(i, part_id)| InsertPartInsert { + for (i, part_id) in ["p1", "p2", "p3"].iter().enumerate() { + db.insert_part(InsertPartInsert { part_type: PartType::Text, text: format!("part {part_id}"), session_id: "s1".into(), message_id: if i < 2 { "m1".into() } else { "m2".into() }, generated_at_unix_ms: 1_000 + i64::try_from(i).expect("part index fits in i64"), }) - .collect(); - db.insert_parts(parts).expect("parts"); + .expect("part"); + } latest_diff_ms } diff --git a/cli/src/services/trace/sync.rs b/cli/src/services/trace/sync.rs new file mode 100644 index 00000000..d62ee6d5 --- /dev/null +++ b/cli/src/services/trace/sync.rs @@ -0,0 +1,712 @@ +//! `sce trace sync` orchestration: resolves the current repository's Agent +//! Trace storage, fetches authoritative control-plane cursors, then +//! synchronizes each of the four independent capture streams in the fixed +//! `messages -> parts -> diff_traces -> agent_traces` order. +//! +//! Consumes the already-shipped [`crate::services::agent_trace_sync`] engine +//! and [`crate::services::agent_trace_sync::control_plane`] client as-is; adds +//! no local sync cursor or persisted progress of its own. + +use std::cell::RefCell; +use std::fmt; +use std::path::Path; +use std::sync::OnceLock; + +use anyhow::Context; +use tokio::runtime::Runtime; + +use crate::services::agent_trace_db::repository::RepositoryAgentTraceDb; +use crate::services::agent_trace_export::{AgentTraceExportReader, AGENT_TRACE_EXPORT_BATCH_SIZE}; +use crate::services::agent_trace_storage::{resolve_agent_trace_storage, AgentTraceStorageContext}; +use crate::services::agent_trace_sync::control_plane::{ + AgentTraceCursors, AgentTraceIngestionBatchRequest, AgentTraceIngestionBatchResponse, + AgentTraceIngestionStateRequest, AuthenticatedControlPlaneClient, ControlPlaneError, + IngestionStream, +}; +use crate::services::agent_trace_sync::{ + sync_stream, AgentTraceExportRow, BatchAttemptOutcome, StreamSyncError, +}; +use crate::services::auth; +use crate::services::config; + +static SYNC_RUNTIME: OnceLock = OnceLock::new(); + +/// Full sync result across all four capture streams, ready for rendering. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct AgentTraceSyncReport { + pub repository_id: String, + pub source_instance_id: String, + pub streams: StreamSyncReports, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct StreamSyncReports { + pub messages: StreamSyncReport, + pub parts: StreamSyncReport, + pub diff_traces: StreamSyncReport, + pub agent_traces: StreamSyncReport, +} + +/// One stream's converged sync outcome. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct StreamSyncReport { + pub uploaded: usize, + pub initial_cursor: i64, + pub final_cursor: i64, + pub batches: usize, +} + +/// Terminal failure of `sce trace sync`. +#[derive(Debug)] +pub enum TraceSyncError { + /// Local repository/storage/config resolution failed. + Runtime(String), + /// The initial `/state` call failed terminally. + ControlPlane(ControlPlaneError), + /// One stream failed to converge. + Stream { + stream: &'static str, + source: StreamSyncError, + }, +} + +impl fmt::Display for TraceSyncError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Runtime(reason) => write!(f, "{reason}"), + Self::ControlPlane(error) => write!(f, "{error}"), + Self::Stream { stream, source } => write!(f, "'{stream}' stream sync failed: {source}"), + } + } +} + +impl std::error::Error for TraceSyncError {} + +/// Resolves the current repository's Agent Trace storage (the same +/// `ContextWithRepoRoot`/`AgentTraceStorageContext`/`resolve_agent_trace_storage` +/// path `sce trace status` uses) and control-plane configuration, then +/// synchronizes all four capture streams. +pub fn run_current_sync(repo_root: &Path) -> Result { + let storage_config = config::resolve_agent_trace_storage_runtime_config(repo_root) + .map_err(|error| TraceSyncError::Runtime(format!("{error:#}")))?; + let context = AgentTraceStorageContext { + repository_root: repo_root, + explicit_repository_id: storage_config.repository_id.as_deref(), + repository_remote: &storage_config.repository_remote, + }; + let storage = resolve_agent_trace_storage(&context) + .map_err(|error| TraceSyncError::Runtime(format!("{error:#}")))?; + + let auth_config = config::resolve_auth_runtime_config(repo_root) + .map_err(|error| TraceSyncError::Runtime(format!("{error:#}")))?; + let client = AuthenticatedControlPlaneClient::new( + reqwest::Client::new(), + auth_config.control_plane_base_url.value.unwrap_or_default(), + auth::WORKOS_DEFAULT_BASE_URL, + auth_config.workos_client_id.value.unwrap_or_default(), + ); + + run_sync_against( + &storage.metadata.repository_id, + &storage.metadata.source_instance_id, + &storage.db, + &client, + ) +} + +/// Testable core: synchronizes all four streams given an already-resolved +/// repository identity, an open Agent Trace database, and a configured +/// control-plane client (production or test-double). +pub(crate) fn run_sync_against( + repository_id: &str, + source_instance_id: &str, + db: &RepositoryAgentTraceDb, + client: &AuthenticatedControlPlaneClient, +) -> Result { + let runtime = shared_runtime()?; + let reader = AgentTraceExportReader::new(db); + + let state_request = AgentTraceIngestionStateRequest { + repository_id: repository_id.to_string(), + source_instance_id: source_instance_id.to_string(), + }; + let state = runtime + .block_on(client.ingestion_state(&state_request)) + .map_err(TraceSyncError::ControlPlane)?; + + let messages = sync_one_stream( + runtime, + client, + repository_id, + source_instance_id, + IngestionStream::Messages, + state.cursors.messages, + "messages", + |cursor, limit| reader.read_messages_after(cursor, limit), + |request| runtime.block_on(client.ingest_messages(request)), + )?; + let parts = sync_one_stream( + runtime, + client, + repository_id, + source_instance_id, + IngestionStream::Parts, + state.cursors.parts, + "parts", + |cursor, limit| reader.read_parts_after(cursor, limit), + |request| runtime.block_on(client.ingest_parts(request)), + )?; + let diff_traces = sync_one_stream( + runtime, + client, + repository_id, + source_instance_id, + IngestionStream::DiffTraces, + state.cursors.diff_traces, + "diff_traces", + |cursor, limit| reader.read_diff_traces_after(cursor, limit), + |request| runtime.block_on(client.ingest_diff_traces(request)), + )?; + let agent_traces = sync_one_stream( + runtime, + client, + repository_id, + source_instance_id, + IngestionStream::AgentTraces, + state.cursors.agent_traces, + "agent_traces", + |cursor, limit| reader.read_agent_traces_after(cursor, limit), + |request| runtime.block_on(client.ingest_agent_traces(request)), + )?; + + Ok(AgentTraceSyncReport { + repository_id: repository_id.to_string(), + source_instance_id: source_instance_id.to_string(), + streams: StreamSyncReports { + messages, + parts, + diff_traces, + agent_traces, + }, + }) +} + +/// Synchronizes one stream via the T04 engine. Genuine `409`/`5xx`/transport +/// ambiguity reconciles through a real `/state` refetch; a terminal +/// control-plane failure (missing/invalid auth, `400`, `403`) short-circuits +/// the reconciliation closure with that failure instead of issuing another +/// network call, so a `403` never mutates local state or retries. +#[allow(clippy::too_many_arguments)] +fn sync_one_stream( + runtime: &Runtime, + client: &AuthenticatedControlPlaneClient, + repository_id: &str, + source_instance_id: &str, + stream: IngestionStream, + initial_cursor: i64, + stream_label: &'static str, + mut read_after: ReadFn, + mut ingest: IngestFn, +) -> Result +where + T: AgentTraceExportRow + Clone, + ReadFn: FnMut(i64, usize) -> anyhow::Result>, + IngestFn: FnMut( + &AgentTraceIngestionBatchRequest, + ) -> Result, +{ + let terminal: RefCell> = RefCell::new(None); + + let outcome = sync_stream( + initial_cursor, + AGENT_TRACE_EXPORT_BATCH_SIZE, + |cursor, limit| { + read_after(cursor, limit).map_err(|error| StreamSyncError::Read(format!("{error:#}"))) + }, + |cursor, rows: &[T]| { + let request = AgentTraceIngestionBatchRequest { + repository_id: repository_id.to_string(), + source_instance_id: source_instance_id.to_string(), + stream, + expected_cursor: cursor, + rows: rows.to_vec(), + }; + match ingest(&request) { + Ok(response) => BatchAttemptOutcome::Accepted { + accepted: response.accepted, + cursor: response.cursor, + }, + Err(ControlPlaneError::Conflict(_)) => BatchAttemptOutcome::Conflict, + Err(error) if is_stream_terminal(&error) => { + *terminal.borrow_mut() = Some(error); + BatchAttemptOutcome::Ambiguous + } + Err(_) => BatchAttemptOutcome::Ambiguous, + } + }, + || { + if let Some(error) = terminal.borrow_mut().take() { + return Err(StreamSyncError::Refresh(error.to_string())); + } + + let state_request = AgentTraceIngestionStateRequest { + repository_id: repository_id.to_string(), + source_instance_id: source_instance_id.to_string(), + }; + let response = runtime + .block_on(client.ingestion_state(&state_request)) + .map_err(|error| StreamSyncError::Refresh(error.to_string()))?; + Ok(cursor_for_stream(&response.cursors, stream)) + }, + ) + .map_err(|source| TraceSyncError::Stream { + stream: stream_label, + source, + })?; + + Ok(StreamSyncReport { + uploaded: outcome.uploaded, + initial_cursor: outcome.initial_cursor, + final_cursor: outcome.final_cursor, + batches: outcome.batches, + }) +} + +/// A control-plane failure that cannot be resolved by reconciling with +/// `/state`: missing/invalid credentials, an unrecoverable `401`, a `400`, a +/// `403` ownership rejection, or a terminal protocol/API mismatch +/// (`404`/`405`/`415`/`422`, `ControlPlaneError::Protocol`). `5xx`, transport +/// failures, and invalid batch responses are genuinely ambiguous and +/// reconcile via a real `/state` call. +fn is_stream_terminal(error: &ControlPlaneError) -> bool { + matches!( + error, + ControlPlaneError::MissingCredentials + | ControlPlaneError::AuthenticationFailed(_) + | ControlPlaneError::BadRequest(_) + | ControlPlaneError::Forbidden(_) + | ControlPlaneError::Storage(_) + | ControlPlaneError::Protocol { .. } + ) +} + +fn cursor_for_stream(cursors: &AgentTraceCursors, stream: IngestionStream) -> i64 { + match stream { + IngestionStream::Messages => cursors.messages, + IngestionStream::Parts => cursors.parts, + IngestionStream::DiffTraces => cursors.diff_traces, + IngestionStream::AgentTraces => cursors.agent_traces, + } +} + +fn shared_runtime() -> Result<&'static Runtime, TraceSyncError> { + if let Some(runtime) = SYNC_RUNTIME.get() { + return Ok(runtime); + } + + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_io() + .enable_time() + .build() + .context("failed to create trace sync command runtime") + .map_err(|error| TraceSyncError::Runtime(format!("{error:#}")))?; + + Ok(SYNC_RUNTIME.get_or_init(|| runtime)) +} + +#[cfg(test)] +mod tests { + use std::fs; + use std::path::PathBuf; + use std::time::{SystemTime, UNIX_EPOCH}; + + use serde_json::json; + + use super::*; + use crate::services::agent_trace_db::{ + AgentTraceInsert, DiffTraceInsert, InsertMessageInsert, InsertPartInsert, MessageRole, + PartType, PAYLOAD_TYPE_PATCH, + }; + use crate::services::agent_trace_sync::control_plane::CredentialStore; + use crate::services::agent_trace_sync::test_http_server::{CannedResponse, TestHttpServer}; + use crate::services::auth::TokenResponse; + use crate::services::token_storage::StoredTokens; + + fn unique_test_db_path(label: &str) -> PathBuf { + let nonce = SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("system time should be after Unix epoch") + .as_nanos(); + std::env::temp_dir() + .join(format!( + "sce-trace-sync-{label}-{}-{nonce}", + std::process::id() + )) + .join("agent-trace.db") + } + + fn remove_test_db(db_path: &std::path::Path) { + if let Some(parent) = db_path.parent() { + let _ = fs::remove_dir_all(parent); + } + } + + /// Always reports a still-valid token; the sync tests exercise streaming + /// behavior, not auth refresh (covered by `control_plane`'s own tests). + struct AlwaysValidCredentialStore; + + impl CredentialStore for AlwaysValidCredentialStore { + fn load(&self) -> Result, ControlPlaneError> { + Ok(Some(StoredTokens { + access_token: "valid-access-token".to_string(), + token_type: "Bearer".to_string(), + expires_in: 3_600, + refresh_token: "refresh-token".to_string(), + scope: None, + stored_at_unix_seconds: SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("system clock before unix epoch") + .as_secs(), + })) + } + + fn save(&self, _token: &TokenResponse) -> Result { + panic!("no token refresh expected in this test"); + } + } + + fn test_client(server: &TestHttpServer) -> AuthenticatedControlPlaneClient { + let http = reqwest::Client::builder() + .danger_accept_invalid_certs(true) + .build() + .expect("build test reqwest client"); + AuthenticatedControlPlaneClient::with_credential_store( + http, + server.base_url.clone(), + server.base_url.clone(), + "test-client-id", + Box::new(AlwaysValidCredentialStore), + ) + } + + fn seed_one_row_per_stream(db: &RepositoryAgentTraceDb) { + db.insert_messages(vec![InsertMessageInsert { + session_id: "sess-1".to_string(), + message_id: "msg-1".to_string(), + role: MessageRole::User, + generated_at_unix_ms: 1_700_000_000_000, + }]) + .expect("seed message"); + db.insert_parts(vec![InsertPartInsert { + part_type: PartType::Text, + text: "hello".to_string(), + session_id: "sess-1".to_string(), + message_id: "msg-1".to_string(), + generated_at_unix_ms: 1_700_000_000_000, + }]) + .expect("seed part"); + db.insert_diff_trace(DiffTraceInsert { + time_ms: 1_700_000_000_000, + session_id: "sess-1", + patch: "Index: a\n", + model_id: None, + tool_name: "opencode", + tool_version: None, + payload_type: PAYLOAD_TYPE_PATCH, + }) + .expect("seed diff_trace"); + db.insert_agent_trace(AgentTraceInsert { + commit_id: "abc123", + commit_time_ms: 1_700_000_000_000, + trace_json: "{\"steps\":[]}", + agent_trace_id: "trace-1", + url: "https://example.com/trace", + remote_url: "", + }) + .expect("seed agent_trace"); + } + + fn state_response( + messages: i64, + parts: i64, + diff_traces: i64, + agent_traces: i64, + ) -> serde_json::Value { + json!({ + "cursors": { + "messages": messages, + "parts": parts, + "diffTraces": diff_traces, + "agentTraces": agent_traces, + } + }) + } + + fn batch_response(cursor: i64) -> serde_json::Value { + json!({ "accepted": 1, "cursor": cursor }) + } + + #[test] + fn full_sync_uploads_all_four_streams_and_second_run_is_naturally_incremental() { + let db_path = unique_test_db_path("full-sync"); + let db = RepositoryAgentTraceDb::new_at(&db_path).expect("test DB should open"); + let metadata = db + .verify_or_initialize_repository_metadata("repo-full-sync") + .expect("metadata should initialize"); + seed_one_row_per_stream(&db); + + let server = TestHttpServer::start(); + server.queue_response(CannedResponse::json(200, &state_response(0, 0, 0, 0))); + server.queue_response(CannedResponse::json(200, &batch_response(1))); + server.queue_response(CannedResponse::json(200, &batch_response(1))); + server.queue_response(CannedResponse::json(200, &batch_response(1))); + server.queue_response(CannedResponse::json(200, &batch_response(1))); + let client = test_client(&server); + + let report = run_sync_against( + &metadata.repository_id, + &metadata.source_instance_id, + &db, + &client, + ) + .expect("first sync should succeed"); + + assert_eq!(server.call_count(), 5); + for stream in [ + report.streams.messages, + report.streams.parts, + report.streams.diff_traces, + report.streams.agent_traces, + ] { + assert_eq!(stream.uploaded, 1); + assert_eq!(stream.initial_cursor, 0); + assert_eq!(stream.final_cursor, 1); + assert_eq!(stream.batches, 1); + } + + server.queue_response(CannedResponse::json(200, &state_response(1, 1, 1, 1))); + + let second_report = run_sync_against( + &metadata.repository_id, + &metadata.source_instance_id, + &db, + &client, + ) + .expect("second sync should succeed"); + + assert_eq!( + server.call_count(), + 6, + "second run should only re-check /state and re-read no already-synced rows" + ); + for stream in [ + second_report.streams.messages, + second_report.streams.parts, + second_report.streams.diff_traces, + second_report.streams.agent_traces, + ] { + assert_eq!(stream.uploaded, 0); + assert_eq!(stream.initial_cursor, 1); + assert_eq!(stream.final_cursor, 1); + assert_eq!(stream.batches, 0); + } + + let db_file_name = db_path + .file_name() + .expect("db path has a file name") + .to_string_lossy() + .into_owned(); + assert!( + !db_path + .parent() + .expect("db has parent dir") + .read_dir() + .expect("read db dir") + .filter_map(Result::ok) + .any(|entry| { + let name = entry.file_name().to_string_lossy().into_owned(); + // Turso may write WAL/SHM sidecar files alongside the DB + // itself; only reject files unrelated to that one DB, which + // would indicate a local sync cursor/DB sneaking in. + !name.starts_with(&db_file_name) + }), + "sync must not create any local cursor file/database/table on disk" + ); + + remove_test_db(&db_path); + } + + #[test] + fn invalid_state_cursor_fails_before_any_batch_request() { + let db_path = unique_test_db_path("invalid-cursor"); + let db = RepositoryAgentTraceDb::new_at(&db_path).expect("test DB should open"); + let metadata = db + .verify_or_initialize_repository_metadata("repo-invalid-cursor") + .expect("metadata should initialize"); + seed_one_row_per_stream(&db); + + let server = TestHttpServer::start(); + // `agentTraces` is one past JS_MAX_SAFE_INTEGER, outside the wire + // contract's representable cursor range. + server.queue_response(CannedResponse::json( + 200, + &state_response(0, 0, 0, 9_007_199_254_740_992), + )); + let client = test_client(&server); + + let error = run_sync_against( + &metadata.repository_id, + &metadata.source_instance_id, + &db, + &client, + ) + .expect_err("out-of-range /state cursor should fail sync"); + + assert!(matches!( + error, + TraceSyncError::ControlPlane(ControlPlaneError::InvalidResponse(_)) + )); + assert_eq!( + server.call_count(), + 1, + "an invalid /state cursor must fail before any /batch request is sent" + ); + + remove_test_db(&db_path); + } + + #[test] + fn terminal_batch_status_fails_without_state_reconciliation() { + let db_path = unique_test_db_path("terminal-batch"); + let db = RepositoryAgentTraceDb::new_at(&db_path).expect("test DB should open"); + let metadata = db + .verify_or_initialize_repository_metadata("repo-terminal-batch") + .expect("metadata should initialize"); + seed_one_row_per_stream(&db); + + let server = TestHttpServer::start(); + server.queue_response(CannedResponse::json(200, &state_response(0, 0, 0, 0))); + server.queue_response(CannedResponse::json( + 404, + &json!({"message": "unknown ingestion route"}), + )); + let client = test_client(&server); + + let error = run_sync_against( + &metadata.repository_id, + &metadata.source_instance_id, + &db, + &client, + ) + .expect_err("a terminal 404 /batch response should fail the sync"); + + assert!( + matches!( + error, + TraceSyncError::Stream { + stream: "messages", + .. + } + ), + "unexpected error: {error:?}" + ); + assert_eq!( + server.call_count(), + 2, + "a terminal /batch status must fail immediately with no /state refetch and no resend" + ); + + remove_test_db(&db_path); + } + + #[test] + fn malformed_2xx_batch_response_still_reconciles_via_state() { + let db_path = unique_test_db_path("malformed-batch"); + let db = RepositoryAgentTraceDb::new_at(&db_path).expect("test DB should open"); + let metadata = db + .verify_or_initialize_repository_metadata("repo-malformed-batch") + .expect("metadata should initialize"); + seed_one_row_per_stream(&db); + + let server = TestHttpServer::start(); + server.queue_response(CannedResponse::json(200, &state_response(0, 0, 0, 0))); + // Syntactically successful but undecodable as `AgentTraceIngestionBatchResponse`. + server.queue_response(CannedResponse::json(200, &json!({"unexpected": "shape"}))); + // Reconciliation refetch: cursor unchanged, so the batch is resent. + server.queue_response(CannedResponse::json(200, &state_response(0, 0, 0, 0))); + server.queue_response(CannedResponse::json(200, &batch_response(1))); + server.queue_response(CannedResponse::json(200, &batch_response(1))); + server.queue_response(CannedResponse::json(200, &batch_response(1))); + server.queue_response(CannedResponse::json(200, &batch_response(1))); + let client = test_client(&server); + + let report = run_sync_against( + &metadata.repository_id, + &metadata.source_instance_id, + &db, + &client, + ) + .expect("an undecodable 2xx /batch body should still reconcile via /state and succeed"); + + assert_eq!( + server.call_count(), + 7, + "an undecodable 2xx /batch body must reconcile via /state before resending, not fail immediately" + ); + for stream in [ + report.streams.messages, + report.streams.parts, + report.streams.diff_traces, + report.streams.agent_traces, + ] { + assert_eq!(stream.uploaded, 1); + assert_eq!(stream.final_cursor, 1); + } + + remove_test_db(&db_path); + } + + #[test] + fn forbidden_state_response_fails_without_mutating_local_metadata() { + let db_path = unique_test_db_path("forbidden"); + let db = RepositoryAgentTraceDb::new_at(&db_path).expect("test DB should open"); + let metadata_before = db + .verify_or_initialize_repository_metadata("repo-forbidden") + .expect("metadata should initialize"); + + let server = TestHttpServer::start(); + server.queue_response(CannedResponse::json( + 403, + &json!({"error": "not the owner"}), + )); + let client = test_client(&server); + + let error = run_sync_against( + &metadata_before.repository_id, + &metadata_before.source_instance_id, + &db, + &client, + ) + .expect_err("403 should fail the sync"); + + assert!(matches!( + error, + TraceSyncError::ControlPlane(ControlPlaneError::Forbidden(_)) + )); + + let metadata_after = db + .verify_or_initialize_repository_metadata("repo-forbidden") + .expect("metadata should still verify"); + assert_eq!( + metadata_after.source_instance_id, + metadata_before.source_instance_id + ); + assert_eq!( + server.call_count(), + 1, + "no reconciliation call should follow a terminal /state failure" + ); + + remove_test_db(&db_path); + } +} diff --git a/config/pkl/base/sce-config-schema.pkl b/config/pkl/base/sce-config-schema.pkl index 067e23bc..55c71faf 100644 --- a/config/pkl/base/sce-config-schema.pkl +++ b/config/pkl/base/sce-config-schema.pkl @@ -93,6 +93,11 @@ local sceConfigSchema = new JsonSchema { ["workos_client_id"] = new JsonSchema { type = "string" } + ["control_plane_base_url"] = new JsonSchema { + type = "string" + description = "Base URL of the control-plane Agent Trace ingestion API used by `sce trace sync`. Defaults to the canonical SCE control plane." + minLength = 1 + } ["agent_trace"] = new JsonSchema { type = "object" description = "Agent Trace repository identity configuration. Selects the repository-scoped Agent Trace database." diff --git a/context/architecture.md b/context/architecture.md index 2277fec6..b4e0ec82 100644 --- a/context/architecture.md +++ b/context/architecture.md @@ -132,7 +132,7 @@ The repository includes a new placeholder Rust binary crate at `cli/`. - `cli/src/services/hooks/mod.rs` defines the current local hook runtime parsing/dispatch (`HookSubcommand`, `run_hooks_subcommand`) plus a commit-msg co-author policy seam (`apply_commit_msg_coauthor_policy`) that injects one canonical SCE trailer only when the enabled-by-default attribution-hooks config/env control is not opted out, `SCE_DISABLED` is false, and the staged-diff AI-overlap preflight confirms AI/editor evidence (`StagedDiffAiOverlapResult::Overlap`); the preflight is wired into `run_commit_msg_subcommand_in_repo` and logs `sce.hooks.commit_msg.ai_overlap_error` on error paths; `cli/src/services/hooks/command.rs` owns the `HooksCommand` payload used by the static `RuntimeCommand` enum. In the current attribution-only baseline, `pre-commit` and `post-rewrite` are deterministic no-op surfaces; `post-commit` requires validated `--remote-url`, threads that URL through the Agent Trace flow, prints it to stderr, and remains an active intersection + Agent Trace persistence entrypoint (captures current commit patch, queries recent repository-level `diff_traces` from the bounded past-7-days window, combines valid patches via `patch::combine_patches`, intersects with post-commit patch via `patch::intersect_patches`, persists result to `post_commit_patch_intersections`, then persists built Agent Trace payloads with range-level `content_hash` values to `agent_traces` in the repository-scoped Agent Trace DB without post-commit file artifacts); `diff-trace` performs STDIN JSON intake, validates required non-empty `sessionID`/`diff`/`tool_name`, optional `model_id` (absent/`null` → `None`), required nullable/non-empty `tool_version` plus required `u64` `time` (Unix epoch milliseconds), rejects values that cannot fit signed `time_ms` storage, prefixes the stored `diff_traces.session_id` before insert construction (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi, same-tool idempotent), and inserts the parsed payload fields into `RepositoryAgentTraceDb` without creating a parsed-payload `context/tmp` artifact; Claude structured `PostToolUse` diff-trace intake extracts direct model metadata from top-level or nested `model` fields and normalizes it with the `claude/` prefix when present. `session-model` is no longer a supported hook route. - Generated Claude settings no longer register `SessionStart` for Agent Trace model attribution, and `sce hooks session-model` is no longer a supported hook command. The `session_models` table/API and diff-trace fallback lookup were removed in T02 of the `remove-session-models-direct-claude-model-id` plan; `diff-trace` now uses only direct payload `model_id` and `tool_version` values. - `cli/src/services/resilience.rs` defines bounded retry/timeout/backoff execution policy (`RetryPolicy`, `run_with_retry`) for transient operation hardening with deterministic failure messaging and retry observability. -- No user-invocable `sce sync` command is wired in the current runtime; local DB bootstrap and setup-time repository-scoped Agent Trace DB initialization flow through lifecycle providers aggregated by setup, while repository-scoped DB health/repair flows through the doctor surface. There is no checkout-scoped DB discovery or `sce trace --legacy` surface (removed by the `retire-legacy-agent-trace-db` plan). +- `sce trace sync [--format text|json]` is implemented: `cli/src/services/trace/sync.rs` resolves repository-scoped Agent Trace storage the same way `sce trace status` does, authenticates against the control plane with stored WorkOS credentials, calls the ingestion `/state` endpoint once, then synchronizes the `messages`/`parts`/`diff_traces`/`agent_traces` capture streams via `AgentTraceExportReader` and a shared per-stream reconciliation engine, and `cli/src/services/trace/render_sync.rs` renders the converged `AgentTraceSyncReport` as concise per-stream text or `camelCase` JSON (see `context/cli/trace-command.md`). Local DB bootstrap and setup-time repository-scoped Agent Trace DB initialization otherwise still flow through lifecycle providers aggregated by setup, while repository-scoped DB health/repair flows through the doctor surface. There is no checkout-scoped DB discovery or `sce trace --legacy` surface (removed by the `retire-legacy-agent-trace-db` plan). - `cli/src/services/patch.rs` defines the standalone patch domain model (`ParsedPatch`, `PatchFileChange`, `FileChangeKind`, `PatchHunk`, `TouchedLine`, `TouchedLineKind`) for in-memory parsed unified-diff representation, capturing only touched lines (added/removed) plus minimal per-file/per-hunk metadata while excluding non-hunk headers and unchanged context lines. All types are `serde`-serializable/deserializable with `snake_case` JSON field naming. The module also provides `parse_patch`, a public parser function that converts raw unified-diff text (both `Index:` SVN-style and `diff --git` git-style formats) into `ParsedPatch` structs, with `ParseError` for actionable malformed-input diagnostics. Storage-agnostic JSON load helpers (`load_patch_from_json` for string input, `load_patch_from_json_bytes` for byte input) reconstruct `ParsedPatch` from serialized JSON content with `PatchLoadError` for actionable deserialization diagnostics. Its patch-set operations now include deterministic ordered combination plus target-shaped intersection that prefers exact touched-line matches and falls back to historical `kind`+`content` matching when incremental diffs and canonical post-commit diffs have drifted line numbers; `parse_patch`, `combine_patches`, and `intersect_patches` are consumed by the active post-commit hook runtime. - `cli/src/services/structured_patch.rs` defines the synchronous structured editor-hook derivation seam. It derives Claude `PostToolUse` `Write` structured-update hunks, `Write` `tool_input.content` create fallback, and `Edit` structured-patch payloads into canonical `ParsedPatch` values plus Claude session/tool metadata, returning deterministic skip reasons for unsupported events/tools/payload shapes. The module is pure and side-effect-free. It is wired into `sce hooks diff-trace` for Claude payload classification at intake (T04) and into `RepositoryAgentTraceDb::recent_diff_trace_patches` for post-commit structured payload parsing dispatch at read time (T05). - `cli/src/services/` contains module boundaries for command_registry, lifecycle, auth_command, config, setup, doctor, hooks, checkout identity, bash_policy, version, completion, help, patch, SCE web URL helpers, shared database infrastructure, local DB adapters, encrypted auth DB adapters, and Agent Trace DB adapters with explicit trait seams for future implementations. `cli/src/services/checkout/` owns checkout ID file infrastructure (the per-checkout Agent Trace DB opener/path helper was removed by the `retire-legacy-agent-trace-db` plan); active setup/hooks use `agent_trace_storage` to establish checkout identity as diagnostics and initialize/open the repository-scoped DB, while `sce doctor` surfaces checkout identity facts plus credential-safe repository Agent Trace DB metadata. There is no checkout-scoped discovery or `sce trace --legacy` surface; any pre-migration `agent-trace-*.db` files on disk are never touched and no longer inspectable via the CLI. `cli/src/services/bash_policy.rs` owns both the CLI-agnostic evaluator logic and the hidden `sce policy bash` command adapter used by OpenCode, Claude, and Pi callers. `cli/src/services/command_registry.rs` defines the static `RuntimeCommand` enum, deterministic `CommandRegistry` name catalog, and `build_default_registry()` function for command dispatch metadata. Service-owned command modules own the runtime command payload structs for help/help-text, version, completion, auth, config, setup, doctor, hooks, and policy. @@ -183,7 +183,7 @@ Investigations T08 (`turso default-features = false`) and T09 (isolating the with rationale in the benchmark doc. Final after-change numbers and remaining bottlenecks are captured in T11. -This phase establishes compile-safe extension seams with a dependency baseline (`anyhow`, `chrono`, `clap`, `clap_complete`, `dirs`, `hmac`, `inquire`, `jsonschema`, `keyring-core`, `murmur3`, `owo-colors`, `rand`, `reqwest`, `serde`, `serde_json`, `sha2`, `tokio`, `tracing`, `turso`, `uuid`, plus target-specific keyring backends); no CLI dev-dependencies are currently declared. Per-user local Turso DB and Agent Trace DB bootstrap/health coverage now exist through setup/doctor flows, while a user-invocable `sce sync` command and broader runtime integrations remain deferred. +This phase establishes compile-safe extension seams with a dependency baseline (`anyhow`, `chrono`, `clap`, `clap_complete`, `dirs`, `hmac`, `inquire`, `jsonschema`, `keyring-core`, `murmur3`, `owo-colors`, `rand`, `reqwest`, `serde`, `serde_json`, `sha2`, `tokio`, `tracing`, `turso`, `uuid`, plus target-specific keyring backends); no CLI dev-dependencies are currently declared. Per-user local Turso DB and Agent Trace DB bootstrap/health coverage now exist through setup/doctor flows; a user-invocable `sce trace sync` command is now fully implemented including rendering (see above), and broader runtime integrations remain deferred. ## SCE plan/code role boundary diff --git a/context/cli/agent-trace-sync-command.md b/context/cli/agent-trace-sync-command.md new file mode 100644 index 00000000..1eed8e97 --- /dev/null +++ b/context/cli/agent-trace-sync-command.md @@ -0,0 +1,60 @@ +# Agent Trace sync architecture + +`sce trace sync` is the composition step that synchronizes a repository's local Agent Trace capture database with the control-plane Agent Trace ingestion API. It composes already-shipped infrastructure — repository/source identity, the read-only export readers, and the existing WorkOS auth/token-storage stack — into one command; it does not redesign the local database, source identity, export readers, or control-plane storage model. + +## User flow + +``` +sce auth login # obtain and store WorkOS credentials +cd # any directory inside the target Git repository +sce trace sync # synchronize this repository's Agent Trace DB +``` + +`sce trace sync --format json` produces the same synchronization with machine-readable output; see [trace-command.md](trace-command.md) for the exact rendering contracts. + +## Composed data flow + +```mermaid +flowchart LR + A[hooks / plugins] --> B[repository Agent Trace DB] + B --> C[AgentTraceExportReader] + C --> D[sce trace sync] + D -- "HTTPS + WorkOS Bearer" --> E[control plane] +``` + +- **hooks/plugins** write local capture rows (`messages`, `parts`, `diff_traces`, `agent_traces`) into the current repository's `RepositoryAgentTraceDb` during normal Git/editor activity — this is unchanged by sync. +- **`AgentTraceExportReader`** (PR #198) is the read-only local export boundary sync uses to read rows after a cursor; sync never queries the repository DB directly. +- **`sce trace sync`** resolves repository storage through the same `agent_trace_storage` path `sce trace status` uses (not the hook-runtime resolver), builds an `AuthenticatedControlPlaneClient` from stored WorkOS credentials and the resolved `control_plane_base_url`, and drives one authoritative `/state` call plus one bounded per-stream reconciliation loop per stream. +- **Credential runtime boundary:** `AuthenticatedControlPlaneClient` keeps the synchronous `CredentialStore` behind an `Arc` and runs every token-storage `load`/`save` through `tokio::task::spawn_blocking`. The underlying encrypted auth DB and Linux Secret Service/zbus APIs are blocking and may create their own Tokio runtime, so they must never execute directly inside the async control-plane request future. Token refresh and HTTP requests remain asynchronous; only credential persistence crosses the blocking boundary. +- **control plane** is the sole source of cursor truth: every invocation starts from `POST /agent-trace/ingestion/state`, uploads via `POST /agent-trace/ingestion/batch`, and advances a stream's cursor only from a validated batch response (`accepted == rows.len()` and `cursor == rows.last().sourceRowId`), never by inferring `cursor + rows.len()`. + +The four streams (`messages`, `parts`, `diff_traces`, `agent_traces`) are independent and synchronized in that fixed order within one invocation. + +## No-local-persistence invariants + +Sync creates no local sync state anywhere on disk: + +- No local sync cursor or cursor file. +- No `agent-trace-sync.db` or equivalent local database/table. +- No Turso Sync and no direct Turso credentials in SCE. +- No `BridgeLock` or local data-warehouse (DWH). + +Because every invocation starts from the control plane's authoritative `/state` cursors instead of local progress, restarts, conflicts, and ambiguous network failures are all recoverable without any client-side persisted state, and running `sce trace sync` twice in a row is naturally incremental — the second run's `/state` reflects the first run's uploads and only unsynced rows are re-read. + +## Recovery semantics + +- **`401` (unexpected):** the control-plane client refreshes the WorkOS token exactly once, saves it, and retries the request exactly once. A second `401` fails the command with `sce auth login` guidance; there is no further retry. +- **`409` (cursor conflict):** the per-stream sync engine reconciles by refetching `/state`, replacing only the affected stream's cursor, and resuming from local rows after the refreshed cursor — already-accepted rows are never resent. +- **Ambiguous batch failure (`5xx`, transport failure, or an undecodable `2xx` body):** the engine reconciles via `/state` before any resend. If the refreshed cursor advanced (the batch was actually committed), sync continues from it without resending. If the cursor is unchanged (the batch was not committed), sync may resend once from the authoritative cursor. +- **Reconciliation bound:** both the `409` and ambiguous-failure reconciliation paths share one bounded attempt counter per stream; exhausting it fails that stream with a "did not converge" error instead of looping unboundedly. +- **Invalid response:** a syntactically successful (`2xx`) but semantically inconsistent batch response (`accepted`/`cursor` not matching the sent rows, or an undecodable body) yields `ControlPlaneError::InvalidResponse` and is treated as ambiguous — it still reconciles via `/state` above, rather than failing the command outright. +- **`403` (ownership rejection):** fails the command immediately with a clear message. It never generates a new `source_instance_id`, mutates local repository metadata, or attempts ownership transfer — a `403` is treated as terminal, not reconciled. +- **Terminal protocol/API mismatch (`404`/`405`/`415`/`422` and other unrecognized `4xx` statuses during `/batch`):** classified as `ControlPlaneError::Protocol` and treated as terminal exactly like `400`/`403` — the stream fails immediately with no `/state` refetch and no resend, since a route/format mismatch is not something reconciling cursors can resolve. +- **Sanitized error messages:** every non-2xx control-plane error surfaces at most a narrow, length-bounded `message`/`error` string extracted from the response body (or a generic per-status fallback when the body is malformed, HTML, oversized, or missing that field) — the raw server response body is never included in a command-visible error. + +## Related context + +- [trace-command.md](trace-command.md) — the full `sce trace` command group, including `sync` request/response rendering (text and JSON shapes). +- [agent-trace-storage.md](agent-trace-storage.md) — the repository-scoped storage resolver sync reuses from `sce trace status`. +- [agent-trace-export-readers.md](../sce/agent-trace-export-readers.md) — the read-only local export boundary sync reads through. +- [auth-db.md](../sce/auth-db.md) — encrypted WorkOS credential storage sync authenticates through. diff --git a/context/cli/cli-command-surface.md b/context/cli/cli-command-surface.md index df7970e9..12137794 100644 --- a/context/cli/cli-command-surface.md +++ b/context/cli/cli-command-surface.md @@ -65,7 +65,7 @@ An interactive `setup` run instead resolves the selection through an `inquire` m `setup` additionally includes a repository-root install engine (`install_embedded_setup_assets`) that stages embedded files and uses a unified remove-and-replace policy for `.opencode/`/`.claude/`/`.pi/` (removing existing targets before swapping staged content, with deterministic recovery guidance on swap failure) while treating bash-policy enforcement files as first-class SCE-managed assets. `setup` now executes end-to-end and prints deterministic completion details including selected target(s) and per-target install count. `doctor` now executes end-to-end with explicit diagnosis and repair-intent surfaces: `sce doctor` stays read-only and `sce doctor --fix` selects repair-intent mode. Agent Trace DB checkout discovery has moved out of `doctor`; `sce trace db list`, `sce trace status`, and `sce trace status --all` operate only on repository-scoped DBs (the checkout-scoped `--legacy` surface was removed by the `retire-legacy-agent-trace-db` plan); see [trace-command.md](trace-command.md). The current `doctor` runtime aggregates `ServiceLifecycle::diagnose` and `ServiceLifecycle::fix` calls across all registered service providers (`config`, `local_db`, `auth_db`, `agent_trace_db`, `hooks`) plus integration checks, covering state-root resolution, global and repo-local `sce/config.json` readability/schema validation, local DB and repository-scoped Agent Trace DB path/health, DB-parent readiness barriers, the repo hook rollout slice when a repository target is detected, and repo-root installed OpenCode, Claude, and Pi integration presence/content health for their embedded setup assets. Fix mode delegates to each provider's `fix` implementation, which reuses the canonical setup hook install flow to repair missing/stale/non-executable required hooks and missing hooks directories, and it can bootstrap missing canonical database parent directories when the resolved paths match canonical owned locations. -A user-invocable `sync` command is not wired in the current CLI surface; local DB and Agent Trace DB bootstrap currently happen through `setup`, and DB health/repair currently happens through `doctor`. Command wiring for `sce sync` is deferred to `0.4.0`. +`sce trace sync [--format text|json]` is the implemented user-invocable sync command: it synchronizes the current repository's Agent Trace DB with the control-plane ingestion API; local DB and Agent Trace DB bootstrap continue to happen through `setup`, and DB health/repair continues to happen through `doctor`. See [agent-trace-sync-command.md](agent-trace-sync-command.md) and [trace-command.md](trace-command.md). ## Command loop and error model @@ -94,7 +94,7 @@ A user-invocable `sync` command is not wired in the current CLI surface; local D - `cli/src/services/completion/mod.rs` defines the completion output contract (`render_completion`) using clap_complete to generate deterministic shell scripts for Bash, Zsh, and Fish; `cli/src/services/completion/command.rs` owns the completion runtime command handler. - `cli/src/services/hooks/mod.rs` defines production local hook runtime parsing/dispatch (`HookSubcommand`, `run_hooks_subcommand`) for `pre-commit`, `commit-msg`, `post-commit`, `post-rewrite`, `diff-trace`, and `conversation-trace`; `cli/src/services/hooks/command.rs` owns the hook runtime command handler. Current runtime behavior is commit-msg-only attribution behind the enabled-by-default attribution gate with explicit opt-out controls; `pre-commit` and `post-rewrite` are deterministic no-ops; `post-commit` requires validated `--remote-url`, threads that value through Agent Trace flow, prints it to stderr, and remains an active intersection + Agent Trace DB persistence path; `diff-trace` performs STDIN JSON intake, required-field validation, and best-effort AgentTraceDb insertion with tool-prefixed stored `session_id` values plus direct nullable `model_id` / `tool_version` attribution. Claude structured `PostToolUse` payloads may derive `model_id` from top-level or nested `model` metadata with `claude/` prefix normalization. `session-model` is no longer a supported hooks route. `cli/src/services/hooks/lifecycle.rs` implements `ServiceLifecycle` for hook health checks, fix, and setup (hook rollout integrity and required-hook installation). - `cli/src/services/resilience.rs` defines shared bounded retry/timeout/backoff execution policy (`RetryPolicy`, `run_with_retry`) with deterministic failure messaging and retry observability hooks. -- No `cli/src/services/sync.rs` module exists in the current codebase; `sce sync` command wiring is deferred, while local DB initialization and health ownership are split between setup and doctor. +- `cli/src/services/trace/sync.rs` implements `sce trace sync` orchestration (control-plane authentication, per-stream reconciliation, and report assembly); local DB initialization and health ownership remain split between setup and doctor. See [agent-trace-sync-command.md](agent-trace-sync-command.md). - `cli/src/services/default_paths.rs` defines the canonical per-user persisted-location seam for config/state/cache roots plus named default file paths for current persisted artifacts (`global config`, `auth tokens`, `local DB`, `agent trace DB`) used by config discovery, token storage, database adapters, and doctor diagnostics; its internal `roots` seam now owns the platform-aware root-directory resolution so non-test production modules consume shared path accessors instead of resolving owned roots directly. - `cli/src/services/agent_trace.rs` defines the canonical Rust SCE web base URL and helpers for Agent Trace conversation URLs, persisted Agent Trace trace URLs, Agent Trace session URLs, and setup-created repo-local config schema URLs. - `cli/src/services/token_storage.rs` defines WorkOS token persistence (`save_tokens`, `load_tokens`, `delete_tokens`) via the encrypted `AuthDb` `auth_credentials` table using a `OnceLock` lazy singleton with constant integer row ID `1`. `token_file_path()` returns the auth DB path. `TokenStorageError` exposes `PathResolution` and `Database` variants. No JSON file I/O remains. @@ -146,4 +146,4 @@ A user-invocable `sync` command is not wired in the current CLI surface; local D ## Scope boundary for this phase - This slice establishes compile-safe crate/module boundaries with implemented setup orchestration and deterministic messaging. -- Local Turso DB bootstrap and health coverage are implemented through `setup` and `doctor`, while `sce sync` command wiring and broader cloud behavior remain intentionally deferred. +- Local Turso DB bootstrap and health coverage are implemented through `setup` and `doctor`; `sce trace sync` command wiring is implemented and documented in [agent-trace-sync-command.md](agent-trace-sync-command.md). diff --git a/context/cli/config-precedence-contract.md b/context/cli/config-precedence-contract.md index e50f8c0d..a8136adf 100644 --- a/context/cli/config-precedence-contract.md +++ b/context/cli/config-precedence-contract.md @@ -41,11 +41,16 @@ Resolved observability values that currently have no CLI flag layer follow the s 1. config file value (`log_file_retention_limit`) 2. default (`10`) -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: +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. Two keys are migrated onto this shared path: -1. environment value (`WORKOS_CLIENT_ID`) -2. config file value (`workos_client_id`) -3. baked default (`client_sce_default`) +- `workos_client_id`, which resolves as: + 1. environment value (`WORKOS_CLIENT_ID`) + 2. config file value (`workos_client_id`) + 3. baked default (`client_sce_default`) +- `control_plane_base_url`, the base URL of the control-plane Agent Trace ingestion API, which resolves as: + 1. environment value (`SCE_CONTROL_PLANE_BASE_URL`) + 2. config file value (`control_plane_base_url`) + 3. baked default (`https://sce.crocoder.dev`) When a supported auth-adjacent key omits a baked default, the same resolver still reports `value: null` / `(unset)` with no resolved source when both env and config inputs are absent. @@ -72,7 +77,7 @@ 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_dir`, `log_file_retention_limit`, `timeout_ms`, `workos_client_id`, `agent_trace`, `policies`, `integrations`. +- Allowed keys: `$schema`, `log_level`, `log_format`, `log_dir`, `log_file_retention_limit`, `timeout_ms`, `workos_client_id`, `control_plane_base_url`, `agent_trace`, `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. @@ -80,6 +85,7 @@ When a default-discovered global or repo-local config file exists but fails JSON - `log_file_retention_limit` must be an integer with minimum `1`; zero, negative, fractional, string, and object values fail schema validation. - `timeout_ms` must be an unsigned integer. - `workos_client_id` must be a string when present. +- `control_plane_base_url` must be a non-empty string when present. - `agent_trace` must be an object when present and currently allows only `repository_id` and `repository_remote`. - `agent_trace.repository_id` must be a non-empty string when present. @@ -127,13 +133,14 @@ When a default-discovered global or repo-local config file exists but fails JSON - Runtime config resolution also carries `validation_errors` for skipped invalid discovered config files; `show` maps them into user-facing warnings, while `validate` maps them into validation issues. - Auth-key JSON output in `show` includes `value`, text-oriented `display_value`, `source`, optional `config_source`, and a key-specific `precedence` string describing the allowed resolution chain. - Auth-key text output in `show` includes `auth_precedence` and abbreviates full values when they look credential-like; fully secret-bearing key classes remain redacted. -- For the currently migrated key `workos_client_id`, `show` reports the baked default with `source: default` when env/config inputs are absent. +- For the migrated keys `workos_client_id` and `control_plane_base_url`, `show` reports the baked default with `source: default` when env/config inputs are absent. ## Auth diagnostics contract - Auth failure guidance for migrated auth keys no longer assumes env-only configuration. - Missing-client-id guidance for `workos_client_id` describes the full allowed chain for this key: `WORKOS_CLIENT_ID`, config-file key `workos_client_id`, or fallback to the baked default when no higher-precedence invalid override blocks it. - Auth login runtime guidance refers to the resolved source chain generically (`WORKOS_CLIENT_ID`, config file, or baked default for `workos_client_id`) instead of env-only wording. +- `control_plane_base_url` resolves through the same shared auth-adjacent key path but has no dedicated auth failure guidance of its own; it is consumed by the Agent Trace control-plane client (`sce trace sync`). ## Related files diff --git a/context/cli/trace-command.md b/context/cli/trace-command.md index a16d405e..75e8f086 100644 --- a/context/cli/trace-command.md +++ b/context/cli/trace-command.md @@ -8,6 +8,7 @@ Lives under `cli/src/services/trace/` with these subcommands: - `sce trace db shell [repository-id-or-alias]` — open an embedded in-process SQL shell for the current repository DB by default, or a discovered repository DB by alias/repository ID. - `sce trace status` — render counts and last-activity for the current repository-scoped DB. - `sce trace status --all` — aggregate counts across every discovered repository DB. +- `sce trace sync [--format text|json]` — synchronize the current repository's Agent Trace DB with the control-plane ingestion API (see [Sync — `services::trace::sync`](#sync--servicestracesync) below). `sce trace` operates only on repository-scoped DBs; there is no `--legacy` flag. The `retire-legacy-agent-trace-db` plan removed checkout-scoped discovery/status/shell access. Any pre-migration `/sce/agent-trace-*.db` files left on disk are never touched by SCE and are no longer inspectable through the CLI. @@ -77,8 +78,15 @@ Text output includes `Repository: `, then checkout ID, database p Text rendering shows discovery summary, totals, and a `By database` table with `Alias`, `Scope`, `ID`, `Status`, and count columns. JSON entries use `scope` (`repository`) and `identifier`. +### Sync — `services::trace::sync`, `render_sync` + +`run_current_sync(repo_root)` resolves the current repository's Agent Trace storage through the same `agent_trace_storage` path `sce trace status` uses (not the hook-runtime resolver), builds an `AuthenticatedControlPlaneClient` from the resolved `control_plane_base_url`/`workos_client_id` config, calls the control-plane `/agent-trace/ingestion/state` endpoint once, then synchronizes the four independent capture streams (`messages`, `parts`, `diff_traces`, `agent_traces`, in that fixed order) via the local `AgentTraceExportReader` and the shared per-stream reconciliation engine, producing an `AgentTraceSyncReport`. A genuinely ambiguous batch outcome (`5xx`, transport failure, invalid response) reconciles by refetching `/state`; a terminal control-plane failure (missing/invalid credentials, `400`, `403`) fails the stream immediately without an extra network call, so a `403` never mutates local repository metadata or retries. No local sync cursor, cursor file, or database is created — every invocation starts from the authoritative `/state` cursors, so repeated runs are naturally incremental. + +`render_sync::render(report, format)` renders the converged `AgentTraceSyncReport`. Text output is a `style::heading("Agent Trace sync complete.")` line, `Repository ID:`/`Source instance ID:` lines, then a padded table with one row per stream (`Stream`, `Uploaded`, `Final cursor`) in the fixed `messages → parts → diff_traces → agent_traces` order — no per-batch or per-row detail is printed. JSON output carries `status`, `command`, `subcommand`, `repositoryId`, `sourceInstanceId`, and `streams.{messages,parts,diffTraces,agentTraces}`, each with `uploaded`/`initialCursor`/`finalCursor`/`batches`; the JSON stream keys are camelCase (`diffTraces`/`agentTraces`) even though the internal `StreamSyncReports` struct fields are `diff_traces`/`agent_traces`. `TraceCommand::execute` dispatches `TraceSubcommandRequest::Sync { format }` to `render_sync::render`, completing the command surface end to end. + ## Related context +- [agent-trace-sync-command.md](agent-trace-sync-command.md) — composed local-to-control-plane sync architecture, user flow, no-local-persistence invariants, and recovery semantics. - [agent-trace-storage.md](agent-trace-storage.md) — repository-scoped storage resolver and active DB path contract. - [checkout-identity.md](checkout-identity.md) — checkout identity diagnostics and never-touch on-disk handling of pre-migration DB files. - [default-path-catalog.md](default-path-catalog.md) — Agent Trace DB path ownership. diff --git a/context/context-map.md b/context/context-map.md index 4f64b6b8..3f0500cd 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -9,7 +9,7 @@ Primary context files: Feature/domain context: -- `context/cli/cli-command-surface.md` (CLI command surface including top-level help with ASCII art banner and gradient rendering, setup install flow with the repeatable `sce setup --workflow ` optional-workflow selection and its interactive post-target multi-select, WorkOS device authorization flow + token storage behavior, attribution-only hook routing with validated post-commit `--remote-url` plumbing plus DB-backed `diff-trace` dual persistence and post-commit Agent Trace payload persistence including range `content_hash`, setup-owned local DB + repository-scoped Agent Trace DB bootstrap plus doctor DB health coverage with credential-safe repository identity diagnostics, centralized Rust SCE web URL helpers in `services::agent_trace`, nested flake release package/app installability, Cargo local install + crates.io readiness policy, and hidden `sce policy bash` command adapter for bash-policy hook callers; `sce sync` command wiring is deferred to `0.4.0`; static `RuntimeCommand` enum dispatch lives in `services/command_registry.rs`, command payload structs for help/version/completion/auth/config/setup/doctor/hooks/policy are owned by their respective `services/{name}/command.rs` files, and clap-to-runtime conversion lives in `services/parse/command_runtime.rs`) +- `context/cli/cli-command-surface.md` (CLI command surface including top-level help with ASCII art banner and gradient rendering, setup install flow with the repeatable `sce setup --workflow ` optional-workflow selection and its interactive post-target multi-select, WorkOS device authorization flow + token storage behavior, attribution-only hook routing with validated post-commit `--remote-url` plumbing plus DB-backed `diff-trace` dual persistence and post-commit Agent Trace payload persistence including range `content_hash`, setup-owned local DB + repository-scoped Agent Trace DB bootstrap plus doctor DB health coverage with credential-safe repository identity diagnostics, centralized Rust SCE web URL helpers in `services::agent_trace`, nested flake release package/app installability, Cargo local install + crates.io readiness policy, and hidden `sce policy bash` command adapter for bash-policy hook callers; implemented `sce trace sync` command wiring synchronizing the current repository's Agent Trace DB with the control plane; static `RuntimeCommand` enum dispatch lives in `services/command_registry.rs`, command payload structs for help/version/completion/auth/config/setup/doctor/hooks/policy are owned by their respective `services/{name}/command.rs` files, and clap-to-runtime conversion lives in `services/parse/command_runtime.rs`) - `context/cli/default-path-catalog.md` (canonical production CLI path-ownership contract centered on `cli/src/services/default_paths.rs`, including persisted auth/config files, named DB paths for auth/local/repository-scoped Agent Trace databases, the default observability log-dir accessor consumed by config resolution with Linux `${XDG_STATE_HOME:-~/.local/state}/sce/logs` fallback semantics, repo-relative, embedded-asset, install, hook, and context-path families plus the regression guard that keeps production path ownership centralized) - `context/cli/repository-identity.md` (repository identity module in `cli/src/services/repository_identity/`: pure scheme-neutral `host[:port]/path` canonicalization for SCP/`ssh://`/HTTPS/`git://` remote URLs with credential stripping, hostname lowercasing, default-port removal, and query/fragment/trailing-`.git` cleanup, trim-only explicit-identity handling, `sha256("sce-repository-id-v1\0" + canonical_identity)` repository IDs, credential-safe fieldless errors, plus the `resolve` runtime submodule applying explicit-config-then-configured-remote precedence with `git config --get remote..url` lookup, `RepositoryIdentitySource` provenance, and `.sce/config.json`-guidance resolution errors that never echo URLs; consumed by the T04 `agent_trace_storage` resolver) - `context/cli/agent-trace-storage.md` (repository-scoped Agent Trace storage resolver in `cli/src/services/agent_trace_storage/`: `AgentTraceStorageContext` inputs mirroring the `agent_trace.*` config keys, `ResolvedAgentTraceStorage` carrying repository identity + checkout ID + `/sce/repos//agent-trace.db` path + open `RepositoryAgentTraceDb` + typed `RepositoryMetadata`, `resolve_agent_trace_storage{,_at_state_root}` setup/lifecycle entrypoints with idempotent concurrent-safe first open via bounded fast-path-then-migrate retry plus narrow one-file schema migration-metadata repair and repository metadata validation, a separate no-migration `resolve_agent_trace_storage_for_hook_runtime{,_at_state_root}` pair that high-frequency hook callers use exclusively and that never runs migration `002` or any migration, path-unsafe repository ID rejection in `default_paths::agent_trace_db_path_for_repository{,_at}`, strict never-touch boundary for any pre-migration checkout-scoped/global DB files, and active hook/runtime plus Agent Trace lifecycle setup call-site consumption after T08) @@ -17,7 +17,8 @@ Feature/domain context: - `context/cli/patch-service.md` (standalone patch domain model, parser, JSON load helpers, and set operations in `cli/src/services/patch.rs` for in-memory parsed unified-diff representation, capturing only touched lines plus minimal per-file/per-hunk metadata, supporting both `Index:` SVN-style and `diff --git` git-style formats, with `ParseError` for actionable malformed-input diagnostics, `PatchLoadError`/`load_patch_from_json`/`load_patch_from_json_bytes` for storage-agnostic JSON reconstruction, `intersect_patches` for target-shaped overlap with exact-match-first and historical `kind`+`content` fallback semantics plus matched-constructed-line `session_id` and matched-constructed-hunk `model_id` provenance inheritance, and `combine_patches` for ordered patch combination with later-wins conflict resolution plus winning-hunk `model_id` provenance inheritance; `parse_patch`, `intersect_patches`, and `combine_patches` are consumed by the active post-commit hook runtime) - `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: repository-scoped-only discovery of `/sce/repos//agent-trace.db` with mtime-desc + repository-id tiebreak alias assignment and required-table readiness probing, no `--legacy` flag or checkout-scoped access, implemented `sce trace db shell` current-repository opening plus alias/repository-ID resolution without external `turso`, implemented `sce trace db list` text + JSON rendering using `services::style::heading` with scope/identifier fields, implemented repository-scoped `sce trace status` with checkout ID diagnostics, implemented `sce trace status --all` aggregation across discovered repository DBs, and the completed removal of `sce doctor dbs` whose discovery scan/rendering moved into `services::trace`) +- `context/cli/trace-command.md` (`sce trace` command group: repository-scoped-only discovery of `/sce/repos//agent-trace.db` with mtime-desc + repository-id tiebreak alias assignment and required-table readiness probing, no `--legacy` flag or checkout-scoped access, implemented `sce trace db shell` current-repository opening plus alias/repository-ID resolution without external `turso`, implemented `sce trace db list` text + JSON rendering using `services::style::heading` with scope/identifier fields, implemented repository-scoped `sce trace status` with checkout ID diagnostics, implemented `sce trace status --all` aggregation across discovered repository DBs, implemented `sce trace sync` control-plane synchronization orchestration with documented text/JSON rendering complete, and the completed removal of `sce doctor dbs` whose discovery scan/rendering moved into `services::trace`) +- `context/cli/agent-trace-sync-command.md` (composed local-to-control-plane `sce trace sync` architecture: the `hooks/plugins → repository Agent Trace DB → AgentTraceExportReader → sce trace sync → HTTPS + WorkOS Bearer → control plane` data flow, the `sce auth login` / `cd ` / `sce trace sync` user flow, the no-local-cursor/no-`agent-trace-sync.db`/no-Turso-Sync/no-`BridgeLock`/no-local-DWH invariants, and `401`/`409`/ambiguous-batch-failure recovery semantics) - `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` with `/sce/logs` defaulting plus config-file/default-only positive `log_file_retention_limit`, config-file-only `agent_trace.repository_id`/`agent_trace.repository_remote` repository-identity keys with default remote `origin`, the catalog-derived `integrations.optional_workflows` optional-workflow selection key, JSON-pointer-prefixed schema-validation errors, 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 repository-scoped Agent Trace DB initialization plus checkout identity diagnostics, 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) diff --git a/context/glossary.md b/context/glossary.md index ef768430..ad0ca0b4 100644 --- a/context/glossary.md +++ b/context/glossary.md @@ -62,7 +62,7 @@ - `cli cargo install contract`: Supported Cargo install surface for the `shared-context-engineering` crate, which installs the `sce` binary: crates.io (`cargo install shared-context-engineering --locked`) and local checkout (`./scripts/run-cli-cargo.sh install --path cli --locked`). Direct `cargo install --git` is unsupported because it has no repository pre-Cargo generation boundary. - `cli crates.io publication posture`: Current Cargo package posture in `cli/Cargo.toml` where crates.io-facing metadata is publication-ready for the `shared-context-engineering` crate, with crate-facing install guidance owned by `cli/README.md`. - `Nix performance recommendations`: Repo-local operator guidance in `AGENTS.md` covering optional user-level `~/.config/nix/nix.conf` tuning (`max-jobs = auto`, `cores = 0`) and the explicit root/admin-only boundary for `/etc/nix/nix.conf` `auto-optimise-store = true`. -- `sce` (CLI foundation): Rust binary crate at `cli/` with implemented auth command flows (`auth login|renew|logout|status`) plus auth-local bare-command guidance (`sce auth`, `sce auth --help`), implemented setup installation flow including lifecycle-aggregated local DB and Agent Trace DB bootstrap, implemented attribution-only `hooks` subcommand routing/validation entrypoints, and no user-invocable `sync` command yet (`sce sync` wiring is deferred to `0.4.0`). +- `sce` (CLI foundation): Rust binary crate at `cli/` with implemented auth command flows (`auth login|renew|logout|status`) plus auth-local bare-command guidance (`sce auth`, `sce auth --help`), implemented setup installation flow including lifecycle-aggregated local DB and Agent Trace DB bootstrap, implemented attribution-only `hooks` subcommand routing/validation entrypoints, and a fully implemented `sce trace sync` command that synchronizes the current repository's Agent Trace DB with the control plane and renders the documented text/JSON output (see `context/cli/trace-command.md`). - `command surface contract`: The current top-level command/help catalog split where `cli/src/cli_schema.rs` owns the real clap-backed command metadata (top-level purpose text plus help visibility for `auth`, `config`, `setup`, `doctor`, `hooks`, `policy`, `version`, and `completion`) and `cli/src/command_surface.rs` consumes that catalog for the custom banner/help surface plus known-command classification, while still adding the synthetic `help` row. - `top-level help visibility metadata`: Per-command `show_in_top_level_help` metadata in `cli/src/cli_schema.rs` that controls whether a known command appears in `sce`, `sce help`, and `sce --help` without affecting direct invocation; the current hidden top-level commands are `auth`, `hooks`, and `policy`, and `cli/src/command_surface.rs` renders the curated top-level help list from that shared metadata. - `command loop`: The `clap` derive-based parser + dispatcher in `cli/src/cli_schema.rs`, `cli/src/services/parse/command_runtime.rs`, and `cli/src/app.rs` that routes `help`, `config`, `setup`, `doctor`, `auth`, `hooks`, `policy`, `version`, and `completion`, executes implemented command flows, emits command-local help payloads for supported subcommand trees, and returns deterministic actionable errors for invalid invocation. @@ -91,7 +91,7 @@ - `DB query retry policy`: Retry policy used by `TursoDb::execute()`, `TursoDb::query()`, `TursoDb::query_map()`, `EncryptedTursoDb::execute()`, `EncryptedTursoDb::query()`, and `EncryptedTursoDb::query_map()` for local Turso operation retry, resolved from `policies.database_retry..query` via the `DATABASE_RETRY_CONFIG` `OnceLock` with fallback to hardcoded defaults (`5` attempts, `200ms` elapsed-attempt timeout, `25ms` initial backoff, `100ms` max backoff; default worst-case failure budget `<= 2_000ms`) through `run_with_retry_sync`. `query_map()` retries the initial query and row-fetch loop, then runs caller row mapping outside retry. - `__sce_migrations`: Per-database migration metadata table created by the shared `TursoConnectionCore` migration path behind public adapter `run_migrations()` methods; records applied migration IDs after successful execution so later setup/lifecycle initialization applies only migrations not yet recorded, while existing metadata-less DBs are brought forward by re-applying the current idempotent migration set and recording each ID. - `CLI generated migration manifest`: Build-time Rust source at `OUT_DIR/generated_migrations.rs` written by `cli/build.rs` from immediate `cli/migrations//*.sql` directories after staging SQL under `OUT_DIR/static/migrations`; constants are named from the database directory (for example `AGENT_TRACE_REPOSITORY_MIGRATIONS`, `AUTH_MIGRATIONS`), sorted by the numeric filename prefix before `_`, and embed staged SQL via `include_str!`. -- `sync command deferral`: Current plan/state note that a user-invocable `sce sync` command is not wired yet and is deferred to `0.4.0`; local DB bootstrap and setup-time repository-scoped Agent Trace DB initialization flow through lifecycle providers aggregated by the setup command, while hook runtime keeps a lazy repository Agent Trace DB fallback for repositories where setup has not run or schema metadata is incomplete, and DB health/repair flows through the doctor surface. +- `sync command deferral` (historical): Former plan/state note that a user-invocable sync command was deferred to `0.4.0`; superseded now that `sce trace sync` is wired (see `context/cli/trace-command.md`). Local DB bootstrap and setup-time repository-scoped Agent Trace DB initialization still flow through lifecycle providers aggregated by the setup command, hook runtime still keeps a lazy repository Agent Trace DB fallback for repositories where setup has not run or schema metadata is incomplete, and DB health/repair still flows through the doctor surface. - `CLI bounded resilience wrapper`: Shared policy in `cli/src/services/resilience.rs` (`RetryPolicy`, async `run_with_retry`, sync `run_with_retry_sync`) that applies deterministic retries/timeouts/capped backoff to transient operations, emits retry observability events, and returns actionable terminal failure guidance. The sync helper is currently wired into shared database constructors for local open/connect retry and into `TursoDb`/`EncryptedTursoDb` operation retry for `execute()`/`query()`/`query_map()`. - `setup service orchestration`: Setup execution logic in `cli/src/services/setup/command.rs` that resolves the repository root, always ensures the durable-context baseline, then for normal modes derives a repo-root-scoped `AppContext` from the runtime command context, dispatches `setup` through the static lifecycle provider catalog (config → local_db → auth_db → agent_trace_db → hooks when requested), handles interactive target selection for config asset installation, and emits deterministic success messaging per target. - `setup target flags`: Mutually-exclusive `sce setup` target selectors (`--opencode`, `--claude`, `--pi`, `--all`) that force non-interactive mode for automation; `--all` expands to opencode+claude+pi and replaced the removed `--both` flag. @@ -161,7 +161,7 @@ - `setup remove-and-replace`: Replacement choreography in `cli/src/services/setup/mod.rs` where existing install targets are removed before staged content is promoted; on swap failure, the engine cleans temporary staging paths and returns deterministic recovery guidance (recover from version control). No backup artifacts are created. - `hooks command routing contract`: Current hook command parser/dispatcher plus runtime wiring in `cli/src/services/hooks/mod.rs` (`HookSubcommand`, `run_hooks_subcommand`) supports `pre-commit`, `commit-msg `, `post-commit`, `post-rewrite `, `diff-trace`, and `conversation-trace` with deterministic invocation validation/usage errors; `session-model` is no longer supported. `commit-msg` is the only active attribution path behind the attribution hooks gate and staged-diff AI-overlap preflight; `pre-commit`/`post-rewrite` are no-ops; `post-commit` persists intersections and built Agent Trace payloads; `diff-trace` persists DB-only AgentTraceDb rows using tool-prefixed `session_id` values plus direct `model_id` and `tool_version` values (session-model fallback was removed in the `remove-session-models-direct-claude-model-id` plan). - `Claude raw hook capture (removed)`: Former hidden/internal `sce hooks claude-capture ` intake path removed in T05 of the `claude-typescript-model-cache-remove-rust-capture` plan. Rust now exposes `diff-trace` and `conversation-trace` intakes for active Claude/OpenCode editor runtimes; `session-model` is also removed from the supported hook command surface. The removed route previously wrote pretty-printed JSON artifacts under `context/tmp/claude/` without AgentTraceDb writes. See `context/sce/claude-raw-hook-capture.md`. -- `deferred sync command`: `sce sync` has no command wiring and no `cli/src/services/sync.rs` module in the current runtime. Local DB initialization and health ownership are split between setup and doctor instead. +- `deferred sync command` (historical): There is no top-level `sce sync` command or `cli/src/services/sync.rs` module; the implemented sync surface is `sce trace sync` under the `trace` command group (see `context/cli/trace-command.md` and `context/cli/agent-trace-sync-command.md`). Local DB initialization and health ownership are still split between setup and doctor. - `sce CLI onboarding guide`: Crate-local documentation at `cli/README.md` that defines runnable placeholder commands, non-goals/safety limits, and roadmap mapping to service modules. - `plan/code overlap map`: Context artifact at `context/sce/plan-code-overlap-map.md` that classifies the thin OpenCode Plan/Code agents, `/change-to-plan`, `/next-task`, `/validate`, and their phase skills into routing, orchestration, and behavior ownership boundaries. - `SCE dedup ownership table`: Context artifact at `context/sce/dedup-ownership-table.md` that assigns one canonical owner per shared behavior domain, lists reference-only consumers, and labels each overlap as `intentional/keep` or `dedup/remove`. diff --git a/context/overview.md b/context/overview.md index ff109f4c..c0dbc108 100644 --- a/context/overview.md +++ b/context/overview.md @@ -36,7 +36,7 @@ The same config resolver now also owns the attribution-hooks gate used by local The config service split now includes `cli/src/services/config/resolver.rs` as the focused owner for config-file discovery, file-layer merging, env/flag/default precedence, auth-key resolution, observability resolution, attribution-hooks resolution, and default-discovered invalid-file degradation; `cli/src/services/config/mod.rs` remains the facade/rendering orchestration surface while preserving existing `services::config` imports. Generated config now includes repo-local OpenCode plugin assets: `sce-bash-policy.ts` plus `sce-agent-trace.ts` are emitted under `config/.opencode/plugins/`; the OpenCode agent-trace plugin extracts `{ sessionID, diff, time, model_id }` from user `message.updated` events with diffs, tracks per-session OpenCode client version from `session.created`/`session.updated`, and sends payloads to `sce hooks diff-trace` with `tool_name="opencode"` plus optional `tool_version`. Claude generated config now routes supported `PostToolUse Write|Edit|MultiEdit|NotebookEdit` events directly to `sce hooks diff-trace`; it no longer registers a `SessionStart` hook or calls `sce hooks session-model`. Rust handles extraction, validation, and persistence without a TypeScript intermediary; the former `config/.claude/plugins/sce-agent-trace.ts` Bun runtime was removed in T07 of the `claude-rust-diff-trace` plan. The Rust hook validates required fields, uses only direct payload `model_id` and `tool_version` (no longer resolves from `session_models`), and persists tool-prefixed `session_id` values (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi), `model_id`, `tool_name`, and nullable `tool_version` into `diff_traces` through AgentTraceDb. Bash-policy now delegates OpenCode enforcement to the Rust `sce policy bash` command: the generated OpenCode plugin at `config/.opencode/plugins/sce-bash-policy.ts` is a thin wrapper that calls `sce policy bash --input normalized --output json` via `spawnSync` and throws on deny decisions; it no longer contains independent TypeScript policy logic. The former `bash-policy/runtime.ts` TypeScript runtime has been removed. Preset... Claude bash-policy enforcement is also generated through `.claude/settings.json` as a `PreToolUse` `Bash` command hook running `sce policy bash`, so Claude and OpenCode both delegate to the Rust policy evaluator without a Claude TypeScript runtime. Pi bash-policy enforcement is delegated the same way through a project-local Pi extension (`config/lib/pi-plugin/sce-pi-extension.ts`, emitted to `config/.pi/extensions/sce/index.ts`) whose `tool_call` handler blocks denied bash commands via `sce policy bash` and fails open when the policy check cannot run (see `context/sce/pi-extension-runtime.md`). -Local database bootstrap is now owned by `LocalDbLifecycle::setup` and `AgentTraceDbLifecycle::setup` aggregated by the setup command. Agent Trace setup creates/reuses the current checkout ID for diagnostics and initializes the repository-scoped `/sce/repos//agent-trace.db` with the repository schema; hook runtime lazily creates or upgrades that repository DB when setup has not run or schema metadata is incomplete. Doctor validates the repository-scoped DB path/health and can bootstrap missing parent directories; outside a Git repository it reports an actionable "requires a Git repository" diagnostic instead of probing a sentinel path. The `sce trace` group operates only on repository-scoped DBs for list/status/status-all/shell UX; the checkout-scoped `--legacy` surface was removed by the `retire-legacy-agent-trace-db` plan (see `context/decisions/2026-07-17-retire-legacy-agent-trace-db.md`). Wiring a user-invocable `sce sync` command is deferred to `0.4.0`. +Local database bootstrap is now owned by `LocalDbLifecycle::setup` and `AgentTraceDbLifecycle::setup` aggregated by the setup command. Agent Trace setup creates/reuses the current checkout ID for diagnostics and initializes the repository-scoped `/sce/repos//agent-trace.db` with the repository schema; hook runtime lazily creates or upgrades that repository DB when setup has not run or schema metadata is incomplete. Doctor validates the repository-scoped DB path/health and can bootstrap missing parent directories; outside a Git repository it reports an actionable "requires a Git repository" diagnostic instead of probing a sentinel path. The `sce trace` group operates only on repository-scoped DBs for list/status/status-all/shell UX; the checkout-scoped `--legacy` surface was removed by the `retire-legacy-agent-trace-db` plan (see `context/decisions/2026-07-17-retire-legacy-agent-trace-db.md`). `sce trace sync` is now fully implemented: it resolves repository-scoped storage, authenticates against the control plane with stored WorkOS credentials, synchronizes the four Agent Trace capture streams, and renders the documented concise text/JSON output (see `context/cli/trace-command.md`). The repository-root flake (`flake.nix`) applies a Rust overlay-backed stable toolchain pinned to `1.95.0` (with `rustfmt` and `clippy`), reads package/check version from the repo-root `.version` file, and builds `packages.sce` through a Crane `buildDepsOnly` + `buildPackage` pipeline. One deterministic pre-Cargo Nix derivation invokes the shared generated-input producer and supplies its validated `SCE_CLI_GENERATED_INPUT_DIR` store path to native, release, test, and Clippy Cargo derivations. Pkl is absent from those Cargo environments; dependency-only and format derivations do not receive the handoff, so canonical generation changes invalidate compiling outputs without invalidating dependency artifacts or formatting. `cli-tests`, `cli-clippy`, and `cli-fmt` remain Crane-backed check derivations. The root flake splits native and release outputs: `packages.sce` and `packages.default` build the **native** development binary (`scePackage`), while `packages.sce-release` builds the release binary (`sceReleasePackage`: static musl on Linux, native on Darwin). So `nix build .#sce` / `.#default`, `nix run . -- --help`, `nix run .#sce -- --help`, and `nix profile install github:crocoder-dev/shared-context-engineering` target the native binary, and `nix build .#sce-release` / `nix run .#sce-release -- ...` (plus `nix run .#release-artifacts`, which builds `.#sce-release`) target the release binary. On Linux the native and release outputs are distinct store paths, and the release output passes the native portability audit. `packages..ci-checks` is the explicit long-running validation tier: `nix build .#ci-checks` builds the `.#sce-release` package and, on Linux, audits the real release binary for forbidden `/nix/store/` references, so the expensive work stays out of `nix flake check` (which never builds `.#sce-release`). Git-commit embedding is **release-only**: `SCE_GIT_COMMIT` is injected via a `releaseCommitArgs` fragment applied only to the release derivations (`scePackageMusl` on Linux, `sceReleasePackageNative` on Darwin), not to `commonCargoArgs`. So native `.#sce`/`.#default` and every `nix flake check` derivation (`cli-tests`, `cli-clippy`, `cli-fmt`) build without the commit in their inputs and stay cache-reusable across commits (native `sce version` reports `unknown`), while `.#sce-release` still reports the real commit via `sce version`. `cli/build.rs` `emit_git_commit` emits `SCE_GIT_COMMIT` only when the env var is explicitly set — no `git rev-parse` fallback and no `.git/HEAD`/`.git/packed-refs` rerun watches. On Darwin the release now uses a distinct native-toolchain derivation (native toolchain + commit), so it diverges from `.#sce` to carry the commit while native stays commit-independent. diff --git a/context/plans/agent-trace-sync-hardening.md b/context/plans/agent-trace-sync-hardening.md new file mode 100644 index 00000000..b80560e6 --- /dev/null +++ b/context/plans/agent-trace-sync-hardening.md @@ -0,0 +1,282 @@ +# Plan: agent-trace-sync-hardening + +## Change summary + +`sce trace sync` (PR #200, `context/plans/agent-trace-sync.md`, now complete) is functionally +wired but has three correctness/hardening gaps ahead of merge, all inside +`cli/src/services/agent_trace_sync/` and `cli/src/services/trace/sync.rs`: + +1. `AgentTraceIngestionStateResponse.cursors` (`AgentTraceCursors { messages, parts, diff_traces, + agent_traces }` in `control_plane.rs`) deserializes any syntactically valid `i64` from + `/state`, including negative values or values above `Number.MAX_SAFE_INTEGER`. Nothing rejects + an out-of-range cursor before it reaches `AgentTraceExportReader::read_*_after(...)` or + `sync_stream`. +2. `AuthenticatedControlPlaneClient::resolve_access_token` calls `auth::is_stored_token_expired` + and then `auth::ensure_valid_token_returning_token` as two separate expiry checks. If time + advances between them at the expiry boundary, the second check refreshes a token the first + check considered valid, and the `was_expired` flag (computed from the first check) suppresses + the save — a refreshed token can be used without ever being persisted. +3. `classify_response` in `control_plane.rs` puts the raw HTTP response body directly into + `ControlPlaneError::{BadRequest,Forbidden,Conflict,ServerError,InvalidResponse}`, so a server + implementation detail (SQL error, stack trace, HTML) can reach the CLI's user-visible error + text verbatim. Separately, unexpected statuses such as `404`/`405`/`415`/`422` are currently + folded into `InvalidResponse`, which `trace/sync.rs::is_stream_terminal` does not treat as + terminal — so a clear protocol/API mismatch during `/batch` incorrectly enters the ambiguous + `/state`-reconciliation loop instead of failing immediately. + +This plan closes all three gaps without touching the sync architecture, the command surface, the +no-local-state invariant, or PR #197/#198 source-identity/export semantics. It only tightens +validation and error classification inside the already-shipped client/engine/orchestration. + +## Acceptance criteria + +- [x] AC1: Every `/state` response is validated so all four cursor fields satisfy + `0 <= cursor <= 9_007_199_254_740_991` before the response is returned to any caller; an + out-of-range value in any field yields `ControlPlaneError::InvalidResponse` and never reaches + `AgentTraceExportReader::read_*_after` or `sync_stream`, whether the response came from the + initial `/state` call or from `409`/ambiguous-result reconciliation. + - Validate: `cargo test --manifest-path cli/Cargo.toml agent_trace_cursors` + - Validate: `cargo test --manifest-path cli/Cargo.toml invalid_state_cursor_fails_before_any_batch_request` +- [x] AC2: Access-token resolution makes exactly one expiry decision. A valid stored token is + reused with zero refresh-endpoint calls and zero `CredentialStore::save` calls; an expired + stored token triggers exactly one refresh-endpoint call and exactly one `save` call, and the + control-plane request uses the refreshed token. No code path can use a refreshed token without + persisting it. + - Validate: `cargo test --manifest-path cli/Cargo.toml valid_token_is_reused_without_resave valid_token_is_not_resaved expired_token_is_refreshed_and_saved` +- [x] AC3: The existing unexpected-`401` behavior is unchanged: one forced refresh, one save, one + retry; a second `401` fails with authentication guidance and no further retry. + - Validate: `cargo test --manifest-path cli/Cargo.toml unexpected_401_refreshes_once_and_retries_once_on_success unexpected_401_twice_fails_without_a_third_attempt` +- [x] AC4: No control-plane error surfaced to the CLI contains an arbitrary raw server response + body. Only a narrow, length-bounded `message`/`error` string field is ever extracted from a + JSON body; malformed, HTML, oversized, empty, or non-string-field bodies fall back to a generic + per-status message. + - Validate: `cargo test --manifest-path cli/Cargo.toml server_error_body_is_not_leaked known_safe_error_payload_is_surfaced html_error_body_falls_back_to_generic_message` +- [x] AC5: During `/batch`, a clearly terminal protocol/API mismatch (`404`, `405`, `415`, `422`, + and other terminal `4xx`, plus a post-refresh `401`) fails the stream immediately without + entering `/state` reconciliation, while a `409`, a transport failure, a `5xx`, and a + syntactically successful (`2xx`) but undecodable batch body all still reconcile via `/state` + exactly as before. + - Validate: `cargo test --manifest-path cli/Cargo.toml terminal_batch_status_fails_without_state_reconciliation malformed_2xx_batch_response_still_reconciles_via_state` + +### Full validation + +- `cargo test --manifest-path cli/Cargo.toml` +- `cargo clippy --manifest-path cli/Cargo.toml -- -D warnings` +- `nix run .#pkl-check-generated` +- `nix flake check` + +### Context sync + +- `context/cli/agent-trace-sync-command.md` — its "Recovery semantics" section must describe the + terminal-vs-ambiguous split for `4xx` batch responses and the sanitized-error-body contract + once implemented. + +## Constraints and non-goals + +- **In scope:** `cli/src/services/agent_trace_sync/control_plane.rs`, + `cli/src/services/agent_trace_sync/mod.rs`, `cli/src/services/trace/sync.rs`, and their test + modules; `context/cli/agent-trace-sync-command.md`. +- **Out of scope:** the sync architecture, the `sce trace sync` command surface, source + identity/export semantics (PR #197/#198), the control-plane API, background sync, workspace + selection, local synchronization state of any kind. +- **Constraints:** reuse `agent_trace_export::JS_MAX_SAFE_INTEGER` / + `validate_js_safe_integer` rather than duplicating the numeric bound; reuse + `auth::renew_stored_token_from_refresh_token` rather than duplicating WorkOS refresh HTTP + logic; do not call the device-authorization flow from the sync client. +- **Non-goal:** redesigning `ControlPlaneError` beyond what's needed to distinguish terminal + protocol/API mismatches from ambiguous batch outcomes and to carry a sanitized message instead + of a raw body. + +## Task stack + +- [x] T01: `Validate /state cursors against the JS-safe-integer range` (status:done) + - Task ID: T01 + - Goal: `AgentTraceIngestionStateResponse` cursors are validated immediately after JSON decoding + in `AuthenticatedControlPlaneClient::send_state_request`, before the response reaches any + caller (the initial `/state` call in `trace/sync.rs::run_sync_against` and the + `/state` refetch inside `sync_one_stream`'s `refresh_cursor` closure use the same client + method, so one change point covers both). + - Boundaries (in/out of scope): In — add `impl AgentTraceCursors { pub fn validate(&self) -> + Result<(), ControlPlaneError> }` in `control_plane.rs` calling + `agent_trace_export::validate_js_safe_integer` on all four fields and mapping a failure to + `ControlPlaneError::InvalidResponse`; call it from `send_state_request` before returning. + Out — changing `AgentTraceCursors`'s field types, changing `/state`'s request shape, changing + `AgentTraceExportReader`. + - Dependencies: none + - Done when: a `/state` response with any cursor field `< 0` or `> 9_007_199_254_740_991` + yields `ControlPlaneError::InvalidResponse` from `ingestion_state`, for each of the four + fields independently; boundary values `0`, `1`, and `9_007_199_254_740_991` are accepted; an + orchestration-level test (mocked control plane returning an invalid `/state` cursor) proves + `run_sync_against` fails before any `/batch` request is sent. + - Verification notes (commands or checks): `cargo test --manifest-path cli/Cargo.toml agent_trace_cursors`; `cargo test --manifest-path cli/Cargo.toml invalid_state_cursor_fails_before_any_batch_request` + - Completed: 2026-08-11 + - Files changed: `cli/src/services/agent_trace_sync/control_plane.rs`, `cli/src/services/trace/sync.rs` + - Evidence: Added `AgentTraceCursors::validate`, called from `send_state_request` after JSON + decoding, so both the initial `/state` call and the `/state` refetch used by reconciliation + are covered by one change point. Added `agent_trace_cursors_accept_boundary_values` and + `agent_trace_cursors_reject_out_of_range_value_in_each_field` (table-driven across all four + fields, boundaries `0`/`1`/`9_007_199_254_740_991` valid, `-1`/`9_007_199_254_740_992` + invalid) in `control_plane.rs`, and `invalid_state_cursor_fails_before_any_batch_request` in + `trace/sync.rs` (mocked `/state` returning an out-of-range `agentTraces` cursor; asserts + `TraceSyncError::ControlPlane(ControlPlaneError::InvalidResponse(_))` and + `server.call_count() == 1`, proving no `/batch` request was sent). `cargo fmt --manifest-path + cli/Cargo.toml` applied to satisfy formatting. Ran `nix flake check`: all checks passed + (`cli-tests`, `cli-clippy`, `cli-fmt`, plus the repo's other checks). + - Notes: none. + +- [x] T02: `Make access-token expiry a single decision` (status:done) + - Task ID: T02 + - Goal: Rewrite `AuthenticatedControlPlaneClient::resolve_access_token` to call + `auth::is_stored_token_expired` exactly once and branch on that single result — refresh via + `auth::renew_stored_token_from_refresh_token` and save only in the expired branch, otherwise + return the stored access token unchanged with no HTTP call and no save. + - Boundaries (in/out of scope): In — `resolve_access_token` body only. Out — + `force_refresh_access_token` (the unexpected-`401` path), `execute_authenticated`, + `auth::ensure_valid_token_returning_token` itself (still used by `auth_command`), the device + authorization flow. + - Done when: a valid stored token path makes zero refresh-endpoint HTTP calls and zero + `CredentialStore::save` calls; an expired stored token path makes exactly one refresh-endpoint + call, exactly one `save` call, and the control-plane request carries the refreshed token; the + existing `valid_token_is_reused_without_resave`, `valid_token_is_not_resaved`, and + `expired_token_is_refreshed_and_saved` tests pass against the rewritten implementation + without depending on wall-clock timing. + - Dependencies: none + - Verification notes (commands or checks): `cargo test --manifest-path cli/Cargo.toml control_plane::tests` + - Completed: 2026-08-11 + - Files changed: `cli/src/services/agent_trace_sync/control_plane.rs` + - Evidence: Rewrote `resolve_access_token` to call `auth::is_stored_token_expired` exactly + once and branch on that single result: a non-expired stored token is returned immediately + with no HTTP call and no save, while an expired one is refreshed via + `auth::renew_stored_token_from_refresh_token` and saved before its access token is returned. + Removed the prior two-call sequence through `auth::ensure_valid_token_returning_token` + (still used elsewhere by `auth_command`, left unchanged) that let the two expiry checks + disagree at the boundary. `valid_token_is_reused_without_resave`, + `valid_token_is_not_resaved`, and `expired_token_is_refreshed_and_saved` pass unmodified + against the rewritten implementation, as do the unexpected-`401` tests + (`force_refresh_access_token`/`execute_authenticated` untouched). Ran + `nix build .#checks.x86_64-linux.cli-tests`: all 280 tests pass. Ran + `nix build .#checks.x86_64-linux.cli-clippy`: clean. + - Notes: none. + +- [x] T03: `Add safe HTTP error-body parsing and a terminal-protocol error variant` (status:done) + - Task ID: T03 + - Goal: Replace raw-body exposure in `classify_response` with a narrow safe parser, and add a + `ControlPlaneError` variant for terminal-but-otherwise-uncategorized `4xx` statuses + (`404`/`405`/`415`/`422` and similar) distinct from `InvalidResponse`, which becomes reserved + for a syntactically successful (`2xx`) but undecodable body. + - Boundaries (in/out of scope): In — a private `fn extract_safe_error_message(body: &str) -> + Option` that accepts only a top-level JSON object with a `message` or `error` string + field under a fixed max length, returning `None` for malformed JSON, HTML, non-object/array + top-level values, missing/non-string fields, or oversized bodies; `classify_response` uses it + to build sanitized `BadRequest`/`Forbidden`/`Conflict`/`ServerError` messages with a + status-specific generic fallback (e.g. `400` -> "control plane rejected the Agent Trace + request", `403` -> "Agent Trace source cannot be synchronized by the current authenticated + user", `409` -> "Agent Trace cursor conflict", `500` -> "control plane encountered an internal + error", `503` -> "control-plane Agent Trace storage is unavailable"); a new + `ControlPlaneError::Protocol { status: reqwest::StatusCode, message: String }` variant (with a + `Display` impl) for other terminal `4xx` responses, built the same sanitized way. Out — + changing which statuses map to `BadRequest`/`Forbidden`/`Conflict`/`ServerError` (unchanged); + `trace/sync.rs::is_stream_terminal` (T04). + - Dependencies: none + - Done when: a `500` response whose body contains `SQLITE_ERROR`, a table name, or a filesystem + path never appears in the resulting error's `Display` output; a `{"message": "..."}` body + surfaces exactly that string and no other field; an HTML `500` body produces the generic + server-error message with no HTML in it; a `404`/`405`/`415`/`422` response yields + `ControlPlaneError::Protocol` rather than `InvalidResponse`. + - Verification notes (commands or checks): `cargo test --manifest-path cli/Cargo.toml server_error_body_is_not_leaked known_safe_error_payload_is_surfaced html_error_body_falls_back_to_generic_message batch_classifies_404_as_protocol_error` + - Completed: 2026-08-11 + - Files changed: `cli/src/services/agent_trace_sync/control_plane.rs`, `cli/src/services/agent_trace_sync/test_http_server.rs` + - Evidence: Added `extract_safe_error_message` (top-level JSON object only, `message` then `error` string field, max 500 bytes, rejects malformed/HTML/non-object/oversized bodies) and `safe_error_message` helper; rewrote `classify_response`'s non-2xx branch to use them with status-specific generic fallbacks for `400`/`403`/`409`/`5xx`(including a dedicated `503` fallback), and to return the new `ControlPlaneError::Protocol { status, message }` variant (with `Display` impl) for any other client-error (`4xx`) status instead of folding it into `InvalidResponse`; `InvalidResponse` for non-4xx/non-5xx unexpected statuses no longer includes the raw body. Added a `CannedResponse::text` constructor to the in-repo test HTTP server (needed to send a non-JSON body) and four tests: `server_error_body_is_not_leaked` (SQL-error-shaped `500` body never appears in `Display` output), `known_safe_error_payload_is_surfaced` (`{"message": ...}` on `400` surfaces exactly that string), `html_error_body_falls_back_to_generic_message` (HTML `500` body produces the generic message with no HTML), and `batch_classifies_404_as_protocol_error` (`404` yields `ControlPlaneError::Protocol` carrying the extracted message). Ran `nix build .#checks.x86_64-linux.cli-tests`: 284 tests pass (up from 280 in T02). Ran `nix build .#checks.x86_64-linux.cli-clippy`: clean. Ran `nix build .#checks.x86_64-linux.cli-fmt`: clean. + - Notes: `cargo test`/`cargo clippy` direct invocation is blocked by this repo's bash-tool policy (`use-nix-flake-check-over-cargo-test`, not satisfiable by any wrapper); ran the equivalent `nix build .#checks.x86_64-linux.{cli-tests,cli-clippy,cli-fmt}` derivations instead, which cover the same test filters plus the full suite. + +- [x] T04: `Keep terminal 4xx statuses out of /batch ambiguous reconciliation` (status:done) + - Task ID: T04 + - Goal: `trace/sync.rs::is_stream_terminal` treats `ControlPlaneError::Protocol(_)` as terminal + alongside the existing `MissingCredentials`/`AuthenticationFailed`/`BadRequest`/`Forbidden`/ + `Storage` cases, so a `404`/`405`/`415`/`422` (and post-refresh `401`, already covered by + `AuthenticationFailed`) during `/batch` fails the stream immediately with no `/state` + refetch, while `409`, transport failures, `5xx`, and a malformed `2xx` batch body (still + `InvalidResponse`, not in `is_stream_terminal`) continue to reconcile via `/state` exactly as + before. + - Boundaries (in/out of scope): In — `is_stream_terminal` in `trace/sync.rs`; new orchestration + tests proving the terminal-vs-ambiguous split for `/batch`; the `context/cli/agent-trace- + sync-command.md` "Recovery semantics" update. Out — `sync_stream`'s reconciliation loop logic + itself (unchanged), the `/state` path (already terminal on invalid response per T01). + - Dependencies: T03 + - Done when: an orchestration test with a mocked `404` `/batch` response proves the sync fails + with no follow-up `/state` call and no batch resend; a second orchestration test with a + mocked `2xx` `/batch` response carrying an undecodable body proves sync still reconciles via + `/state` (regression coverage for the still-ambiguous case); `context/cli/agent-trace-sync- + command.md`'s "Recovery semantics" section names the terminal-`4xx`/sanitized-error behavior. + - Verification notes (commands or checks): `cargo test --manifest-path cli/Cargo.toml terminal_batch_status_fails_without_state_reconciliation malformed_2xx_batch_response_still_reconciles_via_state` + - Completed: 2026-08-11 + - Files changed: `cli/src/services/trace/sync.rs`, `context/cli/agent-trace-sync-command.md` + - Evidence: Added `ControlPlaneError::Protocol { .. }` to `is_stream_terminal`'s match arm in + `trace/sync.rs`, alongside the existing `MissingCredentials`/`AuthenticationFailed`/ + `BadRequest`/`Forbidden`/`Storage` cases, and updated its doc comment. Added + `terminal_batch_status_fails_without_state_reconciliation` (mocked `404` `/batch` response; + asserts `TraceSyncError::Stream { stream: "messages", .. }` and `server.call_count() == 2`, + proving no `/state` refetch and no batch resend followed the terminal status) and + `malformed_2xx_batch_response_still_reconciles_via_state` (mocked `2xx` `/batch` response with + an undecodable body; asserts the sync still succeeds via `/state` reconciliation and resend, + with `server.call_count() == 7` covering the initial `/state`, the malformed batch, the + refetch, the resent + successful messages batch, and the three remaining streams' batches). + Updated `context/cli/agent-trace-sync-command.md`'s "Recovery semantics" section: reworded the + ambiguous-batch-failure bullet to name an undecodable `2xx` body instead of "missing/invalid + response", clarified the "Invalid response" bullet to state it reconciles via `/state` rather + than failing outright, and added a new terminal-protocol-mismatch bullet + (`404`/`405`/`415`/`422`/other unrecognized `4xx` -> `ControlPlaneError::Protocol`, terminal + like `400`/`403`) and a sanitized-error-message bullet (T03's narrow `message`/`error` + extraction, no raw body ever surfaced). Ran `nix build .#checks.x86_64-linux.cli-tests`: 286 + tests pass (up from 284 in T03; an initial run showed 4 unrelated failures in + `agent_trace_db`/`agent_trace_export` from `UNIQUE constraint failed`/row-count-mismatch + errors — reran with no changes and got 286/286 passing, confirming pre-existing test-isolation + flakiness unrelated to this task, not a regression from this change). Ran `nix build + .#checks.x86_64-linux.cli-clippy`: clean. Ran `nix build .#checks.x86_64-linux.cli-fmt`: clean + after reformatting one `assert!(matches!(...))` in the new terminal-status test. + - Notes: `cargo test`/`cargo clippy` direct invocation is blocked by this repo's bash-tool policy + (`use-nix-flake-check-over-cargo-test`); ran the equivalent `nix build + .#checks.x86_64-linux.{cli-tests,cli-clippy,cli-fmt}` derivations instead, which cover the + same test filters plus the full suite. `nix run .#pkl-check-generated` and `nix flake check` + (the plan's "Full validation" commands) were not run for this single-task verification per the + workflow's narrowest-authoritative-check guidance; no `.pkl` schema or flake-wide surface was + touched by this task. + +## Open questions + +None. The three fixes, their required behavior, and their test coverage are fully specified by +the change request against code already read in `control_plane.rs`, `agent_trace_sync/mod.rs`, +and `trace/sync.rs`. + +## Validation Report + +**Status:** validated +**Date:** 2026-08-11 + +### Commands run + +- `cargo test --manifest-path cli/Cargo.toml` (via `nix build .#checks.x86_64-linux.cli-tests --rebuild -L`) -> exit 0 (286 passed; 0 failed; 0 ignored) +- `cargo clippy --manifest-path cli/Cargo.toml -- -D warnings` (via `nix build .#checks.x86_64-linux.cli-clippy -L`) -> exit 0 (clean) +- `nix run .#pkl-check-generated` -> exit 0 (101 files, ephemeral generation matched committed output) +- `nix flake check` -> exit 0 (all checks passed) +- `nix build .#checks.x86_64-linux.cli-fmt -L` -> exit 0 (clean; ran in addition to `nix flake check` to directly confirm formatting) + +### Scaffolding removed + +- None. + +### Success-criteria verification + +- [x] AC1: `/state` cursor validation -> `agent_trace_cursors_accept_boundary_values`, `agent_trace_cursors_reject_out_of_range_value_in_each_field`, and `invalid_state_cursor_fails_before_any_batch_request` all pass. +- [x] AC2: Single expiry decision for access-token resolution -> `valid_token_is_reused_without_resave`, `valid_token_is_not_resaved`, and `expired_token_is_refreshed_and_saved` all pass. +- [x] AC3: Unexpected-401 behavior unchanged -> `unexpected_401_refreshes_once_and_retries_once_on_success` and `unexpected_401_twice_fails_without_a_third_attempt` both pass. +- [x] AC4: No raw server body leaks into control-plane errors -> `server_error_body_is_not_leaked`, `known_safe_error_payload_is_surfaced`, and `html_error_body_falls_back_to_generic_message` all pass. +- [x] AC5: Terminal `4xx` batch statuses skip `/state` reconciliation; ambiguous cases still reconcile -> `terminal_batch_status_fails_without_state_reconciliation` and `malformed_2xx_batch_response_still_reconciles_via_state` both pass. + +### Failed checks and follow-ups + +- None. + +### Residual risks + +- None identified. diff --git a/context/plans/agent-trace-sync.md b/context/plans/agent-trace-sync.md new file mode 100644 index 00000000..9c10a669 --- /dev/null +++ b/context/plans/agent-trace-sync.md @@ -0,0 +1,175 @@ +# Plan: agent-trace-sync + +## Change summary + +Implement `sce trace sync`, the final composition step that synchronizes a repository's local Agent Trace capture database with the control-plane Agent Trace ingestion API. The intended flow is `sce auth login` → `cd ` → `sce trace sync`: resolve the current repository's Agent Trace storage (`repository_id`, `source_instance_id`, `RepositoryAgentTraceDb`) through the existing `agent_trace_storage` resolver, load/refresh stored WorkOS credentials through the existing `auth`/`token_storage` primitives, fetch authoritative server cursors from `POST /agent-trace/ingestion/state`, then for each of the four independent streams (`messages`, `parts`, `diff_traces`, `agent_traces`) read local rows after the cursor via the existing `AgentTraceExportReader` (PR #198) and upload them through `POST /agent-trace/ingestion/batch`, advancing strictly from the validated server response. + +This plan composes already-shipped infrastructure — repository/source identity (`agent-trace-source-instance-id`), the read-only export readers (`agent-trace-export-readers`), and the existing WorkOS auth/token-storage stack — into one new command. It does not redesign the local database, source identity, export readers, or control-plane storage model, and it adds no local sync cursor or local DWH: every invocation starts from `/state`, which is why restarts, conflicts, and ambiguous network failures are all recoverable without any client-side persisted progress. + +## Acceptance criteria + +- [x] AC1: `sce trace sync` and `sce trace sync --format json` parse and route to a dedicated sync command handler under the existing `trace` command group. + - Validate: `cargo test -p shared-context-engineering trace::` (CLI parsing/conversion unit tests) as part of `nix flake check`. +- [x] AC2: With no stored WorkOS credentials, `sce trace sync` fails before making any control-plane request, with guidance to run `sce auth login`. + - Validate: targeted unit test asserting zero HTTP calls reach the test server and the error message contains `sce auth login`. +- [x] AC3: With a valid non-expired stored token, sync reuses it as-is (sends `Authorization: Bearer `) and does not rewrite/save the unchanged credential. With an expired stored token, sync refreshes via the existing WorkOS refresh flow, saves the new token, and uses it for control-plane requests. + - Validate: targeted unit tests against a local test HTTP server asserting exact `Authorization` header values and asserting `save_tokens` is/is not called. +- [x] AC4: An unexpected `401` from `/state` or `/batch` triggers exactly one WorkOS refresh, one saved token, and one retried request; a second `401` fails the command with `sce auth login` guidance and no further retries. + - Validate: targeted unit test driving a canned 401-then-200 (success case) and 401-then-401 (bounded-failure case) sequence against the test server. +- [x] AC5: `POST /agent-trace/ingestion/state` is called exactly once per `sce trace sync` invocation with `{repositoryId, sourceInstanceId}` taken from resolved storage metadata and no workspace/user/checkout/database fields. + - Validate: targeted unit test asserting the exact captured request body and call count. +- [x] AC6: For each of the four streams, sync uploads exactly the local rows after the stream's authoritative cursor, in batches bounded by `AGENT_TRACE_EXPORT_BATCH_SIZE`, advances the cursor only from the validated server response (`accepted == rows.len()` and `cursor == rows.last().sourceRowId`), and never infers the next cursor from `cursor + rows.len()`. + - Validate: targeted unit tests covering an empty database (no batch calls), one batch, >500 rows requiring multiple batches, and gapped source IDs. +- [x] AC7: A `409` response reconciles by refetching `/state`, replacing only the affected stream's cursor, and resuming from local rows after the refreshed cursor without resending already-accepted rows; a bounded reconciliation loop prevents unbounded spinning. + - Validate: targeted unit test reproducing the plan's 409 example (state=10, local rows 11-13, batch 409s, refreshed state=12, only row 13 resent with `expectedCursor=12`). +- [x] AC8: An ambiguous batch failure (5xx, transport failure, or missing/invalid response) reconciles via `/state` before any resend: if the refreshed cursor advanced, sync continues from it without resending; if unchanged, sync may resend once reread from the authoritative cursor, bounded by the same reconciliation limit. + - Validate: targeted unit tests for both the "committed" (refreshed cursor advanced) and "uncommitted" (refreshed cursor unchanged) cases. +- [x] AC9: A syntactically successful but semantically inconsistent batch response (`accepted`/`cursor` not matching the sent rows) fails the command with an invalid-response error rather than advancing state. + - Validate: targeted unit test asserting failure on a mismatched `{cursor, accepted}` response. +- [x] AC10: A `403` ownership rejection fails the command with a clear message, without generating a new `source_instance_id`, mutating local repository metadata, or attempting ownership transfer. + - Validate: targeted unit test asserting the failure message and that `repository_metadata.source_instance_id` on disk is unchanged after the run. +- [x] AC11: A full successful sync across all four streams renders the documented text layout by default and the documented JSON shape under `--format json`, and running sync twice in a row is naturally incremental (the second run's `/state` reflects the first run's uploads and re-reads only unsynced rows), with no local cursor file, database, or table created anywhere on disk. + - Validate: an end-to-end integration test using a temporary `RepositoryAgentTraceDb` seeded with rows in all four streams and a local test HTTP server, run twice in sequence; text/JSON rendering unit tests for exact output shape. + +### Full validation + +- `nix flake check` +- `nix run .#pkl-check-generated` + +### Context sync + +- `context/cli/trace-command.md` +- `context/sce/agent-trace-export-readers.md` +- `context/cli/cli-command-surface.md` +- `context/overview.md` (the `sync command deferral` note and its `0.4.0` framing) +- `context/glossary.md` (`sync command deferral` entry) +- `context/context-map.md` +- A new `context/cli/agent-trace-sync-command.md` (or equivalent) documenting the composed local→control-plane flow and recovery semantics + +## Constraints and non-goals + +- **In scope:** a new `cli/src/services/agent_trace_sync/` module (control-plane DTOs, `AuthenticatedControlPlaneClient`, per-stream sync engine, `AgentTraceSyncReport`, text/JSON rendering); `TraceSubcommand::Sync`/`TraceSubcommandRequest::Sync` CLI wiring in `cli/src/cli_schema.rs`, `cli/src/services/parse/command_runtime.rs`, and `cli/src/services/trace/`; one new config setting for the control-plane base URL (CLI resolver plus its Pkl schema entry); durable context updates listed above. +- **Out of scope:** the local database schema, source identity generation/claim logic, `AgentTraceExportReader` (PR #198), the WorkOS OAuth/device-authorization endpoints themselves, workspace selection, source-ownership transfer, `repositoryRemoteUrl` verification, any control-plane server implementation, and any change to `sce doctor`'s read-only diagnose surface. +- **Constraints:** no local sync cursor, local cursor file, `agent-trace-sync.db`, or equivalent persisted progress; no process-wide or cross-HTTP lock; every state/batch request within one command execution uses the same `repositoryId`/`sourceInstanceId` taken once from resolved storage metadata; batch requests carry exactly one stream per request using the literal stream values `messages`/`parts`/`diff_traces`/`agent_traces` (never the response object's camelCase `diffTraces`/`agentTraces`); stored/refreshed tokens and Authorization headers are never logged or included in JSON output. +- **Non-goal:** background/periodic/daemon sync, sync-on-hook, a multi-stream batch transaction, deriving `code_changes`, patch parsing/analytics, or reviving `agent-trace-sync.db` / local DWH / `BridgeLock` / Turso Sync / direct Turso credentials in SCE. + +## Assumptions + +- **Control-plane base URL config key** follows the existing `workos_client_id` pattern exactly (`cli/src/services/config/resolver.rs` `AuthConfigKeySpec`, flags > env > config file > baked default): a new config-file key plus env override plus a baked default value, added to `config/pkl/base/sce-config-schema.pkl` alongside the existing strict schema. This is the smallest addition matching the request's explicit instruction to reuse existing configuration precedence/baked-default conventions. +- **Test HTTP server**: rather than adding a new dev-dependency (`wiremock`/`httptest`) for a CLI crate that currently declares none, tests use a small in-repo, test-only HTTP/1.1 server (`std::net::TcpListener` + minimal request parsing) that returns canned JSON responses per call, keeps assertions on captured request bodies/headers, and lets tests simulate 401/409/5xx/dropped-connection sequences deterministically. This keeps the dependency footprint unchanged, matching the "keep dependency additions explicit and minimal" convention, and is the smallest solution to the "local/mock HTTP server" requirement. +- **Reconciliation bound**: the bounded 409/ambiguous-failure reconciliation loop uses a small fixed attempt cap (5), matching the order of magnitude of existing retry constants in `auth.rs`/`resilience.rs` (e.g. `TOKEN_REFRESH_MAX_ATTEMPTS = 3`); exhausting it fails the stream with a clear "stream did not converge" error rather than looping. +- **State-request transient retries**: `/state` reuses the existing sync `run_with_retry_sync`/`RetryPolicy` convention from `cli/src/services/resilience.rs` for `500`/`503` responses and transport failures, with `400`/`401`-after-retry/`403` excluded as terminal. `/batch` failures never go through this generic retry wrapper; they always reconcile via `/state` first, per the plan's explicit prohibition on blind `/batch` retry middleware. +- **Module boundary naming**: the new module lives at `cli/src/services/agent_trace_sync/` (`mod.rs` for the report/orchestration engine, `control_plane.rs` for DTOs/HTTP client), matching the repository's existing `agent_trace_{export,storage,db}` naming family rather than a generic `services/control_plane/`. + +## Task stack + +- [x] T01: `Add control-plane base URL config setting` (status:done) + - Task ID: T01 + - Goal: Introduce the smallest new config setting for the control-plane API base URL, resolved with the same `flags > env > config file > baked default` precedence and `AuthConfigKeySpec` machinery already used for `workos_client_id`, injectable in tests without touching production config. + - Boundaries (in/out of scope): In — `cli/src/services/config/resolver.rs` (new `AuthConfigKeySpec` constant, env key, baked default, resolution wiring, `ResolvedAuthRuntimeConfig` field), `cli/src/services/config/` render/show plumbing so `sce config show` reports it with provenance, `config/pkl/base/sce-config-schema.pkl` (new strict-schema key). Out — any control-plane HTTP client code (T03), any CLI surface for `trace sync` (T05). + - Dependencies: none + - Done when: the new key resolves through env/config-file/baked-default precedence with unit test coverage mirroring the existing `workos_client_id` resolver tests; `sce config show` reports it; `nix run .#pkl-check-generated` passes with the updated schema. + - Verification notes (commands or checks): `cargo test -p shared-context-engineering config::` and `nix run .#pkl-check-generated`. + - Evidence: Added `CONTROL_PLANE_BASE_URL_KEY` (`AuthConfigKeySpec`, env `SCE_CONTROL_PLANE_BASE_URL`, baked default `https://sce.crocoder.dev`) in `cli/src/services/config/resolver.rs`, threaded through `RuntimeConfig`/`FileConfig`/`ParsedFileConfigDocument`/`ResolvedAuthRuntimeConfig`, the `control_plane_base_url` top-level config key/schema doc string in `schema.rs`, `sce config show` text/JSON rendering in `render.rs`, and the new `control_plane_base_url` string property in `config/pkl/base/sce-config-schema.pkl`. Added resolver unit tests for baked-default, config-file-over-default, and env-over-config-file-and-default precedence. + - Verification: `nix flake check` — all checks passed (includes `cargo test`/clippy/fmt/pkl-generated). `nix run .#pkl-check-generated` — passed (101 files, inventory hash unchanged in kind, regenerated with new schema key). + +- [x] T02: `Add control-plane ingestion DTOs and stream identity` (status:done) + - Task ID: T02 + - Goal: Define typed serde request/response DTOs for the `/agent-trace/ingestion/state` and `/agent-trace/ingestion/batch` contracts, plus the `IngestionStream` enum with its exact wire values (`messages`, `parts`, `diff_traces`, `agent_traces`), reusing the PR #198 export row types directly as batch row payloads with no additional mapping layer. + - Boundaries (in/out of scope): In — new `cli/src/services/agent_trace_sync/control_plane.rs` (or `dto.rs` within the new module) with `AgentTraceIngestionStateRequest`, `AgentTraceIngestionStateResponse`/`AgentTraceCursors`, `AgentTraceIngestionBatchRequest`, `AgentTraceIngestionBatchResponse`, `IngestionStream`; serde round-trip/shape unit tests. Out — any HTTP client behavior (T03), any sync algorithm (T04). + - Dependencies: none + - Done when: DTOs serialize to the exact camelCase field names and stream string values specified in the contract (verified by unit tests asserting rendered JSON), and `AgentTraceIngestionBatchRequest` composes directly with the four PR #198 export row types without a new per-stream struct. + - Verification notes (commands or checks): `cargo test -p shared-context-engineering agent_trace_sync::control_plane::`. + - Evidence: Added new `cli/src/services/agent_trace_sync/` module (`mod.rs`, `control_plane.rs`), wired via `pub mod agent_trace_sync;` in `cli/src/services/mod.rs`. Defined `IngestionStream` (snake_case wire values `messages`/`parts`/`diff_traces`/`agent_traces`), `AgentTraceIngestionStateRequest` (camelCase `repositoryId`/`sourceInstanceId`), `AgentTraceCursors`/`AgentTraceIngestionStateResponse` (camelCase cursor fields including `diffTraces`/`agentTraces`), generic `AgentTraceIngestionBatchRequest` (camelCase `repositoryId`/`sourceInstanceId`/`stream`/`expectedCursor`/`rows`), and `AgentTraceIngestionBatchResponse` (`accepted`/`cursor`). Added unit tests asserting exact rendered JSON field/stream-value shape and that the generic batch request composes directly with each of the four PR #198 export row types (`AgentTraceMessageExportRow`, `AgentTracePartExportRow`, `AgentTraceDiffTraceExportRow`, `AgentTraceAgentTraceExportRow`). + - Verification: `nix flake check` — all checks passed (cargo test/clippy/fmt). `nix run .#pkl-check-generated` — passed (101 files, inventory hash unchanged; no schema change in this task). + +- [x] T03: `Implement AuthenticatedControlPlaneClient` (status:done) + - Task ID: T03 + - Goal: Implement the authenticated control-plane HTTP client: load stored credentials (fail with `sce auth login` guidance when absent), reuse a still-valid token as-is (no resave), refresh-and-save an expired token, inject the Bearer header, retry exactly once on an unexpected `401` (refresh, save, retry), classify HTTP responses into a typed internal error distinguishing auth failure / transport failure / `400` / `403` / `409` / `5xx` / invalid response, and expose `ingestion_state(...)` plus typed `ingest_messages(...)`/`ingest_parts(...)`/`ingest_diff_traces(...)`/`ingest_agent_traces(...)` operations (thin wrappers over one generic batch-post method) with an injectable base URL. + - Boundaries (in/out of scope): In — `cli/src/services/agent_trace_sync/control_plane.rs` client struct/methods, its error enum, the bespoke in-repo test HTTP server helper, and unit tests for: missing-auth failure with zero HTTP calls, valid-token reuse without resave, expired-token refresh+save, unexpected-401 refresh-once-retry-once (success and bounded-failure cases), exact `/state` request shape with no workspace/user fields, and `400`/`403`/`409`/`5xx` classification. Out — the per-stream sync/reconciliation algorithm (T04); reusing existing `auth::ensure_valid_token_returning_token`/`renew_stored_token_from_refresh_token`/`is_stored_token_expired` and `token_storage::{load_tokens,save_tokens}` rather than reimplementing OAuth. + - Dependencies: T01, T02 + - Done when: all listed unit tests pass against the in-repo test HTTP server; the client never logs a token or Authorization header value; `/state` uses the existing sync resilience retry policy for transient `500`/`503`/transport failures and never retries `400`/`403`/post-refresh `401` as transient. + - Verification notes (commands or checks): `cargo test -p shared-context-engineering agent_trace_sync::control_plane::`. + - Evidence: Added `AuthenticatedControlPlaneClient` and `ControlPlaneError` to `cli/src/services/agent_trace_sync/control_plane.rs`. The client loads/refreshes credentials through a `CredentialStore` seam (`SystemCredentialStore` delegates to the real `token_storage::{load_tokens,save_tokens}` in production; tests inject a `FakeCredentialStore`) so tests can assert exactly when `save` is called without touching the real encrypted auth database. `resolve_access_token` reuses a still-valid stored token as-is and only saves after `auth::ensure_valid_token_returning_token` actually refreshed an expired one; `execute_authenticated` retries exactly once on an unexpected `401` via `force_refresh_access_token`, failing terminally on a second `401`. `ingestion_state(...)` wraps the whole authenticated call in the existing `resilience::run_with_retry`/`RetryPolicy` convention, short-circuiting terminal errors (`400`/`403`/post-refresh-`401`) as an `Ok(StateAttempt::Terminal(..))` so only transport/`5xx` failures are actually retried. `ingest_messages`/`ingest_parts`/`ingest_diff_traces`/`ingest_agent_traces` are thin wrappers over a private generic `post_batch` with no retry wrapper (batch reconciliation is T04's responsibility). `classify_response` maps `400`/`403`/`409`/`5xx`/other-non-2xx into `ControlPlaneError` variants. Added a bespoke in-repo test-only HTTP/1.1 server (`cli/src/services/agent_trace_sync/test_http_server.rs`, `TestHttpServer`/`CannedResponse`, `std::net::TcpListener`-based, no new dev-dependency) and unit tests in `control_plane.rs` covering: zero-HTTP-call missing-credentials failure, valid-token reuse without resave, expired-token refresh+save, unexpected-401 refresh-once-retry-once (success and bounded two-`401` failure), exact `/state` request body/path shape, and `400`/`403`/`409`/`5xx` batch classification. Test `reqwest::Client`s are built with `danger_accept_invalid_certs(true)` since the plain-`http://` loopback test server needs no TLS root store, avoiding a platform-certificate-verifier panic in CA-cert-less sandboxes. + - Verification: `nix flake check` — all checks passed (cargo test/clippy/fmt). `nix run .#pkl-check-generated` — passed (101 files, inventory hash unchanged; no schema change in this task). + +- [x] T04: `Implement per-stream sync engine with 409/ambiguous-failure reconciliation` (status:done) + - Task ID: T04 + - Goal: Implement the generic per-stream synchronization engine: given a cursor, a local-row reader closure, a batch-ingest closure, and a `/state`-refresh closure, loop reading local rows after the cursor and posting bounded batches, validating each response (`accepted == rows.len()`, `cursor == rows.last().sourceRowId`) before advancing, reconciling via `/state` on `409` (replace only the affected stream's cursor, re-read local rows, continue) and on ambiguous/`5xx` batch failures (continue if the refreshed cursor advanced, otherwise bounded resend from the refreshed cursor), and stopping cleanly when no unsynced local rows remain. + - Boundaries (in/out of scope): In — `cli/src/services/agent_trace_sync/mod.rs` engine function/types (`StreamSyncOutcome`/equivalent), a small `source_row_id` accessor over the four PR #198 export row types, the bounded reconciliation attempt cap, and unit tests (using fake/in-memory reader and ingest closures, no HTTP) for: empty database (no batch calls), one batch, >500 rows across multiple batches without loading all unsynced rows into memory at once, gapped source IDs (cursor from last actual ID, not row count), 409 recovery (resend only the unsent tail), lost-response reconciliation for both the committed and uncommitted cases, and invalid-response rejection (mismatched `accepted`/`cursor`). Out — the HTTP client itself (T03, already done); CLI wiring (T05). + - Dependencies: T03 + - Done when: all listed unit tests pass; the engine never infers a cursor from `cursor + rows.len()`; the reconciliation loop is bounded and fails with a clear "did not converge" error past the cap rather than looping unboundedly. + - Verification notes (commands or checks): `cargo test -p shared-context-engineering agent_trace_sync::`. + - Evidence: Added `AgentTraceExportRow` trait (`source_row_id()`) implemented for all four PR #198 export row types, `BatchAttemptOutcome` (`Accepted{accepted,cursor}`/`Conflict`/`Ambiguous`), `StreamSyncError`, `StreamSyncOutcome`, and the generic `sync_stream(initial_cursor, batch_limit, read_after, ingest_batch, refresh_cursor)` engine in `cli/src/services/agent_trace_sync/mod.rs`. `sync_stream` loops reading rows after `cursor` via `read_after`, calls `ingest_batch`, and on `Accepted` validates `accepted == rows.len()` and `cursor == rows.last().source_row_id()` before advancing (`InvalidResponse` otherwise) — the cursor is always taken from the validated server response, never `cursor + rows.len()`. On `Conflict` or `Ambiguous` it increments a shared reconciliation counter (bounded by `RECONCILIATION_MAX_ATTEMPTS = 5`, resetting on progress; exceeding it returns `DidNotConverge`), calls `refresh_cursor`, and resumes the loop from the refreshed cursor — this single path naturally implements both the 409 "resend only the unsent tail" case and the ambiguous committed/uncommitted cases, since an unchanged refreshed cursor causes the same rows to be re-read and resent. Added unit tests over in-memory fake reader/ingest/refresh closures (no HTTP) covering: empty database (no batch calls), one batch, >1000 rows spanning multiple 500-row-capped batches, gapped source IDs, the plan's exact 409 worked example (state=10, rows 11-13, 409, refreshed state=12, only row 13 resent), ambiguous-committed (refreshed cursor advanced, no resend) and ambiguous-uncommitted (refreshed cursor unchanged, resend once) cases, the reconciliation bound (`DidNotConverge`), and invalid-response rejection on mismatched `accepted`/`cursor`. + - Verification: `nix flake check` — all checks passed (270 cargo tests passed including 11 new `agent_trace_sync::tests::*`, clippy, fmt). `nix run .#pkl-check-generated` — passed (101 files, inventory hash unchanged; no schema change in this task). + +- [x] T05: `Wire "sce trace sync" CLI surface and orchestration` (status:done) + - Task ID: T05 + - Goal: Add `sce trace sync [--format text|json]` to the `trace` command group (`TraceSubcommand::Sync`, `TraceSubcommandRequest::Sync`, clap-to-runtime conversion), resolve the current repository's Agent Trace storage using the same `ContextWithRepoRoot`/`AgentTraceStorageContext`/`resolve_agent_trace_storage` path already used by `sce trace status` (not the hook-runtime resolver), build the control-plane client from the resolved base URL, call `/state` once, then run the T04 engine for all four streams in the fixed `messages → parts → diff_traces → agent_traces` order using the T03 client and the existing `AgentTraceExportReader`, assembling an `AgentTraceSyncReport`, and mapping every failure mode (auth, transport, `400`, `403`, `409`-exhausted, `5xx`-exhausted, invalid response, local export/storage failure) to `ClassifiedError` per existing SCE conventions. + - Boundaries (in/out of scope): In — `cli/src/cli_schema.rs` (new `TraceSubcommand::Sync` variant), `cli/src/services/trace/mod.rs` (`TraceSubcommandRequest::Sync`), `cli/src/services/parse/command_runtime.rs` (conversion), `cli/src/services/trace/command.rs` (dispatch) or a new `cli/src/services/trace/sync.rs` orchestration module, `AgentTraceSyncReport`/`StreamSyncReport` types, CLI-parsing unit tests for `sce trace sync` and `sce trace sync --format json`. Out — text/JSON rendering (T06); the sync engine and client internals (T03/T04, consumed as-is). + - Dependencies: T04 + - Done when: `sce trace sync` and `sce trace sync --format json` parse and dispatch correctly; an end-to-end integration test (temporary seeded `RepositoryAgentTraceDb` + in-repo test HTTP server) exercises all four independent streams end to end and asserts a second invocation is naturally incremental (its `/state` reflects the first run's uploads and only unsynced rows are re-read), with no new file/database/table created on disk and `repository_metadata.source_instance_id` unchanged; a `403` failure surfaces clear ownership guidance without mutating local metadata or generating a new `source_instance_id`. + - Verification notes (commands or checks): `cargo test -p shared-context-engineering trace::` and the new end-to-end integration test target. + - Evidence: Added `TraceSubcommand::Sync { format }` in `cli/src/cli_schema.rs`, `TraceSubcommandRequest::Sync { format }` in `cli/src/services/trace/mod.rs`, and clap-to-runtime conversion in `cli/src/services/parse/command_runtime.rs`. Added `cli/src/services/trace/sync.rs`: `run_current_sync(repo_root)` resolves storage via the same `resolve_agent_trace_storage`/`AgentTraceStorageContext` path as `sce trace status`, resolves the control-plane base URL/WorkOS client ID via `config::resolve_auth_runtime_config`, builds an `AuthenticatedControlPlaneClient`, and delegates to the testable `run_sync_against(repository_id, source_instance_id, db, client)`, which calls `/state` once then runs the T04 `sync_stream` engine for `messages → parts → diff_traces → agent_traces` in order via `AgentTraceExportReader`, assembling `AgentTraceSyncReport`/`StreamSyncReports`/`StreamSyncReport`. A per-stream `terminal: RefCell>` distinguishes genuinely ambiguous batch failures (`5xx`/transport/invalid-response — real `/state` reconciliation) from terminal ones (missing/invalid credentials, `400`, `403`, `409`→`Conflict` handled separately) — terminal failures short-circuit the reconciliation closure with the original error instead of issuing another network call, so `403` never mutates local state or retries. All `TraceSyncError` variants map to `ClassifiedError::runtime` in `cli/src/services/trace/command.rs`, which also emits a provisional `{report:#?}` text body pending T06's `render_sync`. Added CLI-parsing unit tests (`trace_sync_parses_to_trace_sync_request_with_default_text_format`, `trace_sync_json_format_parses_to_trace_sync_request`) in `command_runtime.rs`, and two tests in `sync.rs`: an end-to-end test seeding one row per stream in a temp `RepositoryAgentTraceDb`, driving a full sync against the in-repo `TestHttpServer` (asserting all four streams upload/advance correctly and exactly 5 HTTP calls), then a second `run_sync_against` call against the same DB with an advanced `/state` response asserting zero uploads/batches and exactly one additional HTTP call (naturally incremental) and that no sync-created sidecar file/DB appears on disk; and a `403` test asserting the sync fails with `ControlPlaneError::Forbidden`, only 1 HTTP call is made (no reconciliation), and `repository_metadata.source_instance_id` is unchanged. + - Verification: `nix build .#checks.x86_64-linux.cli-tests` — 274 tests passed (includes the new CLI-parsing and `trace::sync` tests). `nix build .#checks.x86_64-linux.cli-clippy` and `.#checks.x86_64-linux.cli-fmt` — passed. `nix run .#pkl-check-generated` — passed (101 files, inventory hash unchanged; no schema change in this task). + +- [x] T06: `Render "sce trace sync" text and JSON output` (status:done) + - Task ID: T06 + - Goal: Implement `render_sync` producing the documented concise text layout (`Agent Trace sync complete.` header, repository/source-instance lines, one row per stream showing rows uploaded and final cursor) and the documented JSON shape (`status`, `repositoryId`, `sourceInstanceId`, `streams.{messages,parts,diffTraces,agentTraces}` each with `uploaded`/`initialCursor`/`finalCursor`/`batches`), wired into the T05 command dispatch and following existing `services::output_format`/`--format` conventions. + - Boundaries (in/out of scope): In — `cli/src/services/trace/render_sync.rs` (or equivalent), its unit tests asserting exact text formatting and exact JSON field names/shape (including the `diffTraces`/`agentTraces` camelCase keys in output despite `diff_traces`/`agent_traces` internal naming), wiring into `TraceCommand::execute`. Out — the report data itself (T05, already produced). + - Dependencies: T05 + - Done when: text output matches the documented concise per-stream layout without printing every batch or row; JSON output matches the documented shape byte-for-byte on field names; both are covered by unit tests. + - Verification notes (commands or checks): `cargo test -p shared-context-engineering trace::render_sync::`. + - Evidence: Added `cli/src/services/trace/render_sync.rs` with `render(report, format)` matching `services::output_format::OutputFormat`. Text rendering (`render_text`) follows the `render_status_all.rs` padded-table convention: `style::heading("Agent Trace sync complete.")`, `Repository ID:`/`Source instance ID:` lines, then a `Stream | Uploaded | Final cursor` table with one row per stream in the fixed `messages → parts → diff_traces → agent_traces` order — no per-batch or per-row detail is printed. JSON rendering (`render_json`) emits `status`/`command`/`subcommand`/`repositoryId`/`sourceInstanceId`/`streams.{messages,parts,diffTraces,agentTraces}`, each stream object carrying `uploaded`/`initialCursor`/`finalCursor`/`batches`; the JSON keys use the documented camelCase (`diffTraces`/`agentTraces`) despite the `StreamSyncReports` struct's `diff_traces`/`agent_traces` field names. Wired into `TraceCommand::execute`'s `Sync` arm (`cli/src/services/trace/command.rs`), replacing the provisional `{report:#?}` debug dump; `mod render_sync;` added to `cli/src/services/trace/mod.rs`. Added unit tests asserting the text layout contains the heading/ID lines/all four stream rows with correct uploaded/final-cursor values and omits per-batch detail, and a JSON-shape test asserting exact field names including the camelCase stream keys and that no snake_case duplicate keys leak through. + - Verification: `nix flake check` — all checks passed (cargo test/clippy/fmt, includes new `trace::render_sync::` unit tests). `nix run .#pkl-check-generated` — passed (101 files, inventory hash unchanged; no schema change in this task). + +- [x] T07: `Document the completed Agent Trace sync architecture` (status:done) + - Task ID: T07 + - Goal: Document the now-complete local-to-control-plane Agent Trace sync architecture and retire the stale "`sce sync` deferred to `0.4.0`" framing left over from before this plan. + - Boundaries (in/out of scope): In — new `context/cli/agent-trace-sync-command.md` (or equivalent) covering the composed flow (`hooks/plugins → repository Agent Trace DB → AgentTraceExportReader → sce trace sync → HTTPS + WorkOS Bearer → control plane`), the exact user flow (`sce auth login` / `cd repo` / `sce trace sync`), the no-local-cursor/no-`agent-trace-sync.db`/no-Turso-Sync/no-`BridgeLock`/no-local-DWH invariants, and the `409`/ambiguous-batch/`401` recovery semantics; updates to `context/cli/trace-command.md`, `context/cli/cli-command-surface.md`, `context/overview.md`, `context/glossary.md` (`sync command deferral` entry), and `context/context-map.md` to reflect that `sce trace sync` is implemented rather than deferred. + - Dependencies: T06 + - Done when: every listed context file accurately describes current-state `sce trace sync` behavior; no remaining context file states `sce sync` is deferred to `0.4.0` or describes a retired sync architecture (`agent-trace-sync.db`, local DWH, `BridgeLock`, Turso Sync, direct Turso credentials in SCE) as active or planned. + - Verification notes (commands or checks): manual review of the listed files; `nix run .#pkl-check-generated` and `nix flake check` (no code change expected in this task, but re-run as the standard post-task baseline). + - Evidence: `context/cli/trace-command.md`, `context/overview.md`, and the glossary's `sync command deferral` entry were already accurate from T05/T06 (no edit needed there). Found and fixed 3 stale statements in `context/cli/cli-command-surface.md` (lines describing "`sync` command is not wired"/"is deferred to `0.4.0`"/"`sce sync` command wiring and broader cloud behavior remain intentionally deferred") to describe implemented `sce trace sync`, and fixed `context/context-map.md`'s `cli-command-surface.md` catalog blurb, which still repeated the stale "`sce sync` command wiring is deferred to `0.4.0`" framing. Added new `context/cli/agent-trace-sync-command.md` documenting the composed flow diagram (Mermaid), the exact `sce auth login` / `cd ` / `sce trace sync` user flow, the no-local-cursor/no-`agent-trace-sync.db`/no-Turso-Sync/no-`BridgeLock`/no-local-DWH invariants, and `401`/`409`/ambiguous-batch-failure/`403` recovery semantics, linked from `context/context-map.md` and from `context/cli/trace-command.md`'s Related context section. Verified no remaining context file (outside historical/immutable `context/decisions/` records and the already-historical glossary entry) describes the retired sync architecture as active or planned. + - Verification: `nix run .#pkl-check-generated` — passed (101 files, inventory hash unchanged; no schema change in this task). `nix flake check` — all checks passed. + +## Open questions + +None. The request is fully specified end to end, including exact wire contracts, recovery algorithms, and test matrices; the two implementation-detail choices this plan had to make (the control-plane base URL config mechanism and the test HTTP server approach) are reversible local decisions recorded under Assumptions rather than open questions. + +## Validation Report + +**Status:** validated +**Date:** 2026-08-11 + +### Commands run + +- `nix flake check` -> exit 0 (all checks passed: cli-tests, cli-clippy, cli-fmt, cli-generated-input, pkl-generated, npm/config-lib bun tests and biome checks, workflow-actionlint, native-portability-audit, flatpak-static-validation, cargo-sources-parity, flatpak-manifest-parity) +- `nix run .#pkl-check-generated` -> exit 0 (Ephemeral Pkl generation passed: 101 files, inventory hash unchanged) +- `nix build .#checks.x86_64-linux.cli-tests --rebuild -L` -> exit 0 for the test run itself (277 passed; 0 failed), but the derivation was flagged non-deterministic on rebuild-comparison (`may not be deterministic: output ... differs`) — an artifact-packaging/timestamp reproducibility property of the Nix build, not a test failure; `nix flake check` (the plan's authored `Full validation` command) already builds and runs this same derivation once and passed cleanly. + +### Scaffolding removed + +- None. No debug-only patches, temporary files, or throwaway artifacts were found; the in-repo `TestHttpServer` test helper is durable test infrastructure documented as an Assumption, not scaffolding. + +### Success-criteria verification + +- [x] AC1: CLI parsing/routing -> `command_runtime.rs` tests `trace_sync_parses_to_trace_sync_request_with_default_text_format`, `trace_sync_json_format_parses_to_trace_sync_request` pass. +- [x] AC2: No-credentials failure -> `control_plane.rs::missing_credentials_fail_before_any_http_call` passes. +- [x] AC3: Token reuse/refresh -> `control_plane.rs::valid_token_is_reused_without_resave`, `expired_token_is_refreshed_and_saved` pass. +- [x] AC4: Bounded 401 retry -> `control_plane.rs::unexpected_401_refreshes_once_and_retries_once_on_success`, `unexpected_401_twice_fails_without_a_third_attempt` pass. +- [x] AC5: Exact `/state` request shape/count -> `control_plane.rs::state_request_body_has_exact_shape_and_call_count`, `state_request_serializes_to_camel_case_fields` pass. +- [x] AC6: Batched, cursor-validated uploads -> `mod.rs::empty_database_makes_no_batch_calls`, `one_batch_uploads_all_rows_and_advances_cursor`, `more_than_five_hundred_rows_span_multiple_bounded_batches`, `gapped_source_ids_advance_cursor_from_last_id_not_row_count` pass. +- [x] AC7: 409 reconciliation -> `mod.rs::conflict_resends_only_the_unsent_tail` reproduces the plan's exact worked example and passes. +- [x] AC8: Ambiguous-failure reconciliation -> `mod.rs::ambiguous_failure_with_advanced_refresh_does_not_resend`, `ambiguous_failure_with_unchanged_refresh_resends_once` pass; bound enforced by `reconciliation_bound_fails_with_did_not_converge`. +- [x] AC9: Invalid-response rejection -> `mod.rs::invalid_response_rejects_mismatched_accepted_and_cursor` passes; also `control_plane.rs::batch_classifies_403_as_forbidden`/`batch_classifies_409_as_conflict` cover response classification. +- [x] AC10: 403 ownership rejection -> `trace/sync.rs::forbidden_state_response_fails_without_mutating_local_metadata` passes (asserts message, single HTTP call, unchanged `source_instance_id` on disk). +- [x] AC11: End-to-end + rendering -> `trace/sync.rs::full_sync_uploads_all_four_streams_and_second_run_is_naturally_incremental` passes (two sequential runs, second is incremental, no sidecar file/DB created); `render_sync.rs::text_renders_concise_per_stream_layout_without_batches`, `text_row_values_match_uploaded_and_final_cursor`, `json_shape_matches_contract` pass. + +### Failed checks and follow-ups + +None. + +### Residual risks + +- The `cli-tests` Nix derivation was observed non-deterministic under `--rebuild` (output hash differs between two builds of the same derivation with identical Cargo.lock/source), unrelated to this plan's code; worth a separate look if CI ever compares build hashes across runs, but it does not affect `nix flake check`, which is this plan's authored `Full validation` gate and passed cleanly. diff --git a/context/sce/agent-trace-export-readers.md b/context/sce/agent-trace-export-readers.md index 63f59a68..0e521fdd 100644 --- a/context/sce/agent-trace-export-readers.md +++ b/context/sce/agent-trace-export-readers.md @@ -1,18 +1,18 @@ # Agent Trace export readers (read-only) -`cli/src/services/agent_trace_export/mod.rs` defines `AgentTraceExportReader<'a>`, the local read/export boundary between one repository-scoped Agent Trace source database and any future outbound sync. It is purely additive over the existing schema: it adds no table, no migration, and no writer. +`cli/src/services/agent_trace_export/mod.rs` defines `AgentTraceExportReader<'a>`, the local read/export boundary between one repository-scoped Agent Trace source database and outbound sync. It is purely additive over the existing schema: it adds no table, no migration, and no writer. ## Layering ```mermaid flowchart LR A["SCE local source DB\n(RepositoryAgentTraceDb)"] --> B["Incremental export reader\n(AgentTraceExportReader)"] - B --> C["Future control-plane client\n(not built by this plan)"] + B --> C["Control-plane client\n(sce trace sync)"] ``` -- **SCE local source DB** — the existing repository-scoped `RepositoryAgentTraceDb` (see [agent-trace-db.md](agent-trace-db.md)), written by the hook/lifecycle paths already documented there. This plan does not change its writer, schema, or migrations. +- **SCE local source DB** — the existing repository-scoped `RepositoryAgentTraceDb` (see [agent-trace-db.md](agent-trace-db.md)), written by the hook/lifecycle paths already documented there. This reader does not change its writer, schema, or migrations. - **Incremental export reader** — `AgentTraceExportReader<'a>`, described below. Read-only, stateless across calls, no network. -- **Future control-plane client** — out of scope for this plan. A later plan composes this reader with an HTTP client and `sce trace sync` orchestration; nothing in this reader assumes or depends on that client existing. +- **Control-plane client** — `sce trace sync` (see [agent-trace-sync-command.md](../cli/agent-trace-sync-command.md)) composes this reader with the authenticated control-plane HTTP client; the reader itself remains unaware of that caller. ## Composition point @@ -56,9 +56,9 @@ Each stream has an owned `serde::Serialize` export-row DTO (`AgentTraceMessageEx This reader introduces no local sync state and no outbound transport: -- No local sync cursor is stored anywhere; the caller (a future `sce trace sync`) owns cursor persistence entirely outside this module. +- No local sync cursor is stored anywhere; the caller (`sce trace sync`) derives cursors from the control plane's `/state` response on every invocation, entirely outside this module. - No `agent-trace-sync.db` or any other new database or table exists. - No Turso Sync, no ETL pipeline, and no data-warehouse (DWH) integration exists. - No network call, no HTTP client, and no auth/WorkOS code exists in this module. -See also: [agent-trace-db.md](agent-trace-db.md), [context-map.md](../context-map.md) +See also: [agent-trace-db.md](agent-trace-db.md), [agent-trace-sync-command.md](../cli/agent-trace-sync-command.md), [context-map.md](../context-map.md) diff --git a/context/sce/auth-db.md b/context/sce/auth-db.md index a069c2ef..227c0d95 100644 --- a/context/sce/auth-db.md +++ b/context/sce/auth-db.md @@ -45,6 +45,7 @@ Current migration baseline: ## Token storage integration - `cli/src/services/token_storage.rs` now uses `AuthDb` for all persistence operations (`save_tokens`, `load_tokens`, `delete_tokens`) via a `OnceLock>` lazy singleton. +- Token-storage operations are synchronous because they wrap the encrypted local DB and OS credential store. Async consumers such as the Agent Trace control-plane client must call them through a blocking-task boundary; the sync command's rule is documented in [agent-trace-sync-command.md](../cli/agent-trace-sync-command.md). - `token_file_path()` returns the auth DB path from `auth_db_path()` instead of a JSON file path. - `TokenStorageError` exposes `PathResolution` and `Database` variants; former `Io`, `Serialization`, `CorruptedTokenFile`, and `Permission` variants have been removed. - No JSON file I/O remains in `token_storage.rs`.