tao.chen/codespace
Archived
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(shell): multi-shell per workspace + fix WS disconnect killing bash

Browse Source 
BREAKING: every shell operation now requires a shellId. The Manager
previously keyed by workspaceID alone (one bash per workspace). It now
keys by (workspaceID, shellID) where shellID is a UUID returned by
Start.

Fixes the long-standing bug where the WS handler's closeAll called
stdin.Close() and killed bash when a WS client disconnected. The
Session owns the pty file; the WS handler no longer closes it. The
pty is only closed by Manager.Stop (explicit) or by captureOutput
when the process naturally exits (EOF).

- internal/shell/manager.go: Manager interface gains List and every
  method takes shellID; storage becomes
  map[workspaceID]map[shellID]*Session; Start returns (shellID, err)
  via uuid.NewString; Resize/Status/ExitStatus/Subscribe/Stdin/Stop
  route by shellID; new List(workspaceID) returns ShellInfo[] in
  creation order.
- internal/shell/session.go: Session gains ShellID + CreatedAt; Status
  type gains ShellID.
- internal/shell/manager_test.go: updated existing tests for new
  signatures; added TestShellMultiInstance (two shells in one
  workspace, no output cross-talk, independent stop, List behavior).
- internal/service/shell_service.go: wrappers carry shellID; new
  List method.
- internal/service/workspace_service.go: auto-start captures/logs
  shellID; Delete iterates and stops all workspace shells.
- internal/api/shell_handler.go: WS closeAll drops stdin.Close();
  start/restart return 201 with {shellId, pid, ...}; new list handler;
  stop/resize take shellId in body.
- internal/api/router.go: GET /api/workspaces/:id/shell (list).
- internal/model/shell.go: new ShellStartResponse, ShellInfo,
  ShellListResponse, ShellStopRequest, ShellRestartRequest; updated
  ShellStatusResponse + ShellResizeRequest to carry shellId.
- go.mod/go.sum: github.com/google/uuid.

E2E:
- workspace create -> 1 auto shell
- start 2 more -> 3 shells in list
- stop 1 -> 2 shells in list
- WS connect -> send cmd -> disconnect -> WS reconnect -> send cmd ->
  response OK, no [process exited] banner
This commit is contained in:
tao.chen
2026-07-06 14:57:03 +08:00
parent f5a6ff8b0d
commit 13b0c4e53f
10 changed files with 479 additions and 168 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/shell/manager_test.go
+95 -28
View File
@@ -10,37 +10,34 @@ import (
)
import (
"os"
"codespace/internal/util"
"github.com/creack/pty"
)
func startTestShell(t *testing.T) (*LocalManager, io.WriteCloser, Subscription) {
func startTestShell(t *testing.T) (*LocalManager, string, io.WriteCloser, Subscription) {
t.Helper()
mgr := NewManager("bash", []string{"-i"})
root := t.TempDir()
if err := mgr.Start("test-ws", root); err != nil {
shellID, err := mgr.Start("test-ws", root)
if err != nil {
t.Fatalf("Start failed: %v", err)
}
stdin, err := mgr.Stdin("test-ws")
stdin, err := mgr.Stdin("test-ws", shellID)
if err != nil {
t.Fatalf("Stdin failed: %v", err)
}
sub, err := mgr.Subscribe("test-ws")
sub, err := mgr.Subscribe("test-ws", shellID)
if err != nil {
t.Fatalf("Subscribe failed: %v", err)
}
t.Cleanup(func() {
sub.Close()
_ = mgr.Stop("test-ws")
_ = mgr.Stop("test-ws", shellID)
})
return mgr, stdin, sub
return mgr, shellID, stdin, sub
}
func waitForMarker(t *testing.T, sub Subscription, stdin io.Writer, input, marker string) string {
@@ -71,8 +68,9 @@ func waitForMarker(t *testing.T, sub Subscription, stdin io.Writer, input, marke
}
func TestShellPTYIsRealTerminal(t *testing.T) {
mgr, stdin, sub := startTestShell(t)
mgr, shellID, stdin, sub := startTestShell(t)
_ = mgr
_ = shellID
out := waitForMarker(t, sub, stdin, "echo HELLO_PTY_TEST\n", "HELLO_PTY_TEST")
@@ -85,7 +83,7 @@ func TestShellPTYIsRealTerminal(t *testing.T) {
}
func TestShellPTYTermEnv(t *testing.T) {
_, stdin, sub := startTestShell(t)
_, _, stdin, sub := startTestShell(t)
out := waitForMarker(t, sub, stdin, "echo \"$TERM\"\n", "xterm-256color")
if !strings.Contains(out, "xterm-256color") {
@@ -94,31 +92,23 @@ func TestShellPTYTermEnv(t *testing.T) {
}
func TestShellResizeUpdatesPTYSize(t *testing.T) {
mgr, stdin, sub := startTestShell(t)
mgr, shellID, stdin, sub := startTestShell(t)
_ = sub
f, ok := stdin.(*os.File)
if !ok {
t.Fatal("stdin is not a pty file")
}
if err := mgr.Resize("test-ws", 120, 40); err != nil {
if err := mgr.Resize("test-ws", shellID, 120, 40); err != nil {
t.Fatalf("Resize failed: %v", err)
}
rows, cols, err := pty.Getsize(f)
if err != nil {
t.Fatalf("Getsize failed: %v", err)
}
if cols != 120 || rows != 40 {
t.Errorf("expected cols=120 rows=40, got cols=%d rows=%d", cols, rows)
out := waitForMarker(t, sub, stdin, "stty size\n", "40 120")
if !strings.Contains(out, "40 120") {
t.Errorf("expected stty size to report 40 120, got:\n%s", out)
}
}
func TestShellResizeRejectsInvalidSize(t *testing.T) {
mgr, _, _ := startTestShell(t)
mgr, shellID, _, _ := startTestShell(t)
if err := mgr.Resize("test-ws", 0, 40); err == nil {
if err := mgr.Resize("test-ws", shellID, 0, 40); err == nil {
t.Fatal("expected error for invalid size")
} else if util.CodeOf(err) != util.CodeBadRequest {
t.Errorf("expected CodeBadRequest, got %v", util.CodeOf(err))
@@ -128,9 +118,86 @@ func TestShellResizeRejectsInvalidSize(t *testing.T) {
func TestShellResizeRejectsMissingSession(t *testing.T) {
mgr := NewManager("bash", []string{"-i"})
if err := mgr.Resize("missing-ws", 80, 24); err == nil {
if err := mgr.Resize("missing-ws", "missing-shell", 80, 24); err == nil {
t.Fatal("expected error for missing session")
} else if util.CodeOf(err) != util.CodeNotFound {
t.Errorf("expected CodeNotFound, got %v", util.CodeOf(err))
}
}
func TestShellMultiInstance(t *testing.T) {
mgr := NewManager("bash", []string{"-i"})
root := t.TempDir()
shellID1, err := mgr.Start("test-ws", root)
if err != nil {
t.Fatalf("Start shell1 failed: %v", err)
}
shellID2, err := mgr.Start("test-ws", root)
if err != nil {
t.Fatalf("Start shell2 failed: %v", err)
}
if shellID1 == shellID2 {
t.Fatalf("expected distinct shell IDs, got %q and %q", shellID1, shellID2)
}
stdin1, err := mgr.Stdin("test-ws", shellID1)
if err != nil {
t.Fatalf("Stdin shell1 failed: %v", err)
}
stdin2, err := mgr.Stdin("test-ws", shellID2)
if err != nil {
t.Fatalf("Stdin shell2 failed: %v", err)
}
sub1, err := mgr.Subscribe("test-ws", shellID1)
if err != nil {
t.Fatalf("Subscribe shell1 failed: %v", err)
}
sub2, err := mgr.Subscribe("test-ws", shellID2)
if err != nil {
t.Fatalf("Subscribe shell2 failed: %v", err)
}
t.Cleanup(func() {
sub1.Close()
sub2.Close()
_ = mgr.Stop("test-ws", shellID1)
_ = mgr.Stop("test-ws", shellID2)
})
list := mgr.List("test-ws")
if len(list) != 2 {
t.Fatalf("List returned %d shells, want 2", len(list))
}
out1 := waitForMarker(t, sub1, stdin1, "echo SHELL_ONE\n", "SHELL_ONE")
if strings.Contains(out1, "SHELL_TWO") {
t.Errorf("shell1 output leaked shell2 output: %s", out1)
}
out2 := waitForMarker(t, sub2, stdin2, "echo SHELL_TWO\n", "SHELL_TWO")
if strings.Contains(out2, "SHELL_ONE") {
t.Errorf("shell2 output leaked shell1 output: %s", out2)
}
if err := mgr.Stop("test-ws", shellID1); err != nil {
t.Fatalf("Stop shell1 failed: %v", err)
}
if _, err := mgr.Stdin("test-ws", shellID1); err == nil {
t.Fatal("expected shell1 stdin to be unavailable after stop")
}
// shell2 should still work.
_ = waitForMarker(t, sub2, stdin2, "echo STILL_ALIVE\n", "STILL_ALIVE")
list = mgr.List("test-ws")
if len(list) != 1 {
t.Fatalf("List returned %d shells after stop, want 1", len(list))
}
if list[0].ShellID != shellID2 {
t.Fatalf("List returned shell %q, want %q", list[0].ShellID, shellID2)
}
}