From de77fdb1a67a2ddbb7bb6a8e87fcf31d3a2b4d2e Mon Sep 17 00:00:00 2001 From: David Abram Date: Thu, 15 Oct 2026 10:51:11 +0200 Subject: [PATCH 1/5] agent-trace: Persist optional commit patch IDs Add migration 007 and thread nullable commit_patch_id through repository Agent Trace inserts, preserving existing rows on upgrade and rejecting pre-migration schemas in hook and doctor paths. Add setup, idempotence, and persistence coverage, and document the schema and lifecycle changes. Plan: agent-trace-commit-patch-id (T01) Co-authored-by: SCE --- .../007_commit_patch_id.sql | 2 + cli/src/services/agent_trace_db/mod.rs | 11 +- cli/src/services/agent_trace_db/repository.rs | 2 - cli/src/services/agent_trace_storage/mod.rs | 268 +++++++++++++++++- cli/src/services/hooks/commit_hooks.rs | 1 + cli/src/services/sync/sync.rs | 1 + context/context-map.md | 3 +- context/glossary.md | 2 +- context/plans/agent-trace-commit-patch-id.md | 110 +++++++ context/sce/agent-trace-db-schema.md | 81 ++++++ context/sce/agent-trace-db.md | 88 +----- 11 files changed, 469 insertions(+), 100 deletions(-) create mode 100644 cli/migrations/agent-trace-repository/007_commit_patch_id.sql create mode 100644 context/plans/agent-trace-commit-patch-id.md create mode 100644 context/sce/agent-trace-db-schema.md diff --git a/cli/migrations/agent-trace-repository/007_commit_patch_id.sql b/cli/migrations/agent-trace-repository/007_commit_patch_id.sql new file mode 100644 index 000000000..da2175e32 --- /dev/null +++ b/cli/migrations/agent-trace-repository/007_commit_patch_id.sql @@ -0,0 +1,2 @@ +ALTER TABLE agent_traces +ADD COLUMN commit_patch_id TEXT; diff --git a/cli/src/services/agent_trace_db/mod.rs b/cli/src/services/agent_trace_db/mod.rs index 9c69240ea..cf4a2f6c5 100644 --- a/cli/src/services/agent_trace_db/mod.rs +++ b/cli/src/services/agent_trace_db/mod.rs @@ -36,7 +36,6 @@ FROM diff_traces WHERE time_ms >= ?1 AND time_ms <= ?2 ORDER BY time_ms ASC, id ASC"; -/// Parameterized SQL for inserting a post-commit patch intersection result. pub const INSERT_POST_COMMIT_PATCH_INTERSECTION_SQL: &str = "INSERT INTO post_commit_patch_intersections ( commit_id, @@ -48,12 +47,9 @@ pub const INSERT_POST_COMMIT_PATCH_INTERSECTION_SQL: &str = intersection_patch ) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)"; -/// Parameterized SQL for inserting a built agent trace payload. pub const INSERT_AGENT_TRACE_SQL: &str = - "INSERT INTO agent_traces (commit_id, commit_time_ms, trace_json, agent_trace_id, url, remote_url) VALUES (?1, ?2, ?3, ?4, ?5, ?6)"; + "INSERT INTO agent_traces (commit_id, commit_time_ms, trace_json, agent_trace_id, url, remote_url, commit_patch_id) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)"; -/// Parameterized SQL for inserting a message row. Duplicate -/// `(session_id, message_id)` writes are ignored. pub const INSERT_MESSAGE_SQL: &str = "INSERT INTO messages (session_id, message_id, role, generated_at_unix_ms) VALUES (?1, ?2, ?3, ?4) @@ -212,7 +208,6 @@ impl RecentDiffTracePatches { } } -/// Post-commit patch intersection result to persist in the agent trace database. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub struct PostCommitPatchIntersectionInsert<'a> { pub commit_id: &'a str, @@ -224,7 +219,6 @@ pub struct PostCommitPatchIntersectionInsert<'a> { pub intersection_patch: &'a str, } -/// Built agent trace payload to persist in the agent trace database. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub struct AgentTraceInsert<'a> { pub commit_id: &'a str, @@ -233,9 +227,9 @@ pub struct AgentTraceInsert<'a> { pub agent_trace_id: &'a str, pub url: &'a str, pub remote_url: &'a str, + pub commit_patch_id: Option<&'a str>, } -/// Message role constraint for the `messages` table. #[derive(Clone, Copy, Debug, Eq, PartialEq, serde::Serialize)] #[serde(rename_all = "lowercase")] pub enum MessageRole { @@ -342,6 +336,7 @@ async fn insert_agent_trace_with( input.agent_trace_id, input.url, input.remote_url, + input.commit_patch_id, ), ) .await diff --git a/cli/src/services/agent_trace_db/repository.rs b/cli/src/services/agent_trace_db/repository.rs index 1bfc8cb95..a5096f5d0 100644 --- a/cli/src/services/agent_trace_db/repository.rs +++ b/cli/src/services/agent_trace_db/repository.rs @@ -397,8 +397,6 @@ impl RepositoryAgentTraceDb { insert_post_commit_patch_intersection_with(self, input).await } - /// Insert a built Agent Trace payload into the repository-scoped - /// `agent_traces` table. pub async fn insert_agent_trace(&self, input: AgentTraceInsert<'_>) -> Result { insert_agent_trace_with(self, input).await } diff --git a/cli/src/services/agent_trace_storage/mod.rs b/cli/src/services/agent_trace_storage/mod.rs index 9d0a103c8..23865d76b 100644 --- a/cli/src/services/agent_trace_storage/mod.rs +++ b/cli/src/services/agent_trace_storage/mod.rs @@ -184,14 +184,6 @@ async fn open_repository_db_concurrently_safe( Err(last_error.expect("repository DB initialization should record an error")) } -/// No-migration open for high-frequency hook-runtime callers. -/// -/// Verifies schema readiness and applies only the same narrow -/// concurrent-first-open migration-metadata repair the setup/lifecycle fast -/// path applies, then initializes `source_instance_id` once readiness is -/// confirmed. Never falls back to running migrations: a missing or -/// baseline-only (pre-`002`) database fails with the existing `sce setup` -/// guidance instead. async fn open_repository_db_for_hook_runtime( db_path: &Path, repository_id: &str, @@ -256,11 +248,269 @@ pub(crate) async fn resolve_existing_agent_trace_storage_for_maintenance_at_stat #[cfg(test)] mod tests { + use std::path::{Path, PathBuf}; + use super::{ + open_repository_db_concurrently_safe, open_repository_db_for_hook_runtime, resolve_existing_agent_trace_storage_for_maintenance_at_state_root, AgentTraceStorageContext, }; - use crate::services::agent_trace_db::repository::ExistingRepositoryDbError; + use crate::generated_migrations::AGENT_TRACE_REPOSITORY_MIGRATIONS; + use crate::services::agent_trace_db::repository::{ + ExistingRepositoryDbError, RepositoryAgentTraceDb, + }; + use crate::services::agent_trace_db::AgentTraceInsert; + use crate::services::db::{DatabaseKind, DbSpec, TursoDb}; + + const COMMIT_PATCH_ID_MIGRATION: &str = "007_commit_patch_id"; + const PRE_COMMIT_PATCH_ID_REPOSITORY_ID: &str = "commit-patch-id-repo"; + const PRE_COMMIT_PATCH_ID_SOURCE_INSTANCE_ID: &str = "pre-007-source-instance"; + const PRE_COMMIT_PATCH_ID_TRACE_JSON: &str = "{\n \"version\": \"0.1.0\"\n}\n"; + + struct Pre007RepositoryAgentTraceDbSpec; + + impl DbSpec for Pre007RepositoryAgentTraceDbSpec { + fn db_name() -> &'static str { + "pre-007 repository Agent Trace DB" + } + + fn db_path() -> anyhow::Result { + anyhow::bail!("pre-007 fixture uses explicit paths") + } + + fn migrations() -> &'static [(&'static str, &'static str)] { + let position = AGENT_TRACE_REPOSITORY_MIGRATIONS + .iter() + .position(|(id, _)| *id == COMMIT_PATCH_ID_MIGRATION) + .expect("commit patch ID migration is embedded"); + &AGENT_TRACE_REPOSITORY_MIGRATIONS[..position] + } + + const KIND: DatabaseKind = DatabaseKind::AgentTrace; + } + + type AgentTraceRow = ( + i64, + String, + i64, + String, + String, + String, + String, + Option, + ); + + async fn create_pre_commit_patch_id_db(path: &Path) { + let db = TursoDb::::new_at(path) + .await + .expect("pre-007 database"); + db.execute( + "INSERT INTO repository_metadata (id, repository_id, source_instance_id) VALUES (1, ?1, ?2)", + ( + PRE_COMMIT_PATCH_ID_REPOSITORY_ID, + PRE_COMMIT_PATCH_ID_SOURCE_INSTANCE_ID, + ), + ) + .await + .expect("seed metadata"); + db.execute( + "INSERT INTO agent_traces (id, commit_id, commit_time_ms, trace_json, agent_trace_id, url, remote_url) VALUES (41, 'abc123', 7, ?1, 'trace-41', 'https://example.test/trace-41', 'https://example.test/repo.git')", + (PRE_COMMIT_PATCH_ID_TRACE_JSON,), + ) + .await + .expect("seed pre-007 trace"); + } + + async fn applied_migration_ids(db: &RepositoryAgentTraceDb) -> Vec { + db.query_map( + "SELECT id FROM __sce_migrations ORDER BY id ASC", + (), + |row| row.get::(0).map_err(Into::into), + ) + .await + .expect("migration ledger") + } + + async fn agent_trace_columns(db: &RepositoryAgentTraceDb) -> Vec<(String, i64)> { + db.query_map("PRAGMA table_info(agent_traces)", (), |row| { + Ok((row.get::(1)?, row.get::(3)?)) + }) + .await + .expect("agent_traces table info") + } + + async fn agent_trace_rows(db: &RepositoryAgentTraceDb) -> Vec { + db.query_map( + "SELECT id, commit_id, commit_time_ms, trace_json, agent_trace_id, url, remote_url, commit_patch_id FROM agent_traces ORDER BY id ASC", + (), + |row| { + Ok(( + row.get(0)?, + row.get(1)?, + row.get(2)?, + row.get(3)?, + row.get(4)?, + row.get(5)?, + row.get(6)?, + row.get(7)?, + )) + }, + ) + .await + .expect("agent_traces rows") + } + + fn expected_migration_ids() -> Vec { + AGENT_TRACE_REPOSITORY_MIGRATIONS + .iter() + .map(|(id, _)| (*id).to_string()) + .collect() + } + + fn pre_commit_patch_id_row() -> AgentTraceRow { + ( + 41, + String::from("abc123"), + 7, + String::from(PRE_COMMIT_PATCH_ID_TRACE_JSON), + String::from("trace-41"), + String::from("https://example.test/trace-41"), + String::from("https://example.test/repo.git"), + None, + ) + } + + #[tokio::test(flavor = "multi_thread")] + async fn commit_patch_id_setup_upgrades_pre_007_database_and_preserves_rows() { + let dir = tempfile::tempdir().expect("temp dir"); + let path = dir.path().join("agent-trace.db"); + create_pre_commit_patch_id_db(&path).await; + + for _ in 0..2 { + let (db, metadata) = + open_repository_db_concurrently_safe(&path, PRE_COMMIT_PATCH_ID_REPOSITORY_ID) + .await + .expect("setup open"); + + assert_eq!( + metadata.source_instance_id, + PRE_COMMIT_PATCH_ID_SOURCE_INSTANCE_ID + ); + assert_eq!(applied_migration_ids(&db).await, expected_migration_ids()); + assert!(agent_trace_columns(&db) + .await + .contains(&(String::from("commit_patch_id"), 0))); + assert_eq!(agent_trace_rows(&db).await, vec![pre_commit_patch_id_row()]); + db.ensure_schema_ready_for_hooks() + .await + .expect("upgraded schema is hook-ready"); + } + } + + #[tokio::test(flavor = "multi_thread")] + async fn commit_patch_id_hook_runtime_and_doctor_refuse_pre_007_database() { + let dir = tempfile::tempdir().expect("temp dir"); + let path = dir.path().join("agent-trace.db"); + create_pre_commit_patch_id_db(&path).await; + + let Err(hook_error) = + open_repository_db_for_hook_runtime(&path, PRE_COMMIT_PATCH_ID_REPOSITORY_ID).await + else { + panic!("hook runtime must not open a pre-007 database"); + }; + let hook_message = format!("{hook_error:#}"); + assert!(hook_message.contains("Run 'sce setup'."), "{hook_message}"); + assert!( + hook_message.contains(COMMIT_PATCH_ID_MIGRATION), + "{hook_message}" + ); + + let Err(doctor_error) = + RepositoryAgentTraceDb::open_existing_schema_ready_at(&path, || {}).await + else { + panic!("doctor readiness must reject a pre-007 database"); + }; + assert!(matches!( + doctor_error, + ExistingRepositoryDbError::IncompatibleSchema(_) + )); + + let db = RepositoryAgentTraceDb::open_existing_without_migrations_at(&path) + .await + .expect("read-only open"); + assert!(!applied_migration_ids(&db) + .await + .iter() + .any(|id| id == COMMIT_PATCH_ID_MIGRATION)); + assert!(!agent_trace_columns(&db) + .await + .iter() + .any(|(name, _)| name == "commit_patch_id")); + } + + #[tokio::test(flavor = "multi_thread")] + async fn commit_patch_id_insert_persists_present_and_absent_values() { + let dir = tempfile::tempdir().expect("temp dir"); + let path = dir.path().join("agent-trace.db"); + let (db, _metadata) = + open_repository_db_concurrently_safe(&path, PRE_COMMIT_PATCH_ID_REPOSITORY_ID) + .await + .expect("setup open"); + + for (agent_trace_id, commit_patch_id) in [ + ( + "trace-with-patch-id", + Some("0123456789abcdef0123456789abcdef01234567"), + ), + ("trace-without-patch-id", None), + ] { + db.insert_agent_trace(AgentTraceInsert { + commit_id: "fedcba9876543210fedcba9876543210fedcba98", + commit_time_ms: 11, + trace_json: PRE_COMMIT_PATCH_ID_TRACE_JSON, + agent_trace_id, + url: "https://example.test/trace", + remote_url: "https://example.test/repo.git", + commit_patch_id, + }) + .await + .expect("insert agent trace"); + } + + let stored = db + .query_map( + "SELECT agent_trace_id, commit_id, trace_json, commit_patch_id FROM agent_traces ORDER BY id ASC", + (), + |row| { + Ok(( + row.get::(0)?, + row.get::(1)?, + row.get::(2)?, + row.get::>(3)?, + )) + }, + ) + .await + .expect("stored traces"); + + assert_eq!( + stored, + vec![ + ( + String::from("trace-with-patch-id"), + String::from("fedcba9876543210fedcba9876543210fedcba98"), + String::from(PRE_COMMIT_PATCH_ID_TRACE_JSON), + Some(String::from("0123456789abcdef0123456789abcdef01234567")), + ), + ( + String::from("trace-without-patch-id"), + String::from("fedcba9876543210fedcba9876543210fedcba98"), + String::from(PRE_COMMIT_PATCH_ID_TRACE_JSON), + None, + ), + ] + ); + } #[tokio::test(flavor = "multi_thread")] async fn agent_trace_storage_maintenance_open_of_missing_db_creates_nothing() { diff --git a/cli/src/services/hooks/commit_hooks.rs b/cli/src/services/hooks/commit_hooks.rs index 26d204df5..42dd436de 100644 --- a/cli/src/services/hooks/commit_hooks.rs +++ b/cli/src/services/hooks/commit_hooks.rs @@ -310,6 +310,7 @@ where agent_trace_id: &agent_trace.id, url: &constructed_url, remote_url, + commit_patch_id: None, }; persist_agent_trace(insert_input).await?; diff --git a/cli/src/services/sync/sync.rs b/cli/src/services/sync/sync.rs index dfc1a4d6d..1e5c96b2d 100644 --- a/cli/src/services/sync/sync.rs +++ b/cli/src/services/sync/sync.rs @@ -1426,6 +1426,7 @@ mod contract_tests { agent_trace_id: "trace", url: "https://example.test/trace", remote_url: "https://example.test/repo.git", + commit_patch_id: None, }) .await .expect("insert agent trace"); diff --git a/context/context-map.md b/context/context-map.md index 3d725a77a..8dde7a6ce 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -92,7 +92,8 @@ Feature/domain context: - `context/sce/agent-trace-db-write-contention-evidence.md` (measured Agent Trace DB write-contention behavior under the busy-timeout/outer-retry/contention-deadline contract: the `lock_contention_tests.rs` suite with its lock-budget boundary, strict N=2–4 in-process and real-hook gates and N=8 stress characterization, policy history from 500 / 1250 to the current 1000 / 2250 defaults, the supported-load failure that motivated the tuning, before-vs-after comparison, and the bounded-policy exhaustion failure mode) - `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, async encrypted token-storage persistence through a Result-valued Tokio OnceCell, and `SCE_AUTH_DB_ENCRYPTION_KEY`/OS credential-store key handling) - `context/sce/telemetry-credential.md` (telemetry-only OpenBao credential: typed Control Plane issuance client, save-on-issue, single-row encrypted `auth.db` storage that never mints on lookup, deterministic auth-login `telemetry_provisioning` status/message rendering with secret-free unavailable warnings, production issue-and-save after WorkOS persistence on fresh/valid-stored/renewed paths, logout deletion alongside WorkOS credentials, issuing-token identity binding with session-ownership checks, repository-to-workspace enrollment bindings, local deletion not revoking the OpenBao token, and its use as the managed OTLP bearer source, the hidden `telemetry-headers` helper and its OS-user trust boundary with no endpoint-pinning claim, and Claude-native telemetry setup, rotation, logout, conflict handling, and override limits) -- `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 additive `source_instance_id` and `claude_model_state` migrations, 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, parts, and the non-exported exact-scope Claude model-state register, repository-level recent diff-trace reads without checkout filtering including persisted hunk-model plus canonical touched-line-session enrichment for structured rows, async 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-db.md` (implemented Agent Trace database adapter: the sole repository-scoped `RepositoryAgentTraceDb` backed by the fresh multi-statement baseline schema plus additive `source_instance_id`, `claude_model_state`, mutation-trace, and `007_commit_patch_id` (nullable `agent_traces.commit_patch_id`, setup-only upgrade) migrations, 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, parts, and the non-exported exact-scope Claude model-state register, repository-level recent diff-trace reads without checkout filtering including persisted hunk-model plus canonical touched-line-session enrichment for structured rows, async 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-db-schema.md` (per-table columns, lookup indexes, and `updated_at` triggers of the repository-scoped Agent Trace database, including the nullable `agent_traces.commit_patch_id` column) - `context/sce/agent-trace-export-readers.md` (implemented `AgentTraceExportReader<'a>` in `cli/src/services/agent_trace_export/mod.rs`: the read-only async 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`; `sce sync` consumes three of the four readers and `read_diff_traces_after` is a retained compatibility surface; no local sync cursor, no `agent-trace-sync.db`, no Turso Sync, no ETL, no DWH) - `context/sce/agent-trace-core-schema-migrations.md` (historical reference for removed local DB schema bootstrap behavior; T03 now implements the actual local DB with migrations) - `context/sce/agent-trace-retry-queue-observability.md` (inactive local-hook retry path plus historical retry/metrics reference) diff --git a/context/glossary.md b/context/glossary.md index 45e8ecc23..e397380ec 100644 --- a/context/glossary.md +++ b/context/glossary.md @@ -41,7 +41,7 @@ - `refresh single-flight guard`: Client-owned async coordination for Agent Trace control-plane authentication. Concurrent callers whose stored access token is expired, or whose request rejected the same token, serialize only refresh-and-save work, re-check credentials after acquiring the guard, and reuse the token saved by the first refresher; valid-token requests do not acquire the guard. The guard is held by the spawned task that performs renewal and save, so caller cancellation cannot release it mid-persistence; `wait_for_pending_refresh` acquires it as a completion barrier before `sce sync` returns. - `snapshot-ref reconciliation`: Explicit, per-worktree maintenance pass (`runtime::ref_maintenance::reconcile_explicit`, run only by `sce doctor --fix`) that deletes orphaned SCE snapshot pins under `refs/sce/mutation-cursor//` while retaining every tree in any worktree's durable roots; never automatic. See `context/cli/mutation-trace-ref-reconciliation.md`. - `ref-reconciliation advisory`: Lightweight hook-path check (`advise_if_due`) after a durably completed `Close`/`Flush` that may record and print a "reconciliation is recommended" reminder; it never reconciles, lists refs, opens the DB, or spawns Git. See `context/cli/mutation-scope-ref-advisory.md`. -- `repository-scoped Agent Trace DB`: Active Agent Trace storage shape where one logical Git repository maps to `/sce/repos//agent-trace.db`. The current seam is `RepositoryAgentTraceDb = TursoDb` in `cli/src/services/agent_trace_db/repository.rs`, backed by the fresh multi-statement `001_repository_schema` baseline plus the additive `002_repository_source_instance_id` migration, with `repository_metadata` (`repository_id` plus `source_instance_id`) plus repository-level trace tables, no `checkout_id` columns, and typed repository-level insert helpers for diff traces, post-commit intersections, Agent Trace rows, messages, and parts. Hook runtime, Agent Trace setup/lifecycle, and `sce sync` resolve repository-scoped storage through `agent_trace_storage`. This is the sole Agent Trace DB adapter; the checkout-scoped adapter and former trace inspection surface were removed by the `retire-legacy-agent-trace-db` plan. +- `repository-scoped Agent Trace DB`: Active Agent Trace storage shape where one logical Git repository maps to `/sce/repos//agent-trace.db`. The current seam is `RepositoryAgentTraceDb = TursoDb` in `cli/src/services/agent_trace_db/repository.rs`, backed by the fresh multi-statement `001_repository_schema` baseline plus the additive `002_repository_source_instance_id` migration, with `repository_metadata` (`repository_id` plus `source_instance_id`) plus repository-level trace tables, no `checkout_id` columns, and typed repository-level insert helpers for diff traces, post-commit intersections, Agent Trace rows, messages, and parts. Hook runtime, Agent Trace setup/lifecycle, and `sce sync` resolve repository-scoped storage through `agent_trace_storage`. This is the sole Agent Trace DB adapter; the checkout-scoped adapter and former trace inspection surface were removed by the `retire-legacy-agent-trace-db` plan. Its nullable `agent_traces.commit_patch_id` column (migration `007_commit_patch_id`) stores the `commit patch ID`: an optional Git patch-equivalence fingerprint of the original commit, never proof of unique commit identity (see [agent-trace-db.md](sce/agent-trace-db.md)). - `source_instance_id`: Physical-database identity column on `repository_metadata`, independent of the logical `repository_id`. Added by the additive `002_repository_source_instance_id` migration (existing/placeholder rows default to an empty string); generated once per physical `agent-trace.db` by application code (`generate_source_instance_id()`, UUID v4 today) and validated with `is_valid_source_instance_id()` (non-empty once trimmed) — never generated in SQL and never derived from `repository_id`, remote URL, checkout ID, filesystem path, hostname, or user/workspace identity. `RepositoryAgentTraceDb::verify_or_initialize_repository_metadata` claims it with a concurrency-safe `UPDATE ... WHERE source_instance_id = ''`, so concurrent first opens of the same physical database converge on one winner and an already-valid value is never overwritten; the value stays stable across reopen and repeated `sce setup` runs. Two independently created databases for the same logical repository (for example two clones) get different `source_instance_id` values. See `context/sce/agent-trace-db.md`. - `checkout registry` (removed): The central JSON registry at `/sce/checkout-registry.json` was removed in the `remove-checkout-registry` plan. The later trace inspection surface was also removed; current checkout IDs are diagnostic metadata only and are not discovered as command targets. Any pre-migration `/sce/agent-trace-*.db` files remain untouched. See `context/cli/checkout-identity.md`. - `generated OpenCode plugin registration contract`: Current generated-config contract where `config/.opencode/opencode.json` serializes the OpenCode `plugin` field from canonical Pkl sources for SCE-managed plugins only; the current registered paths, in order, are `./plugins/sce-bash-policy.ts`, `./plugins/sce-agent-trace.ts`, and `./plugins/sce-mutation-scope.ts`. `sce-mutation-scope` must stay last; the OpenCode config merge appends the generated SCE entries after arbitrary user plugins and `sce doctor` (`inspect_opencode_plugin_ordering_health`) flags a non-last position. Claude does not use an OpenCode-style plugin manifest; Claude bash-policy enforcement is registered through generated `.claude/settings.json` as a `PreToolUse` `Bash` command hook running `sce policy bash`. diff --git a/context/plans/agent-trace-commit-patch-id.md b/context/plans/agent-trace-commit-patch-id.md new file mode 100644 index 000000000..f46164e9c --- /dev/null +++ b/context/plans/agent-trace-commit-patch-id.md @@ -0,0 +1,110 @@ +# Plan: agent-trace-commit-patch-id + +## Change summary + +Extend the existing SCE CLI Agent Trace capture and synchronization pipeline to persist a stable Git patch fingerprint for each original commit. When `post-commit` creates an Agent Trace, SCE computes `git patch-id --stable` from the full committed patch, saves the result in the repository-scoped `agent_traces.commit_patch_id` column, and exports it as the optional `commitPatchId` field of the existing `agent_traces` sync stream. The fingerprint is independent of the original commit SHA and of SCE's AI-attribution intersection patch. + +This extends existing `post-commit` → `RepositoryAgentTraceDb` → `AgentTraceExportReader` → `sce sync` behavior. The Agent Trace JSON, original `commit_id`, source identity, sync cursors, remote stream count, and auto-sync lifecycle remain unchanged. `insert_agent_trace` has exactly one production writer (`run_post_commit_agent_trace_flow` in `cli/src/services/hooks/commit_hooks.rs`; `post-rewrite` is a no-op), so no other capture path needs the column. Control Plane support for the incoming optional `commitPatchId` field is a release prerequisite, not an implementation task in this SCE-only plan. + +## Acceptance criteria + +How this plan is proven complete. Each criterion is observable and names the check that proves it. `/validate` runs these checks; no task in the stack performs final validation. + +- [ ] AC1: For a normal non-merge commit, the captured fingerprint equals the first column of `git patch-id --stable` applied to the commit's entire canonical binary-capable Git patch (the documented pinned invocation); changing only the commit message, or replaying an equivalent patch onto another parent, does not change the fingerprint; user Git diff configuration (prefixes, path quoting, diff algorithm, context, color, external diff/textconv) does not change it. + - Validate: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id` using temporary Git repositories covering commit-message amend, equivalent cherry-pick/rebase, root commit, and user diff-configuration variations (guard with `-- --list` that the filter matches at least one test). +- [ ] AC2: Empty commits and multi-parent merge commits yield a `None` fingerprint; binary-only and rename-only commits produce deterministic fingerprints. A failed, malformed-output, oversized, or timed-out Git subprocess returns an error (surfaced as a sanitized warning) without panicking, leaking a child process, or blocking indefinitely, and the original Agent Trace row is still persisted with `commit_patch_id IS NULL`. + - Validate: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id` with binary, rename, empty, merge, error, malformed-output, size-limit, and timeout cases; assert `None` for empty/merge and an error/diagnostic for actual failures. +- [ ] AC3: An existing repository Agent Trace database at migration `006_mutation_trace_health_invariant` upgrades through `sce setup` to `007_commit_patch_id`, retaining existing trace rows, IDs, URLs, and `trace_json`; old rows have `commit_patch_id IS NULL`. Repeated setup is idempotent. Hook runtime never runs migrations and reports `Run 'sce setup'.` guidance for a pre-007 schema; `sce doctor` reports the schema as not ready until setup runs. + - Validate: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id` against a pre-007 database fixture, inspecting `PRAGMA table_info(agent_traces)`, the `__sce_migrations` ledger, unchanged rows, and hook/doctor readiness results. +- [ ] AC4: Successful post-commit Agent Trace insertion persists the exact captured `commit_id` and its nullable `commit_patch_id` in the same SQL insert, without changing or reserializing the Agent Trace JSON for fingerprint purposes. + - Validate: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id` asserting local database values for successful and unavailable fingerprints and byte-identical `trace_json` versus the pre-change serialization path. +- [ ] AC5: `AgentTraceExportReader::read_agent_traces_after()` returns the stored optional patch ID unchanged; serialized export rows contain `commitPatchId` (string or `null`) and preserve existing fields, `traceJson` bytes, and `sourceRowId` ordering. + - Validate: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id` asserting query mapping, JSON serialization, and `traceJson` byte preservation for rows with and without fingerprints. +- [ ] AC6: `sce sync` and post-commit auto-sync send `commitPatchId` on the existing `agent_traces` batch payload, without introducing a fourth remotely uploaded stream, a new cursor, or changes to `/state`, retry, or conflict behavior. Previously acknowledged rows are not replayed because of this migration. + - Validate: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml sync_uploads_only_three_streams_and_reports_zero_upload_diff_traces` plus a captured local HTTP ingestion request assertion for `rows[*].commitPatchId`, cursor advancement, and no replay of rows at or below the server cursor. + +### Full validation + +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml` +- `nix flake check` + +### Context sync + +- `context/sce/agent-trace-db.md` — new nullable `agent_traces.commit_patch_id` column, additive migration `007_commit_patch_id`, setup-only migration / hook-runtime setup requirement, upgrade/downgrade note; repair existing drift (migration list in the adapter-seam paragraph stops at `005`, the `agent_traces` column list, stale `015_add_diff_traces_payload_type` reference, and the "hook runtime lazy initialization" sentence that contradicts the no-migration hook path). +- `context/sce/agent-trace-hooks-command-routing.md` — best-effort canonical fingerprint capture, exact pinned Git invocation, and failure behavior. +- `context/sce/agent-trace-export-readers.md` — optional `commitPatchId` export field. +- `context/cli/agent-trace-sync-command.md` — unchanged cursor protocol, new field on the existing `agent_traces` stream, Control Plane release dependency, and no historical backfill. + +## Task context synchronization lifecycle + +- **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/migrations/agent-trace-repository/007_commit_patch_id.sql`; `cli/src/services/agent_trace_db/mod.rs` (`AgentTraceInsert`, `INSERT_AGENT_TRACE_SQL`, `insert_agent_trace_with`); `cli/src/services/hooks/commit_hooks.rs` and a narrowly scoped Git patch-ID helper module under `cli/src/services/hooks/`; `cli/src/services/agent_trace_export/mod.rs`; the `sync.rs` test module; focused migration, hook, export, and sync tests; the durable context files listed above. +- **Out of scope:** Control Plane API/migration implementation, GitHub events, reconciliation/lineage graph, historical backfill, new SCE commands, remote `diff_traces` upload, changing the Agent Trace specification, rehashing trace JSON, adding another synchronization stream, and changing the existing `capture_head_patch_from_git` / intersection capture. +- **Canonical algorithm:** version-1 fingerprint is `git patch-id --stable` over the full commit patch generated with explicitly pinned flags, starting from `git show --format= --patch --binary --no-ext-diff --no-textconv --no-renames --no-color --diff-algorithm=myers `. Pin every additional option whose user configuration can change patch-id input (at minimum: source/destination prefix, `core.quotePath`, unified context, indent heuristic, `diff.relative`, `diff.orderFile`) via flags or `-c` overrides, and pass `--stable` explicitly so `patchid.stable`/`patchid.verbatim` config cannot change it. Document the final exact invocation for Control Plane's future matcher. A fingerprint is patch-equivalence evidence, never proof of unique commit identity. +- **Process safety:** no shell pipeline; invoke both Git subprocesses directly with `tokio::process::Command` (`kill_on_drop(true)`, following the precedent in `cli/src/services/mutation_trace/runtime/git_snapshot.rs`), streaming `git show` stdout into `git patch-id` stdin rather than buffering the diff, with a finite overall deadline, a byte bound on streamed patch input and on patch-id output, exit-status and output validation for both processes, and cleanup of both processes on cancellation/error. No dyn dispatch and no new generalized process framework. +- **Capture semantics:** use `post_commit_data.commit_oid` already captured for the Agent Trace, not a second HEAD lookup. Empty commits yield `None`; multi-parent merges yield `None` rather than a first-parent identity. Accept 40-hex (SHA-1) and 64-hex (SHA-256) object IDs and patch IDs; validate output as lowercase hex of the matching length. Never derive the ID from SCE's AI-contributed patch. +- **Failure policy:** fingerprint computation is best-effort. Emit a sanitized warning and persist `NULL` when unavailable, but never weaken existing Agent Trace build, validation, or persistence errors. No automatic retry and no replacement of the original trace row. +- **Migrations:** additive migration `007` rather than altering `001`. `sce setup` applies migrations; high-frequency hooks do not. Because hook readiness requires exact migration-ledger parity, an upgraded CLI fails open on all repository hook writes until `sce setup` reruns (existing precedent from migrations `003`–`006`). Older CLI binaries may reject a newer ledger on downgrade; document upgrade/downgrade behavior. +- **Release dependency:** deploy a Control Plane receiver that accepts and persists optional `commitPatchId` before releasing the SCE export, because the current server decoder may strip an unknown field while acknowledging the row, and acknowledged rows are never re-sent. +- **Non-goal:** automatic re-sync of traces the server already acknowledged. That requires an explicit later backfill/change stream, not cursor rewinding. + +## Assumptions + +- Fingerprints are a nullable column on `agent_traces` rather than a new normalized commits table; this is sufficient for the current single-trace-row-to-commit association and existing export protocol. +- The wire field is `commitPatchId`, a lowercase hex Git patch ID or `null`, always present on serialized `agent_traces` export rows (not skipped when `None`). A future schema may add an algorithm/version discriminator if the canonical algorithm changes. +- Verification commands use the repository convention (`nix develop -c ./scripts/run-cli-cargo.sh …`) instead of bare `cargo test --manifest-path cli/Cargo.toml`, because `cli/build.rs` requires the generated Pkl payload that the wrapper provides; the package is `shared-context-engineering`. +- No new Cargo dependency is needed: `tokio` already enables `process`, `io-util`, and `time`. +- `run_post_commit_agent_trace_flow` currently takes no logger; T03 threads the existing post-commit `Option<&L>` logger (or an equivalent warning seam) into it rather than inventing a new logging channel. +- Test cases may be grouped into table-driven tests sharing temporary repositories; every new test name contains `commit_patch_id` so the targeted filter selects them, and a zero-match filter never counts as passing. + +## Task stack + +- [x] T01: `Persist an optional commit patch ID in the local Agent Trace database` (status:done) + - Task ID: T01 + - Scope: In — `007_commit_patch_id.sql` (`ALTER TABLE agent_traces ADD COLUMN commit_patch_id TEXT`), `AgentTraceInsert.commit_patch_id: Option<&str>`, `INSERT_AGENT_TRACE_SQL` and parameter binding, updating existing `AgentTraceInsert` call sites with `None`, migration/readiness tests, and the `agent-trace-db.md` context update including the listed drift repair. Out — Git computation, hook wiring beyond compiling call sites, export. + - Dependencies: none + - Done when: Fresh and pre-007 repository databases apply the nullable column through the setup/lifecycle migration path; existing rows and `trace_json` are retained with `commit_patch_id IS NULL`; repeated setup is idempotent; the hook-runtime open still refuses a pre-007 schema with `Run 'sce setup'.` guidance; a caller can insert `Some(id)` or `None` in the same Agent Trace insert. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id` (migration/persistence cases). + - Completed: 2026-10-15 + - Files changed: `cli/migrations/agent-trace-repository/007_commit_patch_id.sql` (new), `cli/src/services/agent_trace_db/mod.rs`, `cli/src/services/agent_trace_db/repository.rs`, `cli/src/services/agent_trace_storage/mod.rs`, `cli/src/services/hooks/commit_hooks.rs`, `cli/src/services/sync/sync.rs`, `context/sce/agent-trace-db.md`, `context/plans/agent-trace-commit-patch-id.md` + - Result: Added additive migration `007_commit_patch_id` (`ALTER TABLE agent_traces ADD COLUMN commit_patch_id TEXT`); `AgentTraceInsert` gained `commit_patch_id: Option<&str>` bound as `?7` in `INSERT_AGENT_TRACE_SQL`; the post-commit writer and the sync test call site pass `None`. Three `commit_patch_id` tests in `agent_trace_storage` cover setup upgrade of a pre-007 fixture (test-only `DbSpec` with migrations `001`–`006`) preserving the seeded row, `trace_json`, and `source_instance_id` with `commit_patch_id IS NULL`, idempotent repeated setup with full ledger parity; hook-runtime refusal with `Run 'sce setup'.` and doctor `IncompatibleSchema` readiness without migrating; and `Some`/`None` inserts. Per user instruction, doc comments on touched and adjacent items (`INSERT_AGENT_TRACE_SQL` and neighbors, `AgentTraceInsert` and neighbors, `insert_agent_trace`, `open_repository_db_for_hook_runtime`) were removed; no new code comments were added. + - Verify outcome: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id -- --list` lists 3 tests; the filtered run passed (3 passed, 0 failed). Additional: `... test agent_trace` passed (69 passed); `... test sync_uploads_only_three_streams_and_reports_zero_upload_diff_traces` passed (1 passed); `... clippy --all-targets -- -D warnings` clean. + - Context impact: localized — `context/sce/agent-trace-db.md` updated (new column, migration `007` and upgrade/downgrade note, `AgentTraceInsert` field, drift repair for the `006` migration list, stale `015_add_diff_traces_payload_type` reference, and the hook-runtime lazy-initialization sentence). + - Deviation: setup-path parity is tested through the private storage open functions rather than an end-to-end `sce setup` invocation. + - Context synchronization: synced + +- [ ] T02: `Compute the canonical Git patch ID with bounded subprocesses` (status:todo) + - Task ID: T02 + - Scope: In — a small `hooks/` Git patch-ID helper module, the pinned `git show` options and config overrides, parent-count check for merges, streaming `git show` → `git patch-id --stable` with deadline, byte bounds, exit/output validation and `kill_on_drop` cleanup, the output parser, and temporary-Git-repository tests. Out — hook orchestration, persistence, sync. + - Dependencies: none + - Done when: The async helper takes a repository root and captured commit OID and returns `Result>`; normal, root, binary-only, and rename-only commits match the canonical invocation and are stable across message amend, equivalent cherry-pick, and user diff-configuration variations; empty and multi-parent merge commits return `Ok(None)`; non-zero exit, malformed output, oversized output, and timeout return bounded errors with no surviving child process. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id` (helper cases). + - Context synchronization: pending + +- [ ] T03: `Attach the fingerprint to post-commit Agent Trace insertion` (status:todo) + - Task ID: T03 + - Scope: In — `run_post_commit_agent_trace_flow` and its `run_post_commit_agent_trace_flow_with` test seam, fingerprint-to-insert wiring, best-effort sanitized warning, local storage tests, and the `agent-trace-hooks-command-routing.md` update documenting the exact pinned invocation. Out — changing Agent Trace JSON, changing intersection capture signatures, sync transport. + - Dependencies: T01, T02 + - Done when: The already-captured `commit_oid` is fingerprinted asynchronously after the intersection flow and before the Agent Trace database insert; the stored row carries the nullable ID without any change to `trace_json`; fingerprint errors log a sanitized warning and persist `NULL`; Agent Trace build, validation, and persistence failures keep their existing behavior. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id` (hook/storage cases). + - Context synchronization: pending + +- [ ] T04: `Export commitPatchId through the existing agent_traces sync stream` (status:todo) + - Task ID: T04 + - Scope: In — `AgentTraceAgentTraceExportRow.commit_patch_id: Option`, `SELECT_AGENT_TRACES_AFTER_SQL` and `agent_trace_export_row_from_turso`, JSON serialization, captured HTTP sync assertions in the `sync.rs` tests, and the `agent-trace-export-readers.md` / `agent-trace-sync-command.md` updates (release dependency, no backfill, upgrade/downgrade). Out — sync-engine rewrite, Control Plane migration/API code, historical backfill. + - Dependencies: T01, T03 + - Done when: The stored value is emitted as `commitPatchId` in the existing `agent_traces` ingestion batch; strings and nulls round-trip; `traceJson` remains raw; the three-stream protocol and authoritative server cursors are unchanged and pre-acknowledged rows are not resent; deployment and backfill limitations are documented. + - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id`; `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml sync_uploads_only_three_streams_and_reports_zero_upload_diff_traces`. + - Context synchronization: pending + +## Open questions + +None. The plan limits implementation to the SCE CLI and uses the previously agreed optional `commitPatchId` contract; the Control Plane rollout dependency, setup-required upgrade window, and lack of historical backfill are explicit constraints rather than unresolved design questions. diff --git a/context/sce/agent-trace-db-schema.md b/context/sce/agent-trace-db-schema.md new file mode 100644 index 000000000..71ef33c45 --- /dev/null +++ b/context/sce/agent-trace-db-schema.md @@ -0,0 +1,81 @@ +# Agent Trace Database Schema + +Table columns, lookup indexes, and triggers of the repository-scoped Agent Trace database produced by `AGENT_TRACE_REPOSITORY_MIGRATIONS`. Migration ordering, setup-only application, and upgrade notes live in [agent-trace-db.md](agent-trace-db.md). + +The `diff_traces` baseline migration creates: + +- `id INTEGER PRIMARY KEY` +- `time_ms INTEGER NOT NULL` +- `session_id TEXT NOT NULL` +- `patch TEXT NOT NULL` +- `created_at TEXT NOT NULL DEFAULT (...)` +- `model_id TEXT` +- `tool_name TEXT` +- `tool_version TEXT` +- `payload_type TEXT NOT NULL DEFAULT 'patch'` — discriminator for source payload format; `patch` for `OpenCode` unified-diff payloads, `structured` for `Claude` `PostToolUse` structured payloads. + +The `post_commit_patch_intersections` baseline migration creates: + +- `id INTEGER PRIMARY KEY` +- `commit_id TEXT NOT NULL` +- `post_commit_time_ms INTEGER NOT NULL` +- `recent_window_cutoff_ms INTEGER NOT NULL` +- `recent_window_end_ms INTEGER NOT NULL` +- `loaded_diff_trace_count INTEGER NOT NULL CHECK (loaded_diff_trace_count >= 0)` +- `skipped_diff_trace_count INTEGER NOT NULL CHECK (skipped_diff_trace_count >= 0)` +- `intersection_patch TEXT NOT NULL` +- `created_at TEXT NOT NULL DEFAULT (...)` + +The `agent_traces` baseline migration creates: + +- `id INTEGER PRIMARY KEY` +- `commit_id TEXT NOT NULL` +- `commit_time_ms INTEGER NOT NULL` +- `url TEXT NOT NULL` +- `remote_url TEXT` (nullable) +- `trace_json TEXT NOT NULL` +- `agent_trace_id TEXT NOT NULL UNIQUE` +- `created_at TEXT NOT NULL DEFAULT (...)` + +Migration `007_commit_patch_id` adds: + +- `commit_patch_id TEXT` (nullable) — optional Git patch ID of the original commit; `NULL` for rows created before the migration or when no fingerprint is available. + +The `messages` migration creates: + +- `id INTEGER PRIMARY KEY` +- `session_id TEXT NOT NULL` +- `message_id TEXT NOT NULL` +- `role TEXT NOT NULL CHECK (role IN ('user', 'assistant'))` +- `generated_at_unix_ms INTEGER NOT NULL CHECK (generated_at_unix_ms >= 0)` +- `created_at TEXT NOT NULL DEFAULT (...)` +- `updated_at TEXT NOT NULL DEFAULT (...)` + +The `parts` migration creates: + +- `id INTEGER PRIMARY KEY` +- `type TEXT NOT NULL` +- `text TEXT NOT NULL` +- `message_id TEXT NOT NULL` +- `session_id TEXT NOT NULL` +- `generated_at_unix_ms INTEGER NOT NULL CHECK (generated_at_unix_ms >= 0)` +- `created_at TEXT NOT NULL DEFAULT (...)` +- `updated_at TEXT NOT NULL DEFAULT (...)` + +No foreign keys exist between `messages` and `parts`; rows may be written out of order. The data model uses natural identifiers (`session_id`, `message_id`) for joins rather than DB-level referential integrity. + +Lookup indexes created by the baseline migration set: + +- `idx_diff_traces_time_ms_id` on `(time_ms, id)` +- `idx_agent_traces_agent_trace_id` on `(agent_trace_id)` +- `idx_agent_traces_remote_url` on `(remote_url)` +- `idx_messages_session_message` unique index on `(session_id, message_id)` — enables duplicate-ignore parent message inserts by natural key +- `idx_messages_session_order` on `(session_id, generated_at_unix_ms, id)` — enables chronological session-scoped message retrieval +- `idx_parts_session_message_order` on `(session_id, message_id, generated_at_unix_ms, id)` — enables ordered part joins per message + +`updated_at` triggers defined by the migration set: + +- `trg_messages_updated_at`: fires on `UPDATE` for non-`updated_at` column changes on `messages` +- `trg_parts_updated_at`: fires on `UPDATE` for non-`updated_at` column changes on `parts` + +Both triggers compare `OLD.*` vs `NEW.*` for all mutable columns (excluding `updated_at` itself) and refresh the timestamp only when a real change occurred. diff --git a/context/sce/agent-trace-db.md b/context/sce/agent-trace-db.md index c2d431dc2..4141b6903 100644 --- a/context/sce/agent-trace-db.md +++ b/context/sce/agent-trace-db.md @@ -18,7 +18,7 @@ - `recent_diff_trace_patches(cutoff_time_ms, end_time_ms)`: chronological `diff_traces` read helper for rows in the inclusive window `time_ms >= cutoff_time_ms AND time_ms <= end_time_ms`; parses raw patch text through `parse_patch` and skips malformed rows without failing the query. - `PostCommitPatchIntersectionInsert<'a>`: insert payload for post-commit intersection results with commit metadata, window bounds, loaded/skipped counts, and serialized patch JSON. - `insert_post_commit_patch_intersection()`: domain-specific insert helper using parameterized SQL. -- `AgentTraceInsert<'a>`: insert payload for built Agent Trace rows with `commit_id`, `commit_time_ms`, serialized `trace_json`, `agent_trace_id`, non-null `url`, and required `remote_url: &'a str` (Rust-API-only; DB column stays nullable). +- `AgentTraceInsert<'a>`: insert payload for built Agent Trace rows with `commit_id`, `commit_time_ms`, serialized `trace_json`, `agent_trace_id`, non-null `url`, required `remote_url: &'a str` (Rust-API-only; DB column stays nullable), and `commit_patch_id: Option<&'a str>` bound to the nullable `agent_traces.commit_patch_id` column in the same insert (`None` persists `NULL`). The post-commit writer currently passes `None`. - `insert_agent_trace()`: domain-specific insert helper for `agent_traces` using parameterized SQL. - `MessageRole` enum: `User` / `Assistant` — maps to `messages.role` DB constraint. - `InsertMessageInsert`: owned payload struct with insertable parent `messages` columns (`session_id`, `message_id`, `role`, `generated_at_unix_ms`); message body text belongs to `parts.text`, not the parent message row. @@ -41,7 +41,7 @@ pub type RepositoryAgentTraceDb = TursoDb; Repository persistence, readiness checks, transactions, and migration operations are native async methods inherited from `TursoDb`. Hook, lifecycle, mutation-trace, and sync callers await them on the application runtime before returning or advancing their cursor. The async migration changes no SQL, schema, WAL, encryption, or retry policy. -This adapter has no canonical `DbSpec::db_path()`; callers must resolve `/sce/repos//agent-trace.db` first and use explicit-path `TursoDb` constructors. Its migration list is `generated_migrations::AGENT_TRACE_REPOSITORY_MIGRATIONS`: the fresh multi-statement baseline `cli/migrations/agent-trace-repository/001_repository_schema.sql` plus the additive `002_repository_source_instance_id.sql` (adds `repository_metadata.source_instance_id`), `003_claude_model_state.sql` (adds the non-exported Claude model-state register), `004_mutation_trace_protocol.sql`, and `005_mutation_scope_provenance.sql` (the mutation-cursor protocol tables and the scope-provenance table; see [mutation-trace store](../cli/mutation-trace-store.md) and [mutation-scope provenance](../cli/mutation-scope-provenance.md)). The baseline schema includes `repository_metadata` plus the existing repository-level Agent Trace tables, indexes, and triggers, and intentionally has no `checkout_id` columns on trace tables. `RepositoryAgentTraceDb::verify_or_initialize_repository_metadata(repository_id) -> Result` inserts the singleton metadata row on first initialization, errors if an existing DB stores a different repository ID, and atomically claims `source_instance_id` for this physical database via `UPDATE ... WHERE source_instance_id = ''` (a losing racer's generated candidate is discarded and an already-valid stored value is never overwritten), returning the typed `RepositoryMetadata { repository_id, source_instance_id }`. `source_instance_id` is generated by application code (`generate_source_instance_id()`, UUID v4) and validated with `is_valid_source_instance_id()` (non-empty once trimmed); it is never derived from `repository_id`, remote URL, checkout ID, filesystem path, hostname, or user/workspace identity, and stays stable across reopen and repeated `sce setup` runs. `RepositoryAgentTraceDb::repair_missing_repository_schema_migration_metadata()` is a narrow concurrent-first-open repair seam: it never creates trace tables, but if every required repository schema table already exists and only the one-file baseline migration record is missing, it records `001_repository_schema` and rechecks readiness. +This adapter has no canonical `DbSpec::db_path()`; callers must resolve `/sce/repos//agent-trace.db` first and use explicit-path `TursoDb` constructors. Its migration list is `generated_migrations::AGENT_TRACE_REPOSITORY_MIGRATIONS`: the fresh multi-statement baseline `cli/migrations/agent-trace-repository/001_repository_schema.sql` plus the additive `002_repository_source_instance_id.sql` (adds `repository_metadata.source_instance_id`), `003_claude_model_state.sql` (adds the non-exported Claude model-state register), `004_mutation_trace_protocol.sql`, `005_mutation_scope_provenance.sql`, and `006_mutation_trace_health_invariant.sql` (the mutation-cursor protocol tables, the scope-provenance table, and the health-invariant rebuild; see [mutation-trace store](../cli/mutation-trace-store.md) and [mutation-scope provenance](../cli/mutation-scope-provenance.md)), plus `007_commit_patch_id.sql` (adds the nullable `agent_traces.commit_patch_id` column). The baseline schema includes `repository_metadata` plus the existing repository-level Agent Trace tables, indexes, and triggers, and intentionally has no `checkout_id` columns on trace tables. `RepositoryAgentTraceDb::verify_or_initialize_repository_metadata(repository_id) -> Result` inserts the singleton metadata row on first initialization, errors if an existing DB stores a different repository ID, and atomically claims `source_instance_id` for this physical database via `UPDATE ... WHERE source_instance_id = ''` (a losing racer's generated candidate is discarded and an already-valid stored value is never overwritten), returning the typed `RepositoryMetadata { repository_id, source_instance_id }`. `source_instance_id` is generated by application code (`generate_source_instance_id()`, UUID v4) and validated with `is_valid_source_instance_id()` (non-empty once trimmed); it is never derived from `repository_id`, remote URL, checkout ID, filesystem path, hostname, or user/workspace identity, and stays stable across reopen and repeated `sce setup` runs. `RepositoryAgentTraceDb::repair_missing_repository_schema_migration_metadata()` is a narrow concurrent-first-open repair seam: it never creates trace tables, but if every required repository schema table already exists and only the one-file baseline migration record is missing, it records `001_repository_schema` and rechecks readiness. `RepositoryAgentTraceDb` exposes repository-level write helpers for the current row families by delegating to the same typed insert payloads and parameterized SQL used by the checkout-scoped adapter: `insert_diff_trace`, `insert_post_commit_patch_intersection`, `insert_agent_trace`, `insert_messages`, `insert_parts`, and `insert_conversation_text_event`. It also exposes `recent_diff_trace_patches(cutoff_time_ms, end_time_ms)` by delegating to the shared recent diff-trace query/parser helper, so repository-scoped attribution reads use the same chronological inclusive window semantics without a checkout filter. These methods preserve the existing row shapes and do not add checkout provenance columns or checkout-scoped write/query APIs. @@ -89,6 +89,7 @@ SCE creates one Agent Trace DB per logical Git repository on demand through setu - `002_repository_source_instance_id.sql` (migration ID `002_repository_source_instance_id`) — additive `ALTER TABLE repository_metadata ADD COLUMN source_instance_id TEXT NOT NULL DEFAULT ''`; existing/placeholder rows default to an empty string and application code (never SQL) replaces it through the atomic claim in `verify_or_initialize_repository_metadata`. This migration does not rewrite `001_repository_schema.sql`. See [context/decisions/2026-08-10-agent-trace-source-instance-id.md](../decisions/2026-08-10-agent-trace-source-instance-id.md) for why `source_instance_id` exists as a second identity independent of `repository_id`. - `003_claude_model_state.sql` (migration ID `003_claude_model_state`) — additive `claude_model_state` table keyed by `(session_id, agent_id)`, storing `model_id`, constrained `observation_kind`, `source`, and non-negative `observed_at_ms`. The repository adapter's guarded upsert treats it as a best-effort latest-locally-observed register: older observations do not replace newer ones, equal-time `PostModelSwitch` wins over `SessionStart`, and same-kind ties use deterministic model/source ordering. This table is local attribution state outside the four Agent Trace export streams and has no sync cursor. Repositories created before this migration must rerun `sce setup` so the setup lifecycle applies migration `003_claude_model_state`; hook paths do not migrate. - `004_mutation_trace_protocol`, `005_mutation_scope_provenance` (insert-once `mutation_trace_scope_provenance`) and `006_mutation_trace_health_invariant` (rebuilds the worktree/event tables with the `tainted`/`failure_kind` `CHECK`) — mutation-trace tables; see [mutation-trace-store.md](../cli/mutation-trace-store.md) and [mutation-scope-provenance.md](../cli/mutation-scope-provenance.md). +- `007_commit_patch_id.sql` (migration ID `007_commit_patch_id`) — additive `ALTER TABLE agent_traces ADD COLUMN commit_patch_id TEXT`. Existing rows keep their IDs, URLs, and `trace_json` unchanged and read `commit_patch_id IS NULL`; there is no historical backfill. The column holds an optional Git patch-equivalence fingerprint for the original commit, never a unique commit identity. The former checkout-scoped `AGENT_TRACE_MIGRATIONS` constant and its 15-file `cli/migrations/agent-trace/` chain (`001_create_diff_traces` … `015_add_diff_traces_payload_type`) were removed by the `retire-legacy-agent-trace-db` plan; `build.rs` auto-discovers migration directories, so deleting the directory dropped the constant on regeneration. The repository schema captures the same tables/columns/indexes/triggers that the old incremental chain produced. @@ -96,6 +97,10 @@ The former checkout-scoped `AGENT_TRACE_MIGRATIONS` constant and its 15-file `cl Repositories created before `003_claude_model_state` must rerun `sce setup` to apply the additive migration. Hook runtimes, including `sce hooks claude-model-state`, never run migrations and instead surface the existing `Run 'sce setup'.` guidance until setup completes. +### Migration-007 upgrade note + +Repositories created before `007_commit_patch_id` must rerun `sce setup`, whose setup/lifecycle storage path falls back to `RepositoryAgentTraceDb::new_at` and applies the additive column; repeated setup is idempotent. Because hook readiness requires exact migration-ledger parity, an upgraded CLI fails open on every repository hook write and `sce doctor` reports `AgentTraceDbSchemaNotReady` until setup reruns; hooks never migrate and surface `Run 'sce setup'.` instead. Downgrade is not supported transparently: an older CLI binary sees `007_commit_patch_id` as an unexpected ledger entry and reports the schema as not ready. + The shared `TursoDb` runner records applied IDs in the database-local `__sce_migrations` table. Migration SQL is executed with `execute_batch`, so the one-file repository baseline can contain multiple statements while still recording one migration ID. Repository-scoped storage resolution first resolves `agent_trace.repository_id` / `agent_trace.repository_remote` through config, then splits by caller into two resolution paths in `agent_trace_storage`, both sharing the same identity setup and returning the same `ResolvedAgentTraceStorage { metadata: RepositoryMetadata, .. } `: @@ -105,82 +110,7 @@ Repository-scoped storage resolution first resolves `agent_trace.repository_id` Normal readiness for both paths is based on exact migration metadata parity with `AGENT_TRACE_REPOSITORY_MIGRATIONS`; table introspection is used only by the narrow concurrent-first-open metadata repair seam. -The `diff_traces` baseline migration creates: - -- `id INTEGER PRIMARY KEY` -- `time_ms INTEGER NOT NULL` -- `session_id TEXT NOT NULL` -- `patch TEXT NOT NULL` -- `created_at TEXT NOT NULL DEFAULT (...)` -- `model_id TEXT` -- `tool_name TEXT` -- `tool_version TEXT` - -Migration `015_add_diff_traces_payload_type` adds: - -- `payload_type TEXT NOT NULL DEFAULT 'patch'` — discriminator for source payload format; `patch` for `OpenCode` unified-diff payloads, `structured` for `Claude` `PostToolUse` structured payloads. - -The `post_commit_patch_intersections` baseline migration creates: - -- `id INTEGER PRIMARY KEY` -- `commit_id TEXT NOT NULL` -- `post_commit_time_ms INTEGER NOT NULL` -- `recent_window_cutoff_ms INTEGER NOT NULL` -- `recent_window_end_ms INTEGER NOT NULL` -- `loaded_diff_trace_count INTEGER NOT NULL CHECK (loaded_diff_trace_count >= 0)` -- `skipped_diff_trace_count INTEGER NOT NULL CHECK (skipped_diff_trace_count >= 0)` -- `intersection_patch TEXT NOT NULL` -- `created_at TEXT NOT NULL DEFAULT (...)` - -The `agent_traces` baseline migration creates: - -- `id INTEGER PRIMARY KEY` -- `commit_id TEXT NOT NULL` -- `commit_time_ms INTEGER NOT NULL` -- `url TEXT NOT NULL` -- `remote_url TEXT` (nullable) -- `trace_json TEXT NOT NULL` -- `agent_trace_id TEXT NOT NULL UNIQUE` -- `created_at TEXT NOT NULL DEFAULT (...)` - -The `messages` migration creates: - -- `id INTEGER PRIMARY KEY` -- `session_id TEXT NOT NULL` -- `message_id TEXT NOT NULL` -- `role TEXT NOT NULL CHECK (role IN ('user', 'assistant'))` -- `generated_at_unix_ms INTEGER NOT NULL CHECK (generated_at_unix_ms >= 0)` -- `created_at TEXT NOT NULL DEFAULT (...)` -- `updated_at TEXT NOT NULL DEFAULT (...)` - -The `parts` migration creates: - -- `id INTEGER PRIMARY KEY` -- `type TEXT NOT NULL` -- `text TEXT NOT NULL` -- `message_id TEXT NOT NULL` -- `session_id TEXT NOT NULL` -- `generated_at_unix_ms INTEGER NOT NULL CHECK (generated_at_unix_ms >= 0)` -- `created_at TEXT NOT NULL DEFAULT (...)` -- `updated_at TEXT NOT NULL DEFAULT (...)` - -No foreign keys exist between `messages` and `parts`; rows may be written out of order. The data model uses natural identifiers (`session_id`, `message_id`) for joins rather than DB-level referential integrity. - -Lookup indexes created by the baseline migration set: - -- `idx_diff_traces_time_ms_id` on `(time_ms, id)` -- `idx_agent_traces_agent_trace_id` on `(agent_trace_id)` -- `idx_agent_traces_remote_url` on `(remote_url)` -- `idx_messages_session_message` unique index on `(session_id, message_id)` — enables duplicate-ignore parent message inserts by natural key -- `idx_messages_session_order` on `(session_id, generated_at_unix_ms, id)` — enables chronological session-scoped message retrieval -- `idx_parts_session_message_order` on `(session_id, message_id, generated_at_unix_ms, id)` — enables ordered part joins per message - -`updated_at` triggers defined by the migration set: - -- `trg_messages_updated_at`: fires on `UPDATE` for non-`updated_at` column changes on `messages` -- `trg_parts_updated_at`: fires on `UPDATE` for non-`updated_at` column changes on `parts` - -Both triggers compare `OLD.*` vs `NEW.*` for all mutable columns (excluding `updated_at` itself) and refresh the timestamp only when a real change occurred. +Per-table columns, lookup indexes, and `updated_at` triggers produced by the migration set are listed in [agent-trace-db-schema.md](agent-trace-db-schema.md). ## Lifecycle integration @@ -188,7 +118,7 @@ Both triggers compare `OLD.*` vs `NEW.*` for all mutable columns (excluding `upd - `diagnose()` resolves repository identity from config or the configured Git remote and reports the repository-scoped Agent Trace DB path and parent-directory readiness. When the DB file exists, it opens the file via `RepositoryAgentTraceDb::open_without_migrations_at` and verifies schema readiness via `ensure_schema_ready_for_hooks`, reporting `AgentTraceDbConnectionFailed` if open fails or `AgentTraceDbSchemaNotReady` if the schema is incomplete. Missing repository identity is a manual problem with `.sce/config.json` / remote guidance. Outside repository context there is no repository identity to select a DB and no global/checkout fallback path; the lifecycle returns an actionable "requires a Git repository" diagnostic, surfaced by `diagnose_agent_trace_db_health` as a manual-only `UnableToResolveStateRoot` problem. - `fix()` bootstraps the resolved repository DB parent directory for auto-fixable parent-readiness problems, with the same global parent fallback outside repository context. -- `setup()` resolves repository storage through `agent_trace_storage`, opens/creates `/sce/repos//agent-trace.db` with the repository schema, validates `repository_metadata.repository_id`, and emits setup messaging with the repository ID, `source_instance_id` (`RepositoryMetadata` from the same resolution call), and initialized DB path. Hook runtime lazy initialization remains available for repositories where setup has not run or schema metadata is incomplete. +- `setup()` resolves repository storage through `agent_trace_storage`, opens/creates `/sce/repos//agent-trace.db` with the repository schema, validates `repository_metadata.repository_id`, and emits setup messaging with the repository ID, `source_instance_id` (`RepositoryMetadata` from the same resolution call), and initialized DB path. Hook runtime never initializes or migrates the schema: a missing database or incomplete migration ledger surfaces `Run 'sce setup'.` guidance, and only the narrow concurrent-first-open baseline-metadata repair is applied. - `sce doctor` surfaces lifecycle-owned repository Agent Trace DB health in the `Configuration` section, with `[PASS]`/`[FAIL]`/`[MISS]` status tokens. Outside repository context the lifecycle reports the actionable "requires a Git repository" diagnostic instead of probing a sentinel path. JSON output includes the resolved `agent_trace_db` field; checkout identity is no longer reported (see `context/cli/checkout-identity.md`). - `sce sync` reads `messages`, `parts`, and `agent_traces` rows through the read-only export boundary before sending them to the control plane; `diff_traces` rows are not read or uploaded. See [context/cli/sync-command.md](../cli/sync-command.md). From 82c25e36feae2d63760e5da2bf37dc5c3708eb9a Mon Sep 17 00:00:00 2001 From: David Abram Date: Thu, 15 Oct 2026 11:03:58 +0200 Subject: [PATCH 2/5] hooks: Compute canonical commit patch IDs Agent Trace commit records need a stable patch-equivalence fingerprint independent of commit messages, object IDs, and user Git diff configuration. Add a bounded async Git subprocess pipeline for canonical patch-ID computation, including root, binary, rename-only, empty, merge, SHA-1, and SHA-256 handling with strict output validation and child cleanup. Document the pinned invocation and cover the helper with temporary-repository tests; leave runtime wiring for the next plan task. Plan: agent-trace-commit-patch-id (T02) Co-authored-by: SCE --- cli/src/services/hooks/commit_patch_id.rs | 791 ++++++++++++++++++ cli/src/services/hooks/mod.rs | 2 + context/plans/agent-trace-commit-patch-id.md | 10 +- .../sce/agent-trace-hooks-command-routing.md | 11 + 4 files changed, 812 insertions(+), 2 deletions(-) create mode 100644 cli/src/services/hooks/commit_patch_id.rs diff --git a/cli/src/services/hooks/commit_patch_id.rs b/cli/src/services/hooks/commit_patch_id.rs new file mode 100644 index 000000000..10e18fc3a --- /dev/null +++ b/cli/src/services/hooks/commit_patch_id.rs @@ -0,0 +1,791 @@ +use std::ffi::OsStr; +use std::path::Path; +use std::process::{ExitStatus, Stdio}; +use std::time::Duration; + +use anyhow::{anyhow, bail, Context, Result}; +use tokio::io::{AsyncRead, AsyncReadExt, AsyncWriteExt}; +use tokio::process::{Child, ChildStdin, ChildStdout, Command}; + +const GIT_PROGRAM: &str = "git"; +const COMMIT_PATCH_ID_DEADLINE: Duration = Duration::from_secs(10); +const MAX_PATCH_INPUT_BYTES: u64 = 64 * 1024 * 1024; +const MAX_PATCH_ID_OUTPUT_BYTES: usize = 4 * 1024; +const MAX_REV_LIST_OUTPUT_BYTES: usize = 64 * 1024; +const MAX_RETAINED_STDERR_BYTES: u64 = 4 * 1024; +const STREAM_CHUNK_BYTES: usize = 64 * 1024; +const SHA1_HEX_LEN: usize = 40; +const SHA256_HEX_LEN: usize = 64; + +const SHOW_CONFIG_OVERRIDES: &[&str] = &[ + "core.quotePath=true", + "diff.noprefix=false", + "diff.mnemonicPrefix=false", + "diff.relative=false", + "diff.context=3", + "diff.interHunkContext=0", + "diff.indentHeuristic=true", + "diff.algorithm=myers", + "diff.renames=false", + "diff.suppressBlankEmpty=false", + "diff.submodule=short", + "diff.ignoreSubmodules=none", + "color.ui=never", + "color.diff=never", + "log.showRoot=true", + "log.showSignature=false", +]; + +const SHOW_ARGS: &[&str] = &[ + "show", + "--format=", + "--patch", + "--binary", + "--no-ext-diff", + "--no-textconv", + "--no-renames", + "--no-color", + "--diff-algorithm=myers", + "--unified=3", + "--inter-hunk-context=0", + "--indent-heuristic", + "--src-prefix=a/", + "--dst-prefix=b/", + "--no-relative", + "-O/dev/null", + "--root", + "--no-show-signature", + "--submodule=short", + "--ignore-submodules=none", +]; + +const PATCH_ID_CONFIG_OVERRIDES: &[&str] = &["patchid.stable=true", "patchid.verbatim=false"]; + +const PATCH_ID_ARGS: &[&str] = &["patch-id", "--stable"]; + +const REMOVED_GIT_ENV: &[&str] = &["GIT_EXTERNAL_DIFF", "GIT_DIFF_OPTS"]; + +#[derive(Clone, Copy, Debug)] +struct CommitPatchIdRunner<'a> { + program: &'a OsStr, + leading_args: &'a [&'a OsStr], + deadline: Duration, + max_patch_input_bytes: u64, + max_patch_id_output_bytes: usize, +} + +impl Default for CommitPatchIdRunner<'_> { + fn default() -> Self { + CommitPatchIdRunner { + program: OsStr::new(GIT_PROGRAM), + leading_args: &[], + deadline: COMMIT_PATCH_ID_DEADLINE, + max_patch_input_bytes: MAX_PATCH_INPUT_BYTES, + max_patch_id_output_bytes: MAX_PATCH_ID_OUTPUT_BYTES, + } + } +} + +impl CommitPatchIdRunner<'_> { + fn git_command( + &self, + repository_root: &Path, + config_overrides: &[&str], + args: &[&str], + ) -> Command { + let mut command = Command::new(self.program); + command.args(self.leading_args); + for config_override in config_overrides { + command.arg("-c").arg(config_override); + } + command + .args(args) + .current_dir(repository_root) + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .kill_on_drop(true); + for name in REMOVED_GIT_ENV { + command.env_remove(name); + } + command + } +} + +pub(crate) async fn compute_commit_patch_id( + repository_root: &Path, + commit_oid: &str, +) -> Result> { + compute_commit_patch_id_with(&CommitPatchIdRunner::default(), repository_root, commit_oid).await +} + +async fn compute_commit_patch_id_with( + runner: &CommitPatchIdRunner<'_>, + repository_root: &Path, + commit_oid: &str, +) -> Result> { + if !is_lowercase_object_id(commit_oid) { + bail!("commit patch ID requires a 40- or 64-character lowercase hex commit object ID"); + } + + tokio::time::timeout( + runner.deadline, + compute_commit_patch_id_unbounded(runner, repository_root, commit_oid), + ) + .await + .map_err(|_| { + anyhow!( + "commit patch ID computation exceeded the {}ms deadline", + runner.deadline.as_millis() + ) + })? +} + +async fn compute_commit_patch_id_unbounded( + runner: &CommitPatchIdRunner<'_>, + repository_root: &Path, + commit_oid: &str, +) -> Result> { + if commit_parent_count(runner, repository_root, commit_oid).await? > 1 { + return Ok(None); + } + + let output = run_patch_id_pipeline(runner, repository_root, commit_oid).await?; + parse_patch_id_output(&output, commit_oid.len()) +} + +async fn commit_parent_count( + runner: &CommitPatchIdRunner<'_>, + repository_root: &Path, + commit_oid: &str, +) -> Result { + let mut rev_list = runner + .git_command( + repository_root, + &[], + &["rev-list", "--parents", "-n", "1", commit_oid, "--"], + ) + .spawn() + .context("Failed to spawn git rev-list for commit patch ID")?; + let stdout = rev_list + .stdout + .take() + .context("git rev-list stdout was not captured")?; + let stderr = rev_list + .stderr + .take() + .context("git rev-list stderr was not captured")?; + + let (stdout, (), status) = tokio::try_join!( + read_bounded(stdout, MAX_REV_LIST_OUTPUT_BYTES, "git rev-list output"), + drain_stderr(stderr), + wait_for_exit(&mut rev_list, "git rev-list"), + )?; + ensure_success(status, "git rev-list")?; + + let stdout = String::from_utf8(stdout).context("git rev-list emitted invalid UTF-8")?; + let mut lines = stdout.lines(); + let (Some(line), None) = (lines.next(), lines.next()) else { + bail!("git rev-list emitted malformed parent output"); + }; + let mut tokens = line.split(' '); + if tokens.next() != Some(commit_oid) { + bail!("git rev-list emitted malformed parent output"); + } + let parents: Vec<&str> = tokens.collect(); + if parents.iter().any(|parent| !is_lowercase_object_id(parent)) { + bail!("git rev-list emitted malformed parent output"); + } + Ok(parents.len()) +} + +async fn run_patch_id_pipeline( + runner: &CommitPatchIdRunner<'_>, + repository_root: &Path, + commit_oid: &str, +) -> Result> { + let mut show_args = SHOW_ARGS.to_vec(); + show_args.extend([commit_oid, "--"]); + let mut show = runner + .git_command(repository_root, SHOW_CONFIG_OVERRIDES, &show_args) + .spawn() + .context("Failed to spawn git show for commit patch ID")?; + let mut patch_id = runner + .git_command(repository_root, PATCH_ID_CONFIG_OVERRIDES, PATCH_ID_ARGS) + .stdin(Stdio::piped()) + .spawn() + .context("Failed to spawn git patch-id for commit patch ID")?; + + let show_stdout = show + .stdout + .take() + .context("git show stdout was not captured")?; + let show_stderr = show + .stderr + .take() + .context("git show stderr was not captured")?; + let patch_id_stdin = patch_id + .stdin + .take() + .context("git patch-id stdin was not captured")?; + let patch_id_stdout = patch_id + .stdout + .take() + .context("git patch-id stdout was not captured")?; + let patch_id_stderr = patch_id + .stderr + .take() + .context("git patch-id stderr was not captured")?; + + let ((), output, (), (), show_status, patch_id_status) = tokio::try_join!( + stream_patch(show_stdout, patch_id_stdin, runner.max_patch_input_bytes), + read_bounded( + patch_id_stdout, + runner.max_patch_id_output_bytes, + "git patch-id output" + ), + drain_stderr(show_stderr), + drain_stderr(patch_id_stderr), + wait_for_exit(&mut show, "git show"), + wait_for_exit(&mut patch_id, "git patch-id"), + )?; + ensure_success(show_status, "git show")?; + ensure_success(patch_id_status, "git patch-id")?; + Ok(output) +} + +async fn stream_patch( + mut source: ChildStdout, + mut sink: ChildStdin, + max_patch_input_bytes: u64, +) -> Result<()> { + let mut buffer = vec![0_u8; STREAM_CHUNK_BYTES]; + let mut total_bytes: u64 = 0; + loop { + let read = source + .read(&mut buffer) + .await + .context("Failed to read git show output")?; + if read == 0 { + break; + } + total_bytes += read as u64; + if total_bytes > max_patch_input_bytes { + bail!("git show patch exceeded the {max_patch_input_bytes}-byte limit"); + } + sink.write_all(&buffer[..read]) + .await + .context("Failed to stream patch into git patch-id")?; + } + sink.shutdown() + .await + .context("Failed to close git patch-id input")?; + Ok(()) +} + +async fn read_bounded( + reader: R, + max_bytes: usize, + label: &str, +) -> Result> { + let mut bytes = Vec::new(); + reader + .take(max_bytes as u64 + 1) + .read_to_end(&mut bytes) + .await + .with_context(|| format!("Failed to read {label}"))?; + if bytes.len() > max_bytes { + bail!("{label} exceeded the {max_bytes}-byte limit"); + } + Ok(bytes) +} + +async fn drain_stderr(mut reader: R) -> Result<()> { + let mut retained = Vec::new(); + (&mut reader) + .take(MAX_RETAINED_STDERR_BYTES) + .read_to_end(&mut retained) + .await + .context("Failed to read git stderr")?; + tokio::io::copy(&mut reader, &mut tokio::io::sink()) + .await + .context("Failed to drain git stderr")?; + Ok(()) +} + +async fn wait_for_exit(child: &mut Child, label: &str) -> Result { + child + .wait() + .await + .with_context(|| format!("Failed to wait for {label}")) +} + +fn ensure_success(status: ExitStatus, label: &str) -> Result<()> { + if status.success() { + return Ok(()); + } + match status.code() { + Some(code) => bail!("{label} exited with status {code}"), + None => bail!("{label} was terminated by a signal"), + } +} + +fn parse_patch_id_output(output: &[u8], object_id_len: usize) -> Result> { + let output = std::str::from_utf8(output).context("git patch-id emitted invalid UTF-8")?; + if output.is_empty() { + return Ok(None); + } + let Some(line) = output.strip_suffix('\n') else { + bail!("git patch-id emitted malformed output"); + }; + let mut tokens = line.split(' '); + let (Some(patch_id), Some(commit_id), None) = (tokens.next(), tokens.next(), tokens.next()) + else { + bail!("git patch-id emitted malformed output"); + }; + if patch_id.len() != object_id_len + || commit_id.len() != object_id_len + || !is_lowercase_object_id(patch_id) + || !is_lowercase_object_id(commit_id) + { + bail!("git patch-id emitted malformed output"); + } + Ok(Some(patch_id.to_string())) +} + +fn is_lowercase_object_id(value: &str) -> bool { + matches!(value.len(), SHA1_HEX_LEN | SHA256_HEX_LEN) + && value + .bytes() + .all(|byte| byte.is_ascii_digit() || (b'a'..=b'f').contains(&byte)) +} + +#[cfg(test)] +mod tests { + use std::ffi::OsStr; + use std::path::{Path, PathBuf}; + use std::process::{Command, Stdio}; + use std::time::Duration; + + use super::{ + compute_commit_patch_id, compute_commit_patch_id_with, parse_patch_id_output, + CommitPatchIdRunner, + }; + + const FAKE_GIT_DEADLINE: Duration = Duration::from_millis(500); + const CHILD_EXIT_TIMEOUT: Duration = Duration::from_secs(10); + const CHILD_EXIT_POLL_INTERVAL: Duration = Duration::from_millis(20); + const FAKE_COMMIT_OID: &str = "1111111111111111111111111111111111111111"; + const FAKE_PARENT_OID: &str = "2222222222222222222222222222222222222222"; + + fn isolated_git(cwd: &Path, args: &[&str]) -> std::process::Output { + Command::new("git") + .args(args) + .current_dir(cwd) + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_NOSYSTEM", "1") + .output() + .expect("git command should run") + } + + fn git(cwd: &Path, args: &[&str]) -> String { + let output = isolated_git(cwd, args); + assert!( + output.status.success(), + "git {args:?} failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8(output.stdout) + .expect("git output is UTF-8") + .trim() + .to_string() + } + + fn init_repository(root: &Path, object_format: &str) { + std::fs::create_dir_all(root).expect("repo dir"); + git( + root, + &[ + "init", + "-q", + "-b", + "main", + &format!("--object-format={object_format}"), + ], + ); + git(root, &["config", "user.email", "patch-id@example.invalid"]); + git(root, &["config", "user.name", "Patch Id Test"]); + git(root, &["config", "commit.gpgsign", "false"]); + } + + fn write(root: &Path, path: &str, contents: &[u8]) { + let path = root.join(path); + std::fs::create_dir_all(path.parent().expect("parent")).expect("create parent"); + std::fs::write(path, contents).expect("write file"); + } + + fn commit_all(root: &Path, message: &str) -> String { + git(root, &["add", "-A"]); + git(root, &["commit", "-q", "--allow-empty", "-m", message]); + git(root, &["rev-parse", "HEAD"]) + } + + fn reference_patch_id(root: &Path, oid: &str) -> String { + let show = isolated_git(root, &["show", "--binary", "--no-renames", oid]); + assert!(show.status.success(), "reference git show failed"); + let mut patch_id = Command::new("git") + .args(["patch-id", "--stable"]) + .current_dir(root) + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_NOSYSTEM", "1") + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .spawn() + .expect("spawn reference git patch-id"); + std::io::Write::write_all( + &mut patch_id.stdin.take().expect("patch-id stdin"), + &show.stdout, + ) + .expect("write reference patch"); + let output = patch_id.wait_with_output().expect("reference patch-id"); + String::from_utf8(output.stdout) + .expect("UTF-8") + .split(' ') + .next() + .expect("patch id column") + .to_string() + } + + async fn patch_id(root: &Path, oid: &str) -> Option { + compute_commit_patch_id(root, oid) + .await + .expect("commit patch ID computation should succeed") + } + + fn multi_hunk_contents(edited: bool) -> String { + let mut lines: Vec = (0..40).map(|index| format!("line {index}")).collect(); + lines.insert(5, String::new()); + if edited { + lines[2] = "changed early".to_string(); + lines[9] = "changed nearby".to_string(); + lines[30] = "changed late".to_string(); + lines.insert(20, "{\n inserted();\n}".to_string()); + } + lines.join("\n") + "\n" + } + + fn seed_history(root: &Path) -> (String, String) { + write(root, "base.txt", b"base\n"); + write( + root, + "src/m\u{e4}in.txt", + multi_hunk_contents(false).as_bytes(), + ); + let root_oid = commit_all(root, "root"); + write( + root, + "src/m\u{e4}in.txt", + multi_hunk_contents(true).as_bytes(), + ); + write(root, "notes.txt", b"lowercase notes\n"); + let change_oid = commit_all(root, "change"); + (root_oid, change_oid) + } + + fn apply_hostile_diff_config(root: &Path) { + for (key, value) in [ + ("diff.noprefix", "true"), + ("diff.mnemonicPrefix", "true"), + ("core.quotePath", "false"), + ("diff.context", "10"), + ("diff.interHunkContext", "10"), + ("diff.algorithm", "histogram"), + ("diff.indentHeuristic", "false"), + ("diff.relative", "true"), + ("diff.orderFile", "/nonexistent/sce-order-file"), + ("diff.renames", "copies"), + ("diff.suppressBlankEmpty", "true"), + ("diff.external", "/bin/false"), + ("diff.upper.textconv", "tr a-z A-Z"), + ("color.ui", "always"), + ("color.diff", "always"), + ("log.showRoot", "false"), + ("patchid.verbatim", "true"), + ] { + git(root, &["config", key, value]); + } + write(root, ".git/info/attributes", b"*.txt diff=upper\n"); + } + + #[tokio::test] + async fn commit_patch_id_matches_canonical_invocation_for_root_and_normal_commits() { + let dir = tempfile::tempdir().expect("temp dir"); + let root = dir.path(); + init_repository(root, "sha1"); + let (root_oid, change_oid) = seed_history(root); + + for oid in [&root_oid, &change_oid] { + let computed = patch_id(root, oid).await.expect("non-empty commit"); + assert_eq!(computed, reference_patch_id(root, oid)); + assert_eq!(computed.len(), 40); + } + } + + #[tokio::test] + async fn commit_patch_id_is_stable_across_message_amend_and_equivalent_cherry_pick() { + let dir = tempfile::tempdir().expect("temp dir"); + let root = dir.path(); + init_repository(root, "sha1"); + let (root_oid, change_oid) = seed_history(root); + let original = patch_id(root, &change_oid).await.expect("patch id"); + + git(root, &["commit", "-q", "--amend", "-m", "reworded message"]); + let amended_oid = git(root, &["rev-parse", "HEAD"]); + assert_ne!(amended_oid, change_oid); + assert_eq!(patch_id(root, &amended_oid).await, Some(original.clone())); + + git(root, &["checkout", "-q", "-b", "other", &root_oid]); + write(root, "unrelated.txt", b"other parent\n"); + commit_all(root, "different parent"); + git(root, &["cherry-pick", &change_oid]); + let picked_oid = git(root, &["rev-parse", "HEAD"]); + assert_ne!(picked_oid, change_oid); + assert_eq!(patch_id(root, &picked_oid).await, Some(original)); + } + + #[tokio::test] + async fn commit_patch_id_ignores_user_diff_configuration() { + let dir = tempfile::tempdir().expect("temp dir"); + let root = dir.path(); + init_repository(root, "sha1"); + let (root_oid, change_oid) = seed_history(root); + let clean_root = patch_id(root, &root_oid).await; + let clean_change = patch_id(root, &change_oid).await; + + apply_hostile_diff_config(root); + + assert_eq!(patch_id(root, &root_oid).await, clean_root); + assert_eq!(patch_id(root, &change_oid).await, clean_change); + assert!(clean_root.is_some() && clean_change.is_some()); + } + + #[tokio::test] + async fn commit_patch_id_is_deterministic_for_binary_and_rename_only_commits() { + let dir = tempfile::tempdir().expect("temp dir"); + let root = dir.path(); + init_repository(root, "sha1"); + write(root, "image.bin", &[0, 159, 146, 150, 0, 1, 2]); + write(root, "docs/old-name.md", b"same contents\n"); + commit_all(root, "seed"); + write(root, "image.bin", &[0, 255, 254, 253, 0, 9]); + let binary_oid = commit_all(root, "binary only"); + git(root, &["mv", "docs/old-name.md", "docs/new-name.md"]); + let rename_oid = commit_all(root, "rename only"); + + for oid in [&binary_oid, &rename_oid] { + let first = patch_id(root, oid).await.expect("patch id"); + assert_eq!(patch_id(root, oid).await, Some(first.clone())); + assert_eq!(first, reference_patch_id(root, oid)); + } + assert_ne!( + patch_id(root, &binary_oid).await, + patch_id(root, &rename_oid).await + ); + } + + #[tokio::test] + async fn commit_patch_id_is_none_for_empty_and_merge_commits() { + let dir = tempfile::tempdir().expect("temp dir"); + let root = dir.path(); + init_repository(root, "sha1"); + write(root, "base.txt", b"base\n"); + let base_oid = commit_all(root, "base"); + let empty_oid = commit_all(root, "empty"); + assert_eq!(patch_id(root, &empty_oid).await, None); + + git(root, &["checkout", "-q", "-b", "side", &base_oid]); + write(root, "side.txt", b"side\n"); + commit_all(root, "side"); + git(root, &["checkout", "-q", "main"]); + write(root, "main.txt", b"main\n"); + commit_all(root, "main"); + git(root, &["merge", "-q", "--no-ff", "-m", "merge", "side"]); + let merge_oid = git(root, &["rev-parse", "HEAD"]); + assert_eq!(patch_id(root, &merge_oid).await, None); + } + + #[tokio::test] + async fn commit_patch_id_supports_sha256_repositories() { + let dir = tempfile::tempdir().expect("temp dir"); + let root = dir.path(); + init_repository(root, "sha256"); + let (_, change_oid) = seed_history(root); + assert_eq!(change_oid.len(), 64); + + let computed = patch_id(root, &change_oid).await.expect("patch id"); + assert_eq!(computed.len(), 64); + assert_eq!(computed, reference_patch_id(root, &change_oid)); + } + + #[tokio::test] + async fn commit_patch_id_rejects_invalid_object_ids_and_unknown_commits() { + let dir = tempfile::tempdir().expect("temp dir"); + let root = dir.path(); + init_repository(root, "sha1"); + let (_, change_oid) = seed_history(root); + + for invalid in [ + change_oid.to_uppercase(), + change_oid[..39].to_string(), + "--output=/tmp/x".to_string(), + FAKE_COMMIT_OID.to_string(), + ] { + assert!(compute_commit_patch_id(root, &invalid).await.is_err()); + } + } + + #[tokio::test] + async fn commit_patch_id_enforces_patch_input_and_output_byte_limits() { + let dir = tempfile::tempdir().expect("temp dir"); + let root = dir.path(); + init_repository(root, "sha1"); + let (_, change_oid) = seed_history(root); + + let input_limited = CommitPatchIdRunner { + max_patch_input_bytes: 16, + ..CommitPatchIdRunner::default() + }; + let error = compute_commit_patch_id_with(&input_limited, root, &change_oid) + .await + .expect_err("patch input limit"); + assert!(error.to_string().contains("16-byte limit"), "{error:#}"); + + let output_limited = CommitPatchIdRunner { + max_patch_id_output_bytes: 8, + ..CommitPatchIdRunner::default() + }; + let error = compute_commit_patch_id_with(&output_limited, root, &change_oid) + .await + .expect_err("patch-id output limit"); + assert!(error.to_string().contains("8-byte limit"), "{error:#}"); + } + + #[test] + fn commit_patch_id_parser_rejects_malformed_output() { + let valid = format!("{FAKE_PARENT_OID} {}\n", "0".repeat(40)); + assert_eq!( + parse_patch_id_output(valid.as_bytes(), 40).expect("valid"), + Some(FAKE_PARENT_OID.to_string()) + ); + assert_eq!(parse_patch_id_output(b"", 40).expect("empty"), None); + for malformed in [ + format!("{FAKE_PARENT_OID} {}", "0".repeat(40)), + format!("{} {}\n", "A".repeat(40), "0".repeat(40)), + format!("{FAKE_PARENT_OID}\n"), + format!("{FAKE_PARENT_OID} {}\n", "0".repeat(64)), + format!( + "{FAKE_PARENT_OID} {0}\n{FAKE_PARENT_OID} {0}\n", + "0".repeat(40) + ), + ] { + assert!(parse_patch_id_output(malformed.as_bytes(), 40).is_err()); + } + assert!(parse_patch_id_output(&[0xff, b'\n'], 40).is_err()); + } + + struct FakeGit { + _dir: tempfile::TempDir, + script: PathBuf, + show_pid: PathBuf, + patch_id_pid: PathBuf, + } + + fn fake_git(show_body: &str, patch_id_body: &str) -> FakeGit { + let dir = tempfile::tempdir().expect("temp dir"); + let script = dir.path().join("fake-git.sh"); + let show_pid = dir.path().join("show.pid"); + let patch_id_pid = dir.path().join("patch-id.pid"); + let contents = format!( + "while [ \"$1\" = \"-c\" ]; do shift 2; done\n\ + case \"$1\" in\n\ + rev-list) echo \"{FAKE_COMMIT_OID} {FAKE_PARENT_OID}\" ;;\n\ + show) echo $$ > '{show_pid}'; {show_body} ;;\n\ + patch-id) echo $$ > '{patch_id_pid}'; {patch_id_body} ;;\n\ + esac\n", + show_pid = show_pid.display(), + patch_id_pid = patch_id_pid.display(), + ); + std::fs::write(&script, contents).expect("write fake git"); + FakeGit { + _dir: dir, + script, + show_pid, + patch_id_pid, + } + } + + async fn run_fake_git(fake: &FakeGit) -> anyhow::Result> { + let leading_args = [fake.script.as_os_str()]; + let runner = CommitPatchIdRunner { + program: OsStr::new("/bin/sh"), + leading_args: &leading_args, + deadline: FAKE_GIT_DEADLINE, + ..CommitPatchIdRunner::default() + }; + compute_commit_patch_id_with(&runner, Path::new("."), FAKE_COMMIT_OID).await + } + + async fn assert_process_exited(pid_file: &Path) { + let pid = std::fs::read_to_string(pid_file).expect("pid file"); + let stat = PathBuf::from(format!("/proc/{}/stat", pid.trim())); + let deadline = tokio::time::Instant::now() + CHILD_EXIT_TIMEOUT; + loop { + let running = std::fs::read_to_string(&stat) + .ok() + .and_then(|stat| { + stat.rsplit_once(')') + .map(|(_, rest)| rest.trim_start().to_string()) + }) + .is_some_and(|state| !state.starts_with('Z') && !state.starts_with('X')); + if !running { + return; + } + assert!( + tokio::time::Instant::now() < deadline, + "child {} is still running", + pid.trim() + ); + tokio::time::sleep(CHILD_EXIT_POLL_INTERVAL).await; + } + } + + #[tokio::test] + async fn commit_patch_id_reports_non_zero_git_exit() { + let fake = fake_git("exit 3", "cat >/dev/null"); + let error = run_fake_git(&fake).await.expect_err("non-zero show"); + assert!( + error.to_string().contains("git show exited with status 3"), + "{error:#}" + ); + } + + #[tokio::test] + async fn commit_patch_id_reports_malformed_patch_id_output() { + let fake = fake_git("echo patch", "cat >/dev/null; echo not-a-patch-id"); + let error = run_fake_git(&fake).await.expect_err("malformed output"); + assert!(error.to_string().contains("malformed"), "{error:#}"); + } + + #[cfg(target_os = "linux")] + #[tokio::test] + async fn commit_patch_id_times_out_and_kills_both_git_processes() { + let fake = fake_git("exec sleep 30", "exec cat"); + let started = std::time::Instant::now(); + let error = run_fake_git(&fake).await.expect_err("timeout"); + assert!(error.to_string().contains("deadline"), "{error:#}"); + assert!(started.elapsed() < CHILD_EXIT_TIMEOUT); + + assert_process_exited(&fake.show_pid).await; + assert_process_exited(&fake.patch_id_pid).await; + } +} diff --git a/cli/src/services/hooks/mod.rs b/cli/src/services/hooks/mod.rs index 14b114bd2..960d60652 100644 --- a/cli/src/services/hooks/mod.rs +++ b/cli/src/services/hooks/mod.rs @@ -2,6 +2,8 @@ use anyhow::Context; mod claude_transforms; mod commit_hooks; +#[cfg_attr(not(test), allow(dead_code))] +mod commit_patch_id; mod conversation_trace; mod diff_trace; diff --git a/context/plans/agent-trace-commit-patch-id.md b/context/plans/agent-trace-commit-patch-id.md index f46164e9c..daec46d8f 100644 --- a/context/plans/agent-trace-commit-patch-id.md +++ b/context/plans/agent-trace-commit-patch-id.md @@ -81,13 +81,19 @@ How this plan is proven complete. Each criterion is observable and names the che - Deviation: setup-path parity is tested through the private storage open functions rather than an end-to-end `sce setup` invocation. - Context synchronization: synced -- [ ] T02: `Compute the canonical Git patch ID with bounded subprocesses` (status:todo) +- [x] T02: `Compute the canonical Git patch ID with bounded subprocesses` (status:done) - Task ID: T02 - Scope: In — a small `hooks/` Git patch-ID helper module, the pinned `git show` options and config overrides, parent-count check for merges, streaming `git show` → `git patch-id --stable` with deadline, byte bounds, exit/output validation and `kill_on_drop` cleanup, the output parser, and temporary-Git-repository tests. Out — hook orchestration, persistence, sync. - Dependencies: none - Done when: The async helper takes a repository root and captured commit OID and returns `Result>`; normal, root, binary-only, and rename-only commits match the canonical invocation and are stable across message amend, equivalent cherry-pick, and user diff-configuration variations; empty and multi-parent merge commits return `Ok(None)`; non-zero exit, malformed output, oversized output, and timeout return bounded errors with no surviving child process. - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id` (helper cases). - - Context synchronization: pending + - Completed: 2026-10-15 + - Files changed: `cli/src/services/hooks/commit_patch_id.rs` (new), `cli/src/services/hooks/mod.rs`, `context/plans/agent-trace-commit-patch-id.md` + - Result: Added `compute_commit_patch_id(repository_root, commit_oid) -> Result>` (crate-private; `mod commit_patch_id` carries `#[cfg_attr(not(test), allow(dead_code))]` until T03 wires it). It validates the OID as 40/64 lowercase hex, counts parents via `git rev-list --parents -n 1 --` (more than one parent → `None`), then spawns `git -c core.quotePath=true -c diff.noprefix=false -c diff.mnemonicPrefix=false -c diff.relative=false -c diff.context=3 -c diff.interHunkContext=0 -c diff.indentHeuristic=true -c diff.algorithm=myers -c diff.renames=false -c diff.suppressBlankEmpty=false -c diff.submodule=short -c diff.ignoreSubmodules=none -c color.ui=never -c color.diff=never -c log.showRoot=true -c log.showSignature=false show --format= --patch --binary --no-ext-diff --no-textconv --no-renames --no-color --diff-algorithm=myers --unified=3 --inter-hunk-context=0 --indent-heuristic --src-prefix=a/ --dst-prefix=b/ --no-relative -O/dev/null --root --no-show-signature --submodule=short --ignore-submodules=none --` and streams its stdout into `git -c patchid.stable=true -c patchid.verbatim=false patch-id --stable` (with `GIT_EXTERNAL_DIFF`/`GIT_DIFF_OPTS` removed). All subprocesses are `tokio::process::Command` with `kill_on_drop(true)` under one 10 s `tokio::time::timeout`; streamed patch input is capped at 64 MiB, patch-id output at 4 KiB, rev-list output at 64 KiB, stderr is drained without retention beyond 4 KiB, and both exit statuses are checked. Empty patch-id output → `None`; otherwise exactly one ` \n` line of lowercase hex matching the OID length is required. Errors carry only stage/status/limit text, never diff content or stderr. A private `CommitPatchIdRunner` seam injects program, leading args, deadline, and limits for tests. Twelve new `commit_patch_id` tests cover canonical-invocation parity for root/normal commits, amend and cherry-pick stability, hostile user config (noprefix, mnemonicPrefix, quotePath, context, interHunkContext, histogram, indentHeuristic, relative, missing orderFile, renames=copies, suppressBlankEmpty, external diff, textconv attribute, color, showRoot=false, patchid.verbatim), binary-only and rename-only determinism, empty/merge `None`, SHA-256 repositories, invalid/unknown OIDs, input/output byte limits, malformed parser input, non-zero exit, malformed fake output, and timeout with `/proc` verification that both fake git children exited. + - Verify outcome: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id -- --list` lists 15 tests (12 new + 3 from T01); the filtered run passed (15 passed, 0 failed). Additional: `... clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` clean; `cargo fmt` applied. + - Context impact: none at this task — helper is not wired into any runtime path yet; the exact pinned invocation is documented in `agent-trace-hooks-command-routing.md` by T03 per plan. + - Deviation: beyond the plan's minimum pinned options, also pinned `log.showRoot`/`--root` (otherwise `log.showRoot=false` suppresses root-commit diffs), `-O/dev/null` instead of an empty `diff.orderFile` (empty value makes `git show` fail), `--no-show-signature`, `--submodule=short`, `--ignore-submodules=none`, `--inter-hunk-context=0`, and `diff.suppressBlankEmpty`. + - Context synchronization: synced - [ ] T03: `Attach the fingerprint to post-commit Agent Trace insertion` (status:todo) - Task ID: T03 diff --git a/context/sce/agent-trace-hooks-command-routing.md b/context/sce/agent-trace-hooks-command-routing.md index a8ad96ec7..8e314a811 100644 --- a/context/sce/agent-trace-hooks-command-routing.md +++ b/context/sce/agent-trace-hooks-command-routing.md @@ -138,6 +138,17 @@ projection, and repository DB write. They verify the final Agent Trace JSON contains mutation-derived model/session provenance while direct evidence tables remain separate. +## Commit patch ID helper + +`cli/src/services/hooks/commit_patch_id.rs` provides `compute_commit_patch_id(repository_root, commit_oid) -> Result>`, the canonical version-1 commit fingerprint for `agent_traces.commit_patch_id` ([agent-trace-db.md](agent-trace-db.md)). Post-commit does not call it yet; until it is wired, persisted rows carry `NULL`. + +- Input must be a 40-hex (SHA-1) or 64-hex (SHA-256) lowercase commit OID. `git rev-list --parents -n 1 --` decides the parent count; multi-parent merges return `Ok(None)` (no first-parent identity). An empty commit yields empty `patch-id` output and returns `Ok(None)`. +- The fingerprint is the first column of `git patch-id --stable` over the full commit patch, independent of commit message, SHA, and parent. It is patch-equivalence evidence, not unique commit identity, and never derives from SCE's AI-attribution intersection patch. +- Exact pinned invocation (no shell; `GIT_EXTERNAL_DIFF` and `GIT_DIFF_OPTS` removed from the environment): + - `git -c core.quotePath=true -c diff.noprefix=false -c diff.mnemonicPrefix=false -c diff.relative=false -c diff.context=3 -c diff.interHunkContext=0 -c diff.indentHeuristic=true -c diff.algorithm=myers -c diff.renames=false -c diff.suppressBlankEmpty=false -c diff.submodule=short -c diff.ignoreSubmodules=none -c color.ui=never -c color.diff=never -c log.showRoot=true -c log.showSignature=false show --format= --patch --binary --no-ext-diff --no-textconv --no-renames --no-color --diff-algorithm=myers --unified=3 --inter-hunk-context=0 --indent-heuristic --src-prefix=a/ --dst-prefix=b/ --no-relative -O/dev/null --root --no-show-signature --submodule=short --ignore-submodules=none --` + - stdout streamed into `git -c patchid.stable=true -c patchid.verbatim=false patch-id --stable` +- Process safety: every subprocess is a `tokio::process::Command` with `kill_on_drop(true)` under one 10 s overall deadline; streamed patch input is capped at 64 MiB and `patch-id` output at 4 KiB; stderr is drained; both exit statuses are checked; output must be exactly one ` ` line of lowercase hex matching the OID length. Timeouts, limit breaches, non-zero exits, and malformed output return errors carrying only stage/status/limit text (no diff content or Git stderr), and dropping the future kills both children. + ## Explicit non-goals in the current baseline - No checkpoint handoff file From 61236205cf09a72c1868c80bcc450db6442c5583 Mon Sep 17 00:00:00 2001 From: David Abram Date: Thu, 15 Oct 2026 11:13:16 +0200 Subject: [PATCH 3/5] runtime: Attach commit patch IDs to post-commit Agent Traces Persist the best-effort canonical Git patch ID alongside post-commit Agent Trace rows without changing trace JSON. Resolve failures to NULL with a sanitized warning while preserving existing validation and persistence failures, and add coverage for successful, empty, and failed fingerprints. Ref: context/plans/agent-trace-commit-patch-id.md (T03) Co-authored-by: SCE --- cli/src/services/hooks/commit_hooks.rs | 331 +++++++++++++++++- cli/src/services/hooks/mod.rs | 1 - context/context-map.md | 2 +- context/plans/agent-trace-commit-patch-id.md | 10 +- .../sce/agent-trace-hooks-command-routing.md | 5 + 5 files changed, 342 insertions(+), 7 deletions(-) diff --git a/cli/src/services/hooks/commit_hooks.rs b/cli/src/services/hooks/commit_hooks.rs index 42dd436de..4d2bf2a82 100644 --- a/cli/src/services/hooks/commit_hooks.rs +++ b/cli/src/services/hooks/commit_hooks.rs @@ -20,6 +20,7 @@ use crate::services::patch::{ }; use crate::services::sync::auto_sync; +use super::commit_patch_id::compute_commit_patch_id; use super::runtime::{ commit_msg_policy_gate_passed, current_unix_time_ms, open_agent_trace_db_for_hook_runtime, post_rewrite_no_op_reason, pre_commit_no_op_reason, read_hook_stdin, resolve_runtime_state, @@ -164,13 +165,15 @@ where &PostCommitIntersectionFlowResult, Option, &str, + Option<&L>, ) -> Result, C: FnOnce(&Path) -> Result, Launch: FnOnce(&Path) -> Result<()>, K: std::ops::AsyncFnOnce(&Path) -> Result<()>, { let result = run_intersection_flow(repository_root).await?; - let _agent_trace = run_agent_trace_flow(repository_root, &result, vcs_type, remote_url).await?; + let _agent_trace = + run_agent_trace_flow(repository_root, &result, vcs_type, remote_url, logger).await?; if let Err(error) = run_passive_checkpoint(repository_root).await { if let Some(log) = logger { @@ -204,11 +207,17 @@ pub(crate) async fn run_post_commit_passive_checkpoint(repository_root: &Path) - db.passive_checkpoint().await } -pub(crate) async fn run_post_commit_agent_trace_flow( +pub(crate) const COMMIT_PATCH_ID_UNAVAILABLE_EVENT: &str = + "sce.agent_trace.commit_patch_id_unavailable"; + +pub(crate) async fn run_post_commit_agent_trace_flow< + L: crate::services::observability::traits::Logger, +>( repository_root: &Path, flow_result: &PostCommitIntersectionFlowResult, vcs_type: Option, remote_url: &str, + logger: Option<&L>, ) -> Result { let mut db = open_agent_trace_db_for_hook_runtime( repository_root, @@ -229,11 +238,19 @@ pub(crate) async fn run_post_commit_agent_trace_flow( ) .await; + let commit_patch_id = resolve_post_commit_commit_patch_id( + repository_root, + &flow_result.post_commit_data.commit_oid, + logger, + ) + .await; + run_post_commit_agent_trace_flow_with( flow_result, vcs_type, remote_url, &mutation_ai_patch, + commit_patch_id.as_deref(), |trace_value| { validate_agent_trace_value(trace_value) .map_err(|error| anyhow!(error.to_string())) @@ -252,11 +269,37 @@ pub(crate) async fn run_post_commit_agent_trace_flow( .await } +pub(crate) async fn resolve_post_commit_commit_patch_id< + L: crate::services::observability::traits::Logger, +>( + repository_root: &Path, + commit_oid: &str, + logger: Option<&L>, +) -> Option { + match compute_commit_patch_id(repository_root, commit_oid).await { + Ok(commit_patch_id) => commit_patch_id, + Err(error) => { + if let Some(log) = logger { + log.warn( + COMMIT_PATCH_ID_UNAVAILABLE_EVENT, + &format!( + "Commit patch ID unavailable; persisting Agent Trace without it: {error}" + ), + &[], + None, + ); + } + None + } + } +} + pub(crate) async fn run_post_commit_agent_trace_flow_with( flow_result: &PostCommitIntersectionFlowResult, vcs_type: Option, remote_url: &str, mutation_ai_patch: &ParsedPatch, + commit_patch_id: Option<&str>, validate_agent_trace: V, persist_agent_trace: I, ) -> Result @@ -310,7 +353,7 @@ where agent_trace_id: &agent_trace.id, url: &constructed_url, remote_url, - commit_patch_id: None, + commit_patch_id, }; persist_agent_trace(insert_input).await?; @@ -692,3 +735,285 @@ pub(crate) fn capture_head_patch_from_git(repository_root: &Path) -> Result String { format!("Post-commit patch capture error: {detail} ({context}).") } + +#[cfg(test)] +mod tests { + use std::cell::RefCell; + use std::path::Path; + use std::process::Command; + use std::sync::Mutex; + + use anyhow::{anyhow, Result}; + + use crate::services::agent_trace::{validate_agent_trace_value, AgentTrace}; + use crate::services::agent_trace_db::AgentTraceInsert; + use crate::services::error::CliError; + use crate::services::observability::traits::Logger; + use crate::services::patch::ParsedPatch; + + use super::super::commit_patch_id::compute_commit_patch_id; + use super::{ + capture_post_commit_patch_from_git, resolve_post_commit_commit_patch_id, + run_post_commit_agent_trace_flow_with, PostCommitIntersectionFlowResult, + COMMIT_PATCH_ID_UNAVAILABLE_EVENT, + }; + + const REMOTE_URL: &str = "https://example.invalid/repo.git"; + const SECRET_FILE_CONTENT: &str = "commit-patch-id-secret-line"; + const UNKNOWN_COMMIT_OID: &str = "1111111111111111111111111111111111111111"; + const TRACE_ID_PLACEHOLDER: &str = ""; + + #[derive(Default)] + struct RecordingLogger { + warnings: Mutex>, + } + + impl RecordingLogger { + fn warnings(&self) -> Vec<(String, String)> { + self.warnings.lock().unwrap().clone() + } + } + + impl Logger for RecordingLogger { + fn info(&self, _: &str, _: &str, _: &[(&str, &str)], _: Option<&str>) {} + fn debug(&self, _: &str, _: &str, _: &[(&str, &str)], _: Option<&str>) {} + fn warn(&self, event_id: &str, message: &str, _: &[(&str, &str)], _: Option<&str>) { + self.warnings + .lock() + .unwrap() + .push((event_id.to_string(), message.to_string())); + } + fn error(&self, _: &str, _: &str, _: &[(&str, &str)], _: Option<&str>) {} + fn log_cli_error(&self, _: &CliError, _: Option<&str>) {} + } + + #[derive(Clone, Debug, Eq, PartialEq)] + struct CapturedInsert { + commit_id: String, + commit_time_ms: i64, + trace_json: String, + agent_trace_id: String, + remote_url: String, + commit_patch_id: Option, + } + + impl CapturedInsert { + fn from_insert(insert: &AgentTraceInsert<'_>) -> Self { + Self { + commit_id: insert.commit_id.to_string(), + commit_time_ms: insert.commit_time_ms, + trace_json: insert.trace_json.to_string(), + agent_trace_id: insert.agent_trace_id.to_string(), + remote_url: insert.remote_url.to_string(), + commit_patch_id: insert.commit_patch_id.map(str::to_string), + } + } + } + + fn git(cwd: &Path, args: &[&str]) -> String { + let output = Command::new("git") + .args(args) + .current_dir(cwd) + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_NOSYSTEM", "1") + .output() + .expect("git command should run"); + assert!( + output.status.success(), + "git {args:?} failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8(output.stdout) + .expect("git output is UTF-8") + .trim() + .to_string() + } + + fn init_repository_with_commit(root: &Path) -> String { + git(root, &["init", "-q", "-b", "main"]); + git(root, &["config", "user.email", "patch-id@example.invalid"]); + git(root, &["config", "user.name", "Patch Id Test"]); + git(root, &["config", "commit.gpgsign", "false"]); + std::fs::write(root.join("a.txt"), "one\n").expect("write a.txt"); + git(root, &["add", "a.txt"]); + git(root, &["commit", "-q", "-m", "root"]); + std::fs::write(root.join("a.txt"), format!("one\n{SECRET_FILE_CONTENT}\n")) + .expect("write a.txt"); + git(root, &["commit", "-q", "-am", "change"]); + git(root, &["rev-parse", "HEAD"]) + } + + fn flow_result(root: &Path) -> PostCommitIntersectionFlowResult { + PostCommitIntersectionFlowResult { + combined_recent_patch: empty_patch(), + post_commit_data: capture_post_commit_patch_from_git(root) + .expect("post-commit patch capture"), + tool_name: None, + tool_version: None, + } + } + + fn empty_patch() -> ParsedPatch { + ParsedPatch { files: Vec::new() } + } + + async fn run_flow_capturing( + flow_result: &PostCommitIntersectionFlowResult, + commit_patch_id: Option<&str>, + ) -> (AgentTrace, CapturedInsert) { + let captured = RefCell::new(None); + let agent_trace = run_post_commit_agent_trace_flow_with( + flow_result, + None, + REMOTE_URL, + &empty_patch(), + commit_patch_id, + |trace_value| { + validate_agent_trace_value(trace_value).map_err(|error| anyhow!(error.to_string())) + }, + async |insert: AgentTraceInsert<'_>| -> Result<()> { + *captured.borrow_mut() = Some(CapturedInsert::from_insert(&insert)); + Ok(()) + }, + ) + .await + .expect("post-commit agent trace flow"); + let insert = captured.into_inner().expect("agent trace insert captured"); + (agent_trace, insert) + } + + fn normalized_trace_json(insert: &CapturedInsert) -> String { + insert + .trace_json + .replace(&insert.agent_trace_id, TRACE_ID_PLACEHOLDER) + } + + #[tokio::test] + async fn commit_patch_id_is_attached_to_post_commit_insert_without_changing_trace_json() { + let dir = tempfile::tempdir().expect("temp dir"); + let root = dir.path(); + let commit_oid = init_repository_with_commit(root); + let flow_result = flow_result(root); + assert_eq!(flow_result.post_commit_data.commit_oid, commit_oid); + + let logger = RecordingLogger::default(); + let commit_patch_id = + resolve_post_commit_commit_patch_id(root, &commit_oid, Some(&logger)).await; + let expected = compute_commit_patch_id(root, &commit_oid) + .await + .expect("canonical patch id") + .expect("non-empty commit"); + assert_eq!(commit_patch_id.as_deref(), Some(expected.as_str())); + assert!(logger.warnings().is_empty()); + + let (with_trace, with_id) = run_flow_capturing(&flow_result, Some(&expected)).await; + let (without_trace, without_id) = run_flow_capturing(&flow_result, None).await; + + assert_eq!(with_id.commit_id, commit_oid); + assert_eq!(with_id.commit_patch_id.as_deref(), Some(expected.as_str())); + assert_eq!(without_id.commit_id, commit_oid); + assert_eq!(without_id.commit_patch_id, None); + assert_eq!(with_id.commit_time_ms, without_id.commit_time_ms); + assert_eq!(with_id.remote_url, REMOTE_URL); + + for (agent_trace, insert) in [(&with_trace, &with_id), (&without_trace, &without_id)] { + assert_eq!(insert.agent_trace_id, agent_trace.id); + assert_eq!( + insert.trace_json, + format!("{}\n", serde_json::to_string_pretty(agent_trace).unwrap()) + ); + } + assert!(!with_id.trace_json.contains(&expected)); + assert_eq!( + normalized_trace_json(&with_id), + normalized_trace_json(&without_id) + ); + } + + #[tokio::test] + async fn commit_patch_id_is_none_without_warning_for_empty_post_commit() { + let dir = tempfile::tempdir().expect("temp dir"); + let root = dir.path(); + init_repository_with_commit(root); + git(root, &["commit", "-q", "--allow-empty", "-m", "empty"]); + let empty_oid = git(root, &["rev-parse", "HEAD"]); + + let logger = RecordingLogger::default(); + let commit_patch_id = + resolve_post_commit_commit_patch_id(root, &empty_oid, Some(&logger)).await; + + assert_eq!(commit_patch_id, None); + assert!(logger.warnings().is_empty()); + } + + #[tokio::test] + async fn commit_patch_id_failure_logs_sanitized_warning_and_persists_null() { + let dir = tempfile::tempdir().expect("temp dir"); + let root = dir.path(); + init_repository_with_commit(root); + let flow_result = flow_result(root); + + let logger = RecordingLogger::default(); + let commit_patch_id = + resolve_post_commit_commit_patch_id(root, UNKNOWN_COMMIT_OID, Some(&logger)).await; + assert_eq!(commit_patch_id, None); + + let warnings = logger.warnings(); + assert_eq!(warnings.len(), 1); + let (event_id, message) = &warnings[0]; + assert_eq!(event_id, COMMIT_PATCH_ID_UNAVAILABLE_EVENT); + assert!( + message.starts_with("Commit patch ID unavailable; persisting Agent Trace without it: ") + ); + assert!(!message.contains(SECRET_FILE_CONTENT)); + + let (_, insert) = run_flow_capturing(&flow_result, commit_patch_id.as_deref()).await; + assert_eq!(insert.commit_id, flow_result.post_commit_data.commit_oid); + assert_eq!(insert.commit_patch_id, None); + } + + #[tokio::test] + async fn commit_patch_id_does_not_mask_agent_trace_validation_or_persistence_failures() { + let dir = tempfile::tempdir().expect("temp dir"); + let root = dir.path(); + init_repository_with_commit(root); + let flow_result = flow_result(root); + let commit_patch_id = "a".repeat(40); + + let persisted = RefCell::new(false); + let validation_error = run_post_commit_agent_trace_flow_with( + &flow_result, + None, + REMOTE_URL, + &empty_patch(), + Some(&commit_patch_id), + |_| Err(anyhow!("validation rejected")), + async |_insert: AgentTraceInsert<'_>| -> Result<()> { + *persisted.borrow_mut() = true; + Ok(()) + }, + ) + .await + .expect_err("validation failure must propagate"); + assert_eq!( + validation_error.to_string(), + "Failed to validate built post-commit Agent Trace payload." + ); + assert!(!*persisted.borrow()); + + let persistence_error = run_post_commit_agent_trace_flow_with( + &flow_result, + None, + REMOTE_URL, + &empty_patch(), + Some(&commit_patch_id), + |_| Ok(()), + async |_insert: AgentTraceInsert<'_>| -> Result<()> { + Err(anyhow!("persistence rejected")) + }, + ) + .await + .expect_err("persistence failure must propagate"); + assert_eq!(persistence_error.to_string(), "persistence rejected"); + } +} diff --git a/cli/src/services/hooks/mod.rs b/cli/src/services/hooks/mod.rs index 960d60652..d621f00a0 100644 --- a/cli/src/services/hooks/mod.rs +++ b/cli/src/services/hooks/mod.rs @@ -2,7 +2,6 @@ use anyhow::Context; mod claude_transforms; mod commit_hooks; -#[cfg_attr(not(test), allow(dead_code))] mod commit_patch_id; mod conversation_trace; mod diff_trace; diff --git a/context/context-map.md b/context/context-map.md index 8dde7a6ce..bad1d5ebe 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -99,7 +99,7 @@ Feature/domain context: - `context/sce/agent-trace-retry-queue-observability.md` (inactive local-hook retry path plus historical retry/metrics reference) - `context/sce/agent-trace-local-hooks-mvp-contract-gap-matrix.md` (T01 Local Hooks MVP production contract freeze and deterministic gap matrix for `agent-trace-local-hooks-production-mvp`) - `context/sce/agent-trace-minimal-generator.md` (implemented a library minimal Agent Trace generator seam at `cli/src/services/agent_trace.rs`, used by the active post-commit hook flow to produce strict `0.1.0` JSON payloads with top-level `version`, UUIDv7 `id` derived from commit-time metadata, caller-provided commit-time `timestamp`, optional top-level `vcs` metadata emitted when present (`type` from enum `git|jj|hg|svn`, `revision` from metadata input; current post-commit flow provides `git`), optional top-level `tool` metadata (`name`/`version`) sourced from builder metadata inputs when overlapping AI content exists, always-emitted `metadata.sce.version` sourced from the compiled `sce` CLI package version, and always-emitted `metadata.sce.line_changes` (`{ai,mixed,unknown}` each `{added,removed}` `u64` counters, `#[serde(default)]` for backward-compatible deserialization) carrying exact touched-line attribution counts from canonical `post_commit_patch` hunks reusing the same per-hunk classification, plus per-file trace data from patch inputs via `intersect_patches(constructed_patch, post_commit_patch)` then `post_commit_patch`-anchored hunk classification into `ai`/`mixed`/`unknown` contributor categories, serialized per conversation with a required lookup `url` derived from top-level `AgentTrace.id`, nested `contributor.type` with optional `contributor.model_id` omitted when provenance is missing, optional canonical session links derived from matched touched-line provenance, one derived `ranges[{start_line,end_line,content_hash}]` entry per post-commit or embedded-patch hunk, and range `content_hash` values that hash touched-line kind/content independent of positions and metadata) -- `context/sce/agent-trace-hooks-command-routing.md` (implemented `sce hooks` command routing plus current runtime behavior: enabled-by-default commit-msg attribution with explicit opt-out controls, no-op `pre-commit`/`post-rewrite` entrypoints, active `post-commit` intersection and Agent Trace DB persistence, DB-only `diff-trace` STDIN intake with OpenCode normalized payloads and Claude structured `PostToolUse` payload classification, tool-prefixed stored `diff_traces.session_id` values (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi) while hook diagnostics route with unprefixed producer-native sessions when available, nullable `model_id`/direct `tool_version` persistence with Claude `direct > exact transcript > exact session/agent state > bridge-derived chain state > NULL` attribution, Claude direct-first model metadata extraction from top-level or nested fields followed by fail-open `transcript_path` + `tool_use_id` JSONL lookup and exact local state lookup when model is absent, a main-session-scope-only bridge chain state-miss fallback that seeds a `source="bridge_inherited"` row for the current session (the one diff-trace path that writes state), `claude/` prefix normalization, no parsed-payload artifact persistence under `context/tmp`, the silent local-only `sce hooks claude-model-state` lifecycle intake for synchronous `SessionStart` and asynchronous `PostModelSwitch` writes whose own model-less `SessionStart` bridge attempt cannot read its lazily created transcript, `session-model` removed from the supported hook surface, and `conversation-trace` STDIN intake for normalized batches plus supported raw Claude events with per-item and first-valid-insert batch diagnostic routing; this document also owns the current `diff-trace`, `conversation-trace`, and Claude model-state fail-open intake contracts, and now also routes the hidden non-fail-open `sce hooks mutation-scope` mutation-scope-runtime ingress with its two carried-outcome success variants — distinct from the fail-open `diff-trace`/`conversation-trace` intakes — and the hidden non-fail-open `sce hooks claude-mutation-scope` first-concrete-adapter route (now registered by `sce setup`), deferring the full contracts to `context/cli/mutation-scope-hook-ingress.md` and `context/cli/claude-mutation-scope-integration.md`.) +- `context/sce/agent-trace-hooks-command-routing.md` (implemented `sce hooks` command routing plus current runtime behavior: enabled-by-default commit-msg attribution with explicit opt-out controls, no-op `pre-commit`/`post-rewrite` entrypoints, active `post-commit` intersection and Agent Trace DB persistence with a best-effort canonical `git patch-id --stable` commit patch ID stored in nullable `agent_traces.commit_patch_id` (sanitized warning and `NULL` on failure), DB-only `diff-trace` STDIN intake with OpenCode normalized payloads and Claude structured `PostToolUse` payload classification, tool-prefixed stored `diff_traces.session_id` values (`oc_` for OpenCode, `cc_` for Claude, `pi_` for Pi) while hook diagnostics route with unprefixed producer-native sessions when available, nullable `model_id`/direct `tool_version` persistence with Claude `direct > exact transcript > exact session/agent state > bridge-derived chain state > NULL` attribution, Claude direct-first model metadata extraction from top-level or nested fields followed by fail-open `transcript_path` + `tool_use_id` JSONL lookup and exact local state lookup when model is absent, a main-session-scope-only bridge chain state-miss fallback that seeds a `source="bridge_inherited"` row for the current session (the one diff-trace path that writes state), `claude/` prefix normalization, no parsed-payload artifact persistence under `context/tmp`, the silent local-only `sce hooks claude-model-state` lifecycle intake for synchronous `SessionStart` and asynchronous `PostModelSwitch` writes whose own model-less `SessionStart` bridge attempt cannot read its lazily created transcript, `session-model` removed from the supported hook surface, and `conversation-trace` STDIN intake for normalized batches plus supported raw Claude events with per-item and first-valid-insert batch diagnostic routing; this document also owns the current `diff-trace`, `conversation-trace`, and Claude model-state fail-open intake contracts, and now also routes the hidden non-fail-open `sce hooks mutation-scope` mutation-scope-runtime ingress with its two carried-outcome success variants — distinct from the fail-open `diff-trace`/`conversation-trace` intakes — and the hidden non-fail-open `sce hooks claude-mutation-scope` first-concrete-adapter route (now registered by `sce setup`), deferring the full contracts to `context/cli/mutation-scope-hook-ingress.md` and `context/cli/claude-mutation-scope-integration.md`.) - `context/sce/automated-profile-contract.md` (deterministic gate policy for automated OpenCode profile, including 10 gate categories, permission mappings, automated `/commit` single-commit execution behavior, and automated profile constraints) - `context/sce/bash-tool-policy-enforcement-contract.md` (approved bash-tool blocking contract plus current Rust evaluator seam and OpenCode/Claude delegation references, including config schema, argv-prefix matching, shell/nix unwrapping, custom-policy `satisfied_by` wrapper exemption, fixed preset catalog/messages, and precedence rules) - `context/sce/bash-policy-satisfied-by-wrapper-exemption.md` (custom-policy `satisfied_by` field: optional list of wrapper argv prefixes that exempt a policy from firing when the matched command was unwrapped from one of them; `NormalizedSegment` wrapper-chain tracking, matching model, examples, and scope) diff --git a/context/plans/agent-trace-commit-patch-id.md b/context/plans/agent-trace-commit-patch-id.md index daec46d8f..c996805be 100644 --- a/context/plans/agent-trace-commit-patch-id.md +++ b/context/plans/agent-trace-commit-patch-id.md @@ -95,13 +95,19 @@ How this plan is proven complete. Each criterion is observable and names the che - Deviation: beyond the plan's minimum pinned options, also pinned `log.showRoot`/`--root` (otherwise `log.showRoot=false` suppresses root-commit diffs), `-O/dev/null` instead of an empty `diff.orderFile` (empty value makes `git show` fail), `--no-show-signature`, `--submodule=short`, `--ignore-submodules=none`, `--inter-hunk-context=0`, and `diff.suppressBlankEmpty`. - Context synchronization: synced -- [ ] T03: `Attach the fingerprint to post-commit Agent Trace insertion` (status:todo) +- [x] T03: `Attach the fingerprint to post-commit Agent Trace insertion` (status:done) - Task ID: T03 - Scope: In — `run_post_commit_agent_trace_flow` and its `run_post_commit_agent_trace_flow_with` test seam, fingerprint-to-insert wiring, best-effort sanitized warning, local storage tests, and the `agent-trace-hooks-command-routing.md` update documenting the exact pinned invocation. Out — changing Agent Trace JSON, changing intersection capture signatures, sync transport. - Dependencies: T01, T02 - Done when: The already-captured `commit_oid` is fingerprinted asynchronously after the intersection flow and before the Agent Trace database insert; the stored row carries the nullable ID without any change to `trace_json`; fingerprint errors log a sanitized warning and persist `NULL`; Agent Trace build, validation, and persistence failures keep their existing behavior. - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id` (hook/storage cases). - - Context synchronization: pending + - Completed: 2026-10-15 + - Files changed: `cli/src/services/hooks/commit_hooks.rs`, `cli/src/services/hooks/mod.rs`, `context/sce/agent-trace-hooks-command-routing.md`, `context/plans/agent-trace-commit-patch-id.md` + - Result: `run_post_commit_subcommand_with`'s agent-trace closure type now also receives the post-commit `Option<&L>` logger, threaded into `run_post_commit_agent_trace_flow`. After mutation-AI patch resolution and before the insert, it calls new `resolve_post_commit_commit_patch_id(repository_root, &post_commit_data.commit_oid, logger)`, which returns the `compute_commit_patch_id` value or, on error, logs `Logger::warn` event `sce.agent_trace.commit_patch_id_unavailable` (`COMMIT_PATCH_ID_UNAVAILABLE_EVENT`) with `Commit patch ID unavailable; persisting Agent Trace without it: ` and returns `None`. `run_post_commit_agent_trace_flow_with` gained a `commit_patch_id: Option<&str>` parameter forwarded into `AgentTraceInsert.commit_patch_id`; trace build/validation/serialization are unchanged. Removed the T02 `#[cfg_attr(not(test), allow(dead_code))]` on `mod commit_patch_id`. Four new `commit_patch_id` tests in a new `commit_hooks` test module (temporary Git repositories, capturing persist closure, recording logger) cover: fingerprint attached with exact `commit_id`, and `trace_json` equal to the standard pretty serialization, free of the ID, and identical (modulo the per-run trace UUID) between `Some` and `None`; empty commit → `None` without warning; unknown-OID helper failure → one sanitized warning (no diff content) and `NULL` insert; validation and persistence errors still propagate unchanged. `agent-trace-hooks-command-routing.md` documents the wiring, the exact pinned invocation, `None` cases, bounds, and failure policy. + - Verify outcome: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id` passed (19 passed, 0 failed; 4 new + 15 from T01/T02, so the filter is non-empty). Additional: `... test hooks` passed (49 passed); `... clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` clean; `cargo fmt` applied. + - Context impact: localized — `context/sce/agent-trace-hooks-command-routing.md` updated (post-commit fingerprint wiring, canonical pinned invocation, failure/warning policy). + - Deviation: the fingerprint is computed in the production wrapper and the `_with` seam receives the resolved `Option<&str>`; persistence is asserted through the seam's capturing insert closure (real-DB `Some`/`None` insert coverage already exists from T01). + - Context synchronization: synced - [ ] T04: `Export commitPatchId through the existing agent_traces sync stream` (status:todo) - Task ID: T04 diff --git a/context/sce/agent-trace-hooks-command-routing.md b/context/sce/agent-trace-hooks-command-routing.md index 8e314a811..fa75afb4a 100644 --- a/context/sce/agent-trace-hooks-command-routing.md +++ b/context/sce/agent-trace-hooks-command-routing.md @@ -70,6 +70,11 @@ Interactive `sce setup` persists the independent confirmations at `agent_trace.a - The built Agent Trace payload is converted to JSON `Value` and validated via `agent_trace::validate_agent_trace_value(...)` before persistence. - Validation failures are returned through the same post-commit runtime failure path/class used for Agent Trace DB insertion failures (no silent fallback). - 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`. + - **Commit patch ID (best-effort fingerprint):** after mutation-AI patch resolution and before the insert, `run_post_commit_agent_trace_flow(...)` calls `resolve_post_commit_commit_patch_id(...)`, which fingerprints the already-captured `post_commit_data.commit_oid` (no second `HEAD` lookup) through `hooks::commit_patch_id::compute_commit_patch_id(...)`. The resulting `Option<&str>` is passed to `run_post_commit_agent_trace_flow_with(...)` and stored in the same `agent_traces` insert as nullable `commit_patch_id`; it never enters or reserializes `trace_json`, and is never derived from SCE's AI-attribution intersection patch. It is patch-equivalence evidence, not proof of unique commit identity. + - Canonical version-1 algorithm: the commit's whole patch from the pinned invocation `git -c core.quotePath=true -c diff.noprefix=false -c diff.mnemonicPrefix=false -c diff.relative=false -c diff.context=3 -c diff.interHunkContext=0 -c diff.indentHeuristic=true -c diff.algorithm=myers -c diff.renames=false -c diff.suppressBlankEmpty=false -c diff.submodule=short -c diff.ignoreSubmodules=none -c color.ui=never -c color.diff=never -c log.showRoot=true -c log.showSignature=false show --format= --patch --binary --no-ext-diff --no-textconv --no-renames --no-color --diff-algorithm=myers --unified=3 --inter-hunk-context=0 --indent-heuristic --src-prefix=a/ --dst-prefix=b/ --no-relative -O/dev/null --root --no-show-signature --submodule=short --ignore-submodules=none --`, streamed (no shell pipeline) into `git -c patchid.stable=true -c patchid.verbatim=false patch-id --stable`, with `GIT_EXTERNAL_DIFF` / `GIT_DIFF_OPTS` removed from both environments. The fingerprint is the first output column, validated as lowercase hex of the commit OID's length (40 for SHA-1, 64 for SHA-256). + - Empty commits (no patch-id output) and multi-parent merge commits (detected via `git rev-list --parents -n 1 --`) yield `None` without a warning. + - Every subprocess is a direct `tokio::process::Command` with `kill_on_drop(true)` under a 10 s overall deadline; streamed patch input is capped at 64 MiB, patch-id output at 4 KiB, and both exit statuses plus the output shape are checked. + - Failure policy: any helper error (non-zero exit, malformed or oversized output, timeout, invalid OID) is logged via `Logger::warn` with event `sce.agent_trace.commit_patch_id_unavailable` and message `Commit patch ID unavailable; persisting Agent Trace without it: ` (stage/status/limit text only — never diff content or Git stderr), and the trace is persisted with `commit_patch_id = NULL`. There is no retry. Agent Trace build, validation, and persistence errors are unaffected and keep their existing failure path. - 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. From 848c4ee47829a6e073545a8b39082d6ba1f66735 Mon Sep 17 00:00:00 2001 From: David Abram Date: Thu, 15 Oct 2026 11:26:01 +0200 Subject: [PATCH 4/5] sync: Export commit patch IDs on agent trace batches Preserve each nullable local `commit_patch_id` through the existing `agent_traces` export stream so the Control Plane can associate traces with equivalent patch fingerprints without adding a stream or cursor. Keep raw trace JSON and authoritative server-cursor behavior unchanged, including no replay of acknowledged rows. Completes plan `agent-trace-commit-patch-id`, task T04. The Control Plane receiver must accept and persist `commitPatchId` before release; historical rows remain null and are not backfilled. Co-authored-by: SCE --- cli/src/services/agent_trace_export/mod.rs | 81 +++++++++++---- cli/src/services/sync/sync.rs | 101 +++++++++++++++++-- context/cli/agent-trace-sync-command.md | 8 ++ context/context-map.md | 4 +- context/plans/agent-trace-commit-patch-id.md | 55 ++++++++-- context/sce/agent-trace-export-readers.md | 2 + 6 files changed, 211 insertions(+), 40 deletions(-) diff --git a/cli/src/services/agent_trace_export/mod.rs b/cli/src/services/agent_trace_export/mod.rs index 64b1e77d1..dddf2a5b2 100644 --- a/cli/src/services/agent_trace_export/mod.rs +++ b/cli/src/services/agent_trace_export/mod.rs @@ -51,7 +51,6 @@ pub fn validate_js_safe_integer(value: i64) -> Result<()> { Ok(()) } -/// Owned, wire-compatible export row for the `messages` capture stream. #[derive(Clone, Debug, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub struct AgentTraceMessageExportRow { @@ -62,7 +61,6 @@ pub struct AgentTraceMessageExportRow { pub generated_at_unix_ms: i64, } -/// Owned, wire-compatible export row for the `parts` capture stream. #[derive(Clone, Debug, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub struct AgentTracePartExportRow { @@ -75,7 +73,6 @@ pub struct AgentTracePartExportRow { pub generated_at_unix_ms: i64, } -/// Owned, wire-compatible export row for the `diff_traces` capture stream. #[allow( dead_code, reason = "diff_traces compatibility surface retained per context/decisions/2026-10-01-retire-diff-traces-remote-sync-compatibility-gate.md" @@ -93,7 +90,6 @@ pub struct AgentTraceDiffTraceExportRow { pub payload_type: String, } -/// Owned, wire-compatible export row for the `agent_traces` capture stream. #[derive(Clone, Debug, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] pub struct AgentTraceAgentTraceExportRow { @@ -104,6 +100,7 @@ pub struct AgentTraceAgentTraceExportRow { pub trace_json: String, pub url: String, pub remote_url: Option, + pub commit_patch_id: Option, } const SELECT_MESSAGES_AFTER_SQL: &str = @@ -132,16 +129,12 @@ ORDER BY id ASC LIMIT ?2"; const SELECT_AGENT_TRACES_AFTER_SQL: &str = - "SELECT id, agent_trace_id, commit_id, commit_time_ms, trace_json, url, remote_url + "SELECT id, agent_trace_id, commit_id, commit_time_ms, trace_json, url, remote_url, commit_patch_id FROM agent_traces WHERE id > ?1 ORDER BY id ASC LIMIT ?2"; -/// Read-only incremental export reader over one repository-scoped Agent Trace -/// database. Holds no local cursor, performs no mutation, and makes no -/// network calls; the caller supplies the last server-accepted `id` as -/// `cursor` on every call. pub struct AgentTraceExportReader<'a> { db: &'a RepositoryAgentTraceDb, } @@ -151,8 +144,6 @@ impl<'a> AgentTraceExportReader<'a> { Self { db } } - /// Read `messages` rows with `id > cursor`, ordered by `id ASC`, capped - /// at `limit`. pub async fn read_messages_after( &self, cursor: i64, @@ -178,8 +169,6 @@ impl<'a> AgentTraceExportReader<'a> { Ok(rows) } - /// Read `parts` rows with `id > cursor`, ordered by `id ASC`, capped at - /// `limit`. pub async fn read_parts_after( &self, cursor: i64, @@ -205,9 +194,6 @@ impl<'a> AgentTraceExportReader<'a> { Ok(rows) } - /// Read `diff_traces` rows with `id > cursor`, ordered by `id ASC`, - /// capped at `limit`. `patch` and `payload_type` are returned raw and - /// unmodified: no patch parsing or normalization is performed. #[allow( dead_code, reason = "diff_traces compatibility surface retained per context/decisions/2026-10-01-retire-diff-traces-remote-sync-compatibility-gate.md" @@ -237,9 +223,6 @@ impl<'a> AgentTraceExportReader<'a> { Ok(rows) } - /// Read `agent_traces` rows with `id > cursor`, ordered by `id ASC`, - /// capped at `limit`. `trace_json` is returned as the exact raw string - /// from `SQLite`: no parse/reserialize is performed. pub async fn read_agent_traces_after( &self, cursor: i64, @@ -337,6 +320,9 @@ fn agent_trace_export_row_from_turso(row: &turso::Row) -> Result Result { fn limit_as_i64(limit: usize) -> i64 { i64::try_from(limit).expect("validated limit should fit in i64") } + +#[cfg(test)] +mod tests { + use super::AgentTraceExportReader; + use crate::services::agent_trace_db::transaction_tests::open; + use crate::services::agent_trace_db::AgentTraceInsert; + + const RAW_TRACE_JSON: &str = "{\"b\": 1,\n \"a\":2 }"; + const PATCH_ID: &str = "0123456789abcdef0123456789abcdef01234567"; + + #[tokio::test] + async fn read_agent_traces_after_exports_commit_patch_id_with_raw_trace_json() { + let dir = tempfile::tempdir().expect("tempdir"); + let db = open(&dir.path().join("agent-trace.db")).await; + for (commit_id, commit_patch_id) in [("with", Some(PATCH_ID)), ("without", None)] { + db.insert_agent_trace(AgentTraceInsert { + commit_id, + commit_time_ms: 1, + trace_json: RAW_TRACE_JSON, + agent_trace_id: commit_id, + url: "https://example.test/trace", + remote_url: "https://example.test/repo.git", + commit_patch_id, + }) + .await + .expect("insert agent trace"); + } + + let rows = AgentTraceExportReader::new(&db) + .read_agent_traces_after(0, 100) + .await + .expect("read agent traces"); + + let ids: Vec = rows.iter().map(|row| row.source_row_id).collect(); + assert_eq!(ids, [1, 2]); + assert_eq!(rows[0].commit_patch_id.as_deref(), Some(PATCH_ID)); + assert_eq!(rows[1].commit_patch_id, None); + assert!(rows.iter().all(|row| row.trace_json == RAW_TRACE_JSON)); + + let serialized: Vec = rows + .iter() + .map(|row| serde_json::to_value(row).expect("serialize row")) + .collect(); + assert_eq!(serialized[0]["commitPatchId"], PATCH_ID); + assert!(serialized[1] + .as_object() + .expect("row object") + .get("commitPatchId") + .is_some_and(serde_json::Value::is_null)); + assert!(serialized + .iter() + .all(|row| row["traceJson"] == RAW_TRACE_JSON + && row["commitId"].is_string() + && row["sourceRowId"].is_i64() + && row["remoteUrl"] == "https://example.test/repo.git")); + } +} diff --git a/cli/src/services/sync/sync.rs b/cli/src/services/sync/sync.rs index 1e5c96b2d..bac766ec8 100644 --- a/cli/src/services/sync/sync.rs +++ b/cli/src/services/sync/sync.rs @@ -1345,6 +1345,12 @@ mod contract_tests { type CapturedRequests = Arc>>; fn start_control_plane() -> (String, CapturedRequests) { + start_control_plane_with_agent_traces_cursor(0) + } + + fn start_control_plane_with_agent_traces_cursor( + agent_traces_cursor: i64, + ) -> (String, CapturedRequests) { let listener = TcpListener::bind("127.0.0.1:0").expect("bind control plane"); let base_url = format!("http://{}", listener.local_addr().expect("server addr")); let requests = Arc::new(Mutex::new(Vec::new())); @@ -1387,7 +1393,7 @@ mod contract_tests { "messages": 0, "parts": 0, "diffTraces": DIFF_TRACES_CURSOR, - "agentTraces": 0, + "agentTraces": agent_traces_cursor, }}) } else { let rows = body["rows"].as_array().cloned().unwrap_or_default(); @@ -1412,6 +1418,19 @@ mod contract_tests { (base_url, requests) } + fn test_client(base_url: String) -> AuthenticatedControlPlaneClient { + AuthenticatedControlPlaneClient::with_credential_store( + reqwest::Client::builder() + .tls_certs_only(Vec::new()) + .build() + .expect("test http client"), + base_url, + "http://127.0.0.1:1", + "client", + FixedCredentialStore, + ) + } + #[tokio::test] async fn sync_uploads_only_three_streams_and_reports_zero_upload_diff_traces() { let dir = tempfile::tempdir().expect("tempdir"); @@ -1443,16 +1462,7 @@ mod contract_tests { .expect("insert diff trace"); let (base_url, requests) = start_control_plane(); - let client = AuthenticatedControlPlaneClient::with_credential_store( - reqwest::Client::builder() - .tls_certs_only(Vec::new()) - .build() - .expect("test http client"), - base_url, - "http://127.0.0.1:1", - "client", - FixedCredentialStore, - ); + let client = test_client(base_url); let reader = AgentTraceExportReader::new(&db); let mut reporter = NoopProgressReporter; @@ -1480,4 +1490,73 @@ mod contract_tests { ); assert_eq!(report.streams.diff_traces.final_cursor, DIFF_TRACES_CURSOR); } + + #[tokio::test] + async fn sync_sends_commit_patch_id_on_agent_traces_batch_without_replaying_acknowledged_rows() + { + const PATCH_ID: &str = "0123456789abcdef0123456789abcdef01234567"; + const RAW_TRACE_JSON: &str = "{\"b\": 1,\n \"a\":2 }"; + + let dir = tempfile::tempdir().expect("tempdir"); + let db = open(&dir.path().join("agent-trace.db")).await; + for (commit_id, commit_patch_id) in [ + ("acknowledged", Some(PATCH_ID)), + ("with", Some(PATCH_ID)), + ("without", None), + ] { + db.insert_agent_trace(AgentTraceInsert { + commit_id, + commit_time_ms: 1, + trace_json: RAW_TRACE_JSON, + agent_trace_id: commit_id, + url: "https://example.test/trace", + remote_url: "https://example.test/repo.git", + commit_patch_id, + }) + .await + .expect("insert agent trace"); + } + + let (base_url, requests) = start_control_plane_with_agent_traces_cursor(1); + let client = test_client(base_url); + let reader = AgentTraceExportReader::new(&db); + let mut reporter = NoopProgressReporter; + + let report = run_sync_async("repo", "source", &reader, &client, &mut reporter) + .await + .expect("sync converges"); + + let requests = requests.lock().expect("requests lock").clone(); + let mut streams: Vec<&str> = requests + .iter() + .filter(|(path, _)| path.ends_with("/batch")) + .map(|(_, body)| body["stream"].as_str().expect("stream field")) + .collect(); + streams.sort_unstable(); + assert_eq!(streams, ["agent_traces"]); + + let rows: Vec = requests + .iter() + .filter(|(path, body)| path.ends_with("/batch") && body["stream"] == "agent_traces") + .flat_map(|(_, body)| body["rows"].as_array().cloned().unwrap_or_default()) + .collect(); + let source_row_ids: Vec = rows + .iter() + .map(|row| row["sourceRowId"].as_i64().expect("sourceRowId")) + .collect(); + assert_eq!(source_row_ids, [2, 3]); + assert_eq!(rows[0]["commitPatchId"], PATCH_ID); + assert!(rows[1] + .as_object() + .expect("row object") + .get("commitPatchId") + .is_some_and(Value::is_null)); + assert!(rows + .iter() + .all(|row| row["traceJson"] == RAW_TRACE_JSON && row["commitId"].is_string())); + + assert_eq!(report.streams.agent_traces.initial_cursor, 1); + assert_eq!(report.streams.agent_traces.final_cursor, 3); + assert_eq!(report.streams.agent_traces.uploaded, 2); + } } diff --git a/context/cli/agent-trace-sync-command.md b/context/cli/agent-trace-sync-command.md index 72368b4e1..b0482db45 100644 --- a/context/cli/agent-trace-sync-command.md +++ b/context/cli/agent-trace-sync-command.md @@ -53,6 +53,14 @@ flowchart TD Phase 3 only stops new `diff_traces` batch ingestion and persistence. `/state.cursors.diffTraces` remains required through the compatibility window because the Phase 2 client decodes it as mandatory; a Control Plane that dropped it would break every Phase 2 client. Removing that cursor and the retained surfaces belongs to the later compatibility-removal phase, which removes them from both sides together. The decision is recorded in [Retire diff_traces from remote sync through a staged compatibility gate](../decisions/2026-10-01-retire-diff-traces-remote-sync-compatibility-gate.md). +### Commit patch ID field + +Each `agent_traces` batch row carries an optional `commitPatchId` (lowercase hex Git patch ID or `null`, always present) taken unchanged from the local `agent_traces.commit_patch_id` column; see [agent-trace-export-readers.md](../sce/agent-trace-export-readers.md) and [agent-trace-hooks-command-routing.md](../sce/agent-trace-hooks-command-routing.md) for how it is captured. It rides the existing `agent_traces` stream: there is no fourth remote stream, no new cursor, and no change to `/state`, batch validation, retry, or conflict reconciliation. + +- **Release dependency:** a Control Plane receiver that accepts and persists optional `commitPatchId` must be deployed before releasing an SCE version that exports it. The current server decoder may strip an unknown field while still acknowledging the row, and acknowledged rows are never re-sent. +- **No backfill:** rows the server acknowledged before the upgrade are not replayed, because sync resumes from the authoritative server cursor. Rows captured before migration `007` export `commitPatchId: null`. Re-sending historical fingerprints would require an explicit later backfill or change stream, not cursor rewinding. +- **Upgrade/downgrade:** the column arrives through migration `007_commit_patch_id`, which only `sce setup` applies; until it reruns, repository hooks fail open and capture nothing, and an older CLI binary may reject the newer migration ledger (see [agent-trace-db.md](../sce/agent-trace-db.md)). + ## No-local-persistence invariants Sync creates no local sync state anywhere on disk: diff --git a/context/context-map.md b/context/context-map.md index bad1d5ebe..d9a979c34 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -22,7 +22,7 @@ Feature/domain context: - `context/cli/agent-trace-auto-sync.md` (setup-enabled post-commit Agent Trace synchronization with explicit-false opt-out plus doctor readiness reporting: a newly created repo-local config supplies the explicit `true` opt-in while omitted runtime values resolve to `false`; the existing `sce sync` command is launched once through the current executable after local persistence, with detached null-standard-stream behavior, fail-open startup, canonical managed-block proof of hook readiness, stable text/JSON enabled/disabled/not-ready/not-applicable states, no daemon/queue/high-frequency trigger, and retryability through manual sync and control-plane cursor authority) - `context/cli/sync-command.md` (the top-level `sce sync` command: repository-scoped Agent Trace storage resolution, WorkOS-authenticated three-stream control-plane synchronization (`messages`, `parts`, `agent_traces`; `diff_traces` stays local with a `streams.diffTraces` compatibility no-op entry) through the sync-owned consumer-typed `services::sync::progress` reporter contract with sync-owned events, its generic/no-op contract and `indicatif` presentation adapter for aligned stderr progress with independent stream completion, explicit successful finalization, JSON stdout silence, and rejection of the removed `sce trace` command group) - `context/cli/async-persistence-guarantees.md` (verified guarantees and named limits of the directly awaited Turso persistence migration — exclusive `&mut` transactions, deferred rollback, credential-rotation and terminal-sync behavior, lease/descriptor protection — plus PR1–PR4 staging and the PR3 cleanup ledger) -- `context/cli/agent-trace-sync-command.md` (composed local-to-control-plane `sce sync` architecture: the `hooks/plugins → repository Agent Trace DB → AgentTraceExportReader → sce sync → HTTPS + WorkOS Bearer → control plane` data flow, the `sce auth login` / `cd ` / `sce sync` user flow, the no-local-cursor/no-`agent-trace-sync.db`/no-Turso-Sync/no-`BridgeLock`/no-local-DWH invariants, the four-local-capture-streams versus three-remote-streams boundary with its `diff_traces` release gate, and `401`/`409`/ambiguous-batch-failure recovery semantics) +- `context/cli/agent-trace-sync-command.md` (composed local-to-control-plane `sce sync` architecture: the `hooks/plugins → repository Agent Trace DB → AgentTraceExportReader → sce sync → HTTPS + WorkOS Bearer → control plane` data flow, the `sce auth login` / `cd ` / `sce sync` user flow, the no-local-cursor/no-`agent-trace-sync.db`/no-Turso-Sync/no-`BridgeLock`/no-local-DWH invariants, the four-local-capture-streams versus three-remote-streams boundary with its `diff_traces` release gate, the optional `commitPatchId` field on the existing `agent_traces` stream with its Control Plane release dependency and no-backfill limit, 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 config-file/default `log_to_file`, `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`, `agent_trace.auto_sync` resolution with omitted-value fallback `false`, explicit-false opt-out, and setup-written explicit `true` bootstrap behavior for the post-commit trigger boundary, 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` (method-less marker), `GitOps`/`ProcessGitOps` (command execution and availability), compile-time-typed borrowed AppContext wiring with scoped borrowing across directly awaited commands 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 async 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) @@ -94,7 +94,7 @@ Feature/domain context: - `context/sce/telemetry-credential.md` (telemetry-only OpenBao credential: typed Control Plane issuance client, save-on-issue, single-row encrypted `auth.db` storage that never mints on lookup, deterministic auth-login `telemetry_provisioning` status/message rendering with secret-free unavailable warnings, production issue-and-save after WorkOS persistence on fresh/valid-stored/renewed paths, logout deletion alongside WorkOS credentials, issuing-token identity binding with session-ownership checks, repository-to-workspace enrollment bindings, local deletion not revoking the OpenBao token, and its use as the managed OTLP bearer source, the hidden `telemetry-headers` helper and its OS-user trust boundary with no endpoint-pinning claim, and Claude-native telemetry setup, rotation, logout, conflict handling, and override limits) - `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 additive `source_instance_id`, `claude_model_state`, mutation-trace, and `007_commit_patch_id` (nullable `agent_traces.commit_patch_id`, setup-only upgrade) migrations, 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, parts, and the non-exported exact-scope Claude model-state register, repository-level recent diff-trace reads without checkout filtering including persisted hunk-model plus canonical touched-line-session enrichment for structured rows, async 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-db-schema.md` (per-table columns, lookup indexes, and `updated_at` triggers of the repository-scoped Agent Trace database, including the nullable `agent_traces.commit_patch_id` column) -- `context/sce/agent-trace-export-readers.md` (implemented `AgentTraceExportReader<'a>` in `cli/src/services/agent_trace_export/mod.rs`: the read-only async 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`; `sce sync` consumes three of the four readers and `read_diff_traces_after` is a retained compatibility surface; no local sync cursor, no `agent-trace-sync.db`, no Turso Sync, no ETL, no DWH) +- `context/sce/agent-trace-export-readers.md` (implemented `AgentTraceExportReader<'a>` in `cli/src/services/agent_trace_export/mod.rs`: the read-only async 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, with `agent_traces` rows carrying an always-present nullable `commitPatchId`; composes directly with `ResolvedAgentTraceStorage` without owning `source_instance_id`; `sce sync` consumes three of the four readers and `read_diff_traces_after` is a retained compatibility surface; no local sync cursor, no `agent-trace-sync.db`, no Turso Sync, no ETL, no DWH) - `context/sce/agent-trace-core-schema-migrations.md` (historical reference for removed local DB schema bootstrap behavior; T03 now implements the actual local DB with migrations) - `context/sce/agent-trace-retry-queue-observability.md` (inactive local-hook retry path plus historical retry/metrics reference) - `context/sce/agent-trace-local-hooks-mvp-contract-gap-matrix.md` (T01 Local Hooks MVP production contract freeze and deterministic gap matrix for `agent-trace-local-hooks-production-mvp`) diff --git a/context/plans/agent-trace-commit-patch-id.md b/context/plans/agent-trace-commit-patch-id.md index c996805be..4e2a8d711 100644 --- a/context/plans/agent-trace-commit-patch-id.md +++ b/context/plans/agent-trace-commit-patch-id.md @@ -10,17 +10,17 @@ This extends existing `post-commit` → `RepositoryAgentTraceDb` → `AgentTrace How this plan is proven complete. Each criterion is observable and names the check that proves it. `/validate` runs these checks; no task in the stack performs final validation. -- [ ] AC1: For a normal non-merge commit, the captured fingerprint equals the first column of `git patch-id --stable` applied to the commit's entire canonical binary-capable Git patch (the documented pinned invocation); changing only the commit message, or replaying an equivalent patch onto another parent, does not change the fingerprint; user Git diff configuration (prefixes, path quoting, diff algorithm, context, color, external diff/textconv) does not change it. +- [x] AC1: For a normal non-merge commit, the captured fingerprint equals the first column of `git patch-id --stable` applied to the commit's entire canonical binary-capable Git patch (the documented pinned invocation); changing only the commit message, or replaying an equivalent patch onto another parent, does not change the fingerprint; user Git diff configuration (prefixes, path quoting, diff algorithm, context, color, external diff/textconv) does not change it. - Validate: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id` using temporary Git repositories covering commit-message amend, equivalent cherry-pick/rebase, root commit, and user diff-configuration variations (guard with `-- --list` that the filter matches at least one test). -- [ ] AC2: Empty commits and multi-parent merge commits yield a `None` fingerprint; binary-only and rename-only commits produce deterministic fingerprints. A failed, malformed-output, oversized, or timed-out Git subprocess returns an error (surfaced as a sanitized warning) without panicking, leaking a child process, or blocking indefinitely, and the original Agent Trace row is still persisted with `commit_patch_id IS NULL`. +- [x] AC2: Empty commits and multi-parent merge commits yield a `None` fingerprint; binary-only and rename-only commits produce deterministic fingerprints. A failed, malformed-output, oversized, or timed-out Git subprocess returns an error (surfaced as a sanitized warning) without panicking, leaking a child process, or blocking indefinitely, and the original Agent Trace row is still persisted with `commit_patch_id IS NULL`. - Validate: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id` with binary, rename, empty, merge, error, malformed-output, size-limit, and timeout cases; assert `None` for empty/merge and an error/diagnostic for actual failures. -- [ ] AC3: An existing repository Agent Trace database at migration `006_mutation_trace_health_invariant` upgrades through `sce setup` to `007_commit_patch_id`, retaining existing trace rows, IDs, URLs, and `trace_json`; old rows have `commit_patch_id IS NULL`. Repeated setup is idempotent. Hook runtime never runs migrations and reports `Run 'sce setup'.` guidance for a pre-007 schema; `sce doctor` reports the schema as not ready until setup runs. +- [x] AC3: An existing repository Agent Trace database at migration `006_mutation_trace_health_invariant` upgrades through `sce setup` to `007_commit_patch_id`, retaining existing trace rows, IDs, URLs, and `trace_json`; old rows have `commit_patch_id IS NULL`. Repeated setup is idempotent. Hook runtime never runs migrations and reports `Run 'sce setup'.` guidance for a pre-007 schema; `sce doctor` reports the schema as not ready until setup runs. - Validate: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id` against a pre-007 database fixture, inspecting `PRAGMA table_info(agent_traces)`, the `__sce_migrations` ledger, unchanged rows, and hook/doctor readiness results. -- [ ] AC4: Successful post-commit Agent Trace insertion persists the exact captured `commit_id` and its nullable `commit_patch_id` in the same SQL insert, without changing or reserializing the Agent Trace JSON for fingerprint purposes. +- [x] AC4: Successful post-commit Agent Trace insertion persists the exact captured `commit_id` and its nullable `commit_patch_id` in the same SQL insert, without changing or reserializing the Agent Trace JSON for fingerprint purposes. - Validate: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id` asserting local database values for successful and unavailable fingerprints and byte-identical `trace_json` versus the pre-change serialization path. -- [ ] AC5: `AgentTraceExportReader::read_agent_traces_after()` returns the stored optional patch ID unchanged; serialized export rows contain `commitPatchId` (string or `null`) and preserve existing fields, `traceJson` bytes, and `sourceRowId` ordering. +- [x] AC5: `AgentTraceExportReader::read_agent_traces_after()` returns the stored optional patch ID unchanged; serialized export rows contain `commitPatchId` (string or `null`) and preserve existing fields, `traceJson` bytes, and `sourceRowId` ordering. - Validate: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id` asserting query mapping, JSON serialization, and `traceJson` byte preservation for rows with and without fingerprints. -- [ ] AC6: `sce sync` and post-commit auto-sync send `commitPatchId` on the existing `agent_traces` batch payload, without introducing a fourth remotely uploaded stream, a new cursor, or changes to `/state`, retry, or conflict behavior. Previously acknowledged rows are not replayed because of this migration. +- [x] AC6: `sce sync` and post-commit auto-sync send `commitPatchId` on the existing `agent_traces` batch payload, without introducing a fourth remotely uploaded stream, a new cursor, or changes to `/state`, retry, or conflict behavior. Previously acknowledged rows are not replayed because of this migration. - Validate: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml sync_uploads_only_three_streams_and_reports_zero_upload_diff_traces` plus a captured local HTTP ingestion request assertion for `rows[*].commitPatchId`, cursor advancement, and no replay of rows at or below the server cursor. ### Full validation @@ -109,14 +109,53 @@ How this plan is proven complete. Each criterion is observable and names the che - Deviation: the fingerprint is computed in the production wrapper and the `_with` seam receives the resolved `Option<&str>`; persistence is asserted through the seam's capturing insert closure (real-DB `Some`/`None` insert coverage already exists from T01). - Context synchronization: synced -- [ ] T04: `Export commitPatchId through the existing agent_traces sync stream` (status:todo) +- [x] T04: `Export commitPatchId through the existing agent_traces sync stream` (status:done) - Task ID: T04 - Scope: In — `AgentTraceAgentTraceExportRow.commit_patch_id: Option`, `SELECT_AGENT_TRACES_AFTER_SQL` and `agent_trace_export_row_from_turso`, JSON serialization, captured HTTP sync assertions in the `sync.rs` tests, and the `agent-trace-export-readers.md` / `agent-trace-sync-command.md` updates (release dependency, no backfill, upgrade/downgrade). Out — sync-engine rewrite, Control Plane migration/API code, historical backfill. - Dependencies: T01, T03 - Done when: The stored value is emitted as `commitPatchId` in the existing `agent_traces` ingestion batch; strings and nulls round-trip; `traceJson` remains raw; the three-stream protocol and authoritative server cursors are unchanged and pre-acknowledged rows are not resent; deployment and backfill limitations are documented. - Verify: `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id`; `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml sync_uploads_only_three_streams_and_reports_zero_upload_diff_traces`. - - Context synchronization: pending + - Completed: 2026-10-15 + - Files changed: `cli/src/services/agent_trace_export/mod.rs`, `cli/src/services/sync/sync.rs`, `context/sce/agent-trace-export-readers.md`, `context/cli/agent-trace-sync-command.md`, `context/plans/agent-trace-commit-patch-id.md` + - Result: `AgentTraceAgentTraceExportRow` gained `commit_patch_id: Option` (serialized as always-present `commitPatchId`, string or `null`); `SELECT_AGENT_TRACES_AFTER_SQL` selects `commit_patch_id` as column 7 and `agent_trace_export_row_from_turso` maps it. New test `read_agent_traces_after_exports_commit_patch_id_with_raw_trace_json` covers `Some`/`None` mapping, JSON `commitPatchId` string/explicit `null`, raw non-canonical `traceJson` bytes, and `sourceRowId` order. In `sync.rs` `contract_tests`, `start_control_plane` now delegates to `start_control_plane_with_agent_traces_cursor(i64)` and client construction moved into a `test_client` helper; new test `sync_sends_commit_patch_id_on_agent_traces_batch_without_replaying_acknowledged_rows` seeds three traces with server `agentTraces` cursor 1 and asserts the captured HTTP batch sends only rows 2–3 on the `agent_traces` stream with `commitPatchId` string and `null`, raw `traceJson`, and cursor advancement 1 → 3. Per user instruction, doc comments on the export-row DTOs, `AgentTraceExportReader`, and its `read_*_after` methods were removed; no new code comments were added. Context docs describe the export field, release dependency, no-backfill behavior, and upgrade/downgrade. + - Verify outcome: `... test commit_patch_id` passed (21 passed, 0 failed; 2 new + 19 from T01–T03, so the filter is non-empty); `... test sync_uploads_only_three_streams_and_reports_zero_upload_diff_traces` passed (1 passed). Additional: `... test sync` passed (37 passed); `... clippy --manifest-path cli/Cargo.toml --all-targets -- -D warnings` clean; `cargo fmt` applied. + - Context impact: localized — `context/sce/agent-trace-export-readers.md` (optional `commitPatchId` export field) and `context/cli/agent-trace-sync-command.md` (field on existing `agent_traces` stream, unchanged cursor protocol, Control Plane release dependency, no backfill, upgrade/downgrade) updated. + - Deviation: none. + - Context synchronization: synced ## Open questions None. The plan limits implementation to the SCE CLI and uses the previously agreed optional `commitPatchId` contract; the Control Plane rollout dependency, setup-required upgrade window, and lack of historical backfill are explicit constraints rather than unresolved design questions. + +## Validation Report + +**Status:** validated +**Date:** 2026-10-15 + +### Commands run + +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml` -> exit 0 (606 passed, 0 failed) +- `nix flake check` -> exit 0 (cli-clippy, cli-fmt, cli-tests: all checks passed) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id -- --list` -> exit 0 (filter matches 21 tests) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml commit_patch_id` -> exit 0 (21 passed, 0 failed, 585 filtered out) +- `nix develop -c ./scripts/run-cli-cargo.sh test --manifest-path cli/Cargo.toml sync_uploads_only_three_streams_and_reports_zero_upload_diff_traces` -> exit 0 (1 passed) +- `rg -n 'dbg!|todo!|unimplemented!|eprintln!\("DEBUG|XXX'` over changed CLI source files -> exit 1 (no debug leftovers); `git status --short` shows no untracked temporary files + +### Success-criteria verification + +- [x] AC1: Canonical `git patch-id --stable` fingerprint, stable across message amend, equivalent cherry-pick, and user diff configuration -> `commit_patch_id_matches_canonical_invocation_for_root_and_normal_commits`, `commit_patch_id_is_stable_across_message_amend_and_equivalent_cherry_pick`, `commit_patch_id_ignores_user_diff_configuration`, `commit_patch_id_supports_sha256_repositories` passed +- [x] AC2: Empty/merge -> `None`; binary/rename deterministic; failures, malformed output, size limits, and timeouts return bounded errors, warn, and persist `NULL` -> `commit_patch_id_is_none_for_empty_and_merge_commits`, `commit_patch_id_is_deterministic_for_binary_and_rename_only_commits`, `commit_patch_id_reports_non_zero_git_exit`, `commit_patch_id_reports_malformed_patch_id_output`, `commit_patch_id_parser_rejects_malformed_output`, `commit_patch_id_enforces_patch_input_and_output_byte_limits`, `commit_patch_id_times_out_and_kills_both_git_processes`, `commit_patch_id_failure_logs_sanitized_warning_and_persists_null` passed +- [x] AC3: Pre-007 database upgrades through setup to `007_commit_patch_id`, preserving rows; idempotent; hook runtime and doctor refuse pre-007 schema -> `commit_patch_id_setup_upgrades_pre_007_database_and_preserves_rows`, `commit_patch_id_hook_runtime_and_doctor_refuse_pre_007_database` passed +- [x] AC4: Post-commit insert persists exact `commit_id` and nullable `commit_patch_id` without changing `trace_json` -> `commit_patch_id_is_attached_to_post_commit_insert_without_changing_trace_json`, `commit_patch_id_insert_persists_present_and_absent_values`, `commit_patch_id_does_not_mask_agent_trace_validation_or_persistence_failures` passed +- [x] AC5: Export reader returns stored patch ID; `commitPatchId` serialized as string or `null`; `traceJson` bytes and `sourceRowId` order preserved -> `read_agent_traces_after_exports_commit_patch_id_with_raw_trace_json` passed +- [x] AC6: Sync sends `commitPatchId` on existing `agent_traces` batch, three streams only, cursor advances, acknowledged rows not replayed -> `sync_uploads_only_three_streams_and_reports_zero_upload_diff_traces` and `sync_sends_commit_patch_id_on_agent_traces_batch_without_replaying_acknowledged_rows` passed + +### Failed checks and follow-ups + +- None. + +### Residual risks + +- Release dependency: the Control Plane receiver must accept and persist optional `commitPatchId` before this SCE export ships; acknowledged rows are never re-sent and there is no historical backfill. +- After upgrading, repository hook writes fail open until `sce setup` reruns to apply migration `007`; older CLI binaries may reject the newer migration ledger on downgrade. +- Setup-path migration parity is covered through the private storage open functions rather than an end-to-end `sce setup` invocation (T01 deviation). diff --git a/context/sce/agent-trace-export-readers.md b/context/sce/agent-trace-export-readers.md index 72fb87c96..4bd4f2a00 100644 --- a/context/sce/agent-trace-export-readers.md +++ b/context/sce/agent-trace-export-readers.md @@ -50,6 +50,8 @@ and validates, per returned row before returning: Each stream has an owned `serde::Serialize` export-row DTO (`AgentTraceMessageExportRow`, `AgentTracePartExportRow`, `AgentTraceDiffTraceExportRow`, `AgentTraceAgentTraceExportRow`) with `#[serde(rename_all = "camelCase")]` matching the shipped control-plane ingestion contract; `sourceRowId` is the local `id` unmodified. `post_commit_patch_intersections` is not exported by any reader method. +`AgentTraceAgentTraceExportRow` carries `commit_patch_id: Option`, read from the nullable `agent_traces.commit_patch_id` column (migration `007_commit_patch_id`, see [agent-trace-db.md](agent-trace-db.md)) and returned unchanged. It always serializes as `commitPatchId`: a lowercase hex Git patch ID string, or `null` when the post-commit fingerprint was unavailable (empty commit, merge commit, helper failure) or the row predates migration `007`. The field is never omitted, and adding it does not change `traceJson` bytes, the other existing fields, or `sourceRowId` ordering. + ## Compatibility window The reader still exposes all four `read_*_after` methods, but `sce sync` consumes only three. `read_diff_traces_after` and `AgentTraceDiffTraceExportRow` are deferred-cleanup surfaces with no active caller: `diff_traces` is a local-only capture stream that is no longer uploaded. They keep their signature, validation, and serialization until the compatibility-removal phase described in [agent-trace-sync-command.md](../cli/agent-trace-sync-command.md#release-gate). From 51cb7b0559330d4065a5f58e63069d6d34a43fa8 Mon Sep 17 00:00:00 2001 From: davidabram Date: Thu, 15 Oct 2026 12:24:32 +0200 Subject: [PATCH 5/5] hooks: Gate Linux-only patch ID test helpers on target OS --- cli/src/services/hooks/commit_patch_id.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/cli/src/services/hooks/commit_patch_id.rs b/cli/src/services/hooks/commit_patch_id.rs index 10e18fc3a..c5fe61507 100644 --- a/cli/src/services/hooks/commit_patch_id.rs +++ b/cli/src/services/hooks/commit_patch_id.rs @@ -373,7 +373,9 @@ mod tests { }; const FAKE_GIT_DEADLINE: Duration = Duration::from_millis(500); + #[cfg(target_os = "linux")] const CHILD_EXIT_TIMEOUT: Duration = Duration::from_secs(10); + #[cfg(target_os = "linux")] const CHILD_EXIT_POLL_INTERVAL: Duration = Duration::from_millis(20); const FAKE_COMMIT_OID: &str = "1111111111111111111111111111111111111111"; const FAKE_PARENT_OID: &str = "2222222222222222222222222222222222222222"; @@ -696,7 +698,9 @@ mod tests { struct FakeGit { _dir: tempfile::TempDir, script: PathBuf, + #[cfg(target_os = "linux")] show_pid: PathBuf, + #[cfg(target_os = "linux")] patch_id_pid: PathBuf, } @@ -719,7 +723,9 @@ mod tests { FakeGit { _dir: dir, script, + #[cfg(target_os = "linux")] show_pid, + #[cfg(target_os = "linux")] patch_id_pid, } } @@ -735,6 +741,7 @@ mod tests { compute_commit_patch_id_with(&runner, Path::new("."), FAKE_COMMIT_OID).await } + #[cfg(target_os = "linux")] async fn assert_process_exited(pid_file: &Path) { let pid = std::fs::read_to_string(pid_file).expect("pid file"); let stat = PathBuf::from(format!("/proc/{}/stat", pid.trim()));