From c97b3b7b77c01a187f83d222f8fd6d5b3996141b Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 2 Sep 2026 16:43:28 +0900 Subject: [PATCH] feat: introduce workspace-scoped repository keys --- crates/workspace-api/src/lib.rs | 82 ++- crates/workspace-server/src/repositories.rs | 46 +- .../workspace-server/src/repository_access.rs | 2 +- crates/workspace-server/src/server.rs | 109 ++-- crates/workspace-server/src/store.rs | 566 ++++++++++++++++-- .../src/workdir_create_operations.rs | 2 +- .../workspace-server/src/workdir_removal.rs | 2 +- .../workspace-server/src/workspace_catalog.rs | 38 +- 8 files changed, 672 insertions(+), 175 deletions(-) diff --git a/crates/workspace-api/src/lib.rs b/crates/workspace-api/src/lib.rs index 79153639..54d43821 100644 --- a/crates/workspace-api/src/lib.rs +++ b/crates/workspace-api/src/lib.rs @@ -6,6 +6,54 @@ use serde::{Deserialize, Serialize}; +pub const REPOSITORY_KEY_MIN_LEN: usize = 1; +pub const REPOSITORY_KEY_MAX_LEN: usize = 64; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum RepositoryKeyError { + Length, + Character, + LeadingHyphen, + TrailingHyphen, +} + +impl std::fmt::Display for RepositoryKeyError { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter.write_str(match self { + Self::Length => "must contain between 1 and 64 ASCII bytes", + Self::Character => "must contain only lowercase ASCII letters, digits, and hyphens", + Self::LeadingHyphen => "must not start with a hyphen", + Self::TrailingHyphen => "must not end with a hyphen", + }) + } +} + +impl std::error::Error for RepositoryKeyError {} + +/// Validate one immutable Workspace-scoped Repository key. +/// +/// Keys are deliberately not normalized: callers must submit the exact canonical +/// lowercase ASCII spelling so idempotency and route identity cannot alias. +pub fn validate_repository_key(value: &str) -> Result<(), RepositoryKeyError> { + let bytes = value.as_bytes(); + if !(REPOSITORY_KEY_MIN_LEN..=REPOSITORY_KEY_MAX_LEN).contains(&bytes.len()) { + return Err(RepositoryKeyError::Length); + } + if bytes[0] == b'-' { + return Err(RepositoryKeyError::LeadingHyphen); + } + if bytes[bytes.len() - 1] == b'-' { + return Err(RepositoryKeyError::TrailingHyphen); + } + if !bytes + .iter() + .all(|byte| byte.is_ascii_lowercase() || byte.is_ascii_digit() || *byte == b'-') + { + return Err(RepositoryKeyError::Character); + } + Ok(()) +} + /// Provider-neutral classification of an authoritative Repository source. /// /// Local paths remain distinct from network Git transports so callers cannot @@ -70,8 +118,7 @@ pub struct RepositorySource { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[serde(deny_unknown_fields)] pub struct CreateWorkspaceRepositoryRequest { - pub repository_id: String, - pub display_name: String, + pub repository_key: String, pub source: String, #[serde(default)] pub default_ref: Option, @@ -80,7 +127,7 @@ pub struct CreateWorkspaceRepositoryRequest { #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub struct CreateWorkspaceRepositoryResponse { pub workspace_id: String, - pub repository_id: String, + pub repository_key: String, pub replayed: bool, } @@ -139,8 +186,7 @@ pub struct WorkspaceCatalogListResponse(pub Vec); #[serde(deny_unknown_fields)] pub struct WorkspaceRepositoryRecord { pub workspace_id: String, - pub repository_id: String, - pub name: String, + pub repository_key: String, pub kind: String, pub provider: Option, pub source: RepositorySource, @@ -348,8 +394,7 @@ pub struct GitRepositorySummary { #[cfg_attr(feature = "typescript", derive(ts_rs::TS))] #[serde(deny_unknown_fields)] pub struct RepositorySummary { - pub id: String, - pub display_name: String, + pub repository_key: String, pub kind: String, pub provider: String, pub source: RepositorySource, @@ -410,7 +455,7 @@ pub struct RepositoryDetailResponse { #[serde(deny_unknown_fields)] pub struct RepositoryLogResponse { pub workspace_id: String, - pub repository_id: String, + pub repository_key: String, #[serde(skip_serializing_if = "Option::is_none")] #[cfg_attr(feature = "typescript", ts(optional = nullable))] pub default_selector: Option, @@ -1331,6 +1376,27 @@ mod workdir_typescript_tests { mod tests { use super::*; + #[test] + fn repository_key_validation_is_canonical_and_bounded() { + let max = "a".repeat(64); + for valid in ["a", "main", "repo-42", max.as_str()] { + assert_eq!(validate_repository_key(valid), Ok(()), "{valid}"); + } + let too_long = "a".repeat(65); + for invalid in [ + "", + "-main", + "main-", + "Main", + "main_repo", + "main.repo", + "日本語", + too_long.as_str(), + ] { + assert!(validate_repository_key(invalid).is_err(), "{invalid}"); + } + } + #[test] fn workspace_and_repository_response_shapes_round_trip() { let workspace = serde_json::json!({ diff --git a/crates/workspace-server/src/repositories.rs b/crates/workspace-server/src/repositories.rs index 7e7f9160..a294e7bd 100644 --- a/crates/workspace-server/src/repositories.rs +++ b/crates/workspace-server/src/repositories.rs @@ -15,6 +15,7 @@ pub type RepositorySelector = String; #[derive(Debug, Clone, PartialEq, Eq)] pub struct ConfiguredRepository { pub id: RepositoryId, + pub repository_key: String, pub provider: String, pub source: RepositorySource, pub source_revision: u64, @@ -22,7 +23,6 @@ pub struct ConfiguredRepository { pub observed_status: RepositoryObservedStatus, pub observed_at: Option, pub path: Option, - pub display_name: Option, pub default_selector: Option, } @@ -34,7 +34,7 @@ pub struct RepositoryListProjection { #[derive(Debug, Clone, PartialEq, Eq)] pub struct RepositoryLogRead { - pub repository_id: RepositoryId, + pub repository_key: String, pub default_selector: Option, pub limit: usize, pub commits: Vec, @@ -97,21 +97,28 @@ impl RepositoryRegistryReader { } } - pub fn summary(&self, id: &str) -> Result { - let repository = self - .find(id) - .ok_or_else(|| RepositoryLookupError::UnknownRepository { id: id.to_string() })?; + pub fn summary( + &self, + repository_key: &str, + ) -> Result { + let repository = self.find_by_key(repository_key).ok_or_else(|| { + RepositoryLookupError::UnknownRepository { + id: repository_key.to_string(), + } + })?; Ok(self.summary_for_config(repository)) } pub fn recent_log( &self, - id: &str, + repository_key: &str, limit: Option, ) -> Result { - let repository = self - .find(id) - .ok_or_else(|| RepositoryLookupError::UnknownRepository { id: id.to_string() })?; + let repository = self.find_by_key(repository_key).ok_or_else(|| { + RepositoryLookupError::UnknownRepository { + id: repository_key.to_string(), + } + })?; if repository.provider != "git" { return Err(RepositoryLookupError::UnsupportedProvider { id: repository.id.clone(), @@ -134,7 +141,7 @@ impl RepositoryRegistryReader { }; Ok(RepositoryLogRead { - repository_id: repository.id.clone(), + repository_key: repository.repository_key.clone(), default_selector: repository.default_selector.clone(), limit, commits, @@ -260,6 +267,12 @@ impl RepositoryRegistryReader { Ok(repository) } + fn find_by_key(&self, repository_key: &str) -> Option<&ConfiguredRepository> { + self.repositories + .iter() + .find(|repository| repository.repository_key == repository_key) + } + fn find(&self, id: &str) -> Option<&ConfiguredRepository> { self.repositories .iter() @@ -267,10 +280,6 @@ impl RepositoryRegistryReader { } fn summary_for_config(&self, repository: &ConfiguredRepository) -> RepositorySummary { - let display_name = repository - .display_name - .clone() - .unwrap_or_else(|| repository.id.clone()); let mut diagnostics = Vec::new(); if repository.source.kind == workspace_api::RepositorySourceKind::Http { diagnostics.push(RepositoryDiagnostic { @@ -314,8 +323,7 @@ impl RepositoryRegistryReader { }; RepositorySummary { - id: repository.id.clone(), - display_name, + repository_key: repository.repository_key.clone(), kind: repository.provider.clone(), provider: repository.provider.clone(), source: repository.source.clone(), @@ -600,7 +608,7 @@ mod tests { }; let reader = RepositoryRegistryReader::new(vec![ConfiguredRepository { id: "remote".into(), - display_name: Some("Remote".into()), + repository_key: "remote".into(), provider: "git".into(), source_fingerprint: crate::repository_source::repository_source_fingerprint(&source), source, @@ -724,7 +732,7 @@ mod tests { }; let reader = RepositoryRegistryReader::new(vec![ConfiguredRepository { id: "main".into(), - display_name: Some("Main".into()), + repository_key: "main".into(), provider: "git".into(), source_fingerprint: crate::repository_source::repository_source_fingerprint( &source_descriptor, diff --git a/crates/workspace-server/src/repository_access.rs b/crates/workspace-server/src/repository_access.rs index 16dd500f..d14ce95c 100644 --- a/crates/workspace-server/src/repository_access.rs +++ b/crates/workspace-server/src/repository_access.rs @@ -1846,7 +1846,7 @@ mod tests { .upsert_repository(&RepositoryRecord { workspace_id: "workspace-a".to_string(), repository_id: "remote".to_string(), - name: "Remote".to_string(), + repository_key: "remote".to_string(), kind: "git".to_string(), provider: Some("git".to_string()), source, diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 1343205d..7aef240a 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -288,6 +288,7 @@ impl ServerConfig { .into_iter() .map(|repository| ConfiguredRepository { id: repository.repository_id, + repository_key: repository.repository_key, provider: repository.provider.unwrap_or(repository.kind), path: repository_local_path(&repository.source), source: repository.source, @@ -295,7 +296,6 @@ impl ServerConfig { source_fingerprint: repository.source_fingerprint, observed_status: repository.observed_status, observed_at: repository.observed_at, - display_name: Some(repository.name), default_selector: repository.default_ref, }) .collect(); @@ -1056,8 +1056,7 @@ fn workspace_summary(record: WorkspaceRecord) -> WorkspaceSummary { fn workspace_repository_record(record: RepositoryRecord) -> WorkspaceRepositoryRecord { WorkspaceRepositoryRecord { workspace_id: record.workspace_id, - repository_id: record.repository_id, - name: record.name, + repository_key: record.repository_key, kind: record.kind, provider: record.provider, source: record.source, @@ -2061,13 +2060,14 @@ fn import_configured_repositories( } let now = crate::auth::now_rfc3339(); for repository in &config.repositories { + let repository_id = store + .get_repository_by_key(&config.workspace_id, &repository.repository_key)? + .map(|record| record.repository_id) + .unwrap_or_else(|| Uuid::now_v7().to_string()); store.upsert_repository(&RepositoryRecord { workspace_id: config.workspace_id.clone(), - repository_id: repository.id.clone(), - name: repository - .display_name - .clone() - .unwrap_or_else(|| repository.id.clone()), + repository_id, + repository_key: repository.repository_key.clone(), kind: repository.provider.clone(), provider: Some(repository.provider.clone()), source: repository.source.clone(), @@ -2102,6 +2102,7 @@ fn configured_repository_from_record( let path = repository_local_path(&record.source); Ok(ConfiguredRepository { id: record.repository_id, + repository_key: record.repository_key, provider, path, source: record.source, @@ -2109,7 +2110,6 @@ fn configured_repository_from_record( source_fingerprint: record.source_fingerprint, observed_status: record.observed_status, observed_at: record.observed_at, - display_name: Some(record.name), default_selector: record.default_ref, }) } @@ -7901,8 +7901,9 @@ async fn scoped_create_repository( ) -> ApiResult<(StatusCode, Json)> { require_workspace_owner(&api, &path.workspace_id, &actor, "ManageRepositories").await?; - let repository_id = normalize_repository_identifier(&request.repository_id)?; - let display_name = normalize_repository_text(&request.display_name, "display_name", 256)?; + workspace_api::validate_repository_key(&request.repository_key) + .map_err(|error| Error::InvalidInput(format!("invalid Repository key: {error}")))?; + let repository_key = request.repository_key; let default_ref = request .default_ref .as_deref() @@ -7913,8 +7914,8 @@ async fn scoped_create_repository( let now = Utc::now().to_rfc3339(); let record = RepositoryRecord { workspace_id: path.workspace_id.clone(), - repository_id: repository_id.clone(), - name: display_name, + repository_id: Uuid::now_v7().to_string(), + repository_key: repository_key.clone(), kind: "git".to_string(), provider: Some("git".to_string()), source_fingerprint: repository_source_fingerprint(&source), @@ -7936,7 +7937,7 @@ async fn scoped_create_repository( } RepositoryInsertOutcome::Existing(_) => { return Err(Error::RepositoryConflict(format!( - "repository_id {repository_id} already exists with different registration intent" + "Repository key {repository_key} already exists with different registration intent" )) .into()); } @@ -7946,30 +7947,12 @@ async fn scoped_create_repository( status, Json(CreateWorkspaceRepositoryResponse { workspace_id: path.workspace_id, - repository_id, + repository_key, replayed, }), )) } -fn normalize_repository_identifier(value: &str) -> Result { - let value = value.trim(); - if value.is_empty() || value.len() > 128 { - return Err(Error::InvalidInput( - "repository_id must contain 1..=128 characters".to_string(), - )); - } - if !value - .bytes() - .all(|byte| byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_' | b'.')) - { - return Err(Error::InvalidInput( - "repository_id must contain only ASCII letters, digits, '-', '_', or '.'".to_string(), - )); - } - Ok(value.to_string()) -} - fn normalize_repository_text(value: &str, field: &str, max_len: usize) -> Result { let value = value.trim(); if value.is_empty() || value.len() > max_len || value.chars().any(char::is_control) { @@ -7985,8 +7968,7 @@ fn repository_create_intent_matches( requested: &RepositoryRecord, ) -> bool { existing.workspace_id == requested.workspace_id - && existing.repository_id == requested.repository_id - && existing.name == requested.name + && existing.repository_key == requested.repository_key && existing.kind == requested.kind && existing.provider == requested.provider && existing.source == requested.source @@ -11643,9 +11625,14 @@ async fn list_repositories( async fn repository_detail( State(api): State, - AxumPath(repository_id): AxumPath, + AxumPath(repository_key): AxumPath, ) -> ApiResult> { - let item = repository_lookup(api.live_repository_reader()?.summary(&repository_id))?; + workspace_api::validate_repository_key(&repository_key).map_err(|error| { + ApiError::from(Error::InvalidInput(format!( + "invalid Repository key: {error}" + ))) + })?; + let item = repository_lookup(api.live_repository_reader()?.summary(&repository_key))?; Ok(Json(RepositoryDetailResponse { workspace_id: api.config.workspace_id.clone(), item, @@ -11655,22 +11642,27 @@ async fn repository_detail( async fn repository_log( State(api): State, - AxumPath(repository_id): AxumPath, + AxumPath(repository_key): AxumPath, Query(query): Query, ) -> ApiResult> { + workspace_api::validate_repository_key(&repository_key).map_err(|error| { + ApiError::from(Error::InvalidInput(format!( + "invalid Repository key: {error}" + ))) + })?; let RepositoryLogRead { - repository_id, + repository_key, default_selector, limit, commits, diagnostics, } = repository_lookup( api.live_repository_reader()? - .recent_log(&repository_id, query.limit), + .recent_log(&repository_key, query.limit), )?; Ok(Json(RepositoryLogResponse { workspace_id: api.config.workspace_id, - repository_id, + repository_key, default_selector, limit, items: commits, @@ -14381,10 +14373,7 @@ fn working_directory_repository_options( .iter() .map(|repository| WorkingDirectoryRepositoryOption { id: repository.id.clone(), - display_name: repository - .display_name - .clone() - .unwrap_or_else(|| repository.id.clone()), + display_name: repository.repository_key.clone(), default_selector: repository.default_selector.clone(), }) .collect() @@ -16543,7 +16532,7 @@ mod tests { .upsert_repository(&RepositoryRecord { workspace_id: other_workspace.workspace_id.clone(), repository_id: "foreign".to_string(), - name: "Foreign".to_string(), + repository_key: "foreign".to_string(), kind: "git".to_string(), provider: Some("git".to_string()), source: workspace_api::RepositorySource { @@ -17942,7 +17931,7 @@ mod tests { let repositories = vec![RepositoryRecord { workspace_id: "remote-workspace".to_string(), repository_id: "main".to_string(), - name: "Main".to_string(), + repository_key: "main".to_string(), kind: "git".to_string(), provider: Some("git".to_string()), source_fingerprint: crate::repository_source::repository_source_fingerprint(&source), @@ -17982,6 +17971,7 @@ mod tests { }; config.repositories = vec![ConfiguredRepository { id: TEST_REPOSITORY_ID.to_string(), + repository_key: "test-repository".to_string(), provider: "git".to_string(), source_fingerprint: crate::repository_source::repository_source_fingerprint(&source), source, @@ -17989,7 +17979,6 @@ mod tests { observed_status: workspace_api::RepositoryObservedStatus::Unverified, observed_at: None, path: Some(workspace_root), - display_name: Some("Test Repository".to_string()), default_selector: Some("HEAD".to_string()), }]; config @@ -18135,8 +18124,8 @@ mod tests { operation_key: "create-auth".to_owned(), display_name: "Auth Workspace".to_owned(), repository: crate::workspace_catalog::InitialRepositoryIntent { + repository_key: "main".to_string(), uri: repository.display().to_string(), - display_name: None, default_ref: None, }, }, @@ -18262,8 +18251,8 @@ mod tests { operation_key: "create-authenticated-workspace".to_owned(), display_name: "Authenticated Workspace".to_owned(), repository: crate::workspace_catalog::InitialRepositoryIntent { + repository_key: "main".to_string(), uri: created_repository.display().to_string(), - display_name: Some("Main".to_owned()), default_ref: Some("develop".to_owned()), }, }; @@ -18415,8 +18404,7 @@ mod tests { let repositories_uri = format!("/api/w/{}/repositories", workspace.workspace.workspace_id); let repository_request = serde_json::json!({ - "repository_id": "documentation", - "display_name": "Documentation", + "repository_key": "documentation", "source": temp.path().join("documentation").display().to_string(), "default_ref": "main" }); @@ -18730,8 +18718,8 @@ mod tests { operation_key: "create-a".to_string(), display_name: "Workspace A".to_string(), repository: crate::workspace_catalog::InitialRepositoryIntent { + repository_key: "main".to_string(), uri: repository_a.display().to_string(), - display_name: None, default_ref: None, }, }, @@ -18744,8 +18732,8 @@ mod tests { operation_key: "create-b".to_string(), display_name: "Workspace B".to_string(), repository: crate::workspace_catalog::InitialRepositoryIntent { + repository_key: "main".to_string(), uri: repository_b.display().to_string(), - display_name: None, default_ref: None, }, }, @@ -22217,7 +22205,7 @@ mod tests { .upsert_repository(&RepositoryRecord { workspace_id: api.config.workspace_id.clone(), repository_id: repository_id.to_string(), - name: repository_id.to_string(), + repository_key: repository_id.to_string(), kind: "git".to_string(), provider: Some("git".to_string()), source: workspace_api::RepositorySource { @@ -24987,9 +24975,7 @@ mod tests { .upsert_repository(&RepositoryRecord { workspace_id: TEST_WORKSPACE_ID.to_string(), repository_id: configured_repository.id, - name: configured_repository - .display_name - .unwrap_or_else(|| "Test Repository".to_string()), + repository_key: configured_repository.repository_key, kind: "git".to_string(), provider: Some(configured_repository.provider), source: configured_repository.source, @@ -25289,7 +25275,10 @@ mod tests { let repositories = get_json(app.clone(), "/api/repositories").await; let typed_repositories: workspace_api::RepositoryListResponse = serde_json::from_value(repositories.clone()).unwrap(); - assert_eq!(typed_repositories.items[0].id, TEST_REPOSITORY_ID); + assert_eq!( + typed_repositories.items[0].repository_key, + TEST_REPOSITORY_ID + ); assert_eq!(repositories["items"][0]["id"], TEST_REPOSITORY_ID); assert_eq!(repositories["items"][0]["kind"], "git"); assert_eq!( @@ -25759,6 +25748,7 @@ mod tests { let mut config = test_server_config(root.path()); config.repositories = vec![ConfiguredRepository { id: "files".to_string(), + repository_key: "files".to_string(), provider: "local_fs".to_string(), source: workspace_api::RepositorySource { kind: workspace_api::RepositorySourceKind::LocalPath, @@ -25769,7 +25759,6 @@ mod tests { observed_status: workspace_api::RepositoryObservedStatus::Unverified, observed_at: None, path: Some(root.path().to_path_buf()), - display_name: None, default_selector: None, }]; let store = test_control_store(&config); diff --git a/crates/workspace-server/src/store.rs b/crates/workspace-server/src/store.rs index 137487b6..3b6e6837 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -272,6 +272,11 @@ const MIGRATIONS: &[Migration] = &[ name: "create durable Workdir removal operations", apply: crate::workdir_removal::create_workdir_removal_operations, }, + Migration { + version: 50, + name: "replace public Repository ids with immutable Workspace keys", + apply: migrate_repository_identity_to_keys, + }, ]; struct Migration { @@ -323,7 +328,7 @@ pub struct WorkerCreateReservation { pub struct RepositoryRecord { pub workspace_id: String, pub repository_id: String, - pub name: String, + pub repository_key: String, pub kind: String, pub provider: Option, pub source: RepositorySource, @@ -879,6 +884,16 @@ pub trait ControlPlaneStore: Send + Sync { workspace_id: &str, repository_id: &str, ) -> Result>; + fn get_repository_by_key( + &self, + workspace_id: &str, + repository_key: &str, + ) -> Result> { + Ok(self + .list_repositories(workspace_id)? + .into_iter() + .find(|repository| repository.repository_key == repository_key)) + } fn list_repositories(&self, workspace_id: &str) -> Result>; fn put_flow_source_for_kind( @@ -1914,11 +1929,11 @@ impl ControlPlaneStore for SqliteWorkspaceStore { read_workspace_record, )?; let repository = tx.query_row( - r#"SELECT workspace_id, repository_id, name, kind, provider, + r#"SELECT workspace_id, repository_id, repository_key, kind, provider, source_kind, source_uri, default_ref, source_revision, source_fingerprint, observed_status, observed_at, created_at, updated_at - FROM repositories WHERE workspace_id = ?1 AND repository_id = ?2"#, - params![workspace.workspace_id, record.repository.repository_id], + FROM repositories WHERE workspace_id = ?1 AND repository_key = ?2"#, + params![workspace.workspace_id, record.repository.repository_key], read_repository_record, )?; let config_revision = crate::config_source::load_state(&tx, &workspace.workspace_id)? @@ -1953,15 +1968,19 @@ impl ControlPlaneStore for SqliteWorkspaceStore { } let existing_repository = tx .query_row( - r#"SELECT workspace_id, repository_id, name, kind, provider, + r#"SELECT workspace_id, repository_id, repository_key, kind, provider, source_kind, source_uri, default_ref, source_revision, source_fingerprint, observed_status, observed_at, created_at, updated_at - FROM repositories WHERE workspace_id = ?1 AND repository_id = ?2"#, - params![record.repository.workspace_id, record.repository.repository_id], + FROM repositories WHERE workspace_id = ?1 AND repository_key = ?2"#, + params![record.repository.workspace_id, record.repository.repository_key], read_repository_record, ) .optional()?; - if existing_repository.as_ref() != Some(&record.repository) { + if existing_repository.as_ref().is_none_or(|existing| { + let mut requested = record.repository.clone(); + requested.repository_id.clone_from(&existing.repository_id); + existing != &requested + }) { return Err(Error::WorkspaceConfigConflict( "Workspace initial repository already exists with different metadata" .to_string(), @@ -1993,14 +2012,14 @@ impl ControlPlaneStore for SqliteWorkspaceStore { )?; tx.execute( r#"INSERT INTO repositories ( - workspace_id, repository_id, name, kind, provider, uri, + workspace_id, repository_id, repository_key, kind, provider, uri, source_kind, source_uri, default_ref, source_revision, source_fingerprint, observed_status, observed_at, created_at, updated_at ) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15)"#, params![ record.repository.workspace_id, record.repository.repository_id, - record.repository.name, + record.repository.repository_key, record.repository.kind, record.repository.provider, record.repository.source.uri, @@ -2191,15 +2210,15 @@ impl ControlPlaneStore for SqliteWorkspaceStore { } fn upsert_repository(&self, record: &RepositoryRecord) -> Result<()> { + validate_repository_record_identity(record)?; self.with_conn(|conn| { conn.execute( r#"INSERT INTO repositories ( - workspace_id, repository_id, name, kind, provider, uri, + workspace_id, repository_id, repository_key, kind, provider, uri, source_kind, source_uri, default_ref, source_revision, source_fingerprint, observed_status, observed_at, created_at, updated_at ) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15) - ON CONFLICT(workspace_id, repository_id) DO UPDATE SET - name = excluded.name, + ON CONFLICT(workspace_id, repository_key) DO UPDATE SET kind = excluded.kind, provider = excluded.provider, default_ref = excluded.default_ref, @@ -2209,7 +2228,7 @@ impl ControlPlaneStore for SqliteWorkspaceStore { params![ record.workspace_id, record.repository_id, - record.name, + record.repository_key, record.kind, record.provider, record.source.uri, @@ -2229,16 +2248,17 @@ impl ControlPlaneStore for SqliteWorkspaceStore { } fn insert_repository(&self, record: &RepositoryRecord) -> Result { + validate_repository_record_identity(record)?; self.with_conn_mut(|conn| { let transaction = conn.transaction_with_behavior(TransactionBehavior::Immediate)?; let existing = transaction .query_row( - r#"SELECT workspace_id, repository_id, name, kind, provider, + r#"SELECT workspace_id, repository_id, repository_key, kind, provider, source_kind, source_uri, default_ref, source_revision, source_fingerprint, observed_status, observed_at, created_at, updated_at FROM repositories - WHERE workspace_id = ?1 AND repository_id = ?2"#, - params![record.workspace_id, record.repository_id], + WHERE workspace_id = ?1 AND repository_key = ?2"#, + params![record.workspace_id, record.repository_key], read_repository_record, ) .optional()?; @@ -2249,14 +2269,14 @@ impl ControlPlaneStore for SqliteWorkspaceStore { transaction.execute( r#"INSERT INTO repositories ( - workspace_id, repository_id, name, kind, provider, uri, + workspace_id, repository_id, repository_key, kind, provider, uri, source_kind, source_uri, default_ref, source_revision, source_fingerprint, observed_status, observed_at, created_at, updated_at ) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15)"#, params![ record.workspace_id, record.repository_id, - record.name, + record.repository_key, record.kind, record.provider, record.source.uri, @@ -2283,7 +2303,7 @@ impl ControlPlaneStore for SqliteWorkspaceStore { ) -> Result> { self.with_conn(|conn| { conn.query_row( - r#"SELECT workspace_id, repository_id, name, kind, provider, + r#"SELECT workspace_id, repository_id, repository_key, kind, provider, source_kind, source_uri, default_ref, source_revision, source_fingerprint, observed_status, observed_at, created_at, updated_at FROM repositories @@ -2296,15 +2316,35 @@ impl ControlPlaneStore for SqliteWorkspaceStore { }) } + fn get_repository_by_key( + &self, + workspace_id: &str, + repository_key: &str, + ) -> Result> { + self.with_conn(|conn| { + conn.query_row( + r#"SELECT workspace_id, repository_id, repository_key, kind, provider, + source_kind, source_uri, default_ref, source_revision, + source_fingerprint, observed_status, observed_at, created_at, updated_at + FROM repositories + WHERE workspace_id = ?1 AND repository_key = ?2"#, + params![workspace_id, repository_key], + read_repository_record, + ) + .optional() + .map_err(Error::from) + }) + } + fn list_repositories(&self, workspace_id: &str) -> Result> { self.with_conn(|conn| { let mut stmt = conn.prepare( - r#"SELECT workspace_id, repository_id, name, kind, provider, + r#"SELECT workspace_id, repository_id, repository_key, kind, provider, source_kind, source_uri, default_ref, source_revision, source_fingerprint, observed_status, observed_at, created_at, updated_at FROM repositories WHERE workspace_id = ?1 - ORDER BY repository_id ASC"#, + ORDER BY repository_key ASC"#, )?; let rows = stmt.query_map(params![workspace_id], read_repository_record)?; rows.collect::, _>>() @@ -5332,6 +5372,12 @@ fn read_workspace_record(row: &rusqlite::Row<'_>) -> rusqlite::Result Result<()> { + workspace_api::validate_repository_key(&record.repository_key) + .map_err(|error| Error::InvalidInput(format!("invalid Repository key: {error}")))?; + Ok(()) +} + fn read_repository_record(row: &rusqlite::Row<'_>) -> rusqlite::Result { let source_kind_value = row.get::<_, String>(5)?; let source_kind = workspace_api::RepositorySourceKind::parse(&source_kind_value) @@ -5354,7 +5400,7 @@ fn read_repository_record(row: &rusqlite::Row<'_>) -> rusqlite::Result String { .to_ascii_lowercase() } +fn migrate_repository_identity_to_keys(conn: &Connection) -> Result<()> { + const REPOSITORY_REFERENCE_TABLES: &[&str] = &[ + "artifacts", + "merge_requests", + "typed_tickets", + "workdir_create_operations", + "workdir_registry", + "workdir_removal_operations", + ]; + + // Read-only preflight every legacy public id and every persisted relational + // reference before creating the mapping or mutating authority. + let legacy_repositories = { + let mut stmt = conn.prepare( + "SELECT workspace_id, repository_id FROM repositories ORDER BY workspace_id, repository_id", + )?; + stmt.query_map([], |row| { + Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?)) + })? + .collect::, _>>()? + }; + for (workspace_id, repository_key) in &legacy_repositories { + workspace_api::validate_repository_key(repository_key).map_err(|error| { + Error::Store(format!( + "repository identity migration rejected {workspace_id}/{repository_key:?}: {error}" + )) + })?; + } + + let tables = { + let mut stmt = conn.prepare( + "SELECT name FROM sqlite_schema WHERE type = 'table' AND name NOT LIKE 'sqlite_%' ORDER BY name", + )?; + stmt.query_map([], |row| row.get::<_, String>(0))? + .collect::, _>>()? + }; + for table in &tables { + let quoted = table.replace('"', "\"\""); + let pragma = format!("PRAGMA table_info(\"{quoted}\")"); + let mut stmt = conn.prepare(&pragma)?; + let has_repository_id = stmt + .query_map([], |row| row.get::<_, String>(1))? + .collect::, _>>()? + .iter() + .any(|column| column == "repository_id"); + if has_repository_id + && table != "repositories" + && table != "legacy_repositories" + && !REPOSITORY_REFERENCE_TABLES.contains(&table.as_str()) + { + return Err(Error::Store(format!( + "repository identity migration does not recognize repository_id authority in table {table}" + ))); + } + } + for table in REPOSITORY_REFERENCE_TABLES { + if !tables.iter().any(|candidate| candidate == table) { + continue; + } + let sql = format!( + r#"SELECT COUNT(*) + FROM "{table}" AS child + LEFT JOIN repositories AS repository + ON repository.workspace_id = child.workspace_id + AND repository.repository_id = child.repository_id + WHERE child.repository_id IS NOT NULL + AND repository.repository_id IS NULL"# + ); + let dangling: i64 = conn.query_row(&sql, [], |row| row.get(0))?; + if dangling != 0 { + return Err(Error::Store(format!( + "repository identity migration found {dangling} dangling same-Workspace reference(s) in {table}" + ))); + } + } + + conn.execute_batch( + r#" + CREATE TEMP TABLE repository_identity_v50 ( + workspace_id TEXT NOT NULL, + old_repository_id TEXT NOT NULL, + new_repository_id TEXT NOT NULL, + PRIMARY KEY (workspace_id, old_repository_id), + UNIQUE (new_repository_id) + ) WITHOUT ROWID; + "#, + )?; + for (workspace_id, old_repository_id) in &legacy_repositories { + conn.execute( + r#"INSERT INTO repository_identity_v50 ( + workspace_id, old_repository_id, new_repository_id + ) VALUES (?1, ?2, ?3)"#, + params![workspace_id, old_repository_id, Uuid::now_v7().to_string()], + )?; + } + + for table in REPOSITORY_REFERENCE_TABLES { + if !tables.iter().any(|candidate| candidate == table) { + continue; + } + let sql = format!( + r#"UPDATE "{table}" AS child + SET repository_id = ( + SELECT mapping.new_repository_id + FROM repository_identity_v50 AS mapping + WHERE mapping.workspace_id = child.workspace_id + AND mapping.old_repository_id = child.repository_id + ) + WHERE child.repository_id IS NOT NULL"# + ); + conn.execute(&sql, [])?; + } + + conn.execute_batch( + r#" + CREATE TABLE repositories_v50 ( + workspace_id TEXT NOT NULL, + repository_id TEXT NOT NULL PRIMARY KEY, + repository_key TEXT NOT NULL + CHECK(length(repository_key) BETWEEN 1 AND 64) + CHECK(repository_key NOT GLOB '*[^a-z0-9-]*') + CHECK(substr(repository_key, 1, 1) <> '-') + CHECK(substr(repository_key, -1, 1) <> '-'), + kind TEXT NOT NULL, + provider TEXT, + uri TEXT NOT NULL, + default_ref TEXT, + created_at TEXT NOT NULL, + updated_at TEXT NOT NULL, + source_kind TEXT NOT NULL, + source_uri TEXT NOT NULL, + source_revision INTEGER NOT NULL DEFAULT 1, + source_fingerprint TEXT NOT NULL, + observed_status TEXT NOT NULL DEFAULT 'unverified', + observed_at TEXT, + UNIQUE(workspace_id, repository_key), + UNIQUE(workspace_id, repository_id), + FOREIGN KEY(workspace_id) REFERENCES workspaces(workspace_id) ON DELETE CASCADE + ); + INSERT INTO repositories_v50 ( + workspace_id, repository_id, repository_key, kind, provider, uri, + default_ref, created_at, updated_at, source_kind, source_uri, + source_revision, source_fingerprint, observed_status, observed_at + ) + SELECT repository.workspace_id, + mapping.new_repository_id, + repository.repository_id, + repository.kind, + repository.provider, + repository.uri, + repository.default_ref, + repository.created_at, + repository.updated_at, + repository.source_kind, + repository.source_uri, + repository.source_revision, + repository.source_fingerprint, + repository.observed_status, + repository.observed_at + FROM repositories AS repository + JOIN repository_identity_v50 AS mapping + ON mapping.workspace_id = repository.workspace_id + AND mapping.old_repository_id = repository.repository_id; + DROP TABLE repositories; + ALTER TABLE repositories_v50 RENAME TO repositories; + CREATE INDEX repositories_workspace_provider_idx + ON repositories(workspace_id, provider); + DROP TABLE repository_identity_v50; + "#, + )?; + Ok(()) +} + fn require_workspace_account_owner(conn: &Connection) -> Result<()> { let actual_columns = { let mut statement = conn.prepare("PRAGMA table_info(workspaces)")?; @@ -9523,6 +9742,62 @@ pub(crate) fn apply_migrations_through(conn: &Connection, through_version: i64) continue; } + if migration.version == 50 { + conn.execute_batch( + "PRAGMA foreign_keys = OFF; PRAGMA legacy_alter_table = ON; BEGIN EXCLUSIVE;", + )?; + let result = (|| -> Result<()> { + (migration.apply)(conn)?; + let dangling_reference: Option<(String, String)> = conn + .query_row( + "SELECT name, sql FROM sqlite_schema \ + WHERE sql LIKE '%repositories_v50%' \ + OR sql LIKE '%repository_identity_v50%' LIMIT 1", + [], + |row| Ok((row.get(0)?, row.get(1)?)), + ) + .optional()?; + if let Some((object, sql)) = dangling_reference { + return Err(Error::Store(format!( + "migration 50 left a temporary Repository reference in `{object}`: {sql}" + ))); + } + let foreign_key_failures: i64 = + conn.query_row("SELECT COUNT(*) FROM pragma_foreign_key_check", [], |row| { + row.get(0) + })?; + if foreign_key_failures != 0 { + return Err(Error::Store(format!( + "migration 50 found {foreign_key_failures} foreign key violation(s)" + ))); + } + conn.execute( + "INSERT INTO __yoi_schema_migrations (version, name) VALUES (?1, ?2)", + params![migration.version, migration.name], + )?; + conn.execute_batch("COMMIT;")?; + Ok(()) + })(); + if result.is_err() && !conn.is_autocommit() { + conn.execute_batch("ROLLBACK;")?; + } + conn.execute_batch("PRAGMA legacy_alter_table = OFF; PRAGMA foreign_keys = ON;") + .map_err(|error| { + Error::Store(format!( + "migration 50 could not restore FK enforcement: {error}" + )) + })?; + let foreign_keys_enabled = + conn.query_row("PRAGMA foreign_keys", [], |row| row.get::<_, i64>(0))?; + if foreign_keys_enabled != 1 { + return Err(Error::Store( + "migration 50 did not restore foreign key enforcement".to_string(), + )); + } + result?; + continue; + } + if migration.version == 48 { // Rebuilding the parent Workspace table requires FK enforcement to be disabled // outside the transaction. The migration, verification, and schema marker still @@ -10150,7 +10425,7 @@ mod tests { let store = SqliteWorkspaceStore::open(&path).unwrap(); store .with_conn(|conn| { - assert_eq!(current_schema_version(conn)?, 49); + assert_eq!(current_schema_version(conn)?, 50); assert!(table_exists(conn, "workdir_removal_operations")?); let columns = table_columns(conn, "workdir_removal_operations")?; for required in [ @@ -10180,7 +10455,12 @@ mod tests { ); } let preserved: (String, String, String) = conn.query_row( - "SELECT workspace_id, repository_id, materialization_status FROM workdir_registry WHERE workdir_id='workdir-a'", + "SELECT workdir.workspace_id, repository.repository_key, workdir.materialization_status \ + FROM workdir_registry AS workdir \ + JOIN repositories AS repository \ + ON repository.workspace_id = workdir.workspace_id \ + AND repository.repository_id = workdir.repository_id \ + WHERE workdir.workdir_id='workdir-a'", [], |row| Ok((row.get(0)?, row.get(1)?, row.get(2)?)), )?; @@ -10250,11 +10530,11 @@ mod tests { assign_explicit_test_workspace_owner(&conn); apply_migrations(&conn).unwrap(); - assert_eq!(current_schema_version(&conn).unwrap(), 49); + assert_eq!(current_schema_version(&conn).unwrap(), 50); let remote = conn .query_row( "SELECT source_kind, source_uri, source_revision, source_fingerprint, observed_status \ - FROM repositories WHERE workspace_id = 'workspace-a' AND repository_id = 'remote'", + FROM repositories WHERE workspace_id = 'workspace-a' AND repository_key = 'remote'", [], |row| { Ok(( @@ -10276,7 +10556,7 @@ mod tests { let invalid = conn .query_row( "SELECT source_kind, observed_status FROM repositories \ - WHERE workspace_id = 'workspace-a' AND repository_id = 'invalid'", + WHERE workspace_id = 'workspace-a' AND repository_key = 'invalid'", [], |row| Ok((row.get::<_, String>(0)?, row.get::<_, String>(1)?)), ) @@ -10329,7 +10609,7 @@ mod tests { let before = std::fs::read(&path).unwrap(); let plan = SqliteWorkspaceStore::migration_plan(&path).unwrap(); assert_eq!(plan.current_schema_version, 36); - assert_eq!(plan.target_schema_version, 49); + assert_eq!(plan.target_schema_version, 50); assert!(plan.migration_required); assert_eq!(plan.worker_count, 1); assert_eq!(plan.mappings[0].legacy_worker_id, 7); @@ -10343,7 +10623,7 @@ mod tests { store .with_conn(|conn| { assert!(table_exists(conn, "worker_diagnostics_archives")?); - assert_eq!(current_schema_version(conn)?, 49); + assert_eq!(current_schema_version(conn)?, 50); Ok(()) }) .unwrap(); @@ -10483,7 +10763,7 @@ mod tests { ), ] ); - assert_eq!(current_schema_version(&conn).unwrap(), 49); + assert_eq!(current_schema_version(&conn).unwrap(), 50); let foreign_key_error: Option = conn .query_row("PRAGMA foreign_key_check", [], |row| row.get(0)) .optional() @@ -10613,7 +10893,7 @@ INSERT INTO worker_orphan_diagnostics ( assign_explicit_test_workspace_owner(&conn); apply_migrations(&conn).unwrap(); - assert_eq!(current_schema_version(&conn).unwrap(), 49); + assert_eq!(current_schema_version(&conn).unwrap(), 50); assert!(!table_exists(&conn, "worker_control_delegation_operations").unwrap()); let controller_worker_id: String = conn .query_row( @@ -10731,7 +11011,7 @@ INSERT INTO worker_orphan_diagnostics ( apply_migrations(&conn).unwrap(); - assert_eq!(current_schema_version(&conn).unwrap(), 49); + assert_eq!(current_schema_version(&conn).unwrap(), 50); assert!(table_exists(&conn, "worker_workdir_attachment_reservations").unwrap()); } @@ -10750,7 +11030,7 @@ INSERT INTO worker_orphan_diagnostics ( assign_explicit_test_workspace_owner(&conn); apply_migrations(&conn).unwrap(); - assert_eq!(current_schema_version(&conn).unwrap(), 49); + assert_eq!(current_schema_version(&conn).unwrap(), 50); let settings = conn .query_row( "SELECT settings_revision, language FROM workspace_memory_settings \ @@ -10791,7 +11071,7 @@ CREATE TABLE flow_events (event_id TEXT PRIMARY KEY); apply_migrations(&conn).unwrap(); - assert_eq!(current_schema_version(&conn).unwrap(), 49); + assert_eq!(current_schema_version(&conn).unwrap(), 50); assert!(table_exists(&conn, "flow_sources").unwrap()); assert!(table_exists(&conn, "flow_source_revisions").unwrap()); assert!(!table_exists(&conn, "flow_instances").unwrap()); @@ -10859,7 +11139,7 @@ INSERT INTO worker_workdir_attachment_reservations ( assign_explicit_test_workspace_owner(&conn); apply_migrations(&conn).unwrap(); - assert_eq!(current_schema_version(&conn).unwrap(), 49); + assert_eq!(current_schema_version(&conn).unwrap(), 50); let repositories_sql: String = conn .query_row( "SELECT sql FROM sqlite_master WHERE type = 'table' AND name = 'repositories'", @@ -10867,7 +11147,8 @@ INSERT INTO worker_workdir_attachment_reservations ( |row| row.get(0), ) .unwrap(); - assert!(repositories_sql.contains("PRIMARY KEY (workspace_id, repository_id)")); + assert!(repositories_sql.contains("repository_id TEXT NOT NULL PRIMARY KEY")); + assert!(repositories_sql.contains("UNIQUE(workspace_id, repository_key)")); let preserved: (i64, i64, i64) = ( conn.query_row("SELECT COUNT(*) FROM artifacts", [], |row| row.get(0)) .unwrap(), @@ -10891,14 +11172,20 @@ INSERT INTO worker_workdir_attachment_reservations ( assert_eq!(foreign_key_violations, 0); conn.execute( r#"INSERT INTO repositories ( - workspace_id, repository_id, name, kind, uri, created_at, updated_at - ) VALUES ('workspace-b', 'main', 'Other Main', 'git', '/repo-b', '2', '2')"#, + workspace_id, repository_id, repository_key, kind, provider, uri, default_ref, + created_at, updated_at, source_kind, source_uri, source_revision, + source_fingerprint, observed_status + ) VALUES ( + 'workspace-b', '01890f47-3c22-7cc0-98c4-dc0c0c07398f', 'main', + 'git', 'local', '/repo-b', 'HEAD', '2', '2', 'local', '/repo-b', 1, + 'sha256:test', 'unverified' + )"#, [], ) .unwrap(); assert_eq!( conn.query_row( - "SELECT COUNT(*) FROM repositories WHERE repository_id = 'main'", + "SELECT COUNT(*) FROM repositories WHERE repository_key = 'main'", [], |row| row.get::<_, i64>(0), ) @@ -11001,7 +11288,7 @@ INSERT INTO workdir_registry ( .upsert_repository(&RepositoryRecord { workspace_id: "workspace-a".to_string(), repository_id: "main".to_string(), - name: "Main".to_string(), + repository_key: "main".to_string(), kind: "git".to_string(), provider: Some("git".to_string()), source: RepositorySource { @@ -11042,7 +11329,7 @@ INSERT INTO workdir_registry ( let db = dir.path().join("control-plane.sqlite"); let store = SqliteWorkspaceStore::open(&db).unwrap(); - assert_eq!(store.schema_version().await.unwrap(), 49); + assert_eq!(store.schema_version().await.unwrap(), 50); assert!( !store .with_conn(|conn| table_exists(conn, "worker_workspace_credentials")) @@ -11059,7 +11346,7 @@ INSERT INTO workdir_registry ( store.upsert_workspace(&record).await.unwrap(); let reopened = SqliteWorkspaceStore::open(&db).unwrap(); - assert_eq!(reopened.schema_version().await.unwrap(), 49); + assert_eq!(reopened.schema_version().await.unwrap(), 50); assert_eq!( reopened.get_workspace("local-dev").await.unwrap(), Some(record) @@ -11825,7 +12112,7 @@ INSERT INTO worker_registry ( let migrated = SqliteWorkspaceStore::open(&db_path).unwrap(); migrated .with_conn(|conn| { - assert_eq!(current_schema_version(conn)?, 49); + assert_eq!(current_schema_version(conn)?, 50); assert_eq!( conn.query_row("PRAGMA foreign_keys", [], |row| row.get::<_, i64>(0))?, 1, @@ -11912,7 +12199,7 @@ INSERT INTO worker_registry ( .upsert_repository(&RepositoryRecord { workspace_id: "workspace-role".to_string(), repository_id: "main".to_string(), - name: "Main".to_string(), + repository_key: "main".to_string(), kind: "git".to_string(), provider: Some("git".to_string()), source: RepositorySource { @@ -12176,7 +12463,7 @@ INSERT INTO worker_registry ( assert_eq!(current_schema_version(&conn).unwrap(), 44); apply_migrations(&conn).unwrap(); - assert_eq!(current_schema_version(&conn).unwrap(), 49); + assert_eq!(current_schema_version(&conn).unwrap(), 50); assert!(table_exists(&conn, "workdir_create_operations").unwrap()); let columns = table_columns(&conn, "workdir_create_operations").unwrap(); for required in [ @@ -12203,7 +12490,7 @@ INSERT INTO worker_registry ( assert_eq!(current_schema_version(&conn).unwrap(), 45); apply_migrations(&conn).unwrap(); - assert_eq!(current_schema_version(&conn).unwrap(), 49); + assert_eq!(current_schema_version(&conn).unwrap(), 50); for table in [ "repository_ssh_credentials", "repository_ssh_credential_revisions", @@ -12230,7 +12517,7 @@ INSERT INTO worker_registry ( assert_eq!(current_schema_version(&conn).unwrap(), 46); apply_migrations(&conn).unwrap(); - assert_eq!(current_schema_version(&conn).unwrap(), 49); + assert_eq!(current_schema_version(&conn).unwrap(), 50); let columns = table_columns(&conn, "workdir_create_operations").unwrap(); for required in [ "source_kind", @@ -12258,19 +12545,173 @@ INSERT INTO worker_registry ( } } + #[test] + fn schema_v50_rekeys_repositories_and_same_workspace_references() { + let conn = workspace_owner_schema_47(); + apply_migrations_through(&conn, 49).unwrap(); + conn.execute_batch( + r#" + INSERT INTO accounts ( + account_id, kind, handle, display_name, created_at, updated_at + ) VALUES ('owner-account', 'user', 'owner', 'Owner', '1', '1'); + INSERT INTO workspaces ( + workspace_id, display_name, state, created_at, updated_at, owner_account_id + ) VALUES + ('workspace-a', 'Workspace A', 'active', '1', '1', 'owner-account'), + ('workspace-b', 'Workspace B', 'active', '1', '1', 'owner-account'); + INSERT INTO repositories ( + workspace_id, repository_id, name, kind, provider, uri, default_ref, + created_at, updated_at, source_kind, source_uri, source_revision, + source_fingerprint, observed_status + ) VALUES + ('workspace-a', 'main', 'Legacy A', 'git', 'git', '/repo-a', 'develop', + '1', '1', 'local_path', '/repo-a', 1, 'sha256:a', 'unverified'), + ('workspace-b', 'main', 'Legacy B', 'git', 'git', '/repo-b', 'develop', + '1', '1', 'local_path', '/repo-b', 1, 'sha256:b', 'unverified'); + INSERT INTO artifacts ( + workspace_id, artifact_id, kind, uri, created_at, + created_by_kind, created_by_key, created_by_display, repository_id + ) VALUES + ('workspace-a', 'artifact-a', 'report', 'artifact://a', '1', + 'worker', 'W-1', 'Worker 1', 'main'), + ('workspace-b', 'artifact-b', 'report', 'artifact://b', '1', + 'worker', 'W-2', 'Worker 2', 'main'); + INSERT INTO workdir_registry ( + workspace_id, workdir_id, runtime_id, repository_id, + materialization_status, cleanliness, created_at, updated_at + ) VALUES + ('workspace-a', 'workdir-a', 'runtime-a', 'main', 'present', 'clean', '1', '1'), + ('workspace-b', 'workdir-b', 'runtime-b', 'main', 'present', 'clean', '1', '1'); + "#, + ) + .unwrap(); + + apply_migrations(&conn).unwrap(); + + assert_eq!(current_schema_version(&conn).unwrap(), 50); + let repositories = { + let mut stmt = conn + .prepare( + "SELECT workspace_id, repository_id, repository_key FROM repositories ORDER BY workspace_id", + ) + .unwrap(); + stmt.query_map([], |row| { + Ok(( + row.get::<_, String>(0)?, + row.get::<_, String>(1)?, + row.get::<_, String>(2)?, + )) + }) + .unwrap() + .collect::>>() + .unwrap() + }; + assert_eq!(repositories.len(), 2); + assert_eq!(repositories[0].2, "main"); + assert_eq!(repositories[1].2, "main"); + assert_ne!(repositories[0].1, repositories[1].1); + for (_, repository_id, _) in &repositories { + assert_eq!(Uuid::parse_str(repository_id).unwrap().get_version_num(), 7); + } + for (workspace_id, repository_id, _) in &repositories { + let artifact_repository_id: String = conn + .query_row( + "SELECT repository_id FROM artifacts WHERE workspace_id = ?1", + params![workspace_id], + |row| row.get(0), + ) + .unwrap(); + let workdir_repository_id: String = conn + .query_row( + "SELECT repository_id FROM workdir_registry WHERE workspace_id = ?1", + params![workspace_id], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(&artifact_repository_id, repository_id); + assert_eq!(&workdir_repository_id, repository_id); + } + assert!( + table_columns(&conn, "repositories") + .unwrap() + .iter() + .all(|column| column != "name") + ); + let foreign_key_failures: i64 = conn + .query_row("SELECT COUNT(*) FROM pragma_foreign_key_check", [], |row| { + row.get(0) + }) + .unwrap(); + assert_eq!(foreign_key_failures, 0); + } + + #[test] + fn schema_v50_preflight_rolls_back_invalid_legacy_repository_key() { + let conn = workspace_owner_schema_47(); + apply_migrations_through(&conn, 49).unwrap(); + conn.execute_batch( + r#" + INSERT INTO accounts ( + account_id, kind, handle, display_name, created_at, updated_at + ) VALUES ('owner-account', 'user', 'owner', 'Owner', '1', '1'); + INSERT INTO workspaces ( + workspace_id, display_name, state, created_at, updated_at, owner_account_id + ) VALUES ('workspace-a', 'Workspace A', 'active', '1', '1', 'owner-account'); + INSERT INTO repositories ( + workspace_id, repository_id, name, kind, provider, uri, default_ref, + created_at, updated_at, source_kind, source_uri, source_revision, + source_fingerprint, observed_status + ) VALUES ( + 'workspace-a', 'Invalid_Key', 'Legacy', 'git', 'git', '/repo', 'develop', + '1', '1', 'local_path', '/repo', 1, 'sha256:a', 'unverified' + ); + "#, + ) + .unwrap(); + let schema_before: String = conn + .query_row( + "SELECT sql FROM sqlite_schema WHERE type = 'table' AND name = 'repositories'", + [], + |row| row.get(0), + ) + .unwrap(); + + let error = apply_migrations(&conn).unwrap_err().to_string(); + + assert!(error.contains("Invalid_Key"), "{error}"); + assert!(error.contains("lowercase ASCII"), "{error}"); + assert_eq!(current_schema_version(&conn).unwrap(), 49); + let schema_after: String = conn + .query_row( + "SELECT sql FROM sqlite_schema WHERE type = 'table' AND name = 'repositories'", + [], + |row| row.get(0), + ) + .unwrap(); + assert_eq!(schema_after, schema_before); + let persisted: (String, String) = conn + .query_row( + "SELECT repository_id, name FROM repositories WHERE workspace_id = 'workspace-a'", + [], + |row| Ok((row.get(0)?, row.get(1)?)), + ) + .unwrap(); + assert_eq!(persisted, ("Invalid_Key".to_string(), "Legacy".to_string())); + } + #[test] fn server_refuses_a_database_from_a_newer_schema_generation() { let conn = Connection::open_in_memory().unwrap(); configure_sqlite(&conn).unwrap(); apply_migrations(&conn).unwrap(); conn.execute( - "INSERT INTO __yoi_schema_migrations (version, name) VALUES (50, 'future')", + "INSERT INTO __yoi_schema_migrations (version, name) VALUES (51, 'future')", [], ) .unwrap(); let error = apply_migrations(&conn).unwrap_err().to_string(); - assert!(error.contains("schema version 50 is newer"), "{error}"); + assert!(error.contains("schema version 51 is newer"), "{error}"); assert!(error.contains("refusing to serve"), "{error}"); } @@ -12494,7 +12935,7 @@ VALUES ('workspace-b', 'ticket-b', 'related', 'ticket-a', NULL, 'tester', '2026- assign_explicit_test_workspace_owner(&conn); apply_migrations(&mut conn).unwrap(); - assert_eq!(current_schema_version(&conn).unwrap(), 49); + assert_eq!(current_schema_version(&conn).unwrap(), 50); let workspace_id: Option = conn .query_row( "SELECT workspace_id FROM trusted_runtime_records WHERE runtime_id = 'runtime-a'", @@ -12989,13 +13430,11 @@ WHERE workspace_id = 'workspace-a' [ "workspace_id", "repository_id", - "name", + "repository_key", "kind", "provider", "uri", "default_ref", - "auth_ref_kind", - "auth_ref_key", "created_at", "updated_at", "source_kind", @@ -13123,7 +13562,7 @@ WHERE workspace_id = 'workspace-a' assign_explicit_test_workspace_owner(&conn); let store = SqliteWorkspaceStore::from_connection(conn).unwrap(); - assert_eq!(store.schema_version().await.unwrap(), 49); + assert_eq!(store.schema_version().await.unwrap(), 50); store .with_conn(|conn| { @@ -13312,7 +13751,7 @@ CREATE TABLE ticket_assignment_operations ( #[tokio::test] async fn repository_records_round_trip() { let store = SqliteWorkspaceStore::in_memory().unwrap(); - assert_eq!(store.schema_version().await.unwrap(), 49); + assert_eq!(store.schema_version().await.unwrap(), 50); let workspace = WorkspaceRecord { workspace_id: "local-dev".to_string(), owner_account_id: "owner-account".to_string(), @@ -13326,7 +13765,7 @@ CREATE TABLE ticket_assignment_operations ( let repository = RepositoryRecord { workspace_id: "local-dev".to_string(), repository_id: "main".to_string(), - name: "Yoi".to_string(), + repository_key: "yoi".to_string(), kind: "git".to_string(), provider: Some("git".to_string()), source: RepositorySource { @@ -13371,7 +13810,8 @@ CREATE TABLE ticket_assignment_operations ( store.upsert_workspace(&other_workspace).await.unwrap(); let mut other_repository = repository.clone(); other_repository.workspace_id = other_workspace.workspace_id.clone(); - other_repository.name = "Other Yoi".to_string(); + other_repository.repository_id = "other-main".to_string(); + other_repository.repository_key = "other-yoi".to_string(); other_repository.source.uri = "/other/yoi".to_string(); other_repository.source_fingerprint = crate::repository_source::repository_source_fingerprint(&other_repository.source); @@ -13382,7 +13822,9 @@ CREATE TABLE ticket_assignment_operations ( Some(repository) ); assert_eq!( - store.get_repository("other-workspace", "main").unwrap(), + store + .get_repository("other-workspace", "other-main") + .unwrap(), Some(other_repository) ); } @@ -13390,7 +13832,7 @@ CREATE TABLE ticket_assignment_operations ( #[tokio::test] async fn memory_authority_records_round_trip_and_close_staging() { let store = SqliteWorkspaceStore::in_memory().unwrap(); - assert_eq!(store.schema_version().await.unwrap(), 49); + assert_eq!(store.schema_version().await.unwrap(), 50); let workspace = WorkspaceRecord { workspace_id: "local-dev".to_string(), owner_account_id: "owner-account".to_string(), @@ -13480,7 +13922,7 @@ CREATE TABLE ticket_assignment_operations ( .upsert_repository(&RepositoryRecord { workspace_id: workspace.workspace_id.clone(), repository_id: "repo".to_string(), - name: "Repository".to_string(), + repository_key: "repository".to_string(), kind: "git".to_string(), provider: Some("git".to_string()), source: RepositorySource { @@ -13803,7 +14245,7 @@ CREATE TABLE ticket_assignment_operations ( #[tokio::test] async fn account_and_login_records_round_trip() { let store = SqliteWorkspaceStore::in_memory().unwrap(); - assert_eq!(store.schema_version().await.unwrap(), 49); + assert_eq!(store.schema_version().await.unwrap(), 50); let now = "2026-07-22T00:00:00Z".to_string(); let account = AccountRecord { account_id: "acct-user-alice".to_string(), diff --git a/crates/workspace-server/src/workdir_create_operations.rs b/crates/workspace-server/src/workdir_create_operations.rs index 25fd910d..21f00cc0 100644 --- a/crates/workspace-server/src/workdir_create_operations.rs +++ b/crates/workspace-server/src/workdir_create_operations.rs @@ -369,7 +369,7 @@ mod tests { .upsert_repository(&RepositoryRecord { workspace_id: "workspace".to_string(), repository_id: "main".to_string(), - name: "main".to_string(), + repository_key: "main".to_string(), kind: "git".to_string(), provider: Some("git".to_string()), source: workspace_api::RepositorySource { diff --git a/crates/workspace-server/src/workdir_removal.rs b/crates/workspace-server/src/workdir_removal.rs index e8c104f4..67902829 100644 --- a/crates/workspace-server/src/workdir_removal.rs +++ b/crates/workspace-server/src/workdir_removal.rs @@ -951,7 +951,7 @@ mod tests { .upsert_repository(&RepositoryRecord { workspace_id: "workspace-a".to_string(), repository_id: "repository-a".to_string(), - name: "Repository A".to_string(), + repository_key: "repository-a".to_string(), kind: "git".to_string(), provider: Some("local".to_string()), source: RepositorySource { diff --git a/crates/workspace-server/src/workspace_catalog.rs b/crates/workspace-server/src/workspace_catalog.rs index 531eb6d0..a606dde3 100644 --- a/crates/workspace-server/src/workspace_catalog.rs +++ b/crates/workspace-server/src/workspace_catalog.rs @@ -13,17 +13,15 @@ use crate::store::{ }; use crate::{Error, Result}; -const DEFAULT_REPOSITORY_ID: &str = "main"; const MAX_DISPLAY_NAME_BYTES: usize = 200; const MAX_OPERATION_KEY_BYTES: usize = 200; #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[serde(deny_unknown_fields)] pub struct InitialRepositoryIntent { + pub repository_key: String, pub uri: String, #[serde(default)] - pub display_name: Option, - #[serde(default)] pub default_ref: Option, } @@ -102,14 +100,9 @@ impl WorkspaceCatalogService { normalize_required("display_name", request.display_name, MAX_DISPLAY_NAME_BYTES)?; let repository_source = validate_repository_source(&request.repository.uri)?; let repository_uri = repository_source.uri.clone(); - let repository_name = request - .repository - .display_name - .as_deref() - .map(str::trim) - .filter(|value| !value.is_empty()) - .unwrap_or("Main repository") - .to_string(); + workspace_api::validate_repository_key(&request.repository.repository_key) + .map_err(|error| Error::InvalidInput(format!("invalid Repository key: {error}")))?; + let repository_key = request.repository.repository_key.clone(); let default_ref = request .repository .default_ref @@ -132,8 +125,8 @@ impl WorkspaceCatalogService { requested_workspace_id.as_deref(), &display_name, Some(&owner_account_id), + &repository_key, &repository_uri, - &repository_name, &default_ref, ); let now = Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true); @@ -152,8 +145,8 @@ impl WorkspaceCatalogService { }, repository: RepositoryRecord { workspace_id, - repository_id: DEFAULT_REPOSITORY_ID.to_string(), - name: repository_name, + repository_id: Uuid::now_v7().to_string(), + repository_key: repository_key.clone(), kind: "git".to_string(), provider: Some("git".to_string()), source: repository_source.clone(), @@ -194,8 +187,8 @@ fn workspace_create_fingerprint( requested_workspace_id: Option<&str>, display_name: &str, owner_account_id: Option<&str>, + repository_key: &str, repository_uri: &str, - repository_name: &str, default_ref: &str, ) -> String { let payload = serde_json::json!({ @@ -203,9 +196,8 @@ fn workspace_create_fingerprint( "display_name": display_name, "owner_account_id": owner_account_id, "repository": { - "repository_id": DEFAULT_REPOSITORY_ID, + "repository_key": repository_key, "uri": repository_uri, - "display_name": repository_name, "default_ref": default_ref, "kind": "git", } @@ -258,7 +250,7 @@ mod tests { display_name: "Workspace A".to_string(), repository: InitialRepositoryIntent { uri: repository.path().display().to_string(), - display_name: None, + repository_key: "main".to_string(), default_ref: None, }, }; @@ -302,7 +294,7 @@ mod tests { display_name: "Workspace A".to_string(), repository: InitialRepositoryIntent { uri: repository.path().display().to_string(), - display_name: None, + repository_key: "main".to_string(), default_ref: None, }, }; @@ -355,7 +347,7 @@ mod tests { display_name: "Organization Workspace".to_string(), repository: InitialRepositoryIntent { uri: repository.path().display().to_string(), - display_name: None, + repository_key: "main".to_string(), default_ref: None, }, }, @@ -391,7 +383,7 @@ mod tests { display_name: "Owner A Workspace".to_string(), repository: InitialRepositoryIntent { uri: repository_a.path().display().to_string(), - display_name: None, + repository_key: "main".to_string(), default_ref: None, }, }, @@ -405,7 +397,7 @@ mod tests { display_name: "Owner B Workspace".to_string(), repository: InitialRepositoryIntent { uri: repository_b.path().display().to_string(), - display_name: None, + repository_key: "main".to_string(), default_ref: None, }, }, @@ -441,7 +433,7 @@ mod tests { display_name: "Remote Workspace".to_string(), repository: InitialRepositoryIntent { uri: "ssh://git@example.test/org/repository.git".to_string(), - display_name: Some("Remote Repository".to_string()), + repository_key: "remote".to_string(), default_ref: Some("main".to_string()), }, },