Displays needs reliable safeguards and automatic revert for risky changes

Breadway commented

2026-10-04 11:03:09 +08:00

Owner

Report and evidence

Direct user report during acceptance testing on 2026-10-04: Displays needs safeguards against breaking the layout and revert timers. Local screenshot: /tmp/codex-clipboard-ZbePn6.png.

Confidence: high that this is an unmet acceptance requirement; runtime coverage of the existing countdown has not been independently reproduced. BSET-27/BSET-38 previously tracked a keep-or-revert countdown and are marked done, so investigate this as a regression or incomplete coverage rather than assuming no timer code exists.

Expected outcome

Cover resolution, scale, rotation, position, mirroring and other potentially disruptive display changes with a safe temporary apply flow. Capture the complete live layout, including mirror relationships, before applying. Show a visible Keep/Revert confirmation and countdown; automatically restore the previous working layout on timeout. Explicit Revert must restore it immediately. Persist changes only after confirmation.

Handle apply failures, monitor disconnects and leaving/closing the settings view while confirmation is pending. Recovery must not depend solely on a frontend timer that disappears with the view. Validate input and avoid leaving the user without a usable display. Surface actionable errors and confirm actual compositor state after apply and restore.

Verify successful keep, explicit revert, unattended timeout and failed changes, including mirrored outputs. Related to BSET-43 and BSET-27/BSET-38.

Tracking only; the user explicitly requested no implementation.

## Report and evidence Direct user report during acceptance testing on 2026-10-04: Displays needs safeguards against breaking the layout and revert timers. Local screenshot: /tmp/codex-clipboard-ZbePn6.png. Confidence: high that this is an unmet acceptance requirement; runtime coverage of the existing countdown has not been independently reproduced. BSET-27/BSET-38 previously tracked a keep-or-revert countdown and are marked done, so investigate this as a regression or incomplete coverage rather than assuming no timer code exists. ## Expected outcome Cover resolution, scale, rotation, position, mirroring and other potentially disruptive display changes with a safe temporary apply flow. Capture the complete live layout, including mirror relationships, before applying. Show a visible Keep/Revert confirmation and countdown; automatically restore the previous working layout on timeout. Explicit Revert must restore it immediately. Persist changes only after confirmation. Handle apply failures, monitor disconnects and leaving/closing the settings view while confirmation is pending. Recovery must not depend solely on a frontend timer that disappears with the view. Validate input and avoid leaving the user without a usable display. Surface actionable errors and confirm actual compositor state after apply and restore. Verify successful keep, explicit revert, unattended timeout and failed changes, including mirrored outputs. Related to BSET-43 and BSET-27/BSET-38. Tracking only; the user explicitly requested no implementation.

Breadway commented

2026-10-13 20:16:33 +08:00

Author

Owner

Live check of PR #67 (8c4b839) in a headless BOS VM (Hyprland, one Virtual-1 output), 2026-10-13:

Scale 125% and rotation changes show the Keep/Revert countdown. Revert restores, Keep keeps and writes monitors.json, the deadline reverts with no choice. All pass.
Close the window mid-countdown (app stays alive in the background): deadline still reverts. Pass. On main 98be903 the same case left the screen rotated, so this is a real fix.
SIGTERM (kill -TERM) mid-countdown: the app exits and the screen STAYS rotated (transform=1). Fail. The PR says app exit restores the snapshot, but the RunEvent::Exit hook does not run on SIGTERM. That covers logout, systemd stop and pkill, not just SIGKILL (BSET-67). Suggest a SIGTERM/SIGINT handler (tokio::signal or signal-hook) that runs the same revert before exiting.

Live check of PR #67 (8c4b839) in a headless BOS VM (Hyprland, one Virtual-1 output), 2026-10-13: - Scale 125% and rotation changes show the Keep/Revert countdown. Revert restores, Keep keeps and writes monitors.json, the deadline reverts with no choice. All pass. - Close the window mid-countdown (app stays alive in the background): deadline still reverts. Pass. On main 98be903 the same case left the screen rotated, so this is a real fix. - SIGTERM (kill -TERM) mid-countdown: the app exits and the screen STAYS rotated (transform=1). Fail. The PR says app exit restores the snapshot, but the RunEvent::Exit hook does not run on SIGTERM. That covers logout, systemd stop and pkill, not just SIGKILL (BSET-67). Suggest a SIGTERM/SIGINT handler (tokio::signal or signal-hook) that runs the same revert before exiting.

Breadway commented

2026-10-14 08:48:29 +08:00

Author

Owner

Review of PR #67 (#67), exact head 7a55f598d6, base 14d13158 (unchanged). Verdict: PASS, not merged (reviewer has no Forgejo write or merge tools; awaiting merge by the coordinator).

Checks I ran independently in a detached scratch worktree at that SHA: frontend npm ci, npm run check (0 errors, 0 warnings), npm test, npm run build all pass. src: cargo fmt --check clean, cargo clippy --locked --all-targets -D warnings clean, cargo test --locked 168 passed. Forgejo CI "check / check (push)" is success on the exact head (run 141). PR is mergeable, no reviews or change requests exist.

BSET-62: hyprland.rs dedupe_keep_order in parse_monitors plus uniqueBy in Display.svelte; test duplicate_modes_are_removed_keeping_order asserts 25 unique for both DVI outputs from the fixture. Every keyed each in Display.svelte and Breadpaper.svelte is over static lists or names that are now unique. Pass.
BSET-49: resolve_mirror maps the numeric mirrorOf id to a name (fixture: DVI-I-1 mirrors eDP-1); groupMonitors draws the group; withMirror and the guard tests show unrelated changes keep the mirror and restore replays it. Pass.
BSET-48: display_guard.rs snapshots, validates, applies, reads back, reverts on failure, timeout, unplug, explicit Revert and exit; only keep persists. Validation refuses no-enabled-display, self-mirror and missing, off or mirroring sources. Frontend routes resolution, scale, rotation, vrr, drag and mirror through apply_display_layout. 28a6bfa revert_on_signal (SIGTERM/SIGINT/SIGHUP) is wired in lib.rs setup and is idempotent with the RunEvent::Exit hook. Pass, backed by mock-compositor tests plus the coordinator's live VM evidence (not reproduced by me).

Residual, out of scope: SIGKILL/crash (BSET-67), login include order (BSET-68).

Review of PR #67 (https://git.breadway.dev/Breadway/bos-settings/pulls/67), exact head 7a55f598d6c99792fc3d80521f77866e5e0490d6, base 14d13158 (unchanged). Verdict: PASS, not merged (reviewer has no Forgejo write or merge tools; awaiting merge by the coordinator). Checks I ran independently in a detached scratch worktree at that SHA: frontend npm ci, npm run check (0 errors, 0 warnings), npm test, npm run build all pass. src: cargo fmt --check clean, cargo clippy --locked --all-targets -D warnings clean, cargo test --locked 168 passed. Forgejo CI "check / check (push)" is success on the exact head (run 141). PR is mergeable, no reviews or change requests exist. BSET-62: hyprland.rs dedupe_keep_order in parse_monitors plus uniqueBy in Display.svelte; test duplicate_modes_are_removed_keeping_order asserts 25 unique for both DVI outputs from the fixture. Every keyed each in Display.svelte and Breadpaper.svelte is over static lists or names that are now unique. Pass. BSET-49: resolve_mirror maps the numeric mirrorOf id to a name (fixture: DVI-I-1 mirrors eDP-1); groupMonitors draws the group; withMirror and the guard tests show unrelated changes keep the mirror and restore replays it. Pass. BSET-48: display_guard.rs snapshots, validates, applies, reads back, reverts on failure, timeout, unplug, explicit Revert and exit; only keep persists. Validation refuses no-enabled-display, self-mirror and missing, off or mirroring sources. Frontend routes resolution, scale, rotation, vrr, drag and mirror through apply_display_layout. 28a6bfa revert_on_signal (SIGTERM/SIGINT/SIGHUP) is wired in lib.rs setup and is idempotent with the RunEvent::Exit hook. Pass, backed by mock-compositor tests plus the coordinator's live VM evidence (not reproduced by me). Residual, out of scope: SIGKILL/crash (BSET-67), login include order (BSET-68).

Breadway closed this issue

2026-10-14 11:58:13 +08:00

Breadway commented

2026-10-14 12:05:31 +08:00

Author

Owner

Verified on integrated main a7b3e8b (contains merge 3407152 of PR #67): svelte-check 0/0, npm test, build, clippy -D warnings, cargo test 179 passed incl. display_guard mock-compositor tests. Live VM evidence on the PR build: Keep/Revert countdown, Revert, Keep persists, deadline revert, window-close revert, SIGTERM revert (transform 1 -> 0). SIGKILL gap tracked in BSET-67.

Verified on integrated main a7b3e8b (contains merge 3407152 of PR #67): svelte-check 0/0, npm test, build, clippy -D warnings, cargo test 179 passed incl. display_guard mock-compositor tests. Live VM evidence on the PR build: Keep/Revert countdown, Revert, Keep persists, deadline revert, window-close revert, SIGTERM revert (transform 1 -> 0). SIGKILL gap tracked in BSET-67.

Rows
Columns

Displays needs reliable safeguards and automatic revert for risky changes #49

Report and evidence

Expected outcome