diff --git a/cli/src/services/db/mod.rs b/cli/src/services/db/mod.rs index 9f08cdee..74fc35df 100644 --- a/cli/src/services/db/mod.rs +++ b/cli/src/services/db/mod.rs @@ -621,6 +621,45 @@ impl TursoDb { self.core.run_migrations() } + /// Run a passive WAL checkpoint (`PRAGMA wal_checkpoint(PASSIVE)`). + /// + /// PASSIVE checkpoints only what is currently safe to move from the WAL + /// into the main database file and never blocks on active readers or + /// writers, so it does not guarantee WAL truncation. Safe to call + /// repeatedly. Routine maintenance only; not a durability boundary. + pub fn passive_checkpoint(&self) -> Result<()> { + let operation_name = format!("checkpoint {} database WAL", M::db_name()); + + run_with_retry_sync( + resolve_query_retry_policy::(), + &operation_name, + QUERY_RETRY_HINT, + |_| { + block_on_isolated(&self.core.runtime, async { + let mut rows = self + .core + .conn + .query("PRAGMA wal_checkpoint(PASSIVE)", ()) + .await + .map_err(|e| { + anyhow::anyhow!("{} WAL checkpoint failed: {e}", M::db_name()) + })?; + + while rows + .next() + .await + .map_err(|e| { + anyhow::anyhow!("{} WAL checkpoint row fetch failed: {e}", M::db_name()) + })? + .is_some() + {} + + Ok(()) + }) + }, + ) + } + /// Check migration metadata for problems that would prevent safe hook /// runtime access. /// @@ -891,10 +930,97 @@ impl EncryptedTursoDb { #[cfg(test)] mod tests { + use std::time::{SystemTime, UNIX_EPOCH}; + use super::*; const QUERY_RETRY_FAILURE_BUDGET_MS: u64 = 2_000; + struct TestDbSpec; + + impl DbSpec for TestDbSpec { + fn db_name() -> &'static str { + "test" + } + + fn db_path() -> Result { + unreachable!("tests always open via TursoDb::new_at with an explicit path") + } + + fn migrations() -> &'static [(&'static str, &'static str)] { + &[] + } + + fn db_config_key() -> &'static str { + "test_db" + } + } + + fn unique_test_db_path() -> 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-db-mod-test-{}-{nonce}", std::process::id())) + .join("test.db") + } + + fn open_test_db() -> (TursoDb, PathBuf) { + let db_path = unique_test_db_path(); + let db = TursoDb::::new_at(&db_path).expect("test DB should open"); + db.execute( + "CREATE TABLE IF NOT EXISTS checkpoint_probe (value TEXT NOT NULL)", + (), + ) + .expect("test table creation should succeed"); + + (db, db_path) + } + + fn cleanup_test_db(db: TursoDb, db_path: &Path) { + drop(db); + if let Some(parent) = db_path.parent() { + let _ = fs::remove_dir_all(parent); + } + } + + #[test] + fn passive_checkpoint_keeps_previously_written_data_readable() { + let (db, db_path) = open_test_db(); + + db.execute( + "INSERT INTO checkpoint_probe (value) VALUES (?1)", + ("hello",), + ) + .expect("insert should succeed"); + + db.passive_checkpoint() + .expect("passive checkpoint should succeed"); + + let values = db + .query_map("SELECT value FROM checkpoint_probe", (), |row| { + row.get::(0).map_err(Into::into) + }) + .expect("post-checkpoint read should succeed"); + + assert_eq!(values, vec![String::from("hello")]); + + cleanup_test_db(db, &db_path); + } + + #[test] + fn passive_checkpoint_is_safe_to_call_repeatedly() { + let (db, db_path) = open_test_db(); + + db.passive_checkpoint() + .expect("first passive checkpoint should succeed"); + db.passive_checkpoint() + .expect("second passive checkpoint should succeed"); + + cleanup_test_db(db, &db_path); + } + fn worst_case_retry_failure_budget_ms(policy: RetryPolicy) -> u64 { let attempt_timeouts = policy .timeout_ms diff --git a/cli/src/services/hooks/mod.rs b/cli/src/services/hooks/mod.rs index 21aee355..69661c5e 100644 --- a/cli/src/services/hooks/mod.rs +++ b/cli/src/services/hooks/mod.rs @@ -209,9 +209,12 @@ fn run_hooks_subcommand_in_repo( HookSubcommand::PostCommit { vcs_type, remote_url, - } => { - run_post_commit_subcommand_with_trace(repository_root, *vcs_type, remote_url.as_deref()) - } + } => run_post_commit_subcommand_with_trace( + repository_root, + *vcs_type, + remote_url.as_deref(), + logger, + ), HookSubcommand::PostRewrite { rewrite_method } => { run_post_rewrite_subcommand_with_trace(repository_root, subcommand, rewrite_method) } @@ -1391,6 +1394,7 @@ fn run_post_commit_subcommand( repository_root: &Path, vcs_type: Option, remote_url: &str, + logger: Option<&dyn Logger>, ) -> Result { run_post_commit_subcommand_with( repository_root, @@ -1405,10 +1409,13 @@ fn run_post_commit_subcommand( auto_sync::launch(root); Ok(()) }, + run_post_commit_passive_checkpoint, + logger, ) } -fn run_post_commit_subcommand_with( +#[allow(clippy::too_many_arguments)] +fn run_post_commit_subcommand_with( repository_root: &Path, vcs_type: Option, remote_url: &str, @@ -1416,6 +1423,8 @@ fn run_post_commit_subcommand_with( run_agent_trace_flow: B, resolve_auto_sync: C, launch_auto_sync: L, + run_passive_checkpoint: K, + logger: Option<&dyn Logger>, ) -> Result where F: FnOnce(&Path) -> Result, @@ -1427,10 +1436,22 @@ where ) -> Result, C: FnOnce(&Path) -> Result, L: FnOnce(&Path) -> Result<()>, + K: FnOnce(&Path) -> Result<()>, { let result = run_intersection_flow(repository_root)?; let _agent_trace = run_agent_trace_flow(repository_root, &result, vcs_type, remote_url)?; + if let Err(error) = run_passive_checkpoint(repository_root) { + if let Some(log) = logger { + log.warn( + "sce.agent_trace_db.passive_checkpoint_failed", + &error.to_string(), + &[], + None, + ); + } + } + if resolve_auto_sync(repository_root)? { let _ = launch_auto_sync(repository_root); } @@ -1442,6 +1463,15 @@ where )) } +fn run_post_commit_passive_checkpoint(repository_root: &Path) -> Result<()> { + let db = open_agent_trace_db_for_hook_runtime( + repository_root, + "Failed to open Agent Trace DB for post-commit checkpoint.", + )?; + + db.passive_checkpoint() +} + fn run_post_commit_agent_trace_flow( repository_root: &Path, flow_result: &PostCommitIntersectionFlowResult, @@ -1764,8 +1794,14 @@ fn run_post_commit_subcommand_with_trace( repository_root: &Path, vcs_type: Option, remote_url: Option<&str>, + logger: Option<&dyn Logger>, ) -> Result { - run_post_commit_subcommand(repository_root, vcs_type, remote_url.unwrap_or_default()) + run_post_commit_subcommand( + repository_root, + vcs_type, + remote_url.unwrap_or_default(), + logger, + ) } fn run_post_rewrite_subcommand(repository_root: &Path, rewrite_method: &str) -> Result { @@ -2930,13 +2966,24 @@ mod tests { events.borrow_mut().push("launch"); Ok(()) }, + |_| { + events.borrow_mut().push("checkpoint"); + Ok(()) + }, + None, ) .expect("successful post-commit should remain successful"); assert!(output.contains("post-commit hook processed intersection")); assert_eq!( events.into_inner(), - vec!["intersection", "persistence", "config", "launch"] + vec![ + "intersection", + "persistence", + "checkpoint", + "config", + "launch" + ] ); } @@ -2971,6 +3018,8 @@ mod tests { *launch_called.borrow_mut() = true; Ok(()) }, + |_| panic!("checkpoint must not run after persistence failure"), + None, ) .expect_err("validation failure should be returned"); @@ -2995,6 +3044,8 @@ mod tests { *launch_called.borrow_mut() = true; Ok(()) }, + |_| Ok(()), + None, ) .expect("disabled auto-sync should not affect post-commit success"); @@ -3016,6 +3067,8 @@ mod tests { *launch_called.borrow_mut() = true; Ok(()) }, + |_| panic!("checkpoint must not run after persistence failure"), + None, ) .expect_err("persistence failure should be returned"); @@ -3033,9 +3086,127 @@ mod tests { |_, _, _, _| Ok(minimal_agent_trace()), |_| Ok(true), |_| Err(anyhow!("spawn unavailable")), + |_| Ok(()), + None, ) .expect("launcher failure must not affect post-commit success"); assert!(output.contains("post-commit hook processed intersection")); } + + #[derive(Default)] + struct RecordingLogger { + warnings: std::sync::Mutex>, + } + + impl Logger for RecordingLogger { + fn info( + &self, + _event_id: &str, + _message: &str, + _fields: &[(&str, &str)], + _session_id: Option<&str>, + ) { + } + + fn debug( + &self, + _event_id: &str, + _message: &str, + _fields: &[(&str, &str)], + _session_id: Option<&str>, + ) { + } + + fn warn( + &self, + event_id: &str, + message: &str, + _fields: &[(&str, &str)], + _session_id: Option<&str>, + ) { + self.warnings + .lock() + .expect("warnings mutex should not be poisoned") + .push((event_id.to_string(), message.to_string())); + } + + fn error( + &self, + _event_id: &str, + _message: &str, + _fields: &[(&str, &str)], + _session_id: Option<&str>, + ) { + } + + fn log_cli_error( + &self, + _error: &crate::services::error::CliError, + _session_id: Option<&str>, + ) { + } + } + + #[test] + fn post_commit_checkpoint_runs_once_after_successful_persistence() { + let events = RefCell::new(Vec::new()); + + let output = run_post_commit_subcommand_with( + Path::new("/repo"), + None, + "", + |_| Ok(post_commit_flow_result()), + |_, _, _, _| { + events.borrow_mut().push("persistence"); + Ok(minimal_agent_trace()) + }, + |_| Ok(false), + |_| Ok(()), + |_| { + events.borrow_mut().push("checkpoint"); + Ok(()) + }, + None, + ) + .expect("successful checkpoint should not affect post-commit success"); + + assert!(output.contains("post-commit hook processed intersection")); + assert_eq!(events.into_inner(), vec!["persistence", "checkpoint"]); + } + + #[test] + fn post_commit_checkpoint_failure_is_fail_open_and_logs_warning() { + let logger = RecordingLogger::default(); + let persisted = RefCell::new(false); + + let output = run_post_commit_subcommand_with( + Path::new("/repo"), + None, + "", + |_| Ok(post_commit_flow_result()), + |_, _, _, _| { + *persisted.borrow_mut() = true; + Ok(minimal_agent_trace()) + }, + |_| Ok(false), + |_| Ok(()), + |_| Err(anyhow!("checkpoint failed")), + Some(&logger), + ) + .expect("checkpoint failure must not affect post-commit success"); + + assert!(output.contains("post-commit hook processed intersection")); + assert!(*persisted.borrow()); + assert_eq!( + logger + .warnings + .into_inner() + .expect("warnings mutex should not be poisoned"), + vec![( + String::from("sce.agent_trace_db.passive_checkpoint_failed"), + String::from("checkpoint failed") + )] + ); + } } diff --git a/context/context-map.md b/context/context-map.md index 60b3d560..9163d444 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -61,7 +61,7 @@ Feature/domain context: - `context/sce/agent-trace-post-rewrite-local-remap-ingestion.md` (current post-rewrite no-op baseline plus historical remap-ingestion reference) - `context/sce/agent-trace-rewrite-trace-transformation.md` (current post-rewrite no-op baseline plus historical rewrite-transformation reference) - `context/sce/local-db.md` (implemented `cli/src/services/local_db/mod.rs` local database spec with `LocalDb = TursoDb`, canonical local DB path resolution, zero local migrations, and inherited retry-backed blocking `execute`/`query`/`query_map` methods using the shared Turso adapter) -- `context/sce/shared-turso-db.md` (current shared `cli/src/services/db/mod.rs` Turso database infrastructure seam, including `DbSpec`, generic `TursoDb`, encrypted `EncryptedTursoDb`, build-time generated migration constants from `cli/build.rs`/Cargo `OUT_DIR`, config-driven constructor/open-connect retry via `run_with_retry_sync`, no-migration `TursoDb::open_without_migrations()` / explicit-path `open_without_migrations_at(path)` for hot runtime paths, migration-running `new()` / explicit-path `new_at(path)` / `run_migrations()` with per-database `__sce_migrations` tracking, config-driven operation retry for `execute`/`query`/`query_values`/`query_map` with a `<= 2_000ms` default query failure budget, raw-value row fetching for deterministic operator-facing rendering, row-mapping excluded from retry, generic embedded migration execution, non-mutating `migration_metadata_problems()` and `ensure_schema_ready(setup_guidance)` readiness methods on `TursoDb`, and concrete wrappers for `LocalDb`, `AuthDb`, plus `RepositoryAgentTraceDb`) +- `context/sce/shared-turso-db.md` (current shared `cli/src/services/db/mod.rs` Turso database infrastructure seam, including `DbSpec`, generic `TursoDb`, encrypted `EncryptedTursoDb`, build-time generated migration constants from `cli/build.rs`/Cargo `OUT_DIR`, config-driven constructor/open-connect retry via `run_with_retry_sync`, no-migration `TursoDb::open_without_migrations()` / explicit-path `open_without_migrations_at(path)` for hot runtime paths, migration-running `new()` / explicit-path `new_at(path)` / `run_migrations()` with per-database `__sce_migrations` tracking, config-driven operation retry for `execute`/`query`/`query_values`/`query_map` with a `<= 2_000ms` default query failure budget, raw-value row fetching for deterministic operator-facing rendering, row-mapping excluded from retry, generic embedded migration execution, non-mutating `migration_metadata_problems()` and `ensure_schema_ready(setup_guidance)` readiness methods on `TursoDb`, non-mutating-data `passive_checkpoint()` PASSIVE WAL checkpoint method on `TursoDb` (not on `EncryptedTursoDb`, fail-open, no truncation guarantee, called once by `sce hooks post-commit` after successful Agent Trace persistence — see `context/sce/agent-trace-hooks-command-routing.md`), and concrete wrappers for `LocalDb`, `AuthDb`, plus `RepositoryAgentTraceDb`) - `context/sce/auth-db.md` (encrypted `AuthDb = EncryptedTursoDb` adapter, canonical `/sce/auth.db` path, build-time generated `AUTH_MIGRATIONS` from `cli/migrations/auth/`, auth credential schema and updated-at trigger baseline, lifecycle setup/doctor integration, encrypted token-storage persistence, and `SCE_AUTH_DB_ENCRYPTION_KEY`/OS credential-store key handling) - `context/sce/agent-trace-db.md` (implemented Agent Trace database adapter: the sole repository-scoped `RepositoryAgentTraceDb` backed by the fresh multi-statement baseline schema plus the additive `source_instance_id` migration, with `repository_metadata` carrying both `repository_id` and a concurrency-safe atomic-claim `source_instance_id` (physical database identity, independent of `repository_id`), narrow concurrent-first-open repair for missing one-file baseline migration metadata after all required schema tables exist, no trace-table `checkout_id` columns, repository-level typed insert helpers for diff traces, post-commit intersections, Agent Trace rows, messages, and parts, repository-level recent diff-trace reads without checkout filtering including persisted hunk-model plus canonical touched-line-session enrichment for structured rows, on-demand command/hook initialization with no daemon/background service, and the never-touch on-disk boundary for any pre-migration checkout-scoped/global DB files; the checkout-scoped `AgentTraceDb` adapter, its `agent_trace_db_path()`/`agent_trace_db_path_for_checkout()` helpers, and the 15-file `cli/migrations/agent-trace/` chain were removed by the `retire-legacy-agent-trace-db` plan; active hook writers/readers and Agent Trace setup/lifecycle resolve repository storage through `agent_trace_storage`) - `context/sce/agent-trace-export-readers.md` (implemented `AgentTraceExportReader<'a>` in `cli/src/services/agent_trace_export/mod.rs`: the read-only local export boundary over `RepositoryAgentTraceDb` — `read_messages_after`/`read_parts_after`/`read_diff_traces_after`/`read_agent_traces_after`, each cursor/limit/JS-safe-integer validated, materialized into owned camelCase `serde::Serialize` DTOs; composes directly with `ResolvedAgentTraceStorage` without owning `source_instance_id`; no local sync cursor, no `agent-trace-sync.db`, no Turso Sync, no ETL, no DWH) diff --git a/context/plans/agent-trace-passive-wal-checkpoint.md b/context/plans/agent-trace-passive-wal-checkpoint.md new file mode 100644 index 00000000..965725a6 --- /dev/null +++ b/context/plans/agent-trace-passive-wal-checkpoint.md @@ -0,0 +1,126 @@ +# Plan: agent-trace-passive-wal-checkpoint + +## Change summary + +Add a reusable passive WAL checkpoint operation to the shared `TursoDb` +adapter (`cli/src/services/db/mod.rs`) and use it, for now, only for the +repository-scoped Agent Trace DB (`RepositoryAgentTraceDb`). The checkpoint is +triggered exactly once from the `sce hooks post-commit` lifecycle, after Agent +Trace persistence for that commit has already succeeded, so the high-frequency +`diff-trace` and `conversation-trace` hook writes keep their current no-checkpoint +behavior. This is new maintenance behavior on top of the existing +`experimental_multiprocess_wal(true)` local-DB setup, which today has no +checkpointing at all and therefore no bound on WAL growth. A failed checkpoint +never turns into a durability boundary: it is logged as a warning and the +post-commit hook still reports success as long as Agent Trace persistence +itself succeeded. + +## Acceptance criteria + +- [x] AC1: `TursoDb` exposes `passive_checkpoint(&self) -> Result<()>`, which executes `PRAGMA wal_checkpoint(PASSIVE)` through the existing query/runtime/retry infrastructure (no second Tokio runtime, no bypass of the adapter). + - Validate: `cargo test --manifest-path cli/Cargo.toml services::db` — new tests prove a local DB can write, then successfully run `passive_checkpoint()`, that data stays readable afterward, and that calling it repeatedly is safe, without asserting `-wal` file deletion/truncation. +- [x] AC2: The repository Agent Trace DB (`RepositoryAgentTraceDb = TursoDb`) uses this shared method directly, with no second/duplicate checkpoint abstraction. + - Validate: inspection — `grep -rn "wal_checkpoint" cli/src/services/` shows the `PRAGMA` text in exactly one place (`TursoDb::passive_checkpoint`), and all Agent Trace call sites invoke that shared method. +- [x] AC3: `diff-trace` and `conversation-trace` hook writers do not call `passive_checkpoint()`, and neither does `TursoDb::execute()`, the shared insert helpers, or `Drop`. + - Validate: inspection — `grep -n "passive_checkpoint" cli/src/services/hooks/mod.rs` shows it invoked only from the post-commit path, never from the diff-trace or conversation-trace persistence functions; `grep -n "passive_checkpoint" cli/src/services/db/mod.rs cli/src/services/agent_trace_db/mod.rs` shows no call from `execute()`, `insert_diff_trace()`, `insert_messages()`, `insert_parts()`, or any `Drop` impl. +- [x] AC4: `sce hooks post-commit` attempts exactly one passive checkpoint after Agent Trace persistence for that commit succeeds; a checkpoint failure is logged as a warning through the existing observability logger and does not fail the hook or affect already-persisted data. + - Validate: `cargo test --manifest-path cli/Cargo.toml services::hooks` — lifecycle tests cover successful-persistence+successful-checkpoint (hook succeeds) and successful-persistence+failed-checkpoint (hook still succeeds, previously persisted Agent Trace data remains persisted, a warning is logged). +- [x] AC5: Durable context describes the checkpoint lifecycle: passive-only routine maintenance, the post-commit trigger boundary, that high-frequency hooks do not checkpoint per write, that PASSIVE only checkpoints what is currently safe and does not guarantee WAL truncation, and that checkpoint failure does not invalidate previously committed data. + - Validate: inspection — `context/sce/shared-turso-db.md` and `context/sce/agent-trace-db.md` (and/or `context/sce/agent-trace-hooks-command-routing.md`) state these points. + +### Full validation + +- `cargo test --manifest-path cli/Cargo.toml` +- `cargo clippy --manifest-path cli/Cargo.toml` +- `nix flake check` +- `nix run .#pkl-check-generated` + +### Context sync + +- `context/sce/shared-turso-db.md` — new `passive_checkpoint()` method on `TursoDb`. +- `context/sce/agent-trace-db.md` — repository-scoped adapter reuses the shared checkpoint method; no second abstraction. +- `context/sce/agent-trace-hooks-command-routing.md` — post-commit lifecycle now attempts one passive checkpoint after successful persistence, fail-open with a logged warning. + +## Task context synchronization lifecycle + +Persist this field in every plan; this is durable plan state, not chat state: + +- **Task context synchronization:** every task carries `pending | synced | blocked`. + A completed task must be `synced` before another task can start or the plan can + finish. +- For `blocked`, record **Blocker**, **Required action**, and **Retry condition** + beside the status. Never infer `synced` from conversation history; write every + lifecycle transition to the plan file. + +## Constraints and non-goals + +- **In scope:** `cli/src/services/db/mod.rs` (shared `passive_checkpoint()` plus tests), `cli/src/services/hooks/mod.rs` (post-commit lifecycle wiring, fail-open logging, lifecycle tests), and the durable context files listed under Context sync. +- **Out of scope:** `diff-trace` and `conversation-trace` hook writers, `TursoDb::execute()`, `insert_diff_trace()`, `insert_messages()`, `insert_parts()`, any `Drop` impl, `EncryptedTursoDb`, the local/auth DB adapters, and any checkout-scoped Agent Trace DB surface. +- **Constraints:** reuse the existing `TursoDb` query/runtime/retry infrastructure — no new Tokio runtime, no bypass of the adapter; do not use `bail!` for a checkpoint failure that occurs after successful persistence; do not add retries beyond whatever generic DB behavior already applies to the operation; log the failure via the existing `Logger::warn` observability path with an `sce.agent_trace_db.*`-style event name (e.g. `sce.agent_trace_db.passive_checkpoint_failed`). +- **Non-goal:** WAL-size-based checkpoint thresholds, configurable checkpoint intervals, a background checkpoint worker or daemon, `FULL`/`RESTART`/`TRUNCATE` checkpoint modes, checkpoint-on-every-write behavior, checkpoint logic in `Drop`, or exposing checkpoint statistics beyond what the implementation/tests need. + +## Assumptions + +- The post-commit checkpoint step opens its own repository-scoped `RepositoryAgentTraceDb` handle through the existing `open_agent_trace_db_for_hook_runtime` helper, matching the pattern every other post-commit sub-step already uses, rather than threading one DB instance across the intersection, Agent Trace insert, and checkpoint steps. +- The seam for simulating checkpoint failure in tests is a small injectable checkpoint closure added alongside the existing injectable parameters (`run_intersection_flow`, `run_agent_trace_flow`, `resolve_auto_sync`, `launch_auto_sync`) already present on `run_post_commit_subcommand_with`, not a broader refactor of post-commit orchestration. +- The new warning event is named `sce.agent_trace_db.passive_checkpoint_failed`, following the existing `sce..` convention used by events such as `sce.hooks.diff_trace.agent_trace_db_open_failed`. + +## Task stack + +- [x] T01: `Add shared TursoDb::passive_checkpoint() with tests` (status:done) + - Task ID: T01 + - Scope: In — `cli/src/services/db/mod.rs`: new `pub fn passive_checkpoint(&self) -> Result<()>` on `TursoDb` that runs `PRAGMA wal_checkpoint(PASSIVE)` through the existing runtime/retry helpers; tests proving write-then-checkpoint-then-read and safe repeated calls, without asserting `-wal` file deletion/truncation. Out — `EncryptedTursoDb`, any non-`PASSIVE` checkpoint mode, exposing checkpoint statistics. + - Dependencies: none + - Done when: `TursoDb::passive_checkpoint()` compiles, executes `PRAGMA wal_checkpoint(PASSIVE)` via the shared connection/retry path, and is covered by tests for post-checkpoint readability and repeated-call safety. + - Verify: `cargo test --manifest-path cli/Cargo.toml services::db`; `cargo clippy --manifest-path cli/Cargo.toml` + - Completed: 2026-08-21 + - Files changed: `cli/src/services/db/mod.rs` + - Result: Added `pub fn passive_checkpoint(&self) -> Result<()>` on `TursoDb`, issuing `PRAGMA wal_checkpoint(PASSIVE)` through `conn.query` (draining the result row) inside the existing `run_with_retry_sync`/`block_on_isolated` query path, using `resolve_query_retry_policy::()` and `QUERY_RETRY_HINT` like the other query methods. Marked `#[allow(dead_code)]` since no call site exists until T02. Added a minimal test-only `TestDbSpec: DbSpec` (no migrations) plus `open_test_db`/`cleanup_test_db` helpers and two tests: one write/checkpoint/read-back test and one repeated-call-safety test. + - Verify (actual): `nix flake check` (repository policy blocks direct `cargo test`/`cargo clippy`/`cargo fmt --check` invocations in favor of this) — all checks passed, including `cli-tests` (378 passed, 0 failed, including both new `services::db::tests::passive_checkpoint_*` tests), `cli-clippy`, and `cli-fmt`. + - Deviations: Verification ran via `nix flake check` instead of the plan's literal `cargo test`/`cargo clippy` invocations, per repository bash-tool policy (`use-nix-flake-check-over-cargo-test` etc. in `.sce/config.json`); this exercises the same targeted tests plus clippy/fmt as part of the full check set. Two clippy pedantic findings were fixed during implementation: `#[allow(dead_code)]` added to `passive_checkpoint` (unused until T02 wires a call site) and `cleanup_test_db`'s `db_path` parameter changed from `PathBuf` to `&Path` to satisfy `clippy::needless_pass_by_value`. + - Context impact: Domain. Adds one new public method (`passive_checkpoint()`) to the shared `TursoDb` adapter's contract, documented in `context/sce/shared-turso-db.md`; no architectural or cross-domain change, and no call site yet (T02 wires the post-commit trigger and updates the hooks-routing context file separately). + - Context synchronization: synced + +- [x] T02: `Trigger one fail-open passive checkpoint from post-commit after successful Agent Trace persistence` (status:done) + - Task ID: T02 + - Scope: In — `cli/src/services/hooks/mod.rs`: call `RepositoryAgentTraceDb::passive_checkpoint()` exactly once after the post-commit Agent Trace write path succeeds, logging `sce.agent_trace_db.passive_checkpoint_failed` via `Logger::warn` on failure without failing the hook; lifecycle tests for success+success and success+failure (hook still succeeds, prior persisted data intact, warning emitted). Out — `diff-trace`/`conversation-trace` writers, `TursoDb::execute()`/insert helpers/`Drop`, retry-policy changes. + - Dependencies: T01 + - Done when: `sce hooks post-commit` attempts exactly one `passive_checkpoint()` call after successful persistence; a failing checkpoint still yields a successful post-commit result with previously persisted data intact and a logged warning; `diff-trace`/`conversation-trace` paths remain unchanged. + - Verify: `cargo test --manifest-path cli/Cargo.toml services::hooks` — targeted post-commit lifecycle tests (success+success, success+failure) pass + - Completed: 2026-08-21 + - Files changed: `cli/src/services/hooks/mod.rs`, `cli/src/services/db/mod.rs` + - Result: Added `run_post_commit_passive_checkpoint()`, opening `RepositoryAgentTraceDb` via the existing `open_agent_trace_db_for_hook_runtime` helper and calling `.passive_checkpoint()`. Added an injectable `run_passive_checkpoint: K` closure param (alongside the existing `run_intersection_flow`/`run_agent_trace_flow`/`resolve_auto_sync`/`launch_auto_sync` params) plus a `logger: Option<&dyn Logger>` param to `run_post_commit_subcommand_with`; the checkpoint runs immediately after `run_agent_trace_flow` succeeds and before auto-sync resolution, and a checkpoint failure is logged via `Logger::warn("sce.agent_trace_db.passive_checkpoint_failed", ...)` and otherwise ignored (fail-open). Threaded `logger` through `run_post_commit_subcommand` and `run_post_commit_subcommand_with_trace`, and updated the `HookSubcommand::PostCommit` call site in `run_hooks_subcommand_in_repo` to pass the already-available `logger` (previously unused on this path, unlike diff-trace/conversation-trace). Removed the now-stale `#[allow(dead_code)]` on `TursoDb::passive_checkpoint` in `db/mod.rs` since this task adds its first real call site. Added a test-only `RecordingLogger` (backed by `std::sync::Mutex`, since `Logger: Send + Sync`) and two new lifecycle tests (`post_commit_checkpoint_runs_once_after_successful_persistence`, `post_commit_checkpoint_failure_is_fail_open_and_logs_warning`); updated the 5 pre-existing `run_post_commit_subcommand_with` tests for the new closure/logger params. + - Verify (actual): `nix flake check` (repository policy blocks direct `cargo test`/`cargo clippy`/`cargo fmt --check`) — all checks passed: `cli-tests` (380 passed, 0 failed, including both new `services::hooks::tests::post_commit_checkpoint_*` tests), `cli-clippy`, `cli-fmt`. A first `cli-tests` run failed one unrelated test (`services::agent_trace_storage::tests::new_repository_database_starts_empty_and_shares_repository_level_rows`, `left: 2, right: 1`); stashing all working-tree changes and rerunning `nix flake check` against unmodified `HEAD` reproduced a clean pass, and a subsequent run with this task's changes restored also passed — confirming a pre-existing test-isolation flake in `agent_trace_storage` tests, unrelated to this task's changes, not a regression it introduced. + - Deviations: Verification ran via `nix flake check` instead of the plan's literal `cargo test` invocation, per repository bash-tool policy, matching T01's precedent. Threading `logger: Option<&dyn Logger>` through `run_post_commit_subcommand`/`run_post_commit_subcommand_with_trace`/the `HookSubcommand::PostCommit` call site was required but not explicitly named in the task scope; it was necessary to satisfy AC4's explicit requirement to log via the existing `Logger::warn` observability path, and mirrors the pattern diff-trace/conversation-trace already use. `RecordingLogger` uses `std::sync::Mutex` rather than `RefCell` because `Logger: Send + Sync`. + - Context impact: Domain. Adds a new post-commit lifecycle step (fail-open passive checkpoint after successful Agent Trace persistence) and a new observability warning event (`sce.agent_trace_db.passive_checkpoint_failed`); no architectural or cross-domain change. Durable context (`context/sce/agent-trace-hooks-command-routing.md`) needs to document this new post-commit step per the plan's Context sync section. + - Context synchronization: synced + +## Open questions + +None. The request names the exact method signature, call site, failure semantics, logging convention, test expectations, and an explicit non-goal list, leaving no scope, criteria, or ordering ambiguity to resolve. + +## Validation Report + +**Status:** validated +**Date:** 2026-08-21 + +### Commands run + +- `nix flake check` -> exit 0 (all checks passed: `cli-tests`, `cli-clippy`, `cli-fmt`, and the rest of the flake's check set, run in place of the plan's literal `cargo test --manifest-path cli/Cargo.toml` / `cargo clippy --manifest-path cli/Cargo.toml`, which repository policy `.sce/config.json` blocks in favor of `nix flake check`) +- `nix run .#pkl-check-generated` -> exit 0 (ephemeral Pkl generation passed: 107 files, inventory sha256 `8500d6e4d8cbbe7ae540c52254a0b35b6e48834956823eeaf05e8af347d68bdb`) + +### Success-criteria verification + +- [x] AC1: `TursoDb` exposes `passive_checkpoint(&self) -> Result<()>` executing `PRAGMA wal_checkpoint(PASSIVE)` through the existing query/runtime/retry path -> `nix flake check`'s `cli-tests` job includes `services::db::tests::passive_checkpoint_keeps_previously_written_data_readable` and `services::db::tests::passive_checkpoint_is_safe_to_call_repeatedly`, both passing; `passive_checkpoint` (`cli/src/services/db/mod.rs:630`) issues the PRAGMA via `conn.query` inside `run_with_retry_sync`/`block_on_isolated`. +- [x] AC2: `RepositoryAgentTraceDb` uses the shared method directly, no duplicate abstraction -> `grep -rn "wal_checkpoint" cli/src/services/` shows the PRAGMA text in exactly one place (`cli/src/services/db/mod.rs:642`, inside `passive_checkpoint`); the only call site is `db.passive_checkpoint()` at `cli/src/services/hooks/mod.rs:1472`. +- [x] AC3: `diff-trace`/`conversation-trace` writers, `TursoDb::execute()`, insert helpers, and `Drop` never call `passive_checkpoint()` -> `grep -n "passive_checkpoint" cli/src/services/hooks/mod.rs` shows it referenced only inside the post-commit closure chain (`run_post_commit_subcommand`, `run_post_commit_subcommand_with`, `run_post_commit_passive_checkpoint`) and a matching test; `grep -n "passive_checkpoint" cli/src/services/db/mod.rs cli/src/services/agent_trace_db/mod.rs` shows no hits in `agent_trace_db/mod.rs` and, in `db/mod.rs`, only the method definition and its own tests — no hits inside `execute()` (`db/mod.rs:456`, `:813`) or any `Drop` impl. +- [x] AC4: `sce hooks post-commit` attempts exactly one passive checkpoint after successful Agent Trace persistence, fails open with a logged warning -> `cli/src/services/hooks/mod.rs:1441-1453` runs `run_agent_trace_flow` first (propagating its error via `?`), then calls `run_passive_checkpoint` exactly once and only logs via `Logger::warn("sce.agent_trace_db.passive_checkpoint_failed", ...)` on failure without returning an error; `nix flake check`'s `cli-tests` job includes `services::hooks::tests::post_commit_checkpoint_runs_once_after_successful_persistence` and `services::hooks::tests::post_commit_checkpoint_failure_is_fail_open_and_logs_warning`, both passing. +- [x] AC5: Durable context describes the checkpoint lifecycle -> `context/sce/shared-turso-db.md:25` documents `passive_checkpoint()`, its query/runtime/retry path, that PASSIVE checkpoints only what is currently safe without guaranteeing WAL truncation, that it is routine maintenance and not a durability boundary, and that post-commit is the sole caller; `context/sce/agent-trace-hooks-command-routing.md:64` documents the post-commit trigger point (after Agent Trace persistence, before auto-sync), the fail-open warning event `sce.agent_trace_db.passive_checkpoint_failed`, that a failure does not affect already-persisted data, and that `diff-trace`/`conversation-trace` do not checkpoint per write. + +### Failed checks and follow-ups + +- None. + +### Residual risks + +- None identified. diff --git a/context/sce/agent-trace-hooks-command-routing.md b/context/sce/agent-trace-hooks-command-routing.md index 7ce009ec..89a427cb 100644 --- a/context/sce/agent-trace-hooks-command-routing.md +++ b/context/sce/agent-trace-hooks-command-routing.md @@ -61,6 +61,7 @@ - When validation passes, the payload is serialized and inserted into Agent Trace DB `agent_traces` using `commit_id` from flow-result commit metadata, `commit_time_ms` from flow-result post-commit timestamp metadata, a derived non-null `url` value formatted as `sce.crocoder.dev/trace/`, and the validated runtime `--remote-url` value persisted to nullable `agent_traces.remote_url`. - Post-commit Agent Trace success requires both schema validation and Agent Trace DB `agent_traces` persistence to succeed. - Current command-surface success output is: `post-commit hook processed intersection: commit=, intersection_files=`. +- After Agent Trace validation and `agent_traces` persistence succeed, post-commit runs exactly one passive WAL checkpoint through `RepositoryAgentTraceDb::passive_checkpoint()` (see [shared-turso-db.md](shared-turso-db.md)) before resolving auto-sync. This is routine maintenance, not a durability boundary: a checkpoint failure is logged as a warning via `Logger::warn` with event `sce.agent_trace_db.passive_checkpoint_failed` and does not fail the hook or affect already-persisted Agent Trace data. `diff-trace` and `conversation-trace` do not checkpoint per write; only this one post-commit call site does. - After Agent Trace validation and `agent_traces` persistence succeed, post-commit resolves the config-file-only `agent_trace.auto_sync` gate. When it is `true`, the hook invokes the sync-owned one-shot launcher exactly once with the repository root; the launcher starts the current `sce` executable as detached `sync --format json` work and is not awaited. Explicit `false` configuration does not launch; omitted configuration launches, and validation or persistence failure reaches the existing error path before the gate. Launcher/current-executable/spawn failures are fail-open and do not change the successful post-commit result. No `pre-commit`, `diff-trace`, or `conversation-trace` path invokes automatic synchronization. - `post-rewrite` is a deterministic no-op entrypoint. - `diff-trace` reads STDIN JSON and classifies the payload: diff --git a/context/sce/shared-turso-db.md b/context/sce/shared-turso-db.md index 3952ce61..32411bcc 100644 --- a/context/sce/shared-turso-db.md +++ b/context/sce/shared-turso-db.md @@ -22,6 +22,7 @@ - explicit-path no-migration opening through `open_without_migrations_at(path)` for path-resolved hot runtime callers - `migration_metadata_problems(&self) -> Result>`: non-mutating readiness check that queries `__sce_migrations` metadata and compares applied migration IDs against `M::migrations()`; returns a list of problems (missing metadata table, incomplete applied migrations, unexpected extra migrations) or an empty list when the schema is ready - `ensure_schema_ready(&self, setup_guidance: &str) -> Result<()>`: non-mutating hook-readiness gate that calls `migration_metadata_problems()` and bails with a formatted error including `M::db_name()` and the caller-provided guidance string when problems are found; returns `Ok(())` when the schema is ready + - `passive_checkpoint(&self) -> Result<()>`: runs `PRAGMA wal_checkpoint(PASSIVE)` through the same query/runtime/retry path as `execute()`/`query()` (config-driven query retry, `block_on_isolated`), draining the checkpoint result row without exposing its busy/log/checkpointed statistics. PASSIVE checkpoints only what is currently safe to move from the WAL into the main database file and never blocks on active readers or writers, so it does not guarantee WAL truncation; safe to call repeatedly; not a durability boundary on its own. Routine maintenance only, not exposed on `EncryptedTursoDb`. `sce hooks post-commit` is the sole current caller: it runs `RepositoryAgentTraceDb::passive_checkpoint()` exactly once after post-commit Agent Trace persistence succeeds; a failing checkpoint is logged as a warning and never fails the hook or affects already-persisted data (see [agent-trace-hooks-command-routing.md](agent-trace-hooks-command-routing.md)). - `EncryptedTursoDb`: encrypted-adapter seam parallel to `TursoDb` with the same structural shape (connection, runtime bridge, and spec marker). `EncryptedTursoDb::new()` resolves the encryption key via `encryption_key::get_or_create_encryption_key()` (environment variable `SCE_AUTH_DB_ENCRYPTION_KEY` with OS credential-store fallback), enables Turso experimental local encryption, applies strict `aegis256` cipher selection through `turso::EncryptionOpts` during local DB open/connect, wraps that open/connect block in the same connection-open retry policy resolved from `policies.database_retry..connection_open`, and runs embedded migrations after connect. - `EncryptedTursoDb` exposes the same public synchronous `execute()`, `query()`, `query_map()`, and `run_migrations()` methods; operation methods use the same config-driven query retry policy as `TursoDb`. - `TursoConnectionCore` is internal to `cli/src/services/db/mod.rs` and owns the shared Turso connection plus tokio current-thread runtime bridging used by the public adapter methods; generic embedded migration execution with per-database `__sce_migrations` metadata is delegated to `run_embedded_migrations` helpers; encryption vs unencrypted behavior remains constructor-only at the public adapter layer.