nevaforget
8285bcdf44
All checks were successful
Update PKGBUILD version / update-pkgver (push) Successful in 2s
- users::User: drop the unused `uid` field and its getuid() assignment. The compiler dead_code warning is gone, and the synthetic `u32::MAX` sentinel in the panel fallback is obsolete too. - panel: surface a log::warn! when dirs::home_dir() returns None instead of silently falling back to an empty PathBuf that would make avatars look for .face in the current working directory. - Apply three clippy suggestions: two collapsible if-let + && chains in users::get_avatar_path_with and config::resolve_background_path_with, and a redundant closure in panel::execute_action's spawn_blocking. - main: require MOONSET_DEBUG=1 to escalate log verbosity — mere presence of the var must not dump path info into the journal.
109 lines
11 KiB
Markdown
109 lines
11 KiB
Markdown
# Decisions
|
||
|
||
Architectural and design decisions for Moonset, in reverse chronological order.
|
||
|
||
## 2026-04-24 – Audit LOW fixes: dead uid field, home_dir warn, clippy sweep, debug value (v0.8.5)
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: Five LOW findings cleared in one pass. (1) `User::uid` was populated from `getuid()` but never read — a compiler `dead_code` warning for a field on the public API. (2) Falling back to a synthetic user when `get_current_user()` returned None used `uid: u32::MAX`, an undocumented sentinel that became moot once uid was removed. (3) `dirs::home_dir().unwrap_or_default()` silently yielded `PathBuf::new()` on failure; avatars would then look for `.face` in the current working directory. (4) `cargo clippy` flagged three suggestions (two collapsible `if`, one redundant closure) that had crept in. (5) `MOONSET_DEBUG` promoted log verbosity on mere presence, leaking path information into the journal.
|
||
- **Tradeoffs**: Dropping `uid` from `User` is a minor API break for any internal caller expecting the field — none existed. The synthetic fallback now surfaces `log::warn!` when home resolution fails, which should be rare outside of pathological sandbox environments.
|
||
- **How**: (1) Remove `pub uid: u32` from `User` and the `uid: uid.as_raw()` assignment in `get_current_user`. (2) Panel fallback drops the `uid` field entirely. (3) `dirs::home_dir().unwrap_or_else(|| { log::warn!(...); PathBuf::new() })`. (4) `cargo clippy --fix` for the two collapsible ifs, manual collapse of `if-let` + `&&` chain, redundant closure replaced with the function itself. (5) `MOONSET_DEBUG` now requires the literal value `"1"` to escalate to Debug.
|
||
|
||
## 2026-04-24 – Audit MEDIUM fixes: timeout guard, POSIX locale, button desensitize, wallpaper allowlist (v0.8.4)
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: Five MEDIUM findings: (1) `run_command`'s timeout thread leaked a 30 s gio::spawn_blocking slot if `child.wait()` errored, because `done.store(true)` ran after the `?`. (2) Timeout detection compared `status.signal() == Some(9)` — a hardcoded signal number that also misclassifies OOM-killer SIGKILL as our timeout. (3) `execute_action` never desensitized the button_box, so a double-click or accidental keyboard repeat fired the action twice. (4) `detect_locale` read only `LANG`, ignoring POSIX priority order (`LC_ALL` > `LC_MESSAGES` > `LANG`) — a common dual-language setup picked the wrong UI language. (5) The wallpaper path was passed to gdk-pixbuf without extension or size restriction, widening the image-parser attack surface and allowing unbounded decode latency.
|
||
- **Tradeoffs**: The extension allowlist (`jpg`, `jpeg`, `png`, `webp`) rejects exotic formats users might have used before. The 10 MB size cap rejects uncompressed/high-quality 4K wallpapers; acceptable for a power menu. Memory ordering on the `done` flag is now `Release`/`Acquire` instead of `Relaxed` — no runtime cost but correct across threads.
|
||
- **How**: (1) RAII `DoneGuard` struct sets `done.store(true, Release)` in its `Drop`, so the flag fires on every function exit path. A second `timed_out` AtomicBool distinguishes our SIGKILL from an external one. (2) Replace `Some(9)` with the `timed_out` flag check. (3) `execute_action` now takes `button_box: >k::Box`, calls `set_sensitive(false)` on entry and re-enables it on error paths; success paths that quit skip the re-enable. All call sites updated. (4) `detect_locale` reads `LC_ALL`, `LC_MESSAGES`, `LANG` in order, picking the first non-empty value before falling back to `/etc/locale.conf`. (5) `accept_wallpaper` helper applies extension allowlist + symlink rejection + `MAX_WALLPAPER_FILE_SIZE = 10 MB`, and is called for both config-path and Moonarch fallback.
|
||
|
||
## 2026-04-24 – Audit fix: avoid latent stdout pipe deadlock in run_command (v0.8.3)
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: Audit found that `run_command` piped the child's `stdout` but never drained it, then called blocking `child.wait()`. A child writing more than one OS pipe buffer (~64 KB on Linux) would block on `write()` while the parent blocked in `wait()` — classic pipe deadlock, broken only by the 30 s SIGKILL timeout. Current callers (`systemctl`, `niri msg`, `loginctl`) do not emit that much output, but the structure was fragile and would bite on any future command or changed behaviour.
|
||
- **Tradeoffs**: stdout is now fully discarded. If a future caller needs stdout, it will have to drain it concurrently with `wait()` (separate reader thread).
|
||
- **How**: Replace `.stdout(Stdio::piped())` with `.stdout(Stdio::null())` in `run_command`. `stderr` stays piped — it is drained after `wait()`, which is safe because `wait()` already reaped the child and no further writes can occur.
|
||
|
||
## 2026-03-31 – Fourth audit: release profile, GResource compression, lock stderr, sync markers
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: Fourth triple audit found missing release profile (LTO/strip), uncompressed GResource assets, moonlock stderr suppressed (errors invisible), and duplicated code without sync markers.
|
||
- **Tradeoffs**: moonlock stderr now inherited instead of null — errors appear in moonset's journal context. Acceptable for debugging, no security leak since moonlock logs to its own journal identifier.
|
||
- **How**: (1) `[profile.release]` with LTO, codegen-units=1, strip. (2) `compressed="true"` on GResource entries. (3) `Stdio::inherit()` for moonlock stderr in lock(). (4) SYNC comments on duplicated blur/background functions.
|
||
|
||
## 2026-03-28 – Remove wallpaper from GResource bundle
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: All three Moon projects (moonset, moongreet, moonlock) embedded a 374kB fallback wallpaper in the binary via GResource. Moonarch already installs `/usr/share/moonarch/wallpaper.jpg` at system setup time, making the embedded fallback unnecessary dead weight (~2MB in binary size).
|
||
- **Tradeoffs**: If `/usr/share/moonarch/wallpaper.jpg` is missing and no user config exists, moonset shows a solid CSS background instead of a wallpaper. Acceptable — the power menu is functional without a wallpaper image.
|
||
- **How**: Removed `wallpaper.jpg` from GResource XML and resources directory. `resolve_background_path` returns `Option<PathBuf>`. All wallpaper-related functions handle `None` gracefully. Binary size dropped from ~3.2MB to ~1.3MB.
|
||
|
||
## 2026-03-28 – Switch from env_logger to systemd-journal-logger
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: moonlock and moongreet already use systemd-journal-logger. moonset used env_logger which writes to stderr — not useful for a GUI app launched via keybind. Journal integration enables `journalctl -t moonset` and consistent troubleshooting across all three Moon projects.
|
||
- **Tradeoffs**: Requires systemd at runtime. Graceful fallback to eprintln if journal logger fails. Acceptable since Moonarch targets systemd-based Arch Linux.
|
||
- **How**: Replace `env_logger` dep with `systemd-journal-logger`, add `setup_logging()` with `MOONSET_DEBUG` env var for debug-level output. Same pattern as moonlock/moongreet.
|
||
|
||
## 2026-03-28 – Replace action name dispatch with `quit_after` field
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: Post-action behavior (quit the app or not) was controlled by comparing `action_name == "lock"` — a magic string duplicated from the action definition. Renaming an action would silently break the dispatch.
|
||
- **Tradeoffs**: Adds a field to `ActionDef` that most actions set to `false`. Acceptable because it makes the contract explicit and testable.
|
||
- **How**: `ActionDef.quit_after: bool` — `true` for lock and logout, `false` for hibernate/reboot/shutdown.
|
||
|
||
## 2026-03-28 – GPU blur via GskBlurNode replaces CPU blur
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: CPU-side Gaussian blur (`image` crate) blocked startup and added caching complexity. moonlock already migrated to GPU blur.
|
||
- **Tradeoffs**: GPU blur quality is slightly different (box-blur approximation vs true Gaussian), acceptable for wallpaper backgrounds. Removes `image` crate dependency entirely. No disk cache needed.
|
||
- **How**: `Snapshot::push_blur()` + `GskRenderer::render_texture()` on `connect_realize`. Blur happens once on the GPU when the widget gets its renderer. Symmetric with moonlock and moongreet.
|
||
|
||
## 2026-03-28 – Use absolute paths for system binaries
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: Security audit flagged PATH hijacking risk — relative binary names allow a malicious `$PATH` entry to intercept `systemctl`, `loginctl`, etc.
|
||
- **Tradeoffs**: Hardcoded paths reduce portability to non-Arch distros where binaries may live elsewhere (e.g. `/sbin/`). Acceptable because Moonarch targets Arch Linux exclusively.
|
||
- **How**: All five power action wrappers now use `/usr/bin/` prefixed paths.
|
||
|
||
## 2026-03-28 – Implement power action timeout via try_wait polling
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: `POWER_TIMEOUT` and `PowerError::Timeout` were declared but never wired up. A hanging `systemctl hibernate` (e.g. blocked NFS mount) would freeze the power menu indefinitely.
|
||
- **Tradeoffs**: Polling with `try_wait()` + 100ms sleep is slightly less efficient than a dedicated timeout crate, but avoids adding a dependency for a single use case.
|
||
- **How**: `run_command` now polls `child.try_wait()` against a 30s deadline, kills the child on timeout.
|
||
|
||
## 2026-03-28 – Centralize GRESOURCE_PREFIX
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: The string `/dev/moonarch/moonset` was duplicated in `config.rs`, `users.rs`, and as literal strings in `panel.rs` and `main.rs`. Changing the application ID would require edits in 4+ locations.
|
||
- **Tradeoffs**: Modules now depend on `crate::GRESOURCE_PREFIX` — tighter coupling to main.rs, but acceptable for an internal constant.
|
||
- **How**: Single `pub(crate) const GRESOURCE_PREFIX` in `main.rs`, referenced everywhere else.
|
||
|
||
## 2026-03-28 – Remove journal.md
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: One-time development notes from the Rust rewrite, never updated after initial session. Overlapped with memory system and git history.
|
||
- **Tradeoffs**: Historical context lost from the file, but the information is preserved in git history and the memory system.
|
||
- **How**: Deleted. Useful technical learnings migrated to persistent memory.
|
||
|
||
## 2026-03-27 – OVERLAY layer instead of TOP
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: Waybar occupies the TOP layer. The power menu must appear above it.
|
||
- **Tradeoffs**: OVERLAY is the highest layer — nothing can render above moonset while it's open. This is intentional for a session power menu.
|
||
- **How**: `setup_layer_shell` uses `gtk4_layer_shell::Layer::Overlay` for the panel window.
|
||
|
||
## 2026-03-27 – Lock without confirmation
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: Lock is immediately reversible (just unlock). All other actions (logout, hibernate, reboot, shutdown) are destructive or disruptive.
|
||
- **Tradeoffs**: One less click for the most common action. Risk of accidental lock is negligible since unlocking is trivial.
|
||
- **How**: `ActionDef.needs_confirm = false` for lock; all others require inline confirmation.
|
||
|
||
## 2026-03-27 – Niri-specific logout via `niri msg action quit`
|
||
|
||
- **Who**: ClaudeCode, Dom
|
||
- **Why**: Moonarch is built exclusively for the Niri compositor. Generic Wayland logout mechanisms don't exist — each compositor has its own.
|
||
- **Tradeoffs**: Hard dependency on Niri. If the compositor changes, `power::logout()` must be updated.
|
||
- **How**: `Command::new("/usr/bin/niri").args(["msg", "action", "quit"])`.
|