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"];