Round 6 (Grok verification): fix the live-path breakers.

Grok round-6 verification was No-Go. Fixes:

A. LayoutSlot.id was written as 0 by every constructor
   (generate Stacked/Grid/main_strip, capture_from, the GUI pad
   literals), so the find-by-id lookups in resize_slot / move_slot
   never hit a tile and Free-mode Apply size/position always
   failed with "no layout slot for slot id N". Constructors now
   assign real 1-based ids (index+1, out.len()+1, i+2 for the
   strip); capture_from uses the slot id from the window tuple.
   layout::apply and reset_slot_lock now find the tile by id
   instead of by Vec index.

B. The `slots` IPC formatter emitted "{id} 0x{address} ..." while
   addresses already carry their own 0x prefix ("0xa"), producing
   "1 0x0xa ..."; the parser split on whitespace so any multi-word
   window title broke the field alignment. Both sides now use a
   tab separator and the address passes through unchanged.

C. examples/profile.yaml still shipped the dropped schema
   (window_match block + passthrough list). Replaced with a note
   that matching is by process tree and every mapped hotkey is
   intercepted.

D. CHANGELOG 0.1.0 still advertised passthrough (lines 19, 24)
   and window_match (27, 78). Annotated as removed.

E. Lying comments: launcher.rs called the prefix "per-team" (it is
   per-character); gui.rs::arm_auto_apply doc claimed it matched a
   regex.

F. arm_auto_apply still hardcoded an empty spawned-pid set and
   fell back to matching any client with a non-empty class -- the
   round-4 Item-3 placeholder was what actually ran. It now takes
   the real child pid and matches via pid_is_ancestor.

G. spawn_plan dropped the Child with no wait thread (zombie, same
   bug round-6 fixed in launch_game). Now reaps in a background
   thread.

H. page_session never refreshed the games list, so the dropdown
   was empty on first paint. Added a games_loaded flag and a
   one-shot refresh_games on first paint.

Plus: vfx env-var test race. toplevel_enabled_defaults_off_... and
capture_toplevel_is_gated_when_disabled both touch
ENBOXER_ENABLE_TOPLEVEL and cargo runs unit tests in parallel, so
the gated test intermittently saw the var set by its sibling (the
per-function `static` in session.rs does not serialise across
functions). Added a module-level ENV_LOCK in vfx::tests and
guarded both tests. Verified with three consecutive full runs.

Also: clippy unnecessary_cast in main_strip, and two rustdoc
warnings (raw <pid> and Arc<GbmDevice> read as HTML tags).

cargo test 103/103 (x3); clippy --all-targets -D warnings clean;
cargo doc --no-deps clean.
This commit is contained in:
en 2026-09-17 08:24:03 +02:00
parent 677ae0d3dc
commit 863f7e584e
9 changed files with 95 additions and 70 deletions

View File

@ -16,15 +16,15 @@
- Rust CLI daemon: `enboxer run|press|status|macros|doctor`
- YAML profile: maps, targets (`current` / `others` / `all` / groups), `game_binds`
- Configurable `passthrough` (empty by default; example profile uses ESDF)
- Configurable `passthrough` (removed in round-4 Item 6)
- Hyprland 0.56 Lua binds installed only while a managed game or VFX overlay is focused
- Key delivery via `hl.dsp.send_shortcut` / `send_key_state` (no game injection)
- Stock loot map: assist → CTM on → Interact with Target once → delay → CTM off
- Video FX: `grim` region capture, overlay viewer, hover pass-through to source slot
- Unit tests for hotkey parse, passthrough, targets, loot sequence, example profile load
- Unit tests for hotkey parse, targets, loot sequence, example profile load
- Bind IPC line no longer duplicated `ipc` (Hyprland hotkeys actually reach the daemon)
- Video FX overlay is `mpv` (`--wayland-app-id=enboxer-vfx`) with JSON reload; `grim` capture
- `enboxer run` clears binds on ctrl-c; empty `window_match` matches nothing
- `enboxer run` clears binds on ctrl-c (round-4 Item 3 replaced window_match with process-tree matching)
- Live check: `send_shortcut` to an unfocused XWayland window; overlay window class `enboxer-vfx`
- Per-character `assist_key` / `follow_key`; map steps using `bind: assist|follow` resolve to the current main's keys
- Layout wizard `borderless` toggle installs a Hyprland `window_rule` (rounding 0, border_size 0) on `run`
@ -75,7 +75,7 @@
list previously saved a team with zero Launch rows; it now fills the
rows with `Character::default()` and applies the Lutris game to each.
- **GUI:** `arm_auto_apply` now waits for a client matching
`profile.window_match.class` / `.title` regexes before firing
the process tree rooted at a spawned PID before firing (round-4 Item 3)
`layout-apply`, instead of firing on the first non-empty client list.
With no patterns configured it still requires a non-empty class on the
matched client, so existing profiles behave the same. This prevents

View File

@ -5,17 +5,15 @@ name: team
client: wow-retail
slots: 2
# Hyprland window match (regex). Wine/WoW class is often wow.exe or the wine prefix name.
window_match:
class: "(?i)wow|warcraft"
title: null
# Windows are matched by process tree (round-4 Item 3): the daemon
# spawns the game and matches the window whose pid is in that
# process tree. No class/title regex anymore.
# Keys that are NEVER intercepted. Empty = no skip list.
# ESDF movement stays on the primary even if you later add a map for those letters.
passthrough: ["e", "s", "d", "f"]
# Every mapped hotkey is intercepted (round-4 Item 6 removed the
# `passthrough` skip-list).
# maps = only keys listed under `maps` (rest stay on the front window)
# mirror = clone keys to the other clients (passthrough still skipped)
# mirror = clone keys to the other clients
# off = no intercept; the front window gets everything
mode_default: maps

View File

@ -217,7 +217,7 @@ impl Drop for GbmDevice {
/// device's fd is not closed while the BO is still live. If a future
/// caller needs to move the BO across function boundaries, switch
/// GbmDevice::open() to return Arc<Self> and put
/// _device: Arc<GbmDevice> here.
/// `_device: Arc<GbmDevice>` here.
pub struct GbmBo {
#[allow(dead_code)]
handle: *mut c_void,

View File

@ -63,6 +63,7 @@ pub fn run() -> Result<()> {
selected_node: None,
game_list: Vec::new(),
game_pick: String::new(),
games_loaded: false,
};
eframe::run_native("enBoxer", native, Box::new(|_cc| Ok(Box::new(app))))
.map_err(|e| anyhow::anyhow!("{e}"))?;
@ -128,6 +129,8 @@ struct App {
game_list: Vec<String>,
/// Round-5 item 2: currently picked name in the dropdown.
game_pick: String,
/// Round-6 H: games list refreshed once on first Session paint.
games_loaded: bool,
}
#[derive(Default)]
@ -686,6 +689,12 @@ impl App {
ui.label("Profile name");
ui.text_edit_singleline(&mut self.profile.name);
});
// Round-6 H: refresh the games list the first time the
// page is shown so the dropdown is not empty on first paint.
if !self.games_loaded {
self.games_loaded = true;
self.refresh_games();
}
// Round-5 item 2: game-launcher dropdown. The daemon reads
// ~/.config/enboxer/games.yaml and exposes the names via
// the `list-games` IPC verb. Picking one fires
@ -1966,27 +1975,32 @@ impl App {
cmd.env(k, v);
}
match cmd.spawn() {
Ok(child) => {
Ok(mut child) => {
let pid = child.id();
self.status = format!("{} (pid {})", plan.summary, pid);
// Round-5 Item 4: push the PID into the daemon's
// Session.spawned_pids via the track-pid IPC verb
// so refresh_slots can match the resulting window
// via the process-tree walk.
// Push the PID into the daemon's Session.spawned_pids
// via the track-pid IPC verb so refresh_slots can
// match the resulting window via the process tree.
self.ipc("track-pid", &pid.to_string());
// Round-6 G: reap the child in a background thread.
// Dropping it left a zombie until the GUI exited.
std::thread::spawn(move || {
let _ = child.wait();
});
if ch.auto_apply {
self.status.push_str(" (auto-apply armed)");
self.arm_auto_apply(ch.slot);
self.arm_auto_apply(ch.slot, pid);
}
}
Err(e) => self.error = Some(format!("launch: {e}")),
}
}
/// Poll hyprctl for the matched window and call the existing
/// layout-apply IPC. This is the only path that moves windows
/// automatically after a Launch; everything else is gated.
fn arm_auto_apply(&mut self, _slot: u32) {
/// Poll hyprctl for the window whose pid is in the spawned
/// process tree (`spawned_pid`) and call the layout-apply IPC.
/// This is the only path that moves windows automatically after
/// a Launch; everything else is gated.
fn arm_auto_apply(&mut self, _slot: u32, spawned_pid: u32) {
// Spawn a background thread that polls hyprctl for up to 30 s
// and then sends the layout-apply IPC. Detached; logs on error.
let profile_path = self.path.clone();
@ -2002,31 +2016,15 @@ impl App {
use std::process::Command as SyncCommand;
let start = std::time::Instant::now();
let deadline = std::time::Duration::from_secs(30);
// Item 3: process-tree match replaces regex. A client
// matches if (a) its pid is in any spawned tree root,
// OR (b) no launched PIDs are tracked yet (operator
// is testing interactively without launching) and the
// client has a non-empty class. The full wiring
// (spawned pids from the GUI's launch flow) lands in
// Item 4 (process-tree window discovery + game
// launcher dropdown).
let spawned: std::collections::HashSet<u32> = {
// Read Session.spawned_pids through the IPC
// socket: ask the daemon for its current
// tracked pids. For now (Item 3 placeholder),
// the daemon does not yet store them across the
// IPC boundary; the loop here just matches every
// non-empty client so existing layouts apply.
std::collections::HashSet::new()
};
let matched = |cls: &str, _ttl: &str, pid: i64| -> bool {
if spawned.is_empty() {
return !cls.is_empty();
}
// Round-6 F: process-tree match against the pid we
// actually just spawned. The old code hardcoded an
// empty set and fell back to matching any client with
// a non-empty class, which meant auto-apply fired on
// whatever was visible rather than the window it
// launched.
let matched = |_cls: &str, _ttl: &str, pid: i64| -> bool {
let pid_u = pid.max(0) as u32;
spawned.iter().any(|&root| {
crate::process::pid_is_ancestor(root, pid_u)
})
crate::process::pid_is_ancestor(spawned_pid, pid_u)
};
while start.elapsed() < deadline {
std::thread::sleep(std::time::Duration::from_millis(1000));

View File

@ -1,7 +1,7 @@
//! Spawn-plan builder for the Teams launch flow.
//!
//! Given a [`LutrisGame`] (or its relevant subset) and an optional
//! per-team `wine_prefix` override, returns a [`SpawnPlan`] that the
//! per-character `wine_prefix` override, returns a [`SpawnPlan`] that the
//! GUI can hand to `tokio::process::Command`. We do **not** shell out
//! to the `lutris` CLI; we reproduce the env-var contract directly so the
//! operator sees exactly which variables the launcher sets.

View File

@ -51,9 +51,10 @@ pub fn generate(layout: &Layout, n: u32, mons: &[Monitor]) -> Vec<LayoutSlot> {
LayoutPreset::Stacked => {
let (w, h) = constrain(m.width, m.height);
(0..n)
.map(|_| LayoutSlot {
.enumerate()
.map(|(i, _)| LayoutSlot {
x: m.x,
id: 0,
id: (i as u32) + 1,
y: m.y,
w,
h,
@ -96,7 +97,7 @@ fn grid(m: &Monitor, n: u32, pin: bool) -> Vec<LayoutSlot> {
}
out.push(LayoutSlot {
x: m.x + x as i32 * ww,
id: 0,
id: out.len() as u32 + 1,
y: m.y + y as i32 * wh,
w: ww,
h: wh,
@ -132,7 +133,7 @@ fn main_strip(m: &Monitor, n: u32, layout: &Layout) -> Vec<LayoutSlot> {
};
out.push(LayoutSlot {
x: m.x,
id: 0,
id: 1,
y: main_y,
w: bw,
h: bh,
@ -157,7 +158,7 @@ fn main_strip(m: &Monitor, n: u32, layout: &Layout) -> Vec<LayoutSlot> {
};
out.push(LayoutSlot {
x,
id: 0,
id: i + 2,
y,
w: sw.max(1),
h: sh.max(1),
@ -174,7 +175,7 @@ fn main_strip(m: &Monitor, n: u32, layout: &Layout) -> Vec<LayoutSlot> {
// refresh_slots). Callers must now read the slot list from the
// daemon over IPC so the GUI's view matches what refresh_slots
// produced. This helper shells out to `enboxer ipc slots` and
// parses the JSON response.
// parses the tab-separated response.
pub fn select_windows(_profile: &Profile, _clients: Vec<Client>) -> Vec<(u32, Client)> {
crate::layout::slots_from_daemon().unwrap_or_default()
}
@ -191,15 +192,15 @@ pub fn slots_from_daemon() -> anyhow::Result<Vec<(u32, Client)>> {
return Ok(Vec::new());
}
let text = String::from_utf8_lossy(&out.stdout);
// The daemon returns one slot per line:
// <id> <address> <pid> <class> <title>
// We only need (id, Client) for select_windows.
// Round-6 B: the daemon emits tab-separated fields:
// <id>\t<address>\t<pid>\t<class>\t<title>
// (space-splitting broke any title with a space in it).
let mut out = Vec::new();
for line in text.lines() {
let mut it = line.split_whitespace();
let mut it = line.split('\t');
if let (Some(id), Some(addr), Some(pid)) = (it.next(), it.next(), it.next()) {
let class = it.next().unwrap_or("").to_string();
let title = it.collect::<Vec<_>>().join(" ");
let title = it.next().unwrap_or("").to_string();
if let (Ok(id), Ok(pid)) = (id.parse::<u32>(), pid.parse::<i32>()) {
out.push((id, Client {
address: addr.to_string(),
@ -246,7 +247,9 @@ pub async fn apply(slots: &[LayoutSlot], windows: &[(u32, Client)]) -> Result<()
}
let mons = monitors().await.unwrap_or_default();
for (i, win) in windows {
let Some(geom0) = slots.get((*i as usize).saturating_sub(1)) else {
// Round-6: find the tile by its stable id, not by a
// Vec index. Session slot ids can have gaps after a hide.
let Some(geom0) = slots.iter().find(|s| s.id == *i) else {
continue;
};
let mut geom = geom0.clone();
@ -279,10 +282,10 @@ pub async fn apply(slots: &[LayoutSlot], windows: &[(u32, Client)]) -> Result<()
pub fn capture_from(windows: &[(u32, Client)]) -> Vec<LayoutSlot> {
let mut out = Vec::new();
for (_, c) in windows {
for (id, c) in windows {
out.push(LayoutSlot {
x: c.at[0],
id: 0,
id: *id,
y: c.at[1],
w: c.size[0],
h: c.size[1],

View File

@ -34,7 +34,7 @@ pub fn pid_is_ancestor(ancestor: u32, candidate: u32) -> bool {
false
}
/// Read /proc/<pid>/status and return the PPid field as a u32.
/// Read `/proc/<pid>/status` and return the `PPid` field as a `u32`.
pub fn parent_pid(pid: u32) -> Option<u32> {
let path = Path::new("/proc").join(pid.to_string()).join("status");
let s = std::fs::read_to_string(&path).ok()?;

View File

@ -576,11 +576,15 @@ async fn dispatch_cmd(session: &Arc<Mutex<Session>>, line: &str) -> String {
}
"slots" => {
let g = session.lock().await;
// Round-6 B: tab-separated so multi-word titles do
// not break the parse. The address already carries
// its `0x` prefix from Hyprland, so pass it through
// unchanged (the old code emitted `0x0xa`).
g.slots
.iter()
.map(|(id, c)| {
format!(
"{} 0x{} {} {} {}",
"{}\t{}\t{}\t{}\t{}",
id, c.address, c.pid, c.class, c.title
)
})
@ -1073,13 +1077,19 @@ pub async fn move_slot(session: &Arc<Mutex<Session>>, slot: u32) -> Result<()> {
/// Item 1: clear size_locked on slot-N so the next refresh
/// re-applies the initial size (operator wants the default back).
pub async fn reset_slot_lock(session: &Arc<Mutex<Session>>, slot: u32) -> Result<()> {
let slot_idx = slot.saturating_sub(1) as usize;
let mut g = session.lock().await;
let slot_count = g.engine.profile.layout.slots.len();
if slot_idx >= slot_count {
anyhow::bail!("slot {slot} out of range (have {slot_count})");
}
g.engine.profile.layout.slots[slot_idx].size_locked = false;
// Round-6 A7: find by stable id, not by Vec index.
let Some(ls) = g
.engine
.profile
.layout
.slots
.iter_mut()
.find(|s| s.id == slot)
else {
anyhow::bail!("no layout slot for slot id {slot}");
};
ls.size_locked = false;
Ok(())
}

View File

@ -379,6 +379,15 @@ pub async fn capture_loop(rx: watch::Receiver<Vec<FeedHit>>, hub: Arc<OverlayHub
mod tests {
use super::*;
/// Serialises the two tests that read/write
/// ENBOXER_ENABLE_TOPLEVEL. cargo test runs unit tests in
/// parallel; without a shared lock the sibling test can set
/// the var while the gated test runs, so the gate appears to
/// be off and the error message differs. A module-level
/// static (not a per-function one) is what actually
/// serialises them.
static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
fn feed(viewer: (i32, i32, i32, i32), source: (i32, i32, i32, i32)) -> FeedHit {
FeedHit {
name: "test".into(),
@ -458,6 +467,7 @@ mod tests {
#[test]
fn toplevel_enabled_defaults_off_and_respects_env() {
let _guard = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner());
std::env::remove_var("ENBOXER_ENABLE_TOPLEVEL");
assert!(!toplevel_enabled());
std::env::set_var("ENBOXER_ENABLE_TOPLEVEL", "1");
@ -470,7 +480,13 @@ mod tests {
}
#[tokio::test]
// Holding the std lock across the await is deliberate: the
// whole point is to keep the sibling env test from mutating
// ENBOXER_ENABLE_TOPLEVEL while this one asserts the gate.
// Test-only, single-threaded-ish work, no deadlock risk.
#[allow(clippy::await_holding_lock)]
async fn capture_toplevel_is_gated_when_disabled() {
let _guard = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner());
std::env::remove_var("ENBOXER_ENABLE_TOPLEVEL");
let w = win("0xa", (0, 0), (800, 600));
let dest = std::env::temp_dir().join("enboxer_test_toplevel_gated.png");