From 013f909b9b404b3f7f2652bc2f65888b0f39b4ce Mon Sep 17 00:00:00 2001 From: Hare Date: Fri, 2 Oct 2026 14:27:52 +0900 Subject: [PATCH] fix: require explicit Workspace repository key --- crates/server-api/src/lib.rs | 11 ++ crates/tui/src/backend_workspace_picker.rs | 116 +++++++++++++++--- .../workspace-server/src/workspace_catalog.rs | 43 +++++-- crates/yoi/src/main.rs | 33 ++++- web/workspace/tests/workspace-catalog.test.ts | 31 ++++- 5 files changed, 206 insertions(+), 28 deletions(-) diff --git a/crates/server-api/src/lib.rs b/crates/server-api/src/lib.rs index 089d7c65c..6f0797f73 100644 --- a/crates/server-api/src/lib.rs +++ b/crates/server-api/src/lib.rs @@ -11186,6 +11186,17 @@ mod tests { })) .is_err() ); + assert!( + serde_json::from_value::(serde_json::json!({ + "operation_key": "workspace-create-1", + "display_name": "Workspace", + "repository": { + "uri": "/srv/repositories/platform" + } + })) + .is_err(), + "the API wire shape must not supply a default Repository key" + ); } #[test] diff --git a/crates/tui/src/backend_workspace_picker.rs b/crates/tui/src/backend_workspace_picker.rs index ace33d7e5..af917b668 100644 --- a/crates/tui/src/backend_workspace_picker.rs +++ b/crates/tui/src/backend_workspace_picker.rs @@ -11,7 +11,7 @@ use ratatui::style::{Modifier, Style}; use ratatui::text::{Line, Span}; use ratatui::widgets::{Block, Borders, List, ListItem, ListState, Paragraph}; use std::error::Error; -use std::io::{self, IsTerminal, Write}; +use std::io::{self, BufRead, IsTerminal, Write}; use std::time::{SystemTime, UNIX_EPOCH}; type PickerResult = Result>; @@ -179,18 +179,47 @@ fn prompt_create_request() -> PickerResult } fn prompt_create_request_inner() -> PickerResult> { - println!("Create Workspace (leave display name empty to cancel)"); - let display_name = prompt_line("Workspace display name: ")?; + let stdin = io::stdin(); + let stdout = io::stdout(); + prompt_create_request_with_io(&mut stdin.lock(), &mut stdout.lock()) +} + +fn prompt_create_request_with_io( + input: &mut impl BufRead, + output: &mut impl Write, +) -> PickerResult> { + writeln!( + output, + "Create Workspace (leave display name empty to cancel)" + )?; + let Some(display_name) = prompt_line(input, output, "Workspace display name: ")? else { + return Ok(None); + }; if display_name.is_empty() { return Ok(None); } - let uri = prompt_line("Initial repository absolute path/URI: ")?; + let Some(uri) = prompt_line(input, output, "Initial repository absolute path/URI: ")? else { + return Ok(None); + }; if uri.is_empty() { - println!("Repository path/URI is required."); + writeln!(output, "Repository path/URI is required.")?; return Ok(None); } - let repository_key = prompt_line("Repository key [main]: ")?; - let default_ref = prompt_line("Default ref [repository default]: ")?; + let repository_key = loop { + let Some(repository_key) = prompt_line(input, output, "Repository key (required): ")? + else { + return Ok(None); + }; + if repository_key.is_empty() { + writeln!(output, "Repository key is required.")?; + continue; + } + break repository_key; + }; + let Some(default_ref) = prompt_line(input, output, "Default ref [repository default]: ")? + else { + return Ok(None); + }; let operation_key = format!( "tui-workspace-create-{}-{}", std::process::id(), @@ -204,27 +233,37 @@ fn prompt_create_request_inner() -> PickerResult PickerResult { - print!("{prompt}"); - io::stdout().flush()?; +fn prompt_line( + input: &mut impl BufRead, + output: &mut impl Write, + prompt: &str, +) -> PickerResult> { + write!(output, "{prompt}")?; + output.flush()?; let mut value = String::new(); - io::stdin().read_line(&mut value)?; - Ok(value.trim().to_string()) + if input.read_line(&mut value)? == 0 { + return Ok(None); + } + Ok(Some(value.trim().to_string())) } #[cfg(test)] mod tests { use super::*; + use std::io::Cursor; + + fn prompt_create(input: &str) -> (Option, String) { + let mut input = Cursor::new(input.as_bytes()); + let mut output = Vec::new(); + let request = prompt_create_request_with_io(&mut input, &mut output).unwrap(); + (request, String::from_utf8(output).unwrap()) + } #[test] fn picker_actions_distinguish_switch_refresh_create_and_cancel() { @@ -237,4 +276,47 @@ mod tests { WorkspacePickerAction::Cancel ); } + + #[test] + fn workspace_creation_preserves_an_explicit_non_main_repository_key() { + let (request, output) = prompt_create("Platform\n/srv/platform\nplatform\ndevelop\n"); + let request = request.expect("complete input should create a request"); + + assert_eq!(request.display_name, "Platform"); + assert_eq!(request.repository.uri, "/srv/platform"); + assert_eq!(request.repository.repository_key, "platform"); + assert_eq!(request.repository.default_ref.as_deref(), Some("develop")); + assert!(output.contains("Repository key (required): ")); + assert!(!output.contains("Repository key [main]")); + } + + #[test] + fn workspace_creation_reprompts_for_blank_repository_keys() { + let (request, output) = prompt_create("Platform\n/srv/platform\n\n \nplatform\n\n"); + let request = request.expect("a later explicit key should create a request"); + + assert_eq!(request.repository.repository_key, "platform"); + assert_eq!(output.matches("Repository key is required.").count(), 2); + assert_eq!(output.matches("Repository key (required): ").count(), 3); + } + + #[test] + fn workspace_creation_accepts_explicit_main_repository_key() { + let (request, _) = prompt_create("Workspace\n/srv/repository\nmain\n\n"); + + assert_eq!( + request.unwrap().repository.repository_key, + "main", + "main remains valid when the creator enters it explicitly" + ); + } + + #[test] + fn workspace_creation_cancels_on_eof_instead_of_retrying_forever() { + let (request, output) = prompt_create("Workspace\n/srv/repository\n \n"); + + assert!(request.is_none()); + assert_eq!(output.matches("Repository key is required.").count(), 1); + assert_eq!(output.matches("Repository key (required): ").count(), 2); + } } diff --git a/crates/workspace-server/src/workspace_catalog.rs b/crates/workspace-server/src/workspace_catalog.rs index 3dee38110..ac98cae2e 100644 --- a/crates/workspace-server/src/workspace_catalog.rs +++ b/crates/workspace-server/src/workspace_catalog.rs @@ -296,7 +296,7 @@ mod tests { display_name: "Workspace A".to_string(), repository: InitialRepositoryIntent { uri: repository.path().display().to_string(), - repository_key: "main".to_string(), + repository_key: "platform".to_string(), default_ref: None, }, }; @@ -322,13 +322,11 @@ mod tests { assert_eq!(signing_identity.algorithm, "ed25519"); assert!(signing_identity.public_key.is_some()); assert!(signing_identity.public_key_fingerprint.is_some()); - assert_eq!( - store - .list_repositories(&created.workspace.workspace_id) - .unwrap() - .len(), - 1 - ); + let repositories = store + .list_repositories(&created.workspace.workspace_id) + .unwrap(); + assert_eq!(repositories.len(), 1); + assert_eq!(repositories[0].repository_key, "platform"); assert!( store .load_workspace_config(&created.workspace.workspace_id) @@ -337,6 +335,35 @@ mod tests { ); } + #[test] + fn create_rejects_blank_repository_key_without_persisting_a_workspace() { + let store = Arc::new(SqliteWorkspaceStore::in_memory().unwrap()); + let service = WorkspaceCatalogService::new( + store.clone(), + Arc::new( + crate::workspace_signing_identity::InMemoryWorkspaceSigningMaterialStore::default(), + ), + ); + let repository = git_repository(); + let request = WorkspaceCreateRequest { + operation_key: "blank-repository-key".to_string(), + display_name: "Workspace A".to_string(), + repository: InitialRepositoryIntent { + uri: repository.path().display().to_string(), + repository_key: " ".to_string(), + default_ref: None, + }, + }; + + let error = service + .create(request, owner_account(store.as_ref())) + .unwrap_err() + .to_string(); + + assert!(error.contains("invalid Repository key"), "{error}"); + assert!(store.list_workspaces().unwrap().is_empty()); + } + #[tokio::test] async fn create_recovers_same_reserved_identity_after_material_write_failure() { let temp = tempfile::tempdir().unwrap(); diff --git a/crates/yoi/src/main.rs b/crates/yoi/src/main.rs index 6c36ba56e..1e72559f0 100644 --- a/crates/yoi/src/main.rs +++ b/crates/yoi/src/main.rs @@ -2986,7 +2986,7 @@ backend = "shared" "--display-name".to_string(), "Workspace A".to_string(), "--repository-key".to_string(), - "main".to_string(), + "platform".to_string(), "--repository".to_string(), repository.path().display().to_string(), "--default-ref".to_string(), @@ -3000,7 +3000,7 @@ backend = "shared" }; assert_eq!(options.backend_url, "http://backend.example"); assert_eq!(options.display_name, "Workspace A"); - assert_eq!(options.repository_key, "main"); + assert_eq!(options.repository_key, "platform"); assert_eq!(options.default_ref.as_deref(), Some("develop")); assert_eq!( options.repository_root, @@ -3009,6 +3009,35 @@ backend = "shared" assert!(!repository.path().join(".yoi/workspace.toml").exists()); } + #[test] + fn init_parsing_rejects_an_omitted_repository_key() { + let repository = tempfile::tempdir().unwrap(); + Command::new("git") + .args(["init", "-q"]) + .current_dir(repository.path()) + .status() + .unwrap(); + let resolver = FixedCliConnectionResolver { + backend_url: "http://unused.example", + }; + let args = vec![ + "--backend".to_string(), + "http://backend.example".to_string(), + "init".to_string(), + "--display-name".to_string(), + "Workspace A".to_string(), + "--repository".to_string(), + repository.path().display().to_string(), + ]; + + let error = parse_args_slice_with_connection_resolver(&args, &resolver) + .unwrap_err() + .to_string(); + + assert_eq!(error, "yoi init requires --repository-key KEY"); + assert!(!repository.path().join(".yoi/workspace.toml").exists()); + } + #[test] fn default_backend_target_selects_workspace_from_backend_not_repository_file() { let workspace = tempfile::tempdir().unwrap(); diff --git a/web/workspace/tests/workspace-catalog.test.ts b/web/workspace/tests/workspace-catalog.test.ts index 1839b2bef..186b4fdc5 100644 --- a/web/workspace/tests/workspace-catalog.test.ts +++ b/web/workspace/tests/workspace-catalog.test.ts @@ -1,7 +1,12 @@ declare const Deno: { test(name: string, fn: () => void | Promise): void; + readTextFile(path: URL): Promise; }; +function assert(condition: unknown, message: string): asserts condition { + if (!condition) throw new Error(message); +} + function assertEquals(actual: unknown, expected: unknown): void { if (JSON.stringify(actual) !== JSON.stringify(expected)) { throw new Error( @@ -88,7 +93,7 @@ Deno.test("workspace creation preserves caller-owned operation key across retry" operation_key: "web-create-1", display_name: "Alpha", repository: { - repository_key: "main", + repository_key: "platform", uri: "/srv/alpha", default_ref: "develop", }, @@ -113,3 +118,27 @@ Deno.test("workspace creation preserves caller-owned operation key across retry" ); assertEquals(bodies, [request, request]); }); + +Deno.test("workspace creation form starts with an explicit required Repository key", async () => { + const source = await Deno.readTextFile( + new URL("../src/routes/+page.svelte", import.meta.url), + ); + + assert( + source.includes('let repositoryKey = $state("");'), + "the form must start without an implicit Repository key", + ); + assert( + source.includes("