Repository navigation
🤖 fix: left-align activity chips in the live line without a TODO chip - #7688
Conversation
Without a plain TODO chip, bordered activity chips now follow the status label instead of being pushed next to Stop, and the status and before-Stop dividers only show when a TODO chip is present. Adds SubAgentsOnly desktop and phone story plays for the layout.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
🟢 e2e stories (Haiku): 14 passed22 agent steps · 7 replayed from cache · 15 model calls · 46.3k tokens (24% cached) All 14 tests in 9 files
e2e 0.20.0 · 1m 12s · web · run artifacts 🟢 e2e Electron stories (Haiku): 3 passed3 agent steps · 3 model calls · 8.0k tokens (22% cached) All 3 tests in 3 files
e2e 0.20.0 · 20.7s · electron · run artifacts 🟢 e2e agent repros (Haiku): no tests selectedSpend: $0.00 of the $1.00 cap (proxy ledger) e2e 0.20.0 · 6.6s · 2 targets (web, phone) · run artifacts |
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
## Summary Delivers every follow-up that coder#7660 listed for the composer live line, in one PR: bash rows lead with their command, the plan chip keeps a finished plan until the next turn, the TODO, sub-agents and reviews chips get shortcuts, a "+N" chip shows strip overflow, phone landscape gets 44px sheet targets, and everything that waits on the user folds into one "Waiting on you" card (a summary row plus a sheet on phones). ## Background coder#7660 shipped the live line above the composer and left these items open: - One "Waiting on you" surface for open questions and queued or held messages. - No shortcut for the TODO, sub-agents and reviews chips. - No overflow cue when all five chips do not fit at 390px (the design's "+N" chip). - Bash rows showed the agent's display name instead of the command. - Closing a bash's output dialog opened with the mouse left focus on the page body. - A fully completed plan cleared when its turn ended, so its chip vanished instead of showing 5/5. - Phone landscape (844x390) sheet Close button stayed 32px, because the 44px touch rule stops at 768px wide. - A bash row's terminate button had no accessible name. ## Implementation - **Bash rows** (`BackgroundProcessesBanner.tsx`): the row title is the command, the display name is secondary, terminate has an `aria-label`, and closing the output dialog returns focus to the row that opened it. - **Finished plan chip** (`StreamingMessageAggregator.ts`, `WorkspaceStore.ts`, `todoList.ts`): a plan whose items are all done stays until the next user turn starts instead of clearing at stream end; between turns the sidebar keeps the agent's own status instead of the plan's last step. A finished plan that only the window seed knows (a reload mid-turn whose retained window starts at the running turn) may be the previous turn's, so it still clears when the running turn ends, as before. - **Shortcuts** (`keybinds.ts`, `ChatInputDecoration.tsx`, `chatActivityPanels.tsx`): `OPEN_TODO_PANEL`, `OPEN_SUB_AGENTS_PANEL` and `OPEN_REVIEWS_PANEL` are Ctrl+Alt+O/A/R, matched by `code` with an AltGr guard so international layouts that type characters with AltGr do not trigger them. Like the background bashes shortcut, the chord focuses the list (opening it if needed) and closes it when pressed from inside. They leave the keys to a focused browser or desktop viewport, as the background bashes shortcut now does too. In the activity sheet (phones and short windows), a modal over the inert page, the chords and the background bashes shortcut still swap and close its panels while focus is in it; other dialogs still block them. They are listed in Settings and in `docs/config/keybinds.mdx`. - **"+N" overflow chip** (`ChatInputDecoration.tsx`): when chips overflow the strip, a trailing chip counts the hidden ones and opens the sheet. A hidden chip's entry in the "+N" list opens its panel with focus in it (the popover comes before the "+N" chip in Tab order), and closing returns focus to the "+N" chip. The line can hide a chip at any time (Stop or a status label takes room, or room comes back), and a hidden chip cannot keep focus, so `shownFocusTarget` (`chatActivityPanels.tsx`) picks the first the line shows of: that chip, the "+N" chip, the chip the "+N" chip stood in for, any chip. It runs when a focused chip hides, and when the sheet closes (through `ChatDockSheet`'s `returnFocus` prop), so a chip that left with its activity also hands focus to one the line shows. The background bashes list returns focus itself (a row's Escape, or its chord) and picks the "+N" chip the same way while the bashes chip is hidden (`shownChip`). The "+N" list opens at the line's end with or without a TODO chip (coder#7688 moved bordered chips' panels to the line's start without one): the "+N" chip shows only in a full strip, so it ends the strip either way. - **Landscape targets** (`ChatDockSheet.tsx`): the sheet's Close and tab buttons use `pointer-coarse:size-11`, so they are 44px on touch at any width (the `globals.css` touch rule stops at the phone width); a test-runner check covers the phone landscape story. On touch phones the tabs keep their whole label and count, and the tab strip scrolls instead of shrinking them. - **Waiting on you** (`WaitingOnYou.tsx`, `ChatDockSheet.tsx`): the ask-user question, legacy plan import and interrupted delegated creation docks self-report through `useWaitingOnYou`; `ChatPane` adds the context switch warning, held inputs and queued messages through entry helpers. With two or more, desktop shows one card with tabs and a collapse toggle; queued messages show as a row under it and do not count toward the threshold. Phones show a summary row that opens a sheet with the chosen item and "Also waiting" rows. Each dock renders through a portal into a container that moves between the slot, the card body and the sheet, so switching layouts does not remount it: a half-typed answer keeps its textarea and its focus when another item arrives or leaves. `ChatDockSurface` keeps the same two elements inside and outside the card (only classes change), and a `getSnapshotBeforeUpdate` probe records what had focus before each commit, because removing the card takes the moved content's focus with it before any effect runs. Content that stops waiting (an old-plan notice that found nothing to import) leaves the card and shows inline, with its normal gutter and column. When an item that holds focus leaves while others still wait (the item on show, or on phones a focused "Also waiting" row, such as a queued message that sends itself) and its focus fell to the page, focus moves to what shows the next item (the sheet's content on phones, the selected tab on desktop). A focused control that leaves the open sheet while its item stays (one of several unsent messages) puts focus back in the sheet too. When the card breaks up around the focused control (one item is left, and on phones the sheet closes), the item left over takes focus inline: slot containers are focusable for that (`tabIndex=-1`, not in the Tab order). When the window outgrows the open sheet, the desktop card's selected tab takes focus. `ChatDockSheet` is the sheet extracted from `ChatActivitySheet` so both use it; it makes the covered page inert in a layout effect, so the commit that closes the sheet lifts that before a slot that leaves the sheet puts its focus back (an inert textarea cannot take focus), and Escape from `<body>` closes it too. In the sheet, which scrolls, the queued message's dispatch menu opens below its row (`useInWaitingOnYouSheet`): above it, the sheet cut off its first choices. ## Validation - Unit tests: `WaitingOnYou.test.tsx` (threshold, queued exclusion, tab switching, no remount across layouts, focus when the shown item, a focused sheet row or one of several unsent messages leaves, the phone sheet's 2 to 1 handoff, the card breaking up around a focused tab or sheet row, the sheet to desktop crossing), `chatActivityOverflow.test.tsx` (+N count, panel chords incl. AltGr guard, focused viewports and chords from inside the phone sheet, sheet focus return when a chip or the +N chip hides, the hidden spacer's gap), aggregator and store tests for the kept finished plan (and the seed-only one that clears), bash row tests. - Storybook: `App.waitingOnYou` Desktop, Phone, `CountChangeKeepsTyping` (drops and restores the unsent messages while an answer is typed; fails in Chromium without either part of the remount fix) and `PhoneCountChangeKeepsTyping` (the same in the phone sheet), `App.composerActivityLine` (overflow, landscape 44px check in the test runner, focus on "+N" when a turn hides the chip that opened the sheet, chords that swap and close the phone sheet's panels from inside it, the bashes chord included), `App.composerActivityLine` `NarrowPaneDesktop` (a desktop window with a narrow chat pane: the bashes list opened from "+N" hands focus back to the "+N" chip on Escape and on its chord), `App.phoneViewports`. The new focus assertions fail without their fix. - Remote UAT through Coder Agents (dogfood), against base e5fa140: claims 1 to 5 held (bash rows, finished plan chip, chords incl. AltGr and held-key cases, +N at 320/390/414, 44px landscape targets). Round fu-r1 found the count-change remount (fixed in 9eb14e9). Round fu-r2 confirmed the Codex round 2 fixes and found two phone sheet focus drops (fixed in dfca77d). Round fu-r3 on dfca77d passed and found two low focus gaps (Escape from `<body>` in the Waiting on you sheet, a focused "+N" chip that hides), fixed with the Codex round 4 findings in 7aa3fa0. Round fu-r4 on 7aa3fa0 found that a bashes list opened from the "+N" entry dropped focus to `<body>` on Escape (fixed in a805bfb); its other checks held. Round fu-r5 on 1f9a575 (after merging main) passed: the panel chords inside the activity sheet, a focused queued row that sends itself, the "+N" bashes focus fix and the coder#7688 layout held; it found one low focus gap (the card breaking up around a focused "Also waiting" row dropped focus to `<body>`), fixed with the Codex round 6 findings in 907079d. Round fu-r6 on 907079d passed: the item left over takes focus when the card breaks up (phone and desktop), focus stays in the open sheet when one of several unsent messages leaves, the desktop card's tab takes focus when the window outgrows the sheet, the chip fits one column gap sooner without a TODO chip, and a typed answer keeps its textarea and focus across 2 to 1 on both layouts; 1f9a575 dropped focus to `<body>` in the first three. - `make crap-check` with coverage, `make dry-check` and `make check-phone-chrome` pass. The phone chrome skeleton (`phoneChrome.generated.ts`) is regenerated: the live phone page now has the Waiting on you slots (their containers carry `outline-none`), the strip's `data-activity-strip` marker and the invisible "+N" chip. That check runs only in the merge queue, so PR CI does not show it. ## Risks Medium, limited to the composer's dock area. The Waiting on you portal moves existing docks between containers; a bug there would show as a dock rendering in the wrong place or losing local state. The finished-plan change keeps a plan visible one turn longer; the sidebar status ignores it between turns. The new shortcuts use Ctrl+Alt chords that were unbound. `ChatDockSheet` now lifts the page's inert one phase earlier (layout instead of passive cleanup); focus still returns in the passive cleanup. A click on a dock's plain text now focuses its slot container instead of `<body>`. ## Follow-ups - A focused chip that leaves the line entirely (its last item ends) while no sheet is open drops focus to `<body>`. This predates the PR (same on e5fa140) and is left for a separate change. - The Waiting on you sheet's content host and a slot container that takes focus when the card breaks up have `outline-none`, so after that handoff a keyboard user sees no focus ring until the next Tab (found in UAT round fu-r6). A `focus-visible` ring for both is left for a separate change.
Summary
When the composer's live line has activity chips (sub-agents, bashes, reviews, Chat Instructions) but no TODO chip, the bordered chips now sit as pills right after the status label, with no dividers. Before, a spacer pushed them next to Stop and left a divider on each side of an empty gap (
STREAMING | ... [7] | Stop). With a TODO chip, the layout is unchanged.Implementation
data-activity-chip-plain. InStreamingBarrierView, the spacer, the divider after the status and the divider before Stop are keyed on that marker (group-has-[[data-activity-chip-plain]]/live) instead of on any chip. Everything else still keys ondata-activity-chip.phoneChrome.generated.tspicks up the new spacer class (the idle live line is part of the phone skeleton;make check-phone-chromeruns only in the merge queue's E2E shard). The file was edited in place, not regenerated, because this host's disk was too full for a build, and the only change is that one class string. A dispatchedpr.ymlrun checks it withmake check-phone-chrome.Validation
SubAgentsOnlyDesktop/SubAgentsOnlyPhonestory plays check the chip's position (left of the line's midpoint), that no dividers are drawn, that Stop is on the right, and that the popover opens over the chip. Each assertion fails with its fix reverted (expected 771 to be less than 506; popoverexpected 410 to be less than or equal to 202).Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:medium• Cost:$0.36