diff --git a/relay/src/directory.ts b/relay/src/directory.ts index 7f865d5..3f697da 100644 --- a/relay/src/directory.ts +++ b/relay/src/directory.ts @@ -107,7 +107,7 @@ export class FleetDirectory extends DurableObject { // one PUT, which is one send per socket, against a fleet that has left // half-dead sockets behind or a secret-holder opening them in a loop. A // one-operator fleet reaches double digits. - if (this.ctx.getWebSockets('daemon').length >= MAX_DAEMON_SOCKETS) { + if (this.ctx.getWebSockets('daemon').length >= this.maxDaemonSockets()) { return new Response('{"error":"too many directory sockets"}', { status: 503, headers: JSON_NO_STORE, @@ -155,7 +155,7 @@ export class FleetDirectory extends DurableObject { return stored(key, 200) } const count = (have.get(COUNT_KEY) as number | undefined) ?? 0 - if (count >= MAX_ENTRIES) return full() + if (count >= this.maxEntries()) return full() const version = (have.get(VERSION_KEY) as number | undefined) ?? 0 // One multi-key put: the blob, the count and the version land together or // not at all. @@ -327,6 +327,23 @@ export class FleetDirectory extends DurableObject { // Already closing. } } + + /** + * The cap, read the way the hub reads its deadlines: the binding when there + * is one, the constant when there is not, and the constant again for a + * binding that is not a usable number. Nothing in production binds it; see + * DIRECTORY_MAX_ENTRIES in src/index.ts for why the seam exists at all. + */ + private maxEntries(): number { + const n = Number(this.env.DIRECTORY_MAX_ENTRIES ?? MAX_ENTRIES) + return Number.isFinite(n) && n > 0 ? n : MAX_ENTRIES + } + + /** The socket ceiling, read exactly as the entry cap above is. */ + private maxDaemonSockets(): number { + const n = Number(this.env.DIRECTORY_MAX_DAEMON_SOCKETS ?? MAX_DAEMON_SOCKETS) + return Number.isFinite(n) && n > 0 ? n : MAX_DAEMON_SOCKETS + } } /** The storage prefix for blobs; the rest of the key is the digest. */ diff --git a/relay/src/index.ts b/relay/src/index.ts index 17ad2e3..835ba96 100644 --- a/relay/src/index.ts +++ b/relay/src/index.ts @@ -24,6 +24,23 @@ export interface Env { * closes it, in ms — the same test seam. Unset in production, where the hub * defaults to five minutes (src/hub.ts, CLIENT_IDLE_MS). */ CLIENT_IDLE_TIMEOUT_MS?: string | number + /** How many entries one fleet directory will hold — the same test seam + * (vitest binds a small number). Unset in production, where it is 512 + * (src/directory.ts, MAX_ENTRIES), and the reasoning for that number is + * written down beside it. + * + * It is a seam because the cap is only reachable by filling it, and filling + * it is 512 sequential round trips through a Durable Object — five seconds + * of a runner's time on a good day, and over vitest's own deadline on a + * loaded one. Binding it is the difference between a test about the cap and + * a bet on how fast the machine is. */ + DIRECTORY_MAX_ENTRIES?: string | number + /** How many push sockets one fleet directory will hold — the same test seam + * for the same reason. Unset in production, where it is 256 + * (src/directory.ts, MAX_DAEMON_SOCKETS). Reaching that cap means opening + * 256 WebSockets one at a time and holding them all open, which is the other + * test in this file that timed out on CI and passed on a laptop. */ + DIRECTORY_MAX_DAEMON_SOCKETS?: string | number /** The version of the flue that deployed this Worker, stamped by the * deploy as a plain-text binding (internal/relaydeploy, VersionVar) and * reported on /api/health. It is how a daemon sees that this relay is diff --git a/relay/test/directory.test.ts b/relay/test/directory.test.ts index fde227f..1d0398c 100644 --- a/relay/test/directory.test.ts +++ b/relay/test/directory.test.ts @@ -20,11 +20,33 @@ import { BASE, Leg, machineId, sleep, TEST_SECRET, within } from './harness' /** Mirrors MAX_BLOB_BYTES in src/directory.ts. */ const MAX_BLOB_BYTES = 4096 -/** Mirrors MAX_ENTRIES in src/directory.ts. */ -const MAX_ENTRIES = 512 +/** + * Mirrors DIRECTORY_MAX_ENTRIES in vitest.config.ts, *not* MAX_ENTRIES in + * src/directory.ts — the cap is a test seam, and this is the bound half of it. + * + * Production is 512, and the arithmetic that picks it is written down beside + * the constant. It is not the number to test against: the only way to test a + * cap is to reach it, and reaching 512 is 512 sequential round trips through a + * Durable Object — around five seconds on a quiet machine and past vitest's + * own five-second deadline on a loaded one, which is how these tests came to + * fail in CI and pass on a laptop. This reaches the same branch in a fraction + * of that; what is under test is the refusal, not the size of the number. + * + * It has headroom on purpose. The shared directory — the one `SELF` routes to, + * which nearly every other test in this file writes into — holds ten entries + * by the end of the run, and a bound it could reach would start refusing PUTs + * in tests that are not about the cap at all. That failure reads as "the push + * never came", nowhere near the number that caused it. + */ +const MAX_ENTRIES = 64 -/** Mirrors MAX_DAEMON_SOCKETS in src/directory.ts. */ -const MAX_DAEMON_SOCKETS = 256 +/** + * Mirrors DIRECTORY_MAX_DAEMON_SOCKETS in vitest.config.ts, for the reason + * MAX_ENTRIES above is bound: production is 256, and reaching it means opening + * 256 WebSockets one at a time and holding all of them open. What is under + * test is the refusal at the ceiling, not where the ceiling is. + */ +const MAX_DAEMON_SOCKETS = 8 const DIRECTORY = `${BASE}/directory` diff --git a/relay/vitest.config.ts b/relay/vitest.config.ts index d50c3b4..946d8af 100644 --- a/relay/vitest.config.ts +++ b/relay/vitest.config.ts @@ -42,6 +42,17 @@ export default defineConfig({ HANDSHAKE_TIMEOUT_MS: 600_000, CLIENT_IDLE_TIMEOUT_MS: 600_000, PAIR_TIMEOUT_MS: 250, + // The directory's entry cap, bound small for the same reason the + // deadlines above are bound at all: the only way to test a cap is to + // reach it, and reaching the production 512 is 512 sequential round + // trips through a Durable Object — five seconds on a quiet machine + // and past vitest's own deadline on a loaded one. test/directory.ts + // mirrors this number; production binds nothing and gets 512. + DIRECTORY_MAX_ENTRIES: 64, + // And the socket ceiling, bound small for the same reason: reaching + // 256 means opening 256 WebSockets one at a time and holding every + // one of them open. test/directory.ts mirrors this number too. + DIRECTORY_MAX_DAEMON_SOCKETS: 8, DAEMON_SECRET: 'test-secret', // The deploy stamps this (internal/relaydeploy, VersionVar); binding // it here is what lets the health test pin the passthrough. diff --git a/web/src/components/new-session-dialog.test.tsx b/web/src/components/new-session-dialog.test.tsx new file mode 100644 index 0000000..d747d92 --- /dev/null +++ b/web/src/components/new-session-dialog.test.tsx @@ -0,0 +1,187 @@ +import { render, screen } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { describe, expect, it, vi } from 'vitest' + +import type { NewSessionRequest } from '@/sessions/new-session' +import { NewSessionDialog, type NewSessionDialogProps } from './new-session-dialog' + +const MACHINES = [ + { id: 'local', name: 'mesa.local' }, + { id: 'attic-pi', name: 'Attic Pi' }, +] + +function show(over: Partial = {}) { + const props: NewSessionDialogProps = { + open: true, + initial: {}, + machines: MACHINES, + known: [], + onSubmit: vi.fn(), + onClose: vi.fn(), + ...over, + } + const view = render() + const submitted = () => (props.onSubmit as ReturnType).mock.calls[0]?.[0] as + | NewSessionRequest + | undefined + return { ...view, props, submitted, rerender: view.rerender } +} + +const start = () => userEvent.click(screen.getByRole('button', { name: 'Start session' })) + +describe('NewSessionDialog', () => { + it('submits the ridden machine and nothing else when nothing is typed', async () => { + // The bar this had to clear to be allowed in front of a one-click button: + // opening it and pressing Start must be the old behaviour exactly. + const { submitted } = show({ initial: { machineId: 'local' } }) + + await start() + + expect(submitted()).toEqual({ machineId: 'local', cwd: '', name: '', tags: [] }) + }) + + it('carries a name, a directory and tags', async () => { + const user = userEvent.setup() + const { submitted } = show({ initial: { machineId: 'local' } }) + + await user.type(screen.getByLabelText('Name'), 'deploy') + await user.type(screen.getByLabelText('Directory'), '/srv/app') + await user.type(screen.getByLabelText('Tags'), 'ops{Enter}api{Enter}') + await start() + + expect(submitted()).toEqual({ + machineId: 'local', + cwd: '/srv/app', + name: 'deploy', + tags: ['ops', 'api'], + }) + }) + + it('counts a tag typed but never entered', async () => { + // Somebody who typed a tag and reached straight for Start is done, and a + // dialog that threw the keystroke away would be disagreeing with them in + // silence — the chips are gone before anyone can read what was sent. + const user = userEvent.setup() + const { submitted } = show() + + await user.type(screen.getByLabelText('Tags'), 'staging') + await start() + + expect(submitted()?.tags).toEqual(['staging']) + }) + + it('does not start a session on the Enter that finishes a tag', async () => { + // The tag field sits inside a real form, so its own Enter has to be + // stopped: a set the reader was still assembling is not an answer. + const user = userEvent.setup() + const { props } = show() + + await user.type(screen.getByLabelText('Tags'), 'ops{Enter}') + + expect(props.onSubmit).not.toHaveBeenCalled() + expect(screen.getByRole('button', { name: 'Remove ops' })).toBeTruthy() + }) + + it('submits on Enter from the name field, like every other form', async () => { + const user = userEvent.setup() + const { submitted } = show({ initial: { machineId: 'attic-pi' } }) + + await user.type(screen.getByLabelText('Name'), 'quick{Enter}') + + expect(submitted()).toEqual({ machineId: 'attic-pi', cwd: '', name: 'quick', tags: [] }) + }) + + it('opens on what the press implied, and lets it be edited', async () => { + const user = userEvent.setup() + const { submitted } = show({ + initial: { machineId: 'attic-pi', cwd: '/srv', tags: ['api'] }, + }) + + expect(screen.getByLabelText('Directory')).toHaveProperty('value', '/srv') + + // A prefill and not a decision: the chip is there to be taken off again. + await user.click(screen.getByRole('button', { name: 'Remove api' })) + await start() + + expect(submitted()).toEqual({ machineId: 'attic-pi', cwd: '/srv', name: '', tags: [] }) + }) + + it('offers the fleet’s own tags, minus the ones already chosen', async () => { + const user = userEvent.setup() + const { submitted } = show({ known: ['api', 'ops'], initial: { tags: ['api'] } }) + + expect(screen.queryByRole('button', { name: 'Add api' })).toBeNull() + await user.click(screen.getByRole('button', { name: 'Add ops' })) + await start() + + expect(submitted()?.tags).toEqual(['api', 'ops']) + }) + + it('puts the chosen tags under their field, not above its heading', async () => { + // Leading chips are right for a dialog whose whole subject is tags. Here + // they read as a stray line belonging to the field before them: "No tags + // yet." landed between the Directory input and a heading called Tags. + const user = userEvent.setup() + show() + + await user.type(screen.getByLabelText('Tags'), 'ops{Enter}') + + const field = screen.getByLabelText('Tags') + const chip = screen.getByRole('button', { name: 'Remove ops' }) + expect(field.compareDocumentPosition(chip) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy() + }) + + it('asks which machine only when there is a choice to make', () => { + const { unmount } = show({ machines: [MACHINES[0]!] }) + expect(screen.queryByRole('combobox', { name: 'Machine' })).toBeNull() + unmount() + + show() + expect(screen.getByRole('combobox', { name: 'Machine' })).toBeTruthy() + }) + + it('falls back to the first machine when the press named one that has gone', async () => { + // A heading for a machine that dropped between render and click. Without + // the fallback the trigger renders blank over a form pointing at an id no + // option carries. + const { submitted } = show({ initial: { machineId: 'vanished' } }) + + await start() + + expect(submitted()?.machineId).toBe('local') + }) + + it('takes the machine the fleet has not named yet, once it names it', async () => { + // The terminal screen subscribes to the fleet and the first delivery can + // land after this has rendered. A machine chosen once, at mount, would + // leave the picker empty for good. + const { rerender, submitted, props } = show({ machines: [], initial: { machineId: 'local' } }) + + expect(screen.getByText(/no machine is reachable/i)).toBeTruthy() + + rerender() + await start() + + expect(submitted()?.machineId).toBe('local') + }) + + it('refuses in words when no machine is reachable', async () => { + const { props } = show({ machines: [] }) + + expect(screen.getByRole('button', { name: 'Start session' })).toHaveProperty('disabled', true) + + await start() + + expect(props.onSubmit).not.toHaveBeenCalled() + }) + + it('closes itself after a submit, and on Cancel', async () => { + const { props } = show({ initial: { machineId: 'local' } }) + + await start() + expect(props.onClose).toHaveBeenCalledTimes(1) + + await userEvent.click(screen.getByRole('button', { name: 'Cancel' })) + expect(props.onClose).toHaveBeenCalledTimes(2) + }) +}) diff --git a/web/src/components/new-session-dialog.tsx b/web/src/components/new-session-dialog.tsx new file mode 100644 index 0000000..4db11be --- /dev/null +++ b/web/src/components/new-session-dialog.tsx @@ -0,0 +1,265 @@ +import { useId, useRef, useState, type RefObject } from 'react' + +import { TagField, withTag } from '@/components/tag-field' +import { Button } from '@/components/ui/button' +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '@/components/ui/dialog' +import { Input } from '@/components/ui/input' +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from '@/components/ui/select' +import type { NewSessionRequest } from '@/sessions/new-session' + +/** A machine this dialog may start a session on. */ +export interface NewSessionMachine { + id: string + /** What to call it. Empty falls back to the id, as every other screen does. */ + name: string +} + +export interface NewSessionDialogProps { + open: boolean + /** + * What the press that opened this already implies: the machine of the + * heading, the directory of the terminal underneath, the tag of the group. + * Everything absent opens empty. + */ + initial: Partial + /** The machines that can carry one. An empty list disables the dialog. */ + machines: NewSessionMachine[] + /** Every tag in use across the fleet, offered as one-click additions. */ + known: string[] + /** Called with the whole request. The caller decides where it opens. */ + onSubmit(want: NewSessionRequest): void + /** Dismissal, however it happened, including after a submit. */ + onClose(): void +} + +/** + * The one place a session is asked for. + * + * It exists because of the order the daemon imposes: `spawn` carries no + * metadata, so a name and a tag can only be applied after the session already + * exists. Left to that order, naming a session means starting it, watching a + * terminal come up, going back to the list and renaming the row — and nobody + * does that, so sessions stay called after whatever shell they run. Asking + * first turns the same two round trips into one form: what is typed here is + * carried to the page that starts the session, which applies it the moment + * there is an id to apply it to. + * + * Nothing is required. Every field opens either empty or on what the press + * implied, and submitting all four untouched is exactly the old one-click + * behaviour — which is the bar this had to clear to be allowed in front of it. + * + * The form state lives one component down, inside the content Radix unmounts + * on close, for the reason the rename dialog gives: the next open builds a new + * form over new props, so a name abandoned on one press cannot follow the + * reader to the next. + */ +export function NewSessionDialog({ + open, + initial, + machines, + known, + onSubmit, + onClose, +}: NewSessionDialogProps) { + const field = useRef(null) + + return ( + { + // Escape, the overlay, and the corner X all arrive here as `false`. + // The dialog is controlled by the caller, so this is a request to + // close rather than a closing. + if (!next) onClose() + }} + > + { + // The name, which is the field this dialog exists for. Left to + // itself Radix takes the first thing it can reach, and where that + // lands depends on which prefills happened to render. + event.preventDefault() + field.current?.focus() + }} + > + + New session + + Everything here is optional. The session opens in a tab of its own. + + + { + onSubmit(want) + onClose() + }} + /> + + + ) +} + +/** + * The four fields and the two buttons. + * + * A real form element, so Enter starts the session the way Enter submits + * everywhere else — the browser's own implicit submission, reached identically + * by the button and by the keyboard. The tag field stops its own Enter (see + * TagField), which is what keeps "finish this tag" from meaning "start now". + * + * The machine picker renders only when there is a choice to make. A fleet of + * one is the ordinary case, and a select holding a single option is a control + * that asks a question with one answer. + */ +function NewSessionForm({ + field, + initial, + machines, + known, + onSubmit, + onCancel, +}: { + field: RefObject + initial: Partial + machines: NewSessionMachine[] + known: string[] + onSubmit(want: NewSessionRequest): void + onCancel(): void +}) { + const nameId = useId() + const cwdId = useId() + const machineId = useId() + + const [name, setName] = useState(initial.name ?? '') + const [cwd, setCwd] = useState(initial.cwd ?? '') + const [tags, setTags] = useState(() => [...(initial.tags ?? [])]) + const [draft, setDraft] = useState('') + /** + * The machine the reader picked, or null for "nobody has picked one". + * + * Null rather than a seeded id, so `on` below stays a *derivation* of the + * list rather than a snapshot of it taken once. The list moves: a machine + * can drop while the form is open, and on the terminal screen the fleet's + * first delivery can land after the dialog has already rendered. A seeded + * value would leave the trigger blank and the form pointing at an id no + * option carries. + */ + const [picked, setPicked] = useState(null) + const wanted = picked ?? initial.machineId + const on = machines.find((m) => m.id === wanted)?.id ?? machines[0]?.id ?? '' + + const nothingReachable = machines.length === 0 + + return ( +
{ + event.preventDefault() + if (nothingReachable) return + // The tag field is part of the answer: somebody who typed a tag and + // reached straight for Start is done, and throwing that keystroke away + // on the way out would be disagreeing with them in silence. + onSubmit({ machineId: on, cwd: cwd.trim(), name: name.trim(), tags: withTag(tags, draft) }) + }} + > +
+ + setName(event.target.value)} + /> +
+ +
+ + setCwd(event.target.value)} + className="font-mono" + /> +
+ + {machines.length > 1 && ( +
+ + +
+ )} + + + + {nothingReachable && ( + // Said rather than hidden. The dialog opens from a button that was + // pressed, and a form that silently refused would leave the reader + // pressing Start at a fleet that is not there. +

+ No machine is reachable, so nothing can be started right now. +

+ )} + + + + + + + ) +} diff --git a/web/src/components/session-table.test.tsx b/web/src/components/session-table.test.tsx index 13cbb86..7790c5b 100644 --- a/web/src/components/session-table.test.tsx +++ b/web/src/components/session-table.test.tsx @@ -129,7 +129,11 @@ describe('SessionTable', () => { }) const names = screen.getAllByRole('link').map((a) => a.getAttribute('aria-label')) - expect(names).toEqual(['Open zeta', 'Open alpha', 'Open mid']) + expect(names).toEqual([ + 'Open zeta in a new tab', + 'Open alpha in a new tab', + 'Open mid in a new tab', + ]) }) it('asks for a toggle rather than deciding one', async () => { @@ -170,7 +174,7 @@ describe('SessionTable', () => { // 'name' as always on, whatever the columns preference says. await renderTable({ columns: ['state'] }) - expect(screen.getByRole('link', { name: 'Open zsh' })).toBeTruthy() + expect(screen.getByRole('link', { name: 'Open zsh in a new tab' })).toBeTruthy() expect(screen.getByText('zsh')).toBeTruthy() }) @@ -364,25 +368,28 @@ describe('SessionTable', () => { }) describe('opening', () => { - it('makes the whole row one link to its session', async () => { - const { router } = await renderTable() + it('makes the whole row one link to its session, in a tab of its own', async () => { + await renderTable() - const link = screen.getByRole('link', { name: 'Open zsh' }) + const link = screen.getByRole('link', { name: 'Open zsh in a new tab' }) // A real href on a real anchor: this is what a middle click, a copied // address and a Ctrl/Cmd click all read. expect(link.getAttribute('href')).toBe('/d/m1/s/a1') - - await userEvent.click(link) - expect(router.state.location.pathname).toBe('/d/m1/s/a1') + expect(link.getAttribute('target')).toBe('_blank') + expect(link.getAttribute('rel')).toBe('noopener') }) - it('leaves a modified click to the browser, which owns the new tab', async () => { - // Ctrl/Cmd and middle clicks mean "a new tab" and only the browser can - // honour that. The router must not swallow them — TanStack's Link - // stands aside for a modified click, so the location holds still here - // while a real browser would be opening the terminal beside this tab. + it('leaves the click to the browser, which owns the new tab', async () => { + // A target the router honours by standing aside — TanStack's Link hands + // any click with a target other than _self straight to the browser — so + // the location holds still here while a real browser opens the terminal + // beside this list. Ctrl, Cmd and middle clicks were already the + // browser's, and stay that way. const { router } = await renderTable() - const link = screen.getByRole('link', { name: 'Open zsh' }) + const link = screen.getByRole('link', { name: 'Open zsh in a new tab' }) + + await userEvent.click(link) + expect(router.state.location.pathname).toBe('/sessions') fireEvent.click(link, { ctrlKey: true }) expect(router.state.location.pathname).toBe('/sessions') diff --git a/web/src/components/session-table.tsx b/web/src/components/session-table.tsx index 60069b5..2ee872a 100644 --- a/web/src/components/session-table.tsx +++ b/web/src/components/session-table.tsx @@ -184,7 +184,17 @@ function SessionRow({ diff --git a/web/src/components/tag-editor.tsx b/web/src/components/tag-editor.tsx index 5dfd351..bea03ba 100644 --- a/web/src/components/tag-editor.tsx +++ b/web/src/components/tag-editor.tsx @@ -1,7 +1,6 @@ -import { useId, useRef, useState, type RefObject } from 'react' -import { PlusIcon, XMarkIcon } from '@heroicons/react/16/solid' +import { useRef, useState, type RefObject } from 'react' -import { Badge } from '@/components/ui/badge' +import { TagField, withTag } from '@/components/tag-field' import { Button } from '@/components/ui/button' import { Dialog, @@ -11,7 +10,6 @@ import { DialogHeader, DialogTitle, } from '@/components/ui/dialog' -import { Input } from '@/components/ui/input' export interface TagEditorProps { open: boolean @@ -86,25 +84,6 @@ export function TagEditor({ open, current, known, onSubmit, onClose }: TagEditor ) } -/** - * The set with one more tag in it: trimmed, and unchanged if the tag is blank - * or already there. - * - * Three routes reach this rule and it cannot hold for two of them — a typed - * Enter, a clicked suggestion, and Save over a field the reader never pressed - * Enter on. That third one is the reason this is a function rather than four - * lines inside the first handler that needed them. - * - * The comparison is exact rather than case-folded: `API` and `api` are two - * strings until the daemon says otherwise, and folding them here would drop a - * tag the reader had just watched themselves type. - */ -function withTag(held: string[], tag: string): string[] { - const clean = tag.trim() - if (clean === '' || held.includes(clean)) return held - return [...held, clean] -} - /** * The chips, the field, the suggestions, and the two buttons. * @@ -117,7 +96,8 @@ function withTag(held: string[], tag: string): string[] { * State seeded once from `current` and never resynced: the content this sits * in is unmounted on close, so the seeding happens exactly when the dialog * opens and a set abandoned on one session cannot follow the reader to the - * next. + * next. The draft is held here rather than inside the field for the reason + * Save's own comment gives — it is part of the answer. */ function TagForm({ field, @@ -132,100 +112,19 @@ function TagForm({ onSubmit(tags: string[]): void onCancel(): void }) { - const fieldId = useId() - const suggestionsId = useId() const [tags, setTags] = useState(() => [...current]) const [draft, setDraft] = useState('') - /** A typed Enter and a clicked suggestion, both of which empty the field. */ - const add = (tag: string) => { - setDraft('') - setTags((held) => withTag(held, tag)) - } - - // What the fleet knows, minus what this session already carries, narrowed by - // what has been typed so far. A prefix rather than a substring: a reader - // typing `de` is reaching for a tag they can already half-remember, and - // matching the middle of words would answer with tags they were not naming. - const needle = draft.trim().toLowerCase() - const offered = known.filter( - (tag) => !tags.includes(tag) && tag.toLowerCase().startsWith(needle), - ) - return (
-
- {tags.length === 0 ? ( -

No tags yet.

- ) : ( - tags.map((tag) => ( - /* - The chip is the remove control, which is why its accessible name - says so: the word on screen is the tag, and a reader hearing - only "api, button" would have no idea that pressing it takes the - tag away. The X, and the destructive tint under the pointer, say - the same thing to the eye. - */ - - - - )) - )} -
- -
- - setDraft(event.target.value)} - onKeyDown={(event) => { - if (event.key !== 'Enter') return - // Nothing above would submit on Enter today, but this field will - // one day sit inside something that does. - event.preventDefault() - add(draft) - }} - /> -
- - {offered.length > 0 && ( -
-

- Suggestions -

- {/* Capped in height and scrolled: a fleet with forty tags in it - must not push Save off the bottom of the screen. */} -
- {offered.map((tag) => ( - - ))} -
-
- )} + + + )) + )} +
+ ) + + return ( +
+ {chipsFirst && chips} + +
+ + onDraft(event.target.value)} + onKeyDown={(event) => { + if (event.key !== 'Enter') return + // Stopped whether or not it adds anything. This field sits inside a + // real form in the new-session dialog, and an Enter left to bubble + // would submit that form — starting a session on the keystroke that + // was meant to finish a tag. + event.preventDefault() + add(draft) + }} + /> + {!chipsFirst && chips} +
+ + {offered.length > 0 && ( +
+

+ Suggestions +

+ {/* Capped in height and scrolled: a fleet with forty tags in it must + not push the buttons off the bottom of the screen. */} +
+ {offered.map((tag) => ( + + ))} +
+
+ )} +
+ ) +} diff --git a/web/src/components/terminal.test.tsx b/web/src/components/terminal.test.tsx index a13ea7a..3f8e01a 100644 --- a/web/src/components/terminal.test.tsx +++ b/web/src/components/terminal.test.tsx @@ -1,5 +1,6 @@ import { StrictMode, type ReactNode } from 'react' import { act, fireEvent, render, screen, waitFor } from '@testing-library/react' +import userEvent from '@testing-library/user-event' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { FlueClientProvider } from '@/client/provider' @@ -1658,9 +1659,16 @@ describe('the terminal theme', () => { }) }) -describe('the new-session link', () => { - it('carries the session’s directory and opens a new tab', () => { - const { sock } = mountTerminal((em) => ) +describe('the new-session control', () => { + it('hands this session’s directory up, for whoever owns the form', async () => { + // It used to be a link to `/?cwd=`, which is the dashboard — so a session + // started from a terminal came up behind the whole list. The chip asks + // above it now, and the directory is the one thing only this component + // knows: the list is where a session's cwd arrives. + const onNewSession = vi.fn() + const { sock } = mountTerminal((em) => ( + + )) // Asked for on mount: the list is where the cwd comes from. expect(sock.ofType('list')).toHaveLength(1) @@ -1669,8 +1677,22 @@ describe('the new-session link', () => { sock.emitControl({ type: 'sessions', sessions: [session({ cwd: '/tmp/with space' })] }), ) - const link = screen.getByRole('link', { name: 'New session in this directory' }) - expect(link.getAttribute('href')).toBe(`/?cwd=${encodeURIComponent('/tmp/with space')}`) - expect(link.getAttribute('target')).toBe('_blank') + await userEvent.click(screen.getByRole('button', { name: 'New session in this directory' })) + + expect(onNewSession).toHaveBeenCalledWith('/tmp/with space') + }) + + it('says so rather than guessing when the list has not answered yet', async () => { + // Null, not ''. The form prefills its directory field from this, and an + // empty string there reads as "the reader cleared it" — which is a + // different instruction to the daemon than "nothing is known". + const onNewSession = vi.fn() + mountTerminal((em) => ( + + )) + + await userEvent.click(screen.getByRole('button', { name: 'New session in this directory' })) + + expect(onNewSession).toHaveBeenCalledWith(null) }) }) diff --git a/web/src/components/terminal.tsx b/web/src/components/terminal.tsx index 73d52fb..de6af04 100644 --- a/web/src/components/terminal.tsx +++ b/web/src/components/terminal.tsx @@ -51,6 +51,16 @@ export interface TerminalProps { onRestarted?: (sessionId: string) => void /** Called after Close has closed the dead session; navigate away here. */ onClosed?: () => void + /** + * Called by the `+` in the control strip, with this session's directory when + * the list has said what it is. + * + * The dialog it opens lives above this component, not in it, and that is the + * same bargain the rest of the file keeps: the form needs the fleet's + * machines and the fleet's tags, and a terminal that knew the fleet existed + * would be a terminal that could not be mounted without one. + */ + onNewSession?: (cwd: string | null) => void } /** Named so the test and the markup cannot drift apart. */ @@ -124,6 +134,7 @@ export function Terminal({ createEmulator = createXtermEmulator, onRestarted, onClosed, + onNewSession, }: TerminalProps) { const client = useFlueClient() const switcher = useSwitcher() @@ -883,15 +894,11 @@ export function Terminal({