Four silent failures the attention review found after #126 #128

Merged

lz merged 2 commits from fix/attention-followups into main

2026-09-05 15:22:15 +02:00

lz commented

2026-09-05 13:59:48 +02:00

Owner

5 files, +100/−22.

title was passed to a lucide icon, which lands it on the <svg>. SVG has no title attribute — a tooltip there needs a <title> child. And lucide's hasA11yProp counts the key:

if (prop.startsWith('aria-') || prop === 'role' || prop === 'title') return true;

…so passing it also dropped the aria-hidden="true" the icon would otherwise carry, leaving an unnamed node in the accessibility tree. The attribute is present in the DOM and reads back fine; it just does nothing. Moving the tooltip to an HTML wrapper fixes both.

2 · `working` and `idle` collapsed into each other under reduced motion

The two were separated by motion — a rotating ring against a still one. With the animation gone they were both green rings differing only in opacity, which is not a distinction. At rest working now fills its centre and keeps a static halo: a different shape, not a brighter one.

prefers-reduced-motion is a setting real operators run, and this is exactly the failure my own mock warned about when picking this idiom.

3 · the guard beside it was reading the wrong CSS

Writing the test for #2 exposed it: app.css has more than one prefers-reduced-motion block now — starting brought its own — so slicing from the first match read the wrong rules and then found the strings it wanted further down the file anyway. It passed for the wrong reason. Both tests now walk braces to the block that actually governs these marks.

The blocked count includes the workspace shell; running and the denominator beside it exclude it. 2 running of 2 · 3 waiting on you was reachable. It gets its own row.

Also

The CSS parity loop covered only the four attention states, while every value of the mark is a class name. SESSION_MARKS is now the list and SessionMark derives from it, so running / starting / stopped / errored are pinned too — deleting any one of their rules now fails.

Verification

Three mutations, three correct failures: removing the reduced-motion fill, deleting that block outright, and dropping a liveness rule from app.css each fail exactly the test that covers them.

Browser-verified: the tooltip sits on the wrapper with aria-hidden="true" restored on the icon, working computes filled-green against idle's transparent ring under emulated reduced motion, and the card renders sessions 3 running of 3 above a separate waiting 2 on you.

1496 tests, tsc 0 errors, eslint clean.

Not taken

An exited workspace shell is told to "restart it from the ⋯ menu", but the shell's menu offers only Stop / Start shell agent, gated on the row existing rather than on the agent being up — and SessionEnd does not delete the row. That is a pre-existing gap this feature's wording is merely the first thing to point at, so it wants its own issue rather than a patch here.

Follow-ups to #126. The adversarial pass over that PR's comments kept reporting after it merged, and four of its findings were real. All four fail silently — nothing errors, nothing looks wrong in the DOM. 5 files, +100/−22. ## 1 · the workspace shell's tooltip never worked `title` was passed to a lucide icon, which lands it on the `<svg>`. **SVG has no `title` attribute** — a tooltip there needs a `<title>` *child*. And lucide's `hasA11yProp` counts the *key*: ```js if (prop.startsWith('aria-') || prop === 'role' || prop === 'title') return true; ``` …so passing it also **dropped the `aria-hidden="true"`** the icon would otherwise carry, leaving an unnamed node in the accessibility tree. The attribute is present in the DOM and reads back fine; it just does nothing. Moving the tooltip to an HTML wrapper fixes both. ## 2 · `working` and `idle` collapsed into each other under reduced motion The two were separated by **motion** — a rotating ring against a still one. With the animation gone they were both green rings differing only in opacity, which is not a distinction. At rest `working` now fills its centre and keeps a static halo: a different *shape*, not a brighter one. `prefers-reduced-motion` is a setting real operators run, and this is exactly the failure my own mock warned about when picking this idiom. ## 3 · the guard beside it was reading the wrong CSS Writing the test for #2 exposed it: app.css has **more than one** `prefers-reduced-motion` block now — `starting` brought its own — so slicing from the first match read the wrong rules and then found the strings it wanted further down the file anyway. It passed for the wrong reason. Both tests now walk braces to the block that actually governs these marks. ## 4 · the hover card printed one line from two populations The blocked count includes the workspace shell; `running` and the denominator beside it exclude it. `2 running of 2 · 3 waiting on you` was reachable. It gets its own row. ## Also The CSS parity loop covered only the four attention states, while **every** value of the mark is a class name. `SESSION_MARKS` is now the list and `SessionMark` derives from it, so `running` / `starting` / `stopped` / `errored` are pinned too — deleting any one of their rules now fails. ## Verification Three mutations, three correct failures: removing the reduced-motion fill, deleting that block outright, and dropping a liveness rule from app.css each fail exactly the test that covers them. Browser-verified: the tooltip sits on the wrapper with `aria-hidden="true"` restored on the icon, `working` computes filled-green against `idle`'s transparent ring under emulated reduced motion, and the card renders `sessions 3 running of 3` above a separate `waiting 2 on you`. 1496 tests, `tsc` 0 errors, eslint clean. ## Not taken An exited workspace **shell** is told to *"restart it from the ⋯ menu"*, but the shell's menu offers only Stop / Start shell agent, gated on the row *existing* rather than on the agent being up — and `SessionEnd` does not delete the row. That is a pre-existing gap this feature's wording is merely the first thing to point at, so it wants its own issue rather than a patch here.

lz added 1 commit

2026-09-05 13:59:49 +02:00

fix(ui): four follow-ups the attention review found after #126 merged

ci / images (pull_request) Has been cancelled

Details

ci / nexus (pull_request) Has been cancelled

Details

bfd29465bd

An adversarial pass over the comments in #126 kept going after it landed. Four
things it turned up, all silent failures.

The workspace shell's tooltip never worked. `title` was passed to a lucide icon,
which puts it on the `<svg>` — but SVG has no `title` attribute; a tooltip there
needs a `<title>` CHILD. Worse, lucide's `hasA11yProp` counts the KEY, so
passing it also dropped the `aria-hidden="true"` the icon would otherwise carry,
leaving an unnamed node in the accessibility tree. Both failures are invisible:
the attribute is present in the DOM and reads back fine, it just does nothing.
The tooltip moves to an HTML wrapper, which restores aria-hidden too.

`working` and `idle` collapsed into each other under prefers-reduced-motion. The
two were separated by MOTION — a rotating ring against a still one — so with the
animation gone they were both green rings differing only in opacity, which is
not a distinction. At rest `working` now fills its centre and keeps a static
halo: a different shape rather than a brighter one. The existing guard could not
see this, because it only asserted the selectors appear.

Writing that guard exposed a flaw in the guard beside it. app.css has more than
one prefers-reduced-motion block now — `starting` brought its own — so slicing
from the first match read the wrong rules and then found the strings it wanted
further down the file anyway. Both tests now walk braces to the block that
actually governs these marks.

The rail hover card printed one line from two populations: the blocked count
includes the workspace shell, while `running` and the denominator beside it
exclude it, so `2 running of 2 · 3 waiting on you` was reachable. It gets its
own row.

Also: the CSS parity loop covered only the four attention states, while every
value of the mark is a class name. `SESSION_MARKS` is now the list and the type
derives from it, so `running` / `starting` / `stopped` / `errored` are pinned
too — deleting any one of their rules now fails.

Guards proven by mutation as usual: removing the reduced-motion fill, deleting
the block outright, and dropping a liveness rule from app.css each fail exactly
the test that covers them. Browser-verified: the tooltip now sits on the wrapper
with aria-hidden restored on the icon, working reads filled-green against idle's
transparent ring under reduced motion, and the card renders `sessions 3 of 3`
above a separate `waiting 2 on you`.

Not taken, and worth an issue rather than a patch: an exited workspace SHELL is
told to "restart it from the ⋯ menu", but the shell's menu offers only Stop /
Start shell agent, gated on the row existing rather than on the agent being up —
and SessionEnd does not delete the row. That is a pre-existing gap this feature's
wording is simply the first thing to point at.

lz added 1 commit

2026-09-05 15:06:13 +02:00

refactor(ui): say the same four things in fewer lines

ci / nexus (pull_request) Successful in 9m44s

Details

pr-image-cleanup / delete-pr-images (pull_request) Successful in 7s

Details

ci / images (pull_request) Successful in 21m36s

Details

370b0b8b2b

Same fixes, ~40% less diff. Each is also a better version of what it replaces
rather than a shorter one.

The shell's tooltip no longer needs a wrapper element: an SVG `<title>` CHILD is
the tooltip mechanism SVG actually has, so the glyph carries it directly. That
is strictly better than the wrapper, not merely smaller — lucide sets
aria-hidden only when it has no children, so the `<title>` both provides the
tooltip and supplies the accessible name that its absence then requires. An icon
with a name should not be hidden from the a11y tree.

The reduced-motion halo was an eight-line `::after` that also forced
`position: relative` onto the base rule. `box-shadow` draws the same dot, gap
and ring in one property, outside layout, and the base rule goes back to what it
was.

The test's brace-walking parser is gone. It existed to find the reduced-motion
copy of a rule by WHERE it sits, which needs a parser because @media nests.
Telling it apart by what it DOES needs none: the reduced copy is the one that
switches the animation off. Two small helpers now serve both tests and read
better than the twenty lines they replace.

One guard got tighter rather than shorter, because proving it still bit showed
it did not. "The selector appears in app.css" is not "the mark is painted": a
state whose only surviving rule is its reduced-motion copy renders as an
invisible circle, and deleting `.status-dot.starting`'s real rule passed. It now
requires a rule that sets a background or a border and is not the at-rest copy —
and that mutation fails.

Re-verified in the browser: the tooltip is a `<title>` child with no inert
attribute left behind, the icon is correctly no longer aria-hidden, `working` at
rest is filled green with the box-shadow halo against idle's transparent ring,
and it still spins when motion is allowed.

lz merged commit 9708004f5d into main

2026-09-05 15:22:15 +02:00

lz deleted branch fix/attention-followups

2026-09-05 15:22:16 +02:00

lz referenced this pull request from a commit

2026-09-05 15:22:20 +02:00

Merge pull request 'Four silent failures the attention review found after #126' (#128) from fix/attention-followups into main