fix(quick-terminal): make host-shell PTYs durable across reloads (#2345)
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
package loginpty
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"io"
|
||||
@@ -26,12 +27,23 @@ type LoginCommandLookup interface {
|
||||
Get(id string) (agents.Agent, bool)
|
||||
}
|
||||
|
||||
// ErrNoHostShellDescriptor is the sentinel a HostShellSessionBinder returns
|
||||
// when the client_id has no durable descriptor to bind. It is not fatal: the
|
||||
// host shell PTY still starts and streams, there is simply nothing persistent
|
||||
// to reconcile against.
|
||||
var ErrNoHostShellDescriptor = errors.New("no host shell descriptor for client")
|
||||
|
||||
type HostShellSessionBinder interface {
|
||||
BindHostShellSession(ctx context.Context, tabID, sessionID string) error
|
||||
}
|
||||
|
||||
// Handlers wraps the manager + agent registry to expose HTTP/WS endpoints.
|
||||
type Handlers struct {
|
||||
mgr *Manager
|
||||
registry *registry.Registry
|
||||
logger *zap.Logger
|
||||
upgrader gorillaws.Upgrader
|
||||
mgr *Manager
|
||||
registry *registry.Registry
|
||||
logger *zap.Logger
|
||||
upgrader gorillaws.Upgrader
|
||||
hostShellBinder HostShellSessionBinder
|
||||
}
|
||||
|
||||
// NewHandlers constructs Handlers. If checkOrigin is nil, the upgrader uses
|
||||
@@ -55,6 +67,10 @@ func NewHandlers(mgr *Manager, reg *registry.Registry, log *zap.Logger, checkOri
|
||||
}
|
||||
}
|
||||
|
||||
func (h *Handlers) SetHostShellSessionBinder(binder HostShellSessionBinder) {
|
||||
h.hostShellBinder = binder
|
||||
}
|
||||
|
||||
// defaultCheckOrigin mirrors the policy used by the existing terminal
|
||||
// handler: allow no-origin requests (non-browser clients), allow loopback
|
||||
// origins (dev), otherwise require Origin host to match Request host.
|
||||
@@ -123,6 +139,7 @@ func (h *Handlers) httpStartHostShell(c *gin.Context) {
|
||||
}
|
||||
|
||||
managerKey := hostShellAgentID
|
||||
tabID := ""
|
||||
if len(req.ClientID) > 0 {
|
||||
var clientID string
|
||||
if err := json.Unmarshal(req.ClientID, &clientID); err != nil {
|
||||
@@ -134,20 +151,53 @@ func (h *Handlers) httpStartHostShell(c *gin.Context) {
|
||||
c.JSON(http.StatusBadRequest, gin.H{"error": "client_id must be a UUID"})
|
||||
return
|
||||
}
|
||||
managerKey += ":" + parsedClientID.String()
|
||||
tabID = parsedClientID.String()
|
||||
managerKey += ":" + tabID
|
||||
}
|
||||
|
||||
shell, shellArgs := detectShell()
|
||||
command := append([]string{shell}, shellArgs...)
|
||||
sess, err := h.mgr.StartWithKey(managerKey, hostShellAgentID, command, req.Cols, req.Rows)
|
||||
if err != nil && err != ErrSessionAlreadyRunning {
|
||||
reused := errors.Is(err, ErrSessionAlreadyRunning)
|
||||
if err != nil && !reused {
|
||||
h.logger.Warn("host shell start failed", zap.Error(err))
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
|
||||
return
|
||||
}
|
||||
if tabID != "" && h.hostShellBinder != nil {
|
||||
if handled := h.bindHostShellSession(c, tabID, sess, reused); handled {
|
||||
return
|
||||
}
|
||||
}
|
||||
c.JSON(http.StatusOK, sess.Status())
|
||||
}
|
||||
|
||||
// bindHostShellSession reconciles a started host-shell PTY with its durable
|
||||
// descriptor. It returns true when it has already written an error response, so
|
||||
// the caller must stop processing.
|
||||
//
|
||||
// A missing descriptor (ErrNoHostShellDescriptor) is not fatal: the PTY is a
|
||||
// generic host shell keyed by client_id and callers may attach without ever
|
||||
// persisting a Quick Terminal tab, so the session keeps running. Any other bind
|
||||
// error is a 500; a freshly created session is stopped to avoid leaking it,
|
||||
// while a reused session belongs to an existing attachment and must survive.
|
||||
func (h *Handlers) bindHostShellSession(c *gin.Context, tabID string, sess *Session, reused bool) bool {
|
||||
bindErr := h.hostShellBinder.BindHostShellSession(c.Request.Context(), tabID, sess.ID)
|
||||
if bindErr == nil {
|
||||
return false
|
||||
}
|
||||
if errors.Is(bindErr, ErrNoHostShellDescriptor) {
|
||||
h.logger.Debug("host shell started without durable descriptor", zap.String("tab_id", tabID))
|
||||
return false
|
||||
}
|
||||
if !reused {
|
||||
sess.stop()
|
||||
}
|
||||
h.logger.Warn("host shell bind failed", zap.String("tab_id", tabID), zap.Error(bindErr))
|
||||
c.JSON(http.StatusInternalServerError, gin.H{"error": bindErr.Error()})
|
||||
return true
|
||||
}
|
||||
|
||||
// detectShell picks a sensible interactive shell, preferring $SHELL on Unix
|
||||
// and PowerShell on Windows. The returned arguments are part of the shell
|
||||
// command because PowerShell needs flags to remain an interactive PTY shell.
|
||||
|
||||
@@ -2,16 +2,34 @@ package loginpty
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
"github.com/google/uuid"
|
||||
gorillaws "github.com/gorilla/websocket"
|
||||
)
|
||||
|
||||
type hostShellBinderStub struct {
|
||||
tabID string
|
||||
sessionID string
|
||||
err error
|
||||
}
|
||||
|
||||
func (s *hostShellBinderStub) BindHostShellSession(_ context.Context, tabID, sessionID string) error {
|
||||
s.tabID = tabID
|
||||
s.sessionID = sessionID
|
||||
return s.err
|
||||
}
|
||||
|
||||
func TestDetectShellWindows(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
@@ -60,14 +78,20 @@ func TestDetectShellWindows(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func newHostShellRouter(t *testing.T, mgr *Manager) *gin.Engine {
|
||||
func newHostShellRouterWithBinder(t *testing.T, mgr *Manager, binder HostShellSessionBinder) *gin.Engine {
|
||||
t.Helper()
|
||||
gin.SetMode(gin.TestMode)
|
||||
router := gin.New()
|
||||
NewHandlers(mgr, nil, mgr.log.Zap(), nil).RegisterRoutes(router)
|
||||
handlers := NewHandlers(mgr, nil, mgr.log.Zap(), nil)
|
||||
handlers.SetHostShellSessionBinder(binder)
|
||||
handlers.RegisterRoutes(router)
|
||||
return router
|
||||
}
|
||||
|
||||
func newHostShellRouter(t *testing.T, mgr *Manager) *gin.Engine {
|
||||
return newHostShellRouterWithBinder(t, mgr, nil)
|
||||
}
|
||||
|
||||
func startHostShellRequest(t *testing.T, router http.Handler, body map[string]any) (Status, int) {
|
||||
t.Helper()
|
||||
payload, err := json.Marshal(body)
|
||||
@@ -95,6 +119,39 @@ func stopSession(t *testing.T, mgr *Manager, sessionID string) {
|
||||
}
|
||||
}
|
||||
|
||||
func connectHostShellStream(t *testing.T, serverURL, sessionID string) *gorillaws.Conn {
|
||||
t.Helper()
|
||||
wsURL := "ws" + strings.TrimPrefix(serverURL, "http") + fmt.Sprintf("/api/v1/agent-login/sessions/%s/stream", sessionID)
|
||||
conn, _, err := gorillaws.DefaultDialer.Dial(wsURL, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("dial stream: %v", err)
|
||||
}
|
||||
return conn
|
||||
}
|
||||
|
||||
func waitForHostShellOutput(t *testing.T, conn *gorillaws.Conn, needle string) {
|
||||
t.Helper()
|
||||
deadline := time.Now().Add(15 * time.Second)
|
||||
var output strings.Builder
|
||||
for time.Now().Before(deadline) {
|
||||
if err := conn.SetReadDeadline(time.Now().Add(3 * time.Second)); err != nil {
|
||||
t.Fatalf("set read deadline: %v", err)
|
||||
}
|
||||
messageType, data, err := conn.ReadMessage()
|
||||
if err != nil {
|
||||
t.Fatalf("read message: %v; output so far: %q", err, output.String())
|
||||
}
|
||||
if messageType != gorillaws.BinaryMessage {
|
||||
continue
|
||||
}
|
||||
output.Write(data)
|
||||
if strings.Contains(output.String(), needle) {
|
||||
return
|
||||
}
|
||||
}
|
||||
t.Fatalf("timed out waiting for %q in %q", needle, output.String())
|
||||
}
|
||||
|
||||
func TestHostShellClientIDIsIdempotentAndIndependent(t *testing.T) {
|
||||
mgr := newTestManager(t, nil)
|
||||
router := newHostShellRouter(t, mgr)
|
||||
@@ -189,3 +246,94 @@ func TestHostShellRejectsMalformedJSON(t *testing.T) {
|
||||
t.Fatal("malformed body started a host shell session")
|
||||
}
|
||||
}
|
||||
|
||||
func TestHostShellClientIDBindsSessionBeforeResponding(t *testing.T) {
|
||||
mgr := newTestManager(t, nil)
|
||||
binder := &hostShellBinderStub{}
|
||||
router := newHostShellRouterWithBinder(t, mgr, binder)
|
||||
tabID := uuid.NewString()
|
||||
|
||||
status, code := startHostShellRequest(t, router, map[string]any{"client_id": tabID})
|
||||
if code != http.StatusOK {
|
||||
t.Fatalf("start status = %d", code)
|
||||
}
|
||||
t.Cleanup(func() { stopSession(t, mgr, status.ID) })
|
||||
if binder.tabID != tabID {
|
||||
t.Fatalf("binder tab id = %q, want %q", binder.tabID, tabID)
|
||||
}
|
||||
if binder.sessionID != status.ID {
|
||||
t.Fatalf("binder session id = %q, want %q", binder.sessionID, status.ID)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHostShellStartsWhenNoDescriptorToBind(t *testing.T) {
|
||||
mgr := newTestManager(t, nil)
|
||||
binder := &hostShellBinderStub{err: ErrNoHostShellDescriptor}
|
||||
router := newHostShellRouterWithBinder(t, mgr, binder)
|
||||
tabID := uuid.NewString()
|
||||
|
||||
status, code := startHostShellRequest(t, router, map[string]any{"client_id": tabID})
|
||||
if code != http.StatusOK {
|
||||
t.Fatalf("start status = %d, want %d (missing descriptor must not fail start)", code, http.StatusOK)
|
||||
}
|
||||
t.Cleanup(func() { stopSession(t, mgr, status.ID) })
|
||||
if got := mgr.GetByID(status.ID); got == nil || !got.Status().Running {
|
||||
t.Fatal("host shell session was stopped despite a benign missing-descriptor bind")
|
||||
}
|
||||
}
|
||||
|
||||
func TestHostShellReusedSessionSurvivesBindError(t *testing.T) {
|
||||
mgr := newTestManager(t, nil)
|
||||
binder := &hostShellBinderStub{}
|
||||
router := newHostShellRouterWithBinder(t, mgr, binder)
|
||||
tabID := uuid.NewString()
|
||||
|
||||
first, code := startHostShellRequest(t, router, map[string]any{"client_id": tabID})
|
||||
if code != http.StatusOK {
|
||||
t.Fatalf("first start status = %d", code)
|
||||
}
|
||||
t.Cleanup(func() { stopSession(t, mgr, first.ID) })
|
||||
|
||||
// A subsequent start for the same client reuses the session. A hard bind
|
||||
// error must surface as 500 but must NOT stop the reused (still-attached)
|
||||
// session.
|
||||
binder.err = errors.New("descriptor write failed")
|
||||
_, code = startHostShellRequest(t, router, map[string]any{"client_id": tabID})
|
||||
if code != http.StatusInternalServerError {
|
||||
t.Fatalf("reused start with bind error status = %d, want %d", code, http.StatusInternalServerError)
|
||||
}
|
||||
if got := mgr.GetByID(first.ID); got == nil || !got.Status().Running {
|
||||
t.Fatal("reused host shell session was stopped after a bind error")
|
||||
}
|
||||
}
|
||||
|
||||
func TestHostShellSessionSurvivesWebSocketReconnect(t *testing.T) {
|
||||
mgr := newTestManager(t, nil)
|
||||
router := newHostShellRouter(t, mgr)
|
||||
server := httptest.NewServer(router)
|
||||
defer server.Close()
|
||||
|
||||
clientID := uuid.NewString()
|
||||
status, code := startHostShellRequest(t, router, map[string]any{"client_id": clientID})
|
||||
if code != http.StatusOK {
|
||||
t.Fatalf("start status = %d", code)
|
||||
}
|
||||
t.Cleanup(func() { stopSession(t, mgr, status.ID) })
|
||||
|
||||
firstConn := connectHostShellStream(t, server.URL, status.ID)
|
||||
// Don't wait for a shell prompt: it varies by shell and by user (zsh "%",
|
||||
// bash "$", root "#"), which made this hang under CI's root bash. The
|
||||
// command's own echoed marker is a deterministic readiness signal instead.
|
||||
if err := firstConn.WriteMessage(gorillaws.BinaryMessage, []byte("export KANDEV_QT_ONE=QUICK_TERMINAL_ONE && echo $KANDEV_QT_ONE\n")); err != nil {
|
||||
t.Fatalf("write export command: %v", err)
|
||||
}
|
||||
waitForHostShellOutput(t, firstConn, "QUICK_TERMINAL_ONE")
|
||||
_ = firstConn.Close()
|
||||
|
||||
secondConn := connectHostShellStream(t, server.URL, status.ID)
|
||||
defer func() { _ = secondConn.Close() }()
|
||||
if err := secondConn.WriteMessage(gorillaws.BinaryMessage, []byte("echo $KANDEV_QT_ONE\n")); err != nil {
|
||||
t.Fatalf("write echo command: %v", err)
|
||||
}
|
||||
waitForHostShellOutput(t, secondConn, "QUICK_TERMINAL_ONE")
|
||||
}
|
||||
|
||||
@@ -50,6 +50,7 @@ type Manager struct {
|
||||
sessions map[string]*Session // key: internal uniqueness key
|
||||
byID map[string]*Session // key: sessionID
|
||||
log *logger.Logger
|
||||
timeouts sessionTimeouts
|
||||
|
||||
// onExit is invoked from a goroutine when a session terminates (with the
|
||||
// agent ID and exit code). Used by callers to invalidate discovery cache
|
||||
@@ -69,6 +70,7 @@ func NewManager(log *logger.Logger, onExit func(agentID string, exitCode int, ex
|
||||
byID: map[string]*Session{},
|
||||
log: log.WithFields(zap.String("component", "login-pty")),
|
||||
onExit: onExit,
|
||||
timeouts: defaultSessionTimeouts(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -111,10 +113,12 @@ func (m *Manager) StartWithKey(managerKey, agentID string, cmd []string, cols, r
|
||||
ID: uuid.NewString(),
|
||||
AgentID: agentID,
|
||||
managerKey: managerKey,
|
||||
lifetime: lifetimeForAgent(agentID),
|
||||
Cmd: append([]string(nil), cmd...),
|
||||
subscribers: map[chan<- []byte]struct{}{},
|
||||
log: m.log.WithFields(zap.String("agent", agentID)),
|
||||
startedAt: time.Now(),
|
||||
done: make(chan struct{}),
|
||||
}
|
||||
|
||||
if err := sess.start(cmd, cols, rows); err != nil {
|
||||
@@ -160,10 +164,29 @@ func (m *Manager) StopSession(sessionID string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// StopAll terminates every currently live session and waits for manager cleanup.
|
||||
func (m *Manager) StopAll() error {
|
||||
m.mu.Lock()
|
||||
sessions := make([]*Session, 0, len(m.byID))
|
||||
for _, sess := range m.byID {
|
||||
sessions = append(sessions, sess)
|
||||
}
|
||||
m.mu.Unlock()
|
||||
|
||||
for _, sess := range sessions {
|
||||
sess.stop()
|
||||
}
|
||||
for _, sess := range sessions {
|
||||
sess.waitDone()
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// supervise watches the session's lifetime: waits on the process, enforces
|
||||
// timeouts, then deletes the session from the manager and notifies onExit.
|
||||
func (m *Manager) supervise(sess *Session) {
|
||||
ctx, cancel := context.WithTimeout(context.Background(), HardTimeout)
|
||||
defer close(sess.done)
|
||||
ctx, cancel := m.hardTimeoutContext(sess)
|
||||
defer cancel()
|
||||
|
||||
exited := make(chan exitInfo, 1)
|
||||
@@ -175,8 +198,10 @@ func (m *Manager) supervise(sess *Session) {
|
||||
}
|
||||
}()
|
||||
|
||||
idle := time.NewTimer(IdleTimeout)
|
||||
defer idle.Stop()
|
||||
idle, idleCh := m.newIdleTimer(sess)
|
||||
if idle != nil {
|
||||
defer idle.Stop()
|
||||
}
|
||||
|
||||
var info exitInfo
|
||||
loop:
|
||||
@@ -204,13 +229,16 @@ loop:
|
||||
// it also unblocks a stuck Windows ConPTY Read.
|
||||
sess.stop()
|
||||
break loop
|
||||
case <-idle.C:
|
||||
case <-idleCh:
|
||||
sess.log.Info("login session idle timeout — terminating")
|
||||
sess.stop()
|
||||
case <-ctx.Done():
|
||||
sess.log.Info("login session hard timeout — terminating")
|
||||
sess.stop()
|
||||
case <-sess.activityCh:
|
||||
if idle == nil {
|
||||
continue
|
||||
}
|
||||
// Reset idle timer on output activity.
|
||||
if !idle.Stop() {
|
||||
select {
|
||||
@@ -218,7 +246,7 @@ loop:
|
||||
default:
|
||||
}
|
||||
}
|
||||
idle.Reset(IdleTimeout)
|
||||
idle.Reset(m.timeouts.idle)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -278,6 +306,44 @@ type exitInfo struct {
|
||||
err error
|
||||
}
|
||||
|
||||
type sessionLifetime int
|
||||
|
||||
const (
|
||||
sessionLifetimeBounded sessionLifetime = iota
|
||||
sessionLifetimeUnbounded
|
||||
)
|
||||
|
||||
type sessionTimeouts struct {
|
||||
idle time.Duration
|
||||
hard time.Duration
|
||||
}
|
||||
|
||||
func defaultSessionTimeouts() sessionTimeouts {
|
||||
return sessionTimeouts{idle: IdleTimeout, hard: HardTimeout}
|
||||
}
|
||||
|
||||
func lifetimeForAgent(agentID string) sessionLifetime {
|
||||
if agentID == HostShellAgentID {
|
||||
return sessionLifetimeUnbounded
|
||||
}
|
||||
return sessionLifetimeBounded
|
||||
}
|
||||
|
||||
func (m *Manager) hardTimeoutContext(sess *Session) (context.Context, context.CancelFunc) {
|
||||
if sess.lifetime == sessionLifetimeUnbounded {
|
||||
return context.Background(), func() {}
|
||||
}
|
||||
return context.WithTimeout(context.Background(), m.timeouts.hard)
|
||||
}
|
||||
|
||||
func (m *Manager) newIdleTimer(sess *Session) (*time.Timer, <-chan time.Time) {
|
||||
if sess.lifetime == sessionLifetimeUnbounded {
|
||||
return nil, nil
|
||||
}
|
||||
timer := time.NewTimer(m.timeouts.idle)
|
||||
return timer, timer.C
|
||||
}
|
||||
|
||||
func exitCodeFromError(err error) int {
|
||||
if err == nil {
|
||||
return 0
|
||||
@@ -297,6 +363,7 @@ type Session struct {
|
||||
Cmd []string
|
||||
|
||||
managerKey string
|
||||
lifetime sessionLifetime
|
||||
mu sync.Mutex
|
||||
cmd *exec.Cmd
|
||||
pty ptyHandle
|
||||
@@ -314,10 +381,18 @@ type Session struct {
|
||||
|
||||
activityCh chan struct{} // notifications to supervise loop on output
|
||||
readDone chan struct{} // closed when readLoop exits (all PTY output drained)
|
||||
done chan struct{} // closed when supervise finishes cleanup
|
||||
|
||||
log *logger.Logger
|
||||
}
|
||||
|
||||
func (s *Session) waitDone() {
|
||||
if s.done == nil {
|
||||
return
|
||||
}
|
||||
<-s.done
|
||||
}
|
||||
|
||||
// ManagerKey returns the internal uniqueness key used by Manager. It is
|
||||
// intentionally separate from AgentID so keyed host shells can prove their
|
||||
// descriptor owns the session without exposing the key as an agent identity.
|
||||
|
||||
@@ -19,6 +19,18 @@ func newTestManager(t *testing.T, onExit func(string, int, error)) *Manager {
|
||||
return NewManager(log, onExit)
|
||||
}
|
||||
|
||||
func waitForSessionRemoved(t *testing.T, mgr *Manager, sessionID string) {
|
||||
t.Helper()
|
||||
deadline := time.After(3 * time.Second)
|
||||
for mgr.GetByID(sessionID) != nil {
|
||||
select {
|
||||
case <-deadline:
|
||||
t.Fatalf("session %s was not removed from manager", sessionID)
|
||||
case <-time.After(20 * time.Millisecond):
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// drainOutput consumes the subscriber channel until it closes; returns all bytes received.
|
||||
func drainOutput(sess *Session) []byte {
|
||||
ch := make(chan []byte, 32)
|
||||
@@ -132,14 +144,7 @@ func TestStartWithKey_SeparatesUniquenessFromPublicAgentID(t *testing.T) {
|
||||
}
|
||||
|
||||
first.stop()
|
||||
deadline := time.After(3 * time.Second)
|
||||
for mgr.GetByID(first.ID) != nil {
|
||||
select {
|
||||
case <-deadline:
|
||||
t.Fatal("first session was not removed from manager")
|
||||
case <-time.After(20 * time.Millisecond):
|
||||
}
|
||||
}
|
||||
waitForSessionRemoved(t, mgr, first.ID)
|
||||
if sibling := mgr.GetByID(second.ID); sibling == nil || !sibling.Status().Running {
|
||||
t.Fatal("cleaning up one internal key removed or stopped its sibling")
|
||||
}
|
||||
@@ -200,3 +205,90 @@ func TestWriteAndResize_WhenNotRunning(t *testing.T) {
|
||||
t.Errorf("Resize err = %v, want ErrSessionNotRunning", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestStartWithKey_HostShellSessionsDoNotTimeOut(t *testing.T) {
|
||||
mgr := newTestManager(t, nil)
|
||||
mgr.timeouts = sessionTimeouts{idle: 40 * time.Millisecond, hard: 80 * time.Millisecond}
|
||||
|
||||
sess, err := mgr.StartWithKey("_host_shell:tab-a", HostShellAgentID, []string{"sh", "-c", "sleep 1"}, 80, 24)
|
||||
if err != nil {
|
||||
t.Fatalf("StartWithKey: %v", err)
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
_ = mgr.StopSession(sess.ID)
|
||||
waitForSessionRemoved(t, mgr, sess.ID)
|
||||
})
|
||||
|
||||
time.Sleep(3 * mgr.timeouts.hard)
|
||||
if !sess.Status().Running {
|
||||
t.Fatal("host-shell session stopped despite no-timeout policy")
|
||||
}
|
||||
if mgr.GetByID(sess.ID) == nil {
|
||||
t.Fatal("host-shell session disappeared from manager")
|
||||
}
|
||||
}
|
||||
|
||||
func TestStart_AgentLoginSessionsStillIdleTimeout(t *testing.T) {
|
||||
exited := make(chan struct{}, 1)
|
||||
mgr := newTestManager(t, func(string, int, error) {
|
||||
exited <- struct{}{}
|
||||
})
|
||||
mgr.timeouts = sessionTimeouts{idle: 40 * time.Millisecond, hard: time.Second}
|
||||
|
||||
sess, err := mgr.Start("test-agent", []string{"sh", "-c", "sleep 1"}, 80, 24)
|
||||
if err != nil {
|
||||
t.Fatalf("Start: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = mgr.StopAll() })
|
||||
|
||||
select {
|
||||
case <-exited:
|
||||
case <-time.After(3 * time.Second):
|
||||
t.Fatal("agent login session did not idle-timeout")
|
||||
}
|
||||
waitForSessionRemoved(t, mgr, sess.ID)
|
||||
}
|
||||
|
||||
func TestStart_AgentLoginSessionsStillHardTimeout(t *testing.T) {
|
||||
exited := make(chan struct{}, 1)
|
||||
mgr := newTestManager(t, func(string, int, error) {
|
||||
exited <- struct{}{}
|
||||
})
|
||||
mgr.timeouts = sessionTimeouts{idle: 250 * time.Millisecond, hard: 80 * time.Millisecond}
|
||||
|
||||
sess, err := mgr.Start("test-agent", []string{"sh", "-c", "yes x"}, 80, 24)
|
||||
if err != nil {
|
||||
t.Fatalf("Start: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = mgr.StopAll() })
|
||||
|
||||
select {
|
||||
case <-exited:
|
||||
case <-time.After(3 * time.Second):
|
||||
t.Fatal("agent login session did not hard-timeout")
|
||||
}
|
||||
waitForSessionRemoved(t, mgr, sess.ID)
|
||||
}
|
||||
|
||||
func TestStopAll_StopsEverySessionAndIsIdempotent(t *testing.T) {
|
||||
mgr := newTestManager(t, nil)
|
||||
t.Cleanup(func() { _ = mgr.StopAll() })
|
||||
first, err := mgr.Start("test-agent", []string{"sh", "-c", "sleep 1"}, 80, 24)
|
||||
if err != nil {
|
||||
t.Fatalf("first start: %v", err)
|
||||
}
|
||||
second, err := mgr.StartWithKey("_host_shell:tab-a", HostShellAgentID, []string{"sh", "-c", "sleep 1"}, 80, 24)
|
||||
if err != nil {
|
||||
t.Fatalf("second start: %v", err)
|
||||
}
|
||||
|
||||
if err := mgr.StopAll(); err != nil {
|
||||
t.Fatalf("StopAll: %v", err)
|
||||
}
|
||||
if mgr.GetByID(first.ID) != nil || mgr.GetByID(second.ID) != nil {
|
||||
t.Fatal("StopAll returned before sessions were removed")
|
||||
}
|
||||
if err := mgr.StopAll(); err != nil {
|
||||
t.Fatalf("second StopAll: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1052,7 +1052,14 @@ func registerSecondaryRoutes(
|
||||
// Login PTY: spawns agent login commands under a PTY on the kandev host
|
||||
// (claude auth login, auggie login, ...). The manager is shared with Quick
|
||||
// Terminal so descriptor lifecycle callbacks observe the same sessions.
|
||||
loginpty.NewHandlers(p.loginMgr, p.agentRegistry, p.log.Zap(), nil).RegisterRoutes(p.router)
|
||||
loginHandlers := loginpty.NewHandlers(p.loginMgr, p.agentRegistry, p.log.Zap(), nil)
|
||||
if p.quickTerminalSvc != nil {
|
||||
// Guard the assignment: passing a nil *quickterminal.Service straight
|
||||
// into the interface would create a typed-nil binder that is non-nil to
|
||||
// the handler's nil check but panics when invoked.
|
||||
loginHandlers.SetHostShellSessionBinder(p.quickTerminalSvc)
|
||||
}
|
||||
loginHandlers.RegisterRoutes(p.router)
|
||||
if p.quickTerminalSvc != nil {
|
||||
p.quickTerminalSvc.RegisterRoutes(p.router)
|
||||
}
|
||||
|
||||
@@ -9,6 +9,7 @@ import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/kandev/kandev/internal/agent/loginpty"
|
||||
"github.com/kandev/kandev/internal/common/config"
|
||||
"github.com/kandev/kandev/internal/common/logger"
|
||||
"go.uber.org/goleak"
|
||||
@@ -30,6 +31,31 @@ func TestBuildHTTPServerAbortsWhenInterlockTokenGenerationFails(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildLoginPTYServicesRegistersStopAllCleanup(t *testing.T) {
|
||||
var cleanups []func() error
|
||||
loginMgr, _ := buildLoginPTYServices(testLogger(t), nil, nil, nil, func(fn func() error) {
|
||||
cleanups = append(cleanups, fn)
|
||||
})
|
||||
if len(cleanups) != 1 {
|
||||
t.Fatalf("cleanup count = %d, want 1", len(cleanups))
|
||||
}
|
||||
|
||||
sess, err := loginMgr.StartWithKey("_host_shell:tab-a", loginpty.HostShellAgentID, []string{"sh", "-c", "sleep 1"}, 80, 24)
|
||||
if err != nil {
|
||||
t.Fatalf("StartWithKey: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = loginMgr.StopAll() })
|
||||
if err := cleanups[0](); err != nil {
|
||||
t.Fatalf("cleanup: %v", err)
|
||||
}
|
||||
if loginMgr.GetByID(sess.ID) != nil {
|
||||
t.Fatal("cleanup returned before login session stopped")
|
||||
}
|
||||
if err := cleanups[0](); err != nil {
|
||||
t.Fatalf("second cleanup: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func testLogger(t *testing.T) *logger.Logger {
|
||||
t.Helper()
|
||||
log, err := logger.NewFromZap(zap.NewNop())
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
package backendapp
|
||||
|
||||
import (
|
||||
"github.com/kandev/kandev/internal/agent/loginpty"
|
||||
agentsettingscontroller "github.com/kandev/kandev/internal/agent/settings/controller"
|
||||
"github.com/kandev/kandev/internal/common/logger"
|
||||
"github.com/kandev/kandev/internal/quickterminal"
|
||||
quickterminalrepo "github.com/kandev/kandev/internal/quickterminal/repository"
|
||||
)
|
||||
|
||||
func buildLoginPTYServices(
|
||||
log *logger.Logger,
|
||||
quickTerminalRepo *quickterminalrepo.Repository,
|
||||
taskAuthorizer quickterminal.WorkspaceAuthorizer,
|
||||
agentSettingsController *agentsettingscontroller.Controller,
|
||||
addCleanup func(func() error),
|
||||
) (*loginpty.Manager, *quickterminal.Service) {
|
||||
loginMgr := loginpty.NewManager(log, func(_ string, _ int, _ error) {
|
||||
if agentSettingsController != nil {
|
||||
agentSettingsController.InvalidateDiscoveryCache()
|
||||
}
|
||||
})
|
||||
if addCleanup != nil {
|
||||
addCleanup(loginMgr.StopAll)
|
||||
}
|
||||
quickTerminalSvc := quickterminal.NewService(quickTerminalRepo, loginMgr, taskAuthorizer)
|
||||
loginMgr.SetSessionExitCallback(quickTerminalSvc.HandleSessionExit)
|
||||
return loginMgr, quickTerminalSvc
|
||||
}
|
||||
@@ -45,7 +45,6 @@ import (
|
||||
|
||||
// Agent infrastructure
|
||||
"github.com/kandev/kandev/internal/agent/hostutility"
|
||||
"github.com/kandev/kandev/internal/agent/loginpty"
|
||||
"github.com/kandev/kandev/internal/agent/mcpconfig"
|
||||
"github.com/kandev/kandev/internal/agent/registry"
|
||||
agentctlclient "github.com/kandev/kandev/internal/agent/runtime/agentctl"
|
||||
@@ -110,7 +109,6 @@ import (
|
||||
workflowservice "github.com/kandev/kandev/internal/workflow/service"
|
||||
|
||||
// Repository cloning
|
||||
"github.com/kandev/kandev/internal/quickterminal"
|
||||
"github.com/kandev/kandev/internal/repoclone"
|
||||
"github.com/kandev/kandev/internal/runtimeflags"
|
||||
|
||||
@@ -1811,13 +1809,13 @@ func buildHTTPServer(
|
||||
// The login PTY manager is shared by agent-login dialogs and Quick
|
||||
// Terminal tabs. The latter uses an owner-aware exit callback to keep its
|
||||
// durable descriptor accurate even while no browser is attached.
|
||||
loginMgr := loginpty.NewManager(log, func(_ string, _ int, _ error) {
|
||||
if agentSettingsController != nil {
|
||||
agentSettingsController.InvalidateDiscoveryCache()
|
||||
}
|
||||
})
|
||||
quickTerminalSvc := quickterminal.NewService(repos.QuickTerminal, loginMgr, services.Task)
|
||||
loginMgr.SetSessionExitCallback(quickTerminalSvc.HandleSessionExit)
|
||||
loginMgr, quickTerminalSvc := buildLoginPTYServices(
|
||||
log,
|
||||
repos.QuickTerminal,
|
||||
services.Task,
|
||||
agentSettingsController,
|
||||
addCleanup,
|
||||
)
|
||||
|
||||
// Opt-in authentication. Runs after CORS; in disabled mode it only
|
||||
// injects the synthetic single-user identity (behavior unchanged).
|
||||
|
||||
@@ -163,6 +163,17 @@ func (s *Service) HandleSessionExit(managerKey, sessionID, _ string, exitCode in
|
||||
}
|
||||
}
|
||||
|
||||
func (s *Service) BindHostShellSession(ctx context.Context, tabID, sessionID string) error {
|
||||
_, err := s.UpdateLifecycle(ctx, tabID, sessionID, models.StatusRunning, "", nil)
|
||||
if errors.Is(err, repository.ErrNotFound) {
|
||||
// The host shell was started for a client_id that has no persisted
|
||||
// Quick Terminal tab. Signal the caller to keep the PTY running rather
|
||||
// than treating the missing descriptor as a start failure.
|
||||
return loginpty.ErrNoHostShellDescriptor
|
||||
}
|
||||
return err
|
||||
}
|
||||
|
||||
func (s *Service) reconcileTab(ctx context.Context, tab *models.Tab) error {
|
||||
if tab == nil || tab.SessionID == nil {
|
||||
if tab != nil && tab.Status == models.StatusConnecting {
|
||||
@@ -177,7 +188,11 @@ func (s *Service) reconcileTab(ctx context.Context, tab *models.Tab) error {
|
||||
if !s.ownsSession(tab.TabID, *tab.SessionID) {
|
||||
return s.markUnavailable(ctx, tab)
|
||||
}
|
||||
status := s.manager.GetByID(*tab.SessionID).Status()
|
||||
session := s.manager.GetByID(*tab.SessionID)
|
||||
if session == nil {
|
||||
return s.markUnavailable(ctx, tab)
|
||||
}
|
||||
status := session.Status()
|
||||
if !status.Running {
|
||||
return s.markUnavailable(ctx, tab)
|
||||
}
|
||||
|
||||
@@ -8,6 +8,9 @@ import (
|
||||
"github.com/jmoiron/sqlx"
|
||||
_ "github.com/mattn/go-sqlite3"
|
||||
|
||||
"github.com/kandev/kandev/internal/agent/loginpty"
|
||||
"github.com/kandev/kandev/internal/common/logger"
|
||||
"github.com/kandev/kandev/internal/quickterminal/models"
|
||||
"github.com/kandev/kandev/internal/quickterminal/repository"
|
||||
"github.com/kandev/kandev/internal/user/store"
|
||||
)
|
||||
@@ -28,6 +31,15 @@ func serviceRepository(t *testing.T) *repository.Repository {
|
||||
return repo
|
||||
}
|
||||
|
||||
func serviceManager(t *testing.T) *loginpty.Manager {
|
||||
t.Helper()
|
||||
log, err := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json"})
|
||||
if err != nil {
|
||||
t.Fatalf("logger: %v", err)
|
||||
}
|
||||
return loginpty.NewManager(log, nil)
|
||||
}
|
||||
|
||||
func TestListMarksAConnectingDescriptorWithoutPTYAsUnavailable(t *testing.T) {
|
||||
repo := serviceRepository(t)
|
||||
ctx := context.Background()
|
||||
@@ -72,3 +84,99 @@ func TestCreateRejectsReusingATabIDInAnotherWorkspace(t *testing.T) {
|
||||
t.Fatalf("cross-workspace reuse error = %v, want %v", err, repository.ErrTabIDConflict)
|
||||
}
|
||||
}
|
||||
|
||||
func TestListKeepsARunningQuickTerminalSessionAttached(t *testing.T) {
|
||||
repo := serviceRepository(t)
|
||||
mgr := serviceManager(t)
|
||||
ctx := context.Background()
|
||||
tabID := "11111111-1111-4111-8111-111111111111"
|
||||
|
||||
tab, err := repo.Create(ctx, store.DefaultUserID, "workspace-1", tabID)
|
||||
if err != nil {
|
||||
t.Fatalf("create: %v", err)
|
||||
}
|
||||
sess, err := mgr.StartWithKey(loginpty.HostShellAgentID+":"+tabID, loginpty.HostShellAgentID, []string{"sh", "-c", "sleep 1"}, 80, 24)
|
||||
if err != nil {
|
||||
t.Fatalf("start session: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = mgr.StopAll() })
|
||||
if err := repo.UpdateLifecycle(ctx, tab.UserID, tab.TabID, sess.ID, models.StatusRunning, nil, ""); err != nil {
|
||||
t.Fatalf("UpdateLifecycle: %v", err)
|
||||
}
|
||||
|
||||
svc := NewService(repo, mgr, nil)
|
||||
tabs, err := svc.List(ctx, "workspace-1")
|
||||
if err != nil {
|
||||
t.Fatalf("list: %v", err)
|
||||
}
|
||||
if len(tabs) != 1 {
|
||||
t.Fatalf("tabs count = %d, want 1", len(tabs))
|
||||
}
|
||||
if tabs[0].SessionID == nil || *tabs[0].SessionID != sess.ID {
|
||||
t.Fatalf("session id = %v, want %s", tabs[0].SessionID, sess.ID)
|
||||
}
|
||||
if tabs[0].Status != models.StatusRunning {
|
||||
t.Fatalf("status = %q, want %q", tabs[0].Status, models.StatusRunning)
|
||||
}
|
||||
}
|
||||
|
||||
func TestListMarksMissingQuickTerminalSessionUnavailable(t *testing.T) {
|
||||
repo := serviceRepository(t)
|
||||
ctx := context.Background()
|
||||
tabID := "11111111-1111-4111-8111-111111111111"
|
||||
|
||||
tab, err := repo.Create(ctx, store.DefaultUserID, "workspace-1", tabID)
|
||||
if err != nil {
|
||||
t.Fatalf("create: %v", err)
|
||||
}
|
||||
if err := repo.UpdateLifecycle(ctx, tab.UserID, tab.TabID, "missing-session", models.StatusRunning, nil, ""); err != nil {
|
||||
t.Fatalf("UpdateLifecycle: %v", err)
|
||||
}
|
||||
|
||||
svc := NewService(repo, serviceManager(t), nil)
|
||||
tabs, err := svc.List(ctx, "workspace-1")
|
||||
if err != nil {
|
||||
t.Fatalf("list: %v", err)
|
||||
}
|
||||
if len(tabs) != 1 {
|
||||
t.Fatalf("tabs count = %d, want 1", len(tabs))
|
||||
}
|
||||
if tabs[0].SessionID != nil {
|
||||
t.Fatalf("session id = %v, want nil", tabs[0].SessionID)
|
||||
}
|
||||
if tabs[0].Status != models.StatusExited || tabs[0].Error == "" {
|
||||
t.Fatalf("tab = %#v, want exited/unavailable", tabs[0])
|
||||
}
|
||||
}
|
||||
|
||||
func TestBindHostShellSessionPersistsRunningState(t *testing.T) {
|
||||
repo := serviceRepository(t)
|
||||
mgr := serviceManager(t)
|
||||
ctx := context.Background()
|
||||
tabID := "11111111-1111-4111-8111-111111111111"
|
||||
|
||||
tab, err := repo.Create(ctx, store.DefaultUserID, "workspace-1", tabID)
|
||||
if err != nil {
|
||||
t.Fatalf("create: %v", err)
|
||||
}
|
||||
sess, err := mgr.StartWithKey(loginpty.HostShellAgentID+":"+tabID, loginpty.HostShellAgentID, []string{"sh", "-c", "sleep 1"}, 80, 24)
|
||||
if err != nil {
|
||||
t.Fatalf("start session: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = mgr.StopAll() })
|
||||
|
||||
svc := NewService(repo, mgr, nil)
|
||||
if err := svc.BindHostShellSession(ctx, tab.TabID, sess.ID); err != nil {
|
||||
t.Fatalf("BindHostShellSession: %v", err)
|
||||
}
|
||||
stored, err := repo.Get(ctx, tab.UserID, tab.TabID)
|
||||
if err != nil {
|
||||
t.Fatalf("get: %v", err)
|
||||
}
|
||||
if stored.SessionID == nil || *stored.SessionID != sess.ID {
|
||||
t.Fatalf("session id = %v, want %s", stored.SessionID, sess.ID)
|
||||
}
|
||||
if stored.Status != models.StatusRunning {
|
||||
t.Fatalf("status = %q, want %q", stored.Status, models.StatusRunning)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -57,7 +57,7 @@ function AppSidebarNavigation({ collapsed, inOffice, settingsMode }: AppSidebarN
|
||||
<>
|
||||
<div
|
||||
className={cn(
|
||||
"flex flex-col gap-2 overflow-y-auto",
|
||||
"flex flex-col gap-1 overflow-y-auto",
|
||||
inOffice ? "flex-1 min-h-0 pb-8 scroll-pb-8" : "shrink-0",
|
||||
)}
|
||||
data-testid="app-sidebar-scroll"
|
||||
|
||||
@@ -5,7 +5,11 @@ import { QuickTabAddMenu } from "./quick-tab-add-menu";
|
||||
vi.mock("@kandev/ui/dropdown-menu", () => ({
|
||||
DropdownMenu: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||
DropdownMenuTrigger: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||
DropdownMenuContent: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||
DropdownMenuContent: ({ children, align }: { children: React.ReactNode; align?: string }) => (
|
||||
<div data-testid="quick-chat-add-menu-content" data-align={align}>
|
||||
{children}
|
||||
</div>
|
||||
),
|
||||
DropdownMenuLabel: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
|
||||
DropdownMenuSeparator: () => <hr />,
|
||||
DropdownMenuItem: ({
|
||||
@@ -50,6 +54,9 @@ describe("QuickTabAddMenu", () => {
|
||||
fireEvent.click(screen.getByTestId("quick-chat-add-menu-trigger"));
|
||||
|
||||
expect(await screen.findByText("Agents")).toBeTruthy();
|
||||
expect(screen.getByTestId("quick-chat-add-menu-content").getAttribute("data-align")).toBe(
|
||||
"start",
|
||||
);
|
||||
expect(screen.getByText("New Agent")).toBeTruthy();
|
||||
expect(screen.getByText("Terminals")).toBeTruthy();
|
||||
expect(screen.getByText("New Terminal")).toBeTruthy();
|
||||
|
||||
@@ -35,7 +35,7 @@ export function QuickTabAddMenu({ onNewAgent, onNewTerminal }: QuickTabAddMenuPr
|
||||
<IconPlus className="h-4 w-4 sm:h-3.5 sm:w-3.5" aria-hidden />
|
||||
</Button>
|
||||
</DropdownMenuTrigger>
|
||||
<DropdownMenuContent align="end" className="w-64">
|
||||
<DropdownMenuContent align="start" className="w-64" data-testid="quick-chat-add-menu-content">
|
||||
<DropdownMenuLabel className="text-xs text-muted-foreground">
|
||||
{t("common:agents")}
|
||||
</DropdownMenuLabel>
|
||||
|
||||
@@ -73,25 +73,63 @@ const tab = {
|
||||
};
|
||||
|
||||
describe("QuickTerminalTabView", () => {
|
||||
it("starts a host shell with the terminal tab's stable client id and detaches on unmount", () => {
|
||||
it("starts a host shell with the terminal tab's stable client id and detaches on unmount", async () => {
|
||||
const onStateChange = vi.fn();
|
||||
const view = render(<QuickTerminalTabView tab={tab} onStateChange={onStateChange} />);
|
||||
|
||||
return waitFor(() =>
|
||||
await waitFor(() =>
|
||||
expect(screen.getByTestId("pty-view-probe").getAttribute("data-lifecycle")).toBe(
|
||||
"detach-on-unmount",
|
||||
),
|
||||
).then(() => {
|
||||
fireEvent.click(screen.getByTestId("pty-view-probe"));
|
||||
expect(startHostShell).toHaveBeenCalledWith({ cols: 80, rows: 24 }, { clientId: tab.tabId });
|
||||
view.unmount();
|
||||
);
|
||||
|
||||
fireEvent.click(screen.getByTestId("pty-view-probe"));
|
||||
expect(startHostShell).toHaveBeenCalledWith({ cols: 80, rows: 24 }, { clientId: tab.tabId });
|
||||
await waitFor(() =>
|
||||
expect(onStateChange).toHaveBeenCalledWith({
|
||||
status: "running",
|
||||
sessionId: "session-1",
|
||||
exitCode: null,
|
||||
error: null,
|
||||
}),
|
||||
);
|
||||
|
||||
view.unmount();
|
||||
});
|
||||
|
||||
it("keeps the shared descriptor when a connecting mount is cancelled before its create resolves", async () => {
|
||||
// Under StrictMode the descriptor-create effect runs, tears down, and runs
|
||||
// again for the same stable tabId. The idempotent backend create returns the
|
||||
// same row, so the cancelled first mount must not delete it — otherwise the
|
||||
// live tab (and its bound PTY) is destroyed and cannot survive a reload.
|
||||
let resolveCreate: ((value: unknown) => void) | undefined;
|
||||
createQuickTerminalTab.mockImplementationOnce(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
resolveCreate = resolve;
|
||||
}),
|
||||
);
|
||||
|
||||
const view = render(<QuickTerminalTabView tab={tab} onStateChange={vi.fn()} />);
|
||||
view.unmount();
|
||||
resolveCreate?.({
|
||||
tabId: tab.tabId,
|
||||
workspaceId: tab.workspaceId,
|
||||
sessionId: null,
|
||||
sequence: 1,
|
||||
status: "connecting",
|
||||
});
|
||||
await Promise.resolve();
|
||||
|
||||
expect(deleteQuickTerminalTab).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("renders accessible lifecycle status for the selected terminal", () => {
|
||||
const { rerender } = render(<QuickTerminalTabView tab={tab} onStateChange={vi.fn()} />);
|
||||
|
||||
expect(screen.getByRole("status").textContent).toContain("Connecting to terminal…");
|
||||
expect(screen.getByTestId("quick-terminal-status").textContent).toContain(
|
||||
"Connecting to terminal…",
|
||||
);
|
||||
|
||||
rerender(
|
||||
<QuickTerminalTabView
|
||||
@@ -107,6 +145,8 @@ describe("QuickTerminalTabView", () => {
|
||||
onStateChange={vi.fn()}
|
||||
/>,
|
||||
);
|
||||
expect(screen.getByRole("status").textContent).toContain("Terminal exited with code 7.");
|
||||
expect(screen.getByTestId("quick-terminal-status").textContent).toContain(
|
||||
"Terminal exited with code 7.",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,12 +2,7 @@
|
||||
|
||||
import { useCallback, useEffect, useRef, useState } from "react";
|
||||
import { useTranslation } from "react-i18next";
|
||||
import {
|
||||
createQuickTerminalTab,
|
||||
deleteQuickTerminalTab,
|
||||
startHostShell,
|
||||
toQuickTerminalTab,
|
||||
} from "@/lib/api";
|
||||
import { createQuickTerminalTab, startHostShell, toQuickTerminalTab } from "@/lib/api";
|
||||
import {
|
||||
PtyTerminalView,
|
||||
type PtyTerminalState,
|
||||
@@ -89,9 +84,11 @@ export function QuickTerminalTabView({ tab, onStateChange, onDescriptorReady }:
|
||||
setDescriptorReady(false);
|
||||
createQuickTerminalTab(tab.tabId, tab.workspaceId)
|
||||
.then((descriptor) => {
|
||||
if (cancelled) {
|
||||
return deleteQuickTerminalTab(tab.tabId).catch(() => undefined);
|
||||
}
|
||||
// The descriptor is keyed by the stable tabId and the backend create is
|
||||
// idempotent, so a cancelled mount (StrictMode remount, tab switch,
|
||||
// reload) must NOT delete it — the next mount reuses the same row. Only
|
||||
// an explicit tab close deletes the descriptor, via useQuickTerminalClose.
|
||||
if (cancelled) return;
|
||||
const nextTab = toQuickTerminalTab(descriptor);
|
||||
onDescriptorReadyRef.current?.(nextTab);
|
||||
setDescriptorReady(true);
|
||||
@@ -113,8 +110,17 @@ export function QuickTerminalTabView({ tab, onStateChange, onDescriptorReady }:
|
||||
|
||||
const shouldRenderTerminal = Boolean(tab.sessionId) || (needsDescriptor && descriptorReady);
|
||||
const startSession = useCallback<StartPtySession>(
|
||||
(size, options) => startHostShell(size, options),
|
||||
[],
|
||||
async (size, options) => {
|
||||
const session = await startHostShell(size, options);
|
||||
onStateChange({
|
||||
status: session.running ? "running" : "exited",
|
||||
sessionId: session.session_id,
|
||||
exitCode: session.exit_code ?? null,
|
||||
error: null,
|
||||
});
|
||||
return session;
|
||||
},
|
||||
[onStateChange],
|
||||
);
|
||||
|
||||
return (
|
||||
|
||||
@@ -22,16 +22,16 @@ async function waitForTerminalReady(page: Page) {
|
||||
.not.toBe("");
|
||||
}
|
||||
|
||||
async function sendMarker(page: Page, marker: string) {
|
||||
async function runCommandAndWaitForOutput(page: Page, command: string, expected: string) {
|
||||
await page.getByTestId("quick-terminal-terminal").click();
|
||||
await page.keyboard.type(`echo ${marker}`);
|
||||
await page.keyboard.type(command);
|
||||
await page.keyboard.press("Enter");
|
||||
await expect
|
||||
.poll(() => readQuickTerminalBuffer(page), {
|
||||
timeout: 10_000,
|
||||
message: `Waiting for terminal marker ${marker}`,
|
||||
message: `Waiting for terminal output ${expected}`,
|
||||
})
|
||||
.toContain(marker);
|
||||
.toContain(expected);
|
||||
}
|
||||
|
||||
function terminalTab(dialog: Locator, sequence: number) {
|
||||
@@ -81,19 +81,23 @@ test.describe("quick terminal tabs", () => {
|
||||
await expect(dialog.getByTestId("quick-terminal-terminal")).toBeVisible();
|
||||
await expect(dialog.getByTestId("host-shell-done")).toHaveCount(0);
|
||||
await waitForTerminalReady(testPage);
|
||||
await sendMarker(testPage, "QUICK_TERMINAL_ONE");
|
||||
await runCommandAndWaitForOutput(
|
||||
testPage,
|
||||
"export KANDEV_QT_ONE=QUICK_TERMINAL_ONE && echo $KANDEV_QT_ONE",
|
||||
"QUICK_TERMINAL_ONE",
|
||||
);
|
||||
await expect(dialog.locator('[data-testid="quick-terminal-tab"]')).toHaveCount(1);
|
||||
|
||||
// The descriptor and detached PTY survive a full page reload. The
|
||||
// launcher must reattach the existing session instead of creating a
|
||||
// second terminal, including the buffered marker output.
|
||||
// launcher must reattach the existing shell instead of creating a
|
||||
// second terminal.
|
||||
await testPage.reload();
|
||||
await expect(terminalButton).toBeVisible();
|
||||
await terminalButton.click();
|
||||
await expect(dialog).toBeVisible();
|
||||
await expect(dialog.locator('[data-testid="quick-terminal-tab"]')).toHaveCount(1);
|
||||
await expect(dialog.getByTestId("quick-terminal-tab-panel")).toBeVisible();
|
||||
await expect.poll(() => readQuickTerminalBuffer(testPage)).toContain("QUICK_TERMINAL_ONE");
|
||||
await runCommandAndWaitForOutput(testPage, "echo $KANDEV_QT_ONE", "QUICK_TERMINAL_ONE");
|
||||
|
||||
// Dismissing the shared surface detaches the terminal but does not stop it.
|
||||
await testPage.keyboard.press("Escape");
|
||||
@@ -104,28 +108,45 @@ test.describe("quick terminal tabs", () => {
|
||||
await terminalButton.click();
|
||||
await expect(dialog).toBeVisible();
|
||||
await expect(dialog.locator('[data-testid="quick-terminal-tab"]')).toHaveCount(1);
|
||||
await expect.poll(() => readQuickTerminalBuffer(testPage)).toContain("QUICK_TERMINAL_ONE");
|
||||
await runCommandAndWaitForOutput(testPage, "echo $KANDEV_QT_ONE", "QUICK_TERMINAL_ONE");
|
||||
|
||||
// The grouped menu's New Terminal action always creates a second PTY.
|
||||
await dialog.getByTestId("quick-chat-add-menu-trigger").click();
|
||||
// Radix marks the rest of the page aria-hidden while the menu is open, so
|
||||
// the `dialog` role locator stops resolving; measure the trigger before
|
||||
// opening and reference the menu content by its page-level test id.
|
||||
const addMenuTrigger = testPage.getByTestId("quick-chat-add-menu-trigger");
|
||||
await expect(addMenuTrigger).toBeVisible();
|
||||
const addMenuTriggerBox = await addMenuTrigger.boundingBox();
|
||||
await addMenuTrigger.click();
|
||||
const addMenu = testPage.getByTestId("quick-chat-add-menu-content");
|
||||
await expect(addMenu).toBeVisible();
|
||||
const addMenuBox = await addMenu.boundingBox();
|
||||
expect(addMenuTriggerBox).not.toBeNull();
|
||||
expect(addMenuBox).not.toBeNull();
|
||||
expect(addMenuBox!.x).toBeGreaterThanOrEqual(addMenuTriggerBox!.x - 2);
|
||||
await assertLocatorWithinViewportX(addMenu, "desktop quick chat add menu");
|
||||
await expect(testPage.getByText("Agents", { exact: true })).toBeVisible();
|
||||
await expect(testPage.getByText("Terminals", { exact: true })).toBeVisible();
|
||||
await testPage.getByTestId("quick-chat-new-terminal").click();
|
||||
await expect(dialog.locator('[data-testid="quick-terminal-tab"]')).toHaveCount(2);
|
||||
await waitForTerminalReady(testPage);
|
||||
await sendMarker(testPage, "QUICK_TERMINAL_TWO");
|
||||
await runCommandAndWaitForOutput(
|
||||
testPage,
|
||||
"export KANDEV_QT_TWO=QUICK_TERMINAL_TWO && echo $KANDEV_QT_TWO",
|
||||
"QUICK_TERMINAL_TWO",
|
||||
);
|
||||
|
||||
const firstTab = terminalTab(dialog, 1);
|
||||
const secondTab = terminalTab(dialog, 2);
|
||||
await firstTab.getByRole("button", { name: "Terminal 1", exact: true }).click();
|
||||
await expect.poll(() => readQuickTerminalBuffer(testPage)).toContain("QUICK_TERMINAL_ONE");
|
||||
await runCommandAndWaitForOutput(testPage, "echo $KANDEV_QT_ONE", "QUICK_TERMINAL_ONE");
|
||||
await secondTab.getByRole("button", { name: "Terminal 2", exact: true }).click();
|
||||
await expect.poll(() => readQuickTerminalBuffer(testPage)).toContain("QUICK_TERMINAL_TWO");
|
||||
await runCommandAndWaitForOutput(testPage, "echo $KANDEV_QT_TWO", "QUICK_TERMINAL_TWO");
|
||||
|
||||
// Closing one tab stops/removes only that tab and falls back to its sibling.
|
||||
await secondTab.getByRole("button", { name: "Close Terminal 2" }).click();
|
||||
await expect(dialog.locator('[data-testid="quick-terminal-tab"]')).toHaveCount(1);
|
||||
await expect.poll(() => readQuickTerminalBuffer(testPage)).toContain("QUICK_TERMINAL_ONE");
|
||||
await runCommandAndWaitForOutput(testPage, "echo $KANDEV_QT_ONE", "QUICK_TERMINAL_ONE");
|
||||
|
||||
// The chat launcher switches content kind without discarding the terminal.
|
||||
await testPage.keyboard.press("Escape");
|
||||
|
||||
@@ -2,33 +2,110 @@ import { act, renderHook } from "@testing-library/react";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const reuseOrCreateQuickTerminal = vi.fn();
|
||||
let state = { reuseOrCreateQuickTerminal };
|
||||
const hydrate = vi.fn();
|
||||
const listQuickTerminalTabs = vi.fn();
|
||||
let state = {
|
||||
reuseOrCreateQuickTerminal,
|
||||
hydrate,
|
||||
quickChat: { terminalTabs: [] as Array<{ workspaceId: string }> },
|
||||
};
|
||||
|
||||
vi.mock("@/components/state-provider", () => ({
|
||||
useAppStore: (selector: (value: typeof state) => unknown) => selector(state),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/api/domains/quick-terminal-api", () => ({
|
||||
listQuickTerminalTabs: (...args: unknown[]) => listQuickTerminalTabs(...args),
|
||||
toQuickTerminalTab: (tab: unknown) => tab,
|
||||
}));
|
||||
|
||||
import { useQuickTerminalLauncher } from "./use-quick-terminal-launcher";
|
||||
|
||||
const WORKSPACE_ID = "workspace-1";
|
||||
|
||||
describe("useQuickTerminalLauncher", () => {
|
||||
beforeEach(() => {
|
||||
reuseOrCreateQuickTerminal.mockReset();
|
||||
state = { reuseOrCreateQuickTerminal };
|
||||
hydrate.mockReset();
|
||||
listQuickTerminalTabs.mockReset();
|
||||
listQuickTerminalTabs.mockResolvedValue({ tabs: [] });
|
||||
state = {
|
||||
reuseOrCreateQuickTerminal,
|
||||
hydrate,
|
||||
quickChat: { terminalTabs: [] },
|
||||
};
|
||||
});
|
||||
|
||||
it("reuses or creates the terminal for the requested workspace", () => {
|
||||
const { result } = renderHook(() => useQuickTerminalLauncher("workspace-1"));
|
||||
it("reuses or creates the terminal for the requested workspace", async () => {
|
||||
const { result } = renderHook(() => useQuickTerminalLauncher(WORKSPACE_ID));
|
||||
|
||||
act(() => result.current());
|
||||
await act(async () => {
|
||||
await result.current();
|
||||
});
|
||||
|
||||
expect(reuseOrCreateQuickTerminal).toHaveBeenCalledWith("workspace-1");
|
||||
expect(listQuickTerminalTabs).toHaveBeenCalledWith(WORKSPACE_ID);
|
||||
expect(reuseOrCreateQuickTerminal).toHaveBeenCalledWith(WORKSPACE_ID);
|
||||
});
|
||||
|
||||
it("does not launch without a workspace", () => {
|
||||
it("hydrates shared terminal tabs before reuse when the local store is empty", async () => {
|
||||
listQuickTerminalTabs.mockResolvedValue({
|
||||
tabs: [
|
||||
{
|
||||
tabId: "tab-1",
|
||||
workspaceId: WORKSPACE_ID,
|
||||
sessionId: "session-1",
|
||||
sequence: 1,
|
||||
status: "running",
|
||||
},
|
||||
],
|
||||
});
|
||||
const { result } = renderHook(() => useQuickTerminalLauncher(WORKSPACE_ID));
|
||||
|
||||
await act(async () => {
|
||||
await result.current();
|
||||
});
|
||||
|
||||
expect(hydrate).toHaveBeenCalledWith({
|
||||
quickChat: {
|
||||
terminalTabs: [
|
||||
{
|
||||
tabId: "tab-1",
|
||||
workspaceId: WORKSPACE_ID,
|
||||
sessionId: "session-1",
|
||||
sequence: 1,
|
||||
status: "running",
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
expect(reuseOrCreateQuickTerminal).toHaveBeenCalledWith(WORKSPACE_ID);
|
||||
});
|
||||
|
||||
it("does not relist server terminals when the workspace already has one locally", async () => {
|
||||
state = {
|
||||
reuseOrCreateQuickTerminal,
|
||||
hydrate,
|
||||
quickChat: { terminalTabs: [{ workspaceId: WORKSPACE_ID }] },
|
||||
};
|
||||
const { result } = renderHook(() => useQuickTerminalLauncher(WORKSPACE_ID));
|
||||
|
||||
await act(async () => {
|
||||
await result.current();
|
||||
});
|
||||
|
||||
expect(listQuickTerminalTabs).not.toHaveBeenCalled();
|
||||
expect(hydrate).not.toHaveBeenCalled();
|
||||
expect(reuseOrCreateQuickTerminal).toHaveBeenCalledWith(WORKSPACE_ID);
|
||||
});
|
||||
|
||||
it("does not launch without a workspace", async () => {
|
||||
const { result } = renderHook(() => useQuickTerminalLauncher(null));
|
||||
|
||||
act(() => result.current());
|
||||
await act(async () => {
|
||||
await result.current();
|
||||
});
|
||||
|
||||
expect(listQuickTerminalTabs).not.toHaveBeenCalled();
|
||||
expect(reuseOrCreateQuickTerminal).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3,14 +3,32 @@
|
||||
import { useCallback } from "react";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import { captureQuickChatLauncherFocus } from "@/components/quick-chat/quick-chat-focus";
|
||||
import { listQuickTerminalTabs, toQuickTerminalTab } from "@/lib/api/domains/quick-terminal-api";
|
||||
|
||||
/** Opens or re-selects a workspace's terminal in the shared Quick Chat surface. */
|
||||
export function useQuickTerminalLauncher(workspaceId?: string | null) {
|
||||
const reuseOrCreateQuickTerminal = useAppStore((state) => state.reuseOrCreateQuickTerminal);
|
||||
const hydrate = useAppStore((state) => state.hydrate);
|
||||
const terminalTabs = useAppStore((state) => state.quickChat.terminalTabs);
|
||||
|
||||
return useCallback(() => {
|
||||
return useCallback(async () => {
|
||||
if (!workspaceId) return;
|
||||
captureQuickChatLauncherFocus();
|
||||
|
||||
if (!terminalTabs.some((tab) => tab.workspaceId === workspaceId)) {
|
||||
try {
|
||||
const response = await listQuickTerminalTabs(workspaceId);
|
||||
if (response.tabs.length > 0) {
|
||||
hydrate({
|
||||
quickChat: { terminalTabs: response.tabs.map(toQuickTerminalTab) },
|
||||
});
|
||||
}
|
||||
} catch {
|
||||
// Best-effort. If the shared-state refresh fails, fall back to the
|
||||
// existing local reuse-or-create behavior.
|
||||
}
|
||||
}
|
||||
|
||||
reuseOrCreateQuickTerminal(workspaceId);
|
||||
}, [reuseOrCreateQuickTerminal, workspaceId]);
|
||||
}, [hydrate, reuseOrCreateQuickTerminal, terminalTabs, workspaceId]);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,144 @@
|
||||
---
|
||||
spec: docs/specs/quick-terminal/spec.md
|
||||
created: 2026-08-06
|
||||
status: draft
|
||||
---
|
||||
|
||||
# Implementation Plan: Quick Terminal Durable Session Lifecycle and Menu Alignment
|
||||
|
||||
## Overview
|
||||
|
||||
Two user-reported defects remain after the refresh-persistence work shipped:
|
||||
|
||||
1. A quick terminal does not survive a page reload in practice. A refreshed tab re-numbers
|
||||
("Terminal 7") and a new shell is created instead of reattaching to the existing one.
|
||||
2. The plus-button creation menu opens toward the leading (left) edge of the tab strip, overhanging
|
||||
the workspace edge, which is awkward.
|
||||
|
||||
The persistence infrastructure (durable descriptors, boot payload, resync, reattach by `sessionId`)
|
||||
already exists and is correct. The reload failure is not a missing-descriptor bug; it is a
|
||||
**session-lifetime** bug: quick-terminal PTYs share the agent-login `IdleTimeout = 10m` /
|
||||
`HardTimeout = 30m` in `loginpty`. A quiet or detached terminal is reaped, so on the next boot/resync
|
||||
`reconcileTab` finds no live session, calls `markUnavailable`, and persists `session_id = NULL` /
|
||||
`status = exited`. The user then starts a fresh terminal, and `sequence` keeps climbing.
|
||||
|
||||
The fix gives quick-terminal host-shell sessions their own lifetime policy: **no idle or hard
|
||||
timeout**. They end only on explicit tab close (already stops the PTY), natural process exit
|
||||
(already reported via `HandleSessionExit`), or backend shutdown. To avoid leaking PTYs on shutdown
|
||||
when the timeout no longer reaps them, the `loginpty.Manager` gains a `StopAll` wired into graceful
|
||||
shutdown. The menu fix is a one-line alignment change plus coverage.
|
||||
|
||||
## Confirmed root cause
|
||||
|
||||
- `apps/backend/internal/agent/loginpty/manager.go` `supervise()` applies `IdleTimeout` (10m) and a
|
||||
`HardTimeout` (30m) `context.WithTimeout` to **every** session, including host-shell sessions
|
||||
started by quick terminals. A quiet or detached quick terminal is terminated by these.
|
||||
- `apps/backend/internal/quickterminal/service.go` `reconcileTab()` (run by `List`, which boot state
|
||||
and GET `/quick-terminal-tabs` both call) sees the reaped session as `!Running` and calls
|
||||
`markUnavailable`, persisting `session_id = ""` and `status = exited`. This is why reload shows the
|
||||
terminal as gone and a new one is created.
|
||||
- `apps/web/components/quick-chat/quick-tab-add-menu.tsx` sets `align="end"` on a `w-64`
|
||||
`DropdownMenuContent`; the trigger is at the leading edge of the strip, so the menu opens leftward.
|
||||
|
||||
## Backend
|
||||
|
||||
### Per-session lifetime policy in loginpty
|
||||
|
||||
- Add an explicit lifetime policy to `Session`/`StartWithKey` so a session can opt out of the idle
|
||||
and hard timeouts. Host-shell sessions (`agentID == HostShellAgentID`) use the no-timeout policy;
|
||||
all other agent-login sessions keep the existing 10m/30m behavior. Prefer a small, explicit
|
||||
policy value (e.g. a `noTimeout bool` or a `lifetime` struct on the session) set from `agentID`
|
||||
inside `StartWithKey`, rather than reading `agentID` string comparisons scattered in `supervise`.
|
||||
- In `supervise()`, when the policy disables timeouts, do not arm the idle timer and do not wrap the
|
||||
context in `WithTimeout`; still handle natural process exit (`exited`) and external `stop()` so
|
||||
cleanup, map removal, and `onExit` remain correct. Keep the existing drain/`readDone` behavior.
|
||||
- Add `Manager.StopAll()` that stops every live session (idempotent), for graceful shutdown.
|
||||
|
||||
### Graceful shutdown wiring
|
||||
|
||||
- Register a cleanup in `backendapp` that calls `loginMgr.StopAll()` during graceful shutdown, so
|
||||
no-timeout quick-terminal PTYs are reaped when the backend stops (they previously relied on the
|
||||
30m hard timeout). Follow the existing `addCleanup`/`runCleanups` pattern used for the lifecycle
|
||||
manager.
|
||||
|
||||
## Frontend
|
||||
|
||||
### Menu alignment
|
||||
|
||||
- In `quick-tab-add-menu.tsx`, change `align="end"` to `align="start"` on `DropdownMenuContent` so
|
||||
the menu opens toward the trailing edge. Preserve the existing mobile bottom-sheet treatment.
|
||||
|
||||
### Reload reattachment (verification, likely no behavior change)
|
||||
|
||||
- The reattach path already exists (stable `tabId` → `managerKey`, reconcile trusts backend
|
||||
`running`). With sessions no longer reaped, confirm no additional frontend change is needed for
|
||||
reload reattachment; only add regression coverage. If a residual client-side path still starts a
|
||||
fresh shell for a `running` restored descriptor, fix it in the terminal tab view / reconcile so a
|
||||
restored live descriptor only attaches by `sessionId`.
|
||||
|
||||
## Tests
|
||||
|
||||
- **What:** host-shell sessions are not reaped by idle or hard timeout, while agent-login sessions
|
||||
still are; `StopAll` stops all live sessions and is idempotent; natural exit and external stop
|
||||
still clean up manager maps and fire `onExit` under the no-timeout policy.
|
||||
**Files:** `apps/backend/internal/agent/loginpty/manager_test.go`.
|
||||
**How:** `testing/synctest` to advance fake time past 10m/30m and assert a host-shell session is
|
||||
still running while an agent-login session has exited; direct `StopAll` assertions.
|
||||
- **What:** `reconcileTab` keeps a still-running host-shell session as `running` with its
|
||||
`sessionId` after simulated long idle (no reaping), and only `markUnavailable` when the manager
|
||||
truly has no entry.
|
||||
**Files:** `apps/backend/internal/quickterminal/service_test.go`.
|
||||
**How:** table-driven service tests against a manager holding a live long-idle session and one
|
||||
with no entry.
|
||||
- **What:** graceful-shutdown cleanup invokes `loginMgr.StopAll()`.
|
||||
**Files:** the relevant `apps/backend/internal/backendapp/*_test.go` for cleanup wiring.
|
||||
**How:** assert the registered cleanup stops a seeded live session.
|
||||
- **What:** the add menu renders with start alignment and existing items.
|
||||
**Files:** an existing `apps/web/components/quick-chat/*` test (extend, do not add a new file
|
||||
unless none covers this component).
|
||||
**How:** assert the `DropdownMenuContent` alignment prop / rendered side and the two menu items.
|
||||
|
||||
## E2E Tests
|
||||
|
||||
- **Scenario:** GIVEN a running quick terminal left idle past the old 10m idle window (fast-forward
|
||||
or a shortened test policy hook), WHEN the user reloads and reopens Quick Chat, THEN the same tab
|
||||
and sequence reattach to the same PTY without a new numbered terminal.
|
||||
**File:** `apps/web/e2e/tests/terminal/quick-terminal.spec.ts` (extend the existing reattach spec).
|
||||
**What to verify:** single tab, unchanged sequence/label, same-session marker output, and no
|
||||
second shell. Close every surviving terminal in `finally`.
|
||||
- **Scenario:** GIVEN the plus menu on a desktop pointer viewport, WHEN it opens, THEN it opens
|
||||
toward the trailing edge and stays within the viewport.
|
||||
**File:** `apps/web/e2e/tests/terminal/quick-terminal.spec.ts` or the existing quick-chat menu
|
||||
spec.
|
||||
**What to verify:** menu bounding box is to the right of the trigger and within the viewport.
|
||||
|
||||
## Verification Results
|
||||
|
||||
_To be filled in during implementation._
|
||||
|
||||
## Implementation Waves And Parallel Candidates
|
||||
|
||||
Wave 1 (parallel-safe, independent surfaces):
|
||||
|
||||
- [ ] [Task 01: Per-session lifetime policy and StopAll in loginpty](task-01-loginpty-lifetime-policy.md)
|
||||
- [ ] [Task 02: Fix add-menu alignment](task-02-add-menu-alignment.md)
|
||||
|
||||
Wave 2 (depends on Task 01):
|
||||
|
||||
- [ ] [Task 03: Shutdown wiring, reconcile coverage, and reload E2E](task-03-shutdown-and-reattach-verification.md)
|
||||
|
||||
Task 02 is fully independent of the backend tasks and can land in parallel. Task 03 depends on the
|
||||
Task 01 policy/`StopAll` surface.
|
||||
|
||||
## Risks
|
||||
|
||||
- Removing timeouts means a runaway or forgotten shell can live for the entire backend uptime.
|
||||
Mitigation: explicit tab close already stops the PTY, natural exit is reported, and `StopAll`
|
||||
reaps everything on shutdown. This matches the user's explicit request and stays bounded by
|
||||
process lifetime.
|
||||
- The lifetime policy must not weaken agent-login OAuth reaping. Keep the policy keyed strictly to
|
||||
`HostShellAgentID` and assert the agent-login path still times out.
|
||||
- `supervise` cleanup correctness (map removal, `onExit`) must hold when the idle timer and timeout
|
||||
context are absent. Cover natural exit and external stop under the no-timeout policy.
|
||||
- E2E time-advancement for a 10m idle window is expensive; prefer a test-only shortened policy hook
|
||||
or fake-clock injection rather than a real 10-minute wait.
|
||||
@@ -0,0 +1,63 @@
|
||||
---
|
||||
id: "01-loginpty-lifetime-policy"
|
||||
title: "Per-session lifetime policy and StopAll in loginpty"
|
||||
status: draft
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/quick-terminal/spec.md"
|
||||
---
|
||||
|
||||
# Task 01: Per-session lifetime policy and StopAll in loginpty
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Host-shell sessions (`agentID == loginpty.HostShellAgentID`) run with no idle timeout and no hard
|
||||
(wall-clock) timeout. They end only on natural process exit or an explicit `stop()`/`Stop`/
|
||||
`StopSession`.
|
||||
- Agent-login sessions (any other `agentID`) retain the existing `IdleTimeout` (10m) and
|
||||
`HardTimeout` (30m) behavior unchanged.
|
||||
- The lifetime policy is set once at session creation (in `StartWithKey`, derived from `agentID`),
|
||||
not by scattered string checks inside `supervise`.
|
||||
- Under the no-timeout policy, `supervise` still exits and cleans up correctly on natural process
|
||||
exit and on external `stop()`: the session is removed from `m.sessions` and `m.byID`, and `onExit`
|
||||
fires with the correct exit code.
|
||||
- `Manager.StopAll()` stops every currently live session, is idempotent, and returns after all are
|
||||
stopped.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
(cd apps/backend && go test ./internal/agent/loginpty/... -count=1)
|
||||
(cd apps/backend && golangci-lint run ./internal/agent/loginpty/... --timeout=5m)
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/agent/loginpty/manager.go`
|
||||
- `apps/backend/internal/agent/loginpty/manager_test.go`
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Parallelism
|
||||
|
||||
Parallel-safe with Task 02 (frontend). Task 03 depends on this task's policy and `StopAll` surface.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Spec sections: State machine (no idle/hard timeout row), Failure modes, Persistence guarantees.
|
||||
- Existing `supervise()` loop: `exited`, `idle.C`, `ctx.Done()`, `activityCh` cases; `Session.stop()`
|
||||
idempotency; `waitForCommand()`.
|
||||
- `HostShellAgentID` constant in `apps/backend/internal/agent/loginpty/handlers.go`.
|
||||
|
||||
## Output contract
|
||||
|
||||
Report the policy representation chosen, the `supervise` changes, the `StopAll` implementation, and
|
||||
exact test results. Do not change agent-login timeout constants. Update this task and the parent plan
|
||||
only after the focused tests and lint pass.
|
||||
|
||||
## Results
|
||||
|
||||
_To be filled in during implementation._
|
||||
@@ -0,0 +1,57 @@
|
||||
---
|
||||
id: "02-add-menu-alignment"
|
||||
title: "Fix add-menu alignment"
|
||||
status: draft
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/quick-terminal/spec.md"
|
||||
---
|
||||
|
||||
# Task 02: Fix add-menu alignment
|
||||
|
||||
## Acceptance
|
||||
|
||||
- The Quick Chat plus-button creation menu opens toward the trailing edge of the tab strip (aligned
|
||||
to the button's start), not toward the leading edge, on pointer viewports.
|
||||
- The existing menu content is unchanged: an **Agents** section with **New Agent**, a separator, and
|
||||
a **Terminals** section with **New Terminal**.
|
||||
- The existing mobile/coarse-pointer bottom-sheet treatment for the dropdown remains intact.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
(cd apps/web && pnpm run typecheck)
|
||||
(cd apps && pnpm --filter @kandev/web lint)
|
||||
(cd apps/web && pnpm vitest run components/quick-chat)
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/web/components/quick-chat/quick-tab-add-menu.tsx`
|
||||
- An existing `apps/web/components/quick-chat/*` test that covers the add menu (extend it; add a new
|
||||
test file only if none covers this component).
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Parallelism
|
||||
|
||||
Fully independent of the backend tasks; can land in parallel with Task 01 and Task 03.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Spec section: What (plus-button menu opens toward the trailing edge).
|
||||
- `quick-tab-add-menu.tsx`: `DropdownMenuContent align="end" className="w-64"` is the current source
|
||||
of the leftward opening.
|
||||
- `apps/web/app/globals.css` inset bottom-sheet treatment for Radix DropdownMenu below 640px.
|
||||
|
||||
## Output contract
|
||||
|
||||
Report the alignment change, the test assertion added/updated, and exact command results. Update this
|
||||
task and the parent plan only after typecheck, lint, and the focused test pass.
|
||||
|
||||
## Results
|
||||
|
||||
_To be filled in during implementation._
|
||||
+66
@@ -0,0 +1,66 @@
|
||||
---
|
||||
id: "03-shutdown-and-reattach-verification"
|
||||
title: "Shutdown wiring, reconcile coverage, and reload E2E"
|
||||
status: draft
|
||||
wave: 2
|
||||
depends_on: ["01-loginpty-lifetime-policy"]
|
||||
plan: "plan.md"
|
||||
spec: "../../specs/quick-terminal/spec.md"
|
||||
---
|
||||
|
||||
# Task 03: Shutdown wiring, reconcile coverage, and reload E2E
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Graceful backend shutdown stops all live quick-terminal (host-shell) PTYs by invoking
|
||||
`loginMgr.StopAll()` through the existing cleanup chain, so no-timeout sessions do not leak.
|
||||
- `reconcileTab` keeps a still-running host-shell session as `running` with its `sessionId` after a
|
||||
simulated long idle (session no longer reaped), and only clears/marks unavailable when the manager
|
||||
genuinely has no live entry for the descriptor.
|
||||
- After a page reload of a still-running quick terminal (including one idle past the old 10m idle
|
||||
window), the same tab and `sequence` reattach to the same PTY and no new numbered terminal is
|
||||
created.
|
||||
- If any residual client path starts a fresh shell for a restored `running` descriptor, it is fixed
|
||||
so a restored live descriptor attaches by `sessionId` only.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
(cd apps/backend && go test ./internal/backendapp ./internal/quickterminal/... -count=1)
|
||||
(cd apps/web && pnpm vitest run components/quick-chat lib/state/slices/ui)
|
||||
(cd apps/web && pnpm e2e -- terminal/quick-terminal.spec.ts)
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/backendapp/` (cleanup registration for `loginMgr.StopAll()`) and its test.
|
||||
- `apps/backend/internal/quickterminal/service_test.go`
|
||||
- `apps/web/e2e/tests/terminal/quick-terminal.spec.ts`
|
||||
- Frontend reconcile / terminal tab view only if a residual fresh-start path is found.
|
||||
|
||||
## Dependencies
|
||||
|
||||
Depends on Task 01 (`StopAll` and the no-timeout policy must exist first).
|
||||
|
||||
## Parallelism
|
||||
|
||||
Sequential after Task 01. Independent of Task 02.
|
||||
|
||||
## Inputs
|
||||
|
||||
- Task 01 policy and `Manager.StopAll()`.
|
||||
- `backendapp` `addCleanup`/`runCleanups` graceful-shutdown pattern (mirrors the lifecycle manager
|
||||
cleanup already registered).
|
||||
- `service.go` `reconcileTab` / `List` reconciliation and `ownsSession` guard.
|
||||
- Existing reattach E2E in `apps/web/e2e/tests/terminal/quick-terminal.spec.ts`.
|
||||
|
||||
## Output contract
|
||||
|
||||
Report the cleanup wiring, reconcile test additions, any frontend change made, and exact command
|
||||
results including the E2E run. Prefer a test-only shortened policy hook or fake clock over a real
|
||||
10-minute idle wait in E2E. Update this task and the parent plan only after backend, frontend, and
|
||||
the targeted E2E pass.
|
||||
|
||||
## Results
|
||||
|
||||
_To be filled in during implementation._
|
||||
@@ -1,7 +1,7 @@
|
||||
---
|
||||
status: shipped
|
||||
status: building
|
||||
created: 2026-08-03
|
||||
updated: 2026-08-05
|
||||
updated: 2026-08-06
|
||||
owner: kandev
|
||||
---
|
||||
|
||||
@@ -27,7 +27,9 @@ them without losing work, and return to the most recent terminal without managin
|
||||
- The tab-strip plus button opens a creation menu grouped like the task-detail Dockview add menu:
|
||||
an **Agents** section with **New Agent**, a separator, and a **Terminals** section with
|
||||
**New Terminal**. Existing tabs remain directly selectable in the tab strip rather than being
|
||||
duplicated in the creation menu.
|
||||
duplicated in the creation menu. Because the plus button sits at the leading edge of the tab
|
||||
strip, its menu opens toward the trailing edge (aligned to the button's start) so it does not
|
||||
overhang the workspace edge.
|
||||
- Choosing **New Agent** preserves the current ordinary/configuration setup flow. Choosing
|
||||
**New Terminal** always creates and activates a distinct host-shell terminal, even when another
|
||||
terminal exists.
|
||||
@@ -42,6 +44,10 @@ them without losing work, and return to the most recent terminal without managin
|
||||
- Switching to another tab or dismissing the shared dialog detaches the terminal presentation but
|
||||
leaves its host-shell session running. Reopening the dialog or reselecting the tab reconnects to
|
||||
that same session and replays the backend's available recent output.
|
||||
- A quick-terminal host-shell session has no idle or wall-clock timeout. Once started it runs until
|
||||
the user closes its tab, the shell process exits, or the backend stops. A long-running or quiet
|
||||
process therefore keeps running while the dialog is closed and survives page reloads. This differs
|
||||
from the Agents-page agent-login sessions, which retain their existing idle and hard timeouts.
|
||||
- Closing a terminal tab stops that tab's host-shell session and removes only that tab. Closing a
|
||||
chat tab retains the existing Quick Chat confirmation and task-deletion behavior.
|
||||
- After the active tab is removed, the dialog selects the nearest remaining tab in the same
|
||||
@@ -151,6 +157,7 @@ legacy singleton login surface retains its existing behavior and authorization c
|
||||
| Connecting | Host-shell start succeeds | Store the returned session ID, attach its stream, and mark the tab running. |
|
||||
| Running | Chat/terminal tab switch or dialog dismissal | Detach the rendered terminal; keep the backend PTY and tab descriptor alive. |
|
||||
| Detached | Tab selected or terminal launcher used | Reattach to the same session and replay available buffered output. |
|
||||
| Running or detached | Quiet or long-running work | The session has no idle or hard timeout; it stays running across tab switches, dialog dismissal, and page reloads until closed, exited, or the backend stops. |
|
||||
| Running or detached | PTY exits | Mark the tab exited when observed; retain it for inspection. |
|
||||
| Connecting, running, exited, or error | Terminal tab close | Stop the session when one exists, remove the tab, select the nearest remaining same-workspace tab, or close the dialog when none remains. |
|
||||
|
||||
@@ -168,9 +175,9 @@ checks, and Agents-page authorization behavior remain unchanged.
|
||||
- Closing a terminal tab while its start request is pending removes the tab and stops the session if
|
||||
that request later succeeds. Development StrictMode replay uses the stable client ID and cannot
|
||||
create or stop a sibling session.
|
||||
- A detached session may exit or reach the existing backend idle/hard timeout. Reattaching to a
|
||||
session that no longer exists marks the tab exited or unavailable; it does not create a
|
||||
replacement implicitly.
|
||||
- A detached session may exit on its own (for example the user runs `exit` or the shell process
|
||||
crashes). It is not reaped by an idle or hard timeout. Reattaching to a session that no longer
|
||||
exists marks the tab exited or unavailable; it does not create a replacement implicitly.
|
||||
- A descriptor whose PTY disappeared during a backend restart or while no client was attached is
|
||||
retained with an exited/unavailable status and a cleared session association. The user must
|
||||
choose **New Terminal** to create a replacement.
|
||||
@@ -191,9 +198,11 @@ checks, and Agents-page authorization behavior remain unchanged.
|
||||
for the same authenticated user and workspace. The backend is the durable source of descriptor
|
||||
membership, sequence, lifecycle snapshot, and latest session association.
|
||||
- Host-shell processes and their rolling output buffers live only in backend memory. Dismissing the
|
||||
shared dialog does not stop them, but explicit tab close, process exit, the existing idle/hard
|
||||
timeout, or a backend restart does. A backend restart leaves the descriptor behind and marks it
|
||||
unavailable until the user explicitly creates a new terminal.
|
||||
shared dialog does not stop them, and quick-terminal sessions have no idle or hard timeout, so
|
||||
only explicit tab close, the shell process exiting, or the backend stopping ends one. A backend
|
||||
restart leaves the descriptor behind and marks it unavailable until the user explicitly creates a
|
||||
new terminal. On graceful backend shutdown all live quick-terminal sessions are stopped rather
|
||||
than left to leak.
|
||||
- Reconnection can replay only the backend's existing rolling output buffer; this feature does not
|
||||
add durable terminal history.
|
||||
|
||||
@@ -217,6 +226,13 @@ checks, and Agents-page authorization behavior remain unchanged.
|
||||
- **GIVEN** a running terminal tab with a persisted descriptor, **WHEN** the user refreshes the page,
|
||||
**THEN** the Quick Chat dialog restores one terminal tab with the same sequence and reattaches to
|
||||
the same live session without creating a duplicate PTY.
|
||||
- **GIVEN** a running terminal tab whose process has produced no output and stayed detached for
|
||||
longer than the agent-login idle timeout, **WHEN** the user refreshes the page and reopens Quick
|
||||
Chat, **THEN** the tab reattaches to the same still-running session rather than showing exited and
|
||||
rather than starting a new numbered terminal.
|
||||
- **GIVEN** the plus button at the leading edge of the tab strip, **WHEN** the user opens the
|
||||
creation menu on a pointer device, **THEN** the menu opens toward the trailing edge and stays
|
||||
within the viewport instead of overhanging the leading edge.
|
||||
- **GIVEN** a persisted terminal descriptor whose PTY was lost during a backend restart, **WHEN** the
|
||||
user opens Quick Chat, **THEN** the tab remains visible as exited/unavailable and no replacement
|
||||
session starts until **New Terminal** is chosen.
|
||||
@@ -244,6 +260,10 @@ checks, and Agents-page authorization behavior remain unchanged.
|
||||
|
||||
- Persisting terminal output or keeping a PTY process alive across a backend restart. The durable
|
||||
descriptor only makes the tab discoverable and reports that its old session is unavailable.
|
||||
Removing the idle/hard timeout keeps a quiet session alive while the backend runs; it does not
|
||||
make the process survive the backend stopping.
|
||||
- Changing the Agents-page agent-login session lifecycle, which keeps its existing idle and hard
|
||||
timeouts.
|
||||
- Synchronizing terminal input or output streams between multiple attached clients beyond the
|
||||
existing PTY subscription and rolling buffer behavior.
|
||||
- Task-workspace terminals, repository working-directory selection, or moving terminals between
|
||||
@@ -257,3 +277,5 @@ checks, and Agents-page authorization behavior remain unchanged.
|
||||
[Quick Chat and Terminal Tabs implementation](../../plans/quick-terminal/plan.md)
|
||||
|
||||
[Quick Terminal refresh persistence repair](../../plans/quick-terminal-refresh-persistence/plan.md)
|
||||
|
||||
[Quick Terminal durable session lifecycle and menu alignment](../../plans/quick-terminal-durable-lifecycle/plan.md)
|
||||
|
||||
Reference in New Issue
Block a user