task 53: all ten fixed and pushed; unverified on glass
This commit is contained in:
parent
f28274730f
commit
8ac3267cc1
2 changed files with 193 additions and 157 deletions
|
|
@ -1,166 +1,202 @@
|
|||
# TASK 53 — what an audit of the 2026-08-03 compositor work left open
|
||||
|
||||
**Status: open.** Created 2026-08-04 from an adversarial read of
|
||||
`souveraine-viewtop ba4bd03..c98f4c8` and `souveraine 266af98..914e48f`.
|
||||
**Repo:** `souveraine-viewtop` unless noted.
|
||||
**Status: the ten findings are fixed and pushed** (`souveraine-viewtop ba82168`,
|
||||
2026-08-04). **Not yet run on hardware** — the binary is staged on the phone at
|
||||
`/usr/local/bin/viewtop.ba82168` and lands on an install + greetd restart.
|
||||
Created 2026-08-04 from an adversarial read of `ba4bd03..c98f4c8` and
|
||||
`souveraine 266af98..914e48f`. **Repo:** `souveraine-viewtop` unless noted.
|
||||
|
||||
Six findings from that audit are fixed and pushed (the strip not moving, agent
|
||||
touch reaching the lock surface, the stale settle stranding the strip, the
|
||||
Six findings from that audit were fixed the same day (the strip not moving,
|
||||
agent touch reaching the lock surface, the stale settle stranding the strip, the
|
||||
shared wake latch, workspace-blind focus, the missing workspace verb, per-window
|
||||
capture returning nothing). These are the rest, ranked by what bites first.
|
||||
capture returning nothing). These were the rest.
|
||||
|
||||
The audit's own method note is worth keeping: **every one of these passed all
|
||||
four gates.** A wedged compositor reports `shell=active` with zero errors, so
|
||||
"green" proves nothing about behaviour. Launch an app and read the log.
|
||||
The audit's own method note is the thing to keep: **every one of these passed
|
||||
all four gates.** A wedged compositor reports `shell=active` with zero errors,
|
||||
so "green" proves nothing about behaviour. The fixes below passed four gates and
|
||||
264 tests too. Launch an app and read the log.
|
||||
|
||||
## 1. Two hit tests that disagree
|
||||
## 1. Two hit tests that disagree — FIXED
|
||||
|
||||
`toggle_float` and `drag_window` use `self.space.element_under(point)`.
|
||||
`toggle_float` and `drag_window` used `self.space.element_under(point)`;
|
||||
`client_surface_under` undoes the strip shift *and* the pose. So a three-finger
|
||||
tap or carry picks a different window than a touch does, whenever anything is
|
||||
posed or the strip is off an integer.
|
||||
carry picked a different window than a touch did, whenever anything was posed or
|
||||
the strip was off an integer — §11's second-decider shape in miniature, getting
|
||||
louder as `pose` gets used.
|
||||
|
||||
That is the second-decider shape §11 exists to prevent, in miniature, and it
|
||||
gets louder as `pose` gets used — which is exactly what the overview cards and
|
||||
per-window capture are for. One hit test, used by both.
|
||||
Now `windows_under()` is the one hit test: it walks the space topmost-first,
|
||||
applies `window_origin` (the strip) and `unpose` (the pose), and yields every
|
||||
window under the point with the point in that window's own coordinates. An
|
||||
iterator rather than one answer, because the two callers want different amounts
|
||||
of it — the touch path walks past a window whose input region misses, the carry
|
||||
takes the first.
|
||||
|
||||
## 2. Agent contacts are unbounded and never cleaned up
|
||||
`window_origin` is also what `frame_elements` reads, so the picture and the
|
||||
touch cannot drift.
|
||||
|
||||
`input.rs` `Router::down` inserts into `active` with no cap, and the control
|
||||
socket is line-per-connection so there is no client lifetime to hook. A caller
|
||||
that sends `touch_down` on a thousand slots and disconnects leaves a thousand
|
||||
contacts down forever — and a thousand unterminated `wl_touch` sequences in
|
||||
whatever client received them, which from the client's side is a button stuck
|
||||
pressed. Slow-growth memory in the one process whose death takes the session.
|
||||
## 2. Agent contacts are unbounded and never cleaned up — FIXED
|
||||
|
||||
`TouchCancel` exists and has to be called deliberately. Options: cap the slot
|
||||
namespace, or tie contacts to the control connection that opened them.
|
||||
`SLOTS_PER_ORIGIN = 64`. `Origin::slot()` is fallible now and refuses anything
|
||||
past it, so `Router::active` is bounded by construction rather than by a client
|
||||
lifetime the control socket does not have (it is a line per connection). The
|
||||
refusal surfaces at `serve_intent` as `invalid_argument` naming the bound.
|
||||
|
||||
Also: `Origin::Agent.slot()` is `saturating_add(1 << 16)`, so `slot: u32::MAX`
|
||||
stays `u32::MAX` and reaches `wl_touch`'s `int` id as `-1`, the no-slot
|
||||
sentinel.
|
||||
That also kills the sharper half: `Origin::Agent.slot()` was
|
||||
`saturating_add(1 << 16)`, so `u32::MAX` stayed `u32::MAX` and reached
|
||||
`wl_touch`'s `int` id as `-1` — the no-slot sentinel. Wrapping instead of
|
||||
saturating would have been worse: her slot 64 would alias slot 0 and retarget a
|
||||
contact somebody else holds.
|
||||
|
||||
## 3. `ext_foreign_toplevel_list_v1` has no bind filter, and its handles are empty
|
||||
Three tests pin it, including that the cap stays smaller than the gap between
|
||||
the two namespaces.
|
||||
|
||||
It is the one global added alongside `may_bind` that did not get one, while
|
||||
TASK-51 §5 already names the wlr list as *"admits every client, and window
|
||||
titles cross while the session is locked"*. This is a second unfiltered window
|
||||
list, mitigated only accidentally — by the titles never being set.
|
||||
## 3. `ext_foreign_toplevel_list_v1` had no bind filter, and its handles were empty — FIXED
|
||||
|
||||
Which is the second half: the handles are minted with `String::new()` and never
|
||||
updated. `publish_toplevel` carries title and app_id to the **wlr** state only.
|
||||
A shell drawing an overview card gets the pixels from the standard handle and
|
||||
would have to join two protocols by hand for the label.
|
||||
Both halves, and they had to land together: filling the titles is what would
|
||||
have made the missing filter matter, since the list was mitigated only
|
||||
accidentally — by the titles never being set.
|
||||
|
||||
Note the earlier attempt at this (`0eda51e`) was **reverted**: calling
|
||||
`send_title`/`send_app_id`/`send_done` from `publish_toplevel` fires on every
|
||||
focus change and produced a protocol event storm. Whatever fixes it must be
|
||||
edge-triggered on the strings actually changing.
|
||||
- **`Privileged::WindowList`**, applied to *both* window lists: the standard one
|
||||
through smithay's `new_with_filter`, the wlr one through `GlobalDispatch::
|
||||
can_view` with the seat uid carried in the global's data (`ListGate`), because
|
||||
`can_view` is an associated function with no `self` and no closure to capture.
|
||||
Same ceiling as the other four — any process of the session's user still binds
|
||||
— and TASK-41's grant is what narrows it.
|
||||
- **Titles and app_ids on the standard handles**, edge-triggered. Note the
|
||||
earlier attempt (`0eda51e`) was reverted for a protocol event storm:
|
||||
`publish_toplevel` runs on every focus change, and while smithay's
|
||||
`send_title`/`send_app_id` drop an unchanged value themselves, `send_done`
|
||||
does not — and `done` is what a client relayouts on. So the change is detected
|
||||
first and `done` is sent only when something moved.
|
||||
|
||||
## 4. Errors that are indistinguishable from nothing happening
|
||||
**Still owed and NOT this task's:** titles cross while the session is locked.
|
||||
That is a disclosure gate, asked at a different moment than a bind filter, and
|
||||
it is TASK-51 §5's.
|
||||
|
||||
`DEVICE-STATE-MACHINE.md` §10's rule, applied to tonight's code. Each of these
|
||||
returns the same thing for "we refused", "it is not there", and "our
|
||||
bookkeeping is broken":
|
||||
## 4. Errors that are indistinguishable from nothing happening — FIXED
|
||||
|
||||
- `capture_window_for` collapses four distinct failures into
|
||||
`CaptureFailureReason::Unknown`, two of which are compositor bugs, none of
|
||||
which log.
|
||||
- `copy_window_into` has five silent `return Err(Unknown)` exits.
|
||||
- `drag_window` has three silent returns, and because it fails *without*
|
||||
latching `self.dragging`, a drag that started over the wallpaper can flip to
|
||||
strip mode on a later event — contradicting the "axis decided once and held"
|
||||
comment directly above it.
|
||||
- `fullscreen_request` silently skips the workspace logic if `id_for` or
|
||||
`workspace_of` answers `None`.
|
||||
- `toplevel_destroyed`: if `id_for` returns `None` the `on_workspace` entry is
|
||||
never removed, so that workspace can never look empty again and the strip
|
||||
keeps a phantom room forever.
|
||||
- `border` echoes back what was asked with no read-back anywhere — `state` and
|
||||
`workspaces` do not carry it — so a chain cannot confirm what is in force
|
||||
after a restart. It is also the one composition verb with no `Act::Compose`
|
||||
check.
|
||||
`DEVICE-STATE-MACHINE.md` §10's rule. The protocol's failure vocabulary is only
|
||||
three words wide (`Unknown`, `BufferConstraints`, `Stopped`), so what the client
|
||||
is told cannot always be specific — but what the log says can be.
|
||||
|
||||
## 5. `focus_border` ignores `pose`, and its fallback is the whole panel
|
||||
- `capture_window_for`: a window that closed is `Stopped`, which is what it is.
|
||||
The other two are compositor bugs and warn with what did not match.
|
||||
- `copy_window_into` / `copy_output_into`: every exit logs, at the severity it
|
||||
deserves — no renderer and no output are `error!`, a window that has not
|
||||
committed a buffer yet is `debug!` and not a fault.
|
||||
- `drag_window`: three logged returns, and the axis latch (§7) is what stops a
|
||||
failed pick from letting the same gesture flip to strip mode later.
|
||||
- `fullscreen_request`: refuses out loud, and only where it can (§6).
|
||||
- `toplevel_destroyed`: see §5 and §10 below.
|
||||
- `border`: asks `Act::Compose` like every other composition verb, and `state`
|
||||
reports `borders` so a chain can read back what is in force.
|
||||
|
||||
The border is drawn at raw `loc` and raw `placed_size` while the surface is
|
||||
drawn at `pose_origin` with `scale * pose.scale`. Pose a window to 0.5 — the
|
||||
exact thing per-window capture exists for — and the half-size window sits inside
|
||||
a full-size slab. Borders default on.
|
||||
## 5. `focus_border` ignored `pose`, and its fallback was the whole panel — FIXED
|
||||
|
||||
Worse, the no-registry fallback is `self.registry.output()`, the whole panel.
|
||||
`toplevel_destroyed` calls `registry.remove` and then `retile` → `kick_render`
|
||||
→ `frame_elements` **before** `space.refresh()`, so every window close paints
|
||||
one frame with a full-zone grey block. The same fallback in
|
||||
`client_surface_under` makes that dead window a full-screen hit-test box for
|
||||
the same frame.
|
||||
The border now follows the posed rectangle — the same arithmetic `pose_origin`
|
||||
does, kept in logical pixels because the zone it clamps against is logical.
|
||||
|
||||
## 6. `fullscreen_request` arranges the scene on a client's say-so
|
||||
The fallback was the worse half. `registry.output()` is not a conservative
|
||||
default, it is the largest possible wrong answer, and `toplevel_destroyed`
|
||||
reached it every time: `registry.remove` then `retile` → `kick_render` →
|
||||
`frame_elements`, all before `space.refresh()` had dropped the element. One
|
||||
frame of full-zone grey slab, and a full-screen hit box on a dead window in the
|
||||
same frame. Now `window_extent()` falls back to the window's own geometry, and
|
||||
`toplevel_destroyed` unmaps the element first so "destroyed" is true for every
|
||||
reader at once.
|
||||
|
||||
`move_request`, `resize_request` and `show_window_menu` all ask
|
||||
`client_may_arrange` → *"clients do not arrange the scene."*
|
||||
`fullscreen_request` mints a workspace, reassigns membership, moves the view and
|
||||
retiles, with the gate never asked — including while locked. And it disagrees
|
||||
with itself: `Ask::Fullscreen { on: true }` from the dock goes straight to
|
||||
`configure_filled` and mints nothing, so two paths to the same user action
|
||||
behave differently.
|
||||
## 6. `fullscreen_request` arranged the scene on a client's say-so — FIXED, and the gate question was the wrong one
|
||||
|
||||
## 7. Touch-cancel and a fourth finger leave the drag latched
|
||||
Both doors go through `take_fullscreen_room` / `give_fullscreen_room_back` now.
|
||||
The disagreement the audit found was real and is closed: `Ask::Fullscreen` from
|
||||
the dock minted no workspace, so whether your video got its own room depended on
|
||||
which thing you tapped.
|
||||
|
||||
`touch_cancel` clears the recogniser and the router and **not** `strip_drag`,
|
||||
`dragging`, `strip_samples`, or `Strip::drag_from`. A cancelled three-finger
|
||||
drag leaves the strip mid-drag with nothing in flight, so `on_vblank` never
|
||||
walks it home and the panel sits showing two half-windows until the next
|
||||
gesture happens to end with a `DragEnd`.
|
||||
**It does not ask `client_may_arrange`, and that is deliberate.** That predicate
|
||||
answers "may a client dictate its own geometry" — what `move_request` and
|
||||
`resize_request` are — and it always says no. Fullscreen is not that: it is a
|
||||
state the protocol defines and the compositor implements however it likes, and
|
||||
refusing it would leave the client waiting on a configure that never comes. What
|
||||
*was* genuinely missing is the lock: minting a room moves the view, which
|
||||
changes what the operator finds when they authenticate, and a client does not
|
||||
get to decide that from behind a lock surface. So `disclosure_locked()` refuses
|
||||
it and says so.
|
||||
|
||||
Same latch leaks on a palm: `peak_fingers` is max-concurrent, so a fourth
|
||||
contact makes every later `Drag`/`DragEnd` carry `fingers: 4`, matching neither
|
||||
arm. `LOCK-DPMS-LESSONS.md` §3 documents the FTS controller cancelling streams
|
||||
after a wake, so this is not hypothetical.
|
||||
## 7. Touch-cancel and a fourth finger left the drag latched — FIXED
|
||||
|
||||
## 8. Doc comments that now assert the opposite of the code
|
||||
`abandon_drag()` clears `dragging`, `drag_axis`, `strip_drag` and
|
||||
`strip_samples`, and **releases** the strip at zero velocity rather than
|
||||
snapping it — a cancel means the hand is gone, and the spring should take it
|
||||
from where the hand left it. `touch_cancel` calls it: the recogniser forgetting
|
||||
its contacts never moved the strip it was dragging, and those are two different
|
||||
pieces of state.
|
||||
|
||||
Three were displaced by insertions (the mechanism that ate a `#[cfg]` twice):
|
||||
`copy_output_into`'s doc is on `session_is_toplevel` (with a duplicated
|
||||
`#[cfg]`), `frame_elements`' doc is on `focus_border`, `clock_now`'s is on
|
||||
`touch_time`. Every `cfg` happens to be correct; the duplication is the
|
||||
fingerprint.
|
||||
The palm case is closed by matching `fingers >= 3` on `Drag` and any count on
|
||||
`DragEnd`. `peak_fingers` is max-concurrent, so a fourth contact renamed every
|
||||
later event and neither arm matched — the strip stayed latched half-way with
|
||||
nothing in flight to walk it home. A drag still only *begins* at exactly three.
|
||||
|
||||
Four say false things: *"The registry carries size, not position"* sits directly
|
||||
above the line that writes `at`; *"the hit test owes the inverse mapping"* was
|
||||
written in the same commit that landed `unpose`; *"`publish_toplevel` is what
|
||||
carries them"* is untrue of the standard handles (see §3);
|
||||
`WindowOverview.qml`'s header says *"viewtop has no workspaces at all"*, written
|
||||
the same night as `workspace.rs`.
|
||||
And the axis is latched on **both** outcomes. It used to be inferred from
|
||||
whether `strip_drag` was set, so a vertical drag that began over the wallpaper
|
||||
latched nothing and a later event could flip it into scrolling the world. A
|
||||
latch set on only one of two outcomes is not a latch. `drag_axis_of()` is a free
|
||||
function with four tests; ties go to the window, as the more recoverable answer.
|
||||
|
||||
`INTERFACE-ARCHITECTURE.md` calls a documented guarantee the code does not
|
||||
enforce **the one recurring defect** in this project. These are its inverse and
|
||||
they cost the same.
|
||||
## 8. Doc comments that asserted the opposite of the code — FIXED
|
||||
|
||||
## 9. A floated window cannot rejoin the layout
|
||||
The three displaced by insertions (`copy_output_into`'s doc on
|
||||
`session_is_toplevel`, `frame_elements`' on `focus_border`, and the duplicated
|
||||
`#[cfg]` in `Viewtop::new`) are back where they belong. The four that said false
|
||||
things — the registry carrying "size, not position", the hit test "owing" the
|
||||
inverse mapping, `publish_toplevel` not carrying the standard handles' strings
|
||||
(now it does), and `foreign_toplevel.rs`'s claim that the standard list is
|
||||
deliberately not served — now describe the code.
|
||||
|
||||
`place()` pins a window out of the tiling permanently — deliberately, because
|
||||
geometry is hers and a layout that took it straight back would make the verb a
|
||||
no-op. So a three-finger drag floats a window and there is now **no way to
|
||||
un-float it**.
|
||||
`WindowOverview.qml`'s header (souveraine) said *"viewtop has no workspaces at
|
||||
all"*, written the same night `workspace.rs` landed. Corrected, including what
|
||||
is now true and undrawn: the strip is readable over the control socket and this
|
||||
overview does not show it.
|
||||
|
||||
`toggle_float` used to be that, bound to the three-finger tap. That binding
|
||||
moved to sessiond (`Action::Overview`) when the gesture became a verb, which is
|
||||
right — the compositor must not be both recogniser and binder — and it left the
|
||||
un-float with no caller, so it was deleted rather than kept as dead code.
|
||||
## 9. A floated window could not rejoin the layout — FIXED
|
||||
|
||||
What it wants is a scene intent, beside `place`: "let the layout have this
|
||||
back." One line of state (`placed.remove(&id)`) plus a `retile`. It belongs in
|
||||
`wire`'s `ToCompositor` so the agent can reach it too, since §13 makes geometry
|
||||
hers and this is the inverse of a verb she already has.
|
||||
`unplace` on the wire, in the verb table, and in `serve_intent`. `place()` still
|
||||
pins a window out of the tiling permanently and on purpose — dragging a window
|
||||
somewhere is saying where it should be — but the one-way door has a return now.
|
||||
It answers `was_floating`, because idempotent success and a window that was
|
||||
never floated are the same call and different facts.
|
||||
|
||||
## 10. Render churn
|
||||
It belongs on the wire rather than back on the gesture: it is the inverse of a
|
||||
verb she already has (§13 makes geometry hers), and the binding that used to
|
||||
carry `toggle_float` is sessiond's now.
|
||||
|
||||
`retile()` calls `retile_workspace` per visible workspace and each ends with
|
||||
`kick_render()`, so a strip drag fires two or three renders per motion event.
|
||||
## 10. Render churn — FIXED
|
||||
|
||||
`kick_render` moved out of `retile_workspace` and into `retile`, once. A strip
|
||||
drag lays out two or three workspaces per motion event and each used to queue
|
||||
its own frame.
|
||||
|
||||
Also here rather than in a section of its own: `prune_workspaces()` derives
|
||||
membership from the surface table instead of maintaining it beside one. A
|
||||
destroy that could not resolve an id used to leave the `on_workspace` entry
|
||||
behind, and an entry behind is a room that can **never** look empty again —
|
||||
one in the strip forever, holding nothing, costing a swipe every time you cross
|
||||
it.
|
||||
|
||||
## Acceptance
|
||||
|
||||
Not met yet. It is one thing: **launch an app and read the log**, then
|
||||
|
||||
- open two windows, three-finger tap (overview reaches sessiond), three-finger
|
||||
carry one (it floats), `unplace` it (it tiles again);
|
||||
- pose a window to 0.5 and check the border is around the *window*;
|
||||
- close a window and watch for a grey frame that should no longer exist;
|
||||
- fullscreen from the app and from the dock and get the same workspace;
|
||||
- put a palm down mid-drag and confirm the strip still settles.
|
||||
|
||||
## Connects to
|
||||
|
||||
TASK-51 (viewtop → daily driver), TASK-50 (done; §1 and §5 here are its
|
||||
consequences), TASK-41 (§3's bind filter), `DEVICE-STATE-MACHINE.md` §10 (§4
|
||||
here is that rule), `SESSION-AUTHORITY-DOCTRINE.md` §11 (§1 and §6).
|
||||
TASK-51 (viewtop → daily driver; §5's disclosure gate on the window list is the
|
||||
half deliberately left there), TASK-50 (done; §1 and §5 here were its
|
||||
consequences), TASK-41 (§3's bind filter is uid-only until the attested grant),
|
||||
`DEVICE-STATE-MACHINE.md` §10 (§4 here is that rule),
|
||||
`SESSION-AUTHORITY-DOCTRINE.md` §11 (§1 and §6).
|
||||
|
|
|
|||
|
|
@ -29,7 +29,7 @@ Four that are cheap relative to what they unblock:
|
|||
|
||||
| # | Task | What's left |
|
||||
|---|------|-------------|
|
||||
| 53 | [viewtop audit: what is still open](53-viewtop-audit-open.md) | Nine findings from an adversarial read of the 2026-08-03 work. Two hit tests that disagree, unbounded agent contacts, an unfiltered window list, and a list of errors that look like nothing happening. **All of it passed four gates** — that is the lesson. |
|
||||
| 53 | [viewtop audit: what it left open](53-viewtop-audit-open.md) | All ten findings fixed and pushed (`ba82168`). **Unverified on glass** — the binary is staged on the phone and lands on an install + greetd restart. The acceptance list is in the task. The lesson stands: all of it passed four gates before *and* after. |
|
||||
| 43 | [viewtop: the Souveraine compositor](43-viewtop-compositor.md) | Runs on the phone. Next lane is the ext-session-lock admission gate. The two-client lock handoff is the thing not to get wrong. |
|
||||
| 29 | [SouveraineOS Updater](29-souveraine-updater.md) | Ships and installs by `pacman -Syu`. **Read-only on device** until something runs a polkit agent — that is the shell's job. Also owns TASK-42's layer 2. |
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue