fix(docking): retry the undock; a lost keyword left the panel black
Some checks failed
Check / eval (push) Has been cancelled
Some checks failed
Check / eval (push) Has been cancelled
Round 6 passed on the plug side but unplugging the cable left the laptop
panel black until it was re-docked. The journal named it: across three
undocks, two logged result=enable-timeout and one result=ok — a race, not
a logic error.
Two halves, both needed:
- `hyprctl keyword monitor eDP-1,preferred,auto,1` is accepted (exit 0)
while Hyprland is still tearing down the departing external, then
silently dropped. Re-issue it on every poll for 5s and keep `monitors`
listing the panel — not hyprctl's exit code — as the only proof.
- The watcher fired the undock once off the removal event and marked the
departure handled regardless (`|| true` + awaiting_lid_open=1). Since
known_outputs had already moved, the set-changed guard never reopened
and nothing retried, so one lost keyword cost the panel until re-dock.
The undock is now a queued invariant driven on the 1s tick, bounded at
6 attempts so a genuinely unenableable panel can't pin the inhibitor.
The VM harness cannot reach this race: QEMU's headless backend aborts
Hyprland if the last active output is deleted while the DRM output is
disabled, so the test must undock before removing DP-1 and never collides
the two. Noted there, and V3 queued as round 7 — unplug 5+ times, since
the old failure only hit ~2 in 3.
Verified: V2 — nix flake check --no-build; monitor-fallback.nix exit 0 in
57s; shellcheck clean. The fix itself is V3 pending.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -181,16 +181,19 @@ let
|
||||
valid_output "$internal" || { echo "invalid monitor name" >&2; exit 64; }
|
||||
# Safety-critical ordering: never remove/move away from an external
|
||||
# before the internal panel is active and can receive workspaces.
|
||||
hyprctl keyword monitor "$internal,preferred,auto,1" >/dev/null 2>&1 \
|
||||
|| { log "transition=undock internal=$internal result=enable-failed"; exit 1; }
|
||||
# The enable races the departing external's teardown: a keyword that
|
||||
# lands mid-modeset is accepted (hyprctl exits 0) and then quietly
|
||||
# dropped, so re-issue it every poll instead of trusting one shot.
|
||||
# `monitors` listing the panel — not hyprctl's exit code — is proof.
|
||||
ready=
|
||||
tries=0
|
||||
while [ "$tries" -lt 20 ]; do
|
||||
while [ "$tries" -lt 25 ]; do
|
||||
hyprctl keyword monitor "$internal,preferred,auto,1" >/dev/null 2>&1
|
||||
if hyprctl monitors -j 2>/dev/null \
|
||||
| jq -e --arg m "$internal" 'any(.[]; .name == $m)' >/dev/null; then
|
||||
ready=1; break
|
||||
fi
|
||||
tries=$((tries+1)); sleep 0.1
|
||||
tries=$((tries+1)); sleep 0.2
|
||||
done
|
||||
[ -n "$ready" ] \
|
||||
|| { log "transition=undock internal=$internal result=enable-timeout"; exit 1; }
|
||||
@@ -324,6 +327,8 @@ ${lib.concatStringsSep "\n" (lib.mapAttrsToList
|
||||
inhibitor_pid=
|
||||
inhibitor_file="$rt/nomarchy-dock-lid-inhibitor.pid"
|
||||
awaiting_lid_open=
|
||||
undock_pending=
|
||||
undock_tries=0
|
||||
log() { "$LOGGER" -t nomarchy-display-watch -- "$*"; }
|
||||
internal_output() {
|
||||
if [ -n "''${NOMARCHY_INTERNAL_OUTPUT:-}" ]; then
|
||||
@@ -513,6 +518,9 @@ ${lib.concatStringsSep "\n" (lib.mapAttrsToList
|
||||
known_outputs=$cur
|
||||
|
||||
if [ -n "$added_ext" ]; then
|
||||
# A new external supersedes any undock still queued for an older
|
||||
# departure: the panel is about to be disabled again on purpose.
|
||||
undock_pending=
|
||||
[ -z "$internal" ] || acquire_inhibitor
|
||||
# An external output that was not there a moment ago is an
|
||||
# unambiguous physical plug whether or not we saw the IPC event,
|
||||
@@ -522,13 +530,16 @@ ${lib.concatStringsSep "\n" (lib.mapAttrsToList
|
||||
paint_outputs &
|
||||
log "outputs-changed added=$added_ext audio-reprobe=scheduled"
|
||||
fi
|
||||
# awaiting_lid_open marks an undock already performed for this
|
||||
# awaiting_lid_open marks an undock already completed for this
|
||||
# departure — the guard is what keeps event + tick from doubling up.
|
||||
# The attempt itself is deliberately NOT made here: it is queued and
|
||||
# driven by the invariant below, because the first try races the
|
||||
# external's teardown and losing it must not cost the panel.
|
||||
if [ -n "$gone" ] && [ -z "$ext" ] && [ -n "$internal" ] \
|
||||
&& [ -z "$awaiting_lid_open" ]; then
|
||||
"$TRANSITION" undock "$internal" || true
|
||||
awaiting_lid_open=1
|
||||
log "outputs-changed removed=$gone transition=undock"
|
||||
undock_pending=1
|
||||
undock_tries=0
|
||||
log "outputs-changed removed=$gone transition=undock-queued"
|
||||
fi
|
||||
check_monitors
|
||||
if [ -n "$added_ext" ] \
|
||||
@@ -538,6 +549,28 @@ ${lib.concatStringsSep "\n" (lib.mapAttrsToList
|
||||
fi
|
||||
|
||||
# Safety invariants, re-checked regardless of whether the set moved.
|
||||
#
|
||||
# A dark panel is the worst outcome this watcher can produce, so the
|
||||
# undock is an invariant driven to completion on the tick — not a
|
||||
# one-shot fired off the removal event. On hardware the first attempt
|
||||
# loses the race with the external's teardown often enough that
|
||||
# treating it as final left the laptop black until it was re-docked.
|
||||
if [ -n "$undock_pending" ] && [ -z "$ext" ] && [ -n "$internal" ]; then
|
||||
if "$TRANSITION" undock "$internal"; then
|
||||
undock_pending=
|
||||
awaiting_lid_open=1
|
||||
log "transition=undock result=ok attempts=$((undock_tries+1))"
|
||||
else
|
||||
undock_tries=$((undock_tries+1))
|
||||
# Bounded: a panel that cannot be enabled at all is a different
|
||||
# fault, and retrying forever would pin the lid inhibitor with it.
|
||||
if [ "$undock_tries" -ge 6 ]; then
|
||||
undock_pending=
|
||||
awaiting_lid_open=1
|
||||
log "transition=undock result=gave-up attempts=$undock_tries"
|
||||
fi
|
||||
fi
|
||||
fi
|
||||
if [ -n "$internal" ] && [ -n "$ext" ]; then
|
||||
if [ -z "$inhibitor_pid" ] || ! kill -0 "$inhibitor_pid" 2>/dev/null; then
|
||||
acquire_inhibitor || true
|
||||
|
||||
Reference in New Issue
Block a user