From 450f0486270623d7d33afa62bd5b94f332bf3519 Mon Sep 17 00:00:00 2001 From: Stefanie Jane Date: Mon, 19 Oct 2026 09:39:34 -0700 Subject: [PATCH 1/4] feat(scene): expose whether the primary zone follows the active layout Without a custom LED zone, effect applies and layout publications rewrite the primary zone's layout from the active spatial layout, so the daemon needs to know when a placement saved on that zone belongs in the layout instead. The predicate wraps the existing custom LED zone check so the scene manager and its callers share one rule. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/hypercolor-core/src/scene/mod.rs | 12 +++++++ .../tests/zone_lifecycle_tests.rs | 36 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/crates/hypercolor-core/src/scene/mod.rs b/crates/hypercolor-core/src/scene/mod.rs index 769e895a8..9480ed015 100644 --- a/crates/hypercolor-core/src/scene/mod.rs +++ b/crates/hypercolor-core/src/scene/mod.rs @@ -1705,6 +1705,18 @@ pub fn default_primary_zone(mut layout: SpatialLayout) -> Zone { } } +/// Whether the scene's primary zone covers the full scope and so follows +/// the active spatial layout. +/// +/// Until a custom LED zone exists, effect applies and layout publications +/// rewrite the primary zone's layout from the active layout, so a placement +/// that should last belongs in that layout. Once a custom LED zone claims +/// outputs, the primary zone's layout is scene-local. +#[must_use] +pub fn primary_zone_follows_active_layout(scene: &Scene) -> bool { + !scene_has_custom_led_zones(scene) +} + fn bump_zones_revision(scene: &mut Scene) { scene.zones_revision = scene.zones_revision.saturating_add(1); } diff --git a/crates/hypercolor-core/tests/zone_lifecycle_tests.rs b/crates/hypercolor-core/tests/zone_lifecycle_tests.rs index cedbfe942..0a8dd5f10 100644 --- a/crates/hypercolor-core/tests/zone_lifecycle_tests.rs +++ b/crates/hypercolor-core/tests/zone_lifecycle_tests.rs @@ -2,6 +2,7 @@ use std::collections::HashMap; use hypercolor_core::scene::{ OutputPlacement, SceneManager, ZoneMetaPatch, ZoneMutationError, make_scene, + primary_zone_follows_active_layout, }; use hypercolor_types::device::DeviceId; use hypercolor_types::effect::{EffectCategory, EffectId, EffectMetadata, EffectSource}; @@ -607,6 +608,41 @@ fn layout_sync_preserves_primary_assignment_when_custom_zones_exist() { assert_eq!(after, before); } +#[test] +fn primary_zone_follows_active_layout_until_a_custom_led_zone_exists() { + let mut manager = SceneManager::with_default(); + let scene_id = SceneId::DEFAULT; + manager + .upsert_primary_zone( + &sample_effect("Primary"), + HashMap::new(), + None, + sample_layout("primary-zone"), + ) + .expect("primary should be created"); + let follows = |manager: &SceneManager| { + primary_zone_follows_active_layout( + manager + .active_scene() + .expect("default scene should be active"), + ) + }; + assert!(follows(&manager)); + + let custom = manager + .create_zone(&scene_id, "Custom".to_owned(), None, (320, 200)) + .expect("custom zone should be created"); + assert!( + !follows(&manager), + "an empty custom LED zone already makes the primary layout scene-local" + ); + + manager + .delete_zone(&scene_id, custom) + .expect("custom zone should delete"); + assert!(follows(&manager)); +} + #[test] fn update_zone_layout_merges_placement_and_preserves_identity() { let mut manager = SceneManager::with_default(); From fad5874f0433048d8a7afdf86b1062a1bb443282 Mon Sep 17 00:00:00 2001 From: Stefanie Jane Date: Mon, 19 Oct 2026 09:39:41 -0700 Subject: [PATCH 2/4] fix(studio): keep saved primary-zone placements in the active layout A strip moved and saved in Studio snapped back on the next effect or preset change. The save only rewrote the scene's copy of the primary zone, but while a scene has no custom LED zone that copy follows the active layout: effect applies, layout publications (hotplug, auto layout, layout apply, scene activation) and the startup restore all rebuild it from that layout, which never learned about the move. Saving such a zone now also writes its outputs into the active layout. The layout keeps its id, name, description, canvas size, sampling defaults and version, and takes the zone's outputs in the zone's order. The store entry is persisted and the layout republished through the guarded activation path, so the spatial engine, the layout store and the zone agree. The layout update guard is held from the zone read through the republication and released before connectivity convergence, which takes the guard itself. The workflow runs as one spawned task so a dropped request cannot split the two writes, and the Save answers with the post-publication revision when no other commit landed in between. Scenes with a custom LED zone keep a scene-local primary layout, as before. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/domain/layout/workflows.rs | 94 +++ .../src/domain/scene_tree.rs | 67 +- .../tests/studio_layout_persistence_tests.rs | 608 ++++++++++++++++++ 3 files changed, 761 insertions(+), 8 deletions(-) create mode 100644 crates/hypercolor-daemon/tests/studio_layout_persistence_tests.rs diff --git a/crates/hypercolor-daemon/src/domain/layout/workflows.rs b/crates/hypercolor-daemon/src/domain/layout/workflows.rs index e9ef38030..88fe8332f 100644 --- a/crates/hypercolor-daemon/src/domain/layout/workflows.rs +++ b/crates/hypercolor-daemon/src/domain/layout/workflows.rs @@ -10,6 +10,7 @@ use hypercolor_types::identity::LayoutId; use hypercolor_types::spatial::{SamplingMode, SpatialLayout}; use crate::domain::DomainError; +use crate::scene_transactions::LayoutUpdateGuard; use super::{ LayoutContext, LayoutMutationResult, LayoutMutationTestOperation, LayoutMutationTestPoint, @@ -330,6 +331,99 @@ impl LayoutContext { }) } + /// Give the active layout the outputs of a primary zone that follows it. + /// + /// Without a custom LED zone, effect applies, layout publications, and + /// the startup restore all rewrite the primary zone from the active + /// layout, so a placement saved on that zone has to land in the layout + /// itself. The layout keeps its id, name, description, canvas size, + /// sampling defaults, and version, and takes the zone's outputs in the + /// zone's order. The store entry is replaced and persisted first, then + /// published under the caller's guard; a failed publication restores + /// the previous entry. Returns whether a new layout was published. + /// + /// The caller runs connectivity convergence only after releasing the + /// guard, because that pass takes the guard itself. + /// + /// # Errors + /// + /// Persistence and publication failures, after the store entry has + /// been rolled back. + pub(crate) async fn adopt_primary_zone_layout_under_guard( + &self, + guard: &LayoutUpdateGuard, + zone_layout: &SpatialLayout, + runtime: &LayoutRuntime, + ) -> Result { + let current = self.current(); + if current.zones == zone_layout.zones { + return Ok(false); + } + let previous_zones = current.zones.clone(); + let mut updated = current; + updated.zones.clone_from(&zone_layout.zones); + let layout_id = updated.id.clone(); + let previous_entry = self + .catalog + .entries() + .write() + .await + .insert(layout_id.clone(), updated.clone()); + + if let Err(error) = self.catalog.persist().await { + let rollback = self + .restore_catalog_entry(&layout_id, previous_entry) + .await + .err() + .map(|error| format!("layout store rollback failed: {error}")); + return Err(layout_store_persistence_error( + "zone layout save", + error, + rollback, + )); + } + if let Err(error) = self + .admit_persisted_update_under_guard(guard, updated.clone(), runtime) + .await + { + if let Err(rollback_error) = + self.restore_catalog_entry(&layout_id, previous_entry).await + { + tracing::warn!( + layout_id, + %rollback_error, + "layout store rollback failed after a rejected zone layout publication" + ); + } + return Err(layout_update_domain_error(error)); + } + + self.exclusions + .reconcile_layout(&layout_id, &previous_zones, &updated.zones) + .await; + self.publish_layout_changed(None, layout_id); + Ok(true) + } + + async fn restore_catalog_entry( + &self, + layout_id: &str, + previous: Option, + ) -> anyhow::Result<()> { + { + let mut layouts = self.catalog.entries().write().await; + match previous { + Some(previous) => { + layouts.insert(layout_id.to_owned(), previous); + } + None => { + layouts.remove(layout_id); + } + } + } + self.catalog.persist().await + } + pub(crate) async fn preview( &self, layout: SpatialLayout, diff --git a/crates/hypercolor-daemon/src/domain/scene_tree.rs b/crates/hypercolor-daemon/src/domain/scene_tree.rs index a8374786d..d2b833cd4 100644 --- a/crates/hypercolor-daemon/src/domain/scene_tree.rs +++ b/crates/hypercolor-daemon/src/domain/scene_tree.rs @@ -27,7 +27,9 @@ use hypercolor_types::layer::{LayerSource, SceneLayer, SceneLayerId}; use hypercolor_types::scene::{Scene, SceneId, UnassignedBehavior, Zone, ZoneId, ZoneRole}; use hypercolor_types::spatial::{Output, SpatialLayout}; -use hypercolor_core::scene::{LayerMutationError, OutputPlacement}; +use hypercolor_core::scene::{ + LayerMutationError, OutputPlacement, primary_zone_follows_active_layout, +}; use crate::domain::commit::SceneCommit; use crate::domain::context::SceneContext; @@ -641,32 +643,81 @@ pub async fn unassign_member( /// [`DomainError::NotFound`] for an unknown zone, /// [`DomainError::Validation`] when the placements are not exactly the /// zone's current members, [`DomainError::PreconditionFailed`] for a -/// stale revision. +/// stale revision. A primary zone that follows the active layout can +/// also report a layout store or publication failure after the zone +/// write itself committed. pub async fn set_zone_layout( ctx: &SceneTreeContext, zone_id: ZoneId, placements: Vec, expected_revision: Option, ) -> Result { + // The zone write and the active-layout write it may imply finish + // together even when the caller goes away between them. + let ctx = ctx.clone(); + crate::audit_log::spawn_attributed(async move { + set_zone_layout_workflow(&ctx, zone_id, placements, expected_revision).await + }) + .await + .map_err(|error| { + DomainError::Internal(anyhow::anyhow!("zone layout workflow failed: {error}")) + })? +} + +async fn set_zone_layout_workflow( + ctx: &SceneTreeContext, + zone_id: ZoneId, + placements: Vec, + expected_revision: Option, +) -> Result { + // Taken before the zone is read, so the active layout a following + // primary zone mirrors cannot be replaced between its read and its + // republication. + let layout_guard = ctx.layout.acquire_update_guard().await; let mut mutation = ctx.scene.begin_mutation().await; check_scene_revision(&mutation, expected_revision)?; let scene_id = mutation.active_scene_for_runtime_mutation("laying out a zone")?; ensure_live_zone_mutable(&mutation, zone_id)?; - let stored = active_scene(&mutation)? + let scene = active_scene(&mutation)?; + let stored_zone = scene .zones .iter() .find(|zone| zone.id == zone_id) - .ok_or_else(|| DomainError::not_found(ResourceKind::Zone, zone_id))? - .layout - .clone(); - let layout = layout_from_placements(&stored, placements)?; + .ok_or_else(|| DomainError::not_found(ResourceKind::Zone, zone_id))?; + let follows_active_layout = stored_zone.role == ZoneRole::Primary + && stored_zone.display_target.is_none() + && primary_zone_follows_active_layout(&scene); + let layout = layout_from_placements(&stored_zone.layout, placements)?; let zone = mutation .set_zone_layout(scene_id, zone_id, layout) .map_err(|error| zone_layout_error(error, zone_id))?; mutation.retire_zone_preview(scene_id, zone_id); + let saved_layout = follows_active_layout.then(|| zone.layout.clone()); - let written = finish_zone_mutation(ctx, mutation, scene_id, zone).await?; + let mut written = finish_zone_mutation(ctx, mutation, scene_id, zone).await?; + let Some(saved_layout) = saved_layout else { + return Ok(written); + }; + + let runtime = ctx.scene.layout_runtime(); + let published = ctx + .layout + .adopt_primary_zone_layout_under_guard(&layout_guard, &saved_layout, runtime) + .await?; + if published { + // The publication admits a scene commit of its own. When it is the + // only one since the zone write, its revision is the one the + // caller's next guarded write must carry. + let revision = ctx.scene.revision(); + if revision == written.revision.saturating_add(1) { + written.revision = revision; + } + } + drop(layout_guard); + if published { + ctx.layout.converge_persisted_update(runtime).await; + } Ok(written) } diff --git a/crates/hypercolor-daemon/tests/studio_layout_persistence_tests.rs b/crates/hypercolor-daemon/tests/studio_layout_persistence_tests.rs new file mode 100644 index 000000000..fd84bf79d --- /dev/null +++ b/crates/hypercolor-daemon/tests/studio_layout_persistence_tests.rs @@ -0,0 +1,608 @@ +//! A Studio placement saved on a primary zone that follows the active +//! layout lands in that layout, so every path that rebuilds the zone from +//! the active layout (effect and preset applies, layout publications, the +//! startup restore) keeps it. +#![cfg(feature = "persistence-test-hooks")] + +use std::collections::HashMap; +use std::future::Future; +use std::path::{Path, PathBuf}; +use std::sync::Arc; +use std::time::{Duration, SystemTime}; + +use axum::body::Body; +use http::{Request, StatusCode, header}; +use hypercolor_core::effect::EffectEntry; +use hypercolor_daemon::api; +use hypercolor_daemon::app_state::{AppState, AppStateBuilder}; +use hypercolor_daemon::{layout_store, runtime_state}; +use hypercolor_types::control::ControlValue; +use hypercolor_types::effect::{ + ControlDefinition, ControlKind, ControlType, EffectCategory, EffectId, EffectMetadata, + EffectSource, EffectState, PresetTemplate, +}; +use hypercolor_types::library::PresetId; +use hypercolor_types::scene::{SceneId, ZoneRole}; +use hypercolor_types::spatial::{ + EdgeBehavior, LedTopology, NormalizedPosition, Output, SamplingMode, SpatialLayout, + StripDirection, +}; +use serde_json::json; +use tower::ServiceExt; +use uuid::Uuid; + +const ACTIVE_LAYOUT_ID: &str = "layout-active"; +const AUTO_POSITION: (f64, f64) = (0.25, 0.25); +const SAVED_POSITION: (f64, f64) = (0.8, 0.7); + +struct Rig { + state: Arc, + app: axum::Router, + effect_id: EffectId, + tmp: tempfile::TempDir, +} + +impl Rig { + fn layouts_path(&self) -> PathBuf { + self.tmp.path().join("data").join("layouts.json") + } +} + +/// Boot the shape a fresh user rig has: one stored layout that is also the +/// published one, and an effect running in the Default scene's primary zone. +async fn rig_with_running_effect() -> Rig { + let tmp = tempfile::tempdir().expect("tempdir should be created"); + let data_dir = tmp.path().join("data"); + std::fs::create_dir_all(&data_dir).expect("temp data dir should be created"); + let layouts_path = data_dir.join("layouts.json"); + let state = Arc::new( + AppStateBuilder::new(data_dir) + .with_runtime_state_path(tmp.path().join("runtime-state.json")) + .build(), + ); + let app = api::build_router(Arc::clone(&state), None); + install_active_layout(&state, &layouts_path, active_layout()).await; + let aurora = effect("Aurora"); + register(&state, &aurora).await; + let (started, _) = send( + &state, + &app, + json_request( + "POST", + format!("/api/v1/effects/{}/apply", aurora.id), + json!({}), + ), + ) + .await; + assert_eq!(started.status(), StatusCode::OK); + Rig { + state, + app, + effect_id: aurora.id, + tmp, + } +} + +/// Store `layout`, persist the store the way a running daemon keeps it, +/// and publish it as the active layout. +async fn install_active_layout(state: &AppState, layouts_path: &Path, layout: SpatialLayout) { + let fixture = state.domains.layout.test_fixture(); + let stored = { + let mut catalog = fixture.catalog().write().await; + catalog.insert(layout.id.clone(), layout.clone()); + let mut stored = catalog.values().cloned().collect::>(); + stored.sort_by(|left, right| left.id.cmp(&right.id)); + stored + }; + std::fs::write( + layouts_path, + serde_json::to_vec_pretty(&stored).expect("layout store should serialize"), + ) + .expect("layout store should be written"); + fixture.replace_current(layout); +} + +/// Run `work` to completion, retiring every layout publication it queues +/// the way the render thread would. Returns how many layouts it published. +async fn drive(state: &AppState, work: F) -> (F::Output, usize) { + tokio::pin!(work); + let executor = state.layout_publication_test_executor(); + let mut publications = Vec::new(); + let output = tokio::time::timeout(Duration::from_secs(10), async { + loop { + tokio::select! { + output = &mut work => break output, + () = tokio::time::sleep(Duration::from_millis(1)) => { + if executor.pending_layout_publications() > 0 { + let executor = executor.clone(); + publications.push(tokio::spawn(async move { + executor.execute_next_layout_publication().await + })); + } + } + } + } + }) + .await + .expect("work should finish"); + let mut published = 0; + for publication in publications { + let layout = publication + .await + .expect("layout publication task should not panic") + .expect("layout publication should succeed"); + published += usize::from(layout.is_some()); + } + (output, published) +} + +async fn send( + state: &AppState, + app: &axum::Router, + request: Request, +) -> (axum::response::Response, usize) { + let (response, published) = drive(state, app.clone().oneshot(request)).await; + (response.expect("request should succeed"), published) +} + +async fn body_json(response: axum::response::Response) -> serde_json::Value { + let bytes = axum::body::to_bytes(response.into_body(), usize::MAX) + .await + .expect("response body should read"); + serde_json::from_slice(&bytes).expect("response body should be JSON") +} + +fn json_request(method: &str, uri: String, body: serde_json::Value) -> Request { + Request::builder() + .method(method) + .uri(uri) + .header("content-type", "application/json") + .body(Body::from(body.to_string())) + .expect("request should build") +} + +fn empty_request(method: &str, uri: String) -> Request { + Request::builder() + .method(method) + .uri(uri) + .body(Body::empty()) + .expect("request should build") +} + +fn etag_revision(response: &axum::response::Response) -> u64 { + response + .headers() + .get(header::ETAG) + .and_then(|value| value.to_str().ok()) + .and_then(|value| value.trim_matches('"').parse().ok()) + .expect("mutation responses carry the scene revision") +} + +fn effect(name: &str) -> EffectMetadata { + EffectMetadata { + id: EffectId::new(Uuid::now_v7()), + name: name.to_owned(), + author: "test".to_owned(), + version: "0.1.0".to_owned(), + description: format!("{name} description"), + category: EffectCategory::Ambient, + tags: vec!["test".to_owned()], + controls: vec![ControlDefinition { + id: "speed".to_owned(), + name: "Speed".to_owned(), + kind: ControlKind::Number, + control_type: ControlType::Slider, + default_value: ControlValue::Float(0.25), + min: Some(0.0), + max: Some(1.0), + step: Some(0.05), + labels: Vec::new(), + group: None, + tooltip: None, + aspect_lock: None, + preview_source: None, + binding: None, + }], + presets: vec![PresetTemplate { + id: PresetId::stable("test-fast"), + name: "Fast".to_owned(), + description: None, + controls: HashMap::from([("speed".to_owned(), ControlValue::Float(0.9))]), + }], + audio_reactive: false, + screen_reactive: false, + input_reactive: false, + source: EffectSource::Native { + path: format!("builtin/{name}").into(), + }, + license: None, + } +} + +async fn register(state: &Arc, metadata: &EffectMetadata) { + let _ = state + .domains + .effects + .register(EffectEntry { + metadata: metadata.clone(), + source_path: format!("/tmp/{}.rs", metadata.name).into(), + modified: SystemTime::now(), + state: EffectState::Loading, + }) + .await; +} + +fn strip(id: &str, segment: &str) -> Output { + Output { + id: id.to_owned(), + name: id.to_owned(), + device_id: "mock:controller".to_owned(), + zone_name: Some(segment.to_owned()), + position: NormalizedPosition::new(0.25, 0.25), + size: NormalizedPosition::new(0.5, 0.1), + rotation: 0.0, + scale: 1.0, + display_order: 0, + orientation: None, + topology: LedTopology::Strip { + count: 4, + direction: StripDirection::LeftToRight, + }, + led_positions: Vec::new(), + led_mapping: None, + sampling_mode: Some(SamplingMode::Bilinear), + edge_behavior: Some(EdgeBehavior::Clamp), + shape: None, + shape_preset: None, + attachment: None, + brightness: None, + } +} + +/// Two strips, both at the auto-layout default position. +fn active_layout() -> SpatialLayout { + SpatialLayout { + id: ACTIVE_LAYOUT_ID.to_owned(), + name: "Active".to_owned(), + description: Some("The rig".to_owned()), + canvas_width: 640, + canvas_height: 480, + zones: vec![strip("strip-a", "ch1"), strip("strip-b", "ch2")], + default_sampling_mode: SamplingMode::Bilinear, + default_edge_behavior: EdgeBehavior::Clamp, + version: 3, + } +} + +async fn primary_zone(app: &axum::Router) -> serde_json::Value { + let response = app + .clone() + .oneshot(empty_request("GET", "/api/v1/scene".into())) + .await + .expect("request should succeed"); + assert_eq!(response.status(), StatusCode::OK); + let document = body_json(response).await; + document["data"]["zones"] + .as_array() + .expect("zones array") + .iter() + .find(|zone| zone["role"] == "primary") + .cloned() + .expect("the live tree always carries a primary zone") +} + +async fn scene_revision(app: &axum::Router) -> u64 { + let response = app + .clone() + .oneshot(empty_request("GET", "/api/v1/scene".into())) + .await + .expect("request should succeed"); + etag_revision(&response) +} + +// Positions are f32 on the wire and in the store; round so 0.8f32 compares +// as 0.8. +fn rounded(x: f64, y: f64) -> (f64, f64) { + let round = |value: f64| (value * 1000.0).round() / 1000.0; + (round(x), round(y)) +} + +fn strip_a_in_zone(zone: &serde_json::Value) -> (f64, f64) { + let member_id = zone["members"] + .as_array() + .expect("members") + .iter() + .find(|member| member["segment"] == "ch1") + .expect("strip-a member")["id"] + .clone(); + let placement = zone["layout"]["placements"] + .as_array() + .expect("placements") + .iter() + .find(|placement| placement["member"] == member_id) + .expect("strip-a placement"); + rounded( + placement["position"]["x"].as_f64().expect("x"), + placement["position"]["y"].as_f64().expect("y"), + ) +} + +fn strip_a_in_layout(layout: &SpatialLayout) -> (f64, f64) { + let output = layout + .zones + .iter() + .find(|output| output.id == "strip-a") + .expect("strip-a output"); + rounded(f64::from(output.position.x), f64::from(output.position.y)) +} + +/// Studio's Save: move strip-a and persist through the per-zone route, +/// guarded by the revision the editor last saw. Returns the response and +/// how many layouts the Save published. +async fn studio_moves_strip_a(rig: &Rig) -> (axum::response::Response, usize) { + let zone = primary_zone(&rig.app).await; + let zone_id = zone["id"].as_str().expect("zone id").to_owned(); + let strip_a_member = zone["members"] + .as_array() + .expect("members") + .iter() + .find(|member| member["segment"] == "ch1") + .expect("strip-a member")["id"] + .clone(); + let mut placements = zone["layout"]["placements"].clone(); + for placement in placements.as_array_mut().expect("placements") { + if placement["member"] == strip_a_member { + placement["position"] = json!({ "x": SAVED_POSITION.0, "y": SAVED_POSITION.1 }); + } + } + let mut request = json_request( + "PUT", + format!("/api/v1/scene/zones/{zone_id}/layout"), + json!({ "placements": placements }), + ); + let revision = scene_revision(&rig.app).await; + request.headers_mut().insert( + header::IF_MATCH, + format!("\"{revision}\"").parse().expect("valid If-Match"), + ); + let (saved, published) = send(&rig.state, &rig.app, request).await; + assert_eq!(saved.status(), StatusCode::OK, "Studio Save should land"); + assert_eq!( + strip_a_in_zone(&primary_zone(&rig.app).await), + SAVED_POSITION, + "the saved position is live right after Save" + ); + (saved, published) +} + +async fn apply_preset(rig: &Rig) { + let preset_id = PresetId::stable("test-fast"); + let (applied, _) = send( + &rig.state, + &rig.app, + json_request( + "POST", + format!( + "/api/v1/effects/{}/presets/{preset_id}/apply", + rig.effect_id + ), + json!({}), + ), + ) + .await; + assert_eq!(applied.status(), StatusCode::OK); +} + +#[tokio::test] +async fn saved_primary_zone_placement_survives_a_preset_apply() { + let rig = rig_with_running_effect().await; + studio_moves_strip_a(&rig).await; + + apply_preset(&rig).await; + + assert_eq!( + strip_a_in_zone(&primary_zone(&rig.app).await), + SAVED_POSITION, + "changing presets must not move a strip the user positioned and saved in Studio" + ); +} + +#[tokio::test] +async fn saved_primary_zone_placement_survives_an_effect_switch() { + let rig = rig_with_running_effect().await; + studio_moves_strip_a(&rig).await; + + let nebula = effect("Nebula"); + register(&rig.state, &nebula).await; + let (applied, _) = send( + &rig.state, + &rig.app, + json_request( + "POST", + format!("/api/v1/effects/{}/apply", nebula.id), + json!({}), + ), + ) + .await; + assert_eq!(applied.status(), StatusCode::OK); + + assert_eq!( + strip_a_in_zone(&primary_zone(&rig.app).await), + SAVED_POSITION, + "switching effects must not move a strip the user positioned and saved in Studio" + ); +} + +#[tokio::test] +async fn saved_primary_zone_placement_lands_in_the_active_layout_and_its_store() { + let rig = rig_with_running_effect().await; + let (saved, published) = studio_moves_strip_a(&rig).await; + assert_eq!(published, 1, "the Save republishes the active layout"); + + assert_eq!( + etag_revision(&saved), + scene_revision(&rig.app).await, + "the Save answers with the revision the next guarded write needs" + ); + + let published = rig.state.domains.layout.current(); + assert_eq!(strip_a_in_layout(&published), SAVED_POSITION); + let expected_identity = active_layout(); + assert_eq!(published.id, expected_identity.id); + assert_eq!(published.name, expected_identity.name); + assert_eq!(published.description, expected_identity.description); + assert_eq!( + (published.canvas_width, published.canvas_height), + ( + expected_identity.canvas_width, + expected_identity.canvas_height + ) + ); + assert_eq!(published.version, expected_identity.version); + assert_eq!( + published + .zones + .iter() + .map(|output| output.id.as_str()) + .collect::>(), + vec!["strip-a", "strip-b"] + ); + + let (active, _) = send( + &rig.state, + &rig.app, + empty_request("GET", format!("/api/v1/layouts/{ACTIVE_LAYOUT_ID}")), + ) + .await; + assert_eq!(active.status(), StatusCode::OK); + let mut active = body_json(active).await; + let active: SpatialLayout = + serde_json::from_value(active["data"].take()).expect("stored layout"); + assert_eq!(strip_a_in_layout(&active), SAVED_POSITION); + + let on_disk = layout_store::load(&rig.layouts_path()).expect("layout store should load"); + assert_eq!( + strip_a_in_layout( + on_disk + .get(ACTIVE_LAYOUT_ID) + .expect("active layout persisted") + ), + SAVED_POSITION + ); +} + +#[tokio::test] +async fn saved_primary_zone_placement_survives_reapplying_the_active_layout() { + let rig = rig_with_running_effect().await; + studio_moves_strip_a(&rig).await; + + let (applied, published) = send( + &rig.state, + &rig.app, + empty_request("POST", format!("/api/v1/layouts/{ACTIVE_LAYOUT_ID}/apply")), + ) + .await; + assert_eq!(applied.status(), StatusCode::OK); + assert_eq!(published, 1, "the stored layout republishes"); + + assert_eq!( + strip_a_in_zone(&primary_zone(&rig.app).await), + SAVED_POSITION + ); + apply_preset(&rig).await; + assert_eq!( + strip_a_in_zone(&primary_zone(&rig.app).await), + SAVED_POSITION + ); +} + +#[tokio::test] +async fn saved_primary_zone_placement_survives_a_republication_of_the_live_layout() { + let rig = rig_with_running_effect().await; + studio_moves_strip_a(&rig).await; + + // Device hotplug and auto-layout repair republish the live layout. + let live = rig.state.domains.layout.current(); + let workflows = rig.state.domains.layout.test_workflows(); + let (result, published) = drive(&rig.state, workflows.publish(live)).await; + result.expect("republication should land"); + assert_eq!(published, 1); + + assert_eq!( + strip_a_in_zone(&primary_zone(&rig.app).await), + SAVED_POSITION + ); +} + +#[tokio::test] +async fn saved_primary_zone_placement_is_restored_after_a_restart() { + let rig = rig_with_running_effect().await; + studio_moves_strip_a(&rig).await; + + // Startup restores the stored layout the runtime session names, then + // re-aligns the primary zone to it. + let snapshot = runtime_state::load(&rig.state.runtime_state_path) + .expect("runtime state should load") + .expect("runtime state should exist after Save"); + assert_eq!(snapshot.active_scene_id, Some(SceneId::DEFAULT.to_string())); + assert_eq!(snapshot.active_layout_id.as_deref(), Some(ACTIVE_LAYOUT_ID)); + let persisted_primary = snapshot + .default_scene_zones + .iter() + .find(|zone| zone.role == ZoneRole::Primary) + .expect("persisted primary zone"); + assert_eq!(strip_a_in_layout(&persisted_primary.layout), SAVED_POSITION); + let stored = layout_store::load(&rig.layouts_path()) + .expect("layout store should load") + .remove(ACTIVE_LAYOUT_ID) + .expect("the runtime session's active layout is stored"); + + let restarted = rig_with_running_effect().await; + install_active_layout(&restarted.state, &restarted.layouts_path(), stored).await; + apply_preset(&restarted).await; + + assert_eq!( + strip_a_in_zone(&primary_zone(&restarted.app).await), + SAVED_POSITION, + "the first preset change after a restart keeps the saved placement" + ); +} + +#[tokio::test] +async fn custom_led_zone_keeps_the_primary_layout_scene_local() { + let rig = rig_with_running_effect().await; + let (created, _) = send( + &rig.state, + &rig.app, + json_request( + "POST", + "/api/v1/scene/zones".into(), + json!({ "name": "Desk" }), + ), + ) + .await; + assert_eq!(created.status(), StatusCode::CREATED); + + let (_, published) = studio_moves_strip_a(&rig).await; + assert_eq!(published, 0, "a scene-local Save publishes no layout"); + + apply_preset(&rig).await; + assert_eq!( + strip_a_in_zone(&primary_zone(&rig.app).await), + SAVED_POSITION, + "a scene-local primary layout survives an apply on its own" + ); + assert_eq!( + strip_a_in_layout(&rig.state.domains.layout.current()), + AUTO_POSITION, + "the active layout is untouched" + ); + let stored = rig + .state + .domains + .layout + .resolve(ACTIVE_LAYOUT_ID) + .await + .expect("active layout stays stored"); + assert_eq!(strip_a_in_layout(&stored), AUTO_POSITION); +} From 1a68ad1e8cdf6c94205442493cd08e29fe19c952 Mon Sep 17 00:00:00 2001 From: Stefanie Jane Date: Mon, 19 Oct 2026 10:25:59 -0700 Subject: [PATCH 3/4] fix(studio): carry only what a Save changed into the shared layout Mirroring the whole primary zone into the active layout was wrong whenever the zone was a stale snapshot. A named scene keeps the zone it was created with, and activation never re-aligned it, so a Save there dropped outputs the layout gained since (and auto-excluded their devices), reverted positions saved from another scene, and replaced a stored layout edit that had not been applied yet. A Save now copies only the placement fields of outputs whose placement it changed, by output id, onto the stored entry and onto the published layout separately, wherever each holds that output. Outputs neither layout knows (members assigned in Studio) are added to both, as the startup restore does. Nothing is removed, order and every other field stay put, exclusions are untouched, and an unapplied stored edit stays stored and unapplied. If the store write or the publication is refused, the zone is put back and the refusal is returned, so a Save is never left half applied; a failed rollback is named in the error. Scene activation, deactivation, deleting the active scene, and the configured startup scene now re-align a following primary zone to the active layout, so a scene switch never shows stale positions. Snapshot scenes keep what they captured, and a scene with its own layout is re-aligned again when that layout publishes. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../src/domain/layout/workflows.rs | 157 +++++--- crates/hypercolor-daemon/src/domain/scene.rs | 26 ++ .../src/domain/scene_activation.rs | 1 + .../src/domain/scene_tree.rs | 77 +++- .../src/startup/lifecycle.rs | 1 + .../tests/studio_layout_persistence_tests.rs | 342 +++++++++++++++++- 6 files changed, 530 insertions(+), 74 deletions(-) diff --git a/crates/hypercolor-daemon/src/domain/layout/workflows.rs b/crates/hypercolor-daemon/src/domain/layout/workflows.rs index 88fe8332f..f9632ac06 100644 --- a/crates/hypercolor-daemon/src/domain/layout/workflows.rs +++ b/crates/hypercolor-daemon/src/domain/layout/workflows.rs @@ -7,7 +7,7 @@ use hypercolor_types::api::layouts::{ LayoutSummary, PreviewLayoutResponse, UpdateLayoutRequest, }; use hypercolor_types::identity::LayoutId; -use hypercolor_types::spatial::{SamplingMode, SpatialLayout}; +use hypercolor_types::spatial::{Output, SamplingMode, SpatialLayout}; use crate::domain::DomainError; use crate::scene_transactions::LayoutUpdateGuard; @@ -331,78 +331,103 @@ impl LayoutContext { }) } - /// Give the active layout the outputs of a primary zone that follows it. + /// Carry a Studio save on a primary zone that follows the active + /// layout into that layout. /// - /// Without a custom LED zone, effect applies, layout publications, and - /// the startup restore all rewrite the primary zone from the active - /// layout, so a placement saved on that zone has to land in the layout - /// itself. The layout keeps its id, name, description, canvas size, - /// sampling defaults, and version, and takes the zone's outputs in the - /// zone's order. The store entry is replaced and persisted first, then - /// published under the caller's guard; a failed publication restores - /// the previous entry. Returns whether a new layout was published. + /// Effect applies, layout publications, and the startup restore + /// rebuild such a zone from the active layout, so a placement saved + /// only on the zone is lost at the next rebuild. Only what the save + /// changed travels: for every output whose placement differs between + /// `before` and `after`, the placement fields are copied by output id + /// onto the stored entry and onto the published layout, wherever each + /// already holds that output. Outputs the zone holds that neither + /// layout knows (members assigned in Studio) are added to both, as the + /// startup restore would. Nothing is removed, output order and every + /// other field stay as they were, auto-layout exclusions are left + /// alone, and a stored edit that was never applied stays stored and + /// unapplied. An active layout with no stored entry is only published. /// - /// The caller runs connectivity convergence only after releasing the - /// guard, because that pass takes the guard itself. + /// The stored entry is persisted first, then the published layout goes + /// out under the caller's guard; a rejected publication restores the + /// stored entry. Returns whether a layout was published. The caller + /// runs connectivity convergence only after releasing the guard, + /// because that pass takes the guard itself. /// /// # Errors /// - /// Persistence and publication failures, after the store entry has - /// been rolled back. - pub(crate) async fn adopt_primary_zone_layout_under_guard( + /// Persistence and publication failures, after the stored entry has + /// been rolled back, or naming the rollback failure as well when it + /// could not be. + pub(crate) async fn mirror_primary_zone_save_under_guard( &self, guard: &LayoutUpdateGuard, - zone_layout: &SpatialLayout, + before: &[Output], + after: &[Output], runtime: &LayoutRuntime, ) -> Result { let current = self.current(); - if current.zones == zone_layout.zones { - return Ok(false); + let layout_id = current.id.clone(); + let stored = self.catalog.entries().read().await.get(&layout_id).cloned(); + + let mut published = current.clone(); + let mut stored_next = stored.clone(); + for saved in after { + let moved = before + .iter() + .find(|output| output.id == saved.id) + .is_none_or(|output| placement_differs(output, saved)); + let in_published = copy_placement_if_held(&mut published, saved, moved); + let in_stored = stored_next + .as_mut() + .is_some_and(|layout| copy_placement_if_held(layout, saved, moved)); + if !in_published && !in_stored { + published.zones.push(saved.clone()); + if let Some(layout) = stored_next.as_mut() { + layout.zones.push(saved.clone()); + } + } } - let previous_zones = current.zones.clone(); - let mut updated = current; - updated.zones.clone_from(&zone_layout.zones); - let layout_id = updated.id.clone(); - let previous_entry = self - .catalog - .entries() - .write() - .await - .insert(layout_id.clone(), updated.clone()); - if let Err(error) = self.catalog.persist().await { - let rollback = self - .restore_catalog_entry(&layout_id, previous_entry) + let stored_changed = stored_next != stored; + let published_changed = published != current; + if stored_changed && let Some(layout) = stored_next { + self.catalog + .entries() + .write() .await - .err() - .map(|error| format!("layout store rollback failed: {error}")); - return Err(layout_store_persistence_error( - "zone layout save", - error, - rollback, - )); + .insert(layout_id.clone(), layout); + if let Err(error) = self.catalog.persist().await { + let rollback = self + .restore_catalog_entry(&layout_id, stored.clone()) + .await + .err() + .map(|error| format!("layout store rollback failed: {error}")); + return Err(layout_store_persistence_error( + "zone layout save", + error, + rollback, + )); + } } - if let Err(error) = self - .admit_persisted_update_under_guard(guard, updated.clone(), runtime) - .await + if published_changed + && let Err(error) = self + .admit_persisted_update_under_guard(guard, published, runtime) + .await { - if let Err(rollback_error) = - self.restore_catalog_entry(&layout_id, previous_entry).await + if stored_changed + && let Err(rollback_error) = self.restore_catalog_entry(&layout_id, stored).await { - tracing::warn!( - layout_id, - %rollback_error, - "layout store rollback failed after a rejected zone layout publication" - ); + return Err(DomainError::Internal(anyhow::anyhow!( + "{error}; layout store rollback failed: {rollback_error}" + ))); } return Err(layout_update_domain_error(error)); } - self.exclusions - .reconcile_layout(&layout_id, &previous_zones, &updated.zones) - .await; - self.publish_layout_changed(None, layout_id); - Ok(true) + if stored_changed || published_changed { + self.publish_layout_changed(None, layout_id); + } + Ok(published_changed) } async fn restore_catalog_entry( @@ -734,3 +759,29 @@ impl LayoutContext { active_layout_error.map_or(Ok(()), Err) } } + +/// Whether the fields a zone layout save carries differ between two copies +/// of one output. +fn placement_differs(before: &Output, after: &Output) -> bool { + before.position != after.position + || before.size != after.size + || before.rotation.to_bits() != after.rotation.to_bits() + || before.scale.to_bits() != after.scale.to_bits() + || before.orientation != after.orientation +} + +/// Copy a saved placement onto `layout`'s copy of the output when `moved`. +/// Returns whether `layout` holds the output at all. +fn copy_placement_if_held(layout: &mut SpatialLayout, saved: &Output, moved: bool) -> bool { + let Some(output) = layout.zones.iter_mut().find(|output| output.id == saved.id) else { + return false; + }; + if moved { + output.position = saved.position; + output.size = saved.size; + output.rotation = saved.rotation; + output.scale = saved.scale; + output.orientation = saved.orientation; + } + true +} diff --git a/crates/hypercolor-daemon/src/domain/scene.rs b/crates/hypercolor-daemon/src/domain/scene.rs index 98a604d67..d2434c1af 100644 --- a/crates/hypercolor-daemon/src/domain/scene.rs +++ b/crates/hypercolor-daemon/src/domain/scene.rs @@ -1393,6 +1393,27 @@ impl SceneMutation { self.candidate.sync_primary_zone_layout(layout); } + /// Re-align a newly active scene's primary zone with the active layout + /// when that zone follows it. + /// + /// A scene keeps the primary zone it was created or last active with, + /// and activation alone moves only the priority stack. Without a custom + /// LED zone that copy goes stale as soon as the layout moves on, so a + /// scene switch re-aligns it the way a layout publication does for the + /// scene active at the time. A scene that names its own layout is + /// re-aligned once more when that layout is published after activation, + /// and a snapshot-locked scene keeps what it captured. + pub fn realign_primary_layout(&mut self, layout: &SpatialLayout) { + if self + .candidate + .active_scene() + .is_some_and(Scene::blocks_runtime_mutation) + { + return; + } + self.candidate.sync_primary_zone_layout(layout); + } + /// Restore a persisted scene without scheduling a redundant store write. pub fn restore_scene(&mut self, scene: Scene) -> Result<(), DomainError> { self.candidate.update(scene).map_err(|error| { @@ -2504,6 +2525,7 @@ pub async fn activate_scene( SceneChangeReason::UserActivate, )?; mutation.sync_active_display_surfaces(&display_surfaces); + mutation.realign_primary_layout(&ctx.layout.current()); let commit = ctx.scene.commit(mutation).await?; @@ -3094,6 +3116,9 @@ pub async fn delete_scene( let mut mutation = ctx.scene.begin_mutation().await; let previous_scene_id = mutation.scenes().active_scene_id().copied(); let scene = mutation.delete_scene(&scene_id)?; + if is_active { + mutation.realign_primary_layout(&ctx.layout.current()); + } let current_scene = mutation.scenes().active_scene().cloned(); mutation.retire_scene_previews(scene_id); @@ -3132,6 +3157,7 @@ pub async fn deactivate_scene(ctx: &SceneLibraryContext) -> Result published, + Err(refusal) => { + let error = undo_zone_layout_save( + ctx, + scene_id, + zone_id, + &saved_layout, + previous_layout, + refusal, + ) + .await; + return Err(error); + } + }; if published { // The publication admits a scene commit of its own. When it is the // only one since the zone write, its revision is the one the @@ -721,6 +742,48 @@ async fn set_zone_layout_workflow( Ok(written) } +/// Put back a zone's pre-save layout after the active layout refused the +/// save, which would otherwise revert at the next rebuild of the zone. A +/// zone something else rewrote since is left alone. Answers with the +/// refusal, or with an error naming both failures when the restore fails. +async fn undo_zone_layout_save( + ctx: &SceneTreeContext, + scene_id: SceneId, + zone_id: ZoneId, + saved: &SpatialLayout, + previous: SpatialLayout, + refusal: DomainError, +) -> DomainError { + let restored = ctx + .scene + .commit_retrying(|mutation| { + let still_saved = mutation + .scenes() + .get(&scene_id) + .and_then(|scene| scene.zones.iter().find(|zone| zone.id == zone_id)) + .is_some_and(|zone| zone.layout == *saved); + if !still_saved { + return Ok(None); + } + mutation + .set_zone_layout(scene_id, zone_id, previous.clone()) + .map_err(|error| zone_layout_error(error, zone_id))?; + Ok(Some(())) + }) + .await; + match restored { + Ok(Some(_)) => { + ctx.scene.save_runtime_session().await; + refusal + } + Ok(None) => refusal, + Err(error) => DomainError::Internal(anyhow::anyhow!( + "the zone layout was saved but the active layout refused it ({refusal}); \ + restoring the zone failed: {error}" + )), + } +} + // ── Shared steps ───────────────────────────────────────────────────────── async fn finish_zone_mutation( diff --git a/crates/hypercolor-daemon/src/startup/lifecycle.rs b/crates/hypercolor-daemon/src/startup/lifecycle.rs index 48610d1f0..7eba9770f 100644 --- a/crates/hypercolor-daemon/src/startup/lifecycle.rs +++ b/crates/hypercolor-daemon/src/startup/lifecycle.rs @@ -580,6 +580,7 @@ impl DaemonState { warn!(selector, scene_id = %scene_id, %error, "Failed to activate configured startup scene"); return; } + mutation.realign_primary_layout(self.spatial_engine.snapshot().layout().as_ref()); if let Err(error) = self.scene_manager.commit_mutation(mutation).await { warn!(selector, scene_id = %scene_id, %error, "Failed to commit configured startup scene"); return; diff --git a/crates/hypercolor-daemon/tests/studio_layout_persistence_tests.rs b/crates/hypercolor-daemon/tests/studio_layout_persistence_tests.rs index fd84bf79d..f3803e8f9 100644 --- a/crates/hypercolor-daemon/tests/studio_layout_persistence_tests.rs +++ b/crates/hypercolor-daemon/tests/studio_layout_persistence_tests.rs @@ -1,7 +1,9 @@ //! A Studio placement saved on a primary zone that follows the active //! layout lands in that layout, so every path that rebuilds the zone from -//! the active layout (effect and preset applies, layout publications, the -//! startup restore) keeps it. +//! the active layout (effect and preset applies, layout publications, scene +//! switches, the startup restore) keeps it. Only what the Save changed +//! lands there: outputs, stored edits, and positions saved from other +//! scenes stay as they were. #![cfg(feature = "persistence-test-hooks")] use std::collections::HashMap; @@ -13,6 +15,7 @@ use std::time::{Duration, SystemTime}; use axum::body::Body; use http::{Request, StatusCode, header}; use hypercolor_core::effect::EffectEntry; +use hypercolor_daemon::LayoutTransactionRejection; use hypercolor_daemon::api; use hypercolor_daemon::app_state::{AppState, AppStateBuilder}; use hypercolor_daemon::{layout_store, runtime_state}; @@ -21,6 +24,7 @@ use hypercolor_types::effect::{ ControlDefinition, ControlKind, ControlType, EffectCategory, EffectId, EffectMetadata, EffectSource, EffectState, PresetTemplate, }; +use hypercolor_types::event::ChangeTrigger; use hypercolor_types::library::PresetId; use hypercolor_types::scene::{SceneId, ZoneRole}; use hypercolor_types::spatial::{ @@ -327,32 +331,43 @@ fn strip_a_in_zone(zone: &serde_json::Value) -> (f64, f64) { ) } -fn strip_a_in_layout(layout: &SpatialLayout) -> (f64, f64) { +fn output_in_layout(layout: &SpatialLayout, id: &str) -> (f64, f64) { let output = layout .zones .iter() - .find(|output| output.id == "strip-a") - .expect("strip-a output"); + .find(|output| output.id == id) + .unwrap_or_else(|| panic!("{id} output")); rounded(f64::from(output.position.x), f64::from(output.position.y)) } -/// Studio's Save: move strip-a and persist through the per-zone route, -/// guarded by the revision the editor last saw. Returns the response and -/// how many layouts the Save published. -async fn studio_moves_strip_a(rig: &Rig) -> (axum::response::Response, usize) { +fn strip_a_in_layout(layout: &SpatialLayout) -> (f64, f64) { + output_in_layout(layout, "strip-a") +} + +fn output_ids(layout: &SpatialLayout) -> Vec<&str> { + layout + .zones + .iter() + .map(|output| output.id.as_str()) + .collect() +} + +/// Studio's Save request: move one segment's member and persist through +/// the per-zone route, guarded by the revision the editor last saw. +async fn studio_save_request(rig: &Rig, segment: &str, to: (f64, f64)) -> Request { let zone = primary_zone(&rig.app).await; let zone_id = zone["id"].as_str().expect("zone id").to_owned(); - let strip_a_member = zone["members"] + let member = zone["members"] .as_array() .expect("members") .iter() - .find(|member| member["segment"] == "ch1") - .expect("strip-a member")["id"] + .find(|member| member["segment"] == segment) + .expect("moved member")["id"] .clone(); let mut placements = zone["layout"]["placements"].clone(); for placement in placements.as_array_mut().expect("placements") { - if placement["member"] == strip_a_member { - placement["position"] = json!({ "x": SAVED_POSITION.0, "y": SAVED_POSITION.1 }); + if placement["member"] == member { + placement["position"] = json!({ "x": to.0, "y": to.1 }); } } let mut request = json_request( @@ -365,8 +380,25 @@ async fn studio_moves_strip_a(rig: &Rig) -> (axum::response::Response, usize) { header::IF_MATCH, format!("\"{revision}\"").parse().expect("valid If-Match"), ); + request +} + +/// Save a move of one segment's member. Returns the response and how many +/// layouts the Save published. +async fn studio_moves( + rig: &Rig, + segment: &str, + to: (f64, f64), +) -> (axum::response::Response, usize) { + let request = studio_save_request(rig, segment, to).await; let (saved, published) = send(&rig.state, &rig.app, request).await; assert_eq!(saved.status(), StatusCode::OK, "Studio Save should land"); + (saved, published) +} + +/// Studio's Save of strip-a at [`SAVED_POSITION`]. +async fn studio_moves_strip_a(rig: &Rig) -> (axum::response::Response, usize) { + let (saved, published) = studio_moves(rig, "ch1", SAVED_POSITION).await; assert_eq!( strip_a_in_zone(&primary_zone(&rig.app).await), SAVED_POSITION, @@ -606,3 +638,285 @@ async fn custom_led_zone_keeps_the_primary_layout_scene_local() { .expect("active layout stays stored"); assert_eq!(strip_a_in_layout(&stored), AUTO_POSITION); } + +async fn create_named_scene(rig: &Rig, name: &str) -> String { + let (created, _) = send( + &rig.state, + &rig.app, + json_request("POST", "/api/v1/scenes".into(), json!({ "name": name })), + ) + .await; + assert_eq!(created.status(), StatusCode::CREATED); + body_json(created).await["data"]["id"] + .as_str() + .expect("scene id") + .to_owned() +} + +async fn activate(rig: &Rig, scene_id: &str) { + let (activated, _) = send( + &rig.state, + &rig.app, + json_request( + "POST", + format!("/api/v1/scenes/{scene_id}/activate"), + json!({}), + ), + ) + .await; + assert_eq!(activated.status(), StatusCode::OK); +} + +/// Store an edit of the active layout without applying it. +async fn store_layout_edit(rig: &Rig, body: serde_json::Value) { + let (updated, _) = send( + &rig.state, + &rig.app, + json_request("PUT", format!("/api/v1/layouts/{ACTIVE_LAYOUT_ID}"), body), + ) + .await; + assert_eq!(updated.status(), StatusCode::OK); +} + +fn stored_active_layout(rig: &Rig) -> SpatialLayout { + layout_store::load(&rig.layouts_path()) + .expect("layout store should load") + .remove(ACTIVE_LAYOUT_ID) + .expect("active layout is stored") +} + +fn strip_c_on_another_device() -> Output { + let mut strip_c = strip("strip-c", "ch1"); + strip_c.device_id = "mock:other".to_owned(); + strip_c +} + +#[tokio::test] +async fn activating_a_named_scene_realigns_its_primary_zone_to_the_active_layout() { + let rig = rig_with_running_effect().await; + let gaming = create_named_scene(&rig, "Gaming").await; + studio_moves_strip_a(&rig).await; + + activate(&rig, &gaming).await; + + assert_eq!( + strip_a_in_zone(&primary_zone(&rig.app).await), + SAVED_POSITION, + "a scene created before the Save shows the saved position, not its snapshot" + ); +} + +#[tokio::test] +async fn a_save_in_another_scene_keeps_positions_saved_elsewhere() { + let rig = rig_with_running_effect().await; + let gaming = create_named_scene(&rig, "Gaming").await; + studio_moves_strip_a(&rig).await; + activate(&rig, &gaming).await; + + studio_moves(&rig, "ch2", (0.1, 0.9)).await; + + let published = rig.state.domains.layout.current(); + let stored = stored_active_layout(&rig); + for layout in [&published, &stored] { + assert_eq!(strip_a_in_layout(layout), SAVED_POSITION); + assert_eq!(output_in_layout(layout, "strip-b"), (0.1, 0.9)); + } +} + +#[tokio::test] +async fn a_save_in_a_stale_named_scene_keeps_outputs_only_the_active_layout_holds() { + let rig = rig_with_running_effect().await; + let gaming = create_named_scene(&rig, "Gaming").await; + let mut zones = active_layout().zones; + zones.push(strip_c_on_another_device()); + store_layout_edit(&rig, json!({ "zones": zones })).await; + let (applied, _) = send( + &rig.state, + &rig.app, + empty_request("POST", format!("/api/v1/layouts/{ACTIVE_LAYOUT_ID}/apply")), + ) + .await; + assert_eq!(applied.status(), StatusCode::OK); + activate(&rig, &gaming).await; + + studio_moves_strip_a(&rig).await; + + let published = rig.state.domains.layout.current(); + let stored = stored_active_layout(&rig); + for layout in [&published, &stored] { + assert_eq!(output_ids(layout), vec!["strip-a", "strip-b", "strip-c"]); + assert_eq!(strip_a_in_layout(layout), SAVED_POSITION); + } + let exclusions = std::fs::read_to_string( + rig.tmp + .path() + .join("data") + .join("layout-auto-exclusions.json"), + ) + .unwrap_or_default(); + assert!( + !exclusions.contains("mock:other"), + "a Save never excludes a device from auto-layout: {exclusions}" + ); +} + +#[tokio::test] +async fn a_save_keeps_stored_layout_edits_that_were_never_applied() { + let rig = rig_with_running_effect().await; + let mut zones = active_layout().zones; + zones.push(strip_c_on_another_device()); + store_layout_edit(&rig, json!({ "name": "Renamed", "zones": zones })).await; + + studio_moves_strip_a(&rig).await; + + let stored = stored_active_layout(&rig); + assert_eq!(stored.name, "Renamed"); + assert_eq!(output_ids(&stored), vec!["strip-a", "strip-b", "strip-c"]); + assert_eq!(strip_a_in_layout(&stored), SAVED_POSITION); + let published = rig.state.domains.layout.current(); + assert_eq!(published.name, "Active", "the stored edit stays unapplied"); + assert_eq!(output_ids(&published), vec!["strip-a", "strip-b"]); + assert_eq!(strip_a_in_layout(&published), SAVED_POSITION); +} + +#[tokio::test] +async fn a_scene_that_names_its_own_layout_takes_it_on_activation() { + let rig = rig_with_running_effect().await; + let gaming = create_named_scene(&rig, "Gaming").await; + let mut own = active_layout(); + own.id = "layout-gaming".to_owned(); + own.name = "Gaming".to_owned(); + own.zones[0].position = NormalizedPosition::new(0.4, 0.6); + rig.state + .domains + .layout + .test_fixture() + .catalog() + .write() + .await + .insert(own.id.clone(), own.clone()); + let response = rig + .app + .clone() + .oneshot(empty_request("GET", format!("/api/v1/scenes/{gaming}"))) + .await + .expect("request should succeed"); + let mut document = body_json(response).await["data"].take(); + document["layout_id"] = json!(own.id); + let (replaced, _) = send( + &rig.state, + &rig.app, + json_request("PUT", format!("/api/v1/scenes/{gaming}"), document), + ) + .await; + assert_eq!(replaced.status(), StatusCode::OK); + studio_moves_strip_a(&rig).await; + + activate(&rig, &gaming).await; + + assert_eq!(rig.state.domains.layout.current().id, own.id); + assert_eq!(strip_a_in_zone(&primary_zone(&rig.app).await), (0.4, 0.6)); +} + +#[tokio::test] +async fn a_rejected_publication_leaves_the_save_undone() { + let rig = rig_with_running_effect().await; + let request = studio_save_request(&rig, "ch1", SAVED_POSITION).await; + let work = rig.app.clone().oneshot(request); + tokio::pin!(work); + let executor = rig.state.layout_publication_test_executor(); + let rejected = tokio::time::timeout(Duration::from_secs(10), async { + loop { + tokio::select! { + response = &mut work => break response.expect("request should succeed"), + () = tokio::time::sleep(Duration::from_millis(1)) => { + executor.reject_next_layout_publication( + LayoutTransactionRejection::Superseded, + ); + } + } + } + }) + .await + .expect("the Save should answer"); + + assert_eq!(rejected.status(), StatusCode::CONFLICT); + assert_eq!( + strip_a_in_zone(&primary_zone(&rig.app).await), + AUTO_POSITION, + "a Save that could not reach the active layout is not left half applied" + ); + assert_eq!( + strip_a_in_layout(&rig.state.domains.layout.current()), + AUTO_POSITION + ); + assert_eq!( + strip_a_in_layout(&stored_active_layout(&rig)), + AUTO_POSITION + ); +} + +#[tokio::test] +async fn returning_to_the_default_scene_realigns_its_primary_zone() { + let rig = rig_with_running_effect().await; + let gaming = create_named_scene(&rig, "Gaming").await; + activate(&rig, &gaming).await; + studio_moves_strip_a(&rig).await; + + let (deactivated, _) = send( + &rig.state, + &rig.app, + empty_request("POST", "/api/v1/scene/deactivate".into()), + ) + .await; + assert_eq!(deactivated.status(), StatusCode::OK); + + assert_eq!( + strip_a_in_zone(&primary_zone(&rig.app).await), + SAVED_POSITION, + "the Default scene shows the placement saved while another scene was active" + ); +} + +#[tokio::test] +async fn a_save_adopts_members_neither_layout_holds_yet() { + let rig = rig_with_running_effect().await; + // A member assigned in Studio lives in the primary zone only. + let mut minted = strip("strip-m", "ch9"); + minted.device_id = "mock:minted".to_owned(); + minted.position = NormalizedPosition::new(0.6, 0.6); + let mut zone_layout = active_layout(); + zone_layout.zones.push(minted); + let glow = effect("Glow"); + register(&rig.state, &glow).await; + let mut mutation = rig.state.scene_manager.begin_mutation().await; + mutation + .upsert_primary_zone( + &glow, + HashMap::new(), + None, + zone_layout, + ChangeTrigger::System, + None, + ) + .expect("primary zone should take the minted member"); + hypercolor_daemon::domain::scene::commit_scene(&rig.state.domains.scene, mutation) + .await + .expect("seeded zone should commit"); + + studio_moves_strip_a(&rig).await; + + let published = rig.state.domains.layout.current(); + let stored = stored_active_layout(&rig); + for layout in [&published, &stored] { + assert_eq!(output_ids(layout), vec!["strip-a", "strip-b", "strip-m"]); + assert_eq!(output_in_layout(layout, "strip-m"), (0.6, 0.6)); + } + apply_preset(&rig).await; + let zone = primary_zone(&rig.app).await; + assert_eq!( + zone["members"].as_array().map(Vec::len), + Some(3), + "the adopted member survives the next apply" + ); +} From 93395d6b9444f7439fca1b9a74331ddfd2daf9ca Mon Sep 17 00:00:00 2001 From: Stefanie Jane Date: Mon, 19 Oct 2026 10:25:59 -0700 Subject: [PATCH 4/4] fix(ui): take the saved scene revision from the zone layout response Studio assumed a zone layout Save moves the scene revision by one. A Save on a primary zone that follows the active layout also publishes that layout, which is a second commit, so the next guarded write before the scene refresh landed was refused as stale. The Save now records the revision the daemon reported in ETag, falling back to the old guess only when the header is missing. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/hypercolor-ui/src/api/client.rs | 56 ++++++++++++++++++- crates/hypercolor-ui/src/api/zones.rs | 7 ++- .../src/components/layout_builder.rs | 8 ++- 3 files changed, 64 insertions(+), 7 deletions(-) diff --git a/crates/hypercolor-ui/src/api/client.rs b/crates/hypercolor-ui/src/api/client.rs index 9045d3a96..597759f6d 100644 --- a/crates/hypercolor-ui/src/api/client.rs +++ b/crates/hypercolor-ui/src/api/client.rs @@ -685,13 +685,36 @@ pub async fn send_json_versioned( body: Option<&Req>, if_match: Option, ) -> ApiResult> +where + Req: Serialize + ?Sized, + Res: DeserializeOwned, +{ + Ok( + send_json_versioned_with_revision(method, url, body, if_match) + .await? + .map(|(data, _)| data), + ) +} + +/// [`send_json_versioned`], also returning the revision the daemon sent in +/// `ETag`. One write can move the revision by more than one, so a caller +/// that guards its next write should carry this rather than add one. +pub async fn send_json_versioned_with_revision( + method: HttpMethod, + url: &str, + body: Option<&Req>, + if_match: Option, +) -> ApiResult)>> where Req: Serialize + ?Sized, Res: DeserializeOwned, { let resp = send_request(method, url, body, if_match).await?; match resp.status { - 200..=299 => Ok(MutationOutcome::Applied(parse_envelope(&resp)?)), + 200..=299 => Ok(MutationOutcome::Applied(( + parse_envelope(&resp)?, + etag_revision(&resp), + ))), 412 => { let body: serde_json::Value = serde_json::from_slice(&resp.body).map_err(|e| ApiError::Parse(e.to_string()))?; @@ -706,6 +729,14 @@ where } } +/// Read the scene revision a successful mutation reported in `ETag`. +fn etag_revision(resp: &HttpResponse) -> Option { + resp.headers + .iter() + .find(|header| header.name.eq_ignore_ascii_case("etag")) + .and_then(|header| header.value.trim().trim_matches('"').parse().ok()) +} + /// Extract the authoritative version token from a `412` response body. /// /// The daemon serves the canonical error envelope, so the version the @@ -916,9 +947,9 @@ pub async fn delete_empty(url: &str) -> ApiResult<()> { #[cfg(test)] mod tests { use super::{ - ApiError, DaemonTransport, MutationOutcome, authorization_token, + ApiError, DaemonTransport, HttpHeader, HttpResponse, MutationOutcome, authorization_token, begin_native_daemon_verification, clear_verified_daemon_connection, daemon_url, - extract_error_message, install_verified_daemon_connection, + etag_revision, extract_error_message, install_verified_daemon_connection, parse_dev_control_credential_fragment, reset_daemon_transport_for_test, stale_current_version, validate_relative_path, }; @@ -1029,6 +1060,25 @@ mod tests { ); } + #[test] + fn etag_revision_reads_the_quoted_scene_revision() { + let response = |headers: &[(&str, &str)]| HttpResponse { + status: 200, + headers: headers + .iter() + .map(|(name, value)| HttpHeader { + name: (*name).to_owned(), + value: (*value).to_owned(), + }) + .collect(), + body: Vec::new(), + }; + assert_eq!(etag_revision(&response(&[("etag", "\"18\"")])), Some(18)); + assert_eq!(etag_revision(&response(&[("ETag", "19")])), Some(19)); + assert_eq!(etag_revision(&response(&[("etag", "W/\"x\"")])), None); + assert_eq!(etag_revision(&response(&[])), None); + } + #[test] fn mutation_outcome_map_transforms_applied_payload() { let outcome = MutationOutcome::Applied(21_u64).map(|value| value * 2); diff --git a/crates/hypercolor-ui/src/api/zones.rs b/crates/hypercolor-ui/src/api/zones.rs index 3e6e255d9..fbc3e0eeb 100644 --- a/crates/hypercolor-ui/src/api/zones.rs +++ b/crates/hypercolor-ui/src/api/zones.rs @@ -70,15 +70,18 @@ pub async fn update_zone( .await } +/// Save a zone's placements. The applied outcome carries the scene revision +/// the daemon reported, which a save of a primary zone that follows the +/// active layout moves by two. pub async fn update_zone_layout( zone_id: &str, layout: &SpatialLayout, expected_revision: u64, -) -> ApiResult> { +) -> ApiResult)>> { let request = ZoneLayoutRequest { placements: layout.zones.iter().map(member_placement).collect(), }; - client::send_json_versioned::<_, ZoneResource>( + client::send_json_versioned_with_revision::<_, ZoneResource>( HttpMethod::Put, &format!("/api/v1/scene/zones/{}/layout", path_segment(zone_id)), Some(&request), diff --git a/crates/hypercolor-ui/src/components/layout_builder.rs b/crates/hypercolor-ui/src/components/layout_builder.rs index 0a2943981..456364150 100644 --- a/crates/hypercolor-ui/src/components/layout_builder.rs +++ b/crates/hypercolor-ui/src/components/layout_builder.rs @@ -790,7 +790,7 @@ pub(crate) fn ZoneLayoutProvider( }; leptos::task::spawn_local(async move { match api::zones::update_zone_layout(&zone_id, ¤t, revision).await { - Ok(api::zones::ZoneOutcome::Applied(zone)) => { + Ok(api::zones::ZoneOutcome::Applied((zone, saved_revision))) => { if !studio_history.is_current(generation) { return; } @@ -803,7 +803,11 @@ pub(crate) fn ZoneLayoutProvider( draft.baseline = acknowledged.clone(); } }); - studio_history.record_saved_zone(scene.id, zone, revision.saturating_add(1)); + studio_history.record_saved_zone( + scene.id, + zone, + saved_revision.unwrap_or_else(|| revision.saturating_add(1)), + ); let completion = zone_save_completion( selected_zone_id.get_untracked().as_deref(), active_preview_key.get_value().as_deref(),