diff --git a/README.md b/README.md index 20156e4..a2e0360 100644 --- a/README.md +++ b/README.md @@ -92,6 +92,23 @@ DB_PORT=$(trop reserve --tag db) As with `trop reserve`, these reservations will be associated with the current directory, and thus will be automatically pruned when the directory is removed. +### Releasing reservations + +`trop release` without a tag filter removes every tagged and untagged +reservation at exactly the resolved path in one transaction. Descendant paths +are left alone unless `--recursive` is supplied. + +Use `--tag ` to remove only that tagged reservation, or +`--untagged-only` to remove only the untagged reservation. The two filters are +mutually exclusive, and a filter with no match succeeds as an idempotent no-op. +The same filter selects matching rows below the path when combined with +`--recursive`. + +Release follows the standard path guard: the target must be the current +directory, an ancestor, or a descendant. A sideways unrelated path is rejected +before mutation unless `--allow-unrelated-path`, the corresponding effective +configuration permission, or `--force` authorizes it. + For recurring reservation patterns, you add a "tropfile" (`trop.yaml`) file to your project root, which can then define a "reservation group" like so: ```yaml diff --git a/trop-cli/README.md b/trop-cli/README.md index 3c38467..1bddeac 100644 --- a/trop-cli/README.md +++ b/trop-cli/README.md @@ -135,6 +135,34 @@ phase. Normal mode reports only aggregate prune/expire counts on stderr, while Exhaustion errors distinguish skipped cleanup from an attempted cleanup and report that the remaining ports are reserved, excluded, or occupied. +### Releasing Reservations + +Without a tag filter, `trop release` removes every tagged and untagged +reservation at exactly the resolved path. Planning and deletion for that exact +path share one transaction, so a failure cannot leave only part of the +exact-path set deleted. Descendant reservations remain unless `--recursive` is +supplied. + +```bash +# Release every reservation for the current directory +trop release + +# Release one tagged reservation +trop release --tag web + +# Release only the untagged reservation +trop release --untagged-only +``` + +`--tag` and `--untagged-only` are mutually exclusive. A selector with no match +succeeds as an idempotent no-op, and either selector can be combined with +`--recursive` to select matching descendant rows. + +The target path must be the current directory, an ancestor, or a descendant. +Use `--allow-unrelated-path` to bypass only that relationship check, or +`--force` to authorize the release despite it. The effective +`allow_unrelated_path` configuration permission is honored as well. + ### Use in Build Scripts Example `justfile`: diff --git a/trop-cli/src/cli.rs b/trop-cli/src/cli.rs index 50d5c8b..c01b75d 100644 --- a/trop-cli/src/cli.rs +++ b/trop-cli/src/cli.rs @@ -338,8 +338,15 @@ impl Command { ); ConfigScope::Discover } - Self::Release(_) - | Self::Prune(_) + Self::Release(command) => { + set_true( + &mut command_line, + command.allow_unrelated_path, + ConfigField::AllowUnrelatedPath, + ); + ConfigScope::Discover + } + Self::Prune(_) | Self::AssertReservation(_) | Self::AssertPort(_) | Self::Exclude(_) diff --git a/trop-cli/src/commands/release.rs b/trop-cli/src/commands/release.rs index 1b11226..4aeaa65 100644 --- a/trop-cli/src/commands/release.rs +++ b/trop-cli/src/commands/release.rs @@ -32,6 +32,10 @@ pub struct ReleaseCommand { #[arg(long)] pub force: bool, + /// Allow operations on unrelated paths + #[arg(long)] + pub allow_unrelated_path: bool, + /// Perform a dry run #[arg(long)] pub dry_run: bool, @@ -51,11 +55,18 @@ impl ReleaseCommand { )); } - // 3. Open the database from the shared effective configuration. + // 3. Consume the path permission from the shared effective configuration. + let allow_unrelated_path = context.effective()?.allow_unrelated_path(); + + // 4. Open the database from the shared effective configuration. let mut db = context.open_database()?; // 5. Handle recursive release or single release if self.recursive { + if !self.force && !allow_unrelated_path { + Database::validate_path_relationship(&path, false).map_err(CliError::from)?; + } + // For recursive release, we need to find all reservations under this path // and release them one by one let all_reservations = @@ -84,7 +95,9 @@ impl ReleaseCommand { // Build release options for this reservation let options = ReleaseOptions::new(reservation.key().clone()) .with_force(self.force) - .with_allow_unrelated_path(true); // Already validated + // Validate the requested recursive root once. Descendant keys + // can be sideways from the CWD even when that root is an ancestor. + .with_allow_unrelated_path(true); // Build plan using database connection for reading let plan = ReleasePlan::new(options) @@ -122,7 +135,8 @@ impl ReleaseCommand { } } } else { - // Single release: build key and release it + // Exact release: an omitted filter selects every tag at this path. + let release_all_exact_path_tags = self.tag.is_none() && !self.untagged_only; let tag = if self.untagged_only { None } else { self.tag }; let key = ReservationKey::new(path, tag) @@ -130,12 +144,13 @@ impl ReleaseCommand { let options = ReleaseOptions::new(key) .with_force(self.force) - .with_allow_unrelated_path(true); // Path was resolved from CWD + .with_allow_unrelated_path(allow_unrelated_path); // Begin transaction for single release let tx = db.begin_transaction().map_err(CliError::from)?; let plan = ReleasePlan::new(options) + .with_all_exact_path_tags(release_all_exact_path_tags) .build_plan(&tx) .map_err(CliError::from)?; diff --git a/trop-cli/tests/common/mod.rs b/trop-cli/tests/common/mod.rs index 54436c4..ef593ae 100644 --- a/trop-cli/tests/common/mod.rs +++ b/trop-cli/tests/common/mod.rs @@ -204,7 +204,9 @@ impl TestEnv { /// /// Runs `trop release` for the given path. pub fn release(&self, path: &Path) { - self.command() + let mut command = self.command(); + command + .current_dir(&self.temp_path) .arg("release") .arg("--path") .arg(path) @@ -214,7 +216,9 @@ impl TestEnv { /// Release a reservation with a tag. pub fn release_with_tag(&self, path: &Path, tag: &str) { - self.command() + let mut command = self.command(); + command + .current_dir(&self.temp_path) .arg("release") .arg("--path") .arg(path) diff --git a/trop-cli/tests/error_handling.rs b/trop-cli/tests/error_handling.rs index 08f060b..4161e1f 100644 --- a/trop-cli/tests/error_handling.rs +++ b/trop-cli/tests/error_handling.rs @@ -127,6 +127,7 @@ fn test_success_exit_code() { .arg("release") .arg("--path") .arg(&test_path) + .current_dir(env.path()) .assert() .code(0); } @@ -771,6 +772,7 @@ fn test_release_idempotent_success() { .arg("release") .arg("--path") .arg(&test_path) + .current_dir(env.path()) .output() .unwrap(); @@ -874,6 +876,7 @@ fn test_release_nonexistent_clear_message() { .arg("release") .arg("--path") .arg(&test_path) + .current_dir(env.path()) .output() .unwrap(); diff --git a/trop-cli/tests/global_options.rs b/trop-cli/tests/global_options.rs index 4bd5d56..18fec36 100644 --- a/trop-cli/tests/global_options.rs +++ b/trop-cli/tests/global_options.rs @@ -99,6 +99,7 @@ fn test_verbose_flag_works_with_all_commands() { .arg("release") .arg("--path") .arg(&test_path) + .current_dir(env.path()) .assert() .success(); } @@ -207,6 +208,7 @@ fn test_quiet_flag_works_with_all_commands() { .arg("release") .arg("--path") .arg(&test_path) + .current_dir(env.path()) .output() .unwrap(); assert!(release_out.status.success()); diff --git a/trop-cli/tests/release_command.rs b/trop-cli/tests/release_command.rs index af2b7b1..cc4876e 100644 --- a/trop-cli/tests/release_command.rs +++ b/trop-cli/tests/release_command.rs @@ -10,8 +10,16 @@ mod common; +use assert_cmd::Command; use common::TestEnv; use predicates::prelude::*; +use rusqlite::Connection; + +fn related_command(env: &TestEnv) -> Command { + let mut command = env.command(); + command.current_dir(env.path()); + command +} // ============================================================================ // Basic Release Tests @@ -35,7 +43,7 @@ fn test_release_basic() { assert!(list_before.contains(&port.to_string())); // Release it - env.command() + related_command(&env) .arg("release") .arg("--path") .arg(&test_path) @@ -61,7 +69,7 @@ fn test_release_with_tag() { let port_api = env.reserve_with_tag(&test_path, "api"); // Release the "web" tag - env.command() + related_command(&env) .arg("release") .arg("--path") .arg(&test_path) @@ -86,8 +94,7 @@ fn test_release_without_path_uses_cwd() { let test_path = env.create_dir("test-project"); // Reserve implicitly so both operations use the canonical CWD identity. - let reserve_output = env - .command() + let reserve_output = related_command(&env) .arg("reserve") .arg("--allow-unrelated-path") .current_dir(&test_path) @@ -105,7 +112,7 @@ fn test_release_without_path_uses_cwd() { .expect("Reserve output is not a valid port number"); // Release from within the directory (using current_dir) - let mut cmd = env.command(); + let mut cmd = related_command(&env); cmd.arg("release") .current_dir(&test_path) .assert() @@ -130,7 +137,7 @@ fn test_release_untagged_only() { let port_tagged = env.reserve_with_tag(&test_path, "web"); // Release untagged only - env.command() + related_command(&env) .arg("release") .arg("--path") .arg(&test_path) @@ -165,7 +172,7 @@ fn test_release_recursive() { let port_child2 = env.reserve_simple(&child2); // Release parent recursively - env.command() + related_command(&env) .arg("release") .arg("--path") .arg(&parent) @@ -197,7 +204,7 @@ fn test_release_recursive_with_tag() { let port_child_api = env.reserve_with_tag(&child, "api"); // Release "web" recursively - env.command() + related_command(&env) .arg("release") .arg("--path") .arg(&parent) @@ -230,7 +237,7 @@ fn test_release_non_recursive_preserves_children() { let port_child = env.reserve_simple(&child); // Release parent non-recursively - env.command() + related_command(&env) .arg("release") .arg("--path") .arg(&parent) @@ -262,7 +269,7 @@ fn test_release_dry_run_does_not_release() { let port = env.reserve_simple(&test_path); // Dry-run release - env.command() + related_command(&env) .arg("release") .arg("--path") .arg(&test_path) @@ -287,7 +294,7 @@ fn test_release_dry_run_shows_plan() { env.reserve_simple(&test_path); // Dry-run should show plan on stderr - env.command() + related_command(&env) .arg("release") .arg("--path") .arg(&test_path) @@ -307,8 +314,7 @@ fn test_release_dry_run_with_quiet() { env.reserve_simple(&test_path); // Dry-run with --quiet - let output = env - .command() + let output = related_command(&env) .arg("--quiet") .arg("release") .arg("--path") @@ -342,7 +348,7 @@ fn test_release_with_force() { let port = env.reserve_simple(&test_path); // Release with --force - env.command() + related_command(&env) .arg("release") .arg("--path") .arg(&test_path) @@ -365,8 +371,7 @@ fn test_release_force_on_nonexistent() { let test_path = env.create_dir("test-project"); // Try to release something that doesn't exist with --force - let output = env - .command() + let output = related_command(&env) .arg("release") .arg("--path") .arg(&test_path) @@ -396,7 +401,7 @@ fn test_release_nothing_to_release() { let test_path = env.create_dir("test-project"); // Try to release when no reservation exists - should succeed (idempotent) - env.command() + related_command(&env) .arg("release") .arg("--path") .arg(&test_path) @@ -419,10 +424,10 @@ fn test_release_nonexistent_tag() { let test_path = env.create_dir("test-project"); // Create untagged reservation - env.reserve_simple(&test_path); + let port = env.reserve_simple(&test_path); // Try to release a tag that doesn't exist - should succeed (idempotent) - env.command() + related_command(&env) .arg("release") .arg("--path") .arg(&test_path) @@ -435,6 +440,31 @@ fn test_release_nonexistent_tag() { .or(predicate::str::contains("No reservation")) .or(predicate::str::contains("already released")), ); + + assert!(env.list().contains(&port.to_string())); +} + +/// An untagged-only selector with no match is an idempotent no-op. +#[test] +fn test_release_untagged_only_no_match_preserves_tagged_reservation() { + let env = TestEnv::new(); + let test_path = env.create_dir("test-project"); + let tagged = env.reserve_with_tag(&test_path, "web"); + + related_command(&env) + .arg("release") + .arg("--path") + .arg(&test_path) + .arg("--untagged-only") + .assert() + .success() + .stderr( + predicate::str::contains("not found") + .or(predicate::str::contains("No reservation")) + .or(predicate::str::contains("already released")), + ); + + assert!(env.list().contains(&tagged.to_string())); } /// Test that --tag and --untagged-only are mutually exclusive. @@ -446,7 +476,7 @@ fn test_release_tag_and_untagged_only_conflict() { let test_path = env.create_dir("test-project"); // Try to use both --tag and --untagged-only - env.command() + related_command(&env) .arg("release") .arg("--path") .arg(&test_path) @@ -465,10 +495,12 @@ fn test_release_tag_and_untagged_only_conflict() { #[test] fn test_release_nonexistent_path() { let env = TestEnv::new(); - let fake_path = env.path().join("does-not-exist"); + let fake_path = std::fs::canonicalize(env.path()) + .expect("Failed to canonicalize test root") + .join("does-not-exist"); // Try to release nonexistent path - should succeed (idempotent) - env.command() + related_command(&env) .arg("release") .arg("--path") .arg(&fake_path) @@ -495,7 +527,7 @@ fn test_release_respects_trop_path_env() { let port = env.reserve_simple(&test_path); // Release using env var for path - env.command() + related_command(&env) .arg("release") .env("TROP_PATH", &test_path) .assert() @@ -518,7 +550,7 @@ fn test_cli_path_overrides_env_path_for_release() { let port2 = env.reserve_simple(&path2); // Set env to path1 but use --path for path2 - env.command() + related_command(&env) .arg("release") .arg("--path") .arg(&path2) @@ -549,8 +581,7 @@ fn test_release_success_message() { env.reserve_simple(&test_path); // Release and check output - let output = env - .command() + let output = related_command(&env) .arg("release") .arg("--path") .arg(&test_path) @@ -582,8 +613,7 @@ fn test_release_quiet_mode() { env.reserve_simple(&test_path); // Release with --quiet - let output = env - .command() + let output = related_command(&env) .arg("--quiet") .arg("release") .arg("--path") @@ -610,8 +640,7 @@ fn test_release_verbose_mode() { env.reserve_simple(&test_path); // Release with --verbose - let output = env - .command() + let output = related_command(&env) .arg("--verbose") .arg("release") .arg("--path") @@ -660,35 +689,164 @@ fn test_release_multiple_tags_sequentially() { assert!(!list3.contains(&port3.to_string())); } -/// Test that releasing all reservations at a path works. +/// Test that the default exact-path release removes every tag at that path. /// -/// If there are multiple tags at a path, releasing the path without -/// specifying a tag might release all of them, or might require --recursive -/// or special flags. This test documents the behavior. +/// Descendant reservations must remain unless `--recursive` is supplied. #[test] -fn test_release_path_with_multiple_tags() { +fn test_release_path_without_filter_removes_all_exact_path_tags_only() { let env = TestEnv::new(); let test_path = env.create_dir("test-project"); + let child_path = env.create_dir("test-project/child"); - // Create multiple tagged reservations - env.reserve_with_tag(&test_path, "web"); - env.reserve_with_tag(&test_path, "api"); + let untagged = env.reserve_simple(&test_path); + let web = env.reserve_with_tag(&test_path, "web"); + let api = env.reserve_with_tag(&test_path, "api"); + let child = env.reserve_with_tag(&child_path, "web"); - // Try to release the path (no tag specified) - // This might fail (ambiguous) or release all - behavior depends on implementation - let output = env - .command() + env.command() .arg("release") .arg("--path") .arg(&test_path) - .output() - .unwrap(); + .current_dir(env.path()) + .assert() + .success(); - // Document the behavior: either succeeds or gives clear error - if !output.status.success() { - let stderr = String::from_utf8(output.stderr).unwrap(); - assert!(!stderr.is_empty(), "Should explain why it failed"); - } + let list_output = env.list(); + assert!(!list_output.contains(&untagged.to_string())); + assert!(!list_output.contains(&web.to_string())); + assert!(!list_output.contains(&api.to_string())); + assert!(list_output.contains(&child.to_string())); +} + +/// Releasing a sideways path is rejected before any reservation is changed. +#[test] +fn test_release_unrelated_path_is_rejected_without_partial_mutation() { + let env = TestEnv::new(); + let current_path = env.create_dir("current-project"); + let unrelated_path = env.create_dir("unrelated-project"); + let untagged = env.reserve_simple(&unrelated_path); + let tagged = env.reserve_with_tag(&unrelated_path, "web"); + + env.command() + .arg("release") + .arg("--path") + .arg(&unrelated_path) + .current_dir(¤t_path) + .assert() + .failure() + .stderr(predicate::str::contains("unrelated")); + + let list_output = env.list(); + assert!(list_output.contains(&untagged.to_string())); + assert!(list_output.contains(&tagged.to_string())); +} + +/// Recursive release applies the same guard to its requested root. +#[test] +fn test_release_recursive_unrelated_path_is_rejected_without_mutation() { + let env = TestEnv::new(); + let current_path = env.create_dir("current-project"); + let unrelated_path = env.create_dir("unrelated-project"); + let child_path = env.create_dir("unrelated-project/child"); + let parent = env.reserve_simple(&unrelated_path); + let child = env.reserve_simple(&child_path); + + env.command() + .arg("release") + .arg("--path") + .arg(&unrelated_path) + .arg("--recursive") + .current_dir(¤t_path) + .assert() + .failure() + .stderr(predicate::str::contains("unrelated")); + + let list_output = env.list(); + assert!(list_output.contains(&parent.to_string())); + assert!(list_output.contains(&child.to_string())); +} + +/// The explicit unrelated-path permission bypasses the relationship guard. +#[test] +fn test_release_allow_unrelated_path_releases_target() { + let env = TestEnv::new(); + let current_path = env.create_dir("current-project"); + let unrelated_path = env.create_dir("unrelated-project"); + let untagged = env.reserve_simple(&unrelated_path); + let tagged = env.reserve_with_tag(&unrelated_path, "web"); + + env.command() + .arg("release") + .arg("--path") + .arg(&unrelated_path) + .arg("--allow-unrelated-path") + .current_dir(¤t_path) + .assert() + .success(); + + let list_output = env.list(); + assert!(!list_output.contains(&untagged.to_string())); + assert!(!list_output.contains(&tagged.to_string())); +} + +/// The effective configuration permission also bypasses the path guard. +#[test] +fn test_release_honors_allow_unrelated_path_environment_config() { + let env = TestEnv::new(); + let current_path = env.create_dir("current-project"); + let unrelated_path = env.create_dir("unrelated-project"); + let port = env.reserve_simple(&unrelated_path); + + env.command() + .arg("release") + .arg("--path") + .arg(&unrelated_path) + .env("TROP_ALLOW_UNRELATED_PATH", "true") + .current_dir(¤t_path) + .assert() + .success(); + + assert!(!env.list().contains(&port.to_string())); +} + +/// A late delete failure rolls back every exact-path deletion. +#[test] +fn test_release_exact_path_deletion_is_atomic() { + let env = TestEnv::new(); + let test_path = env.create_dir("test-project"); + let untagged = env.reserve_simple(&test_path); + let tagged = env.reserve_with_tag(&test_path, "web"); + + let connection = + Connection::open(env.data_dir.join("trop.db")).expect("Failed to open test database"); + connection + .execute_batch( + " + CREATE TRIGGER fail_tagged_release + BEFORE DELETE ON reservations + WHEN OLD.tag = 'web' + BEGIN + SELECT RAISE(ABORT, 'forced late exact release failure'); + END; + ", + ) + .expect("Failed to install release failure trigger"); + drop(connection); + + env.command() + .arg("release") + .arg("--path") + .arg(&test_path) + .current_dir(env.path()) + .assert() + .failure() + .stderr(predicate::str::contains( + "forced late exact release failure", + )); + + let list_output = env.list(); + assert!(list_output.contains(&untagged.to_string())); + assert!(list_output.contains(&tagged.to_string())); } // ============================================================================ diff --git a/trop/src/database/operations.rs b/trop/src/database/operations.rs index d0fad1a..c4cdcc6 100644 --- a/trop/src/database/operations.rs +++ b/trop/src/database/operations.rs @@ -330,6 +330,13 @@ const SELECT_BY_PATH_PREFIX: &str = r" ORDER BY path, tag "; +const SELECT_BY_EXACT_PATH: &str = r" + SELECT path, tag, port, project, task, created_at, last_used_at + FROM reservations + WHERE path = ? + ORDER BY tag +"; + const SELECT_TAGGED_BY_EXACT_PATH: &str = r" SELECT path, tag, port, project, task, created_at, last_used_at FROM reservations @@ -859,6 +866,22 @@ impl Database { Ok(reservations) } + /// Returns every tagged and untagged reservation at one exact path. + /// + /// Reservations below the path are deliberately excluded. + pub(crate) fn get_reservations_by_exact_path( + conn: &Connection, + path: &Path, + ) -> Result> { + let mut stmt = conn.prepare_cached(SELECT_BY_EXACT_PATH)?; + let mut rows = stmt.query([path.to_string_lossy().to_string()])?; + let mut reservations = Vec::new(); + while let Some(row) = rows.next()? { + reservations.push(row_to_reservation(row)?); + } + Ok(reservations) + } + /// Returns every tagged reservation at one exact path. /// /// Untagged reservations and reservations below the path are deliberately @@ -1080,12 +1103,19 @@ impl Database { pub fn validate_path_relationship(target_path: &Path, allow_unrelated: bool) -> Result<()> { let current_dir = env::current_dir()?; let lexical_relationship = PathRelationship::between(target_path, ¤t_dir); - // Preserve lexical hierarchy for explicit and nonexistent paths. When - // that comparison is unrelated, compare existing paths physically so - // canonical inferred identities remain compatible with the platform's - // process-CWD spelling (notably Windows verbatim prefixes). + // Preserve lexical hierarchy for explicit paths. When that comparison + // is unrelated, canonicalize each path's existing portion and restore + // any nonexistent target suffix before comparing them physically. + // This keeps canonical inferred identities compatible with the + // platform's process-CWD spelling (notably Windows verbatim prefixes + // and short names) without requiring the target itself to exist. let physically_related = || { - let canonical_target = crate::path::canonicalize::canonicalize(target_path).ok()?; + let (canonical_target_root, target_remainder) = + crate::path::canonicalize::canonicalize_existing(target_path).ok()?; + let canonical_target = match target_remainder { + Some(path) => canonical_target_root.join(path), + None => canonical_target_root, + }; let canonical_current = crate::path::canonicalize::canonicalize(¤t_dir).ok()?; Some( PathRelationship::between(&canonical_target, &canonical_current) @@ -1640,6 +1670,25 @@ mod tests { ); } + #[cfg(unix)] + #[test] + fn test_validate_path_relationship_canonical_nonexistent_descendant() { + use std::os::unix::fs::symlink; + + let _db = create_test_database(); + let cwd = env::current_dir().unwrap(); + let temp = tempfile::tempdir().unwrap(); + let cwd_link = temp.path().join("cwd-link"); + symlink(&cwd, &cwd_link).unwrap(); + let nonexistent_descendant = cwd_link.join("does-not-exist"); + + let result = Database::validate_path_relationship(&nonexistent_descendant, false); + assert!( + result.is_ok(), + "a nonexistent descendant reached through a canonical equivalent must be related: {result:?}" + ); + } + #[test] fn test_validate_path_relationship_unrelated_denied() { let _db = create_test_database(); diff --git a/trop/src/operations/release.rs b/trop/src/operations/release.rs index e3f244a..13b6e47 100644 --- a/trop/src/operations/release.rs +++ b/trop/src/operations/release.rs @@ -73,6 +73,7 @@ impl ReleaseOptions { /// generating a plan that describes what actions to take. pub struct ReleasePlan { options: ReleaseOptions, + all_exact_path_tags: bool, } impl ReleasePlan { @@ -91,7 +92,20 @@ impl ReleasePlan { /// ``` #[must_use] pub const fn new(options: ReleaseOptions) -> Self { - Self { options } + Self { + options, + all_exact_path_tags: false, + } + } + + /// Selects every tagged and untagged reservation at the key's exact path. + /// + /// The default planner behavior remains an exact-key release. CLI callers + /// use this mode when neither `--tag` nor `--untagged-only` is supplied. + #[must_use] + pub const fn with_all_exact_path_tags(mut self, enabled: bool) -> Self { + self.all_exact_path_tags = enabled; + self } /// Builds an operation plan for this release request. @@ -126,12 +140,24 @@ impl ReleasePlan { Database::validate_path_relationship(&self.options.key.path, false)?; } - // Step 2: Check if reservation exists - if Database::get_reservation(conn, &self.options.key)?.is_some() { - // Reservation exists - plan to delete it + // Step 2: Select the exact-path set or exact key requested by the caller. + if self.all_exact_path_tags { + let reservations = + Database::get_reservations_by_exact_path(conn, &self.options.key.path)?; + if reservations.is_empty() { + plan = plan.add_warning(format!( + "No reservations found for {} (already released)", + self.options.key.path.display() + )); + } else { + for reservation in reservations { + plan = + plan.add_action(PlanAction::DeleteReservation(reservation.key().clone())); + } + } + } else if Database::get_reservation(conn, &self.options.key)?.is_some() { plan = plan.add_action(PlanAction::DeleteReservation(self.options.key.clone())); } else { - // Reservation doesn't exist - idempotent, just add a warning plan = plan.add_warning(format!( "No reservation found for {} (already released)", self.options.key @@ -390,6 +416,67 @@ mod tests { assert!(matches!(plan.actions[0], PlanAction::DeleteReservation(_))); } + #[test] + fn test_plan_release_all_exact_path_tags_preserves_descendants() { + let mut db = create_test_database(); + let exact_path = PathBuf::from("/test/path"); + let keys = [ + ReservationKey::new(exact_path.clone(), None).unwrap(), + ReservationKey::new(exact_path.clone(), Some("api".to_string())).unwrap(), + ReservationKey::new(exact_path.clone(), Some("web".to_string())).unwrap(), + ]; + let child_key = + ReservationKey::new(exact_path.join("child"), Some("web".to_string())).unwrap(); + + for (key, port) in keys + .iter() + .cloned() + .chain(std::iter::once(child_key.clone())) + .zip(8080..) + { + let reservation = Reservation::builder(key, Port::try_from(port).unwrap()) + .build() + .unwrap(); + db.create_reservation(&reservation).unwrap(); + } + + let options = ReleaseOptions::new(keys[0].clone()).with_allow_unrelated_path(true); + let plan = ReleasePlan::new(options) + .with_all_exact_path_tags(true) + .build_plan(db.connection()) + .unwrap(); + + let planned_keys = plan + .actions + .iter() + .map(|action| match action { + PlanAction::DeleteReservation(key) => key, + other => panic!("unexpected release action: {other:?}"), + }) + .collect::>(); + assert_eq!(planned_keys.len(), keys.len()); + for key in &keys { + assert!(planned_keys.contains(&key)); + } + assert!(!planned_keys.contains(&&child_key)); + } + + #[test] + fn test_plan_release_all_exact_path_tags_is_idempotent_when_empty() { + let db = create_test_database(); + let key = ReservationKey::new(PathBuf::from("/test/path"), None).unwrap(); + let options = ReleaseOptions::new(key).with_allow_unrelated_path(true); + + let plan = ReleasePlan::new(options) + .with_all_exact_path_tags(true) + .build_plan(db.connection()) + .unwrap(); + + assert!(plan.actions.is_empty()); + assert_eq!(plan.warnings.len(), 1); + assert!(plan.warnings[0].contains("No reservations found")); + } + #[test] fn test_plan_release_nonexistent_reservation() { let db = create_test_database();