diff --git a/docs/superpowers/specs/2026-05-17-migration-extraction-146-design.md b/docs/superpowers/specs/2026-05-17-migration-extraction-146-design.md new file mode 100644 index 0000000..a930c17 --- /dev/null +++ b/docs/superpowers/specs/2026-05-17-migration-extraction-146-design.md @@ -0,0 +1,213 @@ +# Migration Extraction Follow-Up Design + +Issue: #146 ARCH-13 Batch 3: Move command-safety and experimental migrations in follow-up batches + +Parent roadmap: #133 Core PMS Architecture Stabilization V2.1 + +Date: 2026-05-17 +Status: User-approved design; subagent review findings addressed + +## Purpose + +Move the remaining inline migration bodies `V7` through `V19` out of `mhm/src-tauri/src/db.rs` without changing database behavior. + +Although issue #146 is framed around command-safety, outbox, gateway, and agent-related migrations, this approved follow-up intentionally includes `V8` invoice and `V9` group-booking migrations as well. Those versions are included so the follow-up completes the migration extraction after #144/#145 and leaves `db.rs` as a runner/bootstrap module rather than leaving isolated inline migration bodies behind. + +This is a no-behavior-change extraction. It must make `db.rs` a migration runner and database bootstrap module instead of a migration monolith, while preserving schema semantics, migration order, compatibility behavior, command safety behavior, money migration behavior, outbox behavior, gateway runtime behavior, agent runtime behavior, and frontend behavior. + +## Scope + +- Keep database bootstrap and the main migration runner in `mhm/src-tauri/src/db.rs`. +- Keep existing helper functions in `db.rs`, including `set_schema_version`, `execute_compat_alter`, and `restore_foreign_keys_after_v14_migration`. +- Move remaining migration bodies `V7` through `V19` into private modules under `mhm/src-tauri/src/db/`. +- Keep the existing `V1` through `V6` extraction in `mhm/src-tauri/src/db/migrations.rs`. +- Keep the final schema version at `19`. +- Keep existing migration tests and fresh database guards intact. + +`V7` through `V19` cover: + +- `V7`: gateway API key storage +- `V8`: invoice PDF system +- `V9`: group booking system +- `V10`: command idempotency +- `V11`: command terminal error replay payload +- `V12`: operator-ready command ledger metadata +- `V13`: origin idempotency on ledger and folio rows +- `V14`: integer VND money foundation, including the existing command ledger legacy hash compatibility column +- `V15`: command recovery queue and audit actions +- `V16`: durable outbox events +- `V17`: outbox per-aggregate open-row FIFO support +- `V18`: agent safety session, audit, and memory schema +- `V19`: CEO hourly digest run state + +## Non-Goals + +- Do not change SQL semantics. +- Do not reorder migrations. +- Do not combine migrations. +- Do not change the final schema version. +- Do not change command idempotency logic. +- Do not change outbox dispatcher behavior. +- Do not change gateway, agent, digest, Telegram, OpenAI, or frontend runtime behavior. +- Do not gate or delete experimental schemas. +- Do not introduce a new migration framework or registry abstraction. +- Do not move or refactor unrelated backend modules. + +## Context + +Issues #144 and #145 introduced the migration module structure and moved `V1` through `V6` into `mhm/src-tauri/src/db/migrations.rs`. The remaining migrations `V7` through `V19` still live inline in `run_migrations`. + +Issue #146 intentionally has higher risk than the first extraction because the remaining migrations include command-safety, money, outbox, gateway, agent, and digest schema. Those schemas are safety-sensitive even when the runtime surfaces are experimental. + +GitNexus impact analysis for `run_migrations` returned CRITICAL risk: 53 direct callers, 8 affected execution flows, and 20 affected modules. The affected surface includes app startup, database migration tests, command idempotency tests, outbox tests, gateway tests, agent tests, digest tests, setup tests, and booking tests. + +GitNexus impact analysis for `set_schema_version` and `execute_compat_alter` also returned CRITICAL risk. This extraction may widen visibility only as needed for private child migration modules, but it must not change helper behavior. + +## Chosen Approach + +Move all remaining inline migrations `V7` through `V19` in one issue, but split them into domain-focused private modules so the high-risk areas remain reviewable. + +Use this module shape: + +```text +mhm/src-tauri/src/db.rs +mhm/src-tauri/src/db/ + migrations.rs + core_extensions.rs + command_safety.rs + money.rs + outbox.rs + agent.rs +``` + +Module ownership: + +- `migrations.rs` keeps the existing `V1` through `V6` early PMS migrations. +- `core_extensions.rs` owns `V7` through `V9`. +- `command_safety.rs` owns `V10` through `V13` and `V15`. +- `money.rs` owns the `V14` integer VND money migration special case. +- `outbox.rs` owns `V16` and `V17`. +- `agent.rs` owns `V18` and `V19`. + +Rejected alternatives: + +- Keep everything in a single growing `migrations.rs`. This reduces `db.rs` but recreates a migration monolith. +- Create one file per migration version. This gives maximum isolation, but creates more file churn than the current migration count needs. +- Split #146 into multiple issues. This is safest per PR, but the chosen scope is to move all remaining migrations while preserving review boundaries inside the PR. + +## Module Interfaces + +Each new module should expose only parent-module-internal functions needed by `run_migrations`. + +`core_extensions.rs`: + +```rust +pub(super) async fn migrate_v7_gateway_api_keys(pool: &Pool) -> Result<(), sqlx::Error>; +pub(super) async fn migrate_v8_invoice_pdf_system(pool: &Pool) -> Result<(), sqlx::Error>; +pub(super) async fn migrate_v9_group_booking_system(pool: &Pool) -> Result<(), sqlx::Error>; +``` + +`command_safety.rs`: + +```rust +pub(super) async fn migrate_v10_command_idempotency(pool: &Pool) -> Result<(), sqlx::Error>; +pub(super) async fn migrate_v11_command_terminal_error_replay(pool: &Pool) -> Result<(), sqlx::Error>; +pub(super) async fn migrate_v12_command_ledger_metadata(pool: &Pool) -> Result<(), sqlx::Error>; +pub(super) async fn migrate_v13_origin_idempotency(pool: &Pool) -> Result<(), sqlx::Error>; +pub(super) async fn migrate_v15_command_recovery(pool: &Pool) -> Result<(), sqlx::Error>; +``` + +`money.rs`: + +```rust +pub(super) async fn migrate_v14_integer_vnd_money(pool: &Pool) -> Result<(), sqlx::Error>; +``` + +`outbox.rs`: + +```rust +pub(super) async fn migrate_v16_durable_outbox_events(pool: &Pool) -> Result<(), sqlx::Error>; +pub(super) async fn migrate_v17_outbox_fifo_support(pool: &Pool) -> Result<(), sqlx::Error>; +``` + +`agent.rs`: + +```rust +pub(super) async fn migrate_v18_agent_safety_tables(pool: &Pool) -> Result<(), sqlx::Error>; +pub(super) async fn migrate_v19_agent_digest_runs(pool: &Pool) -> Result<(), sqlx::Error>; +``` + +The modules should reuse shared helpers from `db.rs` instead of duplicating compatibility or schema-version logic. + +## Data Flow + +The runtime flow remains unchanged: + +1. `init_db` creates the SQLite pool. +2. `init_db` calls `run_migrations`. +3. `run_migrations` reads the current schema version. +4. `run_migrations` checks each version gate in order. +5. For `V1` through `V19`, `run_migrations` delegates to private migration module functions. +6. `init_db` inserts default settings after migrations complete. + +`run_migrations` remains the only place that decides migration order. The extracted modules own only migration bodies. + +The `V14` flow is the existing exception and must remain behaviorally identical: + +1. Acquire a connection. +2. Disable foreign keys. +3. Run the migration transaction callback. +4. Add `legacy_request_hash` through `execute_compat_alter`. +5. Call `crate::money_migration::migrate_integer_vnd_money`. +6. Set schema version `14`. +7. Restore foreign key behavior through `restore_foreign_keys_after_v14_migration`. + +## Error Handling + +Error behavior must stay unchanged. + +Each extracted migration returns `Result<(), sqlx::Error>` and uses `?` exactly like the current inline code. If a query fails, the transaction is not committed and the error bubbles back to `run_migrations`. + +`execute_compat_alter` keeps its current behavior: duplicate column and already-exists errors are logged and ignored; other errors fail the migration. + +`V14` keeps the existing foreign-key restore path. The extraction must not add a new recovery path, repair path, fallback path, migration skip path, or best-effort schema creation path. + +## Testing + +Validation commands: + +```bash +cd /Users/binhan/HotelManager/mhm/src-tauri && cargo test db::tests +cd /Users/binhan/HotelManager/mhm/src-tauri && cargo test migration +cd /Users/binhan/HotelManager/mhm/src-tauri && cargo test +cd /Users/binhan/HotelManager/mhm/src-tauri && cargo clippy --all-targets -- -D warnings +``` + +Expected coverage: + +- fresh database migration still reaches schema version `19`; +- required PMS, core extension, command safety, outbox, and agent tables still exist; +- fresh migration tests and existing database upgrade tests for `V10` through `V19` still pass where those tests currently exist; +- V14 money conversion and rollback tests still pass; +- V16 and V17 outbox schema, index, and insert contract tests still pass; +- V18 and V19 agent and digest schema tests still pass; +- compatibility alters still ignore duplicate columns where intended; +- later runtime tests that create migrated test pools still pass. + +Before committing implementation changes, run GitNexus change detection. The expected affected scope should be limited to `db.rs`, private `db/` migration module files, and migration execution flows. + +## Review Guardrails + +Implementation should be a mechanical move: + +- preserve SQL strings exactly; +- preserve comments where they identify migration versions or existing behavior; +- preserve transaction boundaries; +- preserve version numbers; +- preserve call order; +- preserve V14 foreign-key handling; +- avoid unrelated formatting churn in untouched code; +- do not edit command idempotency, outbox, gateway, agent, digest, frontend, or service behavior; +- do not edit the unrelated dirty file `mhm/src/stores/useHotelStore.test.ts`. + +Before editing implementation symbols, run GitNexus impact analysis for each edited function or helper. If GitNexus reports that the index is stale, run `npx gitnexus analyze` before relying on impact output. At minimum, run impact analysis for `run_migrations`, `set_schema_version`, `execute_compat_alter`, and `restore_foreign_keys_after_v14_migration` if their bodies or visibility are modified. `run_migrations`, `set_schema_version`, and `execute_compat_alter` are already known CRITICAL risk; implementation should report that blast radius before editing and continue only with the approved mechanical extraction scope. diff --git a/docs/superpowers/specs/2026-05-18-command-lock-key-builder-design.md b/docs/superpowers/specs/2026-05-18-command-lock-key-builder-design.md new file mode 100644 index 0000000..0262fe2 --- /dev/null +++ b/docs/superpowers/specs/2026-05-18-command-lock-key-builder-design.md @@ -0,0 +1,205 @@ +# Issue 149: Command Lock-Key Builder Extraction Design + +Date: 2026-05-18 +Status: Approved for implementation planning + +GitHub issue: + +## Purpose + +Extract the command idempotency lock-key preparation internals into a focused module without changing the stable lock-key format, command serialization behavior, or high-risk write lock coverage. + +This issue exists because lock keys are part of the PMS safety contract. A command that mutates currently covered resources such as bookings, rooms, folios, groups, and settings must keep using the same stable lock key strings so retries, duplicate detection, command recovery metadata, and aggregate locking continue to agree about the same resource. + +## Problem + +`mhm/src-tauri/src/command_idempotency.rs` currently prepares lock-key JSON in more than one place: + +- initial command preparation runs the request lock-key deriver, sorts, deduplicates, and serializes `lock_keys_json`; +- resolved-guard refresh sorts, deduplicates, requires non-empty lock keys, and updates `command_idempotency.lock_keys_json`. + +That behavior is correct but too easy to drift because it is embedded in the large executor file. The issue is not to invent a new lock format. The issue is to isolate the current behavior behind one internal command-idempotency boundary and add format tripwire tests. + +## Goals + +- Add a focused command idempotency lock-key helper module. +- Preserve exact lock-key strings and JSON output. +- Preserve the distinction between optional initial lock keys and required resolved-guard lock keys. +- Preserve existing command payload hashing, ledger intent serialization, summary serialization, replay behavior, and outbox behavior. +- Document the lock-key format in source comments or docs close to the helper module. +- Add or keep tests that assert exact persisted `lock_keys_json` output. + +## Non-Goals + +- Do not change stable lock-key strings. +- Do not change command request hashing or canonical payload semantics. +- Do not change `intent_json`, `summary_json`, `result_summary_json`, or response serialization. +- Do not change which aggregates each command locks. +- Do not migrate all booking, billing, group, invoice, or agent setting lock-key derivers into one large global API. +- Do not refactor `aggregate_locks.rs` unless compilation or tests prove a tiny compatibility change is required. + +## Selected Approach + +Use a command-specific helper module: + +```text +mhm/src-tauri/src/command_idempotency/lock_keys.rs +``` + +The module will be a thin boundary for command idempotency persistence. It will not own the aggregate lock format itself. Existing constructors in `aggregate_locks.rs` remain the source for aggregate strings such as `room:{id}`, `booking:{id}`, `folio:{id}`, and `group:{id}`. Existing ad hoc command formats such as `settings:{setting_key}` must be preserved as existing command lock keys, not moved into a new global constructor as part of this issue. + +This gives command idempotency one place to prepare lock-key JSON while keeping the implementation narrow enough for a safety-sensitive refactor. + +## Stable Lock-Key Format + +The implementation must preserve the currently persisted strings: + +| Aggregate | Format | +| --- | --- | +| Room | `room:{room_id}` | +| Booking | `booking:{booking_id}` | +| Folio | `folio:{booking_id}` | +| Group | `group:{group_id}` | +| Setting | `settings:{setting_key}` | + +The canonical persisted representation remains a stable JSON array of strings after sorting and deduplication: + +```json +["booking:B1","room:R1"] +``` + +Low-risk commands may still persist an empty array: + +```json +[] +``` + +## Architecture + +`command_idempotency.rs` remains the public module root and executor home. `types.rs` continues to own command contract types. The new `lock_keys.rs` module owns only lock-key preparation helpers used by the executor. + +Expected module shape: + +```rust +mod lock_keys; +mod types; +``` + +The helper module should expose narrow parent-only functions, for example: + +```rust +pub(super) fn optional_lock_keys_json(keys: I) -> CommandResult +where + I: IntoIterator, + S: Into; + +pub(super) fn required_lock_keys_json(keys: I) -> CommandResult +where + I: IntoIterator, + S: Into; +``` + +Naming can change during implementation if the final names are clearer, but the boundary should stay narrow: + +- optional helper allows empty input for low-risk/default lock derivation; +- required helper rejects empty input for resolved guard refresh with the current system error classification/code and exact message `Resolved idempotency lock keys are required`; +- both helpers produce stable JSON through the same serialization path. + +## Data Flow + +Initial command preparation: + +```text +WriteCommandRequest + -> request.lock_key_deriver(hash_payload) + -> command_idempotency::lock_keys optional helper + -> stable lock_keys_json + -> command_idempotency row claim +``` + +Resolved guard refresh: + +```text +ResolvedWriteCommandGuard.lock_keys + -> command_idempotency::lock_keys required helper + -> stable lock_keys_json + -> UPDATE command_idempotency.lock_keys_json before business transaction +``` + +The lock-key deriver must continue to run against `hash_payload`, not sanitized ledger intent. This preserves the existing safety rule that operator-safe metadata cleanup cannot accidentally remove conflict keys needed for command locking. + +## Error Handling + +Initial lock keys may be empty. This preserves existing low-risk command behavior where `default_lock_key_deriver` returns no locks. + +Resolved guard lock keys must be non-empty. If the guard resolves to an empty key set, the command must fail before running the guarded mutation and finalize the claimed idempotency row as it does today. + +Deriver-specific errors must pass through unchanged. For example, if a command-specific deriver cannot find `booking_id` or `room_id` in its hash payload, the helper module should not mask that error. + +Duplicate and ordering behavior must match current command idempotency persistence exactly: + +- do not trim or otherwise normalize deriver-returned lock key strings inside the command idempotency helper; +- do not add new blank-key rejection for non-empty key lists in this issue; +- sort lexicographically; +- deduplicate; +- serialize with stable JSON. + +The aggregate lock constructors may trim aggregate IDs before producing strings such as `room:{room_id}`, but #149 must not add a second normalization layer inside command idempotency. The command idempotency helper must not call `aggregate_locks::canonicalize_lock_keys`; that function is for aggregate lock acquisition semantics, not persisted command idempotency metadata. If future work wants to harden blank or whitespace-padded lock keys, that should be a separate behavior-change issue with explicit test updates. + +## Testing + +Tests should act as format tripwires. At minimum, implementation should keep or add coverage for: + +- optional initial lock keys serialize sorted and deduplicated, for example `["booking:B1","room:R1"]`; +- optional initial lock keys allow empty output as `[]`; +- command idempotency helper behavior does not trim deriver-returned strings if a focused helper test covers malformed input; +- existing `settings:{setting_key}` lock output remains documented or covered by a focused tripwire test if the implementation touches that path; +- resolved guard lock keys serialize sorted and deduplicated before the service transaction runs; +- resolved guard empty lock keys still fail with the current system error classification/code and exact message `Resolved idempotency lock keys are required`; +- replay and hash-mismatch behavior remain unchanged. + +Existing command idempotency tests already cover several of these behaviors. The implementation should add only focused tests where exact persisted output is missing. + +Primary validation: + +```bash +cd mhm/src-tauri && cargo test command_idempotency +rg -n "lock_key|lock keys|lock-key" docs mhm/src-tauri/src +``` + +If implementation touches `aggregate_locks.rs`, also run: + +```bash +cd mhm/src-tauri && cargo test aggregate_locks +``` + +## Impact And Risk + +Fresh GitNexus analysis before this design found high blast radius around the safety-sensitive symbols: + +- `prepare_write_command_request`: CRITICAL risk, 7 direct callers, 9 affected execution flows, 3 affected modules. +- `refresh_claim_lock_keys`: LOW risk, 1 direct caller, 0 indexed execution flows, 2 affected modules. +- `canonicalize_lock_keys`: CRITICAL risk, 6 direct callers, 7 affected execution flows, 2 affected modules. +- `aggregate_key`: HIGH risk, 4 direct callers, 3 affected execution flows, 1 affected module. + +Implementation must rerun GitNexus impact analysis before editing any symbol and must stop for user confirmation if fresh analysis reports HIGH or CRITICAL risk for an intended edit. The preferred implementation path avoids editing `aggregate_locks.rs` and keeps changes limited to command idempotency internals plus focused tests. + +## Implementation Boundaries + +Expected files: + +- create `mhm/src-tauri/src/command_idempotency/lock_keys.rs`; +- modify `mhm/src-tauri/src/command_idempotency.rs` to declare the module and use its helpers; +- optionally modify tests in `mhm/src-tauri/src/command_idempotency.rs`; +- do not modify unrelated frontend files. + +The existing dirty file `mhm/src/stores/useHotelStore.test.ts` is unrelated and must not be staged or changed for this issue. + +## Success Criteria + +- Lock-key preparation is isolated in a focused command idempotency module. +- Persisted lock-key JSON is byte-for-byte equivalent for covered cases. +- Low-risk empty lock keys still persist as `[]`. +- Resolved guard empty lock keys still fail before mutation. +- Existing command idempotency tests pass. +- Validation search shows the lock-key format and safety boundary are documented. diff --git a/mhm/src-tauri/src/command_idempotency.rs b/mhm/src-tauri/src/command_idempotency.rs index b3c6ae9..13e665f 100644 --- a/mhm/src-tauri/src/command_idempotency.rs +++ b/mhm/src-tauri/src/command_idempotency.rs @@ -1,3 +1,4 @@ +mod lock_keys; mod types; pub use types::{ @@ -209,16 +210,7 @@ impl WriteCommandExecutor { I: IntoIterator, S: Into, { - let mut lock_keys = lock_keys - .into_iter() - .map(Into::into) - .collect::>(); - lock_keys.sort(); - lock_keys.dedup(); - if lock_keys.is_empty() { - return Err(system_error("Resolved idempotency lock keys are required")); - } - let lock_keys_json = stable_json_string(&serde_json::json!(lock_keys))?; + let lock_keys_json = lock_keys::required_lock_keys_json(lock_keys)?; let now = Utc::now().to_rfc3339(); let result = sqlx::query( "UPDATE command_idempotency @@ -843,11 +835,8 @@ fn stable_request_hash_from_json(intent_json: &str) -> CommandResult { fn prepare_write_command_request( request: WriteCommandRequest, ) -> CommandResult { - let mut lock_keys = (request.lock_key_deriver)(&request.hash_payload)?; - lock_keys.sort(); - lock_keys.dedup(); - - let lock_keys_json = stable_json_string(&serde_json::json!(lock_keys))?; + let lock_keys_json = + lock_keys::optional_lock_keys_json((request.lock_key_deriver)(&request.hash_payload)?)?; let hash_payload_json = stable_json_string(&request.hash_payload)?; let request_hash = stable_request_hash_from_json(&hash_payload_json)?; let intent_json = stable_json_string(&request.ledger_intent.to_value()?)?; @@ -1556,6 +1545,16 @@ mod tests { Ok(vec!["booking:999".to_string()]) } + fn unsorted_duplicate_test_lock_keys( + _intent: &serde_json::Value, + ) -> CommandResult> { + Ok(vec![ + "room:R1".to_string(), + "booking:B1".to_string(), + "room:R1".to_string(), + ]) + } + #[tokio::test] async fn write_command_executor_persists_operator_safe_metadata() { let pool = test_pool().await; @@ -2082,6 +2081,7 @@ mod tests { .expect_err("empty resolved lock keys should fail after claim"); assert_eq!(error.code, codes::SYSTEM_INTERNAL_ERROR); + assert_eq!(error.message, "Resolved idempotency lock keys are required"); let status: String = sqlx::query_scalar( "SELECT status FROM command_idempotency @@ -2171,6 +2171,33 @@ mod tests { ); } + #[test] + fn prepare_write_command_request_serializes_initial_lock_keys_sorted_and_deduplicated() { + let request = WriteCommandRequest::new_low_risk( + serde_json::json!({ "schema": "test.lock_keys.v1" }), + "Initial lock keys", + ) + .expect("request builds") + .with_lock_key_deriver(unsorted_duplicate_test_lock_keys); + + let prepared = prepare_write_command_request(request).expect("request prepares"); + + assert_eq!(prepared.lock_keys_json, "[\"booking:B1\",\"room:R1\"]"); + } + + #[test] + fn prepare_write_command_request_serializes_empty_initial_lock_keys_as_empty_array() { + let request = WriteCommandRequest::new_low_risk( + serde_json::json!({ "schema": "test.empty_lock_keys.v1" }), + "Empty initial lock keys", + ) + .expect("request builds"); + + let prepared = prepare_write_command_request(request).expect("request prepares"); + + assert_eq!(prepared.lock_keys_json, "[]"); + } + #[tokio::test] async fn refresh_claim_lease_write_lock_timeout_fails_claim() { let pool = test_pool().await; diff --git a/mhm/src-tauri/src/command_idempotency/lock_keys.rs b/mhm/src-tauri/src/command_idempotency/lock_keys.rs new file mode 100644 index 0000000..5db039f --- /dev/null +++ b/mhm/src-tauri/src/command_idempotency/lock_keys.rs @@ -0,0 +1,83 @@ +use super::{stable_json_string, system_error}; +use crate::app_error::CommandResult; + +pub(super) fn optional_lock_keys_json(lock_keys: I) -> CommandResult +where + I: IntoIterator, + S: Into, +{ + let lock_keys = canonical_lock_keys(lock_keys); + stable_json_string(&serde_json::json!(lock_keys)) +} + +pub(super) fn required_lock_keys_json(lock_keys: I) -> CommandResult +where + I: IntoIterator, + S: Into, +{ + let lock_keys = canonical_lock_keys(lock_keys); + if lock_keys.is_empty() { + return Err(system_error("Resolved idempotency lock keys are required")); + } + + stable_json_string(&serde_json::json!(lock_keys)) +} + +fn canonical_lock_keys(lock_keys: I) -> Vec +where + I: IntoIterator, + S: Into, +{ + let mut lock_keys = lock_keys.into_iter().map(Into::into).collect::>(); + lock_keys.sort(); + lock_keys.dedup(); + lock_keys +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::app_error::codes; + + #[test] + fn optional_lock_keys_json_sorts_and_deduplicates_without_trimming() { + let lock_keys_json = optional_lock_keys_json([ + "room:R1".to_string(), + " booking:B1".to_string(), + "booking:B1".to_string(), + "room:R1".to_string(), + ]) + .expect("lock keys serialize"); + + assert_eq!( + lock_keys_json, + "[\" booking:B1\",\"booking:B1\",\"room:R1\"]" + ); + } + + #[test] + fn optional_lock_keys_json_allows_empty_initial_keys() { + let lock_keys_json = + optional_lock_keys_json(Vec::::new()).expect("empty keys serialize"); + + assert_eq!(lock_keys_json, "[]"); + } + + #[test] + fn optional_lock_keys_json_preserves_settings_format() { + let lock_keys_json = + optional_lock_keys_json(["settings:ceo_cloud_data_opt_in".to_string()]) + .expect("settings lock key serializes"); + + assert_eq!(lock_keys_json, "[\"settings:ceo_cloud_data_opt_in\"]"); + } + + #[test] + fn required_lock_keys_json_rejects_empty_with_current_system_error() { + let error = required_lock_keys_json(Vec::::new()) + .expect_err("empty resolved keys should fail"); + + assert_eq!(error.code, codes::SYSTEM_INTERNAL_ERROR); + assert_eq!(error.message, "Resolved idempotency lock keys are required"); + } +}