Automations: fix command matching, and four editor/menu UX gaps - #74
Merged
Conversation
…nd chain A rule targeting `command contains "claude"` armed nothing, on every tick. `foreground_command_line` walked from the terminal's shell to the deepest youngest descendant and read only THAT process's command line. Measured on a real machine: every live `claude.exe` has 2-3 children, so the chain is `pwsh -> claude.exe -> bun.exe -> conhost.exe` and the string tested against "claude" was conhost's. Deterministic, not flaky — the agent is on the chain, just never at the end of it, because agents spawn helpers. The same defect hit every needle naming a program that spawns children: `node`, `npm`, `codex`. The tell was next door — `get_foreground_agent_with_exe` runs the identical descent but tests every level, which is why per-agent colour schemes could name Claude while targeting could not. One descent now, expressed once: - `foreground_chain_from` is the pure core, taking `(pid, parent)` pairs so the walk is testable without a live process table — the precedent `has_live_children` already sets in this file. Depth-capped and cycle-guarded, because targeting walks it inside the mutex `ProcSnapshot` holds. - `get_foreground_process_info` and `get_foreground_agent_with_exe` are both re-expressed on it. The latter was a third copy of the walk and the only one left without either guard, on a path that also runs behind `/api/processes`. - `RosterRow.command_line: Option<String>` becomes `command_lines: Vec<String>` rather than a joined string, so the field's name and its doc stay true. Reading the shell link is necessary — an idle `pwsh` matched `pwsh` before and must still — but it hands the matcher a string the APP wrote: both spawn paths append `-NoExit -Command <PS_CWD_INTEGRATION>`, ~300 characters containing `CurrentDirectory`, `FileSystem`, `[Environment]`, `prompt`, `try`, `catch`. Left in, ~30 ordinary needles (`dir`, `path`, `env`, `command`) would have selected every PowerShell terminal in the app, and a rule with a send action would have typed into all of them. `command_line_for` drops exactly that injected sequence — the script compared against the constant, the two flags removed only as the tokens immediately preceding it, so a profile's own `-Command` survives whole. Why nothing caught the original: the descent loop was never exercised against a real parent/child relationship (the one live test ran against the test binary, which its own comment notes has no children), every roster fixture hard-coded `command_line: None`, and `CommandContains` had no integration coverage at all.
…text menu Both menu surfaces rendered nothing when no rule was armed, and the code said why: the section "is not an action on this terminal that is temporarily unavailable, it is a list of rules that do not exist." That was true, and it stopped being true the moment the section gained something to do on an unarmed terminal. Those paragraphs are rewritten in place, not annotated — including in the two callers, which still described the old behaviour. Both hosts now offer, armed or not: - New automation for this terminal — opens the editor on a `blankDraft()` already pinned to it. `automationEditorHost` grew a second request shape for this: there is no id for `GlobalAutomationEditor` to resolve, so the menu hands over the whole draft. Same single-editor refusal as the id path, and the draft lands `enabled: false` like every other new rule. - Add to an existing automation — only `targetMode: 'pinned'` rules, because a criterion rule ignores `targetIds` and offering it would be an item that visibly does nothing. Rules already watching this terminal are skipped for the same reason. `armedMenuLabel(0)` is `'Automations'`, uncounted — one shared function, so the accordion and the flyout parent cannot be renamed apart. Two things the write path got wrong first time: - A rejected `saveAutomation` was swallowed by `void addTerminalToRule(…)`: the user clicked, the menu closed, and the terminal was simply never watched. It toasts now. - It wrote the whole rule as captured when the menu was ASSEMBLED. Since `save_rule_as_of` is an unconditional upsert with no version token, a rule deleted in another window came back from the dead. The id is re-resolved against the live rules at click time and bails audibly if it is gone — which is exactly why `openAutomationEditorFor` resolves ids rather than holding rule objects. The accordion also read `getAutomationRules()` bare, under a comment claiming it re-rendered on every store change. It does not: `useArmedAutomations` returns a stable `EMPTY` for an unarmed terminal, so `emit()` changes no identity — and unarmed is precisely the case this whole change serves. It subscribes now.
…the toggle Three gaps in the rule editor, all about telling the user something the screen was withholding. **The Watch Output card showed one clipped line.** `.au-nval` was `white-space: nowrap` with an ellipsis inside a card fixed at 244x160, so `command contains "claude" · 3 now` became `command contain…`. It clamps at two lines now and `AU_NODE_H` grows to 180 to fit. Every reader of that constant is derived (`AuNode`, `portAnchor`, canvas fit bounds, palette drag) and saved layouts store x/y only, so nothing persisted moves. **The terminal picker was unidentifiable.** Rows carried no `title` at all while id, name and folder each ellipsed, so two terminals in similar folders looked identical — and `shell` and `pid` were fetched and shown nowhere. Rows now carry full text, and hovering opens a card with every field the roster has plus ~6 lines of that terminal's CURRENT screen, read through `/snapshot` (never `/output`, which replays a lossy ring through a fresh parser and is not what the user is looking at). Only the hovered row is fetched; a closed terminal says so rather than showing an empty box that reads as a broken fetch. The card portals out with its own cache, because `CanvasMode` evicts the shared singleton every frame it is open. **A saved template did nothing, silently.** Templates land disabled on purpose, `save()` can only ever move `enabled` toward off, and the toggle refuses while the draft is dirty — so picking a template, editing and saving left the user with no cue that a step remained. Per Tam, the safety property stays and the cue is added: after a save whose result is disabled and has no blocking problems, the Enable toggle pulses briefly. Never when it saved enabled, and never when it saved disabled BECAUSE it is blocked — pointing at a control that will refuse you is worse than silence. The hover card's guards earn their tests: the one that matters is not a leaked timer (three `cancelled` checks already close that) but a slow row resolving after the pointer has moved on, painting the previous terminal's screen under the next terminal's name — the exact mistake a card for telling terminals apart must not make. `AU_NODE_H` is pinned by deriving the required height from the stylesheet's own box terms rather than restating 180, which would have written the same magic number twice and pinned nothing.
…a preview that reads the grid Five findings from the external review of this branch. **Resurrection was still reachable (blocker).** "Add to an existing automation" sent the whole rule object back through `save_automation`, an unconditional upsert whose `None` arm INSERTs — so a rule deleted in another window came back the moment a stale row was clicked. The first fix re-resolved the rule id against the renderer's cached list and claimed in a comment that this closed resurrection outright. It does not: `automation:changed` starts a refresh nobody awaits, and even a fresh cache leaves the delete free to commit between the read and the write. A read that DECIDES a write has to happen on that write's own transaction. So it does. `AutomationStore::add_target_to_rule` evaluates existence and appends in one SQLite transaction and answers `Ok(false)` — nothing written, no rule row and no orphan target row — when the rule is gone; `add_automation_target` is its command, and the menu sends ids instead of a rule. The clobber goes with it: a payload that carries no rule fields has nothing to overwrite a concurrent edit WITH. The false comment is rewritten where it stood. **The hover card's preview.** It held the screen without the id it was read from, so a row change painted terminal A's screen under terminal B's name for one commit — the exact mis-attribution the card exists to prevent, on the one gesture that produces it every time. Screen and process id are now one value, discarded in render rather than corrected by an effect. Its anchor refreshed only on the row's own `mousemove`, and the comment there argued a scroll/resize listener was unnecessary. A wheel over a stationary pointer fires no `mousemove`, and neither does a resize, so both of the two ways a row moves on its own were the uncovered ones. And it stripped escapes from `/snapshot` — a replay stream whose cursor sequences CARRY the column layout, so stripping collapses it. This repo already documented that, in `screen_text`'s own comment and in a passing test, one file away. New `GET /api/terminals/:id/screen` serves the authoritative grid render; nothing is left to strip. **A test that asserted both callbacks fired but never their order**, under a name promising "closes the menu first". `onDismiss` now records whether the editor had opened yet. Every new guard mutation-checked: the listener, the derivation, the failure stamp, the `false` answer, and the dismiss order all fail when reverted. `Canvas/NodeSnapshot.tsx` has the escape-stripping defect too — pre-existing, out of scope here, recorded in plan 031. typecheck clean · jest 3591/209 · cargo 767
…, and a hover that outlived its row The remediation workflow's own mutation pass, run against the guards `79c4b0b` shipped, killed two of them: - `previewLines` strips ANSI again **survived**. Against `/screen`'s plain text `stripAnsi` is a no-op, so a test built on plain fixtures cannot see the mutant at all. The test now feeds it a real replay blob, where stripping is a loss. - Removing the `cur.el !== hoveredEl` staleness guard **survived**. The two re-anchor tests pin that a scroll re-anchors, never that a scroll belonging to the row the pointer just left is ignored. Both were mutants I considered and skipped as weak. They were the two that were hollow. And a defect neither the review nor I had: **a hover outlives its row.** Every other exit from a hover is a pointer gesture, and the roster poll dropping a closed terminal is not one — no `mouseleave` comes for an element React has unmounted. The scroll listener stayed armed against a detached node, reading a rect of zeroes; `evictPreviewsOutside` is gated on `hovered === null`, so the preview cache stopped being evicted for as long as the phantom hover stood; and `hovered.id` resolves against the current roster every render, so a terminal returning under the same id — session restore's exact behaviour — re-opened its card at an anchor captured before it left. The `false` branch of the target add now returns rather than re-indexing: nothing was committed, so there is nothing new to read, and the delete behind it is already emitting the `automation:changed` that refetches this window's list. typecheck clean · jest 3594/209 · cargo 767 (unchanged; renderer-only)
…trusting an ordering argument that measurement refutes
Adversarial verification of the last round returned four open verdicts. None of
the tests were vacuous; all four holes were real.
**The resurrection class survived in Settings.** "Forget this terminal" and the
verbose switch still read a rule from a possibly-stale list, mutated it, and
wrote the whole object back through the unconditional upsert — the mirror of the
gesture already fixed, and reachable from a logging toggle.
`remove_target_from_rule` and `set_verbose_until` join `add_target_to_rule`:
decide and write on one transaction, report a rule that is gone. Verbose is a
logging gate the engine never reads back, so its command deliberately does not
`reload`, and a test asserts it must not. All four source-derived censuses were
extended in this same change.
**The menu-ordering fix rested on a false premise, and I reverted it.** The
claim was that dismissing before a row's action stops a modal mounting under a
live menu. Probing the real component — counting live document listeners from
inside a row's `onSelect` — shows the menu still mounted with both handlers
installed either way round, because the dismissal is a queued setState React
does not flush until the end of the discrete event. The reorder bought nothing
and cost something: `TerminalDisplay`'s close callback refocuses the terminal,
and running it before "Add New Snippet" sets its guard ref fired `engine.focus()`
in the gap before the dialog mounted — under DECSET 1004, a spurious focus-in/
focus-out pair written to the PTY on every click, visible to any TUI that redraws
on focus.
The one real defect the reorder was reaching for — a throwing action stranding
the menu on screen — is closed by making the dismissal unconditional rather than
by moving it: `try { onSelect } finally { close }`. Production order restored,
every claim about ordering rewritten to what measurement supports.
**A hover card anchored to where its row used to be.** The anchor effect was
keyed on the hovered element, and a row moves for a third reason that changes no
element and fires no event: the roster poll dropping a terminal above it, or one
keystroke in the filter box. Same DOM node, no listener re-arms, pointer never
moves — a settled wrong anchor until the user moves the pointer, on a card whose
only job is saying which row this is.
**And the payload test spoke for a helper, not for the wire.** `screen_body`'s
keys were pinned; nothing required the handler to go through it. An inline
`json!({"text": ...})` left every screen test green, and the renderer's
`typeof body?.screen === 'string'` guard turns a missing key into a card stuck on
"Reading its screen…" for ever with nothing reporting a failure.
Every guard here mutation-checked by reverting it: the effect deps, the
`finally`, the wire contract, and the two ordering oracles.
typecheck clean · jest 3605/209 · cargo 776
…give the menu a group, an id and a dwell Five things Tam asked for after taking PR #74 for a run. **"New automation for this terminal" now shows what it decided for you.** The menu hands the editor a rule that already pins the terminal that was right-clicked, and the editor opened it on the gallery's blank canvas — so the one decision that HAD been made was the one thing nothing on screen said. The palette drew its "Start with Watch output" hint for a step that was already configured, and Escape threw the pinned terminal away without a prompt, because a draft compared against itself reads clean. `freshCanvas: boolean` is now `opening: CanvasOpening` — three openings, and a flag could only ever name two of them. `'seeded'` draws `monitor` and selects it, so the inspector opens on *Watch output* with the terminal already ticked, and its dirty BASELINE is the same rule with the seeded pick removed. That is what makes the prompt honest rather than merely loud: it names the pinned terminal as the unsaved work, clears itself on save, and does not nag a user who unticks that terminal and changes nothing else. **The row names its terminal** — `New automation for this terminal (tm-…)`. Three hosts draw that row over three different things, and a window with several panes open offers the identical sentence in each; the id is the one string that says which, and it is the same id the picker ticks and the log prints. **A rule offered by name carries the automation glyph.** Four lists, not the one in the screenshot: armed rules and addable rules, each drawn twice because the section feeds an accordion and a flyout. In the accordion the icon needs a wrapper — `.au-menu-rule` is a column, so a bare sibling would stack above the name rather than sit before it. **Automation joins Command History and Snippets.** It sat among Copy/Paste on the reasoning that it acts on the terminal, which is true of everything in that half of the menu, so it never picked a group. The three flyouts are one group now, and the placement test pins the SEPARATORS as well as the order — "in the group" is a claim about the edges, and relative order alone is satisfied by a group of one. **Menu tooltips wait three seconds.** Sweeping down a freshly-opened menu to the row you were aiming at dragged a trail of native tooltips along behind the pointer. A native delay cannot be changed, but whether there is anything to show can, so the `title` attribute is withheld until the pointer has rested — no second tooltip surface with its own positioning, edge-flipping and theming to answer for. Disabled rows keep theirs unconditionally: they dispatch no mouse events, so a delayed title would never arrive, and theirs is the tooltip that carries the most. Every new guard was mutation-checked by reverting it. Five of the tooltip mutants survived the first pass and are the reason four more tests exist: the two clears covered each other on every path a sweep takes, a disabled ROW had the rule but no test, the dwell's own value was invisible to tests that tick the constant, and the unmount test watched for a React 18 warning that no longer exists — so it now counts the pending timer instead. The submenu-close test one screen above it was vacuous for the same reason and is fixed with it.
`items` is rebuilt by the owner on every one of ITS renders while this menu stays mounted across them. `TerminalDisplay` re-renders with the menu open whenever agent detection settles, and that inserts a "Color scheme for …" item ABOVE Copy, shifting every index below it. A dwelt tooltip keyed on position was then handed to whatever had slid into that slot — the wrong sentence, on a right-looking row, for the rest of the dwell. Two items sharing a label share a key, which only puts the attribute on both; just one of them can be under the pointer, so nothing shows twice. The index stays as the fallback for a labelless item.
The comment claimed it stops a stale tooltip being dragged along behind a sweep. On every path a sweep takes, the row being left clears first and that line has nothing to do — which is why mutation testing found both clears individually removable with the suite green. Its exclusive path is a row leaving the DOM under a stationary pointer, where React reports no mouseleave: filter the list and back, and the row would return with its tooltip already armed.
…ard, and off for a picker
Four findings from an adversarial review of the dwell, three of them defects.
**A tooltip was handed to a row the pointer had never rested on.** `useTooltipDwell`
is cleared only by pointer events, and a flyout's search box is focused the moment
it opens — so re-filtering needs no pointer at all. Command History minted
`history-${i}`, a POSITION, from a list it re-derives on every keystroke: rest on
the top row for three seconds, then type, and a different command inherited the
armed key and popped instantly, once per keystroke. That is the trail the feature
removes, triggered by typing instead of by sweeping.
Both halves are needed and both are here. Row ids are content-derived (a djb2 hash,
because the id becomes a DOM id and commands contain spaces), so nothing can land on
a dwelt key; and the panel resets the dwell when its row set changes, so a row that
SURVIVES a filter does not keep an armed title either. The test that was supposed to
cover this dispatched a hover the scenario it describes explicitly excludes, and so
asserted only that a fresh dwell must be earned — it now asserts the remount state
before any pointer event, and a second test uses the function form of `rows`, which
is what the real builder passes.
**Keyboard and screen-reader users lost the description entirely.** Flyout rows are
`tabIndex={-1}` with focus in the search box, so arrow keys fire no mouse event and
`titleFor` returned `undefined` for every row they ever visited — and for a snippet
or a history command the `title` is the only copy of the full text the label
ellipses. The arrowed-to row and a focused host item keep their titles. "Is the
active row" could not stand in for "was arrowed to": `onMouseEnter` sets `activeIdx`
too, so reading it that way would have handed the hovered row its title instantly
and deleted the feature. Hence an explicit `keyboardNav` mode, with a paired test.
**The path picker is not a sweep surface.** Its labels strip the shared base
directory so three same-named files read as short tails, and the full path lives in
`title` — the tooltip is the menu's content, not an explanation of it. A new
`instantTitles` prop exempts it.
**And the hook moved to `hooks/`.** Scoped to `Terminal/ContextMenu` it delayed the
flyout's automation rows while the accordion drew the identical rules — same names,
same glyph, one inch away in the pane menu — with instant tooltips.
`AutomationMenuSection` may not import from `Terminal/ContextMenu` at runtime (its
stylesheet would follow into two hosts that do not use it), so the shared thing has
to live where neither owns it. Wiring the accordion is the next commit.
Both unmount tests were vacuous and interchangeable: `getTimerCount() > 0` was
satisfied by the 260ms submenu-close timer, so the dwell test stayed green against a
hook with no timer in it, and each went red under the other's bug. Each now isolates
its own subject.
…seeded editor opens on Round-4 review follow-up (26 findings, 23 confirmed, 3 refuted). The dwell had been applied per-FILE rather than per-class: the same rule rows delayed in the terminal menu's flyout and popped instantly in the pane-title accordion, one right-click apart. - The keyboard exemption moves into `useTooltipDwell` as `onFocus`/`onBlur`. It was a private `useState` in `ContextMenu` for one round; three more call sites arrived behind it, and a per-site copy of an accessibility exemption is a per-site chance to ship without it. `onLeave` and `reset` deliberately leave focus alone, so a pointer sweeping across a focused control cannot take a keyboard user's description away. - Both accordion rule rows and `PaneContextMenu`'s Mute — the whole of the enabled-and-titled class outside `ContextMenu` — now dwell. Disabled items keep their titles unconditionally: they dispatch no mouse events, so a dwell over one can never be measured. - The bolt is one exported constant (`AUTOMATION_GLYPH`) rather than four declarations, which is what "stated once" already claimed and was not. - The seeded tests pin `draft.rule` absolutely. A mutant rewriting `targetMode` to 'rule' passed all 369 automation tests while reproducing the exact defect the seeded opening exists to fix. - The dirty baseline is asserted from the unseeded starting point rather than from `draft.rule`; the old form restated the implementation's own formula, so the promise its comment made was one it could not keep. Three claims the code does not support, corrected in place: "back to an untouched blank rule" (the baseline keeps `targetMode: 'pinned'`, which `problems()` still blocks on `targets.empty` — now asserted), the `min-width: 0` on `.au-menu-rule-head` (a no-op on a cross-axis flex item, and it never capped the menu), and "the mark for an automation" (the WebGL toggle carries the same bolt in the same menu). The icon tests assert ORDER, not presence, and the accordion label is swept over two terminals. 23 mutants run, 23 killed. Four survived the first pass and needed new tests: `onEnter`'s own clear (every sweep path fires `onMouseLeave` first, and `reset()` covers only query-driven row changes), and all three `instantTitles` branches for flyout rows, which nothing reached because the picker that motivated the prop is a flat menu. typecheck clean; jest 3636 / 209 suites.
Both mine, both from the previous commit. - `onEnter`'s clear said `reset()` "covers the same shape from the list's side". It covers only part of it: `reset()`'s one caller runs on a change of `query`, so a row set that changes because the DATA under it changed never reaches it. That gap is exactly why this line survived the mutation sweep until a test was written for it, so the comment now says which half is left. - `onBlur`'s keyed clear was justified by React running the new control's `onFocus` before the old one's `onBlur`. Backwards: `focusout` fires before `focusin`, so a bare clear would work today. The keyed form is still right — it just buys order-independence, not a fix for a live bug.
…first Tam: "click New automation -> select predefined template -> it should become Unsaved, when user close it should show confirmation". The gallery handed a picked template to the editor on the 'saved' opening, whose baseline is the rule itself — so it read clean, the "unsaved" chip never showed, and Escape threw away the template the user had just chosen without a word. That is the 'seeded' defect again, one card to the left: a choice had been made and nothing on the way out said so. `CanvasOpening` gains 'template'. It draws like 'saved', because a template IS a complete rule, and its dirty baseline is the blank rule the gallery was showing BEFORE the click — which makes the whole template the unsaved work, because that is what it is. The baseline carries the resolved layout so that arrangement alone can never be what makes a fresh template read dirty. The panel's `View` carries the opening instead of a `fresh: boolean`. That flag named only the blank card, and this is the third answer it cannot give: an existing rule opens clean, the blank card opens clean on an empty canvas, and a template opens dirty on a full one. Same reason `freshCanvas` stopped being a boolean two rounds ago. No new dialog machinery — `requestClose` already asks when `isDirty`, so the confirmation follows from the baseline being right. 6 mutants on the new behaviour, 6 killed. One survived first: giving the BLANK card the 'template' opening too, because a blank rule's template-baseline works out identical to itself, so it still reads clean and only the CANVAS differs. The blank-card test now pins the empty canvas, not just the absence of a prompt. typecheck clean; jest 3642 / 209 suites.
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tam's brief carried one P0 and four P1s.
P0 —
command contains "claude"matched nothing, everThe rule editor kept showing "Nothing to watch. No open terminal matches command contains "claude"" while Claude was visibly running in several terminals.
resolve()was not at fault.foreground_command_linedescended from the terminal's shell to the deepest youngest descendant and read only that process's command line. Walking the real process table on the dev machine:Every live
claude.exehas children, so the string tested against"claude"was conhost's. Deterministic, not flaky — the agent is on the chain, just never at the end of it, because agents spawn helpers.The tell was next door:
get_foreground_agent_with_exeruns the identical descent but tests every level, which is why per-agent colour schemes could name Claude while targeting could not. Three copies of one walk; one answered the question.There is now one descent —
foreground_chain_from, a pure core taking(pid, parent)pairs so it is testable without a live process table (the precedenthas_live_childrenalready sets in that file), depth-capped and cycle-guarded because targeting walks it inside the mutexProcSnapshotholds. All three callers are re-expressed on it.RosterRow.command_line: Option<String>becamecommand_lines: Vec<String>rather than a joined string, so the field's name stays true.This fixes the class:
node,npm,codex— every needle naming a program that spawns helpers — were broken identically.A regression the first version of that fix introduced, caught in review: reading the shell link is necessary (an idle
pwshmatchedpwshbefore and must still), but both spawn paths append-NoExit -Command <PS_CWD_INTEGRATION>— ~300 characters of app-authored script containingCurrentDirectory,FileSystem,[Environment],prompt,try,catch. Left in, roughly thirty ordinary needles (dir,path,env,command) would have selected every PowerShell terminal in the app, and a rule with a send action would have typed into all of them.command_line_fornow drops exactly that injected sequence — script compared against the constant, the two flags removed only as the tokens immediately preceding it — so a profile's own-Commandsurvives whole.P1 — the Watch Output card showed one clipped line
.au-nvalwaswhite-space: nowrapwith an ellipsis inside a card fixed at 244×160. It clamps at two lines now andAU_NODE_Hgrows to 180. Every reader of that constant is derived; saved layouts store x/y only, so nothing persisted moves.P1 — the terminal picker was unidentifiable
Rows carried no
titleat all while id, name and folder each ellipsed, so two terminals in similar folders looked identical — andshell/pidwere fetched and shown nowhere. Rows now carry full text, and hovering opens a card with every roster field plus ~6 lines of that terminal's current screen via/snapshot(never/output, which replays a lossy ring through a fresh parser). Only the hovered row is fetched; a closed terminal says so rather than showing an empty box that reads as a broken fetch.P1 — Automation is always available in the context menu
Both menu surfaces rendered nothing when nothing was armed, and the code said why — a justification that expired the moment the section gained something to do on an unarmed terminal. Rewritten in place, in the section and in its two callers.
Armed or not, both hosts now offer New automation for this terminal (opens the editor on a draft already pinned to it) and Add to an existing automation (only
pinned-mode rules — a criterion rule ignorestargetIds, so offering it would be an item that visibly does nothing).P1 — predefined rules: flash, don't auto-enable
Per Tam's ruling, the safety property stays. Templates land disabled on purpose,
save()can only moveenabledtoward off, and the toggle refuses while the draft is dirty — so a saved template did nothing with no cue that a step remained. After a save whose result is disabled and has no blocking problems, the Enable toggle pulses briefly. Never when it saved enabled, never when it saved disabled because it is blocked.Review
Two independent audits ran before this opened. The spec audit found every acceptance criterion met, no scope creep, one stale comment. The architecture audit found 1 HIGH (the over-match above), 4 MEDIUM, 6 LOW and nine assertions that did not pin what their names claimed — all fixed in this round rather than one per resubmission. The MEDIUMs: a stale rule snapshot that could resurrect a deleted rule through an unconditional upsert; a re-render guarantee that was false for the exact case the feature serves (
useArmedAutomationsreturns a stableEMPTY); an unevicted snapshot cache; andAU_NODE_Hpinned by nothing.Gates
bun run typecheckclean ·bunx jest3642 passed / 209 suites ·cargo test --lib762 passed (no Rust touched after the P0).Every new guard was mutation-checked — including the one that restores the shipped bug (
resolvereading only the last chain link) and the one that restores the over-match (no stripping at all). Each killed exactly the test that names it.After Tam's GUI pass — five more asks, then a sixth
Tam ran the branch and asked for five changes to the terminal right-click menu, then one more after
running it again. All are in.
hands over a rule pinned to that terminal, and the editor used to open it on an EMPTY canvas
compared against itself — so the one decision that had been made was the one thing nothing on
screen said, and Escape threw the pinned terminal away without a word.
freshCanvas: booleanbecame
CanvasOpening, because there were three ways of opening the editor and a flag has two.tm-…).accordion and once in the flyout, not just the one in the screenshot.
longer drags a trail of yellow boxes behind the pointer.
the left: the gallery handed it over on the
'saved'opening, so the editor compared the templateagainst itself and let Escape discard it silently.
CanvasOpeninggained a fourth value.Review rounds
An internal 8-agent workflow (4 find lanes → 4 adversarial verify lanes) reviewed the GUI round:
26 findings, 23 confirmed, 3 refuted (A=4 · B=6 · C=8 · D=8), all fixed here.
The two largest were both in work that had just been called done, and both had the same shape — a
fix applied to the FILE in front of me rather than to the class. The dwell went into
Terminal/ContextMenu.tsxonly, so identical rule rows delayed in the flyout and popped instantlyin the accordion one right-click away; and
dweltwas keyed onrow.id, which Command Historymints as
history-${i}from a list it re-derives on every keystroke — so typing, with no pointerevent at all, handed an armed tooltip to a different command, reproducing the exact trail the
feature removes. Two more were my own tests asserting nothing: one dispatched a hover the scenario
it describes explicitly excludes, and both unmount tests counted a timer that a submenu-close timer
already satisfied.
One finding was refuted and deliberately not acted on: that the withheld
titlecan neverproduce a tooltip at all. The verifier could not establish it and caught the premise error behind
it, so the mechanism was not redesigned on a claim nobody could prove. It remains the one thing only
a real window can settle.
Three PRE-EXISTING defects were confirmed and deliberately left out of scope — none introduced
here, each wanting its own commit:
AutomationEditor.savesendsdraft.rulerather thanruleFromDraft(draft), so a dragged card is never written and the draft stays dirty for ever;"Open the full log" unmounts the editor with no unsaved-changes guard, at three sites; and the
expanded accordion runs off the right edge (pre-existing, though the new
(tm-…)is the first thingcut). Recorded in
termflow-fabric/docs/plan/031under Open (after round 5).Mutation testing
Every new guard was reverted individually and its suites re-run — including the ones that restore
the shipped bugs (
resolvereading only the last chain link; no-Commandstripping at all).The later rounds ran 29 more mutants. Five survived a first pass on the tooltip work and four on
the round-5 work, each revealing a real hole rather than a cosmetic one — the clearest being that
both unmount tests were interchangeable, and that all three
instantTitlesbranches for flyout rowswere unreachable by any test. A sixth pass on the template opening found that giving the BLANK card
the
'template'opening passed everything, because a blank rule's template-baseline works outidentical to itself and only the canvas differs. All closed; final state is every mutant killed.
Not tested here
The GUI. Tam tests that himself — worktree
termflow-core/.claude/wt-automations.