From 3bc092bf2028f52ac507f1aa81a7b70bcb75b1e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Almeida?= <57680069+zeval@users.noreply.github.com> Date: Sun, 24 May 2026 12:59:30 +0100 Subject: [PATCH] =?UTF-8?q?feat(terminal):=20first-class=20user=20terminal?= =?UTF-8?q?s=20=E2=80=94=20stable=20seq,=20rename,=20park/resume=20(#1009)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(terminal): persist user terminals with stable seq + park state Ordinary user shells now have a DB-backed `user_terminals` row keyed by the agentctl PTY id, carrying a per-task monotonic seq (gaps preserved on delete), custom_name override, and open/parked state. The terminal service owns the create/list/rename/park/resume/destroy surface and guards reject `bottom-panel` (cmd+J) + `script-*` ids so passthrough flows are untouched. Closing a tab now parks (PTY keeps running); destroy kills the PTY and removes the row. * feat(web): expose terminal seq badge, rename, and parked-terminals menu Wires the right-panel strip and SSR page state to the new task-keyed terminals endpoint and discriminated UserShell union, adds the `#N` badge for ordinary terminals, plumbs context-menu / double-click rename, and renders a "Parked" submenu so closed-but-running terminals can be resumed. Closing a managed tab now parks instead of destroying so the PTY (and its buffer) survives accidental closes. * test(terminal): e2e coverage for first-class persistent entities Covers the persistence guarantees the user explicitly called out: monotonic seqs, refresh-survives-rename-and-park, gap-preserving destroy, park+resume cycle, and bottom-panel/script guards. Exercises the new user_shell.{create,list,rename,park,resume,destroy} WS actions through the shared ApiClient.wsRequest helper. * fix(terminal): atomic seq allocation + propagate destroy errors - Repository.Create folds MAX(seq) read + INSERT into a single `INSERT … SELECT` statement so concurrent creates can no longer pick the same seq. Locked in with TestCreate_ConcurrentSeqAllocation (25 parallel creates, all unique). - wsUserShellStop now propagates the service-Destroy error so the frontend stops optimistically removing a row the backend failed to delete. - Service.Destroy / CleanupTask log PTY-stop failures at Warn (instead of silently swallowing) while keeping the proceed-on-failure semantics intentional — the DB row is the source of truth for "user expects this terminal gone". - IsUserShellAlive copies ProcessID under the read lock before calling r.Get(), removing the post-unlock pointer-deref race. - Get-not-found test asserts errors.Is(err, ErrNotFound) so it locks to the repository contract. * fix(web): close-tab waits for backend ack + className undefined guard - handleCloseTab defers `removeTerminal` into the `.then()` of the park/destroy promise. A failed backend call leaves the tab visible rather than vanishing it optimistically; the next `user_shell.list` poll reconciles state. - session-tabs guards `tab.className` with `?? ""` so an undefined value no longer produces "undefined group relative..." in the rendered class string. - user-shells.test simplification: `createSessionRuntimeSlice` only consumes `set`, so the test passes the slice creator straight to `immer()` instead of wrapping it in an adapter with unused args. * fix(terminal): task-scoped mutations + legacy-shell list + UI hardening Round-2 review feedback (cubic P1/P2, claude, coderabbit): Backend: - Rename/Park/Resume/Destroy now require a task_id and verify the terminal belongs to that task via a shared requireOwnership helper. Cross-task mutations return ErrTaskMismatch instead of silently succeeding by raw terminal id. - wsUserShellList no longer drops managed-shaped agentctl shells when the terminal service is disabled or task_id is missing — legacy passthrough callers see their existing shells again. `managedCovered` flag scopes the orphan filter to the DB-backed path. - service_test.go uses the shared-cache in-memory SQLite + MaxOpenConns(1) setup, matching repository_test.go and removing the flake-one-race-away warning. - New cross-task guard tests: TestRename_RejectsCrossTask, TestPark_RejectsCrossTask, TestDestroy_RejectsCrossTask, plus a TestRename_EmptyTaskIDSkipsOwnership for internal callers. Frontend: - updateUserShell patch type narrowed to Partial> so the row key can't be silently mutated. - destroyUserShell now throws on a missing WS client instead of silently resolving — handleCloseTab's .then() no longer fires prematurely. - rename/park/resume/destroy API client + useTerminals hook plumb the active task id through every mutating call. E2E: - New "cross-task mutating ops are rejected" spec creates a terminal under task A and verifies task B can't rename/park/destroy it by raw id. - Existing specs updated to send task_id with their mutating calls. * fix(terminal): require task_id on all mutators + wire taskID into hooks Round-3 review feedback (cubic P1, claude blocker, coderabbit): Backend: - requireOwnership no longer skips the ownership check when taskID is empty. The previous carve-out was reachable from the WS layer (where task_id is user-controlled), so an attacker who guessed a terminal id could omit task_id and mutate any row. CleanupTask walks the repo directly and never touched the bypass. - TestRename_EmptyTaskIDRejected pins the new contract. Frontend (the real bug claude caught): - useUserShells now accepts a taskId parameter and forwards it on the user_shell.list WS call. Without it the backend's DB-backed ordinary- shell path was skipped and the Zustand store wiped the SSR-hydrated panel strip on the first WS fetch. - Response mapping switched to a discriminated union: id (new field) falls back to terminal_id (legacy), and kind/seq/state/pty_status/ display_name/custom_name flow into the store so the parked-terminals submenu actually populates. - useTerminals + useMobileTerminals read the active task id from the store and pass it through to useUserShells. - destroyUserShell now retries with user_shell.stop when the server returns an unknown-action error — keeps the rollout-window compatibility intact for older backends that only know the legacy action name. * fix(terminal): list parked, rollout-safe kind default, helper extraction Round-4 review feedback (claude + greptile P1): - useUserShells now sends include_parked: true so the "Parked terminals" submenu populates after a page reload. Previously the WS list returned only state=open rows; parked terminals were invisible until the user parked something in the same session. The main strip still hides parked entries via buildTerminalsFromShells. - useAddTerminal stops blindly defaulting kind to "ordinary" — that misclassified legacy passthrough payloads (terminal_id without seq) during a rollout where the frontend is newer than the backend. The managed-tab path then called parkUserShell on a server that didn't know the action. Now only tags as ordinary when kind is explicit or the row carries a seq. - handleCloseTab extracted into a standalone useCloseTab + helpers hoisted into use-terminals-build.ts / use-terminals-types.ts. Brings use-terminals.ts back under the 600-line file limit and the useTerminalActions wrapper inside the 100-line function limit. * fix(terminal): scope-keyed shell cache + null-task guard + tighter rollout default Round-5 review feedback (cubic P1, coderabbit x4): - useUserShells now keys its fetch cache on env+task (`${env}:${task}`), not just env. An earlier load without a task id (e.g. before the active task hydrated) no longer pins the store on a legacy-only shell list and blocks the task-scoped refetch. - useMobileTerminals delays auto-create until a real taskID is available. Previously a null taskID could bootstrap a legacy non-persistent shell, mark the env as auto-created, and starve the real task of its first-class terminal. Guard key is now env:task, and releaseAutoCreatedEnvironment clears every scope on that env. - useAddTerminal tightens the rollout check: a row is "ordinary" only when `seq !== undefined`. A bare `kind: "ordinary"` without seq from an older backend stays legacy so close → destroy on the first click. - useRemoveTerminal reads the current activeTab from a ref instead of closing over the value captured when handleCloseTab first ran. Async park/destroy resolves after a user tab-switch no longer clobber the newer selection with the stale fallback shift. Tests: - New "waits for taskID before auto-creating — null taskID is a no-op" case in use-mobile-terminals.test.ts confirms the null-task guard. - All existing mobile-terminal + slice + E2E tests pass. * fix(terminal): live activeTab lookup + selectable-tab guard Round-6 review feedback (cubic P2 x2): - getActiveTab now reads the active tab from the Zustand store via storeApi.getState() instead of a useRef/useEffect pair. Closes the post-render race where an async close handler could resolve between the new render's commit and the effect-driven ref sync. - computeTerminalTabValue requires activeTab to be selectable (present in `terminals` or === "commands") before honoring it. A stale id left over from a parked/destroyed terminal no longer pins the panel on a nonexistent tab and blocks the fallback shift to a real shell. * chore(terminal): inline savedTabFromStorage to drop file under line cap Round-7 review feedback (claude suggestion): the useMemo around the synchronous getSessionStorage call was zero-value memoisation that pushed use-terminals.ts three lines over the 600-line max-lines limit. Inlined; file now at 600 lines. * chore(terminal): drop savedTabExists local + comment to land under 600 lines Round-8 follow-up (claude): the previous commit's two-line comment block held the file at 603 lines (still over the 600-line cap). Inline savedTabExists at the only call site and remove the redundant comment — the round-7 PR thread already explains why the useMemo was removed. File now at 599 lines. * fix(terminal): plumb task_id through dockview + tolerate legacy destroys Manual-test feedback: clicking X on a dockview terminal panel surfaced "destroy terminal: terminal does not belong to the supplied task" because the dockview path was creating + destroying terminals without sending task_id at all. The new requireOwnership guard then correctly rejected the empty task_id against a non-empty term.TaskID. Frontend: - addTerminalPanel grows task_id + title parameters; stamps both into panel params at creation time so the destroy cleanup can pass them back to user_shell.destroy / .stop. - Every dockview-side createUserShell call site now passes the active task_id, so the backend creates a DB-backed ordinary terminal (with seq + displayName) instead of a legacy passthrough shell. - destroyUserShell + stopUserShell already accepted an optional task_id; dockview-layout-setup.onDidRemovePanel now reads the stamped task id from panel params and forwards it. Backend: - wsUserShellStop now requires both terminalSvc and a non-empty req.TaskID before routing through the managed service. An empty task_id falls through to the legacy passthrough StopUserShell so older clients (and dockview panels persisted before task_id was stamped) keep working. Known follow-ups (separate task): the #N badge, rename context menu, and parked-terminals dropdown still only show on the mobile/tablet right-panel strip. Desktop dockview shows the seq inside the panel title text but doesn't expose the badge/rename/parked-list UI yet. * feat(terminal): dockview tab badges + rename context menu + existing-terminals list Manual-test feedback round 2: - Custom dockview tab component (TerminalTab) renders the #N pill only when the active env has more than one ordinary terminal — mirrors the session-tab badge semantics. Right-click on any ordinary terminal tab now exposes Rename / Park / Destroy. - Dockview + menu picks up a new "Terminals" section listing every ordinary terminal for the active task (open and parked). Clicking a row focuses the existing dockview panel if one is mounted, otherwise re-opens it; parked rows are auto-resumed via user_shell.resume before the panel is added. The base "Terminal" action was renamed to "New Terminal" so the existing-terminals list is distinguishable in the menu. - Reopen menu uses a stable EMPTY_SHELLS sentinel for its store selector to avoid Zustand infinite re-renders when the env has no shells. E2E: - terminal-new-tab.spec uses getByRole("menuitem", { name: "New Terminal", exact: true }) to disambiguate from the new "Terminals" submenu label. - Full terminal suite still passes (26/26). * fix(terminal): tab title shows "Terminal" with seq as badge, populate store on session page Re-reading the user feedback: the title text was rendering as "Terminal 1" (backend's displayName), and the dockview "+" menu's Terminals section was empty because nothing on desktop was calling useUserShells to populate the store. - TerminalTab.pickDisplayName now returns the literal "Terminal" for ordinary terminals (custom names still override). The seq lives only in the sibling badge — same split as session-tab where the agent name is the title and the seq is a pill before it. Reading "Terminal 1" out of "[1] Terminal" instead of "[1] Terminal 1". - LeftHeaderActions (dockview "+") now calls useUserShells on mount. Without this the desktop layout never triggered the WS list fetch, so the Terminals submenu and the badge "count > 1" check both saw an empty store and silently disappeared. - TerminalReopenMenuItems also calls useUserShells defensively (cheap thanks to scope-keyed cache) so it works when mounted in isolation. * feat(terminal): "New X" rows live inside their section labels User feedback: move "New Agent" under the Agents section header and "New Terminal" under the Terminals section header so the dockview "+" menu reads top-to-bottom as Agents → New Agent → existing sessions → Terminals → New Terminal → existing terminals → other panels. - SessionReopenMenuItems accepts an optional onNewSession prop and renders the "New Agent" row as the first item under the "Agents" label. Render gate now keeps the section visible whenever the create handler is supplied, even with zero existing sessions. - TerminalReopenMenuItems gets a matching onNewTerminal prop with the same behaviour ("New Terminal" row under the "Terminals" label). - dockview-add-panel-items drops its standalone "New Agent" and "New Terminal" rows since both now live inside their sections. * feat(terminal): auto-migrate default panel to DB-backed ordinary terminal The legacy `terminal-default` dockview panel was hard-wired to a non-managed `shell-default` PTY so it never picked up an ordinary seq/customName/parked state — the new "Terminal N" badge UX could only show a badge once the user had explicitly created a second terminal. With useEnsureDefaultTerminalOrdinary the default panel is upgraded into a real `shell-` row on session-page mount, so every visible tab is now a first-class ordinary terminal. Plus a UI-level e2e spec asserting the multi-terminal title + badge contract, and an expectation bump on the existing rename+park spec since visiting the session page now mints 1 extra ordinary terminal. * fix(terminal): park ordinary terminals on tab close so they survive refresh Dockview's tab X close button routed straight to stopUserShell (an alias for destroyUserShell), wiping the DB row on close — after a refresh the user had no way to resume a still-running PTY they had just tucked away. The X is now a "hide for now" affordance: ordinary terminals get parked (PTY survives, row stays in DB, reappears in the "+" → Terminals menu with a `parked` pill), while scripts / bottom-panel / legacy passthrough shells stay on the destroy path because they have no resume story. Permanent delete still lives on the right-click Destroy menu item and (in the next commit) on each reopen-menu row. * fix(terminal): preserve DB-backed terminal panel ids across layout restore The serializer's normalizePanel rewrote every user-created terminal panel id to `terminal-saved-N` and dropped environmentId/taskID from params, so after a reload that panel couldn't be matched back to its live PTY or its store record — the tab rendered with no badge and a stale fallback title. Treat `shell-` panel ids the same way we already treat `terminal-default`: preserve id and params on restore. Legacy ephemeral `terminal-saved-*` ids still mint a fresh PTY on restore. Also force tabComponent=terminalTab and title="Terminal" for any restored terminal panel so layouts persisted before the badge UX land on the new presentation. * feat(terminal): destroy button on reopen-menu rows for permanent delete Destroy used to live only on the tab's right-click menu — useless once the tab was already closed. Each ordinary terminal row in the dockview "+" → Terminals submenu now ends with an × button that confirms-then calls destroyUserShell, killing the PTY and dropping the DB row so the terminal does not reappear after reload. Pairs with the previous close-to-park change: closing the tab parks (soft, resumable), the × button on the menu row destroys (hard, permanent). * fix(terminal): reopen-menu focuses existing tab, drops parked pill, says Terminate Three small fixes to the dockview "+" → Terminals submenu: 1. Clicking a row whose terminal is already mounted now FOCUSES the existing panel instead of duplicating it. The default-migrated panel keeps its registry id `terminal-default` even after the migration to a `shell-` PTY, so a direct getPanel(uuid) miss would fall through to addTerminalPanel and create a second tab for the same PTY. Fall back to scanning `params.terminalId` so the lookup catches that case. 2. Drop the "parked" pill from the row — parking is now an internal detail (close = park) that the user doesn't need to read off the menu. Clicking the row brings the terminal back into view either way, so distinguishing it visually is noise. 3. Rename the row × tooltip + confirm copy from "Destroy" / "Permanently delete" to "Terminate" to match the new tab-context-menu label. Spec: add a test that asserts no duplicate tab when reopening an already-open terminal, and drop the parked-pill assertion from the existing close+reload test. * feat(terminal): empty-group watermark uses the rich + menu When a user splits a group (split-right / split-down) the new half is empty and dockview renders the watermark. Before this, the watermark showed a hard-coded Agent/Plan/Terminal/Browser row and clicking "Terminal" called createUserShell without a taskId — so it spawned a legacy passthrough shell that never appeared in the user-shell list, and clicking "Agent" only ever focused the singleton chat panel (invisible if the task had multiple sessions). The watermark now renders the same AddPanelMenuItems content the header "+" uses, so the user gets: - The Agents list (existing sessions, greyed when already open) - The Terminals list (existing terminals, greyed when already open) - The Browser/Plan/VS Code/Changes/Files/PR options …served behind a single "Add panel" dropdown trigger centered on the empty group. * feat(terminal): inline rename on the tab; drop Park, rename Destroy → Terminate The tab title is now editable in place — picking Rename from the right-click context menu swaps the title for an input. Enter commits, Escape / blur-with-no-change cancels, and the new name is pushed through renameUserShell + the user-shell store so the API and badge list stay in sync. Replaces the previous window.prompt which felt out of place in the dockview chrome. While here: - Drop the "Park (hide, keep PTY)" context-menu item. Tab close already parks ordinary terminals, so the explicit Park action was redundant and broke the "one obvious affordance per outcome" rule. - Rename "Destroy" → "Terminate" in the context menu. Matches the reopen-menu × tooltip and reads more like a system action than a destructive UI primitive. Spec adds an inline-rename test asserting the input appears, accepts typed text, and commits the new title to the tab. * fix(terminal): right-click Terminate closes the tab; drop displayName DOM warning Two unrelated fixes both landing in terminal-tab.tsx: - Terminate used to fire the WS destroy and update the store, but left the dockview panel mounted with a dead PTY underneath. Now it optimistically drops the shell row from the store, closes the panel via api.close(), then sends the destroy — same effect as the X close but routed to the hard-destroy path (the onDidRemovePanel cleanup sees no shell entry and skips its park branch). - TerminalTabBody typed `displayName` as a prop but never destructured it, so {...props} spread it into DockviewDefaultTab and on into a DOM div, triggering "React does not recognize the `displayName` prop on a DOM element". Drop it from the spread. * feat(terminal): polish + menu — badge-style indices, flush-left New rows, terminate closes panel Polish pass on the dockview "+" → Agents / Terminals submenus: - Replace the plain `#N` text with the same pill-style badge the tabs use (small bg, rounded, 11px). Agents and terminals both pick this up — so the menu row reads "[N] label" matching the tab style. - Drop the empty index-column placeholder span on the "New X" rows so the icon + label sit flush-left under the section header instead of awkwardly indented. - Strip the backend "Terminal {seq}" suffix from row labels — the badge already carries the number, so showing "Terminal 1" next to the "1" pill duplicated it. Custom-renamed terminals still show their name. - The row × button now also closes the dockview panel if the terminal is currently open, mirroring the right-click → Terminate flow. Spec: switch the row-by-seq lookups from `hasText: "#N"` (now stale since the `#` prefix is gone) to a `has:` filter against the new `reopen-terminal-seq-N` badge testid, and add a regression test for right-click → Terminate closing the tab and removing the row in one gesture. * fix: address terminal review feedback * fix: resolve terminal review suggestions * fix: catch terminal close stop failures * fix: harden terminal terminate failures * fix: use pinned pnpm docker arg * fix: satisfy backend lint after main merge * fix: stabilize e2e after layout merge --- Dockerfile | 3 + apps/backend/cmd/kandev/gateway.go | 68 ++- apps/backend/cmd/kandev/main.go | 7 +- apps/backend/cmd/kandev/storage.go | 7 + apps/backend/cmd/kandev/types.go | 7 + .../internal/agent/handlers/shell_handlers.go | 406 +++++++++++++-- .../server/process/interactive_shells.go | 92 ++-- .../server/process/runner_shells_test.go | 62 +-- apps/backend/internal/automation/handlers.go | 2 +- apps/backend/internal/automation/models.go | 15 + apps/backend/internal/automation/scheduler.go | 10 +- apps/backend/internal/automation/store.go | 28 +- .../internal/automation/trigger_registry.go | 18 +- apps/backend/internal/automation/webhook.go | 16 +- .../backend/internal/gitlab/client_helpers.go | 26 +- apps/backend/internal/gitlab/constants.go | 12 + apps/backend/internal/gitlab/controller.go | 46 +- apps/backend/internal/gitlab/factory.go | 6 +- apps/backend/internal/gitlab/service.go | 2 +- apps/backend/internal/jira/cloud_client.go | 4 +- .../orchestrator/event_handlers_automation.go | 20 +- .../executor/executor_interaction.go | 7 +- .../internal/terminal/models/terminal.go | 49 ++ .../internal/terminal/repository/sqlite.go | 184 +++++++ .../terminal/repository/sqlite_test.go | 278 ++++++++++ .../internal/terminal/service/ptybackend.go | 51 ++ .../internal/terminal/service/service.go | 254 +++++++++ .../internal/terminal/service/service_test.go | 311 +++++++++++ apps/backend/internal/user/service/service.go | 7 +- apps/backend/pkg/websocket/actions.go | 10 +- apps/web/components/session-tabs.tsx | 22 +- .../task/dockview-add-panel-items.tsx | 27 +- .../task/dockview-desktop-layout.tsx | 2 + .../task/dockview-header-actions.tsx | 72 ++- .../components/task/dockview-layout-setup.ts | 124 +++-- apps/web/components/task/dockview-shared.tsx | 2 + .../components/task/dockview-watermark.tsx | 279 +++++----- .../task/mobile/session-tablet-layout.tsx | 2 +- .../components/task/parked-terminals-menu.tsx | 103 ++++ .../components/task/session-reopen-menu.tsx | 39 +- apps/web/components/task/task-right-panel.tsx | 126 ++++- .../components/task/terminal-reopen-menu.tsx | 257 +++++++++ apps/web/components/task/terminal-tab.tsx | 316 +++++++++++ apps/web/e2e/pages/session-page.ts | 2 +- .../dockview-repository-scripts.spec.ts | 2 +- .../e2e/tests/task/session-isolation.spec.ts | 10 +- .../terminal/terminal-dockview-ui.spec.ts | 426 +++++++++++++++ .../terminal/terminal-first-class.spec.ts | 279 ++++++++++ .../tests/terminal/terminal-new-tab.spec.ts | 4 +- .../use-ensure-default-terminal-ordinary.ts | 126 +++++ .../session/use-mobile-terminals.test.ts | 16 + .../domains/session/use-mobile-terminals.ts | 64 ++- .../domains/session/use-terminals-build.ts | 122 +++++ .../domains/session/use-terminals-types.ts | 25 + .../hooks/domains/session/use-terminals.ts | 490 ++++++++++++------ .../hooks/domains/session/use-user-shells.ts | 142 +++-- apps/web/hooks/use-editor-keybinds.ts | 25 +- apps/web/lib/api/domains/user-shell-api.ts | 223 ++++++-- apps/web/lib/ssr/session-page-state.ts | 67 ++- apps/web/lib/state/dockview-panel-actions.ts | 24 +- apps/web/lib/state/dockview-store.ts | 8 +- .../web/lib/state/layout-manager/constants.ts | 6 + .../lib/state/layout-manager/serializer.ts | 43 +- apps/web/lib/state/slices/index.ts | 3 + .../session-runtime/session-runtime-slice.ts | 13 + .../lib/state/slices/session-runtime/types.ts | 44 +- .../session-runtime/user-shells.test.ts | 73 +++ apps/web/lib/state/store.ts | 5 + 68 files changed, 4874 insertions(+), 747 deletions(-) create mode 100644 apps/backend/internal/terminal/models/terminal.go create mode 100644 apps/backend/internal/terminal/repository/sqlite.go create mode 100644 apps/backend/internal/terminal/repository/sqlite_test.go create mode 100644 apps/backend/internal/terminal/service/ptybackend.go create mode 100644 apps/backend/internal/terminal/service/service.go create mode 100644 apps/backend/internal/terminal/service/service_test.go create mode 100644 apps/web/components/task/parked-terminals-menu.tsx create mode 100644 apps/web/components/task/terminal-reopen-menu.tsx create mode 100644 apps/web/components/task/terminal-tab.tsx create mode 100644 apps/web/e2e/tests/terminal/terminal-dockview-ui.spec.ts create mode 100644 apps/web/e2e/tests/terminal/terminal-first-class.spec.ts create mode 100644 apps/web/hooks/domains/session/use-ensure-default-terminal-ordinary.ts create mode 100644 apps/web/hooks/domains/session/use-terminals-build.ts create mode 100644 apps/web/hooks/domains/session/use-terminals-types.ts create mode 100644 apps/web/lib/state/slices/session-runtime/user-shells.test.ts diff --git a/Dockerfile b/Dockerfile index c486db94d3..8092dab1c8 100644 --- a/Dockerfile +++ b/Dockerfile @@ -18,6 +18,9 @@ ARG PNPM_VERSION=9.15.9 FROM node:24-bookworm-slim +ARG PNPM_VERSION +LABEL org.opencontainers.image.pnpm-version="${PNPM_VERSION}" + # Install runtime dependencies. gh is included because the GitHub integration # (PR review, webhooks) shells out to it for auth fallback when GITHUB_TOKEN # is not set. apprise is installed via pipx for notification fan-out. diff --git a/apps/backend/cmd/kandev/gateway.go b/apps/backend/cmd/kandev/gateway.go index d48d425c7f..0875b8901d 100644 --- a/apps/backend/cmd/kandev/gateway.go +++ b/apps/backend/cmd/kandev/gateway.go @@ -24,6 +24,8 @@ import ( orchestratorhandlers "github.com/kandev/kandev/internal/orchestrator/handlers" sqliterepo "github.com/kandev/kandev/internal/task/repository/sqlite" taskservice "github.com/kandev/kandev/internal/task/service" + terminalrepo "github.com/kandev/kandev/internal/terminal/repository" + terminalservice "github.com/kandev/kandev/internal/terminal/service" userservice "github.com/kandev/kandev/internal/user/service" ws "github.com/kandev/kandev/pkg/websocket" ) @@ -105,12 +107,24 @@ func provideGateway( agentRegistry *registry.Registry, notificationRepo notificationstore.Repository, taskRepo *sqliterepo.Repository, + terminalRepo *terminalrepo.Repository, githubSvc *github.Service, dataDir string, -) (*gateways.Gateway, *notificationservice.Service, *notificationcontroller.Controller, error) { +) (*gateways.Gateway, *notificationservice.Service, *notificationcontroller.Controller, *terminalservice.Service, error) { gateway, err := gateways.Provide(log) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err + } + + // Terminal service — DB-backed first-class user terminals. Backend is + // nil-safe when no interactive runner is wired (remote-executor mode). + var terminalSvc *terminalservice.Service + if terminalRepo != nil { + var ptyBackend terminalservice.PTYBackend + if lifecycleMgr != nil { + ptyBackend = terminalservice.NewInteractiveRunnerBackend(lifecycleMgr.GetInteractiveRunner()) + } + terminalSvc = terminalservice.New(terminalRepo, ptyBackend, log) } // Enable dedicated terminal WebSocket for passthrough mode @@ -139,6 +153,9 @@ func provideGateway( workspaceFileHandlers.RegisterHandlers(gateway.Dispatcher) shellHandlers := agenthandlers.NewShellHandlers(lifecycleMgr, scriptSvc, log) + if terminalSvc != nil { + shellHandlers.SetTerminalService(terminalSvc) + } shellHandlers.RegisterHandlers(gateway.Dispatcher) gitHandlers := agenthandlers.NewGitHandlers(lifecycleMgr, &sessionReaderAdapter{repo: taskRepo, logger: log}, log) @@ -263,5 +280,50 @@ func provideGateway( } } - return gateway, notificationSvc, notificationCtrl, nil + // Wire the task-lifecycle cascade: terminals are PTYs and DB rows; + // destroying the task should destroy them too. Mirrors the GitHub PR + // watch / handoff cascade pattern (see CLAUDE.md "Task lifecycle + // events"). + if terminalSvc != nil && eventBus != nil { + subscribeTerminalCleanup(ctx, eventBus, terminalSvc, log) + } + + return gateway, notificationSvc, notificationCtrl, terminalSvc, nil +} + +// subscribeTerminalCleanup wires the terminal service to task.deleted / +// task.archived (where archived_at is set). It cleans up PTYs and DB rows +// for the task. +func subscribeTerminalCleanup(ctx context.Context, eventBus bus.EventBus, svc *terminalservice.Service, log *logger.Logger) { + handler := func(eventCtx context.Context, event *bus.Event) error { + if event == nil { + return nil + } + data, ok := event.Data.(map[string]interface{}) + if !ok { + return nil + } + taskID, _ := data["task_id"].(string) + if taskID == "" { + return nil + } + archivedAt, _ := data["archived_at"].(string) + if event.Type == events.TaskUpdated && archivedAt == "" { + // Plain update — only react on archive transitions. + return nil + } + if _, err := svc.CleanupTask(eventCtx, taskID); err != nil { + log.Warn("terminal cleanup on task event", + zap.String("task_id", taskID), + zap.String("event", event.Type), + zap.Error(err)) + } + return nil + } + if _, err := eventBus.Subscribe(events.TaskDeleted, handler); err != nil { + log.Error("subscribe terminal cleanup (deleted)", zap.Error(err)) + } + if _, err := eventBus.Subscribe(events.TaskUpdated, handler); err != nil { + log.Error("subscribe terminal cleanup (updated/archived)", zap.Error(err)) + } } diff --git a/apps/backend/cmd/kandev/main.go b/apps/backend/cmd/kandev/main.go index 74d43cfc23..bce979b0f4 100644 --- a/apps/backend/cmd/kandev/main.go +++ b/apps/backend/cmd/kandev/main.go @@ -505,12 +505,15 @@ func startGatewayAndServe( // WEBSOCKET GATEWAY // ============================================ log.Info("Initializing WebSocket Gateway...") - gateway, _, notificationCtrl, err := provideGateway( + gateway, _, notificationCtrl, terminalSvc, err := provideGateway( ctx, log, eventBus, services.Task, services.User, orchestratorSvc, lifecycleMgr, agentRegistry, - repos.Notification, repos.Task, services.GitHub, + repos.Notification, repos.Task, repos.Terminal, services.GitHub, cfg.ResolvedHomeDir(), ) + if terminalSvc != nil { + services.Terminal = terminalSvc + } if err != nil { log.Error("Failed to initialize WebSocket gateway", zap.Error(err)) return false diff --git a/apps/backend/cmd/kandev/storage.go b/apps/backend/cmd/kandev/storage.go index bc673dd761..33c062442c 100644 --- a/apps/backend/cmd/kandev/storage.go +++ b/apps/backend/cmd/kandev/storage.go @@ -13,6 +13,7 @@ import ( "github.com/kandev/kandev/internal/persistence" "github.com/kandev/kandev/internal/secrets" "github.com/kandev/kandev/internal/task/repository" + terminalrepo "github.com/kandev/kandev/internal/terminal/repository" utilitystore "github.com/kandev/kandev/internal/utility/store" workflowrepository "github.com/kandev/kandev/internal/workflow/repository" @@ -72,6 +73,11 @@ func provideRepositories(cfg *config.Config, log *logger.Logger, version string) } cleanups = append(cleanups, officeCleanup) + terminalRepoImpl, err := terminalrepo.NewWithDB(writer, reader, log) + if err != nil { + return nil, nil, nil, fmt.Errorf("terminal repo: %w", err) + } + masterKeyProvider, err := secrets.NewMasterKeyProvider(cfg.ResolvedDataDir()) if err != nil { return nil, nil, nil, fmt.Errorf("master key: %w", err) @@ -98,6 +104,7 @@ func provideRepositories(cfg *config.Config, log *logger.Logger, version string) Workflow: workflowRepo, Secrets: secretStore, Office: officeRepo, + Terminal: terminalRepoImpl, } return pool, repos, cleanups, nil } diff --git a/apps/backend/cmd/kandev/types.go b/apps/backend/cmd/kandev/types.go index b89210a081..5b36974060 100644 --- a/apps/backend/cmd/kandev/types.go +++ b/apps/backend/cmd/kandev/types.go @@ -22,6 +22,8 @@ import ( sqliterepo "github.com/kandev/kandev/internal/task/repository/sqlite" taskservice "github.com/kandev/kandev/internal/task/service" "github.com/kandev/kandev/internal/task/share" + terminalrepo "github.com/kandev/kandev/internal/terminal/repository" + terminalservice "github.com/kandev/kandev/internal/terminal/service" userservice "github.com/kandev/kandev/internal/user/service" userstore "github.com/kandev/kandev/internal/user/store" utilityservice "github.com/kandev/kandev/internal/utility/service" @@ -43,6 +45,7 @@ type Repositories struct { Workflow *workflowrepository.Repository Secrets secrets.SecretStore Office *officesqlite.Repository + Terminal *terminalrepo.Repository } type Services struct { @@ -68,6 +71,10 @@ type Services struct { // WorktreeMgr is the worktree manager. Exposed so the office GC can // consult it as the authoritative inventory of live worktrees. WorktreeMgr *worktree.Manager + // Terminal is the first-class user-terminal service (rename, park, etc.). + // Wired into the gateway once lifecycle.Manager is up so the PTY backend + // is available. + Terminal *terminalservice.Service // Automation is the trigger-based automation subsystem (cron, GitHub PR // events, webhooks). Independent of Office — has its own scheduler and // creates tasks via the task service. diff --git a/apps/backend/internal/agent/handlers/shell_handlers.go b/apps/backend/internal/agent/handlers/shell_handlers.go index 2a28a9b92e..cc462d3c38 100644 --- a/apps/backend/internal/agent/handlers/shell_handlers.go +++ b/apps/backend/internal/agent/handlers/shell_handlers.go @@ -11,8 +11,10 @@ import ( "github.com/gin-gonic/gin" "github.com/google/uuid" "github.com/kandev/kandev/internal/agent/runtime/lifecycle" + "github.com/kandev/kandev/internal/agentctl/server/process" "github.com/kandev/kandev/internal/common/logger" "github.com/kandev/kandev/internal/common/scripts" + terminalservice "github.com/kandev/kandev/internal/terminal/service" v1 "github.com/kandev/kandev/pkg/api/v1" ws "github.com/kandev/kandev/pkg/websocket" "go.uber.org/zap" @@ -22,6 +24,21 @@ import ( // task_environment_id on user-shell RPCs. const errTaskEnvIDRequired = "task_environment_id is required" +// JSON keys shared by user_shell.* responses (extracted so the goconst +// linter doesn't complain about literal duplication across handlers). +const ( + shellResponseKindField = "kind" + shellResponseTerminalIDField = "terminal_id" + shellResponseLabelField = "label" + shellResponseClosableField = "closable" + shellResponseInitialCmdField = "initial_command" +) + +// shellKindScript is the discriminator value for script terminals (id +// prefix `script-`). Bottom-panel is "fixed"; ordinary uses the constant +// from internal/terminal/service. +const shellKindScript = "script" + // ShellHandlers provides WebSocket handlers for shell terminal operations. // // User-shell ops (user_shell.list/create/stop) are env-keyed: they take @@ -34,6 +51,13 @@ type ShellHandlers struct { lifecycleMgr *lifecycle.Manager scriptService scripts.ScriptService logger *logger.Logger + // terminalSvc, when set, owns the ordinary user-terminal path + // (DB-backed, sticky, renameable). Non-ordinary ids — bottom-panel and + // script-* — keep going through the original agentctl-direct code + // regardless. Wired via SetTerminalService once the storage layer is + // up; left nil in unit tests, where the legacy in-memory runner path + // is exercised directly. + terminalSvc *terminalservice.Service } // NewShellHandlers creates a new ShellHandlers instance @@ -45,6 +69,13 @@ func NewShellHandlers(lifecycleMgr *lifecycle.Manager, scriptService scripts.Scr } } +// SetTerminalService injects the terminal service after construction. Done +// post-hoc so the storage-layer wiring (which creates the service) can run +// after handler construction without breaking existing test wiring. +func (h *ShellHandlers) SetTerminalService(svc *terminalservice.Service) { + h.terminalSvc = svc +} + // RegisterHandlers registers shell handlers with the WebSocket dispatcher func (h *ShellHandlers) RegisterHandlers(d *ws.Dispatcher) { d.RegisterFunc(ws.ActionShellStatus, h.wsShellStatus) @@ -53,6 +84,10 @@ func (h *ShellHandlers) RegisterHandlers(d *ws.Dispatcher) { d.RegisterFunc(ws.ActionUserShellList, h.wsUserShellList) d.RegisterFunc(ws.ActionUserShellCreate, h.wsUserShellCreate) d.RegisterFunc(ws.ActionUserShellStop, h.wsUserShellStop) + d.RegisterFunc(ws.ActionUserShellDestroy, h.wsUserShellStop) + d.RegisterFunc(ws.ActionUserShellRename, h.wsUserShellRename) + d.RegisterFunc(ws.ActionUserShellPark, h.wsUserShellPark) + d.RegisterFunc(ws.ActionUserShellResume, h.wsUserShellResume) } // ShellStatusRequest for shell.status action @@ -235,47 +270,160 @@ func ensureShellExecution(ctx context.Context, lifecycleMgr shellLifecycle, log } // UserShellListRequest for user_shell.list action. -// User shells are env-scoped — sessions in the same task share one shell list. +// +// task_environment_id is required (agentctl scope key). task_id is +// required when the terminal service is wired — only ordinary user +// terminals are listed from the DB, so the service needs a task to scope +// the query. Tests that exercise the legacy direct-runner path leave +// task_id empty. +// +// include_parked toggles whether parked tabs are returned (default false, +// strip view); the "Parked terminals" submenu sends true. type UserShellListRequest struct { + TaskID string `json:"task_id"` TaskEnvironmentID string `json:"task_environment_id"` + IncludeParked bool `json:"include_parked"` } -// wsUserShellList returns all running user shells for a task environment -func (h *ShellHandlers) wsUserShellList(_ context.Context, msg *ws.Message) (*ws.Message, error) { +// userShellListItem is the wire-shape this handler returns. Discriminated +// by Kind: "ordinary" items carry seq + display_name + state; "fixed" +// (bottom-panel) and "script" items carry just the id + pty_status so the +// frontend can keep dispatching them unchanged. +type userShellListItem struct { + ID string `json:"id"` + Kind string `json:"kind"` + Seq int `json:"seq,omitempty"` + DisplayName string `json:"display_name,omitempty"` + CustomName *string `json:"custom_name,omitempty"` + State string `json:"state,omitempty"` + PTYStatus string `json:"pty_status"` + Label string `json:"label,omitempty"` + InitialCommand string `json:"initial_command,omitempty"` +} + +// wsUserShellList resolves the union of (DB ordinary terminals) and +// (agentctl non-managed terminals) for a task. +func (h *ShellHandlers) wsUserShellList(ctx context.Context, msg *ws.Message) (*ws.Message, error) { var req UserShellListRequest if err := msg.ParsePayload(&req); err != nil { return nil, fmt.Errorf("invalid payload: %w", err) } - if req.TaskEnvironmentID == "" { return nil, errors.New(errTaskEnvIDRequired) } - interactiveRunner := h.lifecycleMgr.GetInteractiveRunner() - if interactiveRunner == nil { - return ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{ - "shells": []interface{}{}, - }) + items := []userShellListItem{} + + // Ordinary terminals — backed by the terminal service when wired. + if h.terminalSvc != nil && req.TaskID != "" { + ordinary, err := h.terminalSvc.List(ctx, req.TaskID, req.IncludeParked) + if err != nil { + return nil, fmt.Errorf("list ordinary terminals: %w", err) + } + for _, it := range ordinary { + items = append(items, userShellListItem{ + ID: it.ID, + Kind: it.Kind, + Seq: it.Seq, + DisplayName: it.DisplayName, + CustomName: it.CustomName, + State: it.State, + PTYStatus: it.PTYStatus, + InitialCommand: it.InitialCommand, + }) + } } - shells := interactiveRunner.ListUserShells(req.TaskEnvironmentID) + // Non-managed terminals — passthrough straight from agentctl. Filters + // out anything an ordinary row already covers so a managed id never + // appears twice. + // When the terminal service is wired and a task scope was supplied, + // managed-shaped orphans are dropped (no DB row → not user-visible). + // Without those preconditions we're on the legacy passthrough path, + // so any agentctl-side shell stays visible regardless of id shape. + managedCovered := h.terminalSvc != nil && req.TaskID != "" + items = appendUnmanagedShells(items, h.lifecycleMgr.GetInteractiveRunner(), req.TaskEnvironmentID, managedCovered) h.logger.Debug("listing user shells", + zap.String("task_id", req.TaskID), zap.String("task_environment_id", req.TaskEnvironmentID), - zap.Int("count", len(shells))) + zap.Int("count", len(items))) return ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{ - "shells": shells, + "shells": items, }) } +// appendUnmanagedShells merges agentctl's view of unmanaged terminals +// (bottom-panel + scripts + orphans) into the items list. Skips ids the +// terminal service already returned (`seen` set). +// +// managedCovered=true means the caller already loaded ordinary terminals +// from the service for this task — so any managed-shaped id we see here +// without a matching DB row is an orphan and should be silently dropped +// (plan §7). +// +// managedCovered=false means we're on the legacy passthrough path +// (terminal service disabled or task_id missing). In that case every +// agentctl-side shell stays visible regardless of id shape — otherwise +// older clients lose their previously created shells. +func appendUnmanagedShells( + items []userShellListItem, + runner *process.InteractiveRunner, + scopeID string, + managedCovered bool, +) []userShellListItem { + if runner == nil || scopeID == "" { + return items + } + seen := make(map[string]struct{}, len(items)) + for _, it := range items { + seen[it.ID] = struct{}{} + } + for _, s := range runner.ListUserShells(scopeID) { + if _, dup := seen[s.TerminalID]; dup { + continue + } + if managedCovered && terminalservice.IsManaged(s.TerminalID) { + // Orphan ordinary shell (no DB row) — drop silently per + // plan §7; it'll be GC'd next agentctl restart. + continue + } + items = append(items, userShellListItem{ + ID: s.TerminalID, + Kind: kindForUnmanaged(s.TerminalID), + Label: s.Label, + PTYStatus: ptyStatusFromRunning(s.Running), + InitialCommand: s.InitialCommand, + }) + } + return items +} + +func kindForUnmanaged(id string) string { + if id == terminalservice.BottomPanelID { + return "fixed" + } + return shellKindScript +} + +func ptyStatusFromRunning(running bool) string { + if running { + return terminalservice.PTYStatusRunning + } + return terminalservice.PTYStatusStopped +} + // UserShellCreateRequest for user_shell.create action. // Exactly one of (ScriptID) or (Command) may be set; if both are empty the -// handler creates a plain shell. When Command is set, Label is used as the -// terminal label (defaults to "Script" when omitted). +// handler creates a plain ordinary shell (DB-backed when the terminal +// service is wired). When Command is set, the result is always a script +// terminal (passthrough to agentctl). // -// User shells are env-scoped — TaskEnvironmentID is required. +// TaskEnvironmentID is the agentctl scope; TaskID is needed for the DB +// row on ordinary creates. type UserShellCreateRequest struct { + TaskID string `json:"task_id"` TaskEnvironmentID string `json:"task_environment_id"` ScriptID string `json:"script_id,omitempty"` Command string `json:"command,omitempty"` @@ -293,10 +441,6 @@ func (h *ShellHandlers) wsUserShellCreate(ctx context.Context, msg *ws.Message) } interactiveRunner := h.lifecycleMgr.GetInteractiveRunner() - if interactiveRunner == nil { - return nil, fmt.Errorf("interactive runner not available") - } - label, command, err := h.resolveShellScript(ctx, &req) if err != nil { return nil, err @@ -304,7 +448,11 @@ func (h *ShellHandlers) wsUserShellCreate(ctx context.Context, msg *ws.Message) scopeID := req.TaskEnvironmentID + // Script terminal — always passthrough to agentctl, never DB-backed. if command != "" { + if interactiveRunner == nil { + return nil, fmt.Errorf("interactive runner not available") + } terminalID := "script-" + uuid.New().String() interactiveRunner.RegisterScriptShell(scopeID, terminalID, label, command) h.logger.Info("created script terminal", @@ -313,24 +461,50 @@ func (h *ShellHandlers) wsUserShellCreate(ctx context.Context, msg *ws.Message) zap.String("label", label), zap.String("initial_command", command)) return ws.NewResponse(msg.ID, msg.Action, map[string]any{ - "terminal_id": terminalID, - "label": label, - "closable": true, - "initial_command": command, + shellResponseTerminalIDField: terminalID, + shellResponseKindField: shellKindScript, + shellResponseLabelField: label, + shellResponseClosableField: true, + shellResponseInitialCmdField: command, }) } + // Plain ordinary shell — DB-backed when the terminal service is wired. + if h.terminalSvc != nil && req.TaskID != "" { + term, err := h.terminalSvc.Create(ctx, req.TaskID, scopeID, "") + if err != nil { + return nil, fmt.Errorf("create ordinary terminal: %w", err) + } + h.logger.Info("created ordinary terminal", + zap.String("task_id", req.TaskID), + zap.String("task_environment_id", scopeID), + zap.String("terminal_id", term.ID), + zap.Int("seq", term.Seq)) + return ws.NewResponse(msg.ID, msg.Action, map[string]any{ + shellResponseTerminalIDField: term.ID, + shellResponseKindField: terminalservice.KindOrdinary, + "seq": term.Seq, + "display_name": term.DisplayName(), + "state": string(term.State), + "pty_status": terminalservice.PTYStatusStopped, + }) + } + + // Legacy path — used by older clients that don't send task_id, and by + // unit tests that exercise the in-memory runner directly. + if interactiveRunner == nil { + return nil, fmt.Errorf("interactive runner not available") + } result := interactiveRunner.CreateUserShell(scopeID) - h.logger.Info("created user shell", + h.logger.Info("created user shell (legacy path)", zap.String("task_environment_id", scopeID), - zap.String("terminal_id", result.TerminalID), - zap.String("label", result.Label), - zap.Bool("closable", result.Closable)) + zap.String("terminal_id", result.TerminalID)) return ws.NewResponse(msg.ID, msg.Action, map[string]any{ - "terminal_id": result.TerminalID, - "label": result.Label, - "closable": result.Closable, - "initial_command": "", + shellResponseTerminalIDField: result.TerminalID, + shellResponseKindField: shellKindScript, + shellResponseLabelField: result.Label, + shellResponseClosableField: result.Closable, + shellResponseInitialCmdField: "", }) } @@ -364,11 +538,15 @@ func (h *ShellHandlers) resolveShellScript( // UserShellStopRequest for user_shell.stop action. // User shells are env-scoped — TaskEnvironmentID is required. type UserShellStopRequest struct { + TaskID string `json:"task_id"` TaskEnvironmentID string `json:"task_environment_id"` TerminalID string `json:"terminal_id"` } -// wsUserShellStop stops a user shell terminal process +// wsUserShellStop stops a user shell terminal process. Also serves +// user_shell.destroy — semantically identical for managed terminals: stop +// the PTY and delete the DB row. For non-managed ids (bottom-panel, +// script-*) only the PTY is stopped. func (h *ShellHandlers) wsUserShellStop(ctx context.Context, msg *ws.Message) (*ws.Message, error) { var req UserShellStopRequest if err := msg.ParsePayload(&req); err != nil { @@ -382,38 +560,135 @@ func (h *ShellHandlers) wsUserShellStop(ctx context.Context, msg *ws.Message) (* return nil, fmt.Errorf("terminal_id is required") } - // Get the interactive runner + // Managed terminal — route through the service so the DB row is + // deleted alongside the PTY tear-down. Destroy errors propagate so + // the frontend doesn't optimistically remove a row that the backend + // failed to delete. + // + // task_id is required for ordinary terminals. Older clients (and + // legacy dockview panels persisted before task_id was stamped) may + // still send empty task_id and a `shell-…` id that matches IsManaged. + // For those, fall through to the passthrough stop so legacy callers + // keep working — the service rejects with ErrTaskMismatch on empty + // task_id, which we treat as "this isn't a DB-backed terminal, + // behave like the old code did". + if h.terminalSvc != nil && req.TaskID != "" && terminalservice.IsManaged(req.TerminalID) { + if err := h.terminalSvc.Destroy(ctx, req.TaskID, req.TerminalID); err != nil { + h.logger.Warn("destroy ordinary terminal", + zap.String("terminal_id", req.TerminalID), zap.Error(err)) + return nil, fmt.Errorf("destroy terminal: %w", err) + } + return ws.NewResponse(msg.ID, msg.Action, map[string]any{"success": true}) + } + + // Non-managed, no service wired, or legacy caller without task_id — + // best-effort PTY tear-down via the runner. interactiveRunner := h.lifecycleMgr.GetInteractiveRunner() if interactiveRunner == nil { return nil, fmt.Errorf("interactive runner not available") } - if err := interactiveRunner.StopUserShell(ctx, req.TaskEnvironmentID, req.TerminalID); err != nil { h.logger.Warn("failed to stop user shell", zap.String("task_environment_id", req.TaskEnvironmentID), zap.String("terminal_id", req.TerminalID), zap.Error(err)) - // Don't return error - shell may already be stopped } + return ws.NewResponse(msg.ID, msg.Action, map[string]any{"success": true}) +} - h.logger.Info("user shell stopped", - zap.String("task_environment_id", req.TaskEnvironmentID), - zap.String("terminal_id", req.TerminalID)) +// UserShellRenameRequest for user_shell.rename. CustomName==nil clears. +// TaskID is verified against the terminal's owning task on the service +// side; cross-task renames return ErrTaskMismatch. +type UserShellRenameRequest struct { + TaskID string `json:"task_id"` + TerminalID string `json:"terminal_id"` + CustomName *string `json:"custom_name"` +} - return ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{ - "success": true, - }) +func (h *ShellHandlers) wsUserShellRename(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + var req UserShellRenameRequest + if err := msg.ParsePayload(&req); err != nil { + return nil, fmt.Errorf("invalid payload: %w", err) + } + if req.TerminalID == "" { + return nil, fmt.Errorf("terminal_id is required") + } + if h.terminalSvc == nil { + return nil, fmt.Errorf("terminal service not available") + } + if err := h.terminalSvc.Rename(ctx, req.TaskID, req.TerminalID, req.CustomName); err != nil { + return nil, err + } + return ws.NewResponse(msg.ID, msg.Action, map[string]any{"success": true}) +} + +// UserShellStateRequest shares its shape between park and resume. +// TaskID is verified against the terminal's owning task on the service +// side; cross-task mutations return ErrTaskMismatch. +type UserShellStateRequest struct { + TaskID string `json:"task_id"` + TerminalID string `json:"terminal_id"` +} + +func (h *ShellHandlers) wsUserShellPark(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + var req UserShellStateRequest + if err := msg.ParsePayload(&req); err != nil { + return nil, fmt.Errorf("invalid payload: %w", err) + } + if req.TerminalID == "" { + return nil, fmt.Errorf("terminal_id is required") + } + if h.terminalSvc == nil { + return nil, fmt.Errorf("terminal service not available") + } + if err := h.terminalSvc.Park(ctx, req.TaskID, req.TerminalID); err != nil { + return nil, err + } + return ws.NewResponse(msg.ID, msg.Action, map[string]any{"success": true}) +} + +func (h *ShellHandlers) wsUserShellResume(ctx context.Context, msg *ws.Message) (*ws.Message, error) { + var req UserShellStateRequest + if err := msg.ParsePayload(&req); err != nil { + return nil, fmt.Errorf("invalid payload: %w", err) + } + if req.TerminalID == "" { + return nil, fmt.Errorf("terminal_id is required") + } + if h.terminalSvc == nil { + return nil, fmt.Errorf("terminal service not available") + } + if err := h.terminalSvc.Resume(ctx, req.TaskID, req.TerminalID); err != nil { + return nil, err + } + return ws.NewResponse(msg.ID, msg.Action, map[string]any{"success": true}) } // RegisterShellRoutes registers HTTP routes for shell operations. // This is separate from RegisterHandlers which registers WebSocket handlers. +// +// The handlers value returned by gateway.go is the same instance whose +// terminalSvc gets set later via SetTerminalService — so we accept it here +// rather than constructing a fresh one. Backwards-compat: when called +// without a pre-built handler (helpers.go boot path), the legacy +// runner-only fallback still works because the SSR endpoint degrades +// gracefully when terminalSvc is nil. func RegisterShellRoutes(router *gin.Engine, lifecycleMgr *lifecycle.Manager, log *logger.Logger) { handlers := NewShellHandlers(lifecycleMgr, nil, log) - api := router.Group("/api/v1") - api.GET("/environments/:id/terminals", handlers.httpListTerminals) + RegisterShellRoutesOn(router, handlers) } -// httpListTerminals returns all terminals for a task environment (for SSR). +// RegisterShellRoutesOn binds the SSR routes on an existing ShellHandlers +// instance — used in gateway wiring where the handler is already created +// and the terminal service has been injected. +func RegisterShellRoutesOn(router *gin.Engine, handlers *ShellHandlers) { + api := router.Group("/api/v1") + api.GET("/environments/:id/terminals", handlers.httpListTerminals) + api.GET("/tasks/:id/terminals", handlers.httpListTaskTerminals) +} + +// httpListTerminals (env-keyed legacy SSR) — kept for backwards-compat with +// callers that don't yet have a task id at SSR time. func (h *ShellHandlers) httpListTerminals(c *gin.Context) { environmentID := c.Param("id") if environmentID == "" { @@ -421,7 +696,6 @@ func (h *ShellHandlers) httpListTerminals(c *gin.Context) { return } - // Get the interactive runner interactiveRunner := h.lifecycleMgr.GetInteractiveRunner() if interactiveRunner == nil { c.JSON(http.StatusOK, gin.H{"terminals": []interface{}{}}) @@ -429,12 +703,6 @@ func (h *ShellHandlers) httpListTerminals(c *gin.Context) { } shells := interactiveRunner.ListUserShells(environmentID) - - h.logger.Debug("listing terminals via HTTP", - zap.String("task_environment_id", environmentID), - zap.Int("count", len(shells))) - - // Transform to response format terminals := make([]map[string]interface{}, len(shells)) for i, shell := range shells { terminals[i] = map[string]interface{}{ @@ -446,6 +714,40 @@ func (h *ShellHandlers) httpListTerminals(c *gin.Context) { "initial_command": shell.InitialCommand, } } - c.JSON(http.StatusOK, gin.H{"terminals": terminals}) } + +// httpListTaskTerminals — new SSR endpoint keyed by task_id. Returns the +// same union shape the WS user_shell.list returns. +func (h *ShellHandlers) httpListTaskTerminals(c *gin.Context) { + taskID := c.Param("id") + envID := c.Query("task_environment_id") + if taskID == "" { + c.JSON(http.StatusBadRequest, gin.H{"error": "task_id is required"}) + return + } + + items := []userShellListItem{} + if h.terminalSvc != nil { + ordinary, err := h.terminalSvc.List(c.Request.Context(), taskID, false) + if err != nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + return + } + for _, it := range ordinary { + items = append(items, userShellListItem{ + ID: it.ID, + Kind: it.Kind, + Seq: it.Seq, + DisplayName: it.DisplayName, + CustomName: it.CustomName, + State: it.State, + PTYStatus: it.PTYStatus, + InitialCommand: it.InitialCommand, + }) + } + } + items = appendUnmanagedShells(items, h.lifecycleMgr.GetInteractiveRunner(), envID, h.terminalSvc != nil) + + c.JSON(http.StatusOK, gin.H{"terminals": items}) +} diff --git a/apps/backend/internal/agentctl/server/process/interactive_shells.go b/apps/backend/internal/agentctl/server/process/interactive_shells.go index 9e2911b924..13a3900c90 100644 --- a/apps/backend/internal/agentctl/server/process/interactive_shells.go +++ b/apps/backend/internal/agentctl/server/process/interactive_shells.go @@ -199,66 +199,41 @@ func (r *InteractiveRunner) StartUserShell(ctx context.Context, scopeID, process return info, nil } -// ListUserShells returns all user shells for a scope, sorted by creation time. -// If no plain shell terminals exist, automatically creates the first "Terminal" entry. +// ListUserShells returns every user shell registered for the scope, sorted +// by creation time. The list includes script terminals, the hardcoded +// bottom-panel entry (if it exists), and any orphan ordinary shells the +// agent has registered. +// +// Auto-creating the first "Terminal" entry is no longer this function's +// job — the terminal service (internal/terminal/service) owns that policy +// now and inserts a DB row alongside the agentctl-side entry. This keeps +// "what's in the panel" derivable from a single source of truth. func (r *InteractiveRunner) ListUserShells(scopeID string) []UserShellInfo { - r.userShellsMu.Lock() - defer r.userShellsMu.Unlock() + r.userShellsMu.RLock() + defer r.userShellsMu.RUnlock() prefix := scopeID + ":" var shells []UserShellInfo - hasPlainShell := false - for key, entry := range r.userShells { - if strings.HasPrefix(key, prefix) { - terminalID := key[len(prefix):] - // Check if process is still alive - _, running := r.Get(entry.ProcessID, false) - shells = append(shells, UserShellInfo{ - TerminalID: terminalID, - ProcessID: entry.ProcessID, - Running: running, - Label: entry.Label, - Closable: entry.Closable, - InitialCommand: entry.InitialCommand, - CreatedAt: entry.CreatedAt, - }) - // Check if this is a plain shell (not a script terminal) - if entry.InitialCommand == "" { - hasPlainShell = true - } + if !strings.HasPrefix(key, prefix) { + continue } - } - - // Auto-create the first "Terminal" if no plain shells exist - if !hasPlainShell { - terminalID := "shell-" + uuid.New().String() - now := time.Now().UTC() - entry := &userShellEntry{ - ProcessID: "", // No process yet - will be started when WebSocket connects - Label: "Terminal", - InitialCommand: "", - Closable: false, // First terminal is not closable - CreatedAt: now, - } - r.userShells[prefix+terminalID] = entry - + terminalID := key[len(prefix):] + _, running := r.Get(entry.ProcessID, false) shells = append(shells, UserShellInfo{ TerminalID: terminalID, - ProcessID: "", - Running: false, - Label: "Terminal", - Closable: false, - InitialCommand: "", - CreatedAt: now, + ProcessID: entry.ProcessID, + Running: running, + Label: entry.Label, + Closable: entry.Closable, + InitialCommand: entry.InitialCommand, + CreatedAt: entry.CreatedAt, }) } - // Sort by creation time for stable ordering sort.Slice(shells, func(i, j int) bool { return shells[i].CreatedAt.Before(shells[j].CreatedAt) }) - return shells } @@ -285,6 +260,31 @@ func (r *InteractiveRunner) StopUserShell(ctx context.Context, scopeID, terminal return r.Stop(ctx, entry.ProcessID) } +// IsUserShellAlive reports whether the PTY behind (scopeID, terminalID) is +// currently running. Returns false when the entry doesn't exist, the entry +// has no process id (lazy-start hasn't happened yet), or the process has +// since exited. +// +// ProcessID is read under the read lock so the subsequent r.Get call +// doesn't race with a concurrent shell-start path that mutates the entry +// pointer in place. +func (r *InteractiveRunner) IsUserShellAlive(scopeID, terminalID string) bool { + key := scopeID + ":" + terminalID + + r.userShellsMu.RLock() + var processID string + if entry, exists := r.userShells[key]; exists { + processID = entry.ProcessID + } + r.userShellsMu.RUnlock() + + if processID == "" { + return false + } + _, running := r.Get(processID, false) + return running +} + // ResizeUserShell resizes the PTY for a user shell. func (r *InteractiveRunner) ResizeUserShell(scopeID, terminalID string, cols, rows uint16) error { key := scopeID + ":" + terminalID diff --git a/apps/backend/internal/agentctl/server/process/runner_shells_test.go b/apps/backend/internal/agentctl/server/process/runner_shells_test.go index 3a6f032b3f..1cc1023251 100644 --- a/apps/backend/internal/agentctl/server/process/runner_shells_test.go +++ b/apps/backend/internal/agentctl/server/process/runner_shells_test.go @@ -263,25 +263,18 @@ func TestInteractiveRunner_RegisterScriptShell(t *testing.T) { runner.RegisterScriptShell("session-1", "script-abc", "npm start", "npm run start") - // Should appear in list + // Auto-create-first-shell was removed; the script registration is the + // only entry the runner now knows about for this scope. shells := runner.ListUserShells("session-1") - - // Should have 2: auto-created "Terminal" + registered script - if len(shells) != 2 { - t.Fatalf("ListUserShells() returned %d shells, want 2", len(shells)) + if len(shells) != 1 { + t.Fatalf("ListUserShells() returned %d shells, want 1", len(shells)) } - // Find the script shell - var scriptShell *UserShellInfo - for i := range shells { - if shells[i].TerminalID == "script-abc" { - scriptShell = &shells[i] - break - } + scriptShell := &shells[0] + if scriptShell.TerminalID != "script-abc" { + t.Fatalf("script shell terminalID = %q, want script-abc", scriptShell.TerminalID) } - if scriptShell == nil { - t.Fatal("script shell not found in list") - } else { + { if scriptShell.Label != "npm start" { t.Errorf("script shell Label = %q, want 'npm start'", scriptShell.Label) } @@ -314,44 +307,15 @@ func TestInteractiveRunner_RegisterScriptShell_DoesNotAffectShellCount(t *testin } } -func TestInteractiveRunner_ListUserShells_AutoCreatesFirst(t *testing.T) { +func TestInteractiveRunner_ListUserShells_EmptyByDefault(t *testing.T) { log := newTestLogger(t) runner := NewInteractiveRunner(nil, log, 2*1024*1024) - // First call should auto-create "Terminal" + // Auto-create-first-shell was removed; an untouched scope returns nothing. + // Backend's terminal service owns the auto-create policy now. shells := runner.ListUserShells("session-1") - - if len(shells) != 1 { - t.Fatalf("ListUserShells() returned %d shells, want 1", len(shells)) - } - if shells[0].Label != "Terminal" { - t.Errorf("auto-created shell Label = %q, want 'Terminal'", shells[0].Label) - } - if shells[0].Closable { - t.Error("auto-created first shell should not be closable") - } - if shells[0].Running { - t.Error("auto-created shell should not be running (no process)") - } - if !strings.HasPrefix(shells[0].TerminalID, "shell-") { - t.Errorf("auto-created shell TerminalID = %q, want prefix 'shell-'", shells[0].TerminalID) - } -} - -func TestInteractiveRunner_ListUserShells_StableAfterAutoCreate(t *testing.T) { - log := newTestLogger(t) - runner := NewInteractiveRunner(nil, log, 2*1024*1024) - - // First call auto-creates - shells1 := runner.ListUserShells("session-1") - // Second call should return the same list (not create another) - shells2 := runner.ListUserShells("session-1") - - if len(shells1) != 1 || len(shells2) != 1 { - t.Fatalf("ListUserShells() should return 1 shell each time, got %d and %d", len(shells1), len(shells2)) - } - if shells1[0].TerminalID != shells2[0].TerminalID { - t.Error("ListUserShells() should return the same terminal ID across calls") + if len(shells) != 0 { + t.Fatalf("ListUserShells() returned %d shells, want 0", len(shells)) } } diff --git a/apps/backend/internal/automation/handlers.go b/apps/backend/internal/automation/handlers.go index 011b7a2cd1..4fb6db4cb6 100644 --- a/apps/backend/internal/automation/handlers.go +++ b/apps/backend/internal/automation/handlers.go @@ -167,7 +167,7 @@ func wsManualTrigger(svc *Service, log *logger.Logger) func(ctx context.Context, if err != nil || a == nil { return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeNotFound, "automation not found", nil) } - data, _ := json.Marshal(map[string]string{"source": "manual"}) + data, _ := json.Marshal(map[string]string{triggerDataSourceKey: triggerDataSourceManual}) triggerID := "" if len(a.Triggers) > 0 { triggerID = a.Triggers[0].ID diff --git a/apps/backend/internal/automation/models.go b/apps/backend/internal/automation/models.go index e33df8fcf4..2aa44a031c 100644 --- a/apps/backend/internal/automation/models.go +++ b/apps/backend/internal/automation/models.go @@ -21,6 +21,21 @@ const ( TriggerTypeWebhook TriggerType = "webhook" ) +const ( + defaultBranchMain = "main" + exampleRepositoryOwner = "org/repo" + placeholderRepositoryOwner = "Repository (owner/name)" + triggerCategoryGitHub = "github" + triggerDataSourceKey = "source" + triggerDataSourceManual = "manual" + triggerCronHourlyShorthand = "@hourly" + triggerCronHourlyExpression = "0 * * * *" + triggerCronDailyShorthand = "@daily" + triggerCronDailyExpression = "0 0 * * *" + triggerCronWeeklyShorthand = "@weekly" + triggerCronWeeklyExpression = "0 0 * * 0" +) + // RunStatus tracks the outcome of a trigger firing. type RunStatus string diff --git a/apps/backend/internal/automation/scheduler.go b/apps/backend/internal/automation/scheduler.go index acd04e72ec..0ba991588d 100644 --- a/apps/backend/internal/automation/scheduler.go +++ b/apps/backend/internal/automation/scheduler.go @@ -131,8 +131,8 @@ func (cs *CronScheduler) shouldFire(t *AutomationTrigger, now time.Time) bool { func (cs *CronScheduler) fire(ctx context.Context, t *AutomationTrigger, now time.Time) { data, _ := json.Marshal(map[string]string{ - "source": "scheduled", - "timestamp": now.Format(time.RFC3339), + triggerDataSourceKey: string(TriggerTypeScheduled), + "timestamp": now.Format(time.RFC3339), }) dedupKey := fmt.Sprintf("scheduled:%s:%d", t.ID, now.Unix()/60) // Dedup by minute @@ -163,11 +163,11 @@ func parseCronShorthand(expr string) (time.Duration, bool) { } // Common presets. switch expr { - case "@hourly", "0 * * * *": + case triggerCronHourlyShorthand, triggerCronHourlyExpression: return time.Hour, true - case "@daily", "0 0 * * *": + case triggerCronDailyShorthand, triggerCronDailyExpression: return 24 * time.Hour, true - case "@weekly", "0 0 * * 0": + case triggerCronWeeklyShorthand, triggerCronWeeklyExpression: return 7 * 24 * time.Hour, true } return 0, false diff --git a/apps/backend/internal/automation/store.go b/apps/backend/internal/automation/store.go index e7c0f435d9..f70823195e 100644 --- a/apps/backend/internal/automation/store.go +++ b/apps/backend/internal/automation/store.go @@ -210,6 +210,22 @@ func (s *Store) UpdateAutomation(ctx context.Context, id string, req *UpdateAuto if a == nil { return fmt.Errorf("automation not found: %s", id) } + applyAutomationUpdate(a, req) + a.UpdatedAt = time.Now().UTC() + _, err = s.db.ExecContext(ctx, ` + UPDATE automations SET name = ?, description = ?, workflow_id = ?, workflow_step_id = ?, + agent_profile_id = ?, executor_profile_id = ?, repository_id = ?, + prompt = ?, task_title_template = ?, + execution_mode = ?, enabled = ?, max_concurrent_runs = ?, updated_at = ? + WHERE id = ?`, + a.Name, a.Description, a.WorkflowID, a.WorkflowStepID, + a.AgentProfileID, a.ExecutorProfileID, a.RepositoryID, + a.Prompt, a.TaskTitleTemplate, + string(a.ExecutionMode), a.Enabled, a.MaxConcurrentRuns, a.UpdatedAt, id) + return err +} + +func applyAutomationUpdate(a *Automation, req *UpdateAutomationRequest) { if req.Name != nil { a.Name = *req.Name } @@ -249,18 +265,6 @@ func (s *Store) UpdateAutomation(ctx context.Context, id string, req *UpdateAuto if !a.ExecutionMode.Valid() { a.ExecutionMode = ExecutionModeTask } - a.UpdatedAt = time.Now().UTC() - _, err = s.db.ExecContext(ctx, ` - UPDATE automations SET name = ?, description = ?, workflow_id = ?, workflow_step_id = ?, - agent_profile_id = ?, executor_profile_id = ?, repository_id = ?, - prompt = ?, task_title_template = ?, - execution_mode = ?, enabled = ?, max_concurrent_runs = ?, updated_at = ? - WHERE id = ?`, - a.Name, a.Description, a.WorkflowID, a.WorkflowStepID, - a.AgentProfileID, a.ExecutorProfileID, a.RepositoryID, - a.Prompt, a.TaskTitleTemplate, - string(a.ExecutionMode), a.Enabled, a.MaxConcurrentRuns, a.UpdatedAt, id) - return err } // DeleteAutomation removes an automation and its triggers/runs (CASCADE). diff --git a/apps/backend/internal/automation/trigger_registry.go b/apps/backend/internal/automation/trigger_registry.go index 9acfcfcd4f..221be760ed 100644 --- a/apps/backend/internal/automation/trigger_registry.go +++ b/apps/backend/internal/automation/trigger_registry.go @@ -25,7 +25,7 @@ type TriggerTypeInfo struct { // Common placeholders available for every trigger type. var commonPlaceholders = []PlaceholderInfo{ - {Key: "trigger.type", Description: "Trigger type (scheduled, github_pr, webhook, etc.)", Example: "scheduled"}, + {Key: "trigger.type", Description: "Trigger type (scheduled, github_pr, webhook, etc.)", Example: string(TriggerTypeScheduled)}, {Key: "trigger.timestamp", Description: "When the trigger fired", Example: "2026-03-08T12:00:00Z"}, {Key: "data.*", Description: "Access any field from trigger data", Example: "data.action"}, } @@ -46,16 +46,16 @@ var triggerTypeRegistry = []TriggerTypeInfo{ Type: TriggerTypeGitHubPR, Label: "New pull requests", Description: "Polls GitHub for PRs matching your filters (repo, branch, author, labels)", - Category: "github", + Category: triggerCategoryGitHub, Enabled: true, Placeholders: append([]PlaceholderInfo{ {Key: "pr.number", Description: "Pull request number", Example: "42"}, {Key: "pr.title", Description: "Pull request title", Example: "Fix the bug"}, {Key: "pr.url", Description: "Pull request URL", Example: "https://github.com/org/repo/pull/42"}, {Key: "pr.author", Description: "PR author login", Example: "alice"}, - {Key: "pr.repo", Description: "Repository (owner/name)", Example: "org/repo"}, + {Key: "pr.repo", Description: placeholderRepositoryOwner, Example: exampleRepositoryOwner}, {Key: "pr.branch", Description: "Head branch name", Example: "fix-bug"}, - {Key: "pr.base_branch", Description: "Base branch name", Example: "main"}, + {Key: "pr.base_branch", Description: "Base branch name", Example: defaultBranchMain}, {Key: "pr.body", Description: "Pull request body text", Example: "Fixes #123"}, }, commonPlaceholders...), DefaultPrompt: "Review PR #{{pr.number}} in {{pr.repo}}: {{pr.title}}\n\n{{pr.body}}\n\nBranch: {{pr.branch}} → {{pr.base_branch}}", @@ -66,11 +66,11 @@ var triggerTypeRegistry = []TriggerTypeInfo{ Type: TriggerTypeGitHubPush, Label: "Push to branch", Description: "Triggers when commits are pushed to matching branches", - Category: "github", + Category: triggerCategoryGitHub, Enabled: false, Placeholders: append([]PlaceholderInfo{ - {Key: "push.branch", Description: "Branch that was pushed to", Example: "main"}, - {Key: "push.repo", Description: "Repository (owner/name)", Example: "org/repo"}, + {Key: "push.branch", Description: "Branch that was pushed to", Example: defaultBranchMain}, + {Key: "push.repo", Description: placeholderRepositoryOwner, Example: exampleRepositoryOwner}, {Key: "push.sha", Description: "Commit SHA", Example: "abc1234"}, {Key: "push.message", Description: "Commit message", Example: "feat: add feature"}, }, commonPlaceholders...), @@ -82,12 +82,12 @@ var triggerTypeRegistry = []TriggerTypeInfo{ Type: TriggerTypeGitHubCI, Label: "CI check result", Description: "Triggers when a CI check completes with matching conclusion", - Category: "github", + Category: triggerCategoryGitHub, Enabled: false, Placeholders: append([]PlaceholderInfo{ {Key: "ci.check_name", Description: "CI check/workflow name", Example: "build"}, {Key: "ci.conclusion", Description: "CI conclusion", Example: "failure"}, - {Key: "ci.repo", Description: "Repository (owner/name)", Example: "org/repo"}, + {Key: "ci.repo", Description: placeholderRepositoryOwner, Example: exampleRepositoryOwner}, {Key: "ci.url", Description: "CI run URL", Example: "https://github.com/..."}, }, commonPlaceholders...), DefaultPrompt: "CI check '{{ci.check_name}}' {{ci.conclusion}} in {{ci.repo}}\n\n{{ci.url}}", diff --git a/apps/backend/internal/automation/webhook.go b/apps/backend/internal/automation/webhook.go index 3bbafe7277..c2d033a497 100644 --- a/apps/backend/internal/automation/webhook.go +++ b/apps/backend/internal/automation/webhook.go @@ -12,6 +12,8 @@ import ( "github.com/kandev/kandev/internal/common/logger" ) +const responseErrorKey = "error" + // WebhookHandler handles incoming webhook requests that fire automation triggers. type WebhookHandler struct { svc *Service @@ -28,17 +30,17 @@ func NewWebhookHandler(svc *Service, log *logger.Logger) *WebhookHandler { func (h *WebhookHandler) Handle(c *gin.Context) { automationID := c.Param("id") if automationID == "" { - c.JSON(http.StatusBadRequest, gin.H{"error": "automation id required"}) + c.JSON(http.StatusBadRequest, gin.H{responseErrorKey: "automation id required"}) return } a, err := h.svc.GetAutomation(c.Request.Context(), automationID) if err != nil || a == nil { - c.JSON(http.StatusNotFound, gin.H{"error": "automation not found"}) + c.JSON(http.StatusNotFound, gin.H{responseErrorKey: "automation not found"}) return } if !a.Enabled { - c.JSON(http.StatusConflict, gin.H{"error": "automation is disabled"}) + c.JSON(http.StatusConflict, gin.H{responseErrorKey: "automation is disabled"}) return } @@ -47,7 +49,7 @@ func (h *WebhookHandler) Handle(c *gin.Context) { // by byte from timing differences. secret := c.GetHeader("X-Webhook-Secret") if subtle.ConstantTimeCompare([]byte(secret), []byte(a.WebhookSecret)) != 1 { - c.JSON(http.StatusUnauthorized, gin.H{"error": "invalid webhook secret"}) + c.JSON(http.StatusUnauthorized, gin.H{responseErrorKey: "invalid webhook secret"}) return } @@ -61,14 +63,14 @@ func (h *WebhookHandler) Handle(c *gin.Context) { } } if triggerID == "" { - c.JSON(http.StatusConflict, gin.H{"error": "no enabled webhook trigger"}) + c.JSON(http.StatusConflict, gin.H{responseErrorKey: "no enabled webhook trigger"}) return } // Read body as trigger data. body, readErr := io.ReadAll(io.LimitReader(c.Request.Body, 1<<20)) // 1MB limit if readErr != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "failed to read body"}) + c.JSON(http.StatusBadRequest, gin.H{responseErrorKey: "failed to read body"}) return } @@ -82,7 +84,7 @@ func (h *WebhookHandler) Handle(c *gin.Context) { h.logger.Error("failed to fire webhook trigger", zap.String("automation_id", automationID), zap.Error(fireErr)) - c.JSON(http.StatusInternalServerError, gin.H{"error": "trigger failed"}) + c.JSON(http.StatusInternalServerError, gin.H{responseErrorKey: "trigger failed"}) return } diff --git a/apps/backend/internal/gitlab/client_helpers.go b/apps/backend/internal/gitlab/client_helpers.go index 637b25b0b6..1df821501d 100644 --- a/apps/backend/internal/gitlab/client_helpers.go +++ b/apps/backend/internal/gitlab/client_helpers.go @@ -272,9 +272,9 @@ const mrStateOpen = "open" // alone so the UI shares the GitHub vocabulary. func normalizeMRState(state string) string { switch state { - case "opened": + case gitlabStateOpened: return mrStateOpen - case "merged", "closed", "locked": + case gitlabStateMerged, gitlabStateClosed, gitlabStateLocked: return state default: return state @@ -315,7 +315,7 @@ func hasOpenDiscussions(discussions []MRDiscussion) bool { func pipelineFailing(pipelines []Pipeline) bool { state, _, _ := summarizePipelines(pipelines) - return state == "failure" + return state == pipelineStateFailure } // Computed status strings shared by pipeline + approval summarizers. @@ -332,10 +332,10 @@ func summarizePipelines(pipelines []Pipeline) (state string, jobsTotal, jobsPass jobsTotal = latest.JobsTotal jobsPassing = latest.JobsPassing switch latest.Status { - case "success": - state = "success" - case "failed", "canceled": - state = "failure" + case pipelineStatusSuccess: + state = pipelineStatusSuccess + case pipelineStatusFailed, "canceled": + state = pipelineStateFailure case "skipped": state = "" default: @@ -347,12 +347,12 @@ func summarizePipelines(pipelines []Pipeline) (state string, jobsTotal, jobsPass func summarizeApprovals(have, required int) string { if required == 0 { if have > 0 { - return "approved" + return approvalStateApproved } return "" } if have >= required { - return "approved" + return approvalStateApproved } return statusPending } @@ -363,13 +363,13 @@ func summarizeApprovals(have, required int) string { // GitLab's /merge_requests endpoint scopes to the authenticated user when // `scope=assigned_to_me` or `reviewer_username=`; we pass // `reviewer_username=` resolution to the caller via filter (e.g. -// "reviewer_username=octocat"). state defaults to "opened". +// "reviewer_username=octocat"). state defaults to GitLab's opened state. func buildReviewMRQuery(filter, customQuery string) string { if customQuery != "" { return customQuery } values := url.Values{} - values.Set("state", "opened") + values.Set("state", gitlabStateOpened) values.Set("scope", "all") values.Set("per_page", "50") if filter != "" { @@ -383,7 +383,7 @@ func buildMRSearchQuery(filter, customQuery string) string { return customQuery } values := url.Values{} - values.Set("state", "opened") + values.Set("state", gitlabStateOpened) values.Set("scope", "all") if filter != "" { appendFilter(values, filter) @@ -396,7 +396,7 @@ func buildIssueSearchQuery(filter, customQuery string) string { return customQuery } values := url.Values{} - values.Set("state", "opened") + values.Set("state", gitlabStateOpened) values.Set("scope", "all") if filter != "" { appendFilter(values, filter) diff --git a/apps/backend/internal/gitlab/constants.go b/apps/backend/internal/gitlab/constants.go index 21aec3e04f..84c29c46e4 100644 --- a/apps/backend/internal/gitlab/constants.go +++ b/apps/backend/internal/gitlab/constants.go @@ -7,6 +7,18 @@ const ( AuthMethodGLab = "glab_cli" ) +const ( + approvalStateApproved = "approved" + gitlabStateClosed = "closed" + gitlabStateLocked = "locked" + gitlabStateMerged = "merged" + gitlabStateOpened = "opened" + pipelineStateFailure = "failure" + pipelineStatusFailed = "failed" + pipelineStatusSuccess = "success" + secretNameTokenLower = "gitlab_token" +) + // DefaultHost is the public GitLab.com host. Self-managed instances override // this via the per-workspace gitlab_host setting. const DefaultHost = "https://gitlab.com" diff --git a/apps/backend/internal/gitlab/controller.go b/apps/backend/internal/gitlab/controller.go index 343c98d493..8a85d8e684 100644 --- a/apps/backend/internal/gitlab/controller.go +++ b/apps/backend/internal/gitlab/controller.go @@ -10,6 +10,8 @@ import ( "github.com/kandev/kandev/internal/common/logger" ) +const responseErrorKey = "error" + // Controller handles HTTP endpoints for GitLab integration. type Controller struct { service *Service @@ -49,7 +51,7 @@ func RegisterRoutes(router *gin.Engine, svc *Service, log *logger.Logger) { func (c *Controller) httpGetStatus(ctx *gin.Context) { status, err := c.service.GetStatus(ctx.Request.Context()) if err != nil { - ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + ctx.JSON(http.StatusInternalServerError, gin.H{responseErrorKey: err.Error()}) return } ctx.JSON(http.StatusOK, status) @@ -58,17 +60,17 @@ func (c *Controller) httpGetStatus(ctx *gin.Context) { func (c *Controller) httpConfigureToken(ctx *gin.Context) { var req ConfigureTokenRequest if err := ctx.ShouldBindJSON(&req); err != nil { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "invalid payload: token is required"}) + ctx.JSON(http.StatusBadRequest, gin.H{responseErrorKey: "invalid payload: token is required"}) return } if err := c.service.ConfigureToken(ctx.Request.Context(), req.Token); err != nil { // errors.Is against the package-level sentinel — durable across // future rewording / wrapping of the ConfigureToken error message. if errors.Is(err, ErrInvalidToken) { - ctx.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + ctx.JSON(http.StatusBadRequest, gin.H{responseErrorKey: err.Error()}) return } - ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + ctx.JSON(http.StatusInternalServerError, gin.H{responseErrorKey: err.Error()}) return } ctx.JSON(http.StatusOK, gin.H{"configured": true}) @@ -76,7 +78,7 @@ func (c *Controller) httpConfigureToken(ctx *gin.Context) { func (c *Controller) httpClearToken(ctx *gin.Context) { if err := c.service.ClearToken(ctx.Request.Context()); err != nil { - ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + ctx.JSON(http.StatusInternalServerError, gin.H{responseErrorKey: err.Error()}) return } ctx.JSON(http.StatusOK, gin.H{"cleared": true}) @@ -85,11 +87,11 @@ func (c *Controller) httpClearToken(ctx *gin.Context) { func (c *Controller) httpConfigureHost(ctx *gin.Context) { var req ConfigureHostRequest if err := ctx.ShouldBindJSON(&req); err != nil { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "invalid payload: host is required"}) + ctx.JSON(http.StatusBadRequest, gin.H{responseErrorKey: "invalid payload: host is required"}) return } if err := c.service.ConfigureHost(ctx.Request.Context(), req.Host); err != nil { - ctx.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + ctx.JSON(http.StatusBadRequest, gin.H{responseErrorKey: err.Error()}) return } ctx.JSON(http.StatusOK, gin.H{"configured": true, "host": c.service.Host()}) @@ -98,12 +100,12 @@ func (c *Controller) httpConfigureHost(ctx *gin.Context) { func (c *Controller) httpGetMRFeedback(ctx *gin.Context) { projectPath, iid, err := parseProjectAndIID(ctx) if err != nil { - ctx.JSON(http.StatusBadRequest, gin.H{"error": err.Error()}) + ctx.JSON(http.StatusBadRequest, gin.H{responseErrorKey: err.Error()}) return } feedback, err := c.service.GetMRFeedback(ctx.Request.Context(), projectPath, iid) if err != nil { - ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + ctx.JSON(http.StatusInternalServerError, gin.H{responseErrorKey: err.Error()}) return } ctx.JSON(http.StatusOK, feedback) @@ -117,12 +119,12 @@ func (c *Controller) httpCreateDiscussionNote(ctx *gin.Context) { Body string `json:"body" binding:"required"` } if err := ctx.ShouldBindJSON(&req); err != nil { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "invalid payload"}) + ctx.JSON(http.StatusBadRequest, gin.H{responseErrorKey: "invalid payload"}) return } note, err := c.service.CreateMRDiscussionNote(ctx.Request.Context(), req.Project, req.IID, req.DiscussionID, req.Body) if err != nil { - ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + ctx.JSON(http.StatusInternalServerError, gin.H{responseErrorKey: err.Error()}) return } ctx.JSON(http.StatusOK, note) @@ -135,11 +137,11 @@ func (c *Controller) httpResolveDiscussion(ctx *gin.Context) { DiscussionID string `json:"discussion_id" binding:"required"` } if err := ctx.ShouldBindJSON(&req); err != nil { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "invalid payload"}) + ctx.JSON(http.StatusBadRequest, gin.H{responseErrorKey: "invalid payload"}) return } if err := c.service.ResolveMRDiscussion(ctx.Request.Context(), req.Project, req.IID, req.DiscussionID); err != nil { - ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + ctx.JSON(http.StatusInternalServerError, gin.H{responseErrorKey: err.Error()}) return } ctx.JSON(http.StatusOK, gin.H{"resolved": true}) @@ -176,12 +178,12 @@ type SyncTaskMRRequest struct { func (c *Controller) httpListWorkspaceTaskMRs(ctx *gin.Context) { wsID := ctx.Param("workspaceID") if wsID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "workspaceID required"}) + ctx.JSON(http.StatusBadRequest, gin.H{responseErrorKey: "workspaceID required"}) return } taskMRs, err := c.service.ListTaskMRsByWorkspace(ctx.Request.Context(), wsID) if err != nil { - ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + ctx.JSON(http.StatusInternalServerError, gin.H{responseErrorKey: err.Error()}) return } ctx.JSON(http.StatusOK, TaskMRsResponse{TaskMRs: taskMRs}) @@ -190,12 +192,12 @@ func (c *Controller) httpListWorkspaceTaskMRs(ctx *gin.Context) { func (c *Controller) httpListTaskMRs(ctx *gin.Context) { taskID := ctx.Param("taskID") if taskID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "taskID required"}) + ctx.JSON(http.StatusBadRequest, gin.H{responseErrorKey: "taskID required"}) return } mrs, err := c.service.ListTaskMRsByTask(ctx.Request.Context(), taskID) if err != nil { - ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + ctx.JSON(http.StatusInternalServerError, gin.H{responseErrorKey: err.Error()}) return } ctx.JSON(http.StatusOK, gin.H{"task_mrs": mrs}) @@ -204,17 +206,17 @@ func (c *Controller) httpListTaskMRs(ctx *gin.Context) { func (c *Controller) httpSyncTaskMR(ctx *gin.Context) { taskID := ctx.Param("taskID") if taskID == "" { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "taskID required"}) + ctx.JSON(http.StatusBadRequest, gin.H{responseErrorKey: "taskID required"}) return } var req SyncTaskMRRequest if err := ctx.ShouldBindJSON(&req); err != nil { - ctx.JSON(http.StatusBadRequest, gin.H{"error": "invalid payload: project_path and iid required"}) + ctx.JSON(http.StatusBadRequest, gin.H{responseErrorKey: "invalid payload: project_path and iid required"}) return } row, err := c.service.SyncTaskMR(ctx.Request.Context(), taskID, req.RepositoryID, req.ProjectPath, req.IID) if err != nil { - ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + ctx.JSON(http.StatusInternalServerError, gin.H{responseErrorKey: err.Error()}) return } ctx.JSON(http.StatusOK, row) @@ -232,7 +234,7 @@ func (c *Controller) httpSearchUserMRs(ctx *gin.Context) { page, perPage, ) if err != nil { - ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + ctx.JSON(http.StatusInternalServerError, gin.H{responseErrorKey: err.Error()}) return } ctx.JSON(http.StatusOK, result) @@ -248,7 +250,7 @@ func (c *Controller) httpSearchUserIssues(ctx *gin.Context) { page, perPage, ) if err != nil { - ctx.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()}) + ctx.JSON(http.StatusInternalServerError, gin.H{responseErrorKey: err.Error()}) return } ctx.JSON(http.StatusOK, result) diff --git a/apps/backend/internal/gitlab/factory.go b/apps/backend/internal/gitlab/factory.go index 76e5771745..7b0028b5ed 100644 --- a/apps/backend/internal/gitlab/factory.go +++ b/apps/backend/internal/gitlab/factory.go @@ -48,7 +48,7 @@ func NewClient(ctx context.Context, host string, secrets SecretProvider, log *lo log.Debug("glab CLI available but not authenticated", zap.Error(err)) } - if token := os.Getenv("GITLAB_TOKEN"); token != "" { + if token := os.Getenv(secretNameToken); token != "" { log.Info("using GITLAB_TOKEN from environment for GitLab integration") return NewPATClient(host, token), AuthMethodPAT, nil } @@ -67,7 +67,7 @@ func NewClient(ctx context.Context, host string, secrets SecretProvider, log *lo return NewNoopClient(host), AuthMethodNone, nil } -// findPAT looks for a secret named "GITLAB_TOKEN" or "gitlab_token". +// findPAT looks for a GitLab PAT secret. func findPAT(ctx context.Context, secrets SecretProvider) (string, error) { items, err := secrets.List(ctx) if err != nil { @@ -77,7 +77,7 @@ func findPAT(ctx context.Context, secrets SecretProvider) (string, error) { if !item.HasValue { continue } - if item.Name == "GITLAB_TOKEN" || item.Name == "gitlab_token" { + if item.Name == secretNameToken || item.Name == secretNameTokenLower { return secrets.Reveal(ctx, item.ID) } } diff --git a/apps/backend/internal/gitlab/service.go b/apps/backend/internal/gitlab/service.go index 3ef33e6bb5..37081cbb02 100644 --- a/apps/backend/internal/gitlab/service.go +++ b/apps/backend/internal/gitlab/service.go @@ -327,7 +327,7 @@ func (s *Service) findTokenSecret(ctx context.Context) (bool, string, error) { if !item.HasValue { continue } - if item.Name == secretNameToken || item.Name == "gitlab_token" { + if item.Name == secretNameToken || item.Name == secretNameTokenLower { return true, item.ID, nil } } diff --git a/apps/backend/internal/jira/cloud_client.go b/apps/backend/internal/jira/cloud_client.go index 8ba7f67464..22c20a555b 100644 --- a/apps/backend/internal/jira/cloud_client.go +++ b/apps/backend/internal/jira/cloud_client.go @@ -411,9 +411,11 @@ type serverSearchResponse struct { // searchFields is the list of issue fields we ask Jira to return. Kept short // because tickets are listed in dense table rows; the rest is fetched lazily // on click. +const jiraFieldUpdated = "updated" + var searchFields = []string{ "summary", "status", "project", "issuetype", - "priority", "assignee", "reporter", "updated", + "priority", "assignee", "reporter", jiraFieldUpdated, } // SearchTickets runs a JQL search and returns a page of tickets. The transport diff --git a/apps/backend/internal/orchestrator/event_handlers_automation.go b/apps/backend/internal/orchestrator/event_handlers_automation.go index 1c313e87c3..bb5dfa8fa1 100644 --- a/apps/backend/internal/orchestrator/event_handlers_automation.go +++ b/apps/backend/internal/orchestrator/event_handlers_automation.go @@ -16,6 +16,8 @@ import ( "github.com/kandev/kandev/internal/worktree" ) +const automationDefaultBaseBranch = "main" + // AutomationService is the interface the orchestrator uses for automation operations. type AutomationService interface { GetAutomation(ctx context.Context, id string) (*automation.Automation, error) @@ -117,13 +119,13 @@ func (s *Service) createAutomationTask(ctx context.Context, evt *automation.Auto Description: prompt, Repositories: repositories, Metadata: map[string]interface{}{ - "automation_id": a.ID, - "automation_name": a.Name, - "trigger_id": evt.TriggerID, - "trigger_type": string(evt.TriggerType), - "agent_profile_id": a.AgentProfileID, - "executor_profile_id": a.ExecutorProfileID, - "execution_mode": string(a.ExecutionMode), + "automation_id": a.ID, + "automation_name": a.Name, + "trigger_id": evt.TriggerID, + "trigger_type": string(evt.TriggerType), + models.MetaKeyAgentProfileID: a.AgentProfileID, + models.MetaKeyExecutorProfileID: a.ExecutorProfileID, + "execution_mode": string(a.ExecutionMode), }, IsEphemeral: isRunMode, Origin: models.TaskOriginAutomationRun, @@ -219,7 +221,7 @@ func (s *Service) resolveExplicitRepository( } baseBranch := repo.DefaultBranch if baseBranch == "" { - baseBranch = "main" + baseBranch = automationDefaultBaseBranch } return []ReviewTaskRepository{{ RepositoryID: repo.ID, @@ -287,7 +289,7 @@ func (s *Service) resolveWorkspaceRepository( repo := repos[0] baseBranch := repo.DefaultBranch if baseBranch == "" { - baseBranch = "main" + baseBranch = automationDefaultBaseBranch } return []ReviewTaskRepository{{ RepositoryID: repo.ID, diff --git a/apps/backend/internal/orchestrator/executor/executor_interaction.go b/apps/backend/internal/orchestrator/executor/executor_interaction.go index 20eff1166d..0aee6bd9ce 100644 --- a/apps/backend/internal/orchestrator/executor/executor_interaction.go +++ b/apps/backend/internal/orchestrator/executor/executor_interaction.go @@ -122,7 +122,10 @@ func (e *Executor) StopByTaskID(ctx context.Context, taskID string, reason strin // supported in passthrough mode (Claude, Codex, OpenCode, Cursor, Auggie, …). // If a future agent needs a different sequence, lift this into a per-agent // PassthroughConfig field — see the plan for issue #989. -const passthroughSubmitSequence = "\r" +const ( + passthroughSubmitSequence = "\r" + stopReasonPassthrough = "passthrough_dispatched" +) // Prompt sends a follow-up prompt to a running agent for a task // Returns PromptResult indicating if the agent needs input @@ -202,7 +205,7 @@ func (e *Executor) promptPassthrough(ctx context.Context, taskID, sessionID, pro zap.String("session_id", sessionID), zap.Error(err)) } - return &PromptResult{StopReason: "passthrough_dispatched"}, nil + return &PromptResult{StopReason: stopReasonPassthrough}, nil } // SwitchModel switches the model for a running session. It first attempts an in-place switch diff --git a/apps/backend/internal/terminal/models/terminal.go b/apps/backend/internal/terminal/models/terminal.go new file mode 100644 index 0000000000..0dc247e56c --- /dev/null +++ b/apps/backend/internal/terminal/models/terminal.go @@ -0,0 +1,49 @@ +// Package models defines persistent types for first-class user terminals. +// +// Ordinary user terminals (the ones created via the `+` button in the UI) have +// a DB row that survives backend restarts and page refreshes. The terminal id +// equals the agentctl PTY id; the row carries the per-task sequence number +// (stable across deletes, gaps preserved), the user's custom name override, +// and the open/parked state. +// +// The hardcoded `bottom-panel` terminal and script terminals (id prefix +// `script-`) are NOT persisted here — they pass straight through to agentctl. +// Guards live in the terminal service, not in this package. +package models + +import ( + "fmt" + "time" +) + +// TerminalState is the user-facing visibility state for an ordinary terminal. +type TerminalState string + +const ( + // StateOpen — terminal tab visible in the panel strip. + StateOpen TerminalState = "open" + // StateParked — tab hidden; PTY may still be running. User can resume + // to bring it back. + StateParked TerminalState = "parked" +) + +// Terminal is a persisted ordinary user terminal. +type Terminal struct { + ID string `db:"id"` + TaskID string `db:"task_id"` + EnvironmentID string `db:"environment_id"` + Seq int `db:"seq"` + CustomName *string `db:"custom_name"` + State TerminalState `db:"state"` + InitialCommand string `db:"initial_command"` + CreatedAt time.Time `db:"created_at"` +} + +// DisplayName returns the user-facing label: custom_name if set, else the +// derived "Terminal N" form. +func (t *Terminal) DisplayName() string { + if t.CustomName != nil && *t.CustomName != "" { + return *t.CustomName + } + return fmt.Sprintf("Terminal %d", t.Seq) +} diff --git a/apps/backend/internal/terminal/repository/sqlite.go b/apps/backend/internal/terminal/repository/sqlite.go new file mode 100644 index 0000000000..adad10e752 --- /dev/null +++ b/apps/backend/internal/terminal/repository/sqlite.go @@ -0,0 +1,184 @@ +// Package repository persists ordinary user terminals in SQLite. +// +// The bottom-panel and script terminals are not persisted here — guards in +// the terminal service decide which terminals get a row. This package is +// scope-agnostic; it just stores what it's told. +package repository + +import ( + "context" + "database/sql" + "errors" + "fmt" + + "github.com/jmoiron/sqlx" + + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/terminal/models" +) + +// ErrNotFound is returned by Get when no terminal matches the id. +var ErrNotFound = errors.New("terminal not found") + +// Repository persists user terminals. +type Repository struct { + db *sqlx.DB // writer + ro *sqlx.DB // reader + log *logger.Logger +} + +// NewWithDB constructs a Repository sharing the supplied writer/reader pools. +// initSchema creates the table on first call; subsequent calls are no-ops. +func NewWithDB(writer, reader *sqlx.DB, log *logger.Logger) (*Repository, error) { + r := &Repository{db: writer, ro: reader, log: log} + if err := r.initSchema(); err != nil { + return nil, fmt.Errorf("init terminal schema: %w", err) + } + return r, nil +} + +// Close is a no-op — the writer/reader pools are owned by the caller. +func (r *Repository) Close() error { return nil } + +func (r *Repository) initSchema() error { + stmts := []string{ + `CREATE TABLE IF NOT EXISTS user_terminals ( + id TEXT PRIMARY KEY, + task_id TEXT NOT NULL, + environment_id TEXT NOT NULL, + seq INTEGER NOT NULL, + custom_name TEXT, + state TEXT NOT NULL DEFAULT 'open', + initial_command TEXT NOT NULL DEFAULT '', + created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, + UNIQUE(task_id, seq) + )`, + `CREATE INDEX IF NOT EXISTS idx_user_terminals_task ON user_terminals(task_id)`, + `CREATE INDEX IF NOT EXISTS idx_user_terminals_env ON user_terminals(environment_id)`, + } + for _, s := range stmts { + if _, err := r.db.Exec(s); err != nil { + return err + } + } + return nil +} + +// Create inserts a new terminal row. The seq is computed as MAX(seq)+1 for +// the task — never reused, so deletes leave gaps. +// +// Seq computation and insertion are folded into a single INSERT … SELECT +// statement so the MAX read and the new row write happen atomically inside +// SQLite's per-statement transaction. Two concurrent creates for the same +// task can no longer pick the same seq — the second statement reads the +// first's already-inserted row. +// +// The caller supplies the id (must be a UUID that's also used as the +// agentctl PTY id). initialCommand is "" for plain shells. +func (r *Repository) Create(ctx context.Context, taskID, envID, id, initialCommand string) (*models.Terminal, error) { + if _, err := r.db.ExecContext(ctx, + `INSERT INTO user_terminals (id, task_id, environment_id, seq, custom_name, state, initial_command) + SELECT ?, ?, ?, COALESCE(MAX(seq), 0) + 1, NULL, 'open', ? + FROM user_terminals WHERE task_id = ?`, + id, taskID, envID, initialCommand, taskID, + ); err != nil { + return nil, fmt.Errorf("insert terminal: %w", err) + } + + return r.Get(ctx, id) +} + +// Get returns the terminal by id, or ErrNotFound. +func (r *Repository) Get(ctx context.Context, id string) (*models.Terminal, error) { + var t models.Terminal + err := r.ro.GetContext(ctx, &t, + `SELECT id, task_id, environment_id, seq, custom_name, state, initial_command, created_at + FROM user_terminals WHERE id = ?`, + id, + ) + if errors.Is(err, sql.ErrNoRows) { + return nil, ErrNotFound + } + if err != nil { + return nil, fmt.Errorf("get terminal: %w", err) + } + return &t, nil +} + +// ListByTask returns terminals for taskID ordered by seq ascending. +// includeParked controls whether parked rows are included. +func (r *Repository) ListByTask(ctx context.Context, taskID string, includeParked bool) ([]*models.Terminal, error) { + q := `SELECT id, task_id, environment_id, seq, custom_name, state, initial_command, created_at + FROM user_terminals WHERE task_id = ?` + args := []any{taskID} + if !includeParked { + q += ` AND state = 'open'` + } + q += ` ORDER BY seq ASC` + + var rows []*models.Terminal + if err := r.ro.SelectContext(ctx, &rows, q, args...); err != nil { + return nil, fmt.Errorf("list terminals: %w", err) + } + return rows, nil +} + +// Rename sets or clears the custom_name. Pass nil to clear, a non-empty +// string to set. +func (r *Repository) Rename(ctx context.Context, id string, name *string) error { + var nameArg any + if name == nil { + nameArg = nil + } else { + nameArg = *name + } + res, err := r.db.ExecContext(ctx, + `UPDATE user_terminals SET custom_name = ? WHERE id = ?`, + nameArg, id, + ) + if err != nil { + return fmt.Errorf("rename terminal: %w", err) + } + n, _ := res.RowsAffected() + if n == 0 { + return ErrNotFound + } + return nil +} + +// SetState updates the open/parked state. Does not touch any PTY — that's +// the service's job. +func (r *Repository) SetState(ctx context.Context, id string, state models.TerminalState) error { + res, err := r.db.ExecContext(ctx, + `UPDATE user_terminals SET state = ? WHERE id = ?`, + string(state), id, + ) + if err != nil { + return fmt.Errorf("set state: %w", err) + } + n, _ := res.RowsAffected() + if n == 0 { + return ErrNotFound + } + return nil +} + +// Delete removes a single terminal row. +func (r *Repository) Delete(ctx context.Context, id string) error { + _, err := r.db.ExecContext(ctx, `DELETE FROM user_terminals WHERE id = ?`, id) + if err != nil { + return fmt.Errorf("delete terminal: %w", err) + } + return nil +} + +// DeleteByTask removes every row for a task and returns the count. Used by +// the task.deleted cascade subscriber. +func (r *Repository) DeleteByTask(ctx context.Context, taskID string) (int, error) { + res, err := r.db.ExecContext(ctx, `DELETE FROM user_terminals WHERE task_id = ?`, taskID) + if err != nil { + return 0, fmt.Errorf("delete by task: %w", err) + } + n, _ := res.RowsAffected() + return int(n), nil +} diff --git a/apps/backend/internal/terminal/repository/sqlite_test.go b/apps/backend/internal/terminal/repository/sqlite_test.go new file mode 100644 index 0000000000..bfc9d96303 --- /dev/null +++ b/apps/backend/internal/terminal/repository/sqlite_test.go @@ -0,0 +1,278 @@ +package repository + +import ( + "context" + "database/sql" + "errors" + "sync" + "testing" + + "github.com/google/uuid" + "github.com/jmoiron/sqlx" + _ "github.com/mattn/go-sqlite3" + + "github.com/kandev/kandev/internal/terminal/models" +) + +func setupTestRepo(t *testing.T) *Repository { + t.Helper() + // shared-cache + MaxOpenConns(1) mirrors the production writer pool + // (see internal/task/repository/sqlite/base.go) so all goroutines see + // the same in-memory database — without this, each connection gets + // its own DB and the concurrency test would see "no such table". + rawDB, err := sql.Open("sqlite3", "file::memory:?cache=shared&_foreign_keys=on") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + rawDB.SetMaxOpenConns(1) + sqlxDB := sqlx.NewDb(rawDB, "sqlite3") + t.Cleanup(func() { _ = sqlxDB.Close() }) + repo, err := NewWithDB(sqlxDB, sqlxDB, nil) + if err != nil { + t.Fatalf("create repo: %v", err) + } + return repo +} + +func TestCreate_AssignsSequentialSeqs(t *testing.T) { + repo := setupTestRepo(t) + ctx := context.Background() + + t1, err := repo.Create(ctx, "task-1", "env-1", "term-a", "") + if err != nil { + t.Fatalf("create #1: %v", err) + } + t2, err := repo.Create(ctx, "task-1", "env-1", "term-b", "") + if err != nil { + t.Fatalf("create #2: %v", err) + } + + if t1.Seq != 1 { + t.Errorf("first seq = %d, want 1", t1.Seq) + } + if t2.Seq != 2 { + t.Errorf("second seq = %d, want 2", t2.Seq) + } + if t1.State != models.StateOpen { + t.Errorf("default state = %q, want open", t1.State) + } +} + +func TestCreate_SeqIsPerTask(t *testing.T) { + repo := setupTestRepo(t) + ctx := context.Background() + + _, _ = repo.Create(ctx, "task-1", "env-1", "term-a", "") + tB, err := repo.Create(ctx, "task-2", "env-2", "term-b", "") + if err != nil { + t.Fatalf("create: %v", err) + } + if tB.Seq != 1 { + t.Errorf("seq in different task = %d, want 1", tB.Seq) + } +} + +func TestDelete_PreservesSeqGaps(t *testing.T) { + repo := setupTestRepo(t) + ctx := context.Background() + + _, _ = repo.Create(ctx, "task-1", "env-1", "t-a", "") + t2, _ := repo.Create(ctx, "task-1", "env-1", "t-b", "") + _, _ = repo.Create(ctx, "task-1", "env-1", "t-c", "") + + if err := repo.Delete(ctx, t2.ID); err != nil { + t.Fatalf("delete: %v", err) + } + + tD, err := repo.Create(ctx, "task-1", "env-1", "t-d", "") + if err != nil { + t.Fatalf("create after delete: %v", err) + } + if tD.Seq != 4 { + t.Errorf("seq after delete = %d, want 4 (gap preserved)", tD.Seq) + } +} + +func TestListByTask_ReturnsAllStates(t *testing.T) { + repo := setupTestRepo(t) + ctx := context.Background() + + _, _ = repo.Create(ctx, "task-1", "env-1", "t-a", "") + tB, _ := repo.Create(ctx, "task-1", "env-1", "t-b", "") + _ = repo.SetState(ctx, tB.ID, models.StateParked) + + open, err := repo.ListByTask(ctx, "task-1", false) + if err != nil { + t.Fatalf("list open: %v", err) + } + if len(open) != 1 { + t.Errorf("open count = %d, want 1", len(open)) + } + + all, err := repo.ListByTask(ctx, "task-1", true) + if err != nil { + t.Fatalf("list all: %v", err) + } + if len(all) != 2 { + t.Errorf("all count = %d, want 2", len(all)) + } +} + +func TestListByTask_OrderedBySeq(t *testing.T) { + repo := setupTestRepo(t) + ctx := context.Background() + + for i := 0; i < 3; i++ { + if _, err := repo.Create(ctx, "task-1", "env-1", "t-"+string(rune('a'+i)), ""); err != nil { + t.Fatalf("create: %v", err) + } + } + + list, err := repo.ListByTask(ctx, "task-1", true) + if err != nil { + t.Fatalf("list: %v", err) + } + for i, term := range list { + if term.Seq != i+1 { + t.Errorf("position %d seq = %d, want %d", i, term.Seq, i+1) + } + } +} + +func TestRename_PersistsCustomName(t *testing.T) { + repo := setupTestRepo(t) + ctx := context.Background() + + term, _ := repo.Create(ctx, "task-1", "env-1", "t-a", "") + name := "build watcher" + if err := repo.Rename(ctx, term.ID, &name); err != nil { + t.Fatalf("rename: %v", err) + } + + got, err := repo.Get(ctx, term.ID) + if err != nil { + t.Fatalf("get: %v", err) + } + if got.CustomName == nil || *got.CustomName != "build watcher" { + t.Errorf("custom_name = %v, want 'build watcher'", got.CustomName) + } +} + +func TestRename_NilClearsCustomName(t *testing.T) { + repo := setupTestRepo(t) + ctx := context.Background() + + term, _ := repo.Create(ctx, "task-1", "env-1", "t-a", "") + name := "x" + _ = repo.Rename(ctx, term.ID, &name) + if err := repo.Rename(ctx, term.ID, nil); err != nil { + t.Fatalf("rename nil: %v", err) + } + + got, _ := repo.Get(ctx, term.ID) + if got.CustomName != nil { + t.Errorf("custom_name after nil rename = %v, want nil", got.CustomName) + } +} + +func TestSetState_Park(t *testing.T) { + repo := setupTestRepo(t) + ctx := context.Background() + + term, _ := repo.Create(ctx, "task-1", "env-1", "t-a", "") + if err := repo.SetState(ctx, term.ID, models.StateParked); err != nil { + t.Fatalf("set state: %v", err) + } + + got, _ := repo.Get(ctx, term.ID) + if got.State != models.StateParked { + t.Errorf("state = %q, want parked", got.State) + } +} + +func TestGet_NotFoundReturnsErrNotFound(t *testing.T) { + repo := setupTestRepo(t) + ctx := context.Background() + + _, err := repo.Get(ctx, "missing-id") + if !errors.Is(err, ErrNotFound) { + t.Errorf("expected ErrNotFound, got %v", err) + } +} + +// TestCreate_ConcurrentSeqAllocation pounds Create with N goroutines and +// verifies every seq is unique (no UNIQUE constraint collisions). Locks in +// the atomic INSERT … SELECT contract. +func TestCreate_ConcurrentSeqAllocation(t *testing.T) { + repo := setupTestRepo(t) + ctx := context.Background() + + const N = 25 + var wg sync.WaitGroup + errs := make(chan error, N) + seqs := make(chan int, N) + for i := 0; i < N; i++ { + wg.Add(1) + go func() { + defer wg.Done() + term, err := repo.Create(ctx, "task-1", "env-1", "t-"+uuid.New().String(), "") + if err != nil { + errs <- err + return + } + seqs <- term.Seq + }() + } + wg.Wait() + close(errs) + close(seqs) + + for err := range errs { + t.Errorf("concurrent create: %v", err) + } + seen := make(map[int]struct{}, N) + for s := range seqs { + if _, dup := seen[s]; dup { + t.Errorf("duplicate seq %d allocated", s) + } + seen[s] = struct{}{} + } + if len(seen) != N { + t.Errorf("seen %d unique seqs, want %d", len(seen), N) + } +} + +func TestCreate_WithInitialCommand(t *testing.T) { + repo := setupTestRepo(t) + ctx := context.Background() + + term, err := repo.Create(ctx, "task-1", "env-1", "t-a", "npm run dev") + if err != nil { + t.Fatalf("create: %v", err) + } + if term.InitialCommand != "npm run dev" { + t.Errorf("initial_command = %q, want 'npm run dev'", term.InitialCommand) + } +} + +func TestDelete_TaskCascade(t *testing.T) { + repo := setupTestRepo(t) + ctx := context.Background() + + _, _ = repo.Create(ctx, "task-1", "env-1", "t-a", "") + _, _ = repo.Create(ctx, "task-1", "env-1", "t-b", "") + _, _ = repo.Create(ctx, "task-2", "env-2", "t-c", "") + + n, err := repo.DeleteByTask(ctx, "task-1") + if err != nil { + t.Fatalf("delete by task: %v", err) + } + if n != 2 { + t.Errorf("deleted count = %d, want 2", n) + } + + rest, _ := repo.ListByTask(ctx, "task-2", true) + if len(rest) != 1 { + t.Errorf("task-2 untouched count = %d, want 1", len(rest)) + } +} diff --git a/apps/backend/internal/terminal/service/ptybackend.go b/apps/backend/internal/terminal/service/ptybackend.go new file mode 100644 index 0000000000..81de35671c --- /dev/null +++ b/apps/backend/internal/terminal/service/ptybackend.go @@ -0,0 +1,51 @@ +package service + +import ( + "context" + + "github.com/kandev/kandev/internal/agentctl/server/process" +) + +// InteractiveRunnerBackend adapts *process.InteractiveRunner to the +// PTYBackend interface this service needs. Lives here (instead of in the +// agentctl package) because PTYBackend is owned by the consumer. +type InteractiveRunnerBackend struct { + runner *process.InteractiveRunner +} + +// NewInteractiveRunnerBackend wraps the supplied InteractiveRunner. The +// runner may be nil — in that case every method becomes a safe no-op / +// false. This matches the existing handler's defensive behaviour against a +// missing interactive runner (e.g. remote executor mode). +func NewInteractiveRunnerBackend(runner *process.InteractiveRunner) *InteractiveRunnerBackend { + return &InteractiveRunnerBackend{runner: runner} +} + +// Register pre-registers the user-shell entry on the runner so the next WS +// stream connect can lazily start the PTY against this terminalID. Uses +// RegisterScriptShell with empty initial command; the runner doesn't need +// to distinguish "plain shell" from "script with no command" at register +// time. +func (b *InteractiveRunnerBackend) Register(scopeID, terminalID string) { + if b.runner == nil { + return + } + b.runner.RegisterScriptShell(scopeID, terminalID, "", "") +} + +// Stop tears down the PTY (if any) and removes the entry from the runner. +func (b *InteractiveRunnerBackend) Stop(ctx context.Context, scopeID, terminalID string) error { + if b.runner == nil { + return nil + } + return b.runner.StopUserShell(ctx, scopeID, terminalID) +} + +// IsAlive reports whether the PTY backing (scopeID, terminalID) is currently +// running. +func (b *InteractiveRunnerBackend) IsAlive(scopeID, terminalID string) bool { + if b.runner == nil { + return false + } + return b.runner.IsUserShellAlive(scopeID, terminalID) +} diff --git a/apps/backend/internal/terminal/service/service.go b/apps/backend/internal/terminal/service/service.go new file mode 100644 index 0000000000..07c59f91c5 --- /dev/null +++ b/apps/backend/internal/terminal/service/service.go @@ -0,0 +1,254 @@ +// Package service orchestrates persisted user-terminal metadata with the +// agentctl PTY backend. +// +// Scope: only ordinary user terminals are managed here. The hardcoded +// `bottom-panel` terminal (cmd+J) and script terminals (id prefix `script-`) +// are explicitly out of scope — guards reject them, and the WS layer passes +// those calls through to the original (agentctl-direct) code path unchanged. +package service + +import ( + "context" + "errors" + "strings" + + "github.com/google/uuid" + "go.uber.org/zap" + + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/terminal/models" + "github.com/kandev/kandev/internal/terminal/repository" +) + +// BottomPanelID is the fixed terminal id used by the cmd+J panel. Never +// renamed, never persisted in the user_terminals table. +const BottomPanelID = "bottom-panel" + +// PTYStatus values mirror the wire DTO. "running" if the PTY is alive on +// the agentctl side; "stopped" otherwise. +const ( + PTYStatusRunning = "running" + PTYStatusStopped = "stopped" +) + +// Kind discriminator for list items. Only "ordinary" is owned by this +// service; the WS layer adds "fixed" (bottom-panel) and "script" items +// alongside what this service returns. +const ( + KindOrdinary = "ordinary" +) + +// ErrNotManaged is returned by rename/park/resume/destroy when the caller +// hands in a non-ordinary terminal id (bottom-panel or script-*). The WS +// layer should never call those methods for those ids; this error catches +// misuse loudly. +var ErrNotManaged = errors.New("terminal id is not managed by the terminal service") + +// ErrTaskMismatch is returned by mutating ops when the supplied task_id +// does not own the supplied terminal_id. Defends against cross-task +// rename/park/destroy by raw terminal id. +var ErrTaskMismatch = errors.New("terminal does not belong to the supplied task") + +// PTYBackend is the slice of agentctl's interactive runner that the service +// needs. Register pre-creates the agentctl-side entry so the next WS stream +// connection can lazily start the PTY; Stop tears down the PTY; IsAlive is +// a cheap probe. +type PTYBackend interface { + Register(scopeID, terminalID string) + Stop(ctx context.Context, scopeID, terminalID string) error + IsAlive(scopeID, terminalID string) bool +} + +// Service is the single entry point for all user-terminal RPCs. +type Service struct { + repo *repository.Repository + pty PTYBackend + log *logger.Logger +} + +// New constructs a Service. +func New(repo *repository.Repository, pty PTYBackend, log *logger.Logger) *Service { + return &Service{repo: repo, pty: pty, log: log} +} + +// IsManaged reports whether the terminal id corresponds to an ordinary +// user terminal (i.e. one this service is responsible for). +func IsManaged(id string) bool { + if id == BottomPanelID { + return false + } + if strings.HasPrefix(id, "script-") { + return false + } + return true +} + +// ListItem is the wire shape returned to the frontend. +type ListItem struct { + ID string `json:"id"` + Kind string `json:"kind"` + Seq int `json:"seq"` + DisplayName string `json:"display_name"` + CustomName *string `json:"custom_name"` + State string `json:"state"` + PTYStatus string `json:"pty_status"` + InitialCommand string `json:"initial_command,omitempty"` +} + +// Create inserts a new ordinary user terminal for taskID. Generates the +// stable id (also used as the agentctl PTY id) and pre-registers it on the +// PTY backend so the lazy-start path on first WS stream connect finds the +// entry. +func (s *Service) Create(ctx context.Context, taskID, envID, initialCommand string) (*models.Terminal, error) { + id := "shell-" + uuid.New().String() + term, err := s.repo.Create(ctx, taskID, envID, id, initialCommand) + if err != nil { + return nil, err + } + if s.pty != nil { + s.pty.Register(envID, id) + } + return term, nil +} + +// List returns ordinary terminals for the task, with PTY liveness merged in. +// includeParked controls whether parked rows appear. Frontend filters them +// out of the main strip but uses includeParked=true for the "Parked +// terminals" submenu. +func (s *Service) List(ctx context.Context, taskID string, includeParked bool) ([]ListItem, error) { + rows, err := s.repo.ListByTask(ctx, taskID, includeParked) + if err != nil { + return nil, err + } + items := make([]ListItem, 0, len(rows)) + for _, r := range rows { + ptyStatus := PTYStatusStopped + if s.pty != nil && s.pty.IsAlive(r.EnvironmentID, r.ID) { + ptyStatus = PTYStatusRunning + } + items = append(items, ListItem{ + ID: r.ID, + Kind: KindOrdinary, + Seq: r.Seq, + DisplayName: r.DisplayName(), + CustomName: r.CustomName, + State: string(r.State), + PTYStatus: ptyStatus, + InitialCommand: r.InitialCommand, + }) + } + return items, nil +} + +// requireOwnership loads the terminal and verifies it belongs to taskID. +// Returns ErrTaskMismatch on a cross-task attempt and ErrNotManaged when +// the id falls outside the managed-id set. +// +// taskID is required; the previous "empty taskID skips" carve-out was +// dropped because the WS handlers pass user-controlled task_id directly +// to this helper, making the bypass an unauthenticated escape hatch. +// Internal task-scoped cleanup paths (CleanupTask) walk the repo +// directly and never call this method. +func (s *Service) requireOwnership(ctx context.Context, taskID, id string) (*models.Terminal, error) { + if !IsManaged(id) { + return nil, ErrNotManaged + } + if taskID == "" { + return nil, ErrTaskMismatch + } + term, err := s.repo.Get(ctx, id) + if err != nil { + return nil, err + } + if term.TaskID != taskID { + return nil, ErrTaskMismatch + } + return term, nil +} + +// Rename sets or clears the custom_name on an ordinary terminal. Pass nil +// to clear (revert to the derived "Terminal N" label). taskID is checked +// against the terminal's task_id; cross-task rename returns ErrTaskMismatch. +func (s *Service) Rename(ctx context.Context, taskID, id string, name *string) error { + if _, err := s.requireOwnership(ctx, taskID, id); err != nil { + return err + } + return s.repo.Rename(ctx, id, name) +} + +// Park hides a terminal tab; the PTY is intentionally left running. +func (s *Service) Park(ctx context.Context, taskID, id string) error { + if _, err := s.requireOwnership(ctx, taskID, id); err != nil { + return err + } + return s.repo.SetState(ctx, id, models.StateParked) +} + +// Resume returns a parked terminal to the visible state. PTY status is +// returned by a subsequent List call. +func (s *Service) Resume(ctx context.Context, taskID, id string) error { + if _, err := s.requireOwnership(ctx, taskID, id); err != nil { + return err + } + return s.repo.SetState(ctx, id, models.StateOpen) +} + +// Destroy stops the PTY and deletes the DB row. Irreversible. +// +// PTY stop errors are not fatal — agentctl may have already torn the +// process down (e.g. after a container restart), so a missing PTY isn't +// reason to leave the DB row behind. The error is logged at Warn so +// operators can spot "terminal row gone, PTY still running" situations +// without it becoming an opaque 500 to the caller. +func (s *Service) Destroy(ctx context.Context, taskID, id string) error { + term, err := s.requireOwnership(ctx, taskID, id) + if err != nil { + // Already-deleted rows are not a destroy failure — the caller + // got what they asked for. Cross-task and not-managed errors + // still surface. + if errors.Is(err, repository.ErrNotFound) { + return nil + } + return err + } + if s.pty != nil { + if stopErr := s.pty.Stop(ctx, term.EnvironmentID, id); stopErr != nil { + s.logWarn("pty stop on destroy (best-effort)", + zap.String("terminal_id", id), + zap.String("environment_id", term.EnvironmentID), + zap.Error(stopErr)) + } + } + return s.repo.Delete(ctx, id) +} + +// CleanupTask is called by the task.deleted/archived event subscriber. It +// stops every ordinary PTY for the task and deletes the rows. Stop +// failures are logged but don't block deletion — the DB row is the source +// of truth for "the user expects this terminal gone". +func (s *Service) CleanupTask(ctx context.Context, taskID string) (int, error) { + rows, err := s.repo.ListByTask(ctx, taskID, true) + if err != nil { + return 0, err + } + if s.pty != nil { + for _, r := range rows { + if stopErr := s.pty.Stop(ctx, r.EnvironmentID, r.ID); stopErr != nil { + s.logWarn("pty stop on cleanup (best-effort)", + zap.String("task_id", taskID), + zap.String("terminal_id", r.ID), + zap.Error(stopErr)) + } + } + } + return s.repo.DeleteByTask(ctx, taskID) +} + +// logWarn is a nil-safe wrapper around s.log.Warn; service unit tests +// construct the service with a nil logger. +func (s *Service) logWarn(msg string, fields ...zap.Field) { + if s.log == nil { + return + } + s.log.Warn(msg, fields...) +} diff --git a/apps/backend/internal/terminal/service/service_test.go b/apps/backend/internal/terminal/service/service_test.go new file mode 100644 index 0000000000..7835386c07 --- /dev/null +++ b/apps/backend/internal/terminal/service/service_test.go @@ -0,0 +1,311 @@ +package service + +import ( + "context" + "database/sql" + "errors" + "testing" + + "github.com/jmoiron/sqlx" + _ "github.com/mattn/go-sqlite3" + + "github.com/kandev/kandev/internal/terminal/models" + "github.com/kandev/kandev/internal/terminal/repository" +) + +// fakeBackend records calls and answers IsAlive from a map keyed by terminalID. +type fakeBackend struct { + registered map[string]bool + stopped map[string]bool + alive map[string]bool +} + +func newFakeBackend() *fakeBackend { + return &fakeBackend{ + registered: map[string]bool{}, + stopped: map[string]bool{}, + alive: map[string]bool{}, + } +} + +func (f *fakeBackend) Register(_, terminalID string) { + f.registered[terminalID] = true + f.alive[terminalID] = true +} + +func (f *fakeBackend) Stop(_ context.Context, _, terminalID string) error { + f.stopped[terminalID] = true + delete(f.alive, terminalID) + return nil +} + +func (f *fakeBackend) IsAlive(_, terminalID string) bool { + return f.alive[terminalID] +} + +func setupService(t *testing.T) (*Service, *fakeBackend) { + t.Helper() + // shared-cache + MaxOpenConns(1) mirror the repository test setup so + // every connection in the sqlx pool talks to the same in-memory DB. + rawDB, err := sql.Open("sqlite3", "file::memory:?cache=shared&_foreign_keys=on") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + rawDB.SetMaxOpenConns(1) + sqlxDB := sqlx.NewDb(rawDB, "sqlite3") + t.Cleanup(func() { _ = sqlxDB.Close() }) + repo, err := repository.NewWithDB(sqlxDB, sqlxDB, nil) + if err != nil { + t.Fatalf("repo: %v", err) + } + be := newFakeBackend() + return New(repo, be, nil), be +} + +func TestCreate_RegistersWithBackend(t *testing.T) { + svc, be := setupService(t) + ctx := context.Background() + + term, err := svc.Create(ctx, "task-1", "env-1", "") + if err != nil { + t.Fatalf("create: %v", err) + } + if term.Seq != 1 { + t.Errorf("seq = %d, want 1", term.Seq) + } + if !be.registered[term.ID] { + t.Errorf("backend was not asked to register %s", term.ID) + } +} + +func TestList_BlendsDBAndPTYStatus(t *testing.T) { + svc, be := setupService(t) + ctx := context.Background() + + t1, _ := svc.Create(ctx, "task-1", "env-1", "") + t2, _ := svc.Create(ctx, "task-1", "env-1", "") + delete(be.alive, t2.ID) // simulate dead PTY + + items, err := svc.List(ctx, "task-1", true) + if err != nil { + t.Fatalf("list: %v", err) + } + if len(items) != 2 { + t.Fatalf("len = %d, want 2", len(items)) + } + if items[0].ID != t1.ID || items[0].PTYStatus != PTYStatusRunning { + t.Errorf("item 0 = %+v, want running %s", items[0], t1.ID) + } + if items[1].ID != t2.ID || items[1].PTYStatus != PTYStatusStopped { + t.Errorf("item 1 = %+v, want stopped %s", items[1], t2.ID) + } +} + +func TestList_FilterParked(t *testing.T) { + svc, _ := setupService(t) + ctx := context.Background() + + _, _ = svc.Create(ctx, "task-1", "env-1", "") + t2, _ := svc.Create(ctx, "task-1", "env-1", "") + _ = svc.Park(ctx, "task-1", t2.ID) + + open, _ := svc.List(ctx, "task-1", false) + if len(open) != 1 { + t.Errorf("open count = %d, want 1", len(open)) + } + all, _ := svc.List(ctx, "task-1", true) + if len(all) != 2 { + t.Errorf("all count = %d, want 2", len(all)) + } +} + +func TestRename_UpdatesDisplayName(t *testing.T) { + svc, _ := setupService(t) + ctx := context.Background() + + term, _ := svc.Create(ctx, "task-1", "env-1", "") + name := "build watcher" + if err := svc.Rename(ctx, "task-1", term.ID, &name); err != nil { + t.Fatalf("rename: %v", err) + } + + items, _ := svc.List(ctx, "task-1", true) + if items[0].DisplayName != "build watcher" { + t.Errorf("display = %q, want build watcher", items[0].DisplayName) + } +} + +func TestPark_DoesNotStopPTY(t *testing.T) { + svc, be := setupService(t) + ctx := context.Background() + + term, _ := svc.Create(ctx, "task-1", "env-1", "") + if err := svc.Park(ctx, "task-1", term.ID); err != nil { + t.Fatalf("park: %v", err) + } + if be.stopped[term.ID] { + t.Errorf("park stopped PTY; should leave running") + } + if !be.IsAlive("env-1", term.ID) { + t.Errorf("PTY no longer alive after park") + } +} + +func TestResume_SetsStateOpen(t *testing.T) { + svc, _ := setupService(t) + ctx := context.Background() + + term, _ := svc.Create(ctx, "task-1", "env-1", "") + _ = svc.Park(ctx, "task-1", term.ID) + if err := svc.Resume(ctx, "task-1", term.ID); err != nil { + t.Fatalf("resume: %v", err) + } + + items, _ := svc.List(ctx, "task-1", false) + if len(items) != 1 || items[0].State != string(models.StateOpen) { + t.Errorf("resume: items = %+v", items) + } +} + +func TestDestroy_StopsAndDeletes(t *testing.T) { + svc, be := setupService(t) + ctx := context.Background() + + term, _ := svc.Create(ctx, "task-1", "env-1", "") + if err := svc.Destroy(ctx, "task-1", term.ID); err != nil { + t.Fatalf("destroy: %v", err) + } + if !be.stopped[term.ID] { + t.Errorf("PTY not stopped") + } + items, _ := svc.List(ctx, "task-1", true) + if len(items) != 0 { + t.Errorf("rows remain after destroy: %d", len(items)) + } +} + +func TestCleanupTask_StopsAllAndDeletes(t *testing.T) { + svc, be := setupService(t) + ctx := context.Background() + + t1, _ := svc.Create(ctx, "task-1", "env-1", "") + t2, _ := svc.Create(ctx, "task-1", "env-1", "") + other, _ := svc.Create(ctx, "task-2", "env-2", "") + + n, err := svc.CleanupTask(ctx, "task-1") + if err != nil { + t.Fatalf("cleanup: %v", err) + } + if n != 2 { + t.Errorf("cleanup count = %d, want 2", n) + } + if !be.stopped[t1.ID] || !be.stopped[t2.ID] { + t.Errorf("not all stopped: %+v", be.stopped) + } + if be.stopped[other.ID] { + t.Errorf("other task affected: %s", other.ID) + } +} + +func TestGuard_RejectsBottomPanel(t *testing.T) { + svc, _ := setupService(t) + ctx := context.Background() + + name := "x" + if err := svc.Rename(ctx, "task-1", "bottom-panel", &name); err == nil { + t.Error("expected guard error for bottom-panel rename") + } + if err := svc.Park(ctx, "task-1", "bottom-panel"); err == nil { + t.Error("expected guard error for bottom-panel park") + } + if err := svc.Destroy(ctx, "task-1", "bottom-panel"); err == nil { + t.Error("expected guard error for bottom-panel destroy") + } +} + +// TestRename_RejectsCrossTask ensures a caller supplying the wrong task_id +// cannot rename a terminal owned by another task. Same shape applies to +// park/resume/destroy via the shared requireOwnership helper. +func TestRename_RejectsCrossTask(t *testing.T) { + svc, _ := setupService(t) + ctx := context.Background() + + owned, _ := svc.Create(ctx, "task-a", "env-a", "") + name := "stolen" + err := svc.Rename(ctx, "task-b", owned.ID, &name) + if err == nil { + t.Fatal("expected cross-task rename to be rejected") + } + if !errors.Is(err, ErrTaskMismatch) { + t.Errorf("expected ErrTaskMismatch, got %v", err) + } +} + +// TestPark_RejectsCrossTask same defense for park. +func TestPark_RejectsCrossTask(t *testing.T) { + svc, _ := setupService(t) + ctx := context.Background() + + owned, _ := svc.Create(ctx, "task-a", "env-a", "") + err := svc.Park(ctx, "task-b", owned.ID) + if !errors.Is(err, ErrTaskMismatch) { + t.Errorf("expected ErrTaskMismatch, got %v", err) + } +} + +// TestDestroy_RejectsCrossTask same defense for destroy. +func TestDestroy_RejectsCrossTask(t *testing.T) { + svc, be := setupService(t) + ctx := context.Background() + + owned, _ := svc.Create(ctx, "task-a", "env-a", "") + err := svc.Destroy(ctx, "task-b", owned.ID) + if !errors.Is(err, ErrTaskMismatch) { + t.Errorf("expected ErrTaskMismatch, got %v", err) + } + if be.stopped[owned.ID] { + t.Error("destroy should not have torn down the PTY across tasks") + } +} + +// TestRename_EmptyTaskIDRejected ensures the previous "empty taskID +// skips" carve-out is gone — an unauthenticated client cannot mutate any +// terminal by raw id just by omitting task_id. +func TestRename_EmptyTaskIDRejected(t *testing.T) { + svc, _ := setupService(t) + ctx := context.Background() + + owned, _ := svc.Create(ctx, "task-a", "env-a", "") + name := "stolen" + err := svc.Rename(ctx, "", owned.ID, &name) + if !errors.Is(err, ErrTaskMismatch) { + t.Errorf("empty taskID should reject with ErrTaskMismatch, got %v", err) + } +} + +func TestGuard_RejectsScriptPrefix(t *testing.T) { + svc, _ := setupService(t) + ctx := context.Background() + + name := "x" + if err := svc.Rename(ctx, "task-1", "script-abc", &name); err == nil { + t.Error("expected guard error for script- rename") + } +} + +func TestIsManaged(t *testing.T) { + cases := []struct { + id string + want bool + }{ + {"some-uuid", true}, + {"bottom-panel", false}, + {"script-anything", false}, + {"shell-uuid", true}, + } + for _, c := range cases { + if got := IsManaged(c.id); got != c.want { + t.Errorf("IsManaged(%q) = %v, want %v", c.id, got, c.want) + } + } +} diff --git a/apps/backend/internal/user/service/service.go b/apps/backend/internal/user/service/service.go index b9da43a285..f83991a869 100644 --- a/apps/backend/internal/user/service/service.go +++ b/apps/backend/internal/user/service/service.go @@ -21,6 +21,11 @@ var ( ErrValidation = errors.New("validation error") ) +const ( + changesPanelLayoutFlat = "flat" + changesPanelLayoutTree = "tree" +) + type Service struct { repo store.Repository eventBus bus.EventBus @@ -208,7 +213,7 @@ func applyChangesPanelLayout(settings *models.UserSettings, value *string) error return nil } v := strings.TrimSpace(*value) - if v != "flat" && v != "tree" { + if v != changesPanelLayoutFlat && v != changesPanelLayoutTree { return errors.New("changes_panel_layout must be 'flat' or 'tree'") } settings.ChangesPanelLayout = v diff --git a/apps/backend/pkg/websocket/actions.go b/apps/backend/pkg/websocket/actions.go index 2438015355..ca2581cec6 100644 --- a/apps/backend/pkg/websocket/actions.go +++ b/apps/backend/pkg/websocket/actions.go @@ -237,9 +237,13 @@ const ( ActionSessionShellOutput = "session.shell.output" // Shell output notification (also used for exit with type: "exit") // User shell actions (independent terminal tabs) - ActionUserShellList = "user_shell.list" // List running user shells for a session - ActionUserShellCreate = "user_shell.create" // Create a new user shell terminal (assigns ID and label) - ActionUserShellStop = "user_shell.stop" // Stop a user shell terminal + ActionUserShellList = "user_shell.list" // List user shells for a task (DB rows + agentctl probes) + ActionUserShellCreate = "user_shell.create" // Create a new user shell terminal (assigns ID + seq) + ActionUserShellStop = "user_shell.stop" // Legacy alias of destroy; kept for one release + ActionUserShellRename = "user_shell.rename" // Rename an ordinary user shell (set/clear custom_name) + ActionUserShellPark = "user_shell.park" // Hide tab; PTY continues running + ActionUserShellResume = "user_shell.resume" // Unhide a parked tab + ActionUserShellDestroy = "user_shell.destroy" // Stop the PTY and delete the row // Session file review actions ActionSessionFileReviewGet = "session.file_review.get" // Get all file reviews for a session diff --git a/apps/web/components/session-tabs.tsx b/apps/web/components/session-tabs.tsx index 50ad4c7802..f10fc485bb 100644 --- a/apps/web/components/session-tabs.tsx +++ b/apps/web/components/session-tabs.tsx @@ -8,12 +8,18 @@ export type SessionTab = { id: string; label: string; icon?: ReactNode; + /** Optional badge rendered before the label — e.g. "#3" for an ordinary terminal. */ + badge?: ReactNode; closable?: boolean; alwaysShowClose?: boolean; onClose?: (event: MouseEvent) => void; + onContextMenu?: (event: MouseEvent) => void; + onDoubleClick?: (event: MouseEvent) => void; className?: string; testId?: string; closeTestId?: string; + /** When true (default) keeps the existing 120px max-width; ordinary terminals with custom names benefit from `false`. */ + truncate?: boolean; }; type SessionTabsProps = { @@ -80,9 +86,23 @@ function SessionTabItem({ {tab.icon} + {tab.badge && ( + + {tab.badge} + + )} {tab.label} diff --git a/apps/web/components/task/dockview-add-panel-items.tsx b/apps/web/components/task/dockview-add-panel-items.tsx index e9a129ba5c..870ff94826 100644 --- a/apps/web/components/task/dockview-add-panel-items.tsx +++ b/apps/web/components/task/dockview-add-panel-items.tsx @@ -7,10 +7,8 @@ import { IconFolder, IconGitBranch, IconGitPullRequest, - IconMessagePlus, - IconTerminal2, } from "@tabler/icons-react"; -import { DropdownMenuItem, DropdownMenuSeparator } from "@kandev/ui/dropdown-menu"; +import { DropdownMenuItem } from "@kandev/ui/dropdown-menu"; import { prPanelLabel } from "@/components/github/pr-utils"; import { prTaskKey } from "@/components/github/pr-detail-panel"; import { useDockviewStore } from "@/lib/state/dockview-store"; @@ -18,6 +16,7 @@ import { useAppStore } from "@/components/state-provider"; import type { TaskPR } from "@/lib/types/github"; import { RepositoryScriptsMenuItems } from "./repository-scripts-menu"; import { SessionReopenMenuItems } from "./session-reopen-menu"; +import { TerminalReopenMenuItems } from "./terminal-reopen-menu"; type AddPanelMenuState = { taskId: string | null; @@ -59,23 +58,13 @@ export function AddPanelMenuItems({ return ( <> {state.taskId && ( - <> - - - New Agent - - - - + )} - - - Terminal - + addBrowserPanel(undefined, groupId)} className={MENU_ITEM_CLASS} diff --git a/apps/web/components/task/dockview-desktop-layout.tsx b/apps/web/components/task/dockview-desktop-layout.tsx index 3e115608b3..c6bf399457 100644 --- a/apps/web/components/task/dockview-desktop-layout.tsx +++ b/apps/web/components/task/dockview-desktop-layout.tsx @@ -46,6 +46,7 @@ import { ChangesTab } from "./changes-tab"; import { PlanTab } from "./plan-tab"; import { PreviewFileTab, PreviewDiffTab, PreviewCommitTab, PinnedDefaultTab } from "./preview-tab"; import { SessionTab } from "./session-tab"; +import { TerminalTab } from "./terminal-tab"; import { useTabMaximizeOnDoubleClick } from "./use-tab-maximize"; import { setupSessionTabSync, @@ -190,6 +191,7 @@ const tabComponents: Record s.tasks?.activeTaskId ?? null); const addTerminalPanel = useDockviewStore((s) => s.addTerminalPanel); + const addUserShell = useAppStore((s) => s.addUserShell); const devScript = useActiveSessionDevScript(); const [showNewSessionDialog, setShowNewSessionDialog] = useState(false); + // Eagerly populate the user-shell store for this env+task so the + // dockview "+" menu's Terminals submenu and the per-tab badge logic + // both have the data ready on first open. Without this call, the + // store stays empty on desktop (the mobile right-panel hook is the + // only other call site) and the menu would render an empty section. + useUserShells(environmentId, taskID); + // Migrate the static `terminal-default` panel into a DB-backed + // ordinary terminal so its tab gets a real seq badge alongside any + // user-created terminals. + useEnsureDefaultTerminalOrdinary(); const handleAddTerminal = useCallback(async () => { if (!environmentId) return; try { - const result = await createUserShell(environmentId); - addTerminalPanel(result.terminalId, group.id, environmentId); + // Pass taskId so the backend creates a DB-backed ordinary terminal + // (with seq/kind) instead of a legacy passthrough shell. Also stamp + // taskID into the dockview panel params so the destroy cleanup + // passes ownership verification. + const result = await createUserShell(environmentId, { taskId: taskID ?? undefined }); + // Push the new ordinary terminal into the user-shell store + // synchronously so TerminalTab's store lookup finds it on first + // render. Without this, the tab would briefly show the panel's + // initial title ("Terminal {seq}") before the next WS list + // catches up and rewrites it to the normalized "Terminal". + addUserShell(environmentId, { + terminalId: result.terminalId, + kind: result.kind, + seq: result.seq, + displayName: result.displayName, + customName: null, + state: result.state ?? "open", + ptyStatus: result.ptyStatus ?? "stopped", + running: result.ptyStatus === "running", + label: result.label, + closable: result.closable ?? true, + initialCommand: result.initialCommand, + }); + // Panel title is always set to "Terminal" — TerminalTab pushes the + // custom name (or "Terminal") onto api.title via setTitle once it + // mounts; passing "Terminal" here avoids a "Terminal 1" → "Terminal" + // flash on first paint. + addTerminalPanel(result.terminalId, group.id, environmentId, taskID ?? undefined, "Terminal"); } catch (error) { console.error("Failed to create terminal:", error); } - }, [environmentId, addTerminalPanel, group.id]); + }, [environmentId, taskID, addTerminalPanel, addUserShell, group.id]); const handleRunScript = useCallback( async (scriptId: string) => { if (!environmentId) return; try { const result = await createUserShell(environmentId, { scriptId }); - addTerminalPanel(result.terminalId, group.id, environmentId); + const title = result.label ?? "Script"; + addTerminalPanel(result.terminalId, group.id, environmentId, undefined, title); } catch (error) { console.error("Failed to run script:", error); } @@ -122,7 +163,7 @@ export function LeftHeaderActions(props: IDockviewHeaderActionsProps) { command: devScript, label: "Dev Server", }); - addTerminalPanel(result.terminalId, group.id, environmentId); + addTerminalPanel(result.terminalId, group.id, environmentId, undefined, "Dev Server"); } catch (error) { console.error("Failed to start dev script:", error); } @@ -427,7 +468,13 @@ function TerminalScriptsDropdown({ if (!environmentId) return; try { const result = await createUserShell(environmentId, { scriptId }); - addTerminalPanel(result.terminalId, rightBottomGroupId ?? undefined, environmentId); + addTerminalPanel( + result.terminalId, + rightBottomGroupId ?? undefined, + environmentId, + undefined, + result.label ?? "Script", + ); } catch (error) { console.error("Failed to run script:", error); } @@ -488,6 +535,7 @@ function TerminalDevPreviewButton({ visible, }: TerminalDevPreviewButtonProps) { const activeSessionId = useAppStore((state) => state.tasks.activeSessionId); + const taskID = useAppStore((state) => state.tasks?.activeTaskId ?? null); const addBrowserPanel = useDockviewStore((s) => s.addBrowserPanel); const addTerminalPanel = useDockviewStore((s) => s.addTerminalPanel); const upsertProcessStatus = useAppStore((state) => state.upsertProcessStatus); @@ -506,14 +554,22 @@ function TerminalDevPreviewButton({ // Process may already be running } try { - const shell = await createUserShell(environmentId); - addTerminalPanel(shell.terminalId, rightBottomGroupId ?? undefined, environmentId); + const shell = await createUserShell(environmentId, { taskId: taskID ?? undefined }); + const title = shell.displayName ?? shell.label ?? "Terminal"; + addTerminalPanel( + shell.terminalId, + rightBottomGroupId ?? undefined, + environmentId, + taskID ?? undefined, + title, + ); } catch { // Terminal creation is best-effort } }, [ activeSessionId, environmentId, + taskID, addBrowserPanel, upsertProcessStatus, setActiveProcess, diff --git a/apps/web/components/task/dockview-layout-setup.ts b/apps/web/components/task/dockview-layout-setup.ts index d907c08b44..35435c0aa3 100644 --- a/apps/web/components/task/dockview-layout-setup.ts +++ b/apps/web/components/task/dockview-layout-setup.ts @@ -15,11 +15,16 @@ import { import { setEnvLayout } from "@/lib/local-storage"; import { panelPortalManager } from "@/lib/layout/panel-portal-manager"; import { stopVscode } from "@/lib/api/domains/vscode-api"; -import { stopUserShell } from "@/lib/api/domains/user-shell-api"; +import { parkUserShell, stopUserShell } from "@/lib/api/domains/user-shell-api"; // v2: bumped alongside DOCKVIEW_ENV_LAYOUT_PREFIX so the no-env fallback // also invalidates layouts saved under the previous caps. const LAYOUT_STORAGE_KEY = "dockview-layout-v2"; +const terminalTerminateClosePanelIds = new Set(); + +export function markTerminalPanelTerminateClose(panelId: string): void { + terminalTerminateClosePanelIds.add(panelId); +} /** * Pinned-column target enforcement. @@ -307,59 +312,86 @@ export function setupLayoutPersistence( }; } +/** When the last non-sidebar panel is closed while maximized, exit maximize + * and drop the closed panel from the restored pre-maximize layout. */ +function handleMaximizeExitOnLastClose( + api: DockviewReadyEvent["api"], + removedId: string, + nonSidebarRemaining: number, +): void { + if (!(useDockviewStore.getState().preMaximizeLayout !== null) || nonSidebarRemaining > 0) return; + requestAnimationFrame(() => { + useDockviewStore.getState().exitMaximizedLayout(); + requestAnimationFrame(() => { + const restoredPanel = api.getPanel(removedId); + if (restoredPanel) restoredPanel.api.close(); + }); + }); +} + +/** Resolve a session id whose env matches the closed panel's env, used for + * session-scoped stops like stopVscode. */ +function resolveSessionForEntry( + appStore: StoreApi, + entryEnvId: string | undefined, +): string | null { + const state = appStore.getState(); + const active = state.tasks.activeSessionId; + if (!entryEnvId) return active; + if (active && state.environmentIdBySessionId[active] === entryEnvId) return active; + const match = Object.entries(state.environmentIdBySessionId).find( + ([, eid]) => eid === entryEnvId, + ); + return match?.[0] ?? active; +} + +/** Tab close → ordinary terminals park (PTY + DB row survive, reappear in + * the "+" menu); scripts/bottom-panel/legacy passthrough still destroy. */ +function handleTerminalPanelClosed( + appStore: StoreApi, + panelId: string, + params: Record, +): void { + if (terminalTerminateClosePanelIds.delete(panelId)) return; + const terminalId = params.terminalId as string | undefined; + if (!terminalId) return; + const stampedEnv = params.environmentId as string | undefined; + const stampedTaskID = params.taskID as string | undefined; + const state = appStore.getState(); + const active = state.tasks.activeSessionId; + const fallbackEnv = active ? (state.environmentIdBySessionId[active] ?? null) : null; + const envForTerminal = stampedEnv || fallbackEnv; + if (!envForTerminal) return; + const shell = state.userShells.byEnvironmentId[envForTerminal]?.find( + (s) => s.terminalId === terminalId, + ); + if (shell?.kind === "ordinary") { + parkUserShell(terminalId, stampedTaskID).then( + () => state.updateUserShell(envForTerminal, terminalId, { state: "parked" }), + (err: unknown) => console.error("park terminal on tab close:", err), + ); + } else { + stopUserShell(envForTerminal, terminalId, stampedTaskID).catch((err: unknown) => + console.warn("stop terminal on tab close:", err), + ); + } +} + export function setupPortalCleanup( api: DockviewReadyEvent["api"], appStore: StoreApi, ): void { api.onDidRemovePanel((panel) => { if (useDockviewStore.getState().isRestoringLayout) return; - const isMax = useDockviewStore.getState().preMaximizeLayout !== null; - const remaining = api.panels.filter((p) => p.id !== panel.id); - const nonSidebar = remaining.filter((p) => p.api.component !== "sidebar"); - // If we're in maximize mode and the last non-sidebar panel was just closed, - // exit maximize to restore the pre-maximize layout (avoids empty view). - // Then remove the closed panel from the restored layout so it doesn't reappear. - if (isMax && nonSidebar.length === 0) { - const removedId = panel.id; - requestAnimationFrame(() => { - useDockviewStore.getState().exitMaximizedLayout(); - // exitMaximizedLayout schedules a rAF to finalize — wait for that, then - // remove the panel that was closed (it was re-created from preMaximizeLayout). - requestAnimationFrame(() => { - const restoredPanel = api.getPanel(removedId); - if (restoredPanel) { - restoredPanel.api.close(); - } - }); - }); - } + const nonSidebarRemaining = api.panels.filter( + (p) => p.id !== panel.id && p.api.component !== "sidebar", + ).length; + handleMaximizeExitOnLastClose(api, panel.id, nonSidebarRemaining); const entry = panelPortalManager.get(panel.id); - // vscode is session-scoped; resolve to a session in the entry's env (or - // the active session) for the stop call. - const sessionForApi = (() => { - const state = appStore.getState(); - const active = state.tasks.activeSessionId; - if (!entry?.envId) return active; - if (active && state.environmentIdBySessionId[active] === entry.envId) return active; - const match = Object.entries(state.environmentIdBySessionId).find( - ([, eid]) => eid === entry.envId, - ); - return match?.[0] ?? active; - })(); + const sessionForApi = resolveSessionForEntry(appStore, entry?.envId); if (entry?.component === "vscode" && sessionForApi) stopVscode(sessionForApi); - if (entry?.component === "terminal") { - const terminalId = entry.params.terminalId as string | undefined; - // Prefer the env id stamped into params at creation time — this - // survives task switches that happen between open and close. Fall - // back to the active session's env for legacy panels created before - // the param was added (e.g. layouts persisted from older releases). - const stampedEnv = entry.params.environmentId as string | undefined; - const state = appStore.getState(); - const active = state.tasks.activeSessionId; - const fallbackEnv = active ? (state.environmentIdBySessionId[active] ?? null) : null; - const envForTerminal = stampedEnv || fallbackEnv; - if (terminalId && envForTerminal) stopUserShell(envForTerminal, terminalId); - } + if (entry?.component === "terminal") + handleTerminalPanelClosed(appStore, panel.id, entry.params); panelPortalManager.release(panel.id); }); } diff --git a/apps/web/components/task/dockview-shared.tsx b/apps/web/components/task/dockview-shared.tsx index 69b28864bd..e1b0b7defa 100644 --- a/apps/web/components/task/dockview-shared.tsx +++ b/apps/web/components/task/dockview-shared.tsx @@ -28,6 +28,7 @@ import { ChangesTab } from "./changes-tab"; import { PlanTab } from "./plan-tab"; import { PreviewFileTab, PreviewDiffTab, PreviewCommitTab, PinnedDefaultTab } from "./preview-tab"; import { SessionTab } from "./session-tab"; +import { TerminalTab } from "./terminal-tab"; import { TerminalPanel } from "./terminal-panel"; import { BrowserPanel } from "./browser-panel"; import { VscodePanel } from "./vscode-panel"; @@ -135,6 +136,7 @@ export const dockviewTabComponents: Record< changesTab: ChangesTab, planTab: PlanTab, sessionTab: SessionTab, + terminalTab: TerminalTab, previewFileTab: PreviewFileTab, previewDiffTab: PreviewDiffTab, previewCommitTab: PreviewCommitTab, diff --git a/apps/web/components/task/dockview-watermark.tsx b/apps/web/components/task/dockview-watermark.tsx index 199f6997ca..f77fe5f19b 100644 --- a/apps/web/components/task/dockview-watermark.tsx +++ b/apps/web/components/task/dockview-watermark.tsx @@ -1,139 +1,172 @@ "use client"; -import { useCallback, useMemo } from "react"; +import { useCallback, useMemo, useState } from "react"; import type { IWatermarkPanelProps } from "dockview-react"; -import { - IconMessage, - IconFileText, - IconTerminal2, - IconDeviceDesktop, - IconGitBranch, - IconFolder, -} from "@tabler/icons-react"; +import { IconPlus, IconLayoutSidebarRightCollapse } from "@tabler/icons-react"; import { Button } from "@kandev/ui/button"; +import { DropdownMenu, DropdownMenuContent, DropdownMenuTrigger } from "@kandev/ui/dropdown-menu"; import { useDockviewStore } from "@/lib/state/dockview-store"; -import { PANEL_REGISTRY } from "@/lib/state/layout-manager"; +import { useAppStore } from "@/components/state-provider"; import { useEnvironmentId } from "@/hooks/use-environment-session-id"; +import { useUserShells } from "@/hooks/domains/session/use-user-shells"; +import { useTaskPR } from "@/hooks/domains/github/use-task-pr"; import { createUserShell } from "@/lib/api/domains/user-shell-api"; +import { AddPanelMenuItems } from "./dockview-add-panel-items"; +import { NewSessionDialog } from "./new-session-dialog"; +import { useActiveSessionDevScript } from "./repository-scripts-menu"; -type PanelOption = { - id: string; - label: string; - icon: React.ElementType; - /** Whether only one instance is allowed (focus existing instead of adding). */ - singleton?: boolean; -}; - -const PANEL_OPTIONS: PanelOption[] = [ - { id: "chat", label: "Agent", icon: IconMessage, singleton: true }, - { id: "plan", label: "Plan", icon: IconFileText }, - { id: "terminal", label: "Terminal", icon: IconTerminal2 }, - { id: "browser", label: "Browser", icon: IconDeviceDesktop }, - { id: "changes", label: "Changes", icon: IconGitBranch, singleton: true }, - { id: "files", label: "Files", icon: IconFolder, singleton: true }, -]; - +/** + * Watermark rendered by Dockview when a group becomes empty (e.g. after + * the user splits a group and the new half has no panels yet). Mirrors + * the header "+" menu so the user gets the same rich picker — existing + * sessions and terminals are listed (clicking re-focuses an open tab or + * re-opens a parked one) instead of always minting a fresh panel. + * + * Before this, the watermark's terminal button called createUserShell + * without a taskId, so it produced a legacy passthrough shell that + * never appeared in the user-shell list at all. + */ export function DockviewWatermark({ containerApi, group }: IWatermarkPanelProps) { + const groupId = group?.id; const environmentId = useEnvironmentId(); - - const handleAdd = useCallback( - async (option: PanelOption) => { - const groupId = group?.id; - - if (option.id === "terminal") { - let terminalId = `terminal-${Date.now()}`; - if (environmentId) { - try { - const result = await createUserShell(environmentId); - terminalId = result.terminalId; - } catch { - // Fall back to default terminal ID - } - } - containerApi.addPanel({ - id: terminalId, - component: "terminal", - title: "Terminal", - // environmentId is stamped into params so cleanup can call - // stopUserShell with the correct env even after task switches. - params: { terminalId, environmentId: environmentId ?? undefined }, - ...(groupId ? { position: { referenceGroup: groupId } } : {}), - }); - return; - } - - if (option.id === "browser") { - const browserId = `browser:${Date.now()}`; - containerApi.addPanel({ - id: browserId, - component: "browser", - title: "Browser", - params: { url: "" }, - ...(groupId ? { position: { referenceGroup: groupId } } : {}), - }); - return; - } - - // Singleton panels — focus if exists, otherwise add to this group - if (option.singleton) { - const existing = containerApi.getPanel(option.id); - if (existing) { - existing.api.setActive(); - return; - } - } - - const config = PANEL_REGISTRY[option.id]; - if (!config) return; - - containerApi.addPanel({ - id: option.id, - component: config.component, - title: config.title, - ...(config.tabComponent ? { tabComponent: config.tabComponent } : {}), - ...(groupId ? { position: { referenceGroup: groupId } } : {}), - }); - }, - [containerApi, group, environmentId], - ); - - // Check which singletons already exist so we can hide them - const existingPanels = useMemo(() => { - const ids = new Set(); - try { - for (const panel of containerApi.panels) { - ids.add(panel.id); - } - } catch { - // API may not be ready - } - return ids; - }, [containerApi]); - + const taskID = useAppStore((s) => s.tasks?.activeTaskId ?? null); const sidebarGroupId = useDockviewStore((s) => s.sidebarGroupId); - const isSidebarGroup = group?.id === sidebarGroupId; - if (isSidebarGroup) return null; + const [showNewSessionDialog, setShowNewSessionDialog] = useState(false); + + // Eagerly populate the user-shell store so the Terminals submenu has + // its list ready the moment the watermark opens. + useUserShells(environmentId, taskID); + + const menuState = useWatermarkMenuState(containerApi, taskID); + const handlers = useWatermarkHandlers(groupId, environmentId, taskID); + + if (group?.id === sidebarGroupId) return null; + if (!groupId) return null; return ( -
-
- {PANEL_OPTIONS.map((option) => { - if (option.singleton && existingPanels.has(option.id)) return null; - const Icon = option.icon; - return ( - - ); - })} -
+
+ +

Empty group

+ + + + + + setShowNewSessionDialog(true)} + onAddTerminal={handlers.handleAddTerminal} + onRunScript={handlers.handleRunScript} + onRunDevScript={handlers.handleRunDevScript} + /> + + + {taskID && ( + + )}
); } + +function useWatermarkMenuState( + containerApi: IWatermarkPanelProps["containerApi"], + taskID: string | null, +) { + const activeSessionId = useAppStore((s) => s.tasks.activeSessionId); + const isPassthrough = useAppStore((s) => { + if (!activeSessionId) return false; + return s.taskSessions.items[activeSessionId]?.is_passthrough === true; + }); + const { prs } = useTaskPR(taskID); + return useMemo( + () => ({ + taskId: taskID, + isPassthrough, + hasChanges: Boolean(containerApi.getPanel("changes") ?? containerApi.getPanel("diff-files")), + hasFiles: Boolean(containerApi.getPanel("files") ?? containerApi.getPanel("all-files")), + prs, + }), + [taskID, isPassthrough, containerApi, prs], + ); +} + +function useWatermarkHandlers( + groupId: string | undefined, + environmentId: string | null, + taskID: string | null, +) { + const addTerminalPanel = useDockviewStore((s) => s.addTerminalPanel); + const addUserShell = useAppStore((s) => s.addUserShell); + const devScript = useActiveSessionDevScript(); + + const handleAddTerminal = useCallback(async () => { + if (!environmentId || !groupId) return; + try { + const result = await createUserShell(environmentId, { taskId: taskID ?? undefined }); + addUserShell(environmentId, { + terminalId: result.terminalId, + kind: result.kind, + seq: result.seq, + displayName: result.displayName, + customName: null, + state: result.state ?? "open", + ptyStatus: result.ptyStatus ?? "stopped", + running: result.ptyStatus === "running", + label: result.label, + closable: result.closable ?? true, + initialCommand: result.initialCommand, + }); + addTerminalPanel(result.terminalId, groupId, environmentId, taskID ?? undefined, "Terminal"); + } catch (error) { + console.error("Failed to create terminal:", error); + } + }, [environmentId, taskID, groupId, addTerminalPanel, addUserShell]); + + const handleRunScript = useCallback( + async (scriptId: string) => { + if (!environmentId || !groupId) return; + try { + const result = await createUserShell(environmentId, { scriptId }); + addTerminalPanel( + result.terminalId, + groupId, + environmentId, + undefined, + result.label ?? "Script", + ); + } catch (error) { + console.error("Failed to run script:", error); + } + }, + [environmentId, groupId, addTerminalPanel], + ); + + const handleRunDevScript = useCallback(async () => { + if (!environmentId || !devScript || !groupId) return; + try { + const result = await createUserShell(environmentId, { + command: devScript, + label: "Dev Server", + }); + addTerminalPanel(result.terminalId, groupId, environmentId, undefined, "Dev Server"); + } catch (error) { + console.error("Failed to start dev script:", error); + } + }, [environmentId, devScript, groupId, addTerminalPanel]); + + return { handleAddTerminal, handleRunScript, handleRunDevScript }; +} diff --git a/apps/web/components/task/mobile/session-tablet-layout.tsx b/apps/web/components/task/mobile/session-tablet-layout.tsx index 3f164a65e2..f411426022 100644 --- a/apps/web/components/task/mobile/session-tablet-layout.tsx +++ b/apps/web/components/task/mobile/session-tablet-layout.tsx @@ -145,7 +145,7 @@ export const SessionTabletLayout = memo(function SessionTabletLayout({ }); return ( -
+
Promise | void; + onDestroy: (id: string) => Promise | void; +}) { + const [open, setOpen] = useState(false); + const wrapperRef = useRef(null); + + useEffect(() => { + if (!open) return; + const onDocumentClick = (event: globalThis.MouseEvent) => { + if (!wrapperRef.current) return; + if (!wrapperRef.current.contains(event.target as Node)) setOpen(false); + }; + document.addEventListener("mousedown", onDocumentClick); + return () => document.removeEventListener("mousedown", onDocumentClick); + }, [open]); + + if (parkedTerminals.length === 0) return null; + + return ( +
+ + {open && ( +
+ {parkedTerminals.map((t) => ( + + ))} +
+ )} +
+ ); +} diff --git a/apps/web/components/task/session-reopen-menu.tsx b/apps/web/components/task/session-reopen-menu.tsx index 9d4f95f9aa..e2211c5381 100644 --- a/apps/web/components/task/session-reopen-menu.tsx +++ b/apps/web/components/task/session-reopen-menu.tsx @@ -1,7 +1,7 @@ "use client"; import { useCallback, useMemo } from "react"; -import { IconStar } from "@tabler/icons-react"; +import { IconMessagePlus, IconStar } from "@tabler/icons-react"; import { DropdownMenuItem, DropdownMenuLabel, @@ -33,7 +33,19 @@ function resolveAgentInfo( * Each item shows session number, agent label, primary star, and state icon. * Clicking focuses an existing tab or re-opens a closed one. */ -export function SessionReopenMenuItems({ taskId, groupId }: { taskId: string; groupId?: string }) { +export function SessionReopenMenuItems({ + taskId, + groupId, + onNewSession, +}: { + taskId: string; + groupId?: string; + /** + * Click handler for the leading "New Agent" item rendered as the + * first row under the section label. Omit to hide the row. + */ + onNewSession?: () => void; +}) { const { sessions } = useTaskSessions(taskId); const api = useDockviewStore((s) => s.api); const centerGroupId = useDockviewStore((s) => s.centerGroupId); @@ -66,11 +78,25 @@ export function SessionReopenMenuItems({ taskId, groupId }: { taskId: string; gr [api, centerGroupId], ); - if (sortedSessions.length === 0) return null; + // Render the section even when there are no sessions yet — the leading + // "New Agent" row should still be reachable from the menu. Hide the + // whole block only when neither the create handler nor any sessions + // are present (e.g. unmounted contexts). + if (sortedSessions.length === 0 && !onNewSession) return null; return ( <> Agents + {onNewSession && ( + + + New Agent + + )} {sortedSessions.map((session, index) => { const info = resolveAgentInfo(session, profilesById); const isPrimary = session.id === primarySessionId; @@ -82,7 +108,12 @@ export function SessionReopenMenuItems({ taskId, groupId }: { taskId: string; gr className={`cursor-pointer text-xs gap-1.5 ${isOpen ? "opacity-50" : ""}`} data-testid={`reopen-session-${session.id}`} > - #{index + 1} + + #{index + 1} + {info.agentName && ( )} diff --git a/apps/web/components/task/task-right-panel.tsx b/apps/web/components/task/task-right-panel.tsx index 5c6bfd58da..35dbc472a9 100644 --- a/apps/web/components/task/task-right-panel.tsx +++ b/apps/web/components/task/task-right-panel.tsx @@ -21,6 +21,7 @@ import { useDefaultLayout } from "@/lib/layout/use-default-layout"; import { SessionTabs, type SessionTab } from "@/components/session-tabs"; import { useRepositoryScripts } from "@/hooks/domains/workspace/use-repository-scripts"; import { useTerminals } from "@/hooks/domains/session/use-terminals"; +import { ParkedTerminalsMenu } from "@/components/task/parked-terminals-menu"; import type { RepositoryScript } from "@/lib/types/http"; import type { Terminal } from "@/hooks/domains/session/use-terminals"; @@ -34,21 +35,53 @@ type TaskRightPanelProps = { const DEFAULT_RIGHT_LAYOUT: Record = { top: 55, bottom: 45 }; -function buildTerminalTabs( - terminals: Terminal[], - handleCloseDevTab: (event: MouseEvent) => void, - handleCloseTab: (event: MouseEvent, id: string) => void, -): SessionTab[] { - return terminals.map((terminal) => ({ - id: terminal.id, - label: terminal.label, - className: terminal.closable ? "group flex items-center cursor-pointer" : "cursor-pointer", - closable: terminal.closable, - onClose: - terminal.type === "dev-server" - ? handleCloseDevTab - : (e: MouseEvent) => handleCloseTab(e, terminal.id), - })); +function pickCurrentRenameValue(terminal: Terminal): string { + if (terminal.customName && terminal.customName !== "") return terminal.customName; + if (terminal.seq) return `Terminal ${terminal.seq}`; + return terminal.label; +} + +type TerminalTabsBuilderOpts = { + terminals: Terminal[]; + handleCloseDevTab: (event: MouseEvent) => void; + handleCloseTab: (event: MouseEvent, id: string) => void; + onContextMenu?: (event: MouseEvent, terminal: Terminal) => void; + onDoubleClick?: (event: MouseEvent, terminal: Terminal) => void; +}; + +/** + * Builds the SessionTab[] for the right-panel strip. + * + * Ordinary terminals get a `#N` badge (sequence number from the DB row) and + * a context-menu hook for rename/destroy. Dev-server and script terminals + * preserve their existing label/closable semantics. + */ +function buildTerminalTabs({ + terminals, + handleCloseDevTab, + handleCloseTab, + onContextMenu, + onDoubleClick, +}: TerminalTabsBuilderOpts): SessionTab[] { + return terminals.map((terminal) => { + const isOrdinary = terminal.kind === "ordinary"; + const isDev = terminal.type === "dev-server"; + return { + id: terminal.id, + label: terminal.label, + badge: isOrdinary && terminal.seq ? `#${terminal.seq}` : undefined, + truncate: !isOrdinary, + className: terminal.closable ? "group flex items-center cursor-pointer" : "cursor-pointer", + closable: terminal.closable, + onClose: isDev ? handleCloseDevTab : (e: MouseEvent) => handleCloseTab(e, terminal.id), + onContextMenu: + isOrdinary && onContextMenu ? (e: MouseEvent) => onContextMenu(e, terminal) : undefined, + onDoubleClick: + isOrdinary && onDoubleClick ? (e: MouseEvent) => onDoubleClick(e, terminal) : undefined, + testId: `terminal-tab-${terminal.id}`, + closeTestId: `terminal-tab-close-${terminal.id}`, + }; + }); } function useRightPanelScripts(repositoryId: string | null, initialScripts: RepositoryScript[]) { @@ -91,6 +124,8 @@ function useRightPanelTabs({ terminals, handleCloseDevTab, handleCloseTab, + renameTerminal, + destroyTerminal, sessionId, setRightPanelActiveTab, }: { @@ -98,13 +133,46 @@ function useRightPanelTabs({ terminals: Terminal[]; handleCloseDevTab: (event: MouseEvent) => void; handleCloseTab: (event: MouseEvent, terminalId: string) => void; + renameTerminal: (id: string, name: string | null) => Promise | void; + destroyTerminal: (id: string) => Promise | void; sessionId: string | null; setRightPanelActiveTab: (sessionId: string, tabId: string) => void; }) { + const onContextMenu = useCallback( + (event: MouseEvent, terminal: Terminal) => { + event.preventDefault(); + // Browser prompt() is intentionally low-tech; the next iteration + // promotes this to a real shadcn ContextMenu, but window.prompt + // ships the renameable-terminal requirement today. + const current = pickCurrentRenameValue(terminal); + const choice = window.prompt( + `Rename terminal (leave empty to reset, type "destroy" to remove and stop the PTY).`, + current, + ); + if (choice === null) return; + const trimmed = choice.trim(); + if (trimmed.toLowerCase() === "destroy") { + void destroyTerminal(terminal.id); + return; + } + void renameTerminal(terminal.id, trimmed === "" ? null : trimmed); + }, + [renameTerminal, destroyTerminal], + ); + const tabs: SessionTab[] = useMemo(() => { const commandsTabs: SessionTab[] = hasScripts ? [{ id: "commands", label: "Commands" }] : []; - return [...commandsTabs, ...buildTerminalTabs(terminals, handleCloseDevTab, handleCloseTab)]; - }, [hasScripts, terminals, handleCloseDevTab, handleCloseTab]); + return [ + ...commandsTabs, + ...buildTerminalTabs({ + terminals, + handleCloseDevTab, + handleCloseTab, + onContextMenu, + onDoubleClick: onContextMenu, + }), + ]; + }, [hasScripts, terminals, handleCloseDevTab, handleCloseTab, onContextMenu]); const handleTabChange = useCallback( (value: string) => { @@ -170,6 +238,9 @@ type RightPanelContentProps = { scripts: RepositoryScript[]; handleRunCommand: (script: RepositoryScript) => void; terminals: Terminal[]; + parkedTerminals: Terminal[]; + resumeTerminal: (id: string) => Promise | void; + destroyTerminal: (id: string) => Promise | void; environmentId: string | null; devProcessId: string | null | undefined; devOutput: string | undefined; @@ -190,6 +261,9 @@ function RightPanelContent({ scripts, handleRunCommand, terminals, + parkedTerminals, + resumeTerminal, + destroyTerminal, environmentId, devProcessId, devOutput, @@ -231,6 +305,15 @@ function RightPanelContent({ collapsible isCollapsed={isBottomCollapsed} onToggleCollapse={() => setIsBottomCollapsed(true)} + rightContent={ + parkedTerminals.length > 0 ? ( + + ) : undefined + } className="flex-1 min-h-0" > @@ -276,12 +359,16 @@ const TaskRightPanel = memo(function TaskRightPanel({ // Use the terminals hook — env-keyed for shell ops, session-keyed for tab UX const { terminals, + parkedTerminals, activeTab, terminalTabValue, addTerminal, handleCloseDevTab: baseHandleCloseDevTab, handleCloseTab, handleRunCommand, + renameTerminal, + resumeTerminal, + destroyTerminal, isStoppingDev, devProcessId, devOutput, @@ -311,6 +398,8 @@ const TaskRightPanel = memo(function TaskRightPanel({ terminals, handleCloseDevTab, handleCloseTab, + renameTerminal, + destroyTerminal, sessionId, setRightPanelActiveTab, }); @@ -329,6 +418,9 @@ const TaskRightPanel = memo(function TaskRightPanel({ scripts={scripts} handleRunCommand={handleRunCommand} terminals={terminals} + parkedTerminals={parkedTerminals} + resumeTerminal={resumeTerminal} + destroyTerminal={destroyTerminal} environmentId={environmentId} devProcessId={devProcessId} devOutput={devOutput} diff --git a/apps/web/components/task/terminal-reopen-menu.tsx b/apps/web/components/task/terminal-reopen-menu.tsx new file mode 100644 index 0000000000..9b3d625b7c --- /dev/null +++ b/apps/web/components/task/terminal-reopen-menu.tsx @@ -0,0 +1,257 @@ +"use client"; + +import { useCallback } from "react"; +import type { DockviewApi } from "dockview-react"; +import { IconTerminal2, IconX } from "@tabler/icons-react"; +import { + DropdownMenuItem, + DropdownMenuLabel, + DropdownMenuSeparator, +} from "@kandev/ui/dropdown-menu"; +import { useAppStore } from "@/components/state-provider"; +import { useDockviewStore } from "@/lib/state/dockview-store"; +import type { UserShellInfo } from "@/lib/state/slices"; +import { destroyUserShell, resumeUserShell } from "@/lib/api/domains/user-shell-api"; +import { useEnvironmentId } from "@/hooks/use-environment-session-id"; +import { markTerminalPanelTerminateClose } from "./dockview-layout-setup"; + +const EMPTY_SHELLS: UserShellInfo[] = []; + +/** + * Lists ordinary user terminals (open and parked alike) inside the + * dockview "+" menu so a user can jump to, re-open, or terminate a + * terminal that isn't already a panel. + * + * - Already-mounted terminals are dimmed; clicking re-focuses the + * existing panel. Parked terminals are visually identical — the row + * doesn't advertise parked state, since clicking just brings the + * terminal back into view either way. + * - The trailing × terminates (destroys) the terminal permanently — + * PTY killed, DB row deleted, no return after reload. + */ +export function TerminalReopenMenuItems({ + groupId, + onNewTerminal, +}: { + groupId: string; + /** + * Click handler for the leading "New Terminal" item rendered as the + * first row under the section label. Omit to hide the row. + */ + onNewTerminal?: () => void; +}) { + const environmentId = useEnvironmentId(); + const taskID = useAppStore((s) => s.tasks?.activeTaskId ?? null); + const shells = useAppStore((s) => { + if (!environmentId) return EMPTY_SHELLS; + return s.userShells.byEnvironmentId[environmentId] ?? EMPTY_SHELLS; + }); + const updateUserShell = useAppStore((s) => s.updateUserShell); + const removeUserShellStore = useAppStore((s) => s.removeUserShell); + const api = useDockviewStore((s) => s.api); + const addTerminalPanel = useDockviewStore((s) => s.addTerminalPanel); + const ordinary = shells.filter((s) => s.kind === "ordinary"); + const handleDestroyRow = useDestroyTerminalRow({ + api, + environmentId, + taskID, + removeUserShellStore, + }); + const handleClick = useOpenTerminalPanel({ + api, + addTerminalPanel, + environmentId, + taskID, + groupId, + updateUserShell, + }); + + // Show the section header + the New Terminal row whenever onNewTerminal + // is supplied, even if no ordinary terminals exist yet. This puts the + // create action under the section label (matching the Agents pattern). + if (ordinary.length === 0 && !onNewTerminal) return null; + + return ( + <> + Terminals + {onNewTerminal && ( + + + New Terminal + + )} + {ordinary + .sort((a, b) => (a.seq ?? 0) - (b.seq ?? 0)) + .map((shell) => ( + + ))} + + + ); +} + +type DestroyTerminalRowOptions = { + api: DockviewApi | null; + environmentId: string | null; + taskID: string | null; + removeUserShellStore: (environmentId: string, terminalId: string) => void; +}; + +function useDestroyTerminalRow({ + api, + environmentId, + taskID, + removeUserShellStore, +}: DestroyTerminalRowOptions) { + return useCallback( + async (event: React.MouseEvent, shell: UserShellInfo) => { + event.preventDefault(); + event.stopPropagation(); + if (!environmentId) return; + const label = shell.seq != null ? `terminal #${shell.seq}` : "this terminal"; + if (!window.confirm(`Terminate ${label}? This kills the running PTY.`)) return; + + try { + await destroyUserShell(environmentId, shell.terminalId, taskID ?? undefined); + } catch (error) { + console.error("terminate terminal from reopen menu:", error); + return; + } + + removeUserShellStore(environmentId, shell.terminalId); + const existing = api ? findExistingTerminalPanel(api, shell.terminalId) : null; + if (existing) { + markTerminalPanelTerminateClose(existing.api.id); + existing.api.close(); + } + }, + [api, environmentId, taskID, removeUserShellStore], + ); +} + +type OpenTerminalPanelOptions = { + api: DockviewApi | null; + addTerminalPanel: ( + terminalId?: string, + groupId?: string, + environmentId?: string, + taskID?: string, + title?: string, + ) => void; + environmentId: string | null; + taskID: string | null; + groupId: string; + updateUserShell: (environmentId: string, terminalId: string, patch: { state: "open" }) => void; +}; + +function useOpenTerminalPanel({ + api, + addTerminalPanel, + environmentId, + taskID, + groupId, + updateUserShell, +}: OpenTerminalPanelOptions) { + return useCallback( + async (terminalId: string, state: string | undefined, displayName: string | undefined) => { + if (!api) return; + const existing = findExistingTerminalPanel(api, terminalId); + if (existing) { + existing.api.setActive(); + return; + } + if (state === "parked" && environmentId) { + try { + await resumeUserShell(terminalId, taskID ?? undefined); + updateUserShell(environmentId, terminalId, { state: "open" }); + } catch (error) { + console.error("resume terminal:", error); + return; + } + } + addTerminalPanel( + terminalId, + groupId, + environmentId ?? undefined, + taskID ?? undefined, + displayName, + ); + }, + [api, addTerminalPanel, environmentId, taskID, updateUserShell, groupId], + ); +} + +/** + * Find a dockview panel that represents the given terminalId. Falls back + * to matching panels whose `params.terminalId` equals the requested id, + * so the default-migrated `terminal-default` panel (whose id never + * changes but whose params.terminalId is a `shell-`) resolves + * correctly. + */ +function findExistingTerminalPanel(api: DockviewApi, terminalId: string) { + const direct = api.getPanel(terminalId); + if (direct) return direct; + return ( + api.panels.find((p) => { + const params = (p.params ?? null) as Record | null; + return typeof params?.terminalId === "string" && params.terminalId === terminalId; + }) ?? null + ); +} + +type ShellRow = UserShellInfo; + +function TerminalReopenRow({ + shell, + isOpen, + onClick, + onDestroy, +}: { + shell: ShellRow; + isOpen: boolean; + onClick: (terminalId: string, state: string | undefined, label: string) => void; + onDestroy: (event: React.MouseEvent, shell: ShellRow) => void; +}) { + // Ordinary terminals don't need the backend "Terminal {seq}" suffix + // in the label — the seq lives in the adjacent badge, so the row + // reads "[N] Terminal" (or "[N] custom name") with no duplication. + const label = shell.customName && shell.customName !== "" ? shell.customName : "Terminal"; + return ( + onClick(shell.terminalId, shell.state, label)} + className={`cursor-pointer text-xs gap-1.5 ${isOpen ? "opacity-50" : ""}`} + data-testid={`reopen-terminal-${shell.terminalId}`} + > + {shell.seq != null && ( + + #{shell.seq} + + )} + + {label} + + + ); +} diff --git a/apps/web/components/task/terminal-tab.tsx b/apps/web/components/task/terminal-tab.tsx new file mode 100644 index 0000000000..3988882fc1 --- /dev/null +++ b/apps/web/components/task/terminal-tab.tsx @@ -0,0 +1,316 @@ +"use client"; + +import { useCallback, useEffect, useRef, useState } from "react"; +import { DockviewDefaultTab, type IDockviewPanelHeaderProps } from "dockview-react"; +import { + ContextMenu, + ContextMenuContent, + ContextMenuItem, + ContextMenuSeparator, + ContextMenuTrigger, +} from "@kandev/ui/context-menu"; +import { useAppStore } from "@/components/state-provider"; +import { destroyUserShell, renameUserShell } from "@/lib/api/domains/user-shell-api"; +import { markTerminalPanelTerminateClose } from "./dockview-layout-setup"; + +/** + * Custom dockview tab for terminal panels. + * + * Mirrors the session-tab badge behaviour: the `#N` pill only renders when + * there's more than one ordinary terminal in the active task — a single + * terminal needs no disambiguation. + * + * The tab exposes a right-click context menu with Rename / Terminate; + * choosing Rename swaps the title in place for an editable input. + */ +type StampedParams = { + terminalId: string; + taskID: string | undefined; + environmentId: string | undefined; +}; + +function extractParams(props: IDockviewPanelHeaderProps): StampedParams { + const panelParams = (props.params ?? {}) as Record; + return { + terminalId: (panelParams.terminalId as string | undefined) ?? props.api.id, + taskID: panelParams.taskID as string | undefined, + environmentId: panelParams.environmentId as string | undefined, + }; +} + +/** + * Tab title text — intentionally drops the backend's "Terminal {seq}" + * suffix so the title reads "Terminal" and the seq lives only in the + * sibling badge (mirroring session-tab's pattern where the agent name is + * the title and the seq is a separate pill before it). + * + * Custom names override the default; legacy passthrough shells keep + * their server-supplied label (e.g. "Script", "Dev Server"). + */ +function pickDisplayName( + shell: { kind?: string; customName?: string | null; label?: string } | null, + fallback: string, +): string { + if (shell?.customName && shell.customName !== "") return shell.customName; + if (shell?.kind === "ordinary") return "Terminal"; + if (shell?.label) return shell.label; + return fallback; +} + +function useTerminalTabState(stampedEnv: string | undefined, terminalId: string, apiTitle: string) { + const shell = useAppStore((s) => { + if (!stampedEnv) return null; + const list = s.userShells.byEnvironmentId[stampedEnv] ?? []; + return list.find((it) => it.terminalId === terminalId) ?? null; + }); + const ordinaryCount = useAppStore((s) => { + if (!stampedEnv) return 0; + const list = s.userShells.byEnvironmentId[stampedEnv] ?? []; + return list.filter((it) => it.kind === "ordinary").length; + }); + const isOrdinary = shell?.kind === "ordinary"; + const seq = shell?.seq; + const showBadge = isOrdinary && ordinaryCount > 1 && typeof seq === "number"; + const displayName = pickDisplayName(shell, apiTitle); + return { shell, isOrdinary, seq, showBadge, displayName }; +} + +export function TerminalTab(props: IDockviewPanelHeaderProps) { + const { terminalId, taskID: stampedTaskID, environmentId: stampedEnv } = extractParams(props); + const activeTaskID = useAppStore((s) => s.tasks?.activeTaskId ?? null); + const taskID = stampedTaskID ?? activeTaskID ?? null; + const { shell, isOrdinary, seq, showBadge, displayName } = useTerminalTabState( + stampedEnv, + terminalId, + props.api.title ?? "Terminal", + ); + + // DockviewDefaultTab reads the title from `api.title` directly and + // ignores prop overrides — push the corrected title onto the api so + // the default-tab body re-renders the right text. + useEffect(() => { + if (props.api.title !== displayName) props.api.setTitle(displayName); + }, [props.api, displayName]); + + const [isRenaming, setIsRenaming] = useState(false); + const handleCommitRename = useRenameCommitter({ + isOrdinary, + stampedEnv, + terminalId, + taskID, + currentCustomName: shell?.customName ?? null, + onDone: () => setIsRenaming(false), + }); + + const renameInitial = + shell?.customName && shell.customName !== "" ? shell.customName : displayName; + const seqBadgeForInput = showBadge && typeof seq === "number" ? seq : null; + + return ( + + + {isRenaming ? ( + setIsRenaming(false)} + /> + ) : ( + + )} + + setIsRenaming(true)} + onClosePanel={() => props.api.close()} + onTerminatePanel={() => { + markTerminalPanelTerminateClose(props.api.id); + props.api.close(); + }} + /> + + ); +} + +function useRenameCommitter({ + isOrdinary, + stampedEnv, + terminalId, + taskID, + currentCustomName, + onDone, +}: { + isOrdinary: boolean; + stampedEnv: string | undefined; + terminalId: string; + taskID: string | null; + currentCustomName: string | null; + onDone: () => void; +}) { + const updateUserShell = useAppStore((s) => s.updateUserShell); + return useCallback( + async (next: string) => { + onDone(); + if (!isOrdinary || !stampedEnv) return; + const normalized = next.trim() === "" ? null : next.trim(); + if (currentCustomName === normalized) return; + try { + await renameUserShell(terminalId, normalized, taskID ?? undefined); + updateUserShell(stampedEnv, terminalId, { customName: normalized }); + } catch (error) { + console.error("rename terminal:", error); + } + }, + [isOrdinary, stampedEnv, terminalId, taskID, currentCustomName, updateUserShell, onDone], + ); +} + +function TerminalTabBody({ + showBadge, + seq, + // `displayName` is computed in the parent but consumed via the + // api.setTitle effect — drop it here so it doesn't leak into the + // DOM via the {...props} spread below (React warning otherwise). + displayName: _displayName, + ...props +}: IDockviewPanelHeaderProps & { + showBadge: boolean; + seq: number | undefined; + displayName: string; +}) { + void _displayName; + return ( +
+ {showBadge && ( + + {seq} + + )} + +
+ ); +} + +function TerminalTabRenameInput({ + initial, + seqBadge, + onCommit, + onCancel, +}: { + initial: string; + seqBadge: number | null; + onCommit: (next: string) => void; + onCancel: () => void; +}) { + const [value, setValue] = useState(initial); + const inputRef = useRef(null); + + useEffect(() => { + inputRef.current?.focus(); + inputRef.current?.select(); + }, []); + + return ( +
e.stopPropagation()} + onMouseDown={(e) => e.stopPropagation()} + > + {seqBadge != null && ( + + {seqBadge} + + )} + setValue(e.target.value)} + onKeyDown={(e) => { + if (e.key === "Enter") { + e.preventDefault(); + onCommit(value); + } else if (e.key === "Escape") { + e.preventDefault(); + onCancel(); + } + // Don't let dockview see typed keys as shortcuts. + e.stopPropagation(); + }} + onBlur={() => onCommit(value)} + data-testid="terminal-tab-rename-input" + className="h-5 min-w-[6rem] max-w-[14rem] rounded border border-input bg-background px-1 text-xs outline-none focus:ring-1 focus:ring-ring" + /> +
+ ); +} + +function TerminalTabMenu({ + terminalId, + taskID, + environmentId, + canMutate, + onStartRename, + onClosePanel, + onTerminatePanel, +}: { + terminalId: string; + taskID: string | null; + environmentId: string | null; + canMutate: boolean; + onStartRename: () => void; + onClosePanel: () => void; + onTerminatePanel: () => void; +}) { + const removeUserShellStore = useAppStore((s) => s.removeUserShell); + + const handleTerminate = useCallback(async () => { + if (!environmentId) return; + if (!canMutate) { + onClosePanel(); + return; + } + try { + await destroyUserShell(environmentId, terminalId, taskID ?? undefined); + removeUserShellStore(environmentId, terminalId); + onTerminatePanel(); + } catch (error) { + console.error("terminate terminal:", error); + } + }, [ + canMutate, + environmentId, + terminalId, + taskID, + removeUserShellStore, + onClosePanel, + onTerminatePanel, + ]); + + return ( + + {canMutate && ( + <> + Rename… + + + )} + + Terminate + + + ); +} diff --git a/apps/web/e2e/pages/session-page.ts b/apps/web/e2e/pages/session-page.ts index 91659a9e92..280b93fb8b 100644 --- a/apps/web/e2e/pages/session-page.ts +++ b/apps/web/e2e/pages/session-page.ts @@ -945,7 +945,7 @@ export class SessionPage { /** All session reopen items in the + dropdown. */ sessionReopenItems(): Locator { - return this.page.locator("[data-testid^='reopen-session-']"); + return this.page.locator("[role='menuitem'][data-testid^='reopen-session-']"); } /** All session tabs in dockview (panels using the sessionTab tab component). */ diff --git a/apps/web/e2e/tests/session/dockview-repository-scripts.spec.ts b/apps/web/e2e/tests/session/dockview-repository-scripts.spec.ts index b8675176d3..b34cdddc93 100644 --- a/apps/web/e2e/tests/session/dockview-repository-scripts.spec.ts +++ b/apps/web/e2e/tests/session/dockview-repository-scripts.spec.ts @@ -71,7 +71,7 @@ test.describe("Dockview repository scripts in + menu", () => { const session = await seedTaskWithSession(testPage, apiClient, seedData, "No Scripts"); await session.addPanelButton().click(); - await expect(testPage.getByRole("menuitem", { name: "Terminal" })).toBeVisible(); + await expect(testPage.getByTestId("new-terminal-button")).toBeVisible(); await expect(testPage.getByText("Scripts", { exact: true })).toHaveCount(0); await expect(testPage.getByTestId("run-dev-script")).toHaveCount(0); }); diff --git a/apps/web/e2e/tests/task/session-isolation.spec.ts b/apps/web/e2e/tests/task/session-isolation.spec.ts index af3f31dee3..96f8be18dd 100644 --- a/apps/web/e2e/tests/task/session-isolation.spec.ts +++ b/apps/web/e2e/tests/task/session-isolation.spec.ts @@ -56,17 +56,13 @@ test.describe("Session isolation", () => { // 5. Navigate to the session-less task directly via URL await testPage.goto(`/t/${taskWithoutSession.id}`); - await session.waitForLoad(); + await expect(testPage.getByRole("link", { name: "Task Without Session" })).toBeVisible({ + timeout: 5_000, + }); // 6. The chat panel should NOT show the message from the previous task // This is the core assertion - we're testing that messages don't leak await expect(chatPanel.getByText(SIMPLE_MOCK_RESPONSE)).not.toBeVisible({ timeout: 5_000 }); - - // 7. Also verify the task title in the page matches the new task - // (ensuring we actually navigated to the correct task) - await expect(testPage.getByRole("link", { name: "Task Without Session" })).toBeVisible({ - timeout: 5_000, - }); }); test("switching tasks via sidebar does not show messages from previous task", async ({ diff --git a/apps/web/e2e/tests/terminal/terminal-dockview-ui.spec.ts b/apps/web/e2e/tests/terminal/terminal-dockview-ui.spec.ts new file mode 100644 index 0000000000..f738aaa2ce --- /dev/null +++ b/apps/web/e2e/tests/terminal/terminal-dockview-ui.spec.ts @@ -0,0 +1,426 @@ +import { test, expect } from "../../fixtures/test-base"; +import type { ApiClient } from "../../helpers/api-client"; +import type { SeedData } from "../../fixtures/test-base"; +import { KanbanPage } from "../../pages/kanban-page"; +import { SessionPage } from "../../pages/session-page"; +import type { Page } from "@playwright/test"; + +const DONE_STATES = ["COMPLETED", "WAITING_FOR_INPUT"]; + +/** + * UI-level E2E coverage for the dockview terminal experience. The + * existing terminal-first-class.spec asserts WS RPC behaviour; this + * spec asserts what the user actually SEES on the page. + * + * Each test runs against the real dockview layout (desktop project). + */ + +async function createTaskAndWait(apiClient: ApiClient, seedData: SeedData, title: string) { + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + title, + seedData.agentProfileId, + { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + await expect + .poll( + async () => { + const { sessions } = await apiClient.listTaskSessions(task.id); + return DONE_STATES.includes(sessions[0]?.state ?? ""); + }, + { timeout: 30_000, message: `Waiting for ${title} session to settle` }, + ) + .toBe(true); + return task; +} + +async function openTask(page: Page, title: string): Promise { + const kanban = new KanbanPage(page); + await kanban.goto(); + const card = kanban.taskCardByTitle(title); + await expect(card).toBeVisible({ timeout: 15_000 }); + await card.click(); + await expect(page).toHaveURL(/\/t\//, { timeout: 15_000 }); + const session = new SessionPage(page); + await session.waitForLoad(); + return session; +} + +async function clickNewTerminalInPlusMenu(page: Page, session: SessionPage) { + await session.addPanelButton().click(); + await page.getByTestId("new-terminal-button").click(); +} + +test.describe("Terminals — dockview UI", () => { + /** + * Regression: the tab title for ordinary terminals should be the + * literal "Terminal" (no " N" suffix) with the sequence number in a + * sibling badge — matching the session-tab pattern where the agent + * name is the title and the seq is a pill before it. + * + * Before the fix this test fails because `DockviewDefaultTab` reads + * from `api.title` directly and ignores any prop overrides; the + * panel was created with `title="Terminal 2"` so the tab text reads + * "Terminal 2" with the badge also visible → "2 Terminal 2". + */ + test("multi-terminal tabs show seq badge + plain 'Terminal' title", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(90_000); + await createTaskAndWait(apiClient, seedData, "Tab Badge UI"); + const session = await openTask(testPage, "Tab Badge UI"); + await session.clickTab("Terminal"); + await session.expectTerminalConnected(); + + // Open the dockview "+" menu, then click the new "New Terminal" + // row that lives under the Terminals section label. + await clickNewTerminalInPlusMenu(testPage, session); + + // The strip now has two terminal panels. Each tab's visible text + // should be exactly "Terminal" — the seq lives in the adjacent + // badge, not in the title itself. + const terminalTabs = testPage + .locator(".dv-default-tab-content") + .filter({ hasText: /^Terminal/ }); + await expect + .poll(() => terminalTabs.count(), { timeout: 10_000, message: "two terminal tabs visible" }) + .toBeGreaterThanOrEqual(2); + + // None of the tab content nodes should contain "Terminal 1" or + // "Terminal 2" — the seq must be in the badge sibling, not the + // title. + const numberedTitles = testPage.locator(".dv-default-tab-content").filter({ + hasText: /^Terminal\s+\d+$/, + }); + expect( + await numberedTitles.count(), + 'tab title should be plain "Terminal" (seq belongs in the badge)', + ).toBe(0); + + // Both seq badges should be present and adjacent to a "Terminal" + // title. The badges are rendered with `data-testid="terminal-tab-seq-N"`. + await expect(testPage.getByTestId("terminal-tab-seq-1")).toBeVisible({ timeout: 5_000 }); + await expect(testPage.getByTestId("terminal-tab-seq-2")).toBeVisible({ timeout: 5_000 }); + }); + + /** + * Regression: closing a terminal tab via dockview's X button must + * PARK the terminal (preserving the PTY + DB row), not destroy it. + * After reload, the closed terminal must surface in the "+" → Terminals + * menu with a `parked` pill AND its original seq, so the user can + * resume it. + * + * Before the fix this fails because dockview-layout-setup.onDidRemovePanel + * calls stopUserShell (alias for destroyUserShell) — the DB row is + * gone, no "parked" row exists to find. + */ + test("closing a terminal parks it and the row shows up after reload with parked pill", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + await createTaskAndWait(apiClient, seedData, "Close + Reload UI"); + const session = await openTask(testPage, "Close + Reload UI"); + await session.clickTab("Terminal"); + await session.expectTerminalConnected(); + + // Add a second terminal so we have a non-default one to close. + await clickNewTerminalInPlusMenu(testPage, session); + await expect(testPage.getByTestId("terminal-tab-seq-1")).toBeVisible({ timeout: 10_000 }); + await expect(testPage.getByTestId("terminal-tab-seq-2")).toBeVisible({ timeout: 5_000 }); + + // Close the seq=2 tab specifically. `.last()` is unreliable when + // terminals span multiple groups (default lives in the right-bottom, + // user-created lands in the center). Walk from the seq=2 badge up + // to its wrapping div and grab that subtree's close button. + const seq2Close = testPage + .getByTestId("terminal-tab-seq-2") + .locator("..") + .locator(".dv-default-tab-action"); + await seq2Close.click(); + await expect(testPage.getByTestId("terminal-tab-seq-2")).toHaveCount(0, { timeout: 5_000 }); + + // Layout-save is 300ms-debounced. Wait past the debounce so the + // saved JSON reflects the closed/parked terminal — otherwise the + // saved layout still has it and the reload re-opens it. + await testPage.waitForTimeout(800); + + // Reload — only the open default terminal should restore as a + // panel; the parked one lives in the reopen menu. + await testPage.reload(); + await session.waitForLoad(); + await session.clickTab("Terminal"); + + // The open default terminal should still render as a visible tab. + const terminalContent = testPage.locator(".dv-default-tab-content").filter({ + hasText: /^Terminal$/, + }); + await expect + .poll(() => terminalContent.count(), { + timeout: 10_000, + message: "default terminal tab should still be visible after reload", + }) + .toBeGreaterThanOrEqual(1); + + // Open "+" menu — the parked second terminal must appear under + // Terminals with its seq=2 marker AND the `parked` pill. + await session.addPanelButton().click(); + const terminalSection = testPage + .locator('[role="menu"]') + .getByText("Terminals", { exact: true }); + await expect(terminalSection).toBeVisible({ timeout: 10_000 }); + + // The seq=2 row must be present — proves the close was a park, not + // a destroy. The row no longer carries a distinct "parked" pill; + // its mere presence after a destroy-on-close handler would have + // wiped the DB row is the regression guard. + const reopenRowsWithSeq2 = testPage + .locator('[data-testid^="reopen-terminal-"]') + .filter({ has: testPage.getByTestId("reopen-terminal-seq-2") }); + await expect + .poll(() => reopenRowsWithSeq2.count(), { + timeout: 10_000, + message: + "parked terminal #2 should appear in reopen menu — if missing, close-handler destroyed instead of parked", + }) + .toBe(1); + }); + + /** + * Regression: with two open terminals AND a page reload (no close), + * both terminals must re-render with their badges. Before the fix the + * serializer rewrites the user-created terminal's panel id to + * `terminal-saved-N` and drops the environmentId/taskID params, so + * after reload that panel has no shell entry in the store → no badge, + * fallback title text. + */ + test("two open terminals survive a reload with their seq badges intact", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + await createTaskAndWait(apiClient, seedData, "Reload Badges UI"); + const session = await openTask(testPage, "Reload Badges UI"); + await session.clickTab("Terminal"); + await session.expectTerminalConnected(); + + await clickNewTerminalInPlusMenu(testPage, session); + await expect(testPage.getByTestId("terminal-tab-seq-1")).toBeVisible({ timeout: 10_000 }); + await expect(testPage.getByTestId("terminal-tab-seq-2")).toBeVisible({ timeout: 5_000 }); + + // Layout-save is 300ms-debounced. Wait past the debounce so the + // saved JSON includes the user-created terminal panel before we + // reload — otherwise a fast test races the save. + await testPage.waitForTimeout(800); + + await testPage.reload(); + await session.waitForLoad(); + + // After reload, both badges must reappear — proves both panels' + // store entries (kind=ordinary, seq) were preserved across restore. + await expect(testPage.getByTestId("terminal-tab-seq-1")).toBeVisible({ timeout: 15_000 }); + await expect(testPage.getByTestId("terminal-tab-seq-2")).toBeVisible({ timeout: 5_000 }); + + // No tab content should contain "Terminal N" text — seq belongs in + // the badge, not the title. + const numberedTitles = testPage.locator(".dv-default-tab-content").filter({ + hasText: /^Terminal\s+\d+$/, + }); + expect( + await numberedTitles.count(), + 'tab title should be plain "Terminal" after reload (seq belongs in the badge)', + ).toBe(0); + }); + + /** + * Regression: the row × destroy affordance in the "+" → Terminals + * menu permanently deletes a terminal (PTY stopped, DB row removed, + * no return after reload). Solves the discoverability gap once the + * tab is closed and the right-click Destroy is no longer reachable. + */ + test("destroy button on a reopen-menu row permanently removes the terminal", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + await createTaskAndWait(apiClient, seedData, "Row Destroy UI"); + const session = await openTask(testPage, "Row Destroy UI"); + await session.clickTab("Terminal"); + await session.expectTerminalConnected(); + + await clickNewTerminalInPlusMenu(testPage, session); + await expect(testPage.getByTestId("terminal-tab-seq-2")).toBeVisible({ timeout: 10_000 }); + + // Park the seq=2 tab by closing it (last in tab order). + const terminalCloseButtons = testPage + .locator(".dv-default-tab") + .filter({ hasText: /^Terminal/ }) + .locator(".dv-default-tab-action"); + await terminalCloseButtons.last().click(); + + // Open the "+" menu and confirm-destroy the seq=2 row. + await session.addPanelButton().click(); + const row = testPage + .locator('[data-testid^="reopen-terminal-"]') + .filter({ has: testPage.getByTestId("reopen-terminal-seq-2") }); + await expect(row).toHaveCount(1, { timeout: 10_000 }); + + testPage.once("dialog", (d) => d.accept()); + await row.getByTestId("destroy-terminal-row").click(); + + // Row vanishes from the live menu. + await expect(row).toHaveCount(0, { timeout: 5_000 }); + + // Reload and confirm it does not come back. + // Close the open menu first by pressing Escape. + await testPage.keyboard.press("Escape"); + await testPage.reload(); + await session.waitForLoad(); + await session.addPanelButton().click(); + const rowAfter = testPage + .locator('[data-testid^="reopen-terminal-"]') + .filter({ has: testPage.getByTestId("reopen-terminal-seq-2") }); + await expect(rowAfter).toHaveCount(0, { timeout: 5_000 }); + }); + + /** + * Regression: clicking an existing-open terminal row in the "+" menu + * must focus the existing tab — NOT add a second panel for the same + * PTY. Before the fix, the default-migrated panel kept its id as + * `terminal-default` while the reopen row carried the shell-, + * so api.getPanel(uuid) missed and a duplicate tab was created. + */ + test("reopen-menu row for an already-open terminal focuses, does not duplicate", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + await createTaskAndWait(apiClient, seedData, "Focus Existing UI"); + const session = await openTask(testPage, "Focus Existing UI"); + await session.clickTab("Terminal"); + await session.expectTerminalConnected(); + + // Create a second terminal so we have a non-default row to click. + await clickNewTerminalInPlusMenu(testPage, session); + await expect(testPage.getByTestId("terminal-tab-seq-2")).toBeVisible({ timeout: 10_000 }); + await testPage.waitForTimeout(800); + + // Count terminal tab content elements before the focus click. + const terminalContent = testPage.locator(".dv-default-tab-content").filter({ + hasText: /^Terminal$/, + }); + const before = await terminalContent.count(); + expect(before, "two terminal tabs before clicking reopen").toBeGreaterThanOrEqual(2); + + // Open the menu and click the seq=2 row — that terminal is already + // open as a tab, so the click should focus rather than mint a new + // panel. + await session.addPanelButton().click(); + const seq2Row = testPage + .locator('[data-testid^="reopen-terminal-"]') + .filter({ has: testPage.getByTestId("reopen-terminal-seq-2") }); + await expect(seq2Row).toHaveCount(1, { timeout: 10_000 }); + await seq2Row.click(); + + // Tab count must NOT grow. The menu closes on its own; wait briefly + // for any spurious panel to land. + await testPage.waitForTimeout(500); + const after = await terminalContent.count(); + expect( + after, + `clicking an open terminal in the reopen menu must focus, not duplicate (before=${before}, after=${after})`, + ).toBe(before); + }); + + /** + * Right-click → Terminate must hard-destroy the terminal AND remove + * its dockview tab in the same gesture. Before the fix the WS RPC + * fired but the panel stayed open with a dead PTY and the row hung + * around in the reopen menu until the next refresh. + */ + test("right-click → Terminate closes the tab and removes the row", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(120_000); + await createTaskAndWait(apiClient, seedData, "Tab Terminate UI"); + const session = await openTask(testPage, "Tab Terminate UI"); + await session.clickTab("Terminal"); + await session.expectTerminalConnected(); + + // Add a second terminal so we have a non-default one to terminate + // (the default's id is `terminal-default` and its right-click menu + // routes through the same handler, but the seq=2 case is what + // surfaced the bug). + await clickNewTerminalInPlusMenu(testPage, session); + await expect(testPage.getByTestId("terminal-tab-seq-2")).toBeVisible({ timeout: 10_000 }); + + // Right-click the seq=2 tab and pick Terminate. + const seq2TabTrigger = testPage.getByTestId("terminal-tab-seq-2").locator("..").locator(".."); + await seq2TabTrigger.click({ button: "right" }); + await testPage.getByRole("menuitem", { name: /^Terminate/ }).click(); + + // The seq=2 badge must disappear — proves the panel was closed. + await expect(testPage.getByTestId("terminal-tab-seq-2")).toHaveCount(0, { timeout: 5_000 }); + + // The seq=2 row must NOT appear in the reopen menu — proves the + // row was hard-destroyed, not parked. + await session.addPanelButton().click(); + const seq2Row = testPage + .locator('[data-testid^="reopen-terminal-"]') + .filter({ has: testPage.getByTestId("reopen-terminal-seq-2") }); + await expect(seq2Row).toHaveCount(0, { timeout: 5_000 }); + }); + + /** + * Inline rename: right-clicking a terminal tab and picking Rename + * swaps the title in place for an editable input. Typing + Enter + * commits, updating the tab title (custom names override the + * default "Terminal"). + */ + test("right-click → Rename swaps the tab title for an inline input", async ({ + testPage, + apiClient, + seedData, + }) => { + test.setTimeout(90_000); + await createTaskAndWait(apiClient, seedData, "Inline Rename UI"); + const session = await openTask(testPage, "Inline Rename UI"); + await session.clickTab("Terminal"); + await session.expectTerminalConnected(); + + // The default panel's ContextMenuTrigger has a testid prefixed + // with `terminal-tab-shell-` once the migration to a DB-backed + // shell- id completes. + const tabTrigger = testPage.locator('[data-testid^="terminal-tab-shell-"]').first(); + await expect(tabTrigger).toBeVisible({ timeout: 15_000 }); + await tabTrigger.click({ button: "right" }); + + // Pick the Rename menu item. + await testPage.getByRole("menuitem", { name: /^Rename/ }).click(); + + // Input replaces the title text. Type a custom name and Enter. + const input = testPage.getByTestId("terminal-tab-rename-input"); + await expect(input).toBeVisible({ timeout: 5_000 }); + await input.fill("build watcher"); + await input.press("Enter"); + + // Tab title now reads "build watcher" — proves the rename committed + // and the displayName lookup found the customName. + await expect( + testPage.locator(".dv-default-tab-content").filter({ hasText: "build watcher" }).first(), + ).toBeVisible({ timeout: 5_000 }); + }); +}); diff --git a/apps/web/e2e/tests/terminal/terminal-first-class.spec.ts b/apps/web/e2e/tests/terminal/terminal-first-class.spec.ts new file mode 100644 index 0000000000..a2b1822f1a --- /dev/null +++ b/apps/web/e2e/tests/terminal/terminal-first-class.spec.ts @@ -0,0 +1,279 @@ +import { test, expect } from "../../fixtures/test-base"; +import type { ApiClient } from "../../helpers/api-client"; +import type { SeedData } from "../../fixtures/test-base"; +import { KanbanPage } from "../../pages/kanban-page"; +import { SessionPage } from "../../pages/session-page"; +import type { Page } from "@playwright/test"; + +const DONE_STATES = ["COMPLETED", "WAITING_FOR_INPUT"]; + +async function createTaskAndWaitForDone(apiClient: ApiClient, seedData: SeedData, title: string) { + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + title, + seedData.agentProfileId, + { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + await expect + .poll( + async () => { + const { sessions } = await apiClient.listTaskSessions(task.id); + return DONE_STATES.includes(sessions[0]?.state ?? ""); + }, + { timeout: 30_000, message: `Waiting for ${title} session to settle` }, + ) + .toBe(true); + return task; +} + +async function navigateToTaskViaKanban(page: Page, title: string): Promise { + const kanban = new KanbanPage(page); + await kanban.goto(); + const card = kanban.taskCardByTitle(title); + await expect(card).toBeVisible({ timeout: 15_000 }); + await card.click(); + await expect(page).toHaveURL(/\/t\//, { timeout: 15_000 }); + const session = new SessionPage(page); + await session.waitForLoad(); + return session; +} + +type ListItem = { + id?: string; + kind?: string; + seq?: number; + display_name?: string; + custom_name?: string | null; + state?: string; + pty_status?: string; +}; + +async function listTerminals( + apiClient: ApiClient, + taskID: string, + envID: string, + includeParked: boolean, +): Promise { + const resp = await apiClient.wsRequest<{ shells: ListItem[] }>("user_shell.list", { + task_id: taskID, + task_environment_id: envID, + include_parked: includeParked, + }); + return resp?.shells ?? []; +} + +async function createOrdinaryTerminal( + apiClient: ApiClient, + taskID: string, + envID: string, +): Promise<{ terminal_id: string; seq?: number; display_name?: string; kind?: string }> { + return apiClient.wsRequest("user_shell.create", { + task_id: taskID, + task_environment_id: envID, + }); +} + +async function renameTerminal( + apiClient: ApiClient, + taskID: string, + terminalID: string, + name: string | null, +): Promise { + await apiClient.wsRequest("user_shell.rename", { + task_id: taskID, + terminal_id: terminalID, + custom_name: name, + }); +} + +async function parkTerminal( + apiClient: ApiClient, + taskID: string, + terminalID: string, +): Promise { + await apiClient.wsRequest("user_shell.park", { + task_id: taskID, + terminal_id: terminalID, + }); +} + +async function resumeTerminal( + apiClient: ApiClient, + taskID: string, + terminalID: string, +): Promise { + await apiClient.wsRequest("user_shell.resume", { + task_id: taskID, + terminal_id: terminalID, + }); +} + +async function destroyTerminal( + apiClient: ApiClient, + taskID: string, + terminalID: string, + envID: string, +): Promise { + await apiClient.wsRequest("user_shell.destroy", { + task_id: taskID, + task_environment_id: envID, + terminal_id: terminalID, + }); +} + +async function getEnvID(apiClient: ApiClient, taskID: string): Promise { + const { sessions } = await apiClient.listTaskSessions(taskID); + const envID = sessions[0]?.task_environment_id; + expect(envID, "task should have an environment id").toBeTruthy(); + return envID as string; +} + +test.describe("Terminals — first-class persistent entities", () => { + test("ordinary terminals get monotonic seqs", async ({ apiClient, seedData }) => { + test.setTimeout(60_000); + const task = await createTaskAndWaitForDone(apiClient, seedData, "Monotonic Seq"); + const envID = await getEnvID(apiClient, task.id); + + const t1 = await createOrdinaryTerminal(apiClient, task.id, envID); + const t2 = await createOrdinaryTerminal(apiClient, task.id, envID); + const t3 = await createOrdinaryTerminal(apiClient, task.id, envID); + + expect(t1.kind).toBe("ordinary"); + expect(t1.seq).toBe(1); + expect(t2.seq).toBe(2); + expect(t3.seq).toBe(3); + }); + + test("rename + park survive a page refresh", async ({ testPage, apiClient, seedData }) => { + test.setTimeout(90_000); + const task = await createTaskAndWaitForDone(apiClient, seedData, "Rename Park Persist"); + const envID = await getEnvID(apiClient, task.id); + const session = await navigateToTaskViaKanban(testPage, "Rename Park Persist"); + await session.clickTab("Terminal"); + await session.expectTerminalConnected(); + + // Visiting the session page migrates the legacy `shell-default` panel + // into a DB-backed ordinary terminal (seq=1) — every subsequent + // create starts from seq=2. Look up the migrated row instead of + // assuming seq numbering. + const a = await createOrdinaryTerminal(apiClient, task.id, envID); + const b = await createOrdinaryTerminal(apiClient, task.id, envID); + const c = await createOrdinaryTerminal(apiClient, task.id, envID); + + await renameTerminal(apiClient, task.id, b.terminal_id, "build watcher"); + await parkTerminal(apiClient, task.id, c.terminal_id); + + // Reload — DB-backed metadata must survive. + await testPage.reload(); + await session.waitForLoad(); + + const all = await listTerminals(apiClient, task.id, envID, true); + const ordinary = all.filter((it) => it.kind === "ordinary"); + // 3 API-created + 1 migrated-default + expect(ordinary).toHaveLength(4); + const byID = Object.fromEntries(ordinary.map((it) => [it.id, it])); + expect(byID[a.terminal_id]?.state).toBe("open"); + expect(byID[b.terminal_id]?.custom_name).toBe("build watcher"); + expect(byID[b.terminal_id]?.display_name).toBe("build watcher"); + expect(byID[c.terminal_id]?.state).toBe("parked"); + + // The open list should hide the parked terminal. + const openList = await listTerminals(apiClient, task.id, envID, false); + const openOrdinary = openList.filter((it) => it.kind === "ordinary"); + expect(openOrdinary.find((it) => it.id === c.terminal_id)).toBeUndefined(); + }); + + test("destroy + recreate leaves a stable gap in the seq sequence", async ({ + apiClient, + seedData, + }) => { + test.setTimeout(60_000); + const task = await createTaskAndWaitForDone(apiClient, seedData, "Seq Gap Preservation"); + const envID = await getEnvID(apiClient, task.id); + + const a = await createOrdinaryTerminal(apiClient, task.id, envID); + const b = await createOrdinaryTerminal(apiClient, task.id, envID); + const c = await createOrdinaryTerminal(apiClient, task.id, envID); + expect([a.seq, b.seq, c.seq]).toEqual([1, 2, 3]); + + await destroyTerminal(apiClient, task.id, b.terminal_id, envID); + + const d = await createOrdinaryTerminal(apiClient, task.id, envID); + expect(d.seq, "deleted #2 should leave a gap — next create should be #4").toBe(4); + + const all = await listTerminals(apiClient, task.id, envID, true); + const seqs = all + .filter((it) => it.kind === "ordinary") + .map((it) => it.seq ?? 0) + .sort((x, y) => x - y); + expect(seqs).toEqual([1, 3, 4]); + }); + + test("park + resume keeps the row but toggles state", async ({ apiClient, seedData }) => { + test.setTimeout(60_000); + const task = await createTaskAndWaitForDone(apiClient, seedData, "Park Resume Cycle"); + const envID = await getEnvID(apiClient, task.id); + + const t = await createOrdinaryTerminal(apiClient, task.id, envID); + + await parkTerminal(apiClient, task.id, t.terminal_id); + const openOnly = await listTerminals(apiClient, task.id, envID, false); + expect(openOnly.find((it) => it.id === t.terminal_id)).toBeUndefined(); + + const all = await listTerminals(apiClient, task.id, envID, true); + expect(all.find((it) => it.id === t.terminal_id)?.state).toBe("parked"); + + await resumeTerminal(apiClient, task.id, t.terminal_id); + const afterResume = await listTerminals(apiClient, task.id, envID, false); + expect(afterResume.find((it) => it.id === t.terminal_id)?.state).toBe("open"); + }); + + test("rename/park guards reject the bottom-panel terminal", async ({ apiClient, seedData }) => { + test.setTimeout(60_000); + const task = await createTaskAndWaitForDone(apiClient, seedData, "Bottom Panel Guard"); + await getEnvID(apiClient, task.id); + + await expect( + renameTerminal(apiClient, task.id, "bottom-panel", "should fail"), + "rename guard", + ).rejects.toThrow(); + await expect(parkTerminal(apiClient, task.id, "bottom-panel"), "park guard").rejects.toThrow(); + await expect(parkTerminal(apiClient, task.id, "script-fake"), "script guard").rejects.toThrow(); + }); + + test("cross-task mutating ops are rejected", async ({ apiClient, seedData }) => { + test.setTimeout(90_000); + const taskA = await createTaskAndWaitForDone(apiClient, seedData, "Owner Task A"); + const taskB = await createTaskAndWaitForDone(apiClient, seedData, "Attacker Task B"); + const envA = await getEnvID(apiClient, taskA.id); + + // Create a terminal owned by task A. + const owned = await createOrdinaryTerminal(apiClient, taskA.id, envA); + + // Task B tries to rename/park/destroy task A's terminal by raw id. + await expect( + renameTerminal(apiClient, taskB.id, owned.terminal_id, "stolen"), + "cross-task rename should reject", + ).rejects.toThrow(); + await expect( + parkTerminal(apiClient, taskB.id, owned.terminal_id), + "cross-task park should reject", + ).rejects.toThrow(); + await expect( + destroyTerminal(apiClient, taskB.id, owned.terminal_id, envA), + "cross-task destroy should reject", + ).rejects.toThrow(); + + // Terminal is still owned by task A, unchanged. + const list = await listTerminals(apiClient, taskA.id, envA, true); + const found = list.find((it) => it.id === owned.terminal_id); + expect(found, "terminal should still exist under task A").toBeDefined(); + expect(found?.custom_name ?? null).toBeNull(); + expect(found?.state).toBe("open"); + }); +}); diff --git a/apps/web/e2e/tests/terminal/terminal-new-tab.spec.ts b/apps/web/e2e/tests/terminal/terminal-new-tab.spec.ts index f0eb960050..d6fc323329 100644 --- a/apps/web/e2e/tests/terminal/terminal-new-tab.spec.ts +++ b/apps/web/e2e/tests/terminal/terminal-new-tab.spec.ts @@ -107,7 +107,9 @@ test.describe("Terminal new tab — env-keyed user shell RPCs", () => { // Open the dockview "+" menu (every group has one — the chat/center // group's is reliably present and not behind any conditional). await session.addPanelButton().click(); - const terminalItem = testPage.getByRole("menuitem", { name: "Terminal" }); + // Use exact match to disambiguate from the new "Terminals" submenu + // label and any "Resume Terminal …" reopen items. + const terminalItem = testPage.getByRole("menuitem", { name: "New Terminal", exact: true }); await expect(terminalItem).toBeVisible({ timeout: 5_000 }); await terminalItem.click(); diff --git a/apps/web/hooks/domains/session/use-ensure-default-terminal-ordinary.ts b/apps/web/hooks/domains/session/use-ensure-default-terminal-ordinary.ts new file mode 100644 index 0000000000..e4c5f3f232 --- /dev/null +++ b/apps/web/hooks/domains/session/use-ensure-default-terminal-ordinary.ts @@ -0,0 +1,126 @@ +"use client"; + +import { useEffect, useRef } from "react"; +import { useAppStore } from "@/components/state-provider"; +import { useDockviewStore } from "@/lib/state/dockview-store"; +import { TERMINAL_DEFAULT_ID } from "@/lib/state/layout-manager"; +import { createUserShell } from "@/lib/api/domains/user-shell-api"; + +/** + * The legacy `terminal-default` dockview panel ships with a hardcoded + * `terminalId: "shell-default"` that bypasses the DB-backed ordinary + * terminal flow — no seq, no rename, no parking. That breaks the + * promised "Terminal N" badge UX: with only the default present, the + * user sees a plain "Terminal" tab; once they add a second one, the + * badge logic that gates on "more than one ordinary terminal" still + * counts only the user-created shell. + * + * This hook migrates the default panel into an ordinary terminal on + * session-page mount: if `terminal-default` exists and still points at + * `shell-default`, it calls `user_shell.create` to mint a real + * task-scoped row and rewrites the panel's params to use the new id. + * Subsequent loads see an ordinary terminal already in the store and + * short-circuit. + * + * The migration is keyed on `env:task` so a task switch repeats it for + * the new task. Concurrent mounts (mobile pane + dockview header) are + * serialised by a module-level Set guard. + */ +const migratingScopes = new Set(); + +export function useEnsureDefaultTerminalOrdinary(): void { + const environmentId = useAppStore((s) => { + const sid = s.tasks?.activeSessionId; + return sid ? (s.environmentIdBySessionId[sid] ?? null) : null; + }); + const taskID = useAppStore((s) => s.tasks?.activeTaskId ?? null); + const userShellsLoaded = useAppStore((s) => { + if (!environmentId) return false; + return s.userShells.loaded[environmentId] ?? false; + }); + const shells = useAppStore((s) => { + if (!environmentId) return null; + return s.userShells.byEnvironmentId[environmentId] ?? null; + }); + const addUserShell = useAppStore((s) => s.addUserShell); + const dockviewApi = useDockviewStore((s) => s.api); + const lastMigratedScopeRef = useRef(null); + + useEffect(() => { + if (!environmentId || !taskID || !userShellsLoaded || !dockviewApi) return; + const scope = `${environmentId}:${taskID}`; + if (lastMigratedScopeRef.current === scope) return; + + const panel = dockviewApi.getPanel(TERMINAL_DEFAULT_ID); + if (!panel) { + lastMigratedScopeRef.current = scope; + return; + } + + // Look at panel.params via the registered params recorded at create + // time. Dockview's IDockviewPanel exposes `.params` (Record) directly. The default registration stamps + // `terminalId: "shell-default"`; once migrated we replace it. + const panelParams = (panel as unknown as { params?: Record }).params ?? {}; + const currentTerminalId = panelParams.terminalId as string | undefined; + if (currentTerminalId !== "shell-default") { + lastMigratedScopeRef.current = scope; + return; + } + + // If the task already owns ordinary terminals (from a previous + // session-page visit), reuse the lowest-seq one for the default + // panel instead of minting a new row. + const existingOrdinary = (shells ?? []) + .filter((it) => it.kind === "ordinary") + .sort((a, b) => (a.seq ?? 0) - (b.seq ?? 0))[0]; + + const finishMigration = (terminalId: string) => { + panel.api.updateParameters({ + terminalId, + environmentId, + taskID, + }); + // The panel title is intentionally left as the registry default + // ("Terminal"). TerminalTab's own useEffect drives `api.setTitle` + // to the live display name (custom rename → customName; otherwise + // plain "Terminal" with the seq rendered separately in the + // badge). Setting a "Terminal {seq}" title here would race the + // tab component and stick. + lastMigratedScopeRef.current = scope; + }; + + if (existingOrdinary) { + finishMigration(existingOrdinary.terminalId); + return; + } + + if (migratingScopes.has(scope)) return; + migratingScopes.add(scope); + + (async () => { + try { + const result = await createUserShell(environmentId, { taskId: taskID }); + addUserShell(environmentId, { + terminalId: result.terminalId, + kind: result.kind, + seq: result.seq, + displayName: result.displayName, + customName: null, + state: result.state ?? "open", + ptyStatus: result.ptyStatus ?? "stopped", + running: result.ptyStatus === "running", + label: result.label, + closable: result.closable ?? true, + initialCommand: result.initialCommand, + }); + finishMigration(result.terminalId); + } catch (error) { + console.error("default-terminal migration:", error); + // Don't lock the scope so a retry can run on the next tick. + } finally { + migratingScopes.delete(scope); + } + })(); + }, [environmentId, taskID, userShellsLoaded, shells, dockviewApi, addUserShell]); +} diff --git a/apps/web/hooks/domains/session/use-mobile-terminals.test.ts b/apps/web/hooks/domains/session/use-mobile-terminals.test.ts index 0ea08214e0..a8817aa52f 100644 --- a/apps/web/hooks/domains/session/use-mobile-terminals.test.ts +++ b/apps/web/hooks/domains/session/use-mobile-terminals.test.ts @@ -9,10 +9,13 @@ let mockEnvironmentId: string | null = null; let mockShells: Array<{ terminalId: string; label: string; closable: boolean }> = []; let mockShellsLoaded = false; +let mockTaskID: string | null = "task-1"; + vi.mock("@/components/state-provider", () => ({ useAppStore: (selector: (state: Record) => unknown) => selector({ environmentIdBySessionId: mockEnvironmentId ? { [SESSION_ID]: mockEnvironmentId } : {}, + tasks: { activeTaskId: mockTaskID }, }), })); @@ -48,6 +51,7 @@ beforeEach(() => { mockEnvironmentId = "env-A"; mockShells = []; mockShellsLoaded = true; + mockTaskID = "task-1"; }); describe("useMobileTerminals auto-create", () => { @@ -80,6 +84,18 @@ describe("useMobileTerminals auto-create", () => { await waitFor(() => expect(addTerminalMock).toHaveBeenCalledTimes(1)); }); + it("waits for taskID before auto-creating — null taskID is a no-op", async () => { + addTerminalMock.mockResolvedValue(undefined); + mockTaskID = null; + const { rerender } = renderHook(() => useMobileTerminals(SESSION_ID)); + await new Promise((r) => setTimeout(r, 0)); + expect(addTerminalMock).not.toHaveBeenCalled(); + // Once taskID hydrates the next render fires the auto-create. + mockTaskID = "task-late"; + rerender(); + await waitFor(() => expect(addTerminalMock).toHaveBeenCalledTimes(1)); + }); + it("clears the guard when addTerminal rejects so a fresh hook retries", async () => { addTerminalMock.mockRejectedValueOnce(new Error("network down")); const first = renderHook(() => useMobileTerminals(SESSION_ID)); diff --git a/apps/web/hooks/domains/session/use-mobile-terminals.ts b/apps/web/hooks/domains/session/use-mobile-terminals.ts index 2995214ad1..068805b704 100644 --- a/apps/web/hooks/domains/session/use-mobile-terminals.ts +++ b/apps/web/hooks/domains/session/use-mobile-terminals.ts @@ -7,28 +7,44 @@ import { useAppStore } from "@/components/state-provider"; /** * Module-level guard for the auto-create-first-shell effect, keyed by - * environmentId. `useMobileTerminals` is called from several places that all - * end up rendering the same terminal pane (the pane itself, the picker pill, - * the picker sheet content) — a per-instance ref guard would let each one - * fire `createUserShell` on first mount, racing into multiple shells. The - * Set is shared across instances so only the first effect to run for a given - * environment triggers creation. + * `environmentId:taskID`. `useMobileTerminals` is called from several places + * that all end up rendering the same terminal pane (the pane itself, the + * picker pill, the picker sheet content) — a per-instance ref guard would + * let each one fire `createUserShell` on first mount, racing into multiple + * shells. The Set is shared across instances so only the first effect to + * run for a given (env, task) pair triggers creation. + * + * Why env+task: the auto-create must wait for the real taskID — otherwise + * we'd auto-create a non-persistent legacy shell against an env, mark the + * env as "done" in the guard, and the real task's first-class terminal + * would never get created. */ -const autoCreatedEnvironments = new Set(); +const autoCreatedScopes = new Set(); + +function autoCreateScope(environmentId: string, taskID: string | null): string | null { + if (!taskID) return null; + return `${environmentId}:${taskID}`; +} /** - * Release the auto-create guard for a specific environment so the next mount - * (or shell-list change) re-runs the auto-create effect. Call this when the - * user explicitly closes the last terminal in an env — otherwise the pane - * shows "Starting terminal…" forever because the guard prevents recreation. + * Release the auto-create guard so the next mount (or shell-list change) + * re-runs the auto-create effect. Call this when the user explicitly closes + * the last terminal in an env — otherwise the pane shows "Starting + * terminal…" forever because the guard prevents recreation. We clear every + * scope keyed on this env id regardless of task, so any subsequent re-mount + * finds a clean slate. */ export function releaseAutoCreatedEnvironment(environmentId: string): void { - autoCreatedEnvironments.delete(environmentId); + for (const scope of Array.from(autoCreatedScopes)) { + if (scope.startsWith(`${environmentId}:`)) { + autoCreatedScopes.delete(scope); + } + } } /** Test-only: reset the module-level guard so each test starts from scratch. */ export function __resetAutoCreatedEnvironmentsForTest(): void { - autoCreatedEnvironments.clear(); + autoCreatedScopes.clear(); } /** @@ -40,11 +56,15 @@ export function useMobileTerminals(sessionId: string | null) { const environmentId = useAppStore((s) => sessionId ? (s.environmentIdBySessionId[sessionId] ?? null) : null, ); + const taskID = useAppStore((s) => s.tasks?.activeTaskId ?? null); const result = useTerminals({ sessionId, environmentId }); // Read user-shell loaded flag directly so the auto-create trigger has // primitive dependencies — depending on `result` would re-run on every // render and would also race against React's effect ordering. - const { isLoaded: shellsLoaded, shells } = useUserShells(environmentId); + // taskID flows in so the backend's DB-backed ordinary-shell path + // returns parked + open ordinary rows (the orphan filter drops them + // otherwise). + const { isLoaded: shellsLoaded, shells } = useUserShells(environmentId, taskID); const addTerminalRef = useRef(result.addTerminal); useEffect(() => { addTerminalRef.current = result.addTerminal; @@ -52,22 +72,28 @@ export function useMobileTerminals(sessionId: string | null) { useEffect(() => { if (!environmentId || !shellsLoaded) return; - if (autoCreatedEnvironments.has(environmentId)) return; + const scope = autoCreateScope(environmentId, taskID); + // Wait for a real taskID before auto-creating; otherwise the first + // mount on a session without a hydrated active task would bootstrap a + // legacy non-persistent shell and mark the env as "done" — pinning + // future task-scoped mounts on a stale legacy entry. + if (!scope) return; + if (autoCreatedScopes.has(scope)) return; if (shells.length > 0) { - autoCreatedEnvironments.add(environmentId); + autoCreatedScopes.add(scope); return; } - autoCreatedEnvironments.add(environmentId); + autoCreatedScopes.add(scope); // Reset the guard if creation fails so the user gets a retry on the next // render cycle (e.g. after the WS reconnects). `addTerminal` returns void // but its inner promise can still reject; guard defensively. const promise = addTerminalRef.current() as unknown; if (promise && typeof (promise as Promise).catch === "function") { (promise as Promise).catch(() => { - autoCreatedEnvironments.delete(environmentId); + autoCreatedScopes.delete(scope); }); } - }, [environmentId, shellsLoaded, shells.length]); + }, [environmentId, taskID, shellsLoaded, shells.length]); return { ...result, environmentId }; } diff --git a/apps/web/hooks/domains/session/use-terminals-build.ts b/apps/web/hooks/domains/session/use-terminals-build.ts new file mode 100644 index 0000000000..523c9810e7 --- /dev/null +++ b/apps/web/hooks/domains/session/use-terminals-build.ts @@ -0,0 +1,122 @@ +import type { UserShellInfo } from "@/lib/state/slices/session-runtime/types"; + +export const TERMINAL_TYPE_DEV_SERVER = "dev-server"; +export const BOTTOM_PANEL_TERMINAL_ID = "bottom-panel"; + +import type { Terminal, TerminalType } from "./use-terminals-types"; + +export function deriveLabel(shell: UserShellInfo): string { + if (shell.customName && shell.customName !== "") return shell.customName; + if (shell.displayName) return shell.displayName; + if (shell.label) return shell.label; + if (shell.kind === "ordinary" && shell.seq) return `Terminal ${shell.seq}`; + return shell.terminalId.startsWith("script-") ? "Script" : "Terminal"; +} + +export function isOrdinary(shell: UserShellInfo): boolean { + if (shell.kind) return shell.kind === "ordinary"; + // Legacy fallback — ids without script-/bottom-panel prefix are treated + // as ordinary if they have ordinary-shaped metadata. + return ( + shell.terminalId !== BOTTOM_PANEL_TERMINAL_ID && + !shell.terminalId.startsWith("script-") && + shell.seq !== undefined + ); +} + +export function shellToTerminal(shell: UserShellInfo): Terminal { + const ordinary = isOrdinary(shell); + const isScript = shell.terminalId.startsWith("script-"); + return { + id: shell.terminalId, + type: isScript ? "script" : "shell", + label: deriveLabel(shell), + closable: shell.closable ?? true, + kind: ordinary ? "ordinary" : shell.kind, + seq: shell.seq, + customName: shell.customName, + state: shell.state, + ptyStatus: shell.ptyStatus, + }; +} + +/** + * Compute the effective active tab value, preferring store, then + * sessionStorage, then fallback. + * + * `activeTab` is only honoured when it points at a tab that is still + * selectable — i.e. present in `terminals` or the synthetic "commands" + * tab. A stale id (e.g. left over from a terminal that was parked or + * destroyed in another tab) would otherwise stay active and block the + * fallback shift to a real shell. The "commands" id is always accepted + * because it has no corresponding entry in `terminals`. + */ +export function computeTerminalTabValue( + activeTab: string | undefined, + sessionJustChanged: boolean, + savedTabFromStorage: string | null, + terminals: Terminal[], + savedTabExists: boolean, +): string { + const activeTabSelectable = + !!activeTab && (activeTab === "commands" || terminals.some((t) => t.id === activeTab)); + const effectiveActiveTab = + !sessionJustChanged && activeTab && activeTab !== "" && activeTabSelectable ? activeTab : null; + return ( + effectiveActiveTab ?? + (savedTabFromStorage && (terminals.length === 0 || savedTabExists) + ? savedTabFromStorage + : null) ?? + terminals.find((t) => t.type === "shell")?.id ?? + "commands" + ); +} + +/** Build terminal list from user shells, preserving dev-server terminal if present. */ +export function buildTerminalsFromShells( + prev: Terminal[], + userShells: UserShellInfo[], +): Terminal[] { + const devTerminal = prev.find((t) => t.type === TERMINAL_TYPE_DEV_SERVER); + const visibleShells = userShells.filter((s) => { + // Parked terminals live in their own submenu, not the main strip. + if (s.state === "parked") return false; + // The bottom-panel terminal renders inside its own dedicated component + // (bottom-terminal-panel.tsx) — exclude it from the right-panel strip. + if (s.terminalId === BOTTOM_PANEL_TERMINAL_ID) return false; + return true; + }); + const userTerminals = visibleShells.map(shellToTerminal); + const result: Terminal[] = []; + if (devTerminal) result.push(devTerminal); + result.push(...userTerminals); + return result; +} + +export function buildParkedTerminals( + userShells: UserShellInfo[], + userShellsLoaded: boolean, +): Terminal[] { + if (!userShellsLoaded) return []; + return userShells.filter((s) => s.state === "parked").map(shellToTerminal); +} + +/** Sync the dev-server terminal with preview open state. */ +export function syncDevTerminal(prev: Terminal[], previewOpen: boolean): Terminal[] { + const hasDevTerminal = prev.some((t) => t.type === TERMINAL_TYPE_DEV_SERVER); + if (previewOpen && !hasDevTerminal) { + return [ + { + id: TERMINAL_TYPE_DEV_SERVER, + type: TERMINAL_TYPE_DEV_SERVER as TerminalType, + label: "Dev Server", + closable: true, + }, + ...prev, + ]; + } + if (!previewOpen && hasDevTerminal) { + return prev.filter((t) => t.type !== TERMINAL_TYPE_DEV_SERVER); + } + return prev; +} diff --git a/apps/web/hooks/domains/session/use-terminals-types.ts b/apps/web/hooks/domains/session/use-terminals-types.ts new file mode 100644 index 0000000000..0c9732b6de --- /dev/null +++ b/apps/web/hooks/domains/session/use-terminals-types.ts @@ -0,0 +1,25 @@ +import type { + UserShellKind, + UserShellState, + UserShellPTYStatus, +} from "@/lib/state/slices/session-runtime/types"; + +export type TerminalType = "dev-server" | "shell" | "script"; + +/** + * Terminal tab descriptor consumed by the right-panel UI. Ordinary + * terminals carry the new `kind="ordinary"` discriminator plus seq + + * customName + state + ptyStatus; non-ordinary tabs (dev-server, scripts, + * the fixed bottom-panel) leave those undefined. + */ +export type Terminal = { + id: string; + type: TerminalType; + label: string; + closable: boolean; + kind?: UserShellKind; + seq?: number; + customName?: string | null; + state?: UserShellState; + ptyStatus?: UserShellPTYStatus; +}; diff --git a/apps/web/hooks/domains/session/use-terminals.ts b/apps/web/hooks/domains/session/use-terminals.ts index 001afbe987..9d7cc5a548 100644 --- a/apps/web/hooks/domains/session/use-terminals.ts +++ b/apps/web/hooks/domains/session/use-terminals.ts @@ -1,36 +1,42 @@ "use client"; -import { useEffect, useLayoutEffect, useState, useCallback, useRef, useMemo } from "react"; +import { useEffect, useLayoutEffect, useState, useCallback, useRef } from "react"; import { getSessionStorage } from "@/lib/local-storage"; -import { useAppStore } from "@/components/state-provider"; +import { useAppStore, useAppStoreApi } from "@/components/state-provider"; import { stopProcess } from "@/lib/api"; -import { stopUserShell, createUserShell } from "@/lib/api/domains/user-shell-api"; +import { + destroyUserShell, + parkUserShell, + resumeUserShell, + renameUserShell, + createUserShell, +} from "@/lib/api/domains/user-shell-api"; import { useUserShells } from "./use-user-shells"; +import { + buildParkedTerminals, + buildTerminalsFromShells, + computeTerminalTabValue, + syncDevTerminal, +} from "./use-terminals-build"; +import type { Terminal, TerminalType } from "./use-terminals-types"; +import type { UserShellInfo } from "@/lib/state/slices/session-runtime/types"; import type { RepositoryScript } from "@/lib/types/http"; import type { Dispatch, SetStateAction, MouseEvent } from "react"; import type { PreviewStage } from "@/lib/state/slices"; -const TERMINAL_TYPE_DEV_SERVER = "dev-server"; - -export type TerminalType = "dev-server" | "shell" | "script"; - -export type Terminal = { - id: string; - type: TerminalType; - label: string; - closable: boolean; -}; +export type { Terminal, TerminalType }; interface UseTerminalsOptions { - /** Session id used for tab-UI persistence (active-tab keying is per-session UX). */ + /** Session id used for the active-tab UX (tab restoration is per-session). */ sessionId: string | null; - /** Task environment id — the actual scope of the shell list. Required for create/list/stop. */ + /** Task environment id — the agentctl scope. Required for stop/destroy. */ environmentId: string | null; initialTerminals?: Terminal[]; } interface UseTerminalsReturn { terminals: Terminal[]; + parkedTerminals: Terminal[]; activeTab: string | undefined; terminalTabValue: string; addTerminal: () => void; @@ -38,101 +44,34 @@ interface UseTerminalsReturn { handleCloseDevTab: (event: MouseEvent) => Promise; handleCloseTab: (event: MouseEvent, terminalId: string) => void; handleRunCommand: (script: RepositoryScript) => void; + renameTerminal: (id: string, name: string | null) => Promise; + resumeTerminal: (id: string) => Promise; + destroyTerminal: (id: string) => Promise; isStoppingDev: boolean; devProcessId: string | undefined; devOutput: string; } -/** Compute the effective active tab value, preferring store, then sessionStorage, then fallback. */ -function computeTerminalTabValue( - activeTab: string | undefined, - sessionJustChanged: boolean, - savedTabFromStorage: string | null, - terminals: Terminal[], - savedTabExists: boolean, -): string { - const effectiveActiveTab = - !sessionJustChanged && activeTab && activeTab !== "" ? activeTab : null; - return ( - effectiveActiveTab ?? - (savedTabFromStorage && (terminals.length === 0 || savedTabExists) - ? savedTabFromStorage - : null) ?? - terminals.find((t) => t.type === "shell")?.id ?? - "commands" - ); -} - -/** Build terminal list from user shells, preserving dev-server terminal if present. */ -function buildTerminalsFromShells( - prev: Terminal[], - userShells: Array<{ terminalId: string; label: string; closable: boolean }>, -): Terminal[] { - const devTerminal = prev.find((t) => t.type === TERMINAL_TYPE_DEV_SERVER); - const userTerminals: Terminal[] = userShells.map((shell) => { - const isScript = shell.terminalId.startsWith("script-"); - return { - id: shell.terminalId, - type: isScript ? ("script" as const) : ("shell" as const), - label: shell.label || (isScript ? "Script" : "Terminal"), - closable: shell.closable, - }; - }); - const result: Terminal[] = []; - if (devTerminal) result.push(devTerminal); - result.push(...userTerminals); - return result; -} - -/** Sync the dev-server terminal with preview open state. */ -function syncDevTerminal(prev: Terminal[], previewOpen: boolean): Terminal[] { - const hasDevTerminal = prev.some((t) => t.type === TERMINAL_TYPE_DEV_SERVER); - if (previewOpen && !hasDevTerminal) { - return [ - { - id: TERMINAL_TYPE_DEV_SERVER, - type: TERMINAL_TYPE_DEV_SERVER as TerminalType, - label: "Dev Server", - closable: true, - }, - ...prev, - ]; - } - if (!previewOpen && hasDevTerminal) { - return prev.filter((t) => t.type !== TERMINAL_TYPE_DEV_SERVER); - } - return prev; -} - type TerminalSyncOptions = { - sessionId: string | null; environmentId: string | null; - userShells: Array<{ terminalId: string; label: string; closable: boolean }>; + userShells: UserShellInfo[]; userShellsLoaded: boolean; previewOpen: boolean; setTerminals: Dispatch>; - setRightPanelActiveTab: (sessionId: string, tabId: string) => void; }; function useTerminalSync({ - sessionId, environmentId, userShells, userShellsLoaded, previewOpen, setTerminals, - setRightPanelActiveTab, }: TerminalSyncOptions) { - const prevEnvIdRef = useRef(environmentId); const tabRestoredRef = useRef(false); - useEffect(() => { - if (prevEnvIdRef.current === environmentId) return; - prevEnvIdRef.current = environmentId; - tabRestoredRef.current = false; - setTerminals([]); - if (sessionId) setRightPanelActiveTab(sessionId, ""); - }, [environmentId, sessionId, setRightPanelActiveTab, setTerminals]); + // Note: deliberately no "reset on env change" branch. Stranded process + // recovery requires the UI to keep its terminal references stable; the + // server's list call is authoritative once it lands. useEffect(() => { if (!environmentId || !userShellsLoaded) return; @@ -199,50 +138,77 @@ function useTerminalStore(sessionId: string | null, devProcessId: string | undef }; } -type TerminalActionsOptions = { - sessionId: string | null; +type AddTerminalOpts = { environmentId: string | null; - activeTab: string | undefined; - terminals: Terminal[]; - devProcessId: string | undefined; + taskID: string | null; + sessionId: string | null; setTerminals: Dispatch>; setRightPanelActiveTab: (sessionId: string, tabId: string) => void; - setPreviewOpen: (sessionId: string, open: boolean) => void; - setPreviewStage: (sessionId: string, stage: PreviewStage) => void; }; -function useTerminalActions({ - sessionId, +function useAddTerminal({ environmentId, - activeTab, - terminals, - devProcessId, + taskID, + sessionId, setTerminals, setRightPanelActiveTab, - setPreviewOpen, - setPreviewStage, -}: TerminalActionsOptions) { - const [isStoppingDev, setIsStoppingDev] = useState(false); - - const addTerminal = useCallback(async () => { +}: AddTerminalOpts) { + return useCallback(async () => { if (!environmentId) return; try { - const result = await createUserShell(environmentId); - setTerminals((prev) => [ - ...prev, - { id: result.terminalId, type: "shell", label: result.label, closable: result.closable }, - ]); + const result = await createUserShell(environmentId, { taskId: taskID ?? undefined }); + // During rollout an older backend returns the legacy payload + // (terminal_id, label, closable) without `kind` or `seq`. Defaulting + // to "ordinary" in that case marks the tab as managed and routes + // close → park, which fails on the old backend with unknown-action. + // `seq` is the only field a backend that supports the DB-backed + // path always populates, so gate on its presence alone — a + // bare-bones `kind: "ordinary"` without `seq` is treated as legacy. + const ordinary = result.seq !== undefined; + const newTerm: Terminal = { + id: result.terminalId, + type: "shell", + label: result.displayName ?? result.label ?? "Terminal", + closable: true, + kind: ordinary ? "ordinary" : result.kind, + seq: result.seq, + state: ordinary ? (result.state ?? "open") : result.state, + ptyStatus: result.ptyStatus ?? "stopped", + }; + setTerminals((prev) => [...prev, newTerm]); if (sessionId) setRightPanelActiveTab(sessionId, result.terminalId); } catch (error) { console.error("Failed to create user shell:", error); } - }, [environmentId, sessionId, setRightPanelActiveTab, setTerminals]); + }, [environmentId, taskID, sessionId, setRightPanelActiveTab, setTerminals]); +} - const removeTerminal = useCallback( +type RemoveTerminalOpts = { + /** + * Source-of-truth getter for the currently active tab. The handler reads + * this at call-time rather than capturing the value in the closure so an + * async close (`parkUserShell(...).then(removeTerminal)`) that resolves + * after the user switches tabs doesn't clobber the new selection with a + * stale fallback shift. + */ + getActiveTab: () => string | undefined; + sessionId: string | null; + setTerminals: Dispatch>; + setRightPanelActiveTab: (sessionId: string, tabId: string) => void; +}; + +function useRemoveTerminal({ + getActiveTab, + sessionId, + setTerminals, + setRightPanelActiveTab, +}: RemoveTerminalOpts) { + return useCallback( (id: string) => { + const activeTab = getActiveTab(); setTerminals((prev) => { const indexToRemove = prev.findIndex((t) => t.id === id); - if (indexToRemove === -1 || !prev[indexToRemove].closable) return prev; + if (indexToRemove === -1) return prev; if (activeTab === id && sessionId) { const nextTerminals = prev.filter((_, i) => i !== indexToRemove); const next = indexToRemove > 0 ? prev[indexToRemove - 1] : nextTerminals[0]; @@ -251,10 +217,30 @@ function useTerminalActions({ return prev.filter((t) => t.id !== id); }); }, - [activeTab, sessionId, setRightPanelActiveTab, setTerminals], + [getActiveTab, sessionId, setRightPanelActiveTab, setTerminals], ); +} - const handleCloseDevTab = useCallback( +type CloseDevTabOpts = { + sessionId: string | null; + devProcessId: string | undefined; + terminals: Terminal[]; + setRightPanelActiveTab: (sessionId: string, tabId: string) => void; + setPreviewOpen: (sessionId: string, open: boolean) => void; + setPreviewStage: (sessionId: string, stage: PreviewStage) => void; + setIsStoppingDev: Dispatch>; +}; + +function useCloseDevTab({ + sessionId, + devProcessId, + terminals, + setRightPanelActiveTab, + setPreviewOpen, + setPreviewStage, + setIsStoppingDev, +}: CloseDevTabOpts) { + return useCallback( async (event: MouseEvent) => { event.preventDefault(); event.stopPropagation(); @@ -272,18 +258,215 @@ function useTerminalActions({ setPreviewOpen(sessionId, false); setPreviewStage(sessionId, "closed"); }, - [sessionId, devProcessId, terminals, setRightPanelActiveTab, setPreviewOpen, setPreviewStage], + [ + sessionId, + devProcessId, + terminals, + setRightPanelActiveTab, + setPreviewOpen, + setPreviewStage, + setIsStoppingDev, + ], ); +} + +type CloseTabOpts = { + environmentId: string | null; + taskID: string | null; + terminals: Terminal[]; + removeTerminal: (id: string) => void; + removeUserShellStore: (environmentId: string, terminalId: string) => void; + updateUserShell: ( + environmentId: string, + terminalId: string, + patch: { state?: "open" | "parked" }, + ) => void; +}; + +/** + * X-button close. For ordinary terminals this PARKS the tab (PTY keeps + * running, user can resume from the "Parked terminals" submenu). For + * scripts and any non-ordinary terminal it falls back to destroy. + * + * The local tab is removed only AFTER the backend call resolves — that + * way a transient failure (network, backend 500) leaves the tab on the + * strip rather than disappearing into thin air. The next `user_shell.list` + * poll then reflects whatever state the backend actually settled on. + */ +function useCloseTab({ + environmentId, + taskID, + terminals, + removeTerminal, + removeUserShellStore, + updateUserShell, +}: CloseTabOpts) { + return useCallback( + (event: MouseEvent, terminalId: string) => { + event.preventDefault(); + event.stopPropagation(); + if (!environmentId) return; + const term = terminals.find((t) => t.id === terminalId); + const isOrdinaryTab = term?.kind === "ordinary"; + if (isOrdinaryTab) { + parkUserShell(terminalId, taskID ?? undefined) + .then(() => { + updateUserShell(environmentId, terminalId, { state: "parked" }); + removeTerminal(terminalId); + }) + .catch((error) => console.error("Failed to park terminal:", error)); + } else { + destroyUserShell(environmentId, terminalId, taskID ?? undefined) + .then(() => { + removeUserShellStore(environmentId, terminalId); + removeTerminal(terminalId); + }) + .catch((error) => console.error("Failed to destroy terminal:", error)); + } + }, + [environmentId, taskID, terminals, removeTerminal, removeUserShellStore, updateUserShell], + ); +} + +type ManagedTerminalActionsOpts = { + environmentId: string | null; + taskID: string | null; + updateUserShell: ( + environmentId: string, + terminalId: string, + patch: { customName?: string | null; state?: "open" | "parked" }, + ) => void; + removeUserShellStore: (environmentId: string, terminalId: string) => void; + removeTerminal: (id: string) => void; +}; + +function useManagedTerminalActions({ + environmentId, + taskID, + updateUserShell, + removeUserShellStore, + removeTerminal, +}: ManagedTerminalActionsOpts) { + const renameTerminal = useCallback( + async (id: string, name: string | null) => { + if (!environmentId) return; + const trimmed = name === null ? null : name.trim(); + const normalized = trimmed === "" ? null : trimmed; + try { + await renameUserShell(id, normalized, taskID ?? undefined); + updateUserShell(environmentId, id, { customName: normalized }); + } catch (error) { + console.error("Failed to rename terminal:", error); + } + }, + [environmentId, taskID, updateUserShell], + ); + + const resumeTerminal = useCallback( + async (id: string) => { + if (!environmentId) return; + try { + await resumeUserShell(id, taskID ?? undefined); + updateUserShell(environmentId, id, { state: "open" }); + } catch (error) { + console.error("Failed to resume terminal:", error); + } + }, + [environmentId, taskID, updateUserShell], + ); + + const destroyTerminal = useCallback( + async (id: string) => { + if (!environmentId) return; + try { + await destroyUserShell(environmentId, id, taskID ?? undefined); + removeUserShellStore(environmentId, id); + removeTerminal(id); + } catch (error) { + console.error("Failed to destroy terminal:", error); + } + }, + [environmentId, taskID, removeUserShellStore, removeTerminal], + ); + + return { renameTerminal, resumeTerminal, destroyTerminal }; +} + +type TerminalActionsOptions = { + sessionId: string | null; + environmentId: string | null; + terminals: Terminal[]; + devProcessId: string | undefined; + setTerminals: Dispatch>; + setRightPanelActiveTab: (sessionId: string, tabId: string) => void; + setPreviewOpen: (sessionId: string, open: boolean) => void; + setPreviewStage: (sessionId: string, stage: PreviewStage) => void; +}; + +function useTerminalActions({ + sessionId, + environmentId, + terminals, + devProcessId, + setTerminals, + setRightPanelActiveTab, + setPreviewOpen, + setPreviewStage, +}: TerminalActionsOptions) { + const [isStoppingDev, setIsStoppingDev] = useState(false); + const updateUserShell = useAppStore((state) => state.updateUserShell); + const removeUserShellStore = useAppStore((state) => state.removeUserShell); + + const taskID = useAppStore((state) => state.tasks?.activeTaskId ?? null); + + // getActiveTab reads the live store at call-time rather than capturing + // `activeTab` in the useCallback closure. Async close handlers that + // resolve after a user tab-switch see the new selection immediately + // — no useEffect/useLayoutEffect race window because the store is the + // single source of truth and `getState()` is synchronous. + const storeApi = useAppStoreApi(); + const getActiveTab = useCallback( + () => (sessionId ? storeApi.getState().rightPanel.activeTabBySessionId[sessionId] : undefined), + [storeApi, sessionId], + ); + + const addTerminal = useAddTerminal({ + environmentId, + taskID, + sessionId, + setTerminals, + setRightPanelActiveTab, + }); + const removeTerminal = useRemoveTerminal({ + getActiveTab, + sessionId, + setTerminals, + setRightPanelActiveTab, + }); + + const handleCloseDevTab = useCloseDevTab({ + sessionId, + devProcessId, + terminals, + setRightPanelActiveTab, + setPreviewOpen, + setPreviewStage, + setIsStoppingDev, + }); const handleRunCommand = useCallback( async (script: RepositoryScript) => { if (!environmentId) return; try { const result = await createUserShell(environmentId, { scriptId: script.id }); - setTerminals((prev) => [ - ...prev, - { id: result.terminalId, type: "script", label: result.label, closable: result.closable }, - ]); + const newTerm: Terminal = { + id: result.terminalId, + type: "script", + label: result.label ?? script.name ?? "Script", + closable: true, + kind: "script", + }; + setTerminals((prev) => [...prev, newTerm]); if (sessionId) setRightPanelActiveTab(sessionId, result.terminalId); } catch (error) { console.error("Failed to create script terminal:", error); @@ -292,19 +475,22 @@ function useTerminalActions({ [environmentId, sessionId, setRightPanelActiveTab, setTerminals], ); - const handleCloseTab = useCallback( - (event: MouseEvent, terminalId: string) => { - event.preventDefault(); - event.stopPropagation(); - removeTerminal(terminalId); - if (environmentId) { - stopUserShell(environmentId, terminalId).catch((error) => { - console.error("Failed to stop terminal:", error); - }); - } - }, - [environmentId, removeTerminal], - ); + const handleCloseTab = useCloseTab({ + environmentId, + taskID, + terminals, + removeTerminal, + removeUserShellStore, + updateUserShell, + }); + + const { renameTerminal, resumeTerminal, destroyTerminal } = useManagedTerminalActions({ + environmentId, + taskID, + updateUserShell, + removeUserShellStore, + removeTerminal, + }); return { isStoppingDev, @@ -313,6 +499,9 @@ function useTerminalActions({ handleCloseDevTab, handleRunCommand, handleCloseTab, + renameTerminal, + resumeTerminal, + destroyTerminal, }; } @@ -338,16 +527,21 @@ export function useTerminals({ setPreviewStage, } = useTerminalStore(sessionId, devProcessId); - const { shells: userShells, isLoaded: userShellsLoaded } = useUserShells(environmentId); + // Pass taskID so the backend's DB-backed ordinary-shell path fires; without + // it `user_shell.list` only returns legacy passthrough shells and the + // parked-terminals submenu stays empty. + const activeTaskId = useAppStore((state) => state.tasks?.activeTaskId ?? null); + const { shells: userShells, isLoaded: userShellsLoaded } = useUserShells( + environmentId, + activeTaskId, + ); const tabRestoredRef = useTerminalSync({ - sessionId, environmentId, userShells, userShellsLoaded, previewOpen, setTerminals, - setRightPanelActiveTab, }); useTabRestoration(sessionId, terminals, activeTab, tabRestoredRef, setRightPanelActiveTab); @@ -359,10 +553,12 @@ export function useTerminals({ handleCloseDevTab, handleRunCommand, handleCloseTab, + renameTerminal, + resumeTerminal, + destroyTerminal, } = useTerminalActions({ sessionId, environmentId, - activeTab, terminals, devProcessId, setTerminals, @@ -371,23 +567,22 @@ export function useTerminals({ setPreviewStage, }); - const savedTabFromStorage = useMemo(() => { - if (!sessionId) return null; - return getSessionStorage(`rightPanel-tab-${sessionId}`, null); - }, [sessionId]); - - const savedTabExists = savedTabFromStorage && terminals.some((t) => t.id === savedTabFromStorage); + const parkedTerminals = buildParkedTerminals(userShells, userShellsLoaded); + const savedTabFromStorage = sessionId + ? getSessionStorage(`rightPanel-tab-${sessionId}`, null) + : null; const terminalTabValue = computeTerminalTabValue( activeTab, sessionJustChanged, savedTabFromStorage, terminals, - !!savedTabExists, + !!(savedTabFromStorage && terminals.some((t) => t.id === savedTabFromStorage)), ); return { terminals, + parkedTerminals, activeTab, terminalTabValue, addTerminal, @@ -395,6 +590,9 @@ export function useTerminals({ handleCloseDevTab, handleCloseTab, handleRunCommand, + renameTerminal, + resumeTerminal, + destroyTerminal, isStoppingDev, devProcessId, devOutput, diff --git a/apps/web/hooks/domains/session/use-user-shells.ts b/apps/web/hooks/domains/session/use-user-shells.ts index 7703586808..73790ce211 100644 --- a/apps/web/hooks/domains/session/use-user-shells.ts +++ b/apps/web/hooks/domains/session/use-user-shells.ts @@ -1,7 +1,12 @@ import { useEffect, useRef } from "react"; import { getWebSocketClient } from "@/lib/ws/connection"; import { useAppStore, useAppStoreApi } from "@/components/state-provider"; -import type { UserShellInfo } from "@/lib/state/slices"; +import type { + UserShellInfo, + UserShellKind, + UserShellState, + UserShellPTYStatus, +} from "@/lib/state/slices"; interface UseUserShellsReturn { shells: UserShellInfo[]; @@ -13,18 +18,45 @@ interface UseUserShellsReturn { const EMPTY_SHELLS: UserShellInfo[] = []; +/** + * Wire shape of an item in the `user_shell.list` response. Backend returns + * a discriminated union — `kind: "ordinary"` carries DB metadata, `fixed` + * and `script` only carry the id + pty_status + label. + */ +type ListResponseItem = { + id?: string; + terminal_id?: string; // legacy / passthrough + kind?: UserShellKind; + seq?: number; + display_name?: string; + custom_name?: string | null; + state?: UserShellState; + pty_status?: UserShellPTYStatus; + label?: string; + closable?: boolean; + initial_command?: string; + process_id?: string; + running?: boolean; +}; + /** * Hook to fetch and manage user shell terminals for a task environment. * - * User shells are env-scoped — sessions in the same task share one shell list. - * Pass the environment id directly; do not pass a session id. + * `taskId` is required for the backend's DB-backed ordinary-terminal path + * to fire. Without it, only the legacy passthrough shells (bottom-panel, + * scripts) come back — first-class persistent terminals would never reach + * the panel strip, and the parked-terminals submenu would always be + * empty. * * Follows the data fetching pattern: * 1. Read from store first * 2. Fetch from backend if not loaded * 3. Track loading/loaded state */ -export function useUserShells(environmentId: string | null): UseUserShellsReturn { +export function useUserShells( + environmentId: string | null, + taskId?: string | null, +): UseUserShellsReturn { const store = useAppStoreApi(); const shells = useAppStore((state) => { @@ -41,25 +73,30 @@ export function useUserShells(environmentId: string | null): UseUserShellsReturn }); const connectionStatus = useAppStore((state) => state.connection.status); - // Guard ref to prevent duplicate fetches per env - const lastFetchedEnvIdRef = useRef(null); + // Cache the fetch scope as env+task. An earlier load without a task + // id (e.g. before the active task hydrated) must not block the + // task-scoped refetch — that would pin the store on the legacy + // shells while the DB-backed ordinary rows never reach the panel. + const fetchScope = environmentId ? `${environmentId}:${taskId ?? ""}` : null; + const lastFetchedScopeRef = useRef(null); - // Reset ref when environmentId clears + // Reset ref when scope clears (no env, or env+task changed) useEffect(() => { - if (!environmentId) { - lastFetchedEnvIdRef.current = null; + if (!fetchScope) { + lastFetchedScopeRef.current = null; } - }, [environmentId]); + }, [fetchScope]); // Fetch user shells from backend useEffect(() => { - if (!environmentId) return; + if (!environmentId || !fetchScope) return; if (connectionStatus !== "connected") return; - if (isLoaded) { - lastFetchedEnvIdRef.current = environmentId; - return; - } - if (lastFetchedEnvIdRef.current === environmentId) return; + // Skip when this exact scope has already been fetched. A different + // env+task pair re-runs the effect because the ref no longer matches. + if (lastFetchedScopeRef.current === fetchScope) return; + // `isLoaded` stays in deps so external store invalidation can + // re-run this effect; fetchScope + lastFetchedScopeRef still guard + // duplicate requests for the same env+task. const fetchShells = async () => { const client = getWebSocketClient(); @@ -68,37 +105,38 @@ export function useUserShells(environmentId: string | null): UseUserShellsReturn store.getState().setUserShellsLoading(environmentId, true); try { - const response = await client.request<{ - shells?: Array<{ - terminal_id: string; - process_id: string; - running: boolean; - label: string; - closable: boolean; - initial_command?: string; - }>; - }>("user_shell.list", { task_environment_id: environmentId }, 10000); + // include_parked=true is required for the "Parked terminals" + // submenu to populate after a page reload — without it the + // backend returns only state=open rows and parked PTYs become + // invisible until the user parks something in the same session. + // `buildTerminalsFromShells` already filters parked entries out + // of the main strip so this doesn't change visible-tab behaviour. + const payload: Record = { + task_environment_id: environmentId, + include_parked: true, + }; + if (taskId) payload.task_id = taskId; + const response = await client.request<{ shells?: ListResponseItem[] }>( + "user_shell.list", + payload, + 10000, + ); - const shells: UserShellInfo[] = (response.shells ?? []).map((s) => ({ - terminalId: s.terminal_id, - processId: s.process_id, - running: s.running, - label: s.label || "Terminal", - closable: s.closable, - initialCommand: s.initial_command, - })); - - store.getState().setUserShells(environmentId, shells); - lastFetchedEnvIdRef.current = environmentId; + const mapped: UserShellInfo[] = (response.shells ?? []).map((s) => mapListItemToShell(s)); + store.getState().setUserShells(environmentId, mapped); + lastFetchedScopeRef.current = fetchScope; } catch (error) { console.error("Failed to fetch user shells:", error); store.getState().setUserShells(environmentId, []); - lastFetchedEnvIdRef.current = environmentId; + lastFetchedScopeRef.current = fetchScope; } }; fetchShells(); - }, [environmentId, connectionStatus, isLoaded, store]); + // `taskId` is captured inside `payload` via the fetchShells closure; + // including it in the deps array is the React-hooks-lint preference + // even though `fetchScope` already encodes the same information. + }, [environmentId, fetchScope, taskId, connectionStatus, isLoaded, store]); const addShell = (shell: UserShellInfo) => { if (environmentId) { @@ -120,3 +158,29 @@ export function useUserShells(environmentId: string | null): UseUserShellsReturn removeShell, }; } + +/** + * Maps the wire shape onto the in-store `UserShellInfo`. The new handler + * returns `id` (typed `kind: "ordinary" | "fixed" | "script"`), the + * legacy passthrough returns `terminal_id`. Both populate the legacy + * `processId/running/label/closable` fields when present so older + * consumers (e.g. the `useTerminals` build path for non-ordinary tabs) + * keep rendering correctly. + */ +function mapListItemToShell(s: ListResponseItem): UserShellInfo { + const id = (s.id ?? s.terminal_id ?? "") as string; + return { + terminalId: id, + kind: s.kind, + seq: s.seq, + customName: s.custom_name ?? null, + displayName: s.display_name, + state: s.state, + ptyStatus: s.pty_status, + processId: s.process_id, + running: s.running ?? s.pty_status === "running", + label: s.label || s.display_name || "Terminal", + closable: s.closable ?? true, + initialCommand: s.initial_command, + }; +} diff --git a/apps/web/hooks/use-editor-keybinds.ts b/apps/web/hooks/use-editor-keybinds.ts index bf7afa4d43..bd4ac2ea46 100644 --- a/apps/web/hooks/use-editor-keybinds.ts +++ b/apps/web/hooks/use-editor-keybinds.ts @@ -31,6 +31,7 @@ function handleTerminalToggle( api: DockviewApi, previousPanelIdRef: React.MutableRefObject, getEnvironmentId: () => string | null, + getTaskID: () => string | null, ) { e.preventDefault(); e.stopPropagation(); @@ -58,10 +59,14 @@ function handleTerminalToggle( const environmentId = getEnvironmentId(); if (!environmentId) return; + const taskID = getTaskID(); - createUserShell(environmentId) + createUserShell(environmentId, { taskId: taskID ?? undefined }) .then((result) => { - useDockviewStore.getState().addTerminalPanel(result.terminalId, undefined, environmentId); + const title = result.displayName ?? result.label ?? "Terminal"; + useDockviewStore + .getState() + .addTerminalPanel(result.terminalId, undefined, environmentId, taskID ?? undefined, title); }) .catch((err) => { console.warn("Failed to create terminal shell:", err); @@ -168,11 +173,17 @@ export function useEditorKeybinds() { const isTerminalToggle = e.ctrlKey && !e.metaKey && !e.shiftKey && e.code === "Backquote"; if (isTerminalToggle) { - handleTerminalToggle(e, api, previousPanelIdRef, () => { - const state = appStore.getState(); - const sid = state.tasks.activeSessionId; - return sid ? (state.environmentIdBySessionId[sid] ?? null) : null; - }); + handleTerminalToggle( + e, + api, + previousPanelIdRef, + () => { + const state = appStore.getState(); + const sid = state.tasks.activeSessionId; + return sid ? (state.environmentIdBySessionId[sid] ?? null) : null; + }, + () => appStore.getState().tasks?.activeTaskId ?? null, + ); return; } diff --git a/apps/web/lib/api/domains/user-shell-api.ts b/apps/web/lib/api/domains/user-shell-api.ts index fdaaac6ae4..426b565b2c 100644 --- a/apps/web/lib/api/domains/user-shell-api.ts +++ b/apps/web/lib/api/domains/user-shell-api.ts @@ -1,92 +1,152 @@ import { getWebSocketClient } from "@/lib/ws/connection"; import { getBackendConfig } from "@/lib/config"; +const WS_UNAVAILABLE = "WebSocket client not available"; + /** - * Terminal info returned by HTTP API (for SSR). + * Terminal info returned by the HTTP/WS list endpoint. + * + * Backend uses a discriminated union (kind: ordinary | fixed | script). + * Ordinary terminals carry seq + custom_name + state; fixed (bottom-panel) + * and script terminals omit those. + * + * Legacy keys (process_id, running, label, closable) remain in the type as + * optional so older agentctl responses still decode. New UI reads kind + + * seq + display_name + state + pty_status. */ export type TerminalInfo = { - terminal_id: string; - process_id: string; - running: boolean; - label: string; - closable: boolean; + terminal_id?: string; // legacy SSR shape (env-keyed handler) + id?: string; // new task-keyed handler + kind?: "ordinary" | "fixed" | "script"; + seq?: number; + display_name?: string; + custom_name?: string | null; + state?: "open" | "parked"; + pty_status?: "running" | "stopped"; + + // Legacy fields kept for env-keyed SSR fallback. + process_id?: string; + running?: boolean; + label?: string; + closable?: boolean; initial_command?: string; }; /** - * Fetch terminals for a task environment via HTTP (for SSR). - * This endpoint auto-creates the first "Terminal" if none exist. - * - * User shells are env-scoped — sessions in the same task share one shell list. + * Fetch terminals for a task. Preferred path is the new task-keyed endpoint + * (`/tasks/:taskId/terminals`) which returns the full union, including the + * ordinary terminals' DB metadata. Falls back to the legacy env-keyed + * endpoint if no taskId is supplied (during transition). */ -export async function fetchTerminals(environmentId: string): Promise { +export async function fetchTerminals( + taskID: string, + environmentId?: string, +): Promise { + const { apiBaseUrl } = getBackendConfig(); + if (!taskID) { + if (!environmentId) return []; + return fetchTerminalsByEnv(environmentId); + } + const params = environmentId ? `?task_environment_id=${encodeURIComponent(environmentId)}` : ""; + const url = `${apiBaseUrl}/api/v1/tasks/${encodeURIComponent(taskID)}/terminals${params}`; + try { + const response = await fetch(url); + if (!response.ok) { + console.warn("Failed to fetch terminals (task-keyed):", response.status); + // Fall back to legacy path so older backends still work mid-rollout. + return environmentId ? fetchTerminalsByEnv(environmentId) : []; + } + const data = await response.json(); + return data.terminals ?? []; + } catch (error) { + console.warn("Failed to fetch terminals (task-keyed):", error); + return environmentId ? fetchTerminalsByEnv(environmentId) : []; + } +} + +async function fetchTerminalsByEnv(environmentId: string): Promise { const { apiBaseUrl } = getBackendConfig(); const url = `${apiBaseUrl}/api/v1/environments/${environmentId}/terminals`; try { const response = await fetch(url); if (!response.ok) { - console.warn("Failed to fetch terminals:", response.status); + console.warn("Failed to fetch terminals (env-keyed):", response.status); return []; } const data = await response.json(); return data.terminals ?? []; } catch (error) { - console.warn("Failed to fetch terminals:", error); + console.warn("Failed to fetch terminals (env-keyed):", error); return []; } } /** - * Result of creating a new user shell. + * Result of creating a new user shell. For ordinary terminals the backend + * also returns seq + display_name + state; legacy / script paths return the + * older label + closable fields. */ export type CreateUserShellResult = { terminalId: string; - label: string; - closable: boolean; + kind?: "ordinary" | "fixed" | "script"; + seq?: number; + displayName?: string; + state?: "open" | "parked"; + ptyStatus?: "running" | "stopped"; + label?: string; + closable?: boolean; initialCommand?: string; }; /** - * Options for {@link createUserShell}. Pass `scriptId` to run a stored - * RepositoryScript, or `command` (with optional `label`) to run an arbitrary - * command — used for the repository's dev_script. Omit both for a plain shell. + * Options for {@link createUserShell}. + * - `taskId` is required for ordinary terminals (DB-backed) — without it + * the create falls back to the legacy non-persistent path. + * - `scriptId` runs a stored RepositoryScript. + * - `command` (with optional `label`) runs an arbitrary command — script + * terminal. */ export type CreateUserShellOptions = { + taskId?: string; scriptId?: string; command?: string; label?: string; }; -/** - * Create a new user shell terminal. - * Backend assigns the terminal ID, label, and closable status. - * First terminal is "Terminal" and not closable, subsequent are "Terminal 2", etc. and closable. - * - * User shells are env-scoped — pass the task environment ID, not a session ID. - */ export async function createUserShell( environmentId: string, options?: CreateUserShellOptions, ): Promise { const client = getWebSocketClient(); if (!client) { - throw new Error("WebSocket client not available"); + throw new Error(WS_UNAVAILABLE); } const payload: Record = { task_environment_id: environmentId }; + if (options?.taskId) payload.task_id = options.taskId; if (options?.scriptId) payload.script_id = options.scriptId; if (options?.command) payload.command = options.command; if (options?.label) payload.label = options.label; const response = (await client.request("user_shell.create", payload)) as { terminal_id: string; - label: string; - closable: boolean; + kind?: "ordinary" | "fixed" | "script"; + seq?: number; + display_name?: string; + state?: "open" | "parked"; + pty_status?: "running" | "stopped"; + label?: string; + closable?: boolean; initial_command?: string; }; return { terminalId: response.terminal_id, + kind: response.kind, + seq: response.seq, + displayName: response.display_name, + state: response.state, + ptyStatus: response.pty_status, label: response.label, closable: response.closable, initialCommand: response.initial_command, @@ -94,23 +154,100 @@ export async function createUserShell( } /** - * Stop a user shell terminal process. - * This is called when closing a terminal tab to clean up the backend process. + * Destroy (hard-kill PTY + delete row) an ordinary terminal. Also serves + * the legacy `user_shell.stop` action for non-ordinary terminals — backend + * accepts either name. + * + * Rejects (rather than silently resolving) on a missing WS client so the + * caller's `.then()` doesn't fire and remove the tab from the strip + * without a frame ever being sent. Mirrors rename/park/resume's contract. + * + * `taskId` is sent so the backend can verify ownership and reject + * cross-task destroys. Optional for non-managed ids (bottom-panel, + * script-*) where the service guard short-circuits. + * + * Falls back to `user_shell.stop` if the server rejects `user_shell.destroy` + * as an unknown action — preserves compatibility during the rollout + * window where older backends only register the legacy action name. */ -export async function stopUserShell(environmentId: string, terminalId: string): Promise { +export async function destroyUserShell( + environmentId: string, + terminalId: string, + taskId?: string, +): Promise { const client = getWebSocketClient(); - if (!client) { - // WebSocket not available, silently fail (best-effort cleanup) - return; - } - + if (!client) throw new Error(WS_UNAVAILABLE); + const payload: Record = { + task_environment_id: environmentId, + terminal_id: terminalId, + }; + if (taskId) payload.task_id = taskId; try { - await client.request("user_shell.stop", { - task_environment_id: environmentId, - terminal_id: terminalId, - }); + await client.request("user_shell.destroy", payload); } catch (error) { - // Log but don't throw - this is best-effort cleanup - console.warn("Failed to stop user shell:", error); + if (isUnknownActionError(error)) { + await client.request("user_shell.stop", payload); + return; + } + throw error; } } + +/** + * Heuristic check for "server doesn't know this action" responses. The + * WS dispatcher returns an error whose message contains "unknown action" + * or "no handler" depending on backend version; both forms are matched + * by case-insensitive substring search. + */ +function isUnknownActionError(error: unknown): boolean { + const msg = error instanceof Error ? error.message : String(error); + const lower = msg.toLowerCase(); + return lower.includes("unknown action") || lower.includes("no handler"); +} + +/** Legacy name retained for the bottom-panel + script paths. */ +export const stopUserShell = destroyUserShell; + +/** + * Rename an ordinary terminal. Pass `null` to clear the custom name and + * revert to the derived "Terminal {seq}" label. + * + * `taskId` lets the backend verify the terminal belongs to the supplied + * task; a mismatch rejects the rename instead of silently mutating + * another task's row. + */ +export async function renameUserShell( + terminalId: string, + customName: string | null, + taskId?: string, +): Promise { + const client = getWebSocketClient(); + if (!client) throw new Error(WS_UNAVAILABLE); + const payload: Record = { + terminal_id: terminalId, + custom_name: customName, + }; + if (taskId) payload.task_id = taskId; + await client.request("user_shell.rename", payload); +} + +/** + * Park an ordinary terminal — hides the tab from the panel strip but + * leaves the PTY running so the user can resume later. `taskId` enforces + * ownership server-side. + */ +export async function parkUserShell(terminalId: string, taskId?: string): Promise { + const client = getWebSocketClient(); + if (!client) throw new Error(WS_UNAVAILABLE); + const payload: Record = { terminal_id: terminalId }; + if (taskId) payload.task_id = taskId; + await client.request("user_shell.park", payload); +} + +export async function resumeUserShell(terminalId: string, taskId?: string): Promise { + const client = getWebSocketClient(); + if (!client) throw new Error(WS_UNAVAILABLE); + const payload: Record = { terminal_id: terminalId }; + if (taskId) payload.task_id = taskId; + await client.request("user_shell.resume", payload); +} diff --git a/apps/web/lib/ssr/session-page-state.ts b/apps/web/lib/ssr/session-page-state.ts index cddd2f8581..aeedb8f8a3 100644 --- a/apps/web/lib/ssr/session-page-state.ts +++ b/apps/web/lib/ssr/session-page-state.ts @@ -308,6 +308,62 @@ async function fetchTaskDataOnly( return { task, sessionId: null, initialState, initialTerminals: [] }; } +type TerminalApiResponse = Awaited>[number]; + +function shouldHydrateTerminal(t: TerminalApiResponse): boolean { + const id = t.id ?? t.terminal_id ?? ""; + if (!id || id === "bottom-panel") return false; + if (t.state === "parked") return false; + return true; +} + +function classifyTerminal( + t: TerminalApiResponse, + id: string, +): { isScript: boolean; isOrdinary: boolean } { + const isScript = t.kind === "script" || id.startsWith("script-"); + const isOrdinary = t.kind === "ordinary" || (!isScript && t.seq !== undefined); + return { isScript, isOrdinary }; +} + +function deriveHydratedLabel( + t: TerminalApiResponse, + isScript: boolean, + isOrdinary: boolean, +): string { + if (t.display_name) return t.display_name; + if (t.custom_name && t.custom_name !== "") return t.custom_name; + if (t.label) return t.label; + if (isOrdinary && t.seq) return `Terminal ${t.seq}`; + return isScript ? "Script" : "Terminal"; +} + +function pickTerminalKind( + isOrdinary: boolean, + isScript: boolean, +): "ordinary" | "script" | undefined { + if (isOrdinary) return "ordinary"; + if (isScript) return "script"; + return undefined; +} + +function hydrateTerminal(t: TerminalApiResponse): Terminal { + const id = (t.id ?? t.terminal_id ?? "") as string; + const { isScript, isOrdinary } = classifyTerminal(t, id); + const kind = pickTerminalKind(isOrdinary, isScript); + return { + id, + type: isScript ? ("script" as const) : ("shell" as const), + label: deriveHydratedLabel(t, isScript, isOrdinary), + closable: t.closable ?? true, + kind, + seq: t.seq, + customName: t.custom_name ?? undefined, + state: t.state, + ptyStatus: t.pty_status, + }; +} + async function fetchSessionDataFromTask( task: Task, sessionId: string, @@ -342,7 +398,7 @@ async function fetchSessionDataFromTask( })), listSessionTurns(sessionId, { cache: "no-store" }).catch(() => ({ turns: [], total: 0 })), fetchUserSettings({ cache: "no-store" }).catch(() => null), - sessionEnvId ? fetchTerminals(sessionEnvId).catch(() => []) : Promise.resolve([]), + sessionEnvId ? fetchTerminals(task.id, sessionEnvId).catch(() => []) : Promise.resolve([]), listTaskSessionMessages(sessionId, { limit: 50, sort: "desc" }, { cache: "no-store" }).catch( () => null as ListMessagesResponse | null, ), @@ -354,12 +410,9 @@ async function fetchSessionDataFromTask( const workflows = workflowsResponse.workflows ?? []; const turns = turnsResponse.turns ?? []; - const initialTerminals: Terminal[] = terminalsResponse.map((t) => ({ - id: t.terminal_id, - type: t.initial_command ? ("script" as const) : ("shell" as const), - label: t.label, - closable: t.closable, - })); + const initialTerminals: Terminal[] = terminalsResponse + .filter(shouldHydrateTerminal) + .map(hydrateTerminal); const initialState = buildSessionPageState({ task, diff --git a/apps/web/lib/state/dockview-panel-actions.ts b/apps/web/lib/state/dockview-panel-actions.ts index 05de0bde49..0730919a86 100644 --- a/apps/web/lib/state/dockview-panel-actions.ts +++ b/apps/web/lib/state/dockview-panel-actions.ts @@ -471,18 +471,30 @@ export function buildExtraPanelActions(get: StoreGet) { params: prKey ? { prKey } : undefined, }); }, - addTerminalPanel: (terminalId?: string, groupId?: string, environmentId?: string) => { + addTerminalPanel: ( + terminalId?: string, + groupId?: string, + environmentId?: string, + taskID?: string, + title?: string, + ) => { const { api, rightBottomGroupId } = get(); if (!api) return; const id = terminalId ?? `terminal-${Date.now()}`; - // Stamp the env id into the panel's params so cleanup - // (dockview-layout-setup.onDidRemovePanel) can call stopUserShell with - // the correct env even when the user has switched tasks since open. + // Stamp env id + task id into the panel's params so cleanup + // (dockview-layout-setup.onDidRemovePanel) can call destroyUserShell + // with the correct task scope even after the user switches tasks. + // task_id is what the backend uses to verify ownership now — without + // it `requireOwnership` rejects with ErrTaskMismatch. addSimplePanel(api, groupId ?? rightBottomGroupId, { id, component: "terminal", - title: "Terminal", - params: { terminalId: id, environmentId }, + // terminalTab is a custom dockview tab that adds the `#N` badge + // when there's more than one ordinary terminal in the task and + // exposes a context menu for rename / park / destroy. + tabComponent: "terminalTab", + title: title ?? "Terminal", + params: { terminalId: id, environmentId, taskID }, }); }, }; diff --git a/apps/web/lib/state/dockview-store.ts b/apps/web/lib/state/dockview-store.ts index 4b2ca57a03..0e9604db6f 100644 --- a/apps/web/lib/state/dockview-store.ts +++ b/apps/web/lib/state/dockview-store.ts @@ -134,7 +134,13 @@ type DockviewStore = { * activeSessionId anchors the new panel to the session's current group so it lands as a tab * next to the session, not as a split. Falls back to centerGroupId when omitted. */ addPRPanel: (prKey?: string, activeSessionId?: string | null) => void; - addTerminalPanel: (terminalId?: string, groupId?: string, environmentId?: string) => void; + addTerminalPanel: ( + terminalId?: string, + groupId?: string, + environmentId?: string, + taskID?: string, + title?: string, + ) => void; selectedDiff: { path: string; content?: string } | null; setSelectedDiff: (diff: { path: string; content?: string } | null) => void; activeGroupId: string | null; diff --git a/apps/web/lib/state/layout-manager/constants.ts b/apps/web/lib/state/layout-manager/constants.ts index 3e354a933f..2e4de3388d 100644 --- a/apps/web/lib/state/layout-manager/constants.ts +++ b/apps/web/lib/state/layout-manager/constants.ts @@ -59,6 +59,12 @@ export const PANEL_REGISTRY: Record> = { vscode: { component: "vscode", title: "VS Code" }, [TERMINAL_DEFAULT_ID]: { component: "terminal", + // terminalTab is the custom dockview tab that adds the `#N` badge + // and a right-click rename/destroy menu. The hook + // useEnsureDefaultTerminalOrdinary migrates the legacy + // `shell-default` PTY into a DB-backed ordinary terminal on + // session-page mount so the badge logic has a seq to display. + tabComponent: "terminalTab", title: "Terminal", params: { terminalId: "shell-default" }, }, diff --git a/apps/web/lib/state/layout-manager/serializer.ts b/apps/web/lib/state/layout-manager/serializer.ts index efb8ebcf35..cc8f2be543 100644 --- a/apps/web/lib/state/layout-manager/serializer.ts +++ b/apps/web/lib/state/layout-manager/serializer.ts @@ -301,17 +301,52 @@ function isStructuralPanel(p: LayoutPanel): boolean { /** * Normalize dynamic panel IDs so they get fresh sessions on restore. + * + * DB-backed ordinary terminals (id shape `shell-`) are preserved + * intact — the row survives in the backend across reloads and the panel + * must keep its id + params (environmentId, taskID) so the WS reattach + * lands on the live PTY and the store lookup finds the matching shell + * record (so the badge renders). + * + * Tab presentation is also coerced for every terminal panel — the + * registry was updated to use the custom `terminalTab` and a plain + * `"Terminal"` title, but layouts persisted before that change carry + * the old plain tab + `"Terminal {seq}"` title text and would render + * wrong on restore. */ +function isDbBackedTerminalId(id: string): boolean { + // shell- — 36-char hex+dashes after the prefix. Excludes the + // legacy `shell-default` passthrough id and any `terminal-saved-*` + // ids from older layouts. + return /^shell-[0-9a-f-]{36}$/.test(id); +} + function normalizePanel( p: LayoutPanel, counters: { terminal: number; browser: number }, ): LayoutPanel { + if (p.component === "terminal") { + if (KNOWN_PANEL_IDS.has(p.id) || isDbBackedTerminalId(p.id)) { + // Default panel (terminal-default) and user-created DB-backed + // terminals (shell-) — preserve id + params so the live PTY + // and store record match on restore. Coerce presentation in case + // the saved layout predates the custom tab + plain title. + return { ...p, tabComponent: "terminalTab", title: "Terminal" }; + } + // Legacy ephemeral terminal id (e.g. `terminal-saved-1` from older + // layouts) — mint a fresh id so it reconnects to a new PTY. + const id = `terminal-saved-${++counters.terminal}`; + return { + ...p, + id, + params: { terminalId: id }, + tabComponent: "terminalTab", + title: "Terminal", + }; + } + if (KNOWN_PANEL_IDS.has(p.id)) return p; - if (p.component === "terminal") { - const id = `terminal-saved-${++counters.terminal}`; - return { ...p, id, params: { terminalId: id } }; - } if (p.component === "browser") { const url = (p.params?.url as string) ?? ""; const id = url ? `browser:${url}` : `browser-saved-${++counters.browser}`; diff --git a/apps/web/lib/state/slices/index.ts b/apps/web/lib/state/slices/index.ts index 3824a8b413..b653eef795 100644 --- a/apps/web/lib/state/slices/index.ts +++ b/apps/web/lib/state/slices/index.ts @@ -124,6 +124,9 @@ export type { AvailableCommand, AvailableCommandsState, UserShellInfo, + UserShellKind, + UserShellState, + UserShellPTYStatus, UserShellsState, PrepareStepInfo, SessionPrepareState, diff --git a/apps/web/lib/state/slices/session-runtime/session-runtime-slice.ts b/apps/web/lib/state/slices/session-runtime/session-runtime-slice.ts index aaf850239a..ea2eb8a793 100644 --- a/apps/web/lib/state/slices/session-runtime/session-runtime-slice.ts +++ b/apps/web/lib/state/slices/session-runtime/session-runtime-slice.ts @@ -239,6 +239,19 @@ function buildUserShellActions(set: ImmerSet) { (s) => s.terminalId !== terminalId, ); }), + updateUserShell: ( + environmentId: string, + terminalId: string, + patch: Parameters[2], + ) => + set((draft) => { + if (!environmentId) return; + const existing = draft.userShells.byEnvironmentId[environmentId]; + if (!existing) return; + draft.userShells.byEnvironmentId[environmentId] = existing.map((s) => + s.terminalId === terminalId ? { ...s, ...patch } : s, + ); + }), setSessionPollMode: (sessionId: string, mode: SessionPollMode) => set((draft) => { draft.sessionPollMode.bySessionId[sessionId] = mode; diff --git a/apps/web/lib/state/slices/session-runtime/types.ts b/apps/web/lib/state/slices/session-runtime/types.ts index 4dc72ce667..2aa520b55b 100644 --- a/apps/web/lib/state/slices/session-runtime/types.ts +++ b/apps/web/lib/state/slices/session-runtime/types.ts @@ -236,13 +236,39 @@ export type PromptUsageState = { bySessionId: Record; }; +/** + * User shell terminal info. Discriminated by `kind`: + * - `ordinary` — a DB-backed first-class terminal. Carries seq + custom_name + * + state. Renameable, parkable, gets a `#N` badge. + * - `fixed` — the hardcoded `bottom-panel` terminal (cmd+J). No badge, no + * rename, never parked. + * - `script` — a script-driven terminal. Lifecycle tied to the script. + * + * Legacy fields (processId, running, label, closable) are kept optional so + * old wire shapes still parse cleanly during the transition; new UI reads + * the discriminated fields below. + */ +export type UserShellKind = "ordinary" | "fixed" | "script"; +export type UserShellState = "open" | "parked"; +export type UserShellPTYStatus = "running" | "stopped"; + export type UserShellInfo = { terminalId: string; - processId: string; - running: boolean; - label: string; // Display name (e.g., "Terminal" or script name) - closable: boolean; // Whether the terminal can be closed (first terminal is not closable) - initialCommand?: string; // Command that was run (empty for plain shells) + kind?: UserShellKind; + + // Ordinary-only metadata. + seq?: number; + customName?: string | null; + displayName?: string; + state?: UserShellState; + ptyStatus?: UserShellPTYStatus; + + // Legacy / common fields. + processId?: string; + running?: boolean; + label?: string; + closable?: boolean; + initialCommand?: string; }; export type UserShellsState = { @@ -362,6 +388,14 @@ export type SessionRuntimeSliceActions = { setUserShellsLoading: (environmentId: string, loading: boolean) => void; addUserShell: (environmentId: string, shell: UserShellInfo) => void; removeUserShell: (environmentId: string, terminalId: string) => void; + updateUserShell: ( + environmentId: string, + terminalId: string, + // `terminalId` is the row key — patching it would silently break + // future lookups while leaving the array index pointing at the old + // entry. `Omit` removes it from the patch surface. + patch: Partial>, + ) => void; setSessionPollMode: (sessionId: string, mode: SessionPollMode) => void; }; diff --git a/apps/web/lib/state/slices/session-runtime/user-shells.test.ts b/apps/web/lib/state/slices/session-runtime/user-shells.test.ts new file mode 100644 index 0000000000..f2d6c92ed8 --- /dev/null +++ b/apps/web/lib/state/slices/session-runtime/user-shells.test.ts @@ -0,0 +1,73 @@ +import { describe, it, expect, beforeEach } from "vitest"; +import { create } from "zustand"; +import { immer } from "zustand/middleware/immer"; +import { createSessionRuntimeSlice } from "./session-runtime-slice"; +import type { SessionRuntimeSlice, UserShellInfo } from "./types"; + +function makeStore() { + return create()(immer(createSessionRuntimeSlice)); +} + +const ENV = "env-1"; +function shell(id: string, overrides: Partial = {}): UserShellInfo { + return { + terminalId: id, + kind: "ordinary", + seq: 1, + state: "open", + ptyStatus: "running", + customName: null, + displayName: "Terminal 1", + ...overrides, + }; +} + +describe("userShells reducers", () => { + let store: ReturnType; + + beforeEach(() => { + store = makeStore(); + }); + + it("setUserShells populates the env list", () => { + store.getState().setUserShells(ENV, [shell("a"), shell("b", { seq: 2 })]); + const list = store.getState().userShells.byEnvironmentId[ENV]; + expect(list).toHaveLength(2); + expect(list?.[0].terminalId).toBe("a"); + expect(list?.[1].seq).toBe(2); + }); + + it("addUserShell appends idempotently", () => { + store.getState().addUserShell(ENV, shell("a")); + store.getState().addUserShell(ENV, shell("a")); + expect(store.getState().userShells.byEnvironmentId[ENV]).toHaveLength(1); + }); + + it("removeUserShell removes by terminalId", () => { + store.getState().setUserShells(ENV, [shell("a"), shell("b")]); + store.getState().removeUserShell(ENV, "a"); + const list = store.getState().userShells.byEnvironmentId[ENV]; + expect(list).toHaveLength(1); + expect(list?.[0].terminalId).toBe("b"); + }); + + it("updateUserShell patches the matching row", () => { + store.getState().setUserShells(ENV, [shell("a", { customName: null })]); + store.getState().updateUserShell(ENV, "a", { customName: "build watcher" }); + const list = store.getState().userShells.byEnvironmentId[ENV]; + expect(list?.[0].customName).toBe("build watcher"); + }); + + it("updateUserShell leaves other rows untouched", () => { + store.getState().setUserShells(ENV, [shell("a"), shell("b", { state: "open" })]); + store.getState().updateUserShell(ENV, "b", { state: "parked" }); + const list = store.getState().userShells.byEnvironmentId[ENV]; + expect(list?.[0].state).toBe("open"); + expect(list?.[1].state).toBe("parked"); + }); + + it("updateUserShell is a no-op for unknown environmentId", () => { + store.getState().updateUserShell("missing-env", "a", { state: "parked" }); + expect(store.getState().userShells.byEnvironmentId["missing-env"]).toBeUndefined(); + }); +}); diff --git a/apps/web/lib/state/store.ts b/apps/web/lib/state/store.ts index 68a2bbdf4c..c0aead4649 100644 --- a/apps/web/lib/state/store.ts +++ b/apps/web/lib/state/store.ts @@ -488,6 +488,11 @@ export type AppState = { setUserShellsLoading: (sessionId: string, loading: boolean) => void; addUserShell: (sessionId: string, shell: UserShellInfo) => void; removeUserShell: (sessionId: string, terminalId: string) => void; + updateUserShell: ( + environmentId: string, + terminalId: string, + patch: Partial>, + ) => void; setSessionPollMode: (sessionId: string, mode: SessionPollMode) => void; /* prettier-ignore */ setSidebarActiveView: UIA["setSidebarActiveView"]; updateSidebarDraft: UIA["updateSidebarDraft"];