From 6898f14549903c499920dfd21ea68f2bc0138e9e Mon Sep 17 00:00:00 2001 From: David Abram Date: Mon, 10 Aug 2026 13:18:09 +0200 Subject: [PATCH 1/5] agent-trace-db: Add physical source-instance identity Repository-scoped databases previously identified only their logical repository, leaving no stable identity for an individual physical database. Add an additive migration and typed metadata to assign a generated identity once per database, with an atomic claim that makes concurrent initialization converge without overwriting valid values.\n\nCover fresh, repeated, reopened, independent, concurrent, mismatch, and baseline-migration cases with tests, and record the logical-versus-physical identity contract.\n\nPlan: agent-trace-source-instance-id (T01) Co-authored-by: SCE --- .../002_repository_source_instance_id.sql | 10 + cli/src/services/agent_trace_db/repository.rs | 299 ++++++++++++++++-- context/context-map.md | 2 +- context/glossary.md | 5 +- .../plans/agent-trace-source-instance-id.md | 95 ++++++ context/sce/agent-trace-db.md | 5 +- 6 files changed, 390 insertions(+), 26 deletions(-) create mode 100644 cli/migrations/agent-trace-repository/002_repository_source_instance_id.sql create mode 100644 context/plans/agent-trace-source-instance-id.md diff --git a/cli/migrations/agent-trace-repository/002_repository_source_instance_id.sql b/cli/migrations/agent-trace-repository/002_repository_source_instance_id.sql new file mode 100644 index 00000000..e47c54e9 --- /dev/null +++ b/cli/migrations/agent-trace-repository/002_repository_source_instance_id.sql @@ -0,0 +1,10 @@ +-- Adds a physical-database identity alongside the existing logical +-- repository_id. source_instance_id identifies one physical +-- agent-trace.db lineage; it is generated exactly once per database by +-- application code, never by SQL, and stays stable across reopen and +-- setup reruns. Existing/placeholder rows default to an empty string, +-- which application code recognizes as "not yet claimed" and replaces +-- through a concurrency-safe atomic claim. + +ALTER TABLE repository_metadata +ADD COLUMN source_instance_id TEXT NOT NULL DEFAULT ''; diff --git a/cli/src/services/agent_trace_db/repository.rs b/cli/src/services/agent_trace_db/repository.rs index 5e469efe..1fc90b9c 100644 --- a/cli/src/services/agent_trace_db/repository.rs +++ b/cli/src/services/agent_trace_db/repository.rs @@ -6,10 +6,16 @@ //! migrations), because repository-scoped databases are always created new; //! there is no incremental chain and no migration path from legacy //! checkout-scoped databases. Trace tables carry no `checkout_id` columns. +//! +//! `repository_metadata` additionally carries `source_instance_id`, a +//! physical-database identity independent of the logical `repository_id` +//! (added by the additive `002_repository_source_instance_id` migration and +//! generated by application code, never SQL). use std::path::PathBuf; use anyhow::Result; +use uuid::Uuid; use crate::{ generated_migrations, @@ -26,7 +32,7 @@ use super::{ const REPOSITORY_AGENT_TRACE_SCHEMA_SETUP_GUIDANCE: &str = "Run 'sce setup'."; const SELECT_REPOSITORY_METADATA_SQL: &str = - "SELECT repository_id FROM repository_metadata WHERE id = 1"; + "SELECT repository_id, source_instance_id FROM repository_metadata WHERE id = 1"; const SELECT_SQLITE_OBJECT_SQL: &str = "SELECT name FROM sqlite_master WHERE type = ?1 AND name = ?2 LIMIT 1"; const RECORD_REPOSITORY_SCHEMA_MIGRATION_SQL: &str = @@ -47,6 +53,41 @@ const INSERT_REPOSITORY_METADATA_SQL: &str = "INSERT INTO repository_metadata (id, repository_id) VALUES (1, ?1) ON CONFLICT (id) DO NOTHING"; +/// Atomically claims `source_instance_id` for the single metadata row: it +/// only ever replaces an empty placeholder, so a losing racer's candidate is +/// silently discarded (the `UPDATE` affects zero rows) and an already-valid +/// stored value is never overwritten. +const CLAIM_SOURCE_INSTANCE_ID_SQL: &str = + "UPDATE repository_metadata SET source_instance_id = ?1 WHERE id = 1 AND source_instance_id = ''"; + +/// Physical database identity for one repository-scoped Agent Trace DB, +/// alongside the existing logical repository identity. +/// +/// `repository_id` identifies the logical Git repository this database is +/// scoped to. `source_instance_id` identifies this one physical database +/// file's lineage: it is generated once per physical database, independent +/// of `repository_id`, remote URL, checkout ID, filesystem path, hostname, or +/// user/workspace identity, and stays stable across reopen and repeated +/// `sce setup` runs. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct RepositoryMetadata { + pub repository_id: String, + pub source_instance_id: String, +} + +/// Generate a new candidate source-instance identity. +/// +/// Callers must not assume the result is UUID-shaped; `is_valid_source_instance_id` +/// only requires a non-empty (once trimmed) value. +pub fn generate_source_instance_id() -> String { + Uuid::new_v4().to_string() +} + +/// Whether a stored or candidate value is a usable source-instance identity. +pub fn is_valid_source_instance_id(value: &str) -> bool { + !value.trim().is_empty() +} + /// Repository-scoped Agent Trace database configuration. pub struct RepositoryAgentTraceDbSpec; @@ -113,30 +154,72 @@ impl RepositoryAgentTraceDb { Ok(!rows.is_empty()) } - /// Seed repository metadata on first initialization and validate it on - /// every open. + /// Seed repository metadata on first initialization, atomically claim a + /// `source_instance_id` for this physical database if none is set yet, + /// and validate the result on every open. /// /// The stored `repository_id` must match the resolved repository ID for /// this database path; a mismatch means the file does not belong to the /// resolved repository and is an error rather than a write target. - pub fn verify_or_initialize_repository_metadata(&self, repository_id: &str) -> Result<()> { + /// `source_instance_id` is claimed with a concurrency-safe `UPDATE ... + /// WHERE source_instance_id = ''`: concurrent first opens generate their + /// own candidate, but only one claim can affect the row, so every caller + /// re-reads the row afterward and returns whichever value actually won. + pub fn verify_or_initialize_repository_metadata( + &self, + repository_id: &str, + ) -> Result { self.execute(INSERT_REPOSITORY_METADATA_SQL, (repository_id,))?; - let stored = self.query_map(SELECT_REPOSITORY_METADATA_SQL, (), |row| { - row.get::(0).map_err(Into::into) - })?; + let Some((stored_repository_id, source_instance_id)) = + self.select_repository_metadata_row()? + else { + anyhow::bail!( + "repository Agent Trace DB metadata is missing its repository ID row. \ + {REPOSITORY_AGENT_TRACE_SCHEMA_SETUP_GUIDANCE}" + ); + }; - match stored.first() { - Some(stored_repository_id) if stored_repository_id == repository_id => Ok(()), - Some(stored_repository_id) => anyhow::bail!( + if stored_repository_id != repository_id { + anyhow::bail!( "repository Agent Trace DB metadata mismatch: stored repository ID \ {stored_repository_id} does not match resolved repository ID {repository_id}" - ), - None => anyhow::bail!( - "repository Agent Trace DB metadata is missing its repository ID row. \ - {REPOSITORY_AGENT_TRACE_SCHEMA_SETUP_GUIDANCE}" - ), + ); + } + + if is_valid_source_instance_id(&source_instance_id) { + return Ok(RepositoryMetadata { + repository_id: stored_repository_id, + source_instance_id, + }); } + + let candidate = generate_source_instance_id(); + self.execute(CLAIM_SOURCE_INSTANCE_ID_SQL, (candidate.as_str(),))?; + + let (final_repository_id, final_source_instance_id) = + self.select_repository_metadata_row()?.ok_or_else(|| { + anyhow::anyhow!( + "repository Agent Trace DB metadata row disappeared after \ + source-instance-id claim" + ) + })?; + + if !is_valid_source_instance_id(&final_source_instance_id) { + anyhow::bail!("repository Agent Trace DB failed to establish a source-instance ID"); + } + + Ok(RepositoryMetadata { + repository_id: final_repository_id, + source_instance_id: final_source_instance_id, + }) + } + + fn select_repository_metadata_row(&self) -> Result> { + let rows = self.query_map(SELECT_REPOSITORY_METADATA_SQL, (), |row| { + Ok((row.get::(0)?, row.get::(1)?)) + })?; + Ok(rows.into_iter().next()) } /// Insert a diff trace payload into the repository-scoped `diff_traces` @@ -302,8 +385,12 @@ mod tests { .expect("migration metadata query should succeed"); assert_eq!( applied_ids, - vec![String::from("001_repository_schema")], - "repository DBs should be initialized from exactly one schema file" + vec![ + String::from("001_repository_schema"), + String::from("002_repository_source_instance_id"), + ], + "repository DBs should be initialized from the baseline schema plus \ + its additive source-instance-id migration" ); db.ensure_schema_ready_for_hooks() @@ -340,17 +427,108 @@ mod tests { let repository_id = "a".repeat(64); let db = RepositoryAgentTraceDb::new_at(&db_path).expect("repository DB should open"); - db.verify_or_initialize_repository_metadata(&repository_id) + let first = db + .verify_or_initialize_repository_metadata(&repository_id) .expect("first metadata initialization should succeed"); - db.verify_or_initialize_repository_metadata(&repository_id) + assert_eq!(first.repository_id, repository_id); + assert!( + is_valid_source_instance_id(&first.source_instance_id), + "freshly initialized metadata should have a non-empty source-instance ID" + ); + + let repeated = db + .verify_or_initialize_repository_metadata(&repository_id) .expect("repeated validation with the same repository ID should succeed"); + assert_eq!( + repeated.source_instance_id, first.source_instance_id, + "repeated initialization must not regenerate the source-instance ID" + ); drop(db); let reopened = RepositoryAgentTraceDb::open_without_migrations_at(&db_path) .expect("repository DB should reopen"); - reopened + let reopened_metadata = reopened .verify_or_initialize_repository_metadata(&repository_id) .expect("reopen validation with the matching repository ID should succeed"); + assert_eq!( + reopened_metadata.source_instance_id, first.source_instance_id, + "source-instance ID must be stable across reopen" + ); + + remove_test_db(&db_path); + } + + #[test] + fn source_instance_id_is_not_derived_from_repository_id_and_diverges_across_independent_dbs() { + let first_db_path = unique_test_db_path("source-instance-first"); + let second_db_path = unique_test_db_path("source-instance-second"); + let repository_id = "a".repeat(64); + + let first_db = RepositoryAgentTraceDb::new_at(&first_db_path) + .expect("first repository DB should open"); + let second_db = RepositoryAgentTraceDb::new_at(&second_db_path) + .expect("second repository DB should open"); + + let first_metadata = first_db + .verify_or_initialize_repository_metadata(&repository_id) + .expect("first DB metadata initialization should succeed"); + let second_metadata = second_db + .verify_or_initialize_repository_metadata(&repository_id) + .expect("second DB metadata initialization should succeed"); + + assert_eq!(first_metadata.repository_id, second_metadata.repository_id); + assert_ne!( + first_metadata.source_instance_id, second_metadata.source_instance_id, + "two independently created databases for the same logical repository \ + must diverge in source-instance ID" + ); + assert_ne!( + first_metadata.source_instance_id, repository_id, + "source-instance ID must not be derived from repository_id" + ); + + remove_test_db(&first_db_path); + remove_test_db(&second_db_path); + } + + #[test] + fn concurrent_initialization_converges_on_one_source_instance_id() { + use std::sync::Arc; + + let db_path = unique_test_db_path("concurrent-source-instance"); + let repository_id = "a".repeat(64); + + // Create the schema up front so both threads race only on the + // metadata claim, not schema creation. + RepositoryAgentTraceDb::new_at(&db_path).expect("repository DB should open"); + + let db_path = Arc::new(db_path); + let handles: Vec<_> = (0..4) + .map(|_| { + let db_path = Arc::clone(&db_path); + let repository_id = repository_id.clone(); + std::thread::spawn(move || { + let db = RepositoryAgentTraceDb::open_without_migrations_at(&*db_path) + .expect("repository DB should reopen for concurrent claim"); + db.verify_or_initialize_repository_metadata(&repository_id) + .expect("concurrent metadata initialization should succeed") + }) + }) + .collect(); + + let results: Vec = handles + .into_iter() + .map(|handle| handle.join().expect("worker thread should not panic")) + .collect(); + + let winning_id = results[0].source_instance_id.clone(); + for result in &results { + assert_eq!( + result.source_instance_id, winning_id, + "all concurrent initializations must converge on one persisted \ + source-instance ID" + ); + } remove_test_db(&db_path); } @@ -362,7 +540,8 @@ mod tests { let other_repository_id = "b".repeat(64); let db = RepositoryAgentTraceDb::new_at(&db_path).expect("repository DB should open"); - db.verify_or_initialize_repository_metadata(&stored_repository_id) + let stored = db + .verify_or_initialize_repository_metadata(&stored_repository_id) .expect("first metadata initialization should succeed"); let error = db @@ -376,6 +555,84 @@ mod tests { assert!(message.contains(&stored_repository_id)); assert!(message.contains(&other_repository_id)); + let unchanged = db + .verify_or_initialize_repository_metadata(&stored_repository_id) + .expect("re-validating with the original repository ID should still succeed"); + assert_eq!( + unchanged.source_instance_id, stored.source_instance_id, + "a rejected mismatched claim must not alter the stored source-instance ID" + ); + + remove_test_db(&db_path); + } + + #[test] + fn baseline_only_fixture_migrates_and_gets_a_stable_source_instance_id() { + let db_path = unique_test_db_path("baseline-only-fixture"); + let repository_id = "a".repeat(64); + + // Simulate a database created before migration 002 existed: build the + // pre-002 `repository_metadata` shape (no `source_instance_id` + // column) and record only migration 001 as applied, so opening with + // the current embedded migration set exercises the real 001-applied, + // 002-pending upgrade path. + let baseline_only = RepositoryAgentTraceDb::open_without_migrations_at(&db_path) + .expect("baseline-only repository DB should open"); + baseline_only + .execute( + "CREATE TABLE IF NOT EXISTS __sce_migrations ( + id TEXT PRIMARY KEY, + applied_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) +)", + (), + ) + .expect("migration metadata table should create"); + baseline_only + .execute( + "CREATE TABLE IF NOT EXISTS repository_metadata ( + id INTEGER PRIMARY KEY CHECK (id = 1), + repository_id TEXT NOT NULL, + created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) +)", + (), + ) + .expect("baseline repository_metadata table should create"); + baseline_only + .execute( + "INSERT INTO __sce_migrations (id) VALUES ('001_repository_schema')", + (), + ) + .expect("baseline migration record should insert"); + baseline_only + .execute( + "INSERT INTO repository_metadata (id, repository_id) VALUES (1, ?1)", + (repository_id.as_str(),), + ) + .expect("baseline metadata row should seed"); + drop(baseline_only); + + let migrated = + RepositoryAgentTraceDb::new_at(&db_path).expect("repository DB should migrate to 002"); + let metadata = migrated + .verify_or_initialize_repository_metadata(&repository_id) + .expect("metadata initialization after migrating from baseline should succeed"); + assert_eq!(metadata.repository_id, repository_id); + assert!( + is_valid_source_instance_id(&metadata.source_instance_id), + "migrating from a baseline-only fixture should populate a valid source-instance ID" + ); + drop(migrated); + + let reopened = RepositoryAgentTraceDb::open_without_migrations_at(&db_path) + .expect("migrated repository DB should reopen"); + let reopened_metadata = reopened + .verify_or_initialize_repository_metadata(&repository_id) + .expect("reopen after migration should succeed"); + assert_eq!( + reopened_metadata.source_instance_id, metadata.source_instance_id, + "source-instance ID populated during migration must remain stable across reopen" + ); + remove_test_db(&db_path); } diff --git a/context/context-map.md b/context/context-map.md index 9bea165a..c4257b60 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -61,7 +61,7 @@ Feature/domain context: - `context/sce/local-db.md` (implemented `cli/src/services/local_db/mod.rs` local database spec with `LocalDb = TursoDb`, canonical local DB path resolution, zero local migrations, and inherited retry-backed blocking `execute`/`query`/`query_map` methods using the shared Turso adapter) - `context/sce/shared-turso-db.md` (current shared `cli/src/services/db/mod.rs` Turso database infrastructure seam, including `DbSpec`, generic `TursoDb`, encrypted `EncryptedTursoDb`, build-time generated migration constants from `cli/build.rs`/Cargo `OUT_DIR`, config-driven constructor/open-connect retry via `run_with_retry_sync`, no-migration `TursoDb::open_without_migrations()` / explicit-path `open_without_migrations_at(path)` for hot runtime paths, migration-running `new()` / explicit-path `new_at(path)` / `run_migrations()` with per-database `__sce_migrations` tracking, config-driven operation retry for `execute`/`query`/`query_values`/`query_map` with a `<= 2_000ms` default query failure budget, raw-value row fetching for deterministic operator-facing rendering, row-mapping excluded from retry, generic embedded migration execution, non-mutating `migration_metadata_problems()` and `ensure_schema_ready(setup_guidance)` readiness methods on `TursoDb`, and concrete wrappers for `LocalDb`, `AuthDb`, plus `RepositoryAgentTraceDb`) - `context/sce/auth-db.md` (encrypted `AuthDb = EncryptedTursoDb` adapter, canonical `/sce/auth.db` path, build-time generated `AUTH_MIGRATIONS` from `cli/migrations/auth/`, auth credential schema and updated-at trigger baseline, lifecycle setup/doctor integration, encrypted token-storage persistence, and `SCE_AUTH_DB_ENCRYPTION_KEY`/OS credential-store key handling) -- `context/sce/agent-trace-db.md` (implemented Agent Trace database adapter: the sole repository-scoped `RepositoryAgentTraceDb` backed by one fresh multi-statement schema file with `repository_metadata` validation, narrow concurrent-first-open repair for missing one-file baseline migration metadata after all required schema tables exist, no trace-table `checkout_id` columns, repository-level typed insert helpers for diff traces, post-commit intersections, Agent Trace rows, messages, and parts, repository-level recent diff-trace reads without checkout filtering, 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 the additive `source_instance_id` migration, with `repository_metadata` carrying both `repository_id` and a concurrency-safe atomic-claim `source_instance_id` (physical database identity, independent of `repository_id`), narrow concurrent-first-open repair for missing one-file baseline migration metadata after all required schema tables exist, no trace-table `checkout_id` columns, repository-level typed insert helpers for diff traces, post-commit intersections, Agent Trace rows, messages, and parts, repository-level recent diff-trace reads without checkout filtering, 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-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/glossary.md b/context/glossary.md index 67510f9a..2988023c 100644 --- a/context/glossary.md +++ b/context/glossary.md @@ -36,7 +36,8 @@ - `canonical OpenCode plugin registration source`: Shared Pkl-authored plugin-registration definition in `config/pkl/base/opencode.pkl`, re-exported from `config/pkl/renderers/common.pkl` as the canonical plugin list/path JSON consumed by OpenCode renderers before they emit generated `opencode.json` manifests; the current entries are `sce-bash-policy` and `sce-agent-trace`. - `checkout identity`: Stable UUIDv7 identifier assigned to a cloned repository or linked Git worktree, stored in `/sce/checkout-id` (never committed) and resolved via `git rev-parse --git-dir`. The identity is created or reused by `sce setup` through `AgentTraceDbLifecycle::setup()` and also auto-created by hook runtime when `sce setup` has not been run. Checkout identity is now diagnostic metadata for repository-scoped Agent Trace storage; it does not select the active DB and is not stored on Agent Trace rows. Any pre-migration per-checkout DB files at `/sce/agent-trace-{checkout_id}.db` are never touched by SCE and are no longer inspectable via the CLI (the checkout-scoped DB surface was removed by the `retire-legacy-agent-trace-db` plan). See `context/cli/checkout-identity.md`. - `repository identity`: Stable identity of a logical Git repository used to select the active repository-scoped Agent Trace DB path `/sce/repos//agent-trace.db` through the `agent_trace_storage` resolver. Resolved by `cli/src/services/repository_identity/` with precedence: explicit `agent_trace.repository_id` config value, then the URL of the configured Git remote (`agent_trace.repository_remote`, default `origin`), otherwise an actionable `.sce/config.json` error. Remote URLs canonicalize to a scheme-neutral, credential-free `host[:port]/path` form so equivalent SSH/SCP/HTTPS remotes converge, and the repository ID is `sha256("sce-repository-id-v1\0" + canonical_identity)` hex. Distinct from `checkout identity`, which stays per clone/worktree for diagnostics. See `context/cli/repository-identity.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 one fresh multi-statement schema file with `repository_metadata` 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 trace` status/list/shell flows resolve repository-scoped storage through `agent_trace_storage`. This is the sole Agent Trace DB adapter; the checkout-scoped adapter and the `sce trace --legacy` 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 trace` status/list/shell flows resolve repository-scoped storage through `agent_trace_storage`. This is the sole Agent Trace DB adapter; the checkout-scoped adapter and the `sce trace --legacy` inspection surface were removed by the `retire-legacy-agent-trace-db` plan. +- `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. `sce trace db list` now discovers checkouts by scanning `/sce/agent-trace-*.db` files on disk. `checkout_id`, `database_path`, and `last_seen` (from file mtime) are derived from the filesystem; `path` and `remote_url` are no longer rendered. 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 are `./plugins/sce-bash-policy.ts` and `./plugins/sce-agent-trace.ts`. 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`. - `root Biome contract`: Repository-root formatting/linting contract owned by `biome.json`, currently scoped only to `npm/**` and the shared `config/lib/**` plugin package root with package-local `node_modules/**` excluded; the canonical execution path is the root Nix dev shell (`nix develop -c biome ...`). @@ -71,7 +72,7 @@ - `encrypted Turso adapter`: Generic adapter seam in `cli/src/services/db/mod.rs` exposed as `EncryptedTursoDb`, structurally parallel to `TursoDb` (connection, tokio runtime bridge, spec typing). Its constructor resolves the encryption key via `encryption_key::get_or_create_encryption_key(&db_path, db_name)`, which derives a Turso-compatible 64-character hex key from non-empty `SCE_AUTH_DB_ENCRYPTION_KEY` env-secret text before falling back to OS credential-store keyring get-or-create behavior; credential-store default registration is guarded by stable `OnceLock` plus an atomic in-progress flag so errors or panics leave initialization retryable without mutex poisoning. The adapter enables Turso local encryption with strict `aegis256` cipher selection through `turso::EncryptionOpts`, wraps encrypted local open/connect in the default DB connection-open retry policy, and runs embedded migrations after retry has produced a connection; the adapter also exposes retry-backed synchronous `execute`, `query`, `query_map`, and `run_migrations` helpers with `__sce_migrations` tracking parity. - `auth DB adapter`: Module in `cli/src/services/auth_db/mod.rs` that defines `AuthDbSpec` and exposes `AuthDb` as an `EncryptedTursoDb` alias. It resolves the canonical `/sce/auth.db` path with `auth_db_path()`, keeps encryption mandatory with `SCE_AUTH_DB_ENCRYPTION_KEY` env-secret precedence before OS keyring fallback and no plaintext mode, and embeds ordered auth migrations where baseline SQL creates `auth_credentials` without `user_id`, with `updated_at`, and a trigger that auto-refreshes `updated_at` on row updates. Auth runtime token-storage is now wired through `cli/src/services/token_storage.rs`, which persists tokens via the `auth_credentials` table in the encrypted auth DB instead of a JSON file. - `AuthDbLifecycle`: Lifecycle provider in `cli/src/services/auth_db/lifecycle.rs` that implements `ServiceLifecycle` for encrypted auth DB setup/doctor integration. `diagnose` collects auth DB path health problems, `fix` bootstraps missing auth DB parent directory, and `setup` calls `AuthDb::new()`. Registered as `LifecycleProviderId::AuthDb` in the shared lifecycle catalog. -- `agent trace DB adapter`: Modules under `cli/src/services/agent_trace_db/` that define the sole repository-scoped `RepositoryAgentTraceDb = TursoDb` adapter (the checkout-scoped `AgentTraceDb`/`AgentTraceDbSpec` adapter and its 15-file migration chain were removed by the `retire-legacy-agent-trace-db` plan). The repository adapter uses one fresh `agent-trace-repository` schema file with `repository_metadata`, repository-level `diff_traces`, `post_commit_patch_intersections`, `agent_traces`, `messages`, and `parts` tables, no row-level `checkout_id`, typed parameterized insert helpers, and chronological recent `diff_traces` query/parse support. `AgentTraceDbLifecycle` initializes/checks repository-scoped storage through `agent_trace_storage`. +- `agent trace DB adapter`: Modules under `cli/src/services/agent_trace_db/` that define the sole repository-scoped `RepositoryAgentTraceDb = TursoDb` adapter (the checkout-scoped `AgentTraceDb`/`AgentTraceDbSpec` adapter and its 15-file migration chain were removed by the `retire-legacy-agent-trace-db` plan). The repository adapter uses the `agent-trace-repository` migration set (fresh baseline schema plus the additive `source_instance_id` migration) with `repository_metadata`, repository-level `diff_traces`, `post_commit_patch_intersections`, `agent_traces`, `messages`, and `parts` tables, no row-level `checkout_id`, typed parameterized insert helpers, and chronological recent `diff_traces` query/parse support. `AgentTraceDbLifecycle` initializes/checks repository-scoped storage through `agent_trace_storage`. - `structured patch service`: Pure synchronous Rust service in `cli/src/services/structured_patch.rs` that derives supported structured editor hook payloads into canonical `ParsedPatch` values. The current implemented source is Claude `PostToolUse` payloads for `Write` creates and `Edit` structured patches; wired into `sce hooks diff-trace` for Claude payload classification at intake (T04) and into `RepositoryAgentTraceDb::recent_diff_trace_patches` for post-commit structured payload parsing dispatch at read time (T05). - `Agent Trace SCE metadata`: Implementation-owned top-level metadata emitted by `build_agent_trace(...)` as `metadata.sce.version`; the value is sourced from the compiled `sce` CLI package version via `env!("CARGO_PKG_VERSION")`, is schema-validated with the rest of the payload, and is persisted in AgentTraceDb `agent_traces.trace_json` without changing the top-level Agent Trace payload/schema `version`. - `Agent Trace range content_hash`: Per-range `content_hash` emitted by `build_agent_trace(...)` inside every `ranges[]` entry as `murmur3:`, computed from the touched-line kind/content of the `post_commit_patch` or embedded-patch hunk used to emit that range while excluding positions, paths, metadata, and database IDs. diff --git a/context/plans/agent-trace-source-instance-id.md b/context/plans/agent-trace-source-instance-id.md new file mode 100644 index 00000000..6be3aa37 --- /dev/null +++ b/context/plans/agent-trace-source-instance-id.md @@ -0,0 +1,95 @@ +# Plan: agent-trace-source-instance-id + +## Change summary + +Give every repository-scoped `agent-trace.db` a second, independent identity alongside the existing logical `repository_id`: a `source_instance_id` that identifies one physical database lineage. Two independently created `agent-trace.db` files for the same logical repository (for example, two different machines cloning the same repo) must end up with different `source_instance_id` values, generated exactly once per physical database and stable across reopen, `sce setup` reruns, and process restarts. This is purely a local storage identity addition — no remote ingestion, sync, or DWH behavior is implemented or designed here. It recreates the useful part of the abandoned PR #186 without any of its retired architecture (no DWH, no `agent-trace-sync.db`, no ETL, no Turso Sync). + +The schema gains an additive migration (`002_repository_source_instance_id.sql`) on `repository_metadata`, defaulting existing/placeholder rows to an empty string. Application code — never SQL — replaces that placeholder with a generated identity through a concurrency-safe atomic claim, so concurrent SCE processes opening the same database converge on one winner instead of racing to overwrite each other. Repository metadata initialization becomes typed (`RepositoryMetadata { repository_id, source_instance_id }`) and is threaded through `ResolvedAgentTraceStorage` so setup, lifecycle, and hook-runtime callers all observe the same identity. High-frequency hook runtime keeps its existing no-migration boundary: it must never apply migration `002` itself, even though it is allowed to initialize `source_instance_id` once the column is already present from a prior `sce setup`. + +## Acceptance criteria + +- [ ] AC1: A fresh repository-scoped `agent-trace.db` receives a valid, non-empty `source_instance_id` on first initialization, and `repository_id` remains correct. + - Validate: `nix flake check` (runs the new repository-adapter unit tests covering fresh initialization). +- [ ] AC2: `source_instance_id` is stable across database reopen, repeated `verify_or_initialize_repository_metadata` calls (repeated `sce setup`), and is never derived from `repository_id`, remote URL, checkout ID, filesystem path, hostname, or user/workspace identity. + - Validate: `nix flake check` (repository-adapter reopen/repeat/independent-DB unit tests). +- [ ] AC3: Concurrent initialization of the same physical database converges on exactly one persisted `source_instance_id`; a losing racer's generated candidate is discarded in favor of the winner's stored value, and an already-valid `source_instance_id` is never overwritten. + - Validate: `nix flake check` (concurrent-initialization unit test in `cli/src/services/agent_trace_db/repository.rs`). +- [ ] AC4: `resolve_agent_trace_storage` (setup/lifecycle path) returns the same typed `RepositoryMetadata` that database verification/initialization produced, alongside the existing `db`/`db_path`/`checkout_id` fields. + - Validate: `nix flake check` (`agent_trace_storage` unit tests asserting `ResolvedAgentTraceStorage.metadata`). +- [ ] AC5: High-frequency hook runtime resolution never applies migration `002`: before `sce setup` (missing DB, or a baseline-only DB that has migration `001` but not `002`), hook resolution fails with the existing `sce setup` guidance and leaves the stored migration metadata unchanged; after `sce setup`, hook resolution succeeds and returns the same `RepositoryMetadata` setup produced. + - Validate: `nix flake check` (`agent_trace_storage` hook-runtime resolution unit tests: before-setup missing DB, before-setup baseline-only schema, after-setup parity). +- [ ] AC6: `sce setup` Agent Trace diagnostics report the source-instance ID alongside the existing repository ID line, without introducing workspace or remote-ingestion concepts. + - Validate: Inspect `format_repository_storage_setup_message` output in `cli/src/services/agent_trace_db/lifecycle.rs` and its covering test. +- [ ] AC7: A baseline-only fixture (only the original `001` repository schema, no `source_instance_id` column) migrates cleanly to `002`, gets a populated `source_instance_id`, and preserves that value across reopen. + - Validate: `nix flake check` (baseline-fixture migration unit test in `cli/src/services/agent_trace_db/repository.rs`). + +### Full validation + +- `nix flake check` + +### Context sync + +- `context/cli/agent-trace-storage.md` +- `context/sce/agent-trace-db.md` +- `context/context-map.md` (only if either linked domain-file summary needs a one-line update to stay accurate) + +## Constraints and non-goals + +- **In scope:** `cli/migrations/agent-trace-repository/002_repository_source_instance_id.sql`, `cli/src/services/agent_trace_db/repository.rs`, `cli/src/services/agent_trace_storage/mod.rs`, `cli/src/services/agent_trace_db/lifecycle.rs`, `cli/src/services/hooks/mod.rs` (hook-runtime storage call site only), and the durable context files listed above. +- **Out of scope:** `sce trace sync`, any HTTP/remote ingestion, control-plane changes, workspace resolution, WorkOS integration, ingestion cursors, DWH schema/ETL, derived `code_changes`, Turso Sync, `agent-trace-sync.db`, replica paths, bridge locks, and any change to `sce doctor`'s read-only diagnose surface (it does not write to the DB today and stays that way). +- **Constraints:** The migration must be additive and must not rewrite `001_repository_schema.sql`. `source_instance_id` must never be generated inside SQL. The existing separation between no-migration hook-runtime DB access and migration-running setup/lifecycle access must be preserved and, where it does not yet exist for this exact concern, established rather than blurred. +- **Non-goal:** Adding `workspace_id` or any user/host identity to the local repository Agent Trace DB. Designing remote ingestion, even though it is the eventual consumer of `source_instance_id`. + +## Assumptions + +- `is_valid_source_instance_id` validates "non-empty once trimmed" rather than strict UUID-v4 parsing, per the request's own instruction not to make downstream code depend on the identity being UUID-shaped forever; `generate_source_instance_id` still produces UUID v4 strings today. +- `sce doctor`'s diagnose path is read-only and does not open the DB for write today; per the request's "keep this diagnostic only" framing for `sce setup`, doctor's diagnose surface is left unchanged rather than being taught to also initialize/display `source_instance_id`. +- Hook-runtime initialization reuses the same `verify_or_initialize_repository_metadata` atomic-claim logic as setup once `ensure_schema_ready_for_hooks()` confirms exact migration-metadata parity (which guarantees the `source_instance_id` column already exists), rather than adding a separate schema/column introspection check — the readiness check already proves the precondition the request describes. + +## Task stack + +- [x] T01: `Add source-instance identity migration, type, and atomic-claim initialization` (status:done) + - Task ID: T01 + - Goal: Add migration `002_repository_source_instance_id.sql`, the typed `RepositoryMetadata { repository_id, source_instance_id }`, `generate_source_instance_id()`/`is_valid_source_instance_id()` helpers, and change `RepositoryAgentTraceDb::verify_or_initialize_repository_metadata` to return `Result` using the atomic `UPDATE ... WHERE source_instance_id = ''` claim pattern (claim with a generated candidate, always re-read, never overwrite an already-valid value). + - Boundaries (in/out of scope): In — `cli/migrations/agent-trace-repository/002_repository_source_instance_id.sql`, `cli/src/services/agent_trace_db/repository.rs` (type, helpers, method, and its unit tests: fresh DB, stable reopen, repeated initialization, mismatched repository ID unchanged, independent DBs diverge, concurrent initialization converges, migration-from-baseline-only-fixture). Out — any caller outside `repository.rs` (existing callers discard the method's return value via `?;` and keep compiling unchanged); storage-resolution/hook-runtime wiring (T02/T03). + - Dependencies: none + - Done when: The migration is additive and auto-discovered; `RepositoryMetadata`/`generate_source_instance_id`/`is_valid_source_instance_id` are exposed from `repository.rs`; `verify_or_initialize_repository_metadata` returns typed metadata with concurrency-safe initialization; all new/updated unit tests in `repository.rs` pass. + - Verification notes (commands or checks): `nix flake check` (covers `cargo test`/`cargo clippy`/`cargo fmt` for the CLI crate via the Crane check pipeline). + - Evidence: Added additive migration `cli/migrations/agent-trace-repository/002_repository_source_instance_id.sql` (`ALTER TABLE repository_metadata ADD COLUMN source_instance_id TEXT NOT NULL DEFAULT ''`; auto-discovered by `build.rs`, no `001` rewrite). Added `RepositoryMetadata { repository_id, source_instance_id }`, `generate_source_instance_id()` (UUID v4), `is_valid_source_instance_id()` (non-empty once trimmed) to `cli/src/services/agent_trace_db/repository.rs`. Rewrote `verify_or_initialize_repository_metadata` to return `Result`: seeds the row as before, checks `repository_id` for mismatch before touching `source_instance_id`, then claims via `UPDATE repository_metadata SET source_instance_id = ?1 WHERE id = 1 AND source_instance_id = ''` and always re-reads the row afterward so a losing racer's candidate is discarded and an already-valid value is never overwritten. Existing external callers (`agent_trace_storage/mod.rs`, `trace/discovery.rs`) already discarded the prior `Result<()>` via `?`/`.expect(...)` and needed no changes. Added/updated unit tests in `repository.rs`: `open_at_initializes_the_full_schema_from_one_migration` now asserts both `001_repository_schema` and `002_repository_source_instance_id` are applied; `repository_metadata_is_seeded_once_and_validated_on_reopen` asserts a valid non-empty `source_instance_id` that is stable across repeated calls and reopen; new `source_instance_id_is_not_derived_from_repository_id_and_diverges_across_independent_dbs`; new `concurrent_initialization_converges_on_one_source_instance_id` (4 threads racing the claim, all converge on one value); `mismatched_repository_metadata_errors_on_open` now asserts a rejected mismatch leaves the stored `source_instance_id` unchanged; new `baseline_only_fixture_migrates_and_gets_a_stable_source_instance_id` (hand-built pre-002 fixture with `001` recorded and no `source_instance_id` column, migrates cleanly on open, value stable across reopen). + - Verification run: `nix flake check` passed (`cli-fmt`, `cli-clippy` with `#![deny(clippy::pedantic)]`, and `cli-tests`, including all new/updated tests in `repository.rs`). One process note: the new migration SQL file had to be `git add`-staged for Nix's flake source view (git-tracked files only) to see it — the file is included in this task's change set. + +- [ ] T02: `Expose repository metadata through setup/lifecycle storage resolution` (status:todo) + - Task ID: T02 + - Goal: Add `metadata: RepositoryMetadata` to `ResolvedAgentTraceStorage` and thread it through the existing `resolve_agent_trace_storage`/`resolve_agent_trace_storage_at_state_root` (setup/lifecycle) resolution path, which keeps its existing fast-path-then-migrate fallback behavior. + - Boundaries (in/out of scope): In — `cli/src/services/agent_trace_storage/mod.rs` struct/field addition, threading the typed metadata out of `open_repository_db_concurrently_safe`, and updated/added unit tests asserting `ResolvedAgentTraceStorage.metadata`. Out — introducing the dedicated hook-runtime resolution entrypoint (T03); any call-site changes beyond what's needed to keep existing callers compiling against the new field. + - Dependencies: T01 + - Done when: `ResolvedAgentTraceStorage` carries `metadata`; setup/lifecycle resolution returns it populated from the same verification/initialization call that opens the DB; existing `agent_trace_storage` tests plus new metadata-presence assertions pass. + - Verification notes (commands or checks): `nix flake check`. + +- [ ] T03: `Add no-migration hook-runtime storage resolution and wire hooks to it` (status:todo) + - Task ID: T03 + - Goal: Add `resolve_agent_trace_storage_for_hook_runtime`/`_at_state_root` in `cli/src/services/agent_trace_storage/mod.rs` — no-migration open, schema-readiness check, narrow concurrent-first-open metadata repair only, and `source_instance_id` initialization gated on readiness already passing (never a migration fallback) — and switch `open_agent_trace_db_for_hook_runtime` in `cli/src/services/hooks/mod.rs` to call it instead of the setup/lifecycle resolver. + - Boundaries (in/out of scope): In — the new resolution functions and their unit tests (fails before setup on a missing DB, fails before setup on a baseline-only schema without recording migration `002`, succeeds after setup and returns the same `RepositoryMetadata` as the setup path), and the one hook call-site switch. Out — changing `sce trace status`/`sce trace db shell`/`sce doctor` DB-open call sites, which keep using their current resolution paths. + - Dependencies: T02 + - Done when: Hook runtime never runs migration `002` (or any migration) during normal operation; a baseline-only or missing schema fails hook resolution with the existing `sce setup` guidance; a fully set-up repository resolves successfully for hooks with metadata matching setup's; all new tests pass alongside existing hook-behavior tests. + - Verification notes (commands or checks): `nix flake check`. + +- [ ] T04: `Report source-instance ID in setup diagnostics` (status:todo) + - Task ID: T04 + - Goal: Extend `AgentTraceDbLifecycle::setup`'s reported message in `cli/src/services/agent_trace_db/lifecycle.rs` to include the source-instance ID alongside the existing repository ID line, sourced from the `RepositoryMetadata` now returned by storage resolution. + - Boundaries (in/out of scope): In — `RepositoryDatabaseSetup`/`format_repository_storage_setup_message` in `lifecycle.rs` and its covering test. Out — `sce doctor` diagnose output (read-only, unchanged per Assumptions). + - Dependencies: T02 + - Done when: `sce setup`'s Agent Trace messaging includes an `Agent Trace source-instance ID: ...` line using the resolved metadata, with no workspace/remote-ingestion wording introduced. + - Verification notes (commands or checks): `nix flake check`; inspect the formatted message in the lifecycle setup test. + +- [ ] T05: `Document the repository_id vs source_instance_id identity split` (status:todo) + - Task ID: T05 + - Goal: Update `context/cli/agent-trace-storage.md` and `context/sce/agent-trace-db.md` to describe the additive `002` migration, the typed `RepositoryMetadata`, the atomic-claim concurrency contract, and the setup/lifecycle vs. hook-runtime resolution split now that both paths are distinct functions; touch `context/context-map.md` only if a linked summary line becomes materially inaccurate. + - Boundaries (in/out of scope): In — the durable-context files named above. Out — designing or documenting any remote-ingestion consumer of `source_instance_id`; rewriting unrelated sections of those files. + - Dependencies: T04 + - Done when: Both domain files accurately describe `repository_id` (logical Git repository identity) versus `source_instance_id` (physical database lineage identity), the migration, and the resolution split, matching the code shipped in T01–T04. + - Verification notes (commands or checks): Review the updated files against the code from T01–T04 for accuracy; no generated output is touched, so `pkl-check-generated` is not required. + +## Open questions + +None. The request fully specifies the schema change, identity semantics, concurrency pattern, resolution-boundary rules, diagnostics wording, and test coverage; nothing here changes scope, acceptance criteria, or task ordering. diff --git a/context/sce/agent-trace-db.md b/context/sce/agent-trace-db.md index f2c33f53..2bf66ea4 100644 --- a/context/sce/agent-trace-db.md +++ b/context/sce/agent-trace-db.md @@ -36,7 +36,7 @@ pub type RepositoryAgentTraceDb = TursoDb; ``` -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`, currently one fresh multi-statement SQL file at `cli/migrations/agent-trace-repository/001_repository_schema.sql`. The 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)` inserts the singleton metadata row on first initialization and errors if an existing DB stores a different repository ID. `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`). 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_message`, `insert_messages`, `insert_part`, and `insert_parts`. 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. @@ -73,9 +73,10 @@ SCE creates one Agent Trace DB per logical Git repository on demand through setu ## Migrations -`RepositoryAgentTraceDbSpec::migrations()` returns `generated_migrations::AGENT_TRACE_REPOSITORY_MIGRATIONS`, generated from `cli/migrations/agent-trace-repository/` at build time. It is currently one fresh multi-statement baseline file: +`RepositoryAgentTraceDbSpec::migrations()` returns `generated_migrations::AGENT_TRACE_REPOSITORY_MIGRATIONS`, generated from `cli/migrations/agent-trace-repository/` at build time: - `001_repository_schema.sql` (migration ID `001_repository_schema`) — creates `repository_metadata`, `diff_traces` (including `payload_type TEXT NOT NULL DEFAULT 'patch'`), `post_commit_patch_intersections`, `agent_traces`, `messages`, and `parts`, plus the lookup indexes and `updated_at` triggers, in one `execute_batch` statement recorded as a single migration ID. +- `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`. 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. From 849f1885eb30b32380b3374b457a912c38a91f4e Mon Sep 17 00:00:00 2001 From: David Abram Date: Mon, 10 Aug 2026 13:21:34 +0200 Subject: [PATCH 2/5] agent-trace-storage: Add repository metadata to storage resolution Storage resolution previously discarded metadata returned while verifying the repository database, preventing callers from accessing source_instance_id from the same initialization call that opened the database. Preserve the typed metadata through concurrent-safe opening and expose it on ResolvedAgentTraceStorage, with assertions for stable identity across repeated resolution. Plan: agent-trace-source-instance-id Task: T02 Co-authored-by: SCE --- cli/src/services/agent_trace_storage/mod.rs | 27 +++++++++++++------ context/cli/agent-trace-storage.md | 2 +- .../plans/agent-trace-source-instance-id.md | 4 ++- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/cli/src/services/agent_trace_storage/mod.rs b/cli/src/services/agent_trace_storage/mod.rs index a5375ea1..5ae377ba 100644 --- a/cli/src/services/agent_trace_storage/mod.rs +++ b/cli/src/services/agent_trace_storage/mod.rs @@ -14,7 +14,7 @@ use std::time::Duration; use anyhow::{Context, Result}; -use crate::services::agent_trace_db::repository::RepositoryAgentTraceDb; +use crate::services::agent_trace_db::repository::{RepositoryAgentTraceDb, RepositoryMetadata}; use crate::services::checkout::{get_or_create_checkout_id, resolve_git_dir}; use crate::services::default_paths::{ agent_trace_db_path_for_repository, agent_trace_db_path_for_repository_at, @@ -54,6 +54,10 @@ pub struct ResolvedAgentTraceStorage { pub db_path: PathBuf, /// Open repository-scoped Agent Trace database. pub db: RepositoryAgentTraceDb, + /// Physical database identity (`repository_id`/`source_instance_id`) + /// produced by the same verification/initialization call that opened + /// `db`. + pub metadata: RepositoryMetadata, } /// Resolves the repository-scoped Agent Trace storage for a checkout using @@ -112,20 +116,21 @@ fn open_storage( // briefly race on SQLite metadata locks, so retry the fast-path/migrate // sequence a small bounded number of times. let repository_id = &repository_identity.identity.repository_id; - let db = open_repository_db_concurrently_safe(&db_path, repository_id)?; + let (db, metadata) = open_repository_db_concurrently_safe(&db_path, repository_id)?; Ok(ResolvedAgentTraceStorage { repository_identity, checkout_id, db_path, db, + metadata, }) } fn open_repository_db_concurrently_safe( db_path: &Path, repository_id: &str, -) -> Result { +) -> Result<(RepositoryAgentTraceDb, RepositoryMetadata)> { let mut last_error = None; for attempt in 1..=REPOSITORY_DB_INITIALIZATION_ATTEMPTS { @@ -134,16 +139,16 @@ fn open_repository_db_concurrently_safe( if db.ensure_schema_ready_for_hooks().is_err() { db.repair_missing_repository_schema_migration_metadata()?; } - db.verify_or_initialize_repository_metadata(repository_id)?; - Ok(db) + let metadata = db.verify_or_initialize_repository_metadata(repository_id)?; + Ok((db, metadata)) }); match fast_open { - Ok(db) => return Ok(db), + Ok(result) => return Ok(result), Err(fast_error) => match RepositoryAgentTraceDb::new_at(db_path) { Ok(db) => { - db.verify_or_initialize_repository_metadata(repository_id)?; - return Ok(db); + let metadata = db.verify_or_initialize_repository_metadata(repository_id)?; + return Ok((db, metadata)); } Err(init_error) => { last_error = Some(anyhow::anyhow!( @@ -366,6 +371,12 @@ mod tests { first.repository_identity.identity.repository_id, second.repository_identity.identity.repository_id ); + assert_eq!( + first.metadata.repository_id, + first.repository_identity.identity.repository_id + ); + assert_eq!(first.metadata, second.metadata); + assert!(!first.metadata.source_instance_id.trim().is_empty()); std::fs::remove_dir_all(&state_root).expect("clean up state root"); std::fs::remove_dir_all(&repo).expect("clean up repo"); diff --git a/context/cli/agent-trace-storage.md b/context/cli/agent-trace-storage.md index caf2d4c0..9aec6c0d 100644 --- a/context/cli/agent-trace-storage.md +++ b/context/cli/agent-trace-storage.md @@ -5,7 +5,7 @@ Module at `cli/src/services/agent_trace_storage/` (T04 of the `repository-scoped ## Public API - `AgentTraceStorageContext { repository_root, explicit_repository_id, repository_remote }` — borrowed inputs mirroring the `agent_trace.repository_id` / `agent_trace.repository_remote` config keys; callers pass already-resolved configuration values. Active hook runtime and Agent Trace lifecycle setup/health resolve these values through the config service before constructing the storage context. -- `ResolvedAgentTraceStorage { repository_identity, checkout_id, db_path, db }` — resolved repository identity (`ResolvedRepositoryIdentity` including source provenance), the checkout ID for diagnostics (never persisted on Agent Trace rows), the repository-scoped DB path, and the open `RepositoryAgentTraceDb`. +- `ResolvedAgentTraceStorage { repository_identity, checkout_id, db_path, db, metadata }` — resolved repository identity (`ResolvedRepositoryIdentity` including source provenance), the checkout ID for diagnostics (never persisted on Agent Trace rows), the repository-scoped DB path, the open `RepositoryAgentTraceDb`, and the typed `RepositoryMetadata { repository_id, source_instance_id }` produced by the same verification/initialization call that opened `db` (see [../sce/agent-trace-db.md](../sce/agent-trace-db.md) for the `source_instance_id` identity contract). - `resolve_agent_trace_storage(context)` — production entrypoint using the canonical state root from the default-path catalog. - `resolve_agent_trace_storage_at_state_root(context, state_root)` — resolution core against an explicit state root; used by tests to exercise the full path without touching the real user state directory. diff --git a/context/plans/agent-trace-source-instance-id.md b/context/plans/agent-trace-source-instance-id.md index 6be3aa37..f2401ca0 100644 --- a/context/plans/agent-trace-source-instance-id.md +++ b/context/plans/agent-trace-source-instance-id.md @@ -58,13 +58,15 @@ The schema gains an additive migration (`002_repository_source_instance_id.sql`) - Evidence: Added additive migration `cli/migrations/agent-trace-repository/002_repository_source_instance_id.sql` (`ALTER TABLE repository_metadata ADD COLUMN source_instance_id TEXT NOT NULL DEFAULT ''`; auto-discovered by `build.rs`, no `001` rewrite). Added `RepositoryMetadata { repository_id, source_instance_id }`, `generate_source_instance_id()` (UUID v4), `is_valid_source_instance_id()` (non-empty once trimmed) to `cli/src/services/agent_trace_db/repository.rs`. Rewrote `verify_or_initialize_repository_metadata` to return `Result`: seeds the row as before, checks `repository_id` for mismatch before touching `source_instance_id`, then claims via `UPDATE repository_metadata SET source_instance_id = ?1 WHERE id = 1 AND source_instance_id = ''` and always re-reads the row afterward so a losing racer's candidate is discarded and an already-valid value is never overwritten. Existing external callers (`agent_trace_storage/mod.rs`, `trace/discovery.rs`) already discarded the prior `Result<()>` via `?`/`.expect(...)` and needed no changes. Added/updated unit tests in `repository.rs`: `open_at_initializes_the_full_schema_from_one_migration` now asserts both `001_repository_schema` and `002_repository_source_instance_id` are applied; `repository_metadata_is_seeded_once_and_validated_on_reopen` asserts a valid non-empty `source_instance_id` that is stable across repeated calls and reopen; new `source_instance_id_is_not_derived_from_repository_id_and_diverges_across_independent_dbs`; new `concurrent_initialization_converges_on_one_source_instance_id` (4 threads racing the claim, all converge on one value); `mismatched_repository_metadata_errors_on_open` now asserts a rejected mismatch leaves the stored `source_instance_id` unchanged; new `baseline_only_fixture_migrates_and_gets_a_stable_source_instance_id` (hand-built pre-002 fixture with `001` recorded and no `source_instance_id` column, migrates cleanly on open, value stable across reopen). - Verification run: `nix flake check` passed (`cli-fmt`, `cli-clippy` with `#![deny(clippy::pedantic)]`, and `cli-tests`, including all new/updated tests in `repository.rs`). One process note: the new migration SQL file had to be `git add`-staged for Nix's flake source view (git-tracked files only) to see it — the file is included in this task's change set. -- [ ] T02: `Expose repository metadata through setup/lifecycle storage resolution` (status:todo) +- [x] T02: `Expose repository metadata through setup/lifecycle storage resolution` (status:done) - Task ID: T02 - Goal: Add `metadata: RepositoryMetadata` to `ResolvedAgentTraceStorage` and thread it through the existing `resolve_agent_trace_storage`/`resolve_agent_trace_storage_at_state_root` (setup/lifecycle) resolution path, which keeps its existing fast-path-then-migrate fallback behavior. - Boundaries (in/out of scope): In — `cli/src/services/agent_trace_storage/mod.rs` struct/field addition, threading the typed metadata out of `open_repository_db_concurrently_safe`, and updated/added unit tests asserting `ResolvedAgentTraceStorage.metadata`. Out — introducing the dedicated hook-runtime resolution entrypoint (T03); any call-site changes beyond what's needed to keep existing callers compiling against the new field. - Dependencies: T01 - Done when: `ResolvedAgentTraceStorage` carries `metadata`; setup/lifecycle resolution returns it populated from the same verification/initialization call that opens the DB; existing `agent_trace_storage` tests plus new metadata-presence assertions pass. - Verification notes (commands or checks): `nix flake check`. + - Evidence: Added `metadata: RepositoryMetadata` field to `ResolvedAgentTraceStorage` in `cli/src/services/agent_trace_storage/mod.rs`, imported alongside `RepositoryAgentTraceDb`. Changed `open_repository_db_concurrently_safe` to return `(RepositoryAgentTraceDb, RepositoryMetadata)`: both the fast-path branch (`open_without_migrations_at` + optional schema-metadata repair) and the full-init branch (`new_at`) now keep the `RepositoryMetadata` already produced by `verify_or_initialize_repository_metadata` instead of discarding it. `open_storage` destructures `(db, metadata)` and populates the new field; `resolve_agent_trace_storage`/`resolve_agent_trace_storage_at_state_root` are unchanged aside from receiving the new field through `open_storage`. Extended `repeated_resolution_is_idempotent` in `cli/src/services/agent_trace_storage/mod.rs` to assert `first.metadata.repository_id` matches the resolved `repository_id`, `first.metadata == second.metadata` (stable across repeated resolution), and `source_instance_id` is non-empty once trimmed. No other callers needed changes: `lifecycle.rs`, `hooks/mod.rs`, `config/resolver.rs`, `config/types.rs`, and `trace/status.rs` consume `ResolvedAgentTraceStorage` by field access on `db`/`db_path`/`checkout_id`/`repository_identity` and compile unchanged against the additive field. + - Verification run: `nix flake check` passed (`cli-fmt`, `cli-clippy` with `#![deny(clippy::pedantic)]`, and `cli-tests`, including the extended `repeated_resolution_is_idempotent` test and all existing `agent_trace_storage` tests). - [ ] T03: `Add no-migration hook-runtime storage resolution and wire hooks to it` (status:todo) - Task ID: T03 From 254b55bee468833884b0f6979034fe7be840b19e Mon Sep 17 00:00:00 2001 From: David Abram Date: Mon, 10 Aug 2026 13:32:20 +0200 Subject: [PATCH 3/5] agent-trace: Prevent hook runtime database migrations Hook runtime previously used the setup/lifecycle resolver, which could run repository migrations during high-frequency hook execution. Split storage opening by caller so hooks use a no-migration path that preserves setup guidance for missing or incomplete schemas while still validating repository metadata after setup.\n\nUpdate the agent-trace storage context and plan evidence to document the separate contract and cover missing, baseline-only, and initialized database behavior.\n\nPlan: agent-trace-source-instance-id T03 Co-authored-by: SCE --- cli/src/services/agent_trace_storage/mod.rs | 211 +++++++++++++++++- cli/src/services/hooks/mod.rs | 6 +- context/cli/agent-trace-storage.md | 12 +- context/context-map.md | 2 +- .../plans/agent-trace-source-instance-id.md | 4 +- context/sce/agent-trace-db.md | 7 +- 6 files changed, 228 insertions(+), 14 deletions(-) diff --git a/cli/src/services/agent_trace_storage/mod.rs b/cli/src/services/agent_trace_storage/mod.rs index 5ae377ba..3cdf3aae 100644 --- a/cli/src/services/agent_trace_storage/mod.rs +++ b/cli/src/services/agent_trace_storage/mod.rs @@ -84,6 +84,36 @@ pub fn resolve_agent_trace_storage_at_state_root( open_storage(context, repository_identity, db_path) } +/// Resolves repository-scoped Agent Trace storage for high-frequency hook +/// runtime callers using the canonical state root. +/// +/// Never runs migrations, including migration `002`: the database must +/// already have been brought up to date by `sce setup`. A missing or +/// baseline-only (pre-`002`) database fails with the same `sce setup` +/// guidance `ensure_schema_ready_for_hooks` already reports, rather than +/// silently migrating it from a hook path. +pub fn resolve_agent_trace_storage_for_hook_runtime( + context: &AgentTraceStorageContext<'_>, +) -> Result { + let repository_identity = resolve_identity(context)?; + let db_path = agent_trace_db_path_for_repository(&repository_identity.identity.repository_id)?; + open_storage_for_hook_runtime(context, repository_identity, db_path) +} + +/// Hook-runtime resolution core against an explicit state root, so tests can +/// exercise the full path without touching the real user state directory. +pub fn resolve_agent_trace_storage_for_hook_runtime_at_state_root( + context: &AgentTraceStorageContext<'_>, + state_root: &Path, +) -> Result { + let repository_identity = resolve_identity(context)?; + let db_path = agent_trace_db_path_for_repository_at( + state_root, + &repository_identity.identity.repository_id, + )?; + open_storage_for_hook_runtime(context, repository_identity, db_path) +} + fn resolve_identity(context: &AgentTraceStorageContext<'_>) -> Result { resolve_repository_identity( context.repository_root, @@ -97,6 +127,37 @@ fn open_storage( context: &AgentTraceStorageContext<'_>, repository_identity: ResolvedRepositoryIdentity, db_path: PathBuf, +) -> Result { + // Opening the database creates `repos//` when missing; + // directory creation is idempotent and first-time schema initialization may + // briefly race on SQLite metadata locks, so retry the fast-path/migrate + // sequence a small bounded number of times. + open_storage_with( + context, + repository_identity, + db_path, + open_repository_db_concurrently_safe, + ) +} + +fn open_storage_for_hook_runtime( + context: &AgentTraceStorageContext<'_>, + repository_identity: ResolvedRepositoryIdentity, + db_path: PathBuf, +) -> Result { + open_storage_with( + context, + repository_identity, + db_path, + open_repository_db_for_hook_runtime, + ) +} + +fn open_storage_with( + context: &AgentTraceStorageContext<'_>, + repository_identity: ResolvedRepositoryIdentity, + db_path: PathBuf, + open_db: impl FnOnce(&Path, &str) -> Result<(RepositoryAgentTraceDb, RepositoryMetadata)>, ) -> Result { let git_dir = resolve_git_dir(context.repository_root).with_context(|| { format!( @@ -111,12 +172,8 @@ fn open_storage( ) })?; - // Opening the database creates `repos//` when missing; - // directory creation is idempotent and first-time schema initialization may - // briefly race on SQLite metadata locks, so retry the fast-path/migrate - // sequence a small bounded number of times. let repository_id = &repository_identity.identity.repository_id; - let (db, metadata) = open_repository_db_concurrently_safe(&db_path, repository_id)?; + let (db, metadata) = open_db(&db_path, repository_id)?; Ok(ResolvedAgentTraceStorage { repository_identity, @@ -168,6 +225,28 @@ 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. +fn open_repository_db_for_hook_runtime( + db_path: &Path, + repository_id: &str, +) -> Result<(RepositoryAgentTraceDb, RepositoryMetadata)> { + let db = RepositoryAgentTraceDb::open_for_hooks_without_migrations_at(db_path)?; + + if db.ensure_schema_ready_for_hooks().is_err() { + db.repair_missing_repository_schema_migration_metadata()?; + } + + let metadata = db.verify_or_initialize_repository_metadata(repository_id)?; + Ok((db, metadata)) +} + #[cfg(test)] mod tests { use super::*; @@ -519,6 +598,128 @@ mod tests { std::fs::remove_dir_all(&repo).expect("clean up repo"); } + #[test] + fn hook_runtime_resolution_fails_before_setup_when_db_is_missing() { + let state_root = unique_temp_dir("state-hook-missing"); + let repo = init_git_repo_with_remote("hook-missing", "git@github.com:acme/widgets.git"); + + let Err(error) = resolve_agent_trace_storage_for_hook_runtime_at_state_root( + &context_for(&repo), + &state_root, + ) else { + panic!("hook runtime resolution must fail before sce setup runs"); + }; + assert!( + error.to_string().contains("sce setup"), + "unexpected error: {error}" + ); + + std::fs::remove_dir_all(&state_root).expect("clean up state root"); + std::fs::remove_dir_all(&repo).expect("clean up repo"); + } + + #[test] + fn hook_runtime_resolution_fails_before_setup_on_baseline_only_schema_without_recording_migration_002( + ) { + let state_root = unique_temp_dir("state-hook-baseline"); + let repo = init_git_repo_with_remote("hook-baseline", "git@github.com:acme/widgets.git"); + let identity = resolve_repository_identity(&repo, None, "origin") + .expect("identity should resolve for baseline fixture setup"); + let repository_id = identity.identity.repository_id.clone(); + let db_path = agent_trace_db_path_for_repository_at(&state_root, &repository_id) + .expect("db path should resolve for baseline fixture setup"); + + // Simulate a database created before migration 002 existed: build the + // pre-002 `repository_metadata` shape (no `source_instance_id` + // column) and record only migration 001 as applied, mirroring the + // fixture in `repository.rs`'s + // `baseline_only_fixture_migrates_and_gets_a_stable_source_instance_id`. + let baseline_only = RepositoryAgentTraceDb::open_without_migrations_at(&db_path) + .expect("baseline-only repository DB should open"); + baseline_only + .execute( + "CREATE TABLE IF NOT EXISTS __sce_migrations ( + id TEXT PRIMARY KEY, + applied_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) +)", + (), + ) + .expect("migration metadata table should create"); + baseline_only + .execute( + "CREATE TABLE IF NOT EXISTS repository_metadata ( + id INTEGER PRIMARY KEY CHECK (id = 1), + repository_id TEXT NOT NULL, + created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ', 'now')) +)", + (), + ) + .expect("baseline repository_metadata table should create"); + baseline_only + .execute( + "INSERT INTO __sce_migrations (id) VALUES ('001_repository_schema')", + (), + ) + .expect("baseline migration record should insert"); + baseline_only + .execute( + "INSERT INTO repository_metadata (id, repository_id) VALUES (1, ?1)", + (repository_id.as_str(),), + ) + .expect("baseline metadata row should seed"); + drop(baseline_only); + + let Err(error) = resolve_agent_trace_storage_for_hook_runtime_at_state_root( + &context_for(&repo), + &state_root, + ) else { + panic!("hook runtime resolution must fail on a baseline-only (pre-002) schema"); + }; + assert!( + error.to_string().contains("sce setup"), + "unexpected error: {error}" + ); + + let reopened = RepositoryAgentTraceDb::open_without_migrations_at(&db_path) + .expect("baseline DB should still be openable after the failed hook resolution"); + let problems = reopened + .migration_metadata_problems() + .expect("migration metadata problems should be queryable"); + assert!( + problems.iter().any(|problem| problem.contains("002")), + "migration 002 must remain unrecorded: {problems:?}" + ); + + std::fs::remove_dir_all(&state_root).expect("clean up state root"); + std::fs::remove_dir_all(&repo).expect("clean up repo"); + } + + #[test] + fn hook_runtime_resolution_after_setup_matches_setup_path_metadata() { + let state_root = unique_temp_dir("state-hook-after-setup"); + let repo = init_git_repo_with_remote("hook-after-setup", "git@github.com:acme/widgets.git"); + + let setup = resolve_agent_trace_storage_at_state_root(&context_for(&repo), &state_root) + .expect("setup/lifecycle resolution should succeed"); + + let hook_runtime = resolve_agent_trace_storage_for_hook_runtime_at_state_root( + &context_for(&repo), + &state_root, + ) + .expect("hook runtime resolution should succeed once sce setup has run"); + + assert_eq!(hook_runtime.db_path, setup.db_path); + assert_eq!(hook_runtime.metadata, setup.metadata); + assert_eq!( + hook_runtime.metadata.repository_id, + setup.repository_identity.identity.repository_id + ); + assert!(!hook_runtime.metadata.source_instance_id.trim().is_empty()); + + std::fs::remove_dir_all(&state_root).expect("clean up state root"); + std::fs::remove_dir_all(&repo).expect("clean up repo"); + } + #[test] fn path_traversal_repository_id_is_rejected() { for bad in ["../escape", "a/b", "a\\b", ".", ".."] { diff --git a/cli/src/services/hooks/mod.rs b/cli/src/services/hooks/mod.rs index 7a6d44fd..eb2914e0 100644 --- a/cli/src/services/hooks/mod.rs +++ b/cli/src/services/hooks/mod.rs @@ -19,7 +19,9 @@ use crate::services::agent_trace_db::{ PartType, PostCommitPatchIntersectionInsert, RecentDiffTracePatches, PAYLOAD_TYPE_PATCH, PAYLOAD_TYPE_STRUCTURED, }; -use crate::services::agent_trace_storage::{resolve_agent_trace_storage, AgentTraceStorageContext}; +use crate::services::agent_trace_storage::{ + resolve_agent_trace_storage_for_hook_runtime, AgentTraceStorageContext, +}; use crate::services::config; use crate::services::observability::traits::Logger; use crate::services::patch::{ @@ -337,7 +339,7 @@ fn open_agent_trace_db_for_hook_runtime( repository_remote: &storage_config.repository_remote, }; - resolve_agent_trace_storage(&storage_context) + resolve_agent_trace_storage_for_hook_runtime(&storage_context) .map(|storage| storage.db) .context(context_message) } diff --git a/context/cli/agent-trace-storage.md b/context/cli/agent-trace-storage.md index 9aec6c0d..d999d042 100644 --- a/context/cli/agent-trace-storage.md +++ b/context/cli/agent-trace-storage.md @@ -6,15 +6,19 @@ Module at `cli/src/services/agent_trace_storage/` (T04 of the `repository-scoped - `AgentTraceStorageContext { repository_root, explicit_repository_id, repository_remote }` — borrowed inputs mirroring the `agent_trace.repository_id` / `agent_trace.repository_remote` config keys; callers pass already-resolved configuration values. Active hook runtime and Agent Trace lifecycle setup/health resolve these values through the config service before constructing the storage context. - `ResolvedAgentTraceStorage { repository_identity, checkout_id, db_path, db, metadata }` — resolved repository identity (`ResolvedRepositoryIdentity` including source provenance), the checkout ID for diagnostics (never persisted on Agent Trace rows), the repository-scoped DB path, the open `RepositoryAgentTraceDb`, and the typed `RepositoryMetadata { repository_id, source_instance_id }` produced by the same verification/initialization call that opened `db` (see [../sce/agent-trace-db.md](../sce/agent-trace-db.md) for the `source_instance_id` identity contract). -- `resolve_agent_trace_storage(context)` — production entrypoint using the canonical state root from the default-path catalog. -- `resolve_agent_trace_storage_at_state_root(context, state_root)` — resolution core against an explicit state root; used by tests to exercise the full path without touching the real user state directory. +- `resolve_agent_trace_storage(context)` — setup/lifecycle production entrypoint using the canonical state root from the default-path catalog; may run migrations, including migration `002`. +- `resolve_agent_trace_storage_at_state_root(context, state_root)` — setup/lifecycle resolution core against an explicit state root; used by tests to exercise the full path without touching the real user state directory. +- `resolve_agent_trace_storage_for_hook_runtime(context)` — hook-runtime production entrypoint using the canonical state root; never runs migrations, including migration `002`. +- `resolve_agent_trace_storage_for_hook_runtime_at_state_root(context, state_root)` — hook-runtime resolution core against an explicit state root; used by tests. ## Resolution flow 1. Repository identity via `repository_identity::resolve` precedence (explicit config ID → configured remote URL, default `origin`); resolution errors carry `.sce/config.json` guidance and never echo URLs. A failed identity resolution creates no state directories. 2. Checkout identity reuse via `checkout::resolve_git_dir` + `get_or_create_checkout_id` (`/sce/checkout-id`). 3. DB path from `default_paths::agent_trace_db_path_for_repository{,_at}`, which rejects empty or path-unsafe repository IDs (separators, `.`, `..`). -4. DB open through `agent_trace_db::repository::RepositoryAgentTraceDb` with a bounded concurrent-safe fast-path-then-migrate sequence: `open_without_migrations_at` + `ensure_schema_ready_for_hooks()` + `verify_or_initialize_repository_metadata(repository_id)`, falling back to migration-running `new_at` plus the same metadata validation. Directory creation rides on `TursoDb` parent-dir `create_dir_all`. If concurrent first open leaves all repository schema tables present but the one-file baseline migration record missing, the repository adapter records that missing metadata before rechecking readiness; otherwise the resolver retries the open/migrate sequence for a bounded window while SQLite/Turso locks clear. +4. DB open splits by caller through `agent_trace_db::repository::RepositoryAgentTraceDb`, sharing steps 1–3 through an internal `open_storage_with` helper parameterized by the DB-opener: + - Setup/lifecycle (`resolve_agent_trace_storage{,_at_state_root}`): a bounded concurrent-safe fast-path-then-migrate sequence — `open_without_migrations_at` + `ensure_schema_ready_for_hooks()` + `verify_or_initialize_repository_metadata(repository_id)`, falling back to migration-running `new_at` plus the same metadata validation. Directory creation rides on `TursoDb` parent-dir `create_dir_all`. If concurrent first open leaves all repository schema tables present but the one-file baseline migration record missing, the repository adapter records that missing metadata before rechecking readiness; otherwise the resolver retries the open/migrate sequence for a bounded window while SQLite/Turso locks clear. + - Hook runtime (`resolve_agent_trace_storage_for_hook_runtime{,_at_state_root}`): `open_for_hooks_without_migrations_at` + `ensure_schema_ready_for_hooks()`, applying the same narrow concurrent-first-open metadata repair on failure but never falling back to `new_at`/migrations. A missing or baseline-only (pre-`002`) database fails with `ensure_schema_ready_for_hooks`'s existing `Run 'sce setup'.` guidance. Once readiness passes, `verify_or_initialize_repository_metadata(repository_id)` initializes `source_instance_id` when needed, matching setup's result for an already-set-up repository. `sce hooks` command call sites use this path via `open_agent_trace_db_for_hook_runtime` in `cli/src/services/hooks/mod.rs`. ## Never-touch on-disk boundary @@ -22,6 +26,6 @@ The resolver never selects, creates, or touches pre-migration checkout-scoped `< ## Status -Registered in `cli/src/services/mod.rs` and consumed by hook runtime, Agent Trace lifecycle setup, and current-repository trace status/shell flows. T05 changed the resolved DB handle to the repository-scoped adapter and validates the stored `repository_metadata.repository_id` before returning storage; T08 wired hooks/lifecycle to pass resolved config values into this context; T09 wired trace UX to repository-scoped storage (the checkout-scoped `sce trace --legacy` surface was later removed by the `retire-legacy-agent-trace-db` plan). Covered by in-module tests: repository separation, SSH/HTTPS clone consolidation, linked-worktree consolidation, explicit-ID override, idempotent re-resolution, missing-identity guidance, path-segment validation, pre-migration checkout DB byte preservation/non-selection, empty fresh repository DB state, repository-level row sharing across equivalent clone checkouts, credential-safe remote canonicalization, and concurrent first-open convergence (`nix build .#checks..cli-tests`). +Registered in `cli/src/services/mod.rs` and consumed by hook runtime, Agent Trace lifecycle setup, and current-repository trace status/shell flows. T05 changed the resolved DB handle to the repository-scoped adapter and validates the stored `repository_metadata.repository_id` before returning storage; T08 wired hooks/lifecycle to pass resolved config values into this context; T09 wired trace UX to repository-scoped storage (the checkout-scoped `sce trace --legacy` surface was later removed by the `retire-legacy-agent-trace-db` plan). The `agent-trace-source-instance-id` plan's T03 split hook-runtime resolution into its own no-migration entrypoint, switching `open_agent_trace_db_for_hook_runtime` in `cli/src/services/hooks/mod.rs` off the setup/lifecycle resolver so hook runtime never runs migration `002` (or any migration). Covered by in-module tests: repository separation, SSH/HTTPS clone consolidation, linked-worktree consolidation, explicit-ID override, idempotent re-resolution, missing-identity guidance, path-segment validation, pre-migration checkout DB byte preservation/non-selection, empty fresh repository DB state, repository-level row sharing across equivalent clone checkouts, credential-safe remote canonicalization, concurrent first-open convergence, and hook-runtime resolution (fails before setup on a missing DB, fails before setup on a baseline-only pre-`002` schema without recording migration `002`, and matches setup's `RepositoryMetadata` once setup has run) (`nix build .#checks..cli-tests`). See also: [repository-identity.md](repository-identity.md), [checkout-identity.md](checkout-identity.md), [default-path-catalog.md](default-path-catalog.md), [../sce/agent-trace-db.md](../sce/agent-trace-db.md) diff --git a/context/context-map.md b/context/context-map.md index c4257b60..530d6e0f 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -12,7 +12,7 @@ Feature/domain context: - `context/cli/cli-command-surface.md` (CLI command surface including top-level help with ASCII art banner and gradient rendering, setup install flow with the repeatable `sce setup --workflow ` optional-workflow selection and its interactive post-target multi-select, WorkOS device authorization flow + token storage behavior, attribution-only hook routing with validated post-commit `--remote-url` plumbing plus DB-backed `diff-trace` dual persistence and post-commit Agent Trace payload persistence including range `content_hash`, setup-owned local DB + repository-scoped Agent Trace DB bootstrap plus doctor DB health coverage with credential-safe repository identity diagnostics, centralized Rust SCE web URL helpers in `services::agent_trace`, nested flake release package/app installability, Cargo local install + crates.io readiness policy, and hidden `sce policy bash` command adapter for bash-policy hook callers; `sce sync` command wiring is deferred to `0.4.0`; static `RuntimeCommand` enum dispatch lives in `services/command_registry.rs`, command payload structs for help/version/completion/auth/config/setup/doctor/hooks/policy are owned by their respective `services/{name}/command.rs` files, and clap-to-runtime conversion lives in `services/parse/command_runtime.rs`) - `context/cli/default-path-catalog.md` (canonical production CLI path-ownership contract centered on `cli/src/services/default_paths.rs`, including persisted auth/config files, named DB paths for auth/local/repository-scoped Agent Trace databases, the default observability log-dir accessor consumed by config resolution with Linux `${XDG_STATE_HOME:-~/.local/state}/sce/logs` fallback semantics, repo-relative, embedded-asset, install, hook, and context-path families plus the regression guard that keeps production path ownership centralized) - `context/cli/repository-identity.md` (repository identity module in `cli/src/services/repository_identity/`: pure scheme-neutral `host[:port]/path` canonicalization for SCP/`ssh://`/HTTPS/`git://` remote URLs with credential stripping, hostname lowercasing, default-port removal, and query/fragment/trailing-`.git` cleanup, trim-only explicit-identity handling, `sha256("sce-repository-id-v1\0" + canonical_identity)` repository IDs, credential-safe fieldless errors, plus the `resolve` runtime submodule applying explicit-config-then-configured-remote precedence with `git config --get remote..url` lookup, `RepositoryIdentitySource` provenance, and `.sce/config.json`-guidance resolution errors that never echo URLs; consumed by the T04 `agent_trace_storage` resolver) -- `context/cli/agent-trace-storage.md` (repository-scoped Agent Trace storage resolver in `cli/src/services/agent_trace_storage/`: `AgentTraceStorageContext` inputs mirroring the `agent_trace.*` config keys, `ResolvedAgentTraceStorage` carrying repository identity + checkout ID + `/sce/repos//agent-trace.db` path + open `RepositoryAgentTraceDb`, `resolve_agent_trace_storage{,_at_state_root}` entrypoints with idempotent concurrent-safe first open via bounded fast-path-then-migrate retry plus narrow one-file schema migration-metadata repair and repository metadata validation, path-unsafe repository ID rejection in `default_paths::agent_trace_db_path_for_repository{,_at}`, strict never-touch boundary for any pre-migration checkout-scoped/global DB files, and active hook/runtime plus Agent Trace lifecycle setup call-site consumption after T08) +- `context/cli/agent-trace-storage.md` (repository-scoped Agent Trace storage resolver in `cli/src/services/agent_trace_storage/`: `AgentTraceStorageContext` inputs mirroring the `agent_trace.*` config keys, `ResolvedAgentTraceStorage` carrying repository identity + checkout ID + `/sce/repos//agent-trace.db` path + open `RepositoryAgentTraceDb` + typed `RepositoryMetadata`, `resolve_agent_trace_storage{,_at_state_root}` setup/lifecycle entrypoints with idempotent concurrent-safe first open via bounded fast-path-then-migrate retry plus narrow one-file schema migration-metadata repair and repository metadata validation, a separate no-migration `resolve_agent_trace_storage_for_hook_runtime{,_at_state_root}` pair that high-frequency hook callers use exclusively and that never runs migration `002` or any migration, path-unsafe repository ID rejection in `default_paths::agent_trace_db_path_for_repository{,_at}`, strict never-touch boundary for any pre-migration checkout-scoped/global DB files, and active hook/runtime plus Agent Trace lifecycle setup call-site consumption after T08) - `context/cli/checkout-identity.md` (current checkout identity infrastructure in `cli/src/services/checkout/`, including `/sce/checkout-id` UUIDv7 storage, setup/hook integration that creates/reuses checkout identity as repository-scoped Agent Trace diagnostic metadata, the removed per-checkout DB opener/path helper, `sce doctor` checkout identity display, and the never-touch on-disk handling of pre-migration checkout-scoped DB files that are no longer inspectable via the CLI) - `context/cli/patch-service.md` (standalone patch domain model, parser, JSON load helpers, and set operations in `cli/src/services/patch.rs` for in-memory parsed unified-diff representation, capturing only touched lines plus minimal per-file/per-hunk metadata, supporting both `Index:` SVN-style and `diff --git` git-style formats, with `ParseError` for actionable malformed-input diagnostics, `PatchLoadError`/`load_patch_from_json`/`load_patch_from_json_bytes` for storage-agnostic JSON reconstruction, `intersect_patches` for target-shaped overlap with exact-match-first and historical `kind`+`content` fallback semantics plus matched-constructed-line `session_id` and matched-constructed-hunk `model_id` provenance inheritance, and `combine_patches` for ordered patch combination with later-wins conflict resolution plus winning-hunk `model_id` provenance inheritance; `parse_patch`, `intersect_patches`, and `combine_patches` are consumed by the active post-commit hook runtime) - `context/cli/structured-patch-service.md` (Claude structured editor-hook derivation in `cli/src/services/structured_patch.rs`, including `Write` structured-update hunks, `Write` `tool_input.content` create fallback, `Edit` structured patches, deterministic skip reasons, `ParsedPatch` output semantics, and Rust golden fixture coverage) diff --git a/context/plans/agent-trace-source-instance-id.md b/context/plans/agent-trace-source-instance-id.md index f2401ca0..4fd59359 100644 --- a/context/plans/agent-trace-source-instance-id.md +++ b/context/plans/agent-trace-source-instance-id.md @@ -68,13 +68,15 @@ The schema gains an additive migration (`002_repository_source_instance_id.sql`) - Evidence: Added `metadata: RepositoryMetadata` field to `ResolvedAgentTraceStorage` in `cli/src/services/agent_trace_storage/mod.rs`, imported alongside `RepositoryAgentTraceDb`. Changed `open_repository_db_concurrently_safe` to return `(RepositoryAgentTraceDb, RepositoryMetadata)`: both the fast-path branch (`open_without_migrations_at` + optional schema-metadata repair) and the full-init branch (`new_at`) now keep the `RepositoryMetadata` already produced by `verify_or_initialize_repository_metadata` instead of discarding it. `open_storage` destructures `(db, metadata)` and populates the new field; `resolve_agent_trace_storage`/`resolve_agent_trace_storage_at_state_root` are unchanged aside from receiving the new field through `open_storage`. Extended `repeated_resolution_is_idempotent` in `cli/src/services/agent_trace_storage/mod.rs` to assert `first.metadata.repository_id` matches the resolved `repository_id`, `first.metadata == second.metadata` (stable across repeated resolution), and `source_instance_id` is non-empty once trimmed. No other callers needed changes: `lifecycle.rs`, `hooks/mod.rs`, `config/resolver.rs`, `config/types.rs`, and `trace/status.rs` consume `ResolvedAgentTraceStorage` by field access on `db`/`db_path`/`checkout_id`/`repository_identity` and compile unchanged against the additive field. - Verification run: `nix flake check` passed (`cli-fmt`, `cli-clippy` with `#![deny(clippy::pedantic)]`, and `cli-tests`, including the extended `repeated_resolution_is_idempotent` test and all existing `agent_trace_storage` tests). -- [ ] T03: `Add no-migration hook-runtime storage resolution and wire hooks to it` (status:todo) +- [x] T03: `Add no-migration hook-runtime storage resolution and wire hooks to it` (status:done) - Task ID: T03 - Goal: Add `resolve_agent_trace_storage_for_hook_runtime`/`_at_state_root` in `cli/src/services/agent_trace_storage/mod.rs` — no-migration open, schema-readiness check, narrow concurrent-first-open metadata repair only, and `source_instance_id` initialization gated on readiness already passing (never a migration fallback) — and switch `open_agent_trace_db_for_hook_runtime` in `cli/src/services/hooks/mod.rs` to call it instead of the setup/lifecycle resolver. - Boundaries (in/out of scope): In — the new resolution functions and their unit tests (fails before setup on a missing DB, fails before setup on a baseline-only schema without recording migration `002`, succeeds after setup and returns the same `RepositoryMetadata` as the setup path), and the one hook call-site switch. Out — changing `sce trace status`/`sce trace db shell`/`sce doctor` DB-open call sites, which keep using their current resolution paths. - Dependencies: T02 - Done when: Hook runtime never runs migration `002` (or any migration) during normal operation; a baseline-only or missing schema fails hook resolution with the existing `sce setup` guidance; a fully set-up repository resolves successfully for hooks with metadata matching setup's; all new tests pass alongside existing hook-behavior tests. - Verification notes (commands or checks): `nix flake check`. + - Evidence: Added `resolve_agent_trace_storage_for_hook_runtime`/`_at_state_root` in `cli/src/services/agent_trace_storage/mod.rs`. Refactored the shared git-dir/checkout-id resolution out of `open_storage` into a new `open_storage_with` helper parameterized by a DB-opener closure, so the existing setup/lifecycle path (`open_repository_db_concurrently_safe`, unchanged fast-path-then-migrate behavior) and the new hook-runtime path share one code path for everything except how the database itself is opened. Added `open_repository_db_for_hook_runtime`: opens with `RepositoryAgentTraceDb::open_for_hooks_without_migrations_at` (no migrations), applies the existing narrow `repair_missing_repository_schema_migration_metadata` repair only when `ensure_schema_ready_for_hooks` fails, then calls `verify_or_initialize_repository_metadata` — never falls back to `new_at`/full migration, so a missing or baseline-only (pre-`002`) database surfaces `ensure_schema_ready_for_hooks`'s existing `Run 'sce setup'.` guidance instead of silently migrating. Switched `open_agent_trace_db_for_hook_runtime` in `cli/src/services/hooks/mod.rs` to call `resolve_agent_trace_storage_for_hook_runtime` instead of the setup/lifecycle `resolve_agent_trace_storage`; no other hook call sites needed changes. Added three unit tests in `agent_trace_storage/mod.rs`: `hook_runtime_resolution_fails_before_setup_when_db_is_missing` (fresh state root, no DB, asserts the error mentions `sce setup`); `hook_runtime_resolution_fails_before_setup_on_baseline_only_schema_without_recording_migration_002` (hand-built pre-002 fixture — `__sce_migrations`/`repository_metadata` tables with only `001_repository_schema` recorded, matching the same reduced fixture shape used by `repository.rs`'s existing baseline-fixture test — asserts hook resolution fails with `sce setup` guidance and `migration_metadata_problems` still reports migration `002` unrecorded afterward); `hook_runtime_resolution_after_setup_matches_setup_path_metadata` (runs the setup/lifecycle resolver first, then the hook-runtime resolver against the same state root, asserts identical `db_path` and `RepositoryMetadata`, and a non-empty `source_instance_id`). + - Verification run: `nix flake check` passed (`cli-fmt`, `cli-clippy` with `#![deny(clippy::pedantic)]`, and `cli-tests`, including all three new hook-runtime resolution tests alongside the full existing suite). - [ ] T04: `Report source-instance ID in setup diagnostics` (status:todo) - Task ID: T04 diff --git a/context/sce/agent-trace-db.md b/context/sce/agent-trace-db.md index 2bf66ea4..0abade24 100644 --- a/context/sce/agent-trace-db.md +++ b/context/sce/agent-trace-db.md @@ -82,7 +82,12 @@ The former checkout-scoped `AGENT_TRACE_MIGRATIONS` constant and its 15-file `cl 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 hook DB resolution first resolves `agent_trace.repository_id` / `agent_trace.repository_remote` through config, then uses `agent_trace_storage::resolve_agent_trace_storage(...)`. The storage resolver tries `RepositoryAgentTraceDb::open_without_migrations_at(path)` + `ensure_schema_ready_for_hooks()` + repository metadata validation first. If setup has not initialized the repository DB, metadata is absent, or migrations are incomplete, it falls back to migration-running `RepositoryAgentTraceDb::new_at(path)` and validates/seeds `repository_metadata.repository_id` before returning. The resolver retries the fast-path/migration sequence for a bounded window during concurrent first opens; if another opener completed the one-file schema but the baseline migration record is missing, the repository adapter records that metadata only after verifying all required repository schema tables already exist. When the fallback also fails, the error context includes the fast-path failure reason (`(fast-path attempt: {fast_error})`) so both failure causes are visible in diagnostics. Normal readiness 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. +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/checkout-ID setup and returning the same `ResolvedAgentTraceStorage { metadata: RepositoryMetadata, .. } `: + +- **Setup/lifecycle** (`resolve_agent_trace_storage(...)`): tries `RepositoryAgentTraceDb::open_without_migrations_at(path)` + `ensure_schema_ready_for_hooks()` + repository metadata validation first. If the repository DB has not been initialized, metadata is absent, or migrations are incomplete, it falls back to migration-running `RepositoryAgentTraceDb::new_at(path)` and validates/seeds `repository_metadata.repository_id` before returning. This is the only path that may run migration `002` (or any migration). It retries the fast-path/migration sequence for a bounded window during concurrent first opens; if another opener completed the one-file schema but the baseline migration record is missing, the repository adapter records that metadata only after verifying all required repository schema tables already exist. When the fallback also fails, the error context includes the fast-path failure reason (`(fast-path attempt: {fast_error})`) so both failure causes are visible in diagnostics. +- **Hook runtime** (`resolve_agent_trace_storage_for_hook_runtime(...)`): opens with `RepositoryAgentTraceDb::open_for_hooks_without_migrations_at(path)` and never falls back to `new_at`/migrations. If `ensure_schema_ready_for_hooks()` fails, it applies only the same narrow concurrent-first-open migration-metadata repair the setup path uses, then re-checks readiness; a missing database or a baseline-only (pre-`002`) database surfaces the existing `Run 'sce setup'.` guidance instead of migrating. Once readiness passes, it calls the same `verify_or_initialize_repository_metadata(repository_id)` to initialize `source_instance_id` when needed. `sce hooks conversation-trace`/`diff-trace`/`commit-msg` (and any other high-frequency hook caller) use this path exclusively via `open_agent_trace_db_for_hook_runtime` in `cli/src/services/hooks/mod.rs`. + +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: From fdea4c237e7ca47acab1c3da366bbdbe1adb7a90 Mon Sep 17 00:00:00 2001 From: David Abram Date: Mon, 10 Aug 2026 13:36:53 +0200 Subject: [PATCH 4/5] agent-trace-db: Report source-instance ID in setup diagnostics Setup diagnostics previously omitted the source-instance identity even though storage resolution already returned it. Include the ID in the lifecycle setup payload and rendered message, with coverage for configured and absent remotes, so setup output exposes the complete repository identity without changing workspace or ingestion behavior.\n\nPlan: agent-trace-source-instance-id.md (T04) Co-authored-by: SCE --- cli/src/services/agent_trace_db/lifecycle.rs | 50 ++++++++++++++++++- .../plans/agent-trace-source-instance-id.md | 4 +- context/sce/agent-trace-db.md | 2 +- 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/cli/src/services/agent_trace_db/lifecycle.rs b/cli/src/services/agent_trace_db/lifecycle.rs index c5787a52..08ad3959 100644 --- a/cli/src/services/agent_trace_db/lifecycle.rs +++ b/cli/src/services/agent_trace_db/lifecycle.rs @@ -81,6 +81,7 @@ struct RepositoryDatabaseSetup { identity_source: String, configured_remote: Option, checkout_id: String, + source_instance_id: String, database_path: PathBuf, } @@ -107,6 +108,7 @@ fn initialize_repository_agent_trace_db(repo_root: &Path) -> Result S .map(|remote| format!("\nAgent Trace configured remote: {remote}")) .unwrap_or_default(); format!( - "Agent Trace repository ID: {}\nAgent Trace identity source: {}\nAgent Trace canonical identity: {}{}\nAgent Trace checkout identity: {}\nAgent Trace repository-scoped database initialized at '{}'.", + "Agent Trace repository ID: {}\nAgent Trace identity source: {}\nAgent Trace canonical identity: {}{}\nAgent Trace checkout identity: {}\nAgent Trace source-instance ID: {}\nAgent Trace repository-scoped database initialized at '{}'.", setup.repository_id, setup.identity_source, setup.canonical_identity, remote_line, setup.checkout_id, + setup.source_instance_id, setup.database_path.display() ) } @@ -223,3 +226,48 @@ fn resolve_lifecycle_agent_trace_db_path(repo_root: Option<&Path>) -> Result) -> RepositoryDatabaseSetup { + RepositoryDatabaseSetup { + repository_id: String::from("repo-123"), + canonical_identity: String::from("github.com/example/repo"), + identity_source: String::from("remote_url"), + configured_remote: configured_remote.map(String::from), + checkout_id: String::from("checkout-abc"), + source_instance_id: String::from("11111111-2222-3333-4444-555555555555"), + database_path: PathBuf::from("/tmp/agent-trace.db"), + } + } + + #[test] + fn formatted_message_includes_source_instance_id_line() { + let message = format_repository_storage_setup_message(&setup(Some("origin"))); + + assert!( + message + .contains("Agent Trace source-instance ID: 11111111-2222-3333-4444-555555555555"), + "expected message to report the source-instance ID, got: {message}" + ); + assert!( + message.contains("Agent Trace repository ID: repo-123"), + "expected message to still report the repository ID, got: {message}" + ); + assert!( + message.contains("Agent Trace configured remote: origin"), + "expected message to still report the configured remote, got: {message}" + ); + } + + #[test] + fn formatted_message_omits_remote_line_but_keeps_source_instance_id() { + let message = format_repository_storage_setup_message(&setup(None)); + + assert!(!message.contains("Agent Trace configured remote:")); + assert!(message + .contains("Agent Trace source-instance ID: 11111111-2222-3333-4444-555555555555")); + } +} diff --git a/context/plans/agent-trace-source-instance-id.md b/context/plans/agent-trace-source-instance-id.md index 4fd59359..aec75b21 100644 --- a/context/plans/agent-trace-source-instance-id.md +++ b/context/plans/agent-trace-source-instance-id.md @@ -78,13 +78,15 @@ The schema gains an additive migration (`002_repository_source_instance_id.sql`) - Evidence: Added `resolve_agent_trace_storage_for_hook_runtime`/`_at_state_root` in `cli/src/services/agent_trace_storage/mod.rs`. Refactored the shared git-dir/checkout-id resolution out of `open_storage` into a new `open_storage_with` helper parameterized by a DB-opener closure, so the existing setup/lifecycle path (`open_repository_db_concurrently_safe`, unchanged fast-path-then-migrate behavior) and the new hook-runtime path share one code path for everything except how the database itself is opened. Added `open_repository_db_for_hook_runtime`: opens with `RepositoryAgentTraceDb::open_for_hooks_without_migrations_at` (no migrations), applies the existing narrow `repair_missing_repository_schema_migration_metadata` repair only when `ensure_schema_ready_for_hooks` fails, then calls `verify_or_initialize_repository_metadata` — never falls back to `new_at`/full migration, so a missing or baseline-only (pre-`002`) database surfaces `ensure_schema_ready_for_hooks`'s existing `Run 'sce setup'.` guidance instead of silently migrating. Switched `open_agent_trace_db_for_hook_runtime` in `cli/src/services/hooks/mod.rs` to call `resolve_agent_trace_storage_for_hook_runtime` instead of the setup/lifecycle `resolve_agent_trace_storage`; no other hook call sites needed changes. Added three unit tests in `agent_trace_storage/mod.rs`: `hook_runtime_resolution_fails_before_setup_when_db_is_missing` (fresh state root, no DB, asserts the error mentions `sce setup`); `hook_runtime_resolution_fails_before_setup_on_baseline_only_schema_without_recording_migration_002` (hand-built pre-002 fixture — `__sce_migrations`/`repository_metadata` tables with only `001_repository_schema` recorded, matching the same reduced fixture shape used by `repository.rs`'s existing baseline-fixture test — asserts hook resolution fails with `sce setup` guidance and `migration_metadata_problems` still reports migration `002` unrecorded afterward); `hook_runtime_resolution_after_setup_matches_setup_path_metadata` (runs the setup/lifecycle resolver first, then the hook-runtime resolver against the same state root, asserts identical `db_path` and `RepositoryMetadata`, and a non-empty `source_instance_id`). - Verification run: `nix flake check` passed (`cli-fmt`, `cli-clippy` with `#![deny(clippy::pedantic)]`, and `cli-tests`, including all three new hook-runtime resolution tests alongside the full existing suite). -- [ ] T04: `Report source-instance ID in setup diagnostics` (status:todo) +- [x] T04: `Report source-instance ID in setup diagnostics` (status:done) - Task ID: T04 - Goal: Extend `AgentTraceDbLifecycle::setup`'s reported message in `cli/src/services/agent_trace_db/lifecycle.rs` to include the source-instance ID alongside the existing repository ID line, sourced from the `RepositoryMetadata` now returned by storage resolution. - Boundaries (in/out of scope): In — `RepositoryDatabaseSetup`/`format_repository_storage_setup_message` in `lifecycle.rs` and its covering test. Out — `sce doctor` diagnose output (read-only, unchanged per Assumptions). - Dependencies: T02 - Done when: `sce setup`'s Agent Trace messaging includes an `Agent Trace source-instance ID: ...` line using the resolved metadata, with no workspace/remote-ingestion wording introduced. - Verification notes (commands or checks): `nix flake check`; inspect the formatted message in the lifecycle setup test. + - Evidence: Added `source_instance_id: String` to `RepositoryDatabaseSetup` in `cli/src/services/agent_trace_db/lifecycle.rs`, populated in `initialize_repository_agent_trace_db` from `storage.metadata.source_instance_id` (the `RepositoryMetadata` returned by storage resolution since T02). `format_repository_storage_setup_message` now emits an `Agent Trace source-instance ID: {value}` line between the checkout identity line and the "database initialized at" line, introducing no workspace/remote-ingestion wording. No covering test existed for this function before this task, so a new `#[cfg(test)] mod tests` was added with two unit tests: `formatted_message_includes_source_instance_id_line` (asserts the source-instance ID and repository ID lines both appear, with a configured remote present) and `formatted_message_omits_remote_line_but_keeps_source_instance_id` (asserts the remote line is correctly omitted while the source-instance ID line still appears when no remote is configured). + - Verification run: `nix flake check` passed (`cli-fmt`, `cli-clippy` with `#![deny(clippy::pedantic)]`, and `cli-tests`, including the two new lifecycle setup-message tests). - [ ] T05: `Document the repository_id vs source_instance_id identity split` (status:todo) - Task ID: T05 diff --git a/context/sce/agent-trace-db.md b/context/sce/agent-trace-db.md index 0abade24..ee03816c 100644 --- a/context/sce/agent-trace-db.md +++ b/context/sce/agent-trace-db.md @@ -172,7 +172,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`, creates/reuses the current checkout identity for diagnostics, opens/creates `/sce/repos//agent-trace.db` with the repository schema, validates `repository_metadata.repository_id`, and emits setup messaging with the repository ID, checkout ID, 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`, creates/reuses the current checkout identity for diagnostics, opens/creates `/sce/repos//agent-trace.db` with the repository schema, validates `repository_metadata.repository_id`, and emits setup messaging with the repository ID, checkout 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. - `sce doctor` surfaces checkout identity facts where available and 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 `checkout_identity` when available plus the resolved `agent_trace_db` field. - `sce trace db list` discovers repository DBs under `/sce/repos//agent-trace.db`, reporting text or JSON sorted by mtime descending. See [context/cli/trace-command.md](../cli/trace-command.md). From 62e8d5ea52caded40a26de149eeceb6474813dee Mon Sep 17 00:00:00 2001 From: David Abram Date: Mon, 10 Aug 2026 13:45:39 +0200 Subject: [PATCH 5/5] agent-trace: Document physical database identity Distinguish the logical repository ID from the physical database lineage ID and preserve the migration, concurrency, and setup/runtime resolution contracts. Record the accepted decision and mark the implementation plan complete with its validation evidence.\n\nPlan: context/plans/agent-trace-source-instance-id.md (T01, T02, T03, T04, T05) Co-authored-by: SCE --- context/context-map.md | 1 + ...26-08-10-agent-trace-source-instance-id.md | 119 ++++++++++++++++++ .../plans/agent-trace-source-instance-id.md | 49 ++++++-- context/sce/agent-trace-db.md | 2 +- 4 files changed, 162 insertions(+), 9 deletions(-) create mode 100644 context/decisions/2026-08-10-agent-trace-source-instance-id.md diff --git a/context/context-map.md b/context/context-map.md index 530d6e0f..aa291d8a 100644 --- a/context/context-map.md +++ b/context/context-map.md @@ -109,3 +109,4 @@ Recent decision records: - `context/decisions/2026-03-09-migrate-lexopt-to-clap.md` (CLI argument parsing migration from lexopt to clap derive macros) - `context/decisions/2026-03-25-first-install-channels.md` (approved first-wave install/distribution scope for `sce`, canonical naming, and Nix-owned build policy) - `context/decisions/2026-07-17-retire-legacy-agent-trace-db.md` (retire the checkout-scoped Agent Trace DB surface; `RepositoryAgentTraceDb` is the sole adapter, no `sce trace --legacy`, no global/checkout fallback path; pre-migration on-disk files are never touched and no longer inspectable via the CLI) +- `context/decisions/2026-08-10-agent-trace-source-instance-id.md` (adds `source_instance_id` as a physical-database-lineage identity on `repository_metadata`, independent of and never derived from `repository_id`; concurrency-safe atomic claim; local storage identity only, no remote-ingestion architecture designed) diff --git a/context/decisions/2026-08-10-agent-trace-source-instance-id.md b/context/decisions/2026-08-10-agent-trace-source-instance-id.md new file mode 100644 index 00000000..d1158a58 --- /dev/null +++ b/context/decisions/2026-08-10-agent-trace-source-instance-id.md @@ -0,0 +1,119 @@ +# Decision: Give repository-scoped Agent Trace DBs a physical-instance identity independent of repository identity + +Date: 2026-08-10 +Status: Accepted +Plan: `context/plans/agent-trace-source-instance-id.md` +Task: T01, T02, T03, T04, T05 + +## Context + +`RepositoryAgentTraceDb` identifies a database by `repository_metadata.repository_id`, +a logical Git-repository identity shared by every clone/checkout of the same +repository (see [`repository identity`](../glossary.md)). That identity alone +cannot distinguish two independently created physical `agent-trace.db` files for +the same logical repository — for example, the same repository cloned on two +different machines. A prior, abandoned design (PR #186) attempted to solve +physical-instance identification bundled together with a full remote-ingestion +architecture (a data warehouse, `agent-trace-sync.db`, ETL, and Turso Sync), and +was retired without shipping. The durable, reusable part of that design — a +stable per-physical-database identity — was worth recreating on its own, without +committing to any remote-ingestion architecture. + +## Decision + +Add `source_instance_id` as a second, independent identity column on +`repository_metadata`, added by the additive migration +`002_repository_source_instance_id.sql`. It identifies one physical database +lineage: generated once by application code the first time a given physical +`agent-trace.db` is initialized, and stable across reopen, `sce setup` reruns, +and process restarts. It is never derived from `repository_id`, remote URL, +checkout ID, filesystem path, hostname, or user/workspace identity, so two +independently created databases for the same logical repository always diverge. +Concurrent first opens of the same physical database converge on exactly one +persisted value through an atomic `UPDATE ... WHERE source_instance_id = ''` +claim; a losing racer's generated candidate is discarded, and an already-valid +stored value is never overwritten. `RepositoryMetadata { repository_id, +source_instance_id }` is the typed result threaded through +`ResolvedAgentTraceStorage` for both the setup/lifecycle resolution path +(`resolve_agent_trace_storage`, which may run migration `002`) and the +no-migration hook-runtime resolution path (`resolve_agent_trace_storage_for_hook_runtime`, +which never runs migration `002` or any migration). + +## Rationale + +Splitting physical-instance identity from logical-repository identity as two +columns on the same row keeps both concepts queryable together without +conflating them: `repository_id` answers "which logical repository," while +`source_instance_id` answers "which physical database." Generating the value in +application code rather than SQL keeps the identity's shape (UUID v4 today) +free to evolve without a schema dependency, and validating it only as +"non-empty once trimmed" (`is_valid_source_instance_id`) avoids hard-coding +UUID-v4 parsing into downstream consumers. The atomic claim pattern is the +minimal concurrency-safe primitive needed to guarantee exactly one winner +across concurrent SCE processes opening the same new database, without +introducing a separate locking mechanism. + +## Alternatives considered + +- **Derive the identity from a stable local signal (hostname, filesystem path, + or checkout ID)** — rejected: none of these are guaranteed stable or unique + per physical database file, and the request explicitly ruled this out to keep + the identity meaningful even if a database file is copied or moved. +- **Recreate the full PR #186 architecture (DWH, `agent-trace-sync.db`, ETL, + Turso Sync) alongside the identity column** — rejected: that architecture was + abandoned and is unrelated to the local storage-identity problem; bundling it + back in would reintroduce the same retired complexity this plan exists to + avoid. +- **Let hook-runtime resolution run migration `002` like any other migration** + — rejected: it would blur the existing no-migration boundary between + high-frequency hook-runtime DB access and migration-running setup/lifecycle + access, a separation this plan preserves and extends rather than erodes. + +## Compatibility and risks + +- Additive migration only: `001_repository_schema.sql` is untouched, and + existing/placeholder rows default `source_instance_id` to an empty string, + so no existing database is invalidated. +- A losing racer under concurrent first-open must discard its generated + candidate; the atomic claim and always-re-read pattern makes this safe, but + any future caller of `verify_or_initialize_repository_metadata` must + preserve that re-read rather than trusting its own candidate. +- Hook-runtime resolution failing closed (no migration fallback) on a + baseline-only or missing database is intentional: a pre-`002` or pre-setup + repository must fail with `sce setup` guidance rather than silently + migrating from the high-frequency hook path. + +## Guardrails + +- No remote ingestion, sync, DWH, or `agent-trace-sync.db` behavior is + designed or implied by this decision; `source_instance_id` is local storage + identity only. +- No workspace, host, or user/account identity is added to the local + repository Agent Trace DB by this decision. +- `sce doctor`'s read-only diagnose surface stays unchanged; only `sce setup` + diagnostics report the new identity. + +## Consequences + +- Every repository-scoped `agent-trace.db` now carries a stable, physically + unique identity that a future remote-ingestion consumer could use to + distinguish rows originating from different physical databases for the same + logical repository — without this decision committing to what that consumer + looks like. +- The setup/lifecycle-vs-hook-runtime resolution split, already implicit in + the codebase, is now an explicit, separately named, separately tested + boundary (`resolve_agent_trace_storage` vs. + `resolve_agent_trace_storage_for_hook_runtime`), which future callers must + choose between deliberately rather than defaulting to one path. + +## Follow-up + +None. + +## References + +- Plan: [`agent-trace-source-instance-id`](../plans/agent-trace-source-instance-id.md) +- Task: T01, T02, T03, T04, T05 +- Current-state context: [`context/cli/agent-trace-storage.md`](../cli/agent-trace-storage.md) +- Current-state context: [`context/sce/agent-trace-db.md`](../sce/agent-trace-db.md) +- Related decision: [`Retire the legacy checkout-scoped Agent Trace DB surface`](2026-07-17-retire-legacy-agent-trace-db.md) diff --git a/context/plans/agent-trace-source-instance-id.md b/context/plans/agent-trace-source-instance-id.md index aec75b21..873767a5 100644 --- a/context/plans/agent-trace-source-instance-id.md +++ b/context/plans/agent-trace-source-instance-id.md @@ -8,19 +8,19 @@ The schema gains an additive migration (`002_repository_source_instance_id.sql`) ## Acceptance criteria -- [ ] AC1: A fresh repository-scoped `agent-trace.db` receives a valid, non-empty `source_instance_id` on first initialization, and `repository_id` remains correct. +- [x] AC1: A fresh repository-scoped `agent-trace.db` receives a valid, non-empty `source_instance_id` on first initialization, and `repository_id` remains correct. - Validate: `nix flake check` (runs the new repository-adapter unit tests covering fresh initialization). -- [ ] AC2: `source_instance_id` is stable across database reopen, repeated `verify_or_initialize_repository_metadata` calls (repeated `sce setup`), and is never derived from `repository_id`, remote URL, checkout ID, filesystem path, hostname, or user/workspace identity. +- [x] AC2: `source_instance_id` is stable across database reopen, repeated `verify_or_initialize_repository_metadata` calls (repeated `sce setup`), and is never derived from `repository_id`, remote URL, checkout ID, filesystem path, hostname, or user/workspace identity. - Validate: `nix flake check` (repository-adapter reopen/repeat/independent-DB unit tests). -- [ ] AC3: Concurrent initialization of the same physical database converges on exactly one persisted `source_instance_id`; a losing racer's generated candidate is discarded in favor of the winner's stored value, and an already-valid `source_instance_id` is never overwritten. +- [x] AC3: Concurrent initialization of the same physical database converges on exactly one persisted `source_instance_id`; a losing racer's generated candidate is discarded in favor of the winner's stored value, and an already-valid `source_instance_id` is never overwritten. - Validate: `nix flake check` (concurrent-initialization unit test in `cli/src/services/agent_trace_db/repository.rs`). -- [ ] AC4: `resolve_agent_trace_storage` (setup/lifecycle path) returns the same typed `RepositoryMetadata` that database verification/initialization produced, alongside the existing `db`/`db_path`/`checkout_id` fields. +- [x] AC4: `resolve_agent_trace_storage` (setup/lifecycle path) returns the same typed `RepositoryMetadata` that database verification/initialization produced, alongside the existing `db`/`db_path`/`checkout_id` fields. - Validate: `nix flake check` (`agent_trace_storage` unit tests asserting `ResolvedAgentTraceStorage.metadata`). -- [ ] AC5: High-frequency hook runtime resolution never applies migration `002`: before `sce setup` (missing DB, or a baseline-only DB that has migration `001` but not `002`), hook resolution fails with the existing `sce setup` guidance and leaves the stored migration metadata unchanged; after `sce setup`, hook resolution succeeds and returns the same `RepositoryMetadata` setup produced. +- [x] AC5: High-frequency hook runtime resolution never applies migration `002`: before `sce setup` (missing DB, or a baseline-only DB that has migration `001` but not `002`), hook resolution fails with the existing `sce setup` guidance and leaves the stored migration metadata unchanged; after `sce setup`, hook resolution succeeds and returns the same `RepositoryMetadata` setup produced. - Validate: `nix flake check` (`agent_trace_storage` hook-runtime resolution unit tests: before-setup missing DB, before-setup baseline-only schema, after-setup parity). -- [ ] AC6: `sce setup` Agent Trace diagnostics report the source-instance ID alongside the existing repository ID line, without introducing workspace or remote-ingestion concepts. +- [x] AC6: `sce setup` Agent Trace diagnostics report the source-instance ID alongside the existing repository ID line, without introducing workspace or remote-ingestion concepts. - Validate: Inspect `format_repository_storage_setup_message` output in `cli/src/services/agent_trace_db/lifecycle.rs` and its covering test. -- [ ] AC7: A baseline-only fixture (only the original `001` repository schema, no `source_instance_id` column) migrates cleanly to `002`, gets a populated `source_instance_id`, and preserves that value across reopen. +- [x] AC7: A baseline-only fixture (only the original `001` repository schema, no `source_instance_id` column) migrates cleanly to `002`, gets a populated `source_instance_id`, and preserves that value across reopen. - Validate: `nix flake check` (baseline-fixture migration unit test in `cli/src/services/agent_trace_db/repository.rs`). ### Full validation @@ -88,14 +88,47 @@ The schema gains an additive migration (`002_repository_source_instance_id.sql`) - Evidence: Added `source_instance_id: String` to `RepositoryDatabaseSetup` in `cli/src/services/agent_trace_db/lifecycle.rs`, populated in `initialize_repository_agent_trace_db` from `storage.metadata.source_instance_id` (the `RepositoryMetadata` returned by storage resolution since T02). `format_repository_storage_setup_message` now emits an `Agent Trace source-instance ID: {value}` line between the checkout identity line and the "database initialized at" line, introducing no workspace/remote-ingestion wording. No covering test existed for this function before this task, so a new `#[cfg(test)] mod tests` was added with two unit tests: `formatted_message_includes_source_instance_id_line` (asserts the source-instance ID and repository ID lines both appear, with a configured remote present) and `formatted_message_omits_remote_line_but_keeps_source_instance_id` (asserts the remote line is correctly omitted while the source-instance ID line still appears when no remote is configured). - Verification run: `nix flake check` passed (`cli-fmt`, `cli-clippy` with `#![deny(clippy::pedantic)]`, and `cli-tests`, including the two new lifecycle setup-message tests). -- [ ] T05: `Document the repository_id vs source_instance_id identity split` (status:todo) +- [x] T05: `Document the repository_id vs source_instance_id identity split` (status:done) - Task ID: T05 - Goal: Update `context/cli/agent-trace-storage.md` and `context/sce/agent-trace-db.md` to describe the additive `002` migration, the typed `RepositoryMetadata`, the atomic-claim concurrency contract, and the setup/lifecycle vs. hook-runtime resolution split now that both paths are distinct functions; touch `context/context-map.md` only if a linked summary line becomes materially inaccurate. - Boundaries (in/out of scope): In — the durable-context files named above. Out — designing or documenting any remote-ingestion consumer of `source_instance_id`; rewriting unrelated sections of those files. - Dependencies: T04 - Done when: Both domain files accurately describe `repository_id` (logical Git repository identity) versus `source_instance_id` (physical database lineage identity), the migration, and the resolution split, matching the code shipped in T01–T04. - Verification notes (commands or checks): Review the updated files against the code from T01–T04 for accuracy; no generated output is touched, so `pkl-check-generated` is not required. + - Evidence: Reviewed `context/cli/agent-trace-storage.md`, `context/sce/agent-trace-db.md`, and `context/context-map.md` against the code shipped in T01–T04 (`cli/src/services/agent_trace_storage/mod.rs`, `cli/src/services/agent_trace_db/repository.rs`, `cli/src/services/agent_trace_db/lifecycle.rs`). Each preceding task's own context-synchronization step had already incrementally documented this identity split as it shipped: `agent-trace-storage.md` already describes the `metadata: RepositoryMetadata` field on `ResolvedAgentTraceStorage`, both `resolve_agent_trace_storage{,_at_state_root}` (setup/lifecycle, may run migration `002`) and `resolve_agent_trace_storage_for_hook_runtime{,_at_state_root}` (hook runtime, never runs migration `002`) entrypoints, and the fast-path-then-migrate vs. no-migration resolution split; `agent-trace-db.md` already describes migration `002_repository_source_instance_id.sql`, the typed `RepositoryMetadata { repository_id, source_instance_id }`, the atomic `UPDATE ... WHERE source_instance_id = ''` claim contract (losing racer's candidate discarded, already-valid value never overwritten), and the same setup/lifecycle-vs-hook-runtime split; `context-map.md`'s summary lines for both files already match. No content in any of the three files was found to be missing, stale, or contradicted by the T01–T04 code, so no edit was made — the task's "done when" condition was already satisfied by prior tasks' incremental synchronization. + - Verification run: Manual review only (no code or generated output touched); confirmed via `grep` that the documented function/type names (`resolve_agent_trace_storage_for_hook_runtime`, `ResolvedAgentTraceStorage`, `RepositoryMetadata`, `generate_source_instance_id`, `is_valid_source_instance_id`, `verify_or_initialize_repository_metadata`) exist in the source files referenced by the docs. ## Open questions None. The request fully specifies the schema change, identity semantics, concurrency pattern, resolution-boundary rules, diagnostics wording, and test coverage; nothing here changes scope, acceptance criteria, or task ordering. + +## Validation Report + +**Status:** validated +**Date:** 2026-08-10 + +### Commands run + +- `nix flake check` -> exit 0 (`all checks passed!`; builds and runs `cli-fmt`, `cli-clippy` (`#![deny(clippy::pedantic)]`), and `cli-tests` for the CLI crate, including all new/updated tests from T01–T04) + +### Scaffolding removed + +- None. + +### Success-criteria verification + +- [x] AC1: Fresh DB gets a valid, non-empty `source_instance_id`; `repository_id` stays correct -> `nix flake check` (`repository.rs` fresh-initialization tests); confirmed by code inspection of `verify_or_initialize_repository_metadata` in `cli/src/services/agent_trace_db/repository.rs:168-215`. +- [x] AC2: Stable across reopen/repeat, never derived from other identities -> `nix flake check` (`repository_metadata_is_seeded_once_and_validated_on_reopen`, `source_instance_id_is_not_derived_from_repository_id_and_diverges_across_independent_dbs`); confirmed by code inspection. +- [x] AC3: Concurrent initialization converges on one value; loser discarded, valid value never overwritten -> `nix flake check` (`concurrent_initialization_converges_on_one_source_instance_id`, 4-thread race); confirmed by the atomic `UPDATE ... WHERE source_instance_id = ''` claim plus always-re-read pattern in `repository.rs:197-215`. +- [x] AC4: `resolve_agent_trace_storage` returns the same typed `RepositoryMetadata` alongside `db`/`db_path`/`checkout_id` -> `nix flake check` (`repeated_resolution_is_idempotent` asserting `ResolvedAgentTraceStorage.metadata`); confirmed by inspection of `cli/src/services/agent_trace_storage/mod.rs:45-183`. +- [x] AC5: Hook runtime never applies migration `002`; fails pre-setup with `sce setup` guidance, succeeds post-setup with matching metadata -> `nix flake check` (`hook_runtime_resolution_fails_before_setup_when_db_is_missing`, `hook_runtime_resolution_fails_before_setup_on_baseline_only_schema_without_recording_migration_002`, `hook_runtime_resolution_after_setup_matches_setup_path_metadata`); confirmed by inspection of `resolve_agent_trace_storage_for_hook_runtime{,_at_state_root}` in `agent_trace_storage/mod.rs`. +- [x] AC6: `sce setup` diagnostics report the source-instance ID alongside repository ID, no workspace/remote-ingestion wording -> Inspected `format_repository_storage_setup_message` in `cli/src/services/agent_trace_db/lifecycle.rs:116-131` (emits `Agent Trace source-instance ID: {value}`) and its two covering tests (`formatted_message_includes_source_instance_id_line`, `formatted_message_omits_remote_line_but_keeps_source_instance_id`), both passing under `nix flake check`. +- [x] AC7: Baseline-only fixture migrates cleanly to `002`, gets a populated `source_instance_id`, stable across reopen -> `nix flake check` (`baseline_only_fixture_migrates_and_gets_a_stable_source_instance_id`). + +### Failed checks and follow-ups + +- None. + +### Residual risks + +- None identified. diff --git a/context/sce/agent-trace-db.md b/context/sce/agent-trace-db.md index ee03816c..d53ce6f4 100644 --- a/context/sce/agent-trace-db.md +++ b/context/sce/agent-trace-db.md @@ -76,7 +76,7 @@ SCE creates one Agent Trace DB per logical Git repository on demand through setu `RepositoryAgentTraceDbSpec::migrations()` returns `generated_migrations::AGENT_TRACE_REPOSITORY_MIGRATIONS`, generated from `cli/migrations/agent-trace-repository/` at build time: - `001_repository_schema.sql` (migration ID `001_repository_schema`) — creates `repository_metadata`, `diff_traces` (including `payload_type TEXT NOT NULL DEFAULT 'patch'`), `post_commit_patch_intersections`, `agent_traces`, `messages`, and `parts`, plus the lookup indexes and `updated_at` triggers, in one `execute_batch` statement recorded as a single migration ID. -- `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`. +- `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`. 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.