Split terminal and agents into separate panels (#3049) - #935
Merged
Conversation
* feat(ok): split terminal and agents panels
* fix(ok): let terminal tabs launch a CLI again
The split gave terminal hosts `showClis={false}` and pinned their New choice
to a bare shell, which removed the whole dropdown (`hasMenu` also goes false
with no agent rows) from both the bottom dock and the standalone terminal
window. Picking a TUI was the point of that menu, so restore it: the CLI +
Terminal rows are back, the pick sticks, and the primary, ⌘J reveal-seed and
⇧⌘J all repeat it.
One deliberate departure from the pre-split behavior. `resolveLauncherSelection`
falls back to the first enabled CLI when nothing is picked — right for a
composer, wrong for the terminal dock, where it would drop a user who never
chose a TUI straight into one. A CLI now counts only as an explicit remembered
pick; every other outcome is a plain shell.
⇧⌘J needed its own resolution rather than reusing the Ask-AI one, which turns
the bare-terminal knobs off on purpose: a passage needs an AI, but a promptless
new session may legitimately be a shell. Reusing it would have made "pick
Terminal, press ⇧⌘J" open an agent. `claimsAskAi` serves both callers now, so
it is `claimsSessionKind`.
* feat(ok): move the edge reveal tab to the agents panel
Exactly one panel should advertise itself with a persistent edge tab, and it
should be the agents panel: an agent conversation is the thing worth a one-click
return, and a chord is not discoverable. It is no longer gated on having
conversations — a never-opened panel now shows the tab too, since its cold entry
(the New button) is a fine place to land.
The terminal loses its tab. It is a ⌘J surface, and a second permanent tab over
the editor footer's bottom-right competed with the Ask AI composer for that
corner. That cascades out a fair amount of now-dead wiring: TerminalDock's
onReveal/showRevealTab, EditorArea's onRevealTerminal and terminalAvailable,
EditorPane's revealTerminal, the whole agentsHasSessions chain, and
EditorFooter's reserveRightGutter (which existed only to pad clear of that tab).
The bottom Ask AI composer now also hides while the agents panel is open. Unlike
the terminal it does not overlap the composer, so this is about redundancy
rather than real estate: an open panel is already a place to type the request.
The gate's own test was not catching the new required input — packages/app
excludes *.test.ts* from tsc, so the missing field read as undefined (falsy) and
passed. Added explicitly, with hide-cases for both predicates.
* fix(ok): drop the orphaned onHasSessionsChange prop, bump changeset to minor
Removing the agents reveal tab's has-sessions gate took out the prop's only
consumer, leaving the declaration, destructure and effect firing into an
always-undefined callback — with JSDoc still claiming column visibility is
session-presence-driven, which is no longer true of either panel.
The changeset also declared `patch`. Per .changeset/README.md a new feature or
additive API surface is `minor` pre-1.0, and this adds panels, shortcuts, a menu
item and palette entries; the fixed group bumps in lock-step on the highest
declared level, so `patch` under-versions the whole group.
Both found by /review-local.
* fix(ok): close the review's coverage, a11y and hardening gaps
Both Major findings were coverage gaps on the split's own headline behaviors,
and both are mutation-checked — the production code was broken to confirm each
new test actually catches the regression described, rather than passing
vacuously:
- The reload-survival test hardcoded `agentPanelVisible: false`, so the
`setAgentsVisible` restore branch never ran. Deleting it (or letting the field
name drift across the IPC seam) would have left the agents panel collapsed
after every reload, silently. Adds the agents case and a both-retained case,
which also pins that restoring one panel does not drag the other open.
- `buildViewMenuStateDeps` mapped `agentPanelVisible` and `onToggleAgentPanel`
with neither under test; `menu.test.ts` passes props straight to
`buildMenuTemplate` and bypasses the mapping. A dropped field would freeze the
native item on "Show Agents"; a copy-pasted action ID would close the terminal
instead.
Both panels also rendered a tablist named "Sessions". They are co-visible now,
so a screen-reader user met two indistinguishable tablists (WCAG 1.3.1) — the
name is derived from `edge` (the standalone window has none and is a terminal),
and the test asserts the two names DIFFER rather than merely exist.
Hardening: `set-dock-state` validated `req.surface` nowhere, though it indexes
the per-window order map and the sibling `cli-preflight` handler guards its own
discriminant for exactly this reason; `getDockState` collapsed every IPC failure
into a silent null, making "my panel comes back empty" ungreppable. The
reviewer also proposed dropping the caller's now-"redundant" catch — declined:
the web path can still throw synchronously outside the promise, and the second
call site is a bare `void ...then()` with no catch of its own.
Found by /review-local.
* fix(ok): share the sticky pick across both session hosts
pullfrog caught a real double-dispatch bug. The two hosts partition Ask-AI and
⇧⌘J work by comparing the kind each one resolves, which only holds while they
read the same inputs — but `stickyAgentId` and `preferBareTerminal` were
mount-time `useState` snapshots, and the stores wrote to localStorage with no
listener. One New-dropdown pick mutated only the terminal dock's copy, and from
that click on both hosts claimed: ⇧⌘J opened two sessions and revealed both
panels, and ⌘J-with-selection delivered the passage twice.
Both stores now carry the module-scope-state + listener-Set + useSyncExternalStore
shape `lib/acp/enabled-agents.ts` already uses, so every surface observes one
value and the invariant holds by construction rather than by comment. The
composers get the same fix for free. Deliberately cache-free: the values are
primitives, so an unchanged read is a no-op re-render, whereas a module cache
survives `localStorage.clear()` — that leaked one test's CLI pick into the next
while I was building this.
Also from review:
- ⌘J on the web host preventDefaulted and then hit `if (!terminalAvailable)`,
which can never be false there (the effect already returned when okDesktop was
non-null) — swallowing the browser's own ⌘J for a no-op. It now claims the
chord only when it will act. The existing test pinned the swallow; it now pins
the opposite.
- ⌥⌘L is KDE Plasma's built-in alternate for Lock Session, grabbed by
kglobalaccel at the shell level, so the rehomed Ask AI chord would have locked
the screen on KDE. Moved to ⇧⌘L — unclaimed by KDE and GNOME defaults, still
the L family, and the chord Cursor uses to send a selection to chat.
- AgentsPanelWidthStorage -> WidthStorage, matching doc-panel-width-store and
terminal-height-store.
Found by /review-local and the PR's cloud reviewers.
* fix(ok): drop a duplicated JSDoc, pin the width-key migration contract
Two follow-ups from cloud review of f779c69d2.
`EmptyEditorState` carried two consecutive JSDoc blocks on one param — the first
still describing the removed `terminalDock: string | null`, so a reader scanning
the type inline met contradictory documentation.
The width store's tests all referenced `AGENTS_PANEL_WIDTH_KEY` rather than its
value, but the migration contract IS the value: the key stays
`ok-terminal-width-v1` so a user who sized the right column when it held the
terminal keeps that width now the agents panel owns it. Renaming the constant and
its value together would have kept every test green while silently resetting
everyone's panel width — and the terminal/agents naming mismatch makes exactly
that rename tempting (I renamed a different symbol in this file one commit ago).
* test(ok): close the three pending recommendations from re-review
None were blocking — the re-review landed APPROVE WITH SUGGESTIONS — but all
three are correct and cheap, so they land here rather than as tracked debt.
`claimsSessionKind`'s complement arm kept the partition total, but total is not
correct: a new `LauncherSelection` member would have fallen silently to the
terminal dock, which may be the wrong owner. The terminal side's kinds are now a
total `Record`, so adding a member is a compile error here instead of a quiet
mis-route — and the Record IS the predicate, not a decorative assertion beside it.
`threadLaunch` forwarding had no coverage: the host mock captured `launch` and
dropped `threadLaunch`, so nothing proved EditorPane forwards the intent or that
it reveals the panel first (a thread launched into a hidden panel is invisible
work). Mutation-checked — removing the prop fails the test.
`getInitialAgentsPanelWidth`'s two guards were untested, a gap inherited from the
store this replaced. The interesting one is a `localStorage` GETTER that throws
(file://, Safari private mode, sandboxed iframes), where `typeof localStorage` is
not a safe probe.
---------
GitOrigin-RevId: d9c3d8f4ec9d58f905b100a67f01e1d0bdfbc3b7
|
|
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.
Copybara-translated 1 Inkeep OSS change. Rebase-merge this PR so the prepared commit lands directly on public main.