diff --git a/PROTOCOL.md b/PROTOCOL.md index 41052ec..ff279c5 100644 --- a/PROTOCOL.md +++ b/PROTOCOL.md @@ -173,9 +173,10 @@ Errors: `{"error": "message"}` with appropriate status. `POST /api/spawn` semantics: -1. Clone/pull repo into volume `lvmh-repo-` (slug = repo path with `/` - → `-`), default branch unless `branch` given. Concurrent spawns on same - volume serialize. +1. Clone/pull repo into volume `lvmh-repo-` (slug = repo path with + `/` → `--`, plus `-` + first 6 hex of sha256(repo path) so distinct + paths can never collide), default branch unless `branch` given. + Concurrent spawns on same volume serialize. 2. Create container from image `lvmh-worker:latest` (build from repo's `docker/worker.Dockerfile` if missing), env: `ZAI_RENAUD_API_KEY`, `LVMH_TOKEN`, `LVMH_URL`, provider/models.json mounted read-only, diff --git a/daemon/api.go b/daemon/api.go index 0428b0f..188ef84 100644 --- a/daemon/api.go +++ b/daemon/api.go @@ -280,6 +280,10 @@ func (s *Server) handleSpawn(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "repo must look like group/project") return } + if body.Branch != "" && !branchRe.MatchString(body.Branch) { + writeError(w, http.StatusBadRequest, "invalid branch name") + return + } res, err := s.spawn.Start(r.Context(), body.Repo, body.Branch) if err != nil { writeError(w, http.StatusInternalServerError, err.Error()) diff --git a/daemon/api_test.go b/daemon/api_test.go index 95c9ed4..efc2d8d 100644 --- a/daemon/api_test.go +++ b/daemon/api_test.go @@ -106,16 +106,33 @@ func TestAPIEventsReplayShape(t *testing.T) { func TestAPISpawnValidation(t *testing.T) { ts, _ := newTestServer(t) - req, _ := http.NewRequest(http.MethodPost, ts.URL+"/api/spawn", - strings.NewReader(`{"repo":"no-slash"}`)) - req.Header.Set("Authorization", "Bearer "+testToken) - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("spawn: %v", err) + post := func(body string) int { + req, _ := http.NewRequest(http.MethodPost, ts.URL+"/api/spawn", strings.NewReader(body)) + req.Header.Set("Authorization", "Bearer "+testToken) + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("spawn: %v", err) + } + resp.Body.Close() + return resp.StatusCode } - resp.Body.Close() - if resp.StatusCode != http.StatusBadRequest { - t.Fatalf("bad repo → %d, want 400", resp.StatusCode) + if code := post(`{"repo":"no-slash"}`); code != http.StatusBadRequest { + t.Fatalf("bad repo → %d, want 400", code) + } + // branch is passed to git: anything outside the safe charset → 400 + for _, branch := range []string{ + "main; rm -rf /", + "feature one", // space + "-oProxyCommand=x", // leading option-ish + "main$(id)", + } { + if code := post(`{"repo":"group/project","branch":"` + branch + `"}`); code != http.StatusBadRequest { + t.Fatalf("branch %q → %d, want 400", branch, code) + } + } + // empty branch (default) stays accepted at this validation layer + if code := post(`{"repo":"group/project","branch":""}`); code == http.StatusBadRequest { + t.Fatal("empty branch must not be rejected as invalid") } } diff --git a/daemon/docker.go b/daemon/docker.go index 34fb638..3db0278 100644 --- a/daemon/docker.go +++ b/daemon/docker.go @@ -7,6 +7,7 @@ import ( "bytes" "context" "crypto/rand" + "crypto/sha256" "encoding/hex" "encoding/json" "errors" @@ -22,6 +23,7 @@ import ( "sync" "time" + cerrdefs "github.com/containerd/errdefs" "github.com/docker/docker/api/types/build" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/filters" @@ -69,10 +71,20 @@ var errNoContainer = errors.New("no container for session") var repoPathRe = regexp.MustCompile(`^[A-Za-z0-9_.-]+(/[A-Za-z0-9_.-]+)+$`) +// branchRe guards spawn branch names against git option injection. +var branchRe = regexp.MustCompile(`^[A-Za-z0-9._/-]+$`) + // validRepoPath accepts "group/project" style paths (at least two segments). func validRepoPath(repo string) bool { return repoPathRe.MatchString(repo) } -func repoSlug(repo string) string { return strings.ReplaceAll(repo, "/", "--") } +// repoSlug maps a repo path to a collision-free filesystem/volume name: +// the `/`→`--` form alone is ambiguous ("a/b--c" vs "a/b/c"), so the +// first 6 hex chars of the sha256 of the full path disambiguate. Changing +// this changes existing volume names (one-time fresh clone per repo). +func repoSlug(repo string) string { + sum := sha256.Sum256([]byte(repo)) + return strings.ReplaceAll(repo, "/", "--") + "-" + hex.EncodeToString(sum[:3]) +} // newUUID returns a random RFC 4122 v4 UUID string. func newUUID() string { @@ -214,11 +226,11 @@ func (s *Spawner) deleteJob(sessionID string) { // Start launches the async spawn pipeline and returns the new sessionId. func (s *Spawner) Start(ctx context.Context, repo, branch string) (SpawnResult, error) { - if _, err := s.imageExists(ctx); err != nil { + exists, err := s.imageExists(ctx) + if err != nil { return SpawnResult{}, fmt.Errorf("docker unavailable: %w", err) } - exists, err := s.imageExists(ctx) - if err == nil && !exists { + if !exists { if _, statErr := os.Stat(s.dockerfile); statErr != nil { return SpawnResult{}, fmt.Errorf( "image %s not found and no worker Dockerfile at %s (set %s or run `make worker-image`)", @@ -258,7 +270,7 @@ func (s *Spawner) runJob(repo, branch, sessionID string) { lock.Lock() defer lock.Unlock() - if err := s.cloneOrUpdate(repo, branch, slug); err != nil { + if err := s.cloneOrUpdate(s.ctx, repo, branch, slug); err != nil { s.setJob(sessionID, repo, stateError, "", err.Error()) return } @@ -283,7 +295,7 @@ func (s *Spawner) runJob(repo, branch, sessionID string) { // cloneOrUpdate clones the repo into reposDir/ or fast-forwards it. // Credentials never appear in the clone URL (which git persists into // .git/config); auth is passed per-invocation via http.extraHeader. -func (s *Spawner) cloneOrUpdate(repo, branch, slug string) error { +func (s *Spawner) cloneOrUpdate(ctx context.Context, repo, branch, slug string) error { dir := filepath.Join(s.reposDir, slug) cloneURL, err := s.cloneURL(repo) if err != nil { @@ -291,15 +303,15 @@ func (s *Spawner) cloneOrUpdate(repo, branch, slug string) error { } if st, err := os.Stat(filepath.Join(dir, ".git")); err == nil && st.IsDir() { auth := s.gitAuthArgs() - if branch != "" && gitBranch(dir) != branch { - if err := gitRun(dir, append(append([]string{}, auth...), "fetch", "origin", branch)...); err != nil { + if branch != "" && gitBranch(ctx, dir) != branch { + if err := gitRun(ctx, dir, append(append([]string{}, auth...), "fetch", "origin", branch)...); err != nil { return fmt.Errorf("git fetch %s %s: %w", repo, branch, err) } - if err := gitRun(dir, "checkout", branch); err != nil { + if err := gitRun(ctx, dir, "checkout", branch); err != nil { return fmt.Errorf("git checkout %s: %w", repo, err) } } - if err := gitRun(dir, append(append([]string{}, auth...), "pull", "--ff-only")...); err != nil { + if err := gitRun(ctx, dir, append(append([]string{}, auth...), "pull", "--ff-only")...); err != nil { return fmt.Errorf("git pull %s: %w", repo, err) } return nil @@ -312,7 +324,7 @@ func (s *Spawner) cloneOrUpdate(repo, branch, slug string) error { args = append(args, "--branch", branch) } args = append(args, "--", cloneURL, dir) - if err := gitRun("", args...); err != nil { + if err := gitRun(ctx, "", args...); err != nil { return fmt.Errorf("git clone %s: %w", repo, err) } return nil @@ -338,16 +350,16 @@ func (s *Spawner) gitAuthArgs() []string { } // gitBranch returns the checked-out branch of an existing clone, "" on failure. -func gitBranch(dir string) string { - out, err := exec.Command("git", "-C", dir, "rev-parse", "--abbrev-ref", "HEAD").Output() +func gitBranch(ctx context.Context, dir string) string { + out, err := exec.CommandContext(ctx, "git", "-C", dir, "rev-parse", "--abbrev-ref", "HEAD").Output() if err != nil { return "" } return strings.TrimSpace(string(out)) } -func gitRun(dir string, args ...string) error { - cmd := exec.Command("git", args...) +func gitRun(ctx context.Context, dir string, args ...string) error { + cmd := exec.CommandContext(ctx, "git", args...) if dir != "" { cmd.Dir = dir } @@ -445,7 +457,7 @@ func (s *Spawner) createAndStart(ctx context.Context, repo, slug, sessionID stri return "", err } if fresh { - if err := s.seedVolume(ctx, slug, repoVolume); err != nil { + if err := s.seedVolume(ctx, slug, repoVolume, sessionID); err != nil { // drop the half-seeded volume so the next spawn retries fresh // instead of silently booting into an empty workspace. if rmErr := s.cli.VolumeRemove(context.Background(), repoVolume, true); rmErr != nil { @@ -497,9 +509,14 @@ func (s *Spawner) createAndStart(ctx context.Context, repo, slug, sessionID stri } // ensureRepoVolume creates the per-repo volume; reports whether it is fresh. +// Only a definitive "no such volume" (docker errdefs NotFound) counts as +// fresh: a transient inspect error must never re-seed over an existing +// volume, so it propagates instead. func (s *Spawner) ensureRepoVolume(ctx context.Context, name string) (bool, error) { if _, err := s.cli.VolumeInspect(ctx, name); err == nil { return false, nil + } else if !cerrdefs.IsNotFound(err) { + return false, fmt.Errorf("volume %s: %w", name, err) } if _, err := s.cli.VolumeCreate(ctx, volume.CreateOptions{Name: name}); err != nil { return false, fmt.Errorf("volume %s: %w", name, err) @@ -511,7 +528,7 @@ func (s *Spawner) ensureRepoVolume(ctx context.Context, name string) (bool, erro // streaming a tar of the clone into a paused container via the docker API // (CopyToContainer). No bind mounts: bind sources are HOST paths, but the // clone lives inside the daemon container's filesystem. -func (s *Spawner) seedVolume(ctx context.Context, slug, repoVolume string) error { +func (s *Spawner) seedVolume(ctx context.Context, slug, repoVolume, sessionID string) error { repoDir := filepath.Join(s.reposDir, slug) cfg := &container.Config{ Image: imageRefWorker, @@ -521,7 +538,14 @@ func (s *Spawner) seedVolume(ctx context.Context, slug, repoVolume string) error Binds: []string{repoVolume + ":" + workspaceMount}, Init: &[]bool{true}[0], } - created, err := s.cli.ContainerCreate(ctx, cfg, hostCfg, nil, nil, "lvmh-seed-"+slug) + // Unique name per session: a fixed name would conflict forever after a + // crash between create and the deferred remove. + unq := strings.ReplaceAll(sessionID, "-", "") + if len(unq) > 8 { + unq = unq[:8] + } + name := "lvmh-seed-" + slug + "-" + unq + created, err := s.cli.ContainerCreate(ctx, cfg, hostCfg, nil, nil, name) if err != nil { return err } @@ -554,7 +578,9 @@ func (s *Spawner) removeContainer(ctx context.Context, id string) { } // RemoveSession stops and removes the container spawned for a session. -func (s *Spawner) RemoveSession(ctx context.Context, sessionID string) error { +// Stop and remove run on background contexts on purpose: a caller hanging up +// mid-request must not leave an orphaned container behind a deleted DB row. +func (s *Spawner) RemoveSession(_ context.Context, sessionID string) error { row, ok, err := s.store.GetContainer(sessionID) if err != nil { return err @@ -568,7 +594,7 @@ func (s *Spawner) RemoveSession(ctx context.Context, sessionID string) error { return fmt.Errorf("docker stop: %w", err) } cancel() - s.removeContainer(ctx, row.ContainerID) + s.removeContainer(context.Background(), row.ContainerID) if err := s.store.DeleteContainer(sessionID); err != nil { return err } @@ -577,7 +603,9 @@ func (s *Spawner) RemoveSession(ctx context.Context, sessionID string) error { return nil } -// tarDir writes a tar archive of dir (excluding .git-heavy junk) into w. +// tarDir writes a tar archive of dir (nothing excluded, .git included) into w. +// Symlinks are archived as symlinks with their target in Linkname so docker's +// untar can recreate them; only regular files carry content. func tarDir(w io.Writer, dir string) error { tw := tar.NewWriter(w) defer tw.Close() @@ -593,7 +621,14 @@ func tarDir(w io.Writer, dir string) error { return nil } name := filepath.ToSlash(rel) - hdr, err := tar.FileInfoHeader(info, "") + link := "" + if info.Mode()&os.ModeSymlink != 0 { + link, err = os.Readlink(path) + if err != nil { + return err + } + } + hdr, err := tar.FileInfoHeader(info, link) if err != nil { return err } diff --git a/daemon/docker_test.go b/daemon/docker_test.go index ec5af04..915d9ae 100644 --- a/daemon/docker_test.go +++ b/daemon/docker_test.go @@ -53,17 +53,18 @@ type fakeDocker struct { create []recordedCreate archives []int // sizes of accepted CopyToContainer tar streams - failBuild bool - failBuildHTTP bool - failCreate bool - failStart bool - failStop bool - failWait bool - failVolumeCreate bool - failVolumeDelete bool - failArchive bool - archiveHang bool - waitHang bool + failBuild bool + failBuildHTTP bool + failCreate bool + failStart bool + failStop bool + failWait bool + failVolumeCreate bool + failVolumeDelete bool + failVolumeInspect bool + failArchive bool + archiveHang bool + waitHang bool unknown []string } @@ -203,6 +204,10 @@ func (f *fakeDocker) ServeHTTP(w http.ResponseWriter, r *http.Request) { f.mu.Lock() exists := f.volume[name] f.mu.Unlock() + if f.failVolumeInspect { + writeJSONNow(w, http.StatusInternalServerError, `{"message":"transient docker error"}`) + return + } if exists { writeJSONNow(w, http.StatusOK, fmt.Sprintf(`{"Name":%q,"CreatedAt":"2024-01-01T00:00:00Z"}`, name)) return @@ -288,10 +293,12 @@ const ( fakeGitModeFail string = "fail" fakeGitModeNoisy string = "noisy" fakeGitModeSilent string = "silent" + fakeGitModeHang string = "hang" ) // useFakeGit puts a shim `git` first on PATH. Modes: ok (exit 0), fail -// (stderr + exit 1), noisy (600-byte stderr + exit 1, for truncation tests). +// (stderr + exit 1), noisy (600-byte stderr + exit 1, for truncation tests), +// hang (sleeps; only ctx cancellation can end it). func useFakeGit(t *testing.T, mode string) string { t.Helper() dir := t.TempDir() @@ -302,6 +309,7 @@ func useFakeGit(t *testing.T, mode string) string { " fail) echo 'fatal: repository not found'; exit 1;;\n" + " noisy) printf '%s\\n' \"" + strings.Repeat("x", 600) + "\"; exit 1;;\n" + " silent) exit 1;;\n" + + " hang) exec sleep 30;;\n" + "esac\n" + "# branch probe: emit the configured HEAD name\n" + "if [ \"$1\" = '-C' ] && [ \"$3\" = rev-parse ]; then printf '%s' \"$FAKE_GIT_HEAD\"; exit 0; fi\n" + diff --git a/daemon/go.mod b/daemon/go.mod index 714304a..ced047b 100644 --- a/daemon/go.mod +++ b/daemon/go.mod @@ -3,6 +3,7 @@ module lvmh-daemon go 1.25.0 require ( + github.com/containerd/errdefs v1.0.0 github.com/docker/docker v28.5.2+incompatible github.com/gorilla/websocket v1.5.3 modernc.org/sqlite v1.56.0 @@ -11,7 +12,6 @@ require ( require ( github.com/Microsoft/go-winio v0.6.2 // indirect github.com/cespare/xxhash/v2 v2.3.0 // indirect - github.com/containerd/errdefs v1.0.0 // indirect github.com/containerd/errdefs/pkg v0.3.0 // indirect github.com/containerd/log v0.1.0 // indirect github.com/distribution/reference v0.6.0 // indirect diff --git a/daemon/spawner_test.go b/daemon/spawner_test.go index c7d9ae0..0d031ec 100644 --- a/daemon/spawner_test.go +++ b/daemon/spawner_test.go @@ -6,6 +6,8 @@ import ( "archive/tar" "bytes" "context" + "crypto/sha256" + "encoding/hex" "errors" "fmt" "io" @@ -14,6 +16,7 @@ import ( "os" "path/filepath" "reflect" + "regexp" "strings" "testing" "time" @@ -75,7 +78,7 @@ func TestSpawnerStartHappyPath(t *testing.T) { t.Fatalf("missing env %v", wantEnv) } wantBinds := []string{ - "lvmh-repo-group--project:" + workspaceMount, + volumeRepoPrefix + repoSlug("group/project") + ":" + workspaceMount, volumeSessions + ":" + sessionsMount, volumePiCache + ":" + cacheMount, } @@ -102,7 +105,7 @@ func TestSpawnerStartHappyPath(t *testing.T) { if seed[0].Image != imageRefWorker { t.Fatalf("seed create = %+v", seed[0]) } - wantSeedBinds := []string{"lvmh-repo-group--project:" + workspaceMount} + wantSeedBinds := []string{volumeRepoPrefix + repoSlug("group/project") + ":" + workspaceMount} if !reflect.DeepEqual(seed[0].HostConfig.Binds, wantSeedBinds) { t.Fatalf("seed binds = %v, want %v (no host-path binds)", seed[0].HostConfig.Binds, wantSeedBinds) } @@ -260,7 +263,7 @@ func TestSpawnerCloneOrUpdatePullsExisting(t *testing.T) { if err := os.MkdirAll(filepath.Join(dir, ".git"), 0o755); err != nil { t.Fatalf("mkdir .git: %v", err) } - if err := sp.cloneOrUpdate("group/project", "", repoSlug("group/project")); err != nil { + if err := sp.cloneOrUpdate(context.Background(), "group/project", "", repoSlug("group/project")); err != nil { t.Fatalf("cloneOrUpdate: %v", err) } calls := readGitLog(t, gitLog) @@ -343,7 +346,9 @@ func TestSpawnerSeedVolumeCancel(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() done := make(chan error, 1) - go func() { done <- sp.seedVolume(ctx, repoSlug("group/project"), "lvmh-repo-group--project") }() + go func() { + done <- sp.seedVolume(ctx, repoSlug("group/project"), volumeRepoPrefix+repoSlug("group/project"), "s1") + }() waitFor(t, 5*time.Second, func() bool { return f.hasCallSuffix(http.MethodPut, "/archive") }) cancel() select { @@ -386,12 +391,24 @@ func TestSpawnerSameRepoSpawnsSerialize(t *testing.T) { } func TestRepoSlugAndUUID(t *testing.T) { - if got := repoSlug("a/b/c"); got != "a--b--c" { - t.Fatalf("repoSlug = %q", got) + slugRe := regexp.MustCompile(`^a--b--c-[0-9a-f]{6}$`) + if got := repoSlug("a/b/c"); !slugRe.MatchString(got) { + t.Fatalf("repoSlug = %q, want a--b--c-<6 hex>", got) } - // "a/b/c" and "a/b-c" must map to distinct slugs (no collision). - if repoSlug("a/b/c") == repoSlug("a/b-c") { - t.Fatalf("slug collision: %q", repoSlug("a/b/c")) + // sanitized path + first 6 hex of sha256(full repo path) + sum := sha256.Sum256([]byte("a/b/c")) + want := "a--b--c-" + hex.EncodeToString(sum[:3]) + if got := repoSlug("a/b/c"); got != want { + t.Fatalf("repoSlug = %q, want %q", got, want) + } + // "/"→"--" alone is ambiguous: distinct repo paths must never collide. + seen := map[string]bool{} + for _, repo := range []string{"a/b/c", "a/b-c", "a/b--c", "a/b--c/d"} { + s := repoSlug(repo) + if seen[s] { + t.Fatalf("slug collision: %q", s) + } + seen[s] = true } id := newUUID() if len(id) != 36 || id[8] != '-' || id[13] != '-' || id[18] != '-' || id[23] != '-' { @@ -473,10 +490,10 @@ func TestSpawnerFailedSeedRemovesRepoVolume(t *testing.T) { if !strings.Contains(job.Message, "seed") { t.Fatalf("job message = %q, want seed failure", job.Message) } - if !f.hasCall(http.MethodDelete, "/volumes/lvmh-repo-group--project") { + if !f.hasCall(http.MethodDelete, "/volumes/"+volumeRepoPrefix+repoSlug("group/project")) { t.Fatal("failed seed must force-remove the repo volume for a fresh retry") } - if f.volumeExists("lvmh-repo-group--project") { + if f.volumeExists(volumeRepoPrefix + repoSlug("group/project")) { t.Fatal("repo volume must not linger half-seeded") } } @@ -506,7 +523,7 @@ func TestSpawnerCloneOrUpdateBranchFetchFails(t *testing.T) { if err := os.MkdirAll(filepath.Join(sp.reposDir, slug, ".git"), 0o755); err != nil { t.Fatalf("mkdir .git: %v", err) } - err := sp.cloneOrUpdate("group/project", "dev", slug) + err := sp.cloneOrUpdate(context.Background(), "group/project", "dev", slug) if err == nil || !strings.Contains(err.Error(), "git fetch") { t.Fatalf("cloneOrUpdate branch fetch failure = %v, want git fetch error", err) } @@ -564,7 +581,7 @@ func TestSpawnerCloneOrUpdateSwitchesBranch(t *testing.T) { if err := os.MkdirAll(filepath.Join(dir, ".git"), 0o755); err != nil { t.Fatalf("mkdir .git: %v", err) } - if err := sp.cloneOrUpdate("group/project", tc.branch, slug); err != nil { + if err := sp.cloneOrUpdate(context.Background(), "group/project", tc.branch, slug); err != nil { t.Fatalf("cloneOrUpdate: %v", err) } calls := readGitLog(t, gitLog) @@ -674,7 +691,7 @@ func TestTarDir(t *testing.T) { func TestGitRunSilentFailure(t *testing.T) { useFakeGit(t, fakeGitModeSilent) - err := gitRun("", "clone", "x") + err := gitRun(context.Background(), "", "clone", "x") if err == nil || strings.Contains(err.Error(), "clone x") { // silent failure surfaces the bare exec error, not a padded message t.Fatalf("gitRun silent failure = %v", err) @@ -723,7 +740,7 @@ func TestSpawnerCloneOrUpdateErrors(t *testing.T) { sp, _ := newTestSpawner(t, f) // unparseable repo path (control char) → cloneURL parse error - if err := sp.cloneOrUpdate("bad\nrepo", "", repoSlug("bad\nrepo")); err == nil { + if err := sp.cloneOrUpdate(context.Background(), "bad\nrepo", "", repoSlug("bad\nrepo")); err == nil { t.Fatal("cloneOrUpdate with control-char repo must fail") } @@ -733,7 +750,7 @@ func TestSpawnerCloneOrUpdateErrors(t *testing.T) { t.Fatalf("write file: %v", err) } sp.reposDir = file - if err := sp.cloneOrUpdate("group/project", "", repoSlug("group/project")); err == nil { + if err := sp.cloneOrUpdate(context.Background(), "group/project", "", repoSlug("group/project")); err == nil { t.Fatal("cloneOrUpdate with file reposDir must fail") } } @@ -770,7 +787,7 @@ func TestSpawnerEnsureImageErrors(t *testing.T) { func TestSpawnerSessionsVolumeCreateFails(t *testing.T) { useFakeGit(t, fakeGitModeOK) f := newFakeDocker() - f.volume["lvmh-repo-group--project"] = true // repo volume exists → skip seed + f.volume[volumeRepoPrefix+repoSlug("group/project")] = true // repo volume exists → skip seed f.failVolumeCreate = true sp, _ := newTestSpawner(t, f) res, err := sp.Start(context.Background(), "group/project", "") @@ -790,12 +807,12 @@ func TestSpawnerSeedVolumeCreateStartFail(t *testing.T) { ctx := context.Background() f.failCreate = true - if err := sp.seedVolume(ctx, repoSlug("group/project"), "lvmh-repo-group--project"); err == nil { + if err := sp.seedVolume(ctx, repoSlug("group/project"), volumeRepoPrefix+repoSlug("group/project"), "s1"); err == nil { t.Fatal("seedVolume with failing create must fail") } f.failCreate = false f.failStart = true - if err := sp.seedVolume(ctx, repoSlug("group/project"), "lvmh-repo-group--project"); err == nil { + if err := sp.seedVolume(ctx, repoSlug("group/project"), volumeRepoPrefix+repoSlug("group/project"), "s1"); err == nil { t.Fatal("seedVolume with failing start must fail") } } @@ -836,7 +853,7 @@ func TestSpawnerCloneOrUpdatePullFails(t *testing.T) { if err := os.MkdirAll(filepath.Join(dir, ".git"), 0o755); err != nil { t.Fatalf("mkdir .git: %v", err) } - err := sp.cloneOrUpdate("group/project", "", repoSlug("group/project")) + err := sp.cloneOrUpdate(context.Background(), "group/project", "", repoSlug("group/project")) if err == nil || !strings.Contains(err.Error(), "git pull") { t.Fatalf("cloneOrUpdate pull failure = %v, want git pull error", err) } @@ -847,7 +864,7 @@ func TestSpawnerWorkerCreateStartFailures(t *testing.T) { // worker container create/start instead of at the seed container. useFakeGit(t, fakeGitModeOK) f := newFakeDocker() - f.volume["lvmh-repo-group--project"] = true + f.volume[volumeRepoPrefix+repoSlug("group/project")] = true sp, _ := newTestSpawner(t, f) f.failCreate = true @@ -959,11 +976,214 @@ func TestTarDirUnreadableFile(t *testing.T) { func TestGitRunUnderivableExit(t *testing.T) { // gitRun is the exec seam: with real git present, invoking a nonexistent // subcommand must surface stderr, and trimming applies to long output. - if err := gitRun("", "version"); err != nil { + if err := gitRun(context.Background(), "", "version"); err != nil { t.Fatalf("git version: %v", err) } - err := gitRun("", "this-subcommand-does-not-exist") + err := gitRun(context.Background(), "", "this-subcommand-does-not-exist") if err == nil || !strings.Contains(err.Error(), "this-subcommand-does-not-exist") { t.Fatalf("gitRun unknown subcommand = %v", err) } } + +// --- round-2 regression tests --- + +// TestTarDirSymlinks pins the symlink contract: headers carry TypeSymlink, +// the real target in Linkname, and no content body (Size 0). Docker's untar +// recreates symlinks via os.Symlink(Linkname, path); an empty Linkname made +// every symlink-bearing repo fail to seed permanently. +func TestTarDirSymlinks(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "file.txt"), []byte("content"), 0o644); err != nil { + t.Fatalf("write: %v", err) + } + if err := os.Symlink("file.txt", filepath.Join(dir, "rel-link")); err != nil { + t.Fatalf("rel symlink: %v", err) + } + if err := os.Symlink(filepath.Join(dir, "file.txt"), filepath.Join(dir, "abs-link")); err != nil { + t.Fatalf("abs symlink: %v", err) + } + + var buf bytes.Buffer + if err := tarDir(&buf, dir); err != nil { + t.Fatalf("tarDir: %v", err) + } + type linkInfo struct { + linkname string + size int64 + } + links := map[string]linkInfo{} + regulars := 0 + tr := tar.NewReader(bytes.NewReader(buf.Bytes())) + for { + hdr, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + t.Fatalf("tar next: %v", err) + } + if hdr.Typeflag == tar.TypeSymlink { + links[hdr.Name] = linkInfo{hdr.Linkname, hdr.Size} + continue + } + if hdr.Typeflag == tar.TypeReg { + regulars++ + } + } + if regulars != 1 { + t.Fatalf("regular files = %d, want 1", regulars) + } + want := map[string]string{ + "rel-link": "file.txt", + "abs-link": filepath.Join(dir, "file.txt"), + } + for name, target := range want { + got, ok := links[name] + if !ok { + t.Fatalf("symlink %q missing from archive (have %v)", name, links) + } + if got.linkname != target { + t.Fatalf("symlink %q Linkname = %q, want %q", name, got.linkname, target) + } + if got.size != 0 { + t.Fatalf("symlink %q has Size %d, want 0 (no content body)", name, got.size) + } + } +} + +// TestSpawnerRemoveSessionSurvivesClientHangup: a cancelled request context +// (client hangup) must not skip the container remove — the DB row is deleted +// either way, so a skipped remove would orphan the container forever. +func TestSpawnerRemoveSessionSurvivesClientHangup(t *testing.T) { + useFakeGit(t, fakeGitModeOK) + f := newFakeDocker() + sp, store := newTestSpawner(t, f) + + if err := store.UpsertContainer("s1", "cid-9", "group/project"); err != nil { + t.Fatalf("upsert container: %v", err) + } + ctx, cancel := context.WithCancel(context.Background()) + cancel() // client already gone + if err := sp.RemoveSession(ctx, "s1"); err != nil { + t.Fatalf("RemoveSession with cancelled ctx: %v", err) + } + if !f.hasCall(http.MethodDelete, "/containers/cid-9") { + t.Fatal("remove must run on a background ctx despite the cancelled request ctx") + } + if _, ok, _ := store.GetContainer("s1"); ok { + t.Fatal("container row must be deleted") + } + f.assertNoUnknown(t) +} + +// TestSpawnerSeedVolumeUniqueNames: two seeds for the same repo must not +// reuse one container name — a crash between create and the deferred remove +// would then block every future spawn with a name conflict. +func TestSpawnerSeedVolumeUniqueNames(t *testing.T) { + useFakeGit(t, fakeGitModeOK) + f := newFakeDocker() + sp, _ := newTestSpawner(t, f) + + slug := repoSlug("group/project") + repoVolume := volumeRepoPrefix + slug + if err := os.MkdirAll(filepath.Join(sp.reposDir, slug), 0o755); err != nil { + t.Fatalf("mkdir clone dir: %v", err) + } + if err := os.WriteFile(filepath.Join(sp.reposDir, slug, "README.md"), []byte("x"), 0o644); err != nil { + t.Fatalf("write README: %v", err) + } + for _, sid := range []string{"aaaaaaaa-1111-2222-3333-444444444444", "bbbbbbbb-1111-2222-3333-444444444444"} { + if err := sp.seedVolume(context.Background(), slug, repoVolume, sid); err != nil { + t.Fatalf("seedVolume(%s): %v", sid, err) + } + } + seeds := f.createsByName("lvmh-seed-") + if len(seeds) != 2 { + t.Fatalf("seed containers = %+v, want 2", seeds) + } + if seeds[0].Name == seeds[1].Name { + t.Fatalf("seed names must differ per session: %q", seeds[0].Name) + } + for _, s := range seeds { + if !strings.HasPrefix(s.Name, "lvmh-seed-"+slug+"-") { + t.Fatalf("seed name %q, want prefix lvmh-seed-%s-", s.Name, slug) + } + } + f.assertNoUnknown(t) +} + +// TestSpawnerEnsureRepoVolumeTransientInspectError: a non-NotFound volume +// inspect error must propagate, never be treated as "fresh" (which would +// seed over an existing volume). +func TestSpawnerEnsureRepoVolumeTransientInspectError(t *testing.T) { + f := newFakeDocker() + f.failVolumeInspect = true + f.volume[volumeRepoPrefix+repoSlug("group/project")] = true // volume exists + sp, _ := newTestSpawner(t, f) + + fresh, err := sp.ensureRepoVolume(context.Background(), volumeRepoPrefix+repoSlug("group/project")) + if err == nil || !strings.Contains(err.Error(), "transient docker error") { + t.Fatalf("ensureRepoVolume = fresh:%v err:%v, want transient error propagated", fresh, err) + } + if fresh { + t.Fatal("transient inspect error must never report a fresh volume") + } + if f.countCalls(http.MethodPost, "/volumes/create") != 0 { + t.Fatal("no volume create may follow a transient inspect error") + } + f.assertNoUnknown(t) +} + +// TestSpawnerCloneOrUpdateRespectsCtx: git spawned via CommandContext must +// die when the context is cancelled (daemon shutdown must not hang on git). +func TestSpawnerCloneOrUpdateRespectsCtx(t *testing.T) { + useFakeGit(t, fakeGitModeHang) // clone sleeps 30s + f := newFakeDocker() + sp, _ := newTestSpawner(t, f) + + ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond) + defer cancel() + start := time.Now() + err := sp.cloneOrUpdate(ctx, "group/project", "", repoSlug("group/project")) + if err == nil { + t.Fatal("cloneOrUpdate must fail when ctx expires mid-clone") + } + if elapsed := time.Since(start); elapsed > 5*time.Second { + t.Fatalf("cloneOrUpdate took %v, want prompt cancellation", elapsed) + } + if ctx.Err() == nil { + t.Fatal("expected the context to be the failure cause") + } +} + +// TestGitBranchRespectsCtx: the branch probe must honour cancellation too. +func TestGitBranchRespectsCtx(t *testing.T) { + useFakeGit(t, fakeGitModeHang) + ctx, cancel := context.WithTimeout(context.Background(), 300*time.Millisecond) + defer cancel() + start := time.Now() + if got := gitBranch(ctx, t.TempDir()); got != "" { + t.Fatalf("gitBranch under cancelled ctx = %q, want \"\"", got) + } + if elapsed := time.Since(start); elapsed > 5*time.Second { + t.Fatalf("gitBranch took %v, want prompt cancellation", elapsed) + } +} + +// TestSpawnerStartDedupesImageList: Start must list images exactly once +// (plus once more inside ensureImage) — a regression guard for the removed +// double imageExists call. +func TestSpawnerStartDedupesImageList(t *testing.T) { + useFakeGit(t, fakeGitModeOK) + f := newFakeDocker() + sp, _ := newTestSpawner(t, f) + res, err := sp.Start(context.Background(), "group/project", "") + if err != nil { + t.Fatalf("Start: %v", err) + } + waitJobState(t, sp, res.SessionID, stateRunning) + if n := f.countCalls(http.MethodGet, "/images/json"); n != 2 { + t.Fatalf("image list calls = %d, want 2 (Start + ensureImage)", n) + } + f.assertNoUnknown(t) +} diff --git a/plugin/lvmh-agent.ts b/plugin/lvmh-agent.ts index 7b59167..7dc5e87 100644 --- a/plugin/lvmh-agent.ts +++ b/plugin/lvmh-agent.ts @@ -504,9 +504,7 @@ export default function (pi: ExtensionAPI): void { ? (maxAssignedSeq.get(currentSessionId) ?? 0) : 0; if (lastSeq > maxAssigned) { - const covered: number = replayBuf.filter( - (f) => f.seq <= lastSeq, - ).length; + const covered: number = replayBuf.filter((f) => f.seq <= lastSeq).length; if (covered > 0) { droppedEvents += covered; log( diff --git a/web/src/App.test.tsx b/web/src/App.test.tsx index 1025e69..6e238ab 100644 --- a/web/src/App.test.tsx +++ b/web/src/App.test.tsx @@ -102,6 +102,24 @@ describe("App gate", () => { ); await screen.findByLabelText("Sessions"); }); + + it("first-run: saving settings starts the ws manager (A)", async () => { + seedApi(); + renderApp(); + fireEvent.input(screen.getByLabelText("Bearer token"), { + target: { value: "tok" }, + }); + fireEvent.submit( + screen + .getByRole("button", { name: "Connect" }) + .closest("form") as HTMLFormElement, + ); + // before the fix the effect deps never changed after the gate save, + // so no manager ever started and the app stayed on the connecting banner + await screen.findByLabelText("Sessions"); + await waitFor(() => expect(FakeWebSocket.instances.length).toBe(1)); + await screen.findByText(/reconnecting/i); + }); }); describe("App shell", () => { @@ -113,9 +131,7 @@ describe("App shell", () => { await waitFor(() => expect(FakeWebSocket.instances.length).toBe(1)); const sock = FakeWebSocket.last(); act(() => sock.serverOpen()); - expect( - screen.getByTitle("ws open"), - ).toBeInTheDocument(); + expect(screen.getByTitle("ws open")).toBeInTheDocument(); const sidebar = screen.getByLabelText("Sessions"); const links = Array.from(sidebar.querySelectorAll("a.session-link")).map( @@ -229,7 +245,9 @@ describe("ErrorBoundary direct", () => { function Bomb(): React.ReactNode { throw new Error("kaboom-ui"); } - const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => undefined); + const consoleSpy = vi + .spyOn(console, "error") + .mockImplementation(() => undefined); function Harness(): React.ReactNode { return ( diff --git a/web/src/App.tsx b/web/src/App.tsx index 0bccb99..1c6ffe5 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -88,7 +88,7 @@ export default function App() { const [configured, setConfigured] = useState(getSettings() !== null); const [sidebarOpen, setSidebarOpen] = useState(false); const { toasts, push } = useToasts(); - const store = useSessions(push); + const store = useSessions(push, configured); const location = useLocation(); useEffect(() => { diff --git a/web/src/ChatStream.test.tsx b/web/src/ChatStream.test.tsx index f64baa4..0522070 100644 --- a/web/src/ChatStream.test.tsx +++ b/web/src/ChatStream.test.tsx @@ -1,483 +1,410 @@ import { act, fireEvent, render, screen } from "@testing-library/react"; -import { MemoryRouter, Route, Routes } from "react-router-dom"; -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import type { Repo, SessionListItem } from "./protocol"; -import { Route as ApiRoute } from "./protocol"; -import { fetchJson } from "./api"; -import SpawnView from "./SpawnView"; -import type { SessionsStore } from "./store"; -import { jsonResponse, mockFetchJson, seedSettings } from "./test/setup"; +import userEvent from "@testing-library/user-event"; +import { describe, expect, it } from "vitest"; +import type { ChatMessage, ToolState } from "./derive"; +import ChatStream, { Bubble, TypingIndicator } from "./ChatStream"; -const repo = (path: string, branch = "main"): Repo => ({ - path, - name: path.split("/")[1] ?? path, - namespace: path.split("/")[0] ?? "g", - lastActivityAt: "2024-05-01T00:00:00Z", - webUrl: `https://gl/${path}`, - defaultBranch: branch, -}); - -const PUSH_TOAST = vi.fn(); - -function makeStore(over: Partial = {}): SessionsStore { +function msg(partial: Partial): ChatMessage { return { - sessions: [], - state: "open", - spawnJobs: [], - refresh: async (): Promise => - fetchJson(ApiRoute.Sessions), - subscribe: (): (() => void) => () => undefined, - ...over, + key: `k-${Math.random()}`, + role: "assistant", + text: "", + thinking: null, + toolCalls: [], + toolCallId: null, + streaming: false, + ...partial, }; } -function tree(store: SessionsStore): React.ReactElement { - return ( - - - } - /> - } /> - - - ); -} - -/** Flush pending microtasks + React effects (works under fake timers). */ -async function flush(ticks = 4): Promise { - for (let i = 0; i < ticks; i += 1) { - // eslint-disable-next-line no-await-in-loop - await act(async () => { - await Promise.resolve(); - }); - } -} - -beforeEach(() => { - PUSH_TOAST.mockClear(); - seedSettings(); +const tool = (p: Partial): ToolState => ({ + id: "c1", + name: "bash", + args: "", + running: false, + isError: false, + preview: "", + ...p, }); -afterEach(() => { - vi.restoreAllMocks(); -}); +describe("Bubble", () => { + it("renders plain text per role class", () => { + const { container } = render( + , + ); + expect(container.querySelector(".bubble-row.user")).not.toBeNull(); + expect(container.textContent).toContain("hi there"); + }); + + it("renders empty assistant text as nothing but shows nothing when empty", () => { + const { container } = render( + , + ); + expect(container.querySelector(".bubble")?.children).toHaveLength(0); + }); + + it("toolResult collapses to a one-line preview, expands to full text", async () => { + const long = `${"x".repeat(200)}`; + render( + , + ); + const details = screen + .getByText("result") + .closest("details") as HTMLDetailsElement; + expect(details.open).toBe(false); + expect(details.textContent).toContain("…"); + await userEvent.click(screen.getByText("result")); + expect(details.open).toBe(true); + expect(details.textContent).toContain(long); + }); -describe("SpawnView status", () => { - it("shows checking state, then error when gitlab status fails", async () => { - mockFetchJson((url) => { - if (url.includes("/api/gitlab/status")) - return jsonResponse({ error: "down" }, 500); - return []; - }); - render(tree(makeStore())); - expect(screen.getByText("checking gitlab…")).toBeInTheDocument(); - await flush(); - expect(screen.getByText(/gitlab status failed: down/)).toBeInTheDocument(); + it("toolResult with short text keeps full one-line preview", () => { + render( + , + ); + const details = screen + .getByText("result") + .closest("details") as HTMLDetailsElement; + expect(details.textContent).toContain("short out"); + expect(details.textContent).not.toContain("…"); }); -}); -describe("SpawnView connect flow", () => { - it("requires a token", async () => { - mockFetchJson((url) => { - if (url.endsWith("/api/gitlab/status")) - return { connected: false, baseUrl: "https://gl" }; - return []; - }); - render(tree(makeStore())); - await flush(); - fireEvent.click(screen.getByRole("button", { name: "Connect" })); - await flush(); - expect(screen.getByText("token required")).toBeInTheDocument(); + it("flattens whitespace in previews", () => { + render( + , + ); + expect(screen.getAllByText("a b c").length).toBeGreaterThan(0); }); + + it("assistant tool calls attach tool cards", async () => { + const tools = new Map([ + [ + "c1", + tool({ + id: "c1", + name: "bash", + args: "ls -la", + running: false, + isError: false, + preview: "file", + }), + ], + ]); + render( + , + ); + expect(screen.getByText("🛠 bash")).toBeInTheDocument(); + expect(screen.getByText("finished")).toBeInTheDocument(); - it("connects, clears the PAT, loads repos and shows the picker", async () => { - let connected = false; - mockFetchJson((url, init) => { - if (url.endsWith("/api/gitlab/status")) - return { - connected, - baseUrl: "https://gl", - username: connected ? "alice" : undefined, - }; - if (url.endsWith("/api/gitlab/connect") && init?.method === "POST") { - connected = true; - return { username: "alice" }; - } - if (url.endsWith("/api/gitlab/repos")) - return [repo("g/one"), repo("g/two")]; - return []; - }); - render(tree(makeStore())); - await flush(); - expect(screen.getByText("Connect GitLab")).toBeInTheDocument(); - - const pat = screen.getByLabelText( - "GitLab personal access token", - ) as HTMLInputElement; - fireEvent.input(pat, { target: { value: "glpat-x" } }); - fireEvent.click(screen.getByRole("button", { name: "Connect" })); - await flush(); - - expect(screen.getByText("g/one")).toBeInTheDocument(); - expect(screen.getByText("g/two")).toBeInTheDocument(); - expect(screen.queryByLabelText("GitLab personal access token")).toBeNull(); - expect(screen.getByText("Repository")).toBeInTheDocument(); + const summary = screen + .getByText("🛠 bash") + .closest("summary") as HTMLElement; + const card = summary.closest("details") as HTMLDetailsElement; + expect(card.open).toBe(false); + await userEvent.click(summary); + expect(card.open).toBe(true); + expect(card.textContent).toContain("ls -la"); + expect(card.textContent).toContain("file"); }); - it("connect failure shows the error and keeps the gate", async () => { - mockFetchJson((url, init) => { - if (url.endsWith("/api/gitlab/status")) - return { connected: false, baseUrl: "https://gl" }; - if (url.endsWith("/api/gitlab/connect") && init?.method === "POST") - return jsonResponse({ error: "bad pat" }, 401); - return []; - }); - render(tree(makeStore())); - await flush(); - fireEvent.input(screen.getByLabelText("GitLab personal access token"), { - target: { value: "glpat-bad" }, - }); - fireEvent.click(screen.getByRole("button", { name: "Connect" })); - await flush(); - expect(screen.getByText("bad pat")).toBeInTheDocument(); - expect(screen.getByText("Connect GitLab")).toBeInTheDocument(); + it("tool card status variants: running, error, done", () => { + const tools = new Map([ + ["c1", tool({ id: "c1", running: true })], + ["c2", tool({ id: "c2", running: false, isError: true })], + ["c3", tool({ id: "c3", running: false, isError: false })], + ]); + render( + ({ + id, + name: `t-${id}`, + argsJson: "{}", + })), + })} + tools={tools} + />, + ); + expect(screen.getByText("working…")).toBeInTheDocument(); + expect(screen.getByText("error")).toBeInTheDocument(); + expect(screen.getByText("done")).toBeInTheDocument(); }); - it("connected on load fetches repos immediately", async () => { - mockFetchJson((url) => { - if (url.endsWith("/api/gitlab/status")) - return { connected: true, baseUrl: "https://gl", username: "alice" }; - if (url.endsWith("/api/gitlab/repos")) return [repo("g/quick")]; - return []; - }); - render(tree(makeStore())); - await flush(); - expect(screen.getByText("g/quick")).toBeInTheDocument(); + it("tool call with no matching state renders no card", () => { + const { container } = render( + , + ); + expect(container.querySelectorAll(".tool-card")).toHaveLength(0); }); -}); -describe("SpawnView repo picker", () => { - function connectedMock(): void { - mockFetchJson((url) => { - if (url.endsWith("/api/gitlab/status")) - return { connected: true, baseUrl: "https://gl", username: "alice" }; - if (url.endsWith("/api/gitlab/repos")) - return [repo("g/alpha"), repo("g/beta", "dev")]; - return []; - }); - } + it("thinking block only for non-empty thinking", async () => { + render(); + const details = screen + .getByText("thinking") + .closest("details") as HTMLDetailsElement; + await userEvent.click(screen.getByText("thinking")); + expect(details.open).toBe(true); + expect(details.textContent).toContain("because"); - it("filters repos by query, keyboard selects, empty filter message", async () => { - connectedMock(); - render(tree(makeStore())); - await flush(); - expect(screen.getByText("g/alpha")).toBeInTheDocument(); - - const filter = screen.getByLabelText("Filter repositories"); - fireEvent.input(filter, { target: { value: "beta" } }); - expect(screen.queryByText("g/alpha")).toBeNull(); - expect(screen.getByText("g/beta")).toBeInTheDocument(); - - const item = screen - .getByText("g/beta") - .closest(".repo-item") as HTMLElement; - fireEvent.keyDown(item, { key: "Enter" }); - expect(screen.getByLabelText("Branch")).toHaveValue("dev"); - - fireEvent.input(filter, { target: { value: "zzz" } }); - expect(screen.getByText("no matching repos")).toBeInTheDocument(); + const { container } = render( + , + ); + expect(container.querySelector(".thinking")).toBeNull(); }); - it("shows loading repos while the list is pending", async () => { - const gate = { resolve: null as ((v: unknown) => void) | null }; - mockFetchJson((url) => { - if (url.endsWith("/api/gitlab/status")) - return { connected: true, baseUrl: "https://gl", username: "alice" }; - if (url.endsWith("/api/gitlab/repos")) - return new Promise((res) => { - gate.resolve = res; - }); - return []; - }); - render(tree(makeStore())); - await flush(); - expect(screen.getByText("loading repos…")).toBeInTheDocument(); - gate.resolve?.([repo("g/late")]); - await flush(); - expect(await screen.findByText("g/late")).toBeInTheDocument(); + it("streaming bubble shows the caret", () => { + const { container } = render( + , + ); + expect(container.querySelector(".stream-caret")).not.toBeNull(); }); +}); - it("spawn without selection shows pick-a-repo error", async () => { - connectedMock(); - render(tree(makeStore())); - await flush(); - expect(screen.getByLabelText("Spawn container")).toBeDisabled(); - expect(screen.getByText("select a repo above")).toBeInTheDocument(); +describe("TypingIndicator", () => { + it("renders three dots with aria-live", () => { + const { container } = render(); + expect(container.querySelector('[aria-live="polite"]')).not.toBeNull(); + expect(container.querySelectorAll(".dot")).toHaveLength(3); }); }); + +describe("ChatStream", () => { + function stream(p: { + messages: ChatMessage[]; + busy: boolean; + hasOlder?: boolean; + loadingOlder?: boolean; + onOlder?: () => void; + }): React.ReactElement { + return ( + undefined)} + /> + ); + } + + it("renders messages and typing indicator while busy with no open stream", () => { + const { container, rerender } = render( + stream({ messages: [msg({ key: "a", text: "one" })], busy: true }), + ); + expect(container.querySelector(".typing")).not.toBeNull(); -describe("SpawnView spawn+poll", () => { - beforeEach(() => { - vi.useFakeTimers(); + rerender( + stream({ + messages: [ + msg({ key: "a", text: "one" }), + msg({ key: "b", streaming: true }), + ], + busy: true, + }), + ); + expect(container.querySelector(".typing")).toBeNull(); + rerender( + stream({ messages: [msg({ key: "a", text: "one" })], busy: false }), + ); + expect(container.querySelector(".typing")).toBeNull(); }); - it("spawns, polls until online, then navigates to the chat", async () => { - const posts: Array<[string, RequestInit | undefined]> = []; - let sessionsOnline = false; // flipped after the first poll tick - const fetchMock = mockFetchJson((url, init) => { - if (init?.method === "POST" && url.endsWith("/api/spawn")) { - posts.push([url, init]); - return { sessionId: "new-1", containerId: "abc123def456" }; - } - if (url.endsWith("/api/sessions")) - return sessionsOnline - ? [ - { - id: "new-1", - name: "spawned", - cwd: "/w", - model: "m", - provider: "p", - agent: true, - repo: "g/proj", - startedAt: 1, - online: true, - lastEventAt: 1, - }, - ] - : []; - if (url.endsWith("/api/spawn/status")) return []; - if (url.endsWith("/api/gitlab/status")) - return { connected: true, baseUrl: "https://gl", username: "alice" }; - if (url.endsWith("/api/gitlab/repos")) return [repo("g/proj")]; - return []; + it("sticks to bottom on new messages, un-pins on user scroll up, re-pins near bottom", () => { + const { container, rerender } = render( + stream({ messages: [msg({ key: "a" })], busy: false }), + ); + const scroller = container.querySelector(".chat-scroll") as HTMLElement; + Object.defineProperty(scroller, "scrollHeight", { + configurable: true, + value: 1000, }); - const store = makeStore(); - const { unmount } = render(tree(store)); - await flush(); - fireEvent.click(screen.getByText("g/proj")); - fireEvent.click(screen.getByLabelText("Spawn container")); - await flush(); - - expect(posts).toHaveLength(1); - expect((posts[0]?.[1] as RequestInit).body).toBe( - JSON.stringify({ repo: "g/proj", branch: "main" }), + Object.defineProperty(scroller, "clientHeight", { + configurable: true, + value: 300, + }); + + scroller.dispatchEvent(new Event("scroll")); + // pinned: scrollTop at bottom + scroller.scrollTop = 700; + Object.defineProperty(scroller, "scrollTop", { + configurable: true, + writable: true, + value: 700, + }); + scroller.dispatchEvent(new Event("scroll")); + rerender( + stream({ + messages: [msg({ key: "a" }), msg({ key: "b" })], + busy: false, + }), + ); + expect(scroller.scrollTop).toBe(1000); + + // scroll far up -> unpin + Object.defineProperty(scroller, "scrollTop", { + configurable: true, + writable: true, + value: 0, + }); + scroller.dispatchEvent(new Event("scroll")); + rerender( + stream({ + messages: [msg({ key: "a" }), msg({ key: "b" }), msg({ key: "c" })], + busy: false, + }), + ); + expect(scroller.scrollTop).toBe(0); + + // scroll near bottom (within 80px) -> pinned again + Object.defineProperty(scroller, "scrollTop", { + configurable: true, + writable: true, + value: 940, + }); + scroller.dispatchEvent(new Event("scroll")); + rerender( + stream({ + messages: [ + msg({ key: "a" }), + msg({ key: "b" }), + msg({ key: "c" }), + msg({ key: "d" }), + ], + busy: false, + }), + ); + expect(scroller.scrollTop).toBe(1000); + }); + + it("load-older button renders only when an older page exists and fires on click", () => { + const onOlder = vi.fn(); + const { rerender } = render( + stream({ messages: [msg({ key: "a" })], busy: false }), ); - expect(screen.getByText("Spawning…")).toBeInTheDocument(); - expect(screen.getByText(/container abc123def456/)).toBeInTheDocument(); expect( - screen.getByText("waiting for session to come online…"), - ).toBeInTheDocument(); + screen.queryByRole("button", { name: "Load older messages" }), + ).toBeNull(); - // first tick: session not online yet - await act(async () => { - vi.advanceTimersByTime(1500); - }); - await flush(); - expect(screen.queryByTestId("chat-route")).toBeNull(); + rerender( + stream({ + messages: [msg({ key: "a" })], + busy: false, + hasOlder: true, + onOlder, + }), + ); + const btn = screen.getByRole("button", { name: "Load older messages" }); + fireEvent.click(btn); + expect(onOlder).toHaveBeenCalledTimes(1); - // each poll tick issues exactly one sessions fetch (refresh reuse, S7) - const sessionsFetches = fetchMock.mock.calls.filter(([u]) => - String(u).endsWith("/api/sessions"), - ).length; - expect(sessionsFetches).toBe(1); - - // session comes online -> next tick navigates - sessionsOnline = true; - await act(async () => { - vi.advanceTimersByTime(1500); - }); - await flush(); - expect(screen.getByTestId("chat-route")).toBeInTheDocument(); - unmount(); + rerender( + stream({ + messages: [msg({ key: "a" })], + busy: false, + hasOlder: true, + loadingOlder: true, + onOlder, + }), + ); + expect( + screen.getByRole("button", { name: "Load older messages" }), + ).toBeDisabled(); }); +}); - it("spawning view shows job line from status and spawn error text", async () => { - mockFetchJson((url, init) => { - if (init?.method === "POST" && url.endsWith("/api/spawn")) - return { sessionId: "sx", containerId: "" }; - if (url.endsWith("/api/gitlab/status")) - return { connected: true, baseUrl: "https://gl", username: "alice" }; - if (url.endsWith("/api/gitlab/repos")) return [repo("g/p")]; - return []; - }); - const store = makeStore({ - spawnJobs: [ - { sessionId: "sx", repo: "g/p", state: "cloning", containerId: "" }, - ], - }); - render(tree(store)); - await flush(); - fireEvent.click(screen.getByText("g/p")); - fireEvent.click(screen.getByLabelText("Spawn container")); - await flush(); - expect(screen.getByText("Spawning…")).toBeInTheDocument(); - expect(screen.getByText("g/p: cloning")).toBeInTheDocument(); - }); +describe("copy button", () => { + it("copy button writes message text and flashes copied", async () => { + vi.useFakeTimers(); + const writeText = vi.fn(() => Promise.resolve()); + Object.assign(navigator, { clipboard: { writeText } }); + render( + , + ); + const btn = screen.getByRole("button", { name: "Copy message" }); + fireEvent.click(btn); + expect(writeText).toHaveBeenCalledWith("copy me"); + await vi.waitFor(() => + expect(screen.getByText("copied")).toBeInTheDocument(), + ); - it("branch left blank sends repo only; poll refresh failure toasts", async () => { - const bodies: string[] = []; - mockFetchJson((url, init) => { - if (init?.method === "POST" && url.endsWith("/api/spawn")) { - bodies.push(String(init.body)); - return { sessionId: "new-2", containerId: "cccccccccccc" }; - } - if (url.endsWith("/api/spawn/status")) return []; - if (url.endsWith("/api/gitlab/status")) - return { connected: true, baseUrl: "https://gl", username: "alice" }; - if (url.endsWith("/api/gitlab/repos")) return [repo("g/blank")]; - return []; - }); - const failingRefresh = makeStore({ - refresh: (): Promise => - Promise.reject(new Error("boom")), - }); - render(tree(failingRefresh)); - await flush(); - fireEvent.click(screen.getByText("g/blank")); - fireEvent.change(screen.getByLabelText("Branch"), { - target: { value: "" }, - }); - fireEvent.click(screen.getByLabelText("Spawn container")); - await flush(); - expect(bodies).toEqual([JSON.stringify({ repo: "g/blank" })]); - - await act(async () => { - vi.advanceTimersByTime(1500); - }); - await flush(); - expect(PUSH_TOAST).toHaveBeenCalledWith("boom"); - }); - - it("spawn POST failure shows the error", async () => { - mockFetchJson((url, init) => { - if (init?.method === "POST" && url.endsWith("/api/spawn")) - return jsonResponse({ error: "no docker" }, 500); - if (url.endsWith("/api/gitlab/status")) - return { connected: true, baseUrl: "https://gl", username: "alice" }; - if (url.endsWith("/api/gitlab/repos")) return [repo("g/x")]; - return []; - }); - render(tree(makeStore())); - await flush(); - fireEvent.click(screen.getByText("g/x")); - fireEvent.click(screen.getByLabelText("Spawn container")); - await flush(); - expect(screen.getByText("no docker")).toBeInTheDocument(); - }); - - it("polling gives up after the tick cap and stays on the page", async () => { - let statusCalls = 0; - mockFetchJson((url) => { - if (url.endsWith("/api/spawn/status")) { - statusCalls += 1; - return []; - } - if (url.endsWith("/api/gitlab/status")) - return { connected: true, baseUrl: "https://gl", username: "alice" }; - if (url.endsWith("/api/gitlab/repos")) return [repo("g/slow")]; - if (url.endsWith("/api/spawn")) - return { sessionId: "slow-1", containerId: "d" }; - return []; - }); - const { unmount } = render(tree(makeStore())); - await flush(); - fireEvent.click(screen.getByText("g/slow")); - fireEvent.click(screen.getByLabelText("Spawn container")); - await flush(); - expect(screen.getByText("Spawning…")).toBeInTheDocument(); - - await act(async () => { - vi.advanceTimersByTime(1500 * 402); - }); - await flush(); - const afterCap = statusCalls; - await act(async () => { - vi.advanceTimersByTime(1500 * 10); - }); - await flush(); - expect(statusCalls).toBe(afterCap); - expect(screen.getByText("Spawning…")).toBeInTheDocument(); - unmount(); - }); - - it("spawn job line renders from store.spawnJobs", async () => { - mockFetchJson((url, init) => { - if (init?.method === "POST" && url.endsWith("/api/spawn")) - return { sessionId: "sj-1", containerId: "cid" }; - if (url.endsWith("/api/spawn/status")) return []; - if (url.endsWith("/api/gitlab/status")) - return { connected: true, baseUrl: "https://gl", username: "alice" }; - if (url.endsWith("/api/gitlab/repos")) return [repo("g/p")]; - return []; - }); - const store = makeStore(); - const { rerender } = render(tree(store)); - await flush(); - fireEvent.click(screen.getByText("g/p")); - fireEvent.click(screen.getByLabelText("Spawn container")); - await flush(); - expect(screen.getByText("Spawning…")).toBeInTheDocument(); - - store.spawnJobs = [{ repo: "g/p", state: "cloning", sessionId: "sj-1" }]; + // feedback clears after COPY_FEEDBACK_MS (H) act(() => { - rerender(tree(store)); + vi.advanceTimersByTime(1200); }); - expect(screen.getByText("g/p: cloning")).toBeInTheDocument(); - }); -}); - -describe("SpawnSteps", () => { - it("renders progress steps matching job state", async () => { - mockFetchJson((url, init) => { - if (init?.method === "POST" && url.endsWith("/api/spawn")) - return { sessionId: "sp1", containerId: "" }; - if (url.endsWith("/api/gitlab/status")) - return { connected: true, baseUrl: "https://gl", username: "a" }; - if (url.endsWith("/api/gitlab/repos")) return [repo("g/p")]; - return []; - }); - const store = makeStore({ - spawnJobs: [ - { sessionId: "sp1", repo: "g/p", state: "building", containerId: "" }, - ], - }); - render(tree(store)); - await flush(); - fireEvent.click(screen.getByText("g/p")); - fireEvent.click(screen.getByLabelText("Spawn container")); - await flush(); - const steps = document.querySelectorAll(".spawn-progress .step"); - expect(steps).toHaveLength(4); - expect(steps[0]?.className).toContain("done"); - expect(steps[1]?.className).toContain("current"); - expect(steps[2]?.className).toBe("step"); + expect(screen.getByText("copy")).toBeInTheDocument(); + vi.useRealTimers(); }); - it("progress element carries progressbar semantics for the current step", async () => { - mockFetchJson((url, init) => { - if (init?.method === "POST" && url.endsWith("/api/spawn")) - return { sessionId: "sp2", containerId: "" }; - if (url.endsWith("/api/gitlab/status")) - return { connected: true, baseUrl: "https://gl", username: "a" }; - if (url.endsWith("/api/gitlab/repos")) return [repo("g/p")]; - return []; - }); - const store = makeStore({ - spawnJobs: [ - { sessionId: "sp2", repo: "g/p", state: "building", containerId: "" }, - ], - }); - render(tree(store)); - await flush(); - fireEvent.click(screen.getByText("g/p")); - fireEvent.click(screen.getByLabelText("Spawn container")); - await flush(); - const bar = screen.getByRole("progressbar"); - expect(bar).toHaveAttribute("aria-valuemin", "1"); - expect(bar).toHaveAttribute("aria-valuemax", "4"); - expect(bar).toHaveAttribute("aria-valuenow", "2"); // building = step 2 + it("user bubbles get a copy button, toolResults do not", () => { + const { rerender } = render( + , + ); + expect( + screen.getByRole("button", { name: "Copy message" }), + ).toBeInTheDocument(); + rerender( + , + ); + expect(screen.queryByRole("button", { name: "Copy message" })).toBeNull(); }); }); diff --git a/web/src/ChatStream.tsx b/web/src/ChatStream.tsx index 2804af4..1091643 100644 --- a/web/src/ChatStream.tsx +++ b/web/src/ChatStream.tsx @@ -2,6 +2,8 @@ import { useEffect, useRef, useState } from "react"; import type { ChatMessage, ToolState } from "./derive"; const PREVIEW_LEN: number = 120; +const COPY_FEEDBACK_MS: number = 1200; +const PIN_THRESHOLD_PX: number = 80; function oneLine(text: string): string { const flat = text.replace(/\s+/g, " ").trim(); @@ -18,7 +20,7 @@ function CopyButton({ text }: { text: string }): React.ReactNode { onClick={() => { void navigator.clipboard?.writeText(text).then(() => { setCopied(true); - window.setTimeout(() => setCopied(false), 1200); + window.setTimeout(() => setCopied(false), COPY_FEEDBACK_MS); }); }} > @@ -156,7 +158,8 @@ export default function ChatStream({ const onScroll = (): void => { const el = scrollRef.current; if (el === null) return; - pinnedRef.current = el.scrollHeight - el.scrollTop - el.clientHeight < 80; + pinnedRef.current = + el.scrollHeight - el.scrollTop - el.clientHeight < PIN_THRESHOLD_PX; }; useEffect(() => { diff --git a/web/src/ChatView.test.tsx b/web/src/ChatView.test.tsx index ce21565..f50318e 100644 --- a/web/src/ChatView.test.tsx +++ b/web/src/ChatView.test.tsx @@ -313,6 +313,25 @@ describe("ChatView", () => { expect(ta.value).toBe(""); }); + it("Enter during IME composition confirms candidates, not a send (E)", async () => { + const fetchMock = mockFetchJson((_url, init) => + init?.method === "POST" ? { ok: true } : [], + ); + renderChat(makeStore()); + const ta = screen.getByLabelText("Message") as HTMLTextAreaElement; + await userEvent.type(ta, "こん"); + fireEvent.keyDown(ta, { key: "Enter", isComposing: true }); + const posts = (): number => + fetchMock.mock.calls.filter( + (c) => (c[1] as RequestInit | undefined)?.method === "POST", + ).length; + expect(posts()).toBe(0); + expect(ta.value).toBe("こん"); // composition text survives the confirm + + fireEvent.keyDown(ta, { key: "Enter" }); // composition finished: real send + await vi.waitFor(() => expect(posts()).toBe(1)); + }); + it("abort posts to the abort route", async () => { const fetchMock = mockFetchJson((_url, init) => init?.method === "POST" ? { ok: true } : [], @@ -810,6 +829,92 @@ describe("ChatView stale async guards (S1/S4) and send draft (S6)", () => { await vi.waitFor(() => expect(pushToast).toHaveBeenCalledWith("nope")); expect(ta.value).toBe("precious draft"); }); + + it("session switch clears the draft instead of leaking it (B)", async () => { + mockFetchJson(() => []); + render( + + + , + ); + const ta = screen.getByLabelText("Message") as HTMLTextAreaElement; + await userEvent.type(ta, "secret for s1"); + await userEvent.click( + screen.getByRole("button", { name: "switch session" }), + ); + await screen.findByText("ghost"); + expect(ta.value).toBe(""); // s1's draft must not appear in ghost's composer + }); + + it("session switch resets a stuck sending flag (B)", async () => { + let releaseSend: ((v: unknown) => void) | null = null; + mockFetchJson((_url, init) => { + if (init?.method === "POST") + return new Promise((res) => { + releaseSend = res; + }); + return []; + }); + render( + + + , + ); + const ta = screen.getByLabelText("Message") as HTMLTextAreaElement; + await userEvent.type(ta, "hangs"); + fireEvent.keyDown(ta, { key: "Enter" }); // send starts and never settles + + await userEvent.click( + screen.getByRole("button", { name: "switch session" }), + ); + await screen.findByText("ghost"); + const ta2 = screen.getByLabelText("Message") as HTMLTextAreaElement; + await userEvent.type(ta2, "fresh"); + // sending=true surviving the switch would keep the ghost button disabled + expect(screen.getByLabelText("Send message")).toBeEnabled(); + await act(async () => { + releaseSend?.([]); + }); + }); + + it("resubscribes only when the subscribe identity (manager) changes, not on store churn (G)", async () => { + mockFetchJson(() => []); + let subscribeCalls: number = 0; + const makeSubscribe = + (): SessionsStore["subscribe"] => (sessionId, onEvents) => { + subscribeCalls += 1; + currentSub = { sessionId, onEvents }; + return () => { + if (currentSub !== null && currentSub.sessionId === sessionId) + currentSub = null; + }; + }; + const liveSubscribe = makeSubscribe(); + const { rerender } = renderChat({ + ...makeStore(), + subscribe: liveSubscribe, + }); + await waitFor(() => expect(subscribeCalls).toBe(1)); + + // store identity changes with every session_list frame; the live + // subscription must survive instead of unsubscribe/resubscribe churn + const churned = makeStore({ + sessions: [{ ...sessions[0]!, online: false }], + }); + rerenderChatAgain(rerender, { ...churned, subscribe: liveSubscribe }); + for (let i = 0; i < 4; i += 1) { + // eslint-disable-next-line no-await-in-loop + await act(async () => { + await Promise.resolve(); + }); + } + expect(subscribeCalls).toBe(1); + expect(currentSub).not.toBeNull(); + + // manager replaced -> new subscribe identity -> resubscribe happens + rerenderChatAgain(rerender, { ...makeStore(), subscribe: makeSubscribe() }); + await waitFor(() => expect(subscribeCalls).toBe(2)); + }); }); describe("ChatView unknown session", () => { diff --git a/web/src/ChatView.tsx b/web/src/ChatView.tsx index 09554e7..59ebd66 100644 --- a/web/src/ChatView.tsx +++ b/web/src/ChatView.tsx @@ -76,6 +76,10 @@ export default function ChatView({ store, pushToast }: Props) { setEvents([]); setHasOlder(false); lastSeqRef.current = 0; + // a draft (or stuck sending state) from the previous session must not + // leak into the new one + setDraft(""); + setSending(false); let alive = true; void (async () => { try { @@ -96,11 +100,13 @@ export default function ChatView({ store, pushToast }: Props) { }; }, [sessionId, applyEvents]); - // ws subscription + // ws subscription: dep on store.subscribe (stable per manager) rather than + // the whole store object — every sessions-list change would otherwise + // churn unsubscribe/resubscribe and can drop events in the gap useEffect(() => { if (sessionId.length === 0 || store.state !== "open") return; return store.subscribe(sessionId, applyEvents); - }, [sessionId, store.state, store, applyEvents]); + }, [sessionId, store.state, store.subscribe, applyEvents]); // missed events after reconnect (persisted only); a response for a // previous session must not merge here nor touch the cursor (S1) @@ -187,6 +193,8 @@ export default function ChatView({ store, pushToast }: Props) { }; const onKeyDown = (e: React.KeyboardEvent): void => { + // IME composition: Enter confirms the candidate window, not a send + if (e.nativeEvent.isComposing) return; if (e.key === SEND_KEY && !e.shiftKey) { e.preventDefault(); void send(); diff --git a/web/src/SessionsView.test.tsx b/web/src/SessionsView.test.tsx index a41f862..17c1a46 100644 --- a/web/src/SessionsView.test.tsx +++ b/web/src/SessionsView.test.tsx @@ -77,6 +77,12 @@ describe("SessionsView", () => { expect(skel).not.toBeNull(); }); + it("sessions arriving before the 600ms window end the skeleton immediately (F)", () => { + renderView({ sessions: [session({ id: "s1", name: "live" })] }); + expect(document.querySelector(".skeleton")).toBeNull(); + expect(screen.getByText("live")).toBeInTheDocument(); + }); + it("renders cards sorted by last activity with fallbacks", () => { renderView({ sessions: [ diff --git a/web/src/SessionsView.tsx b/web/src/SessionsView.tsx index aff8d78..1a914cb 100644 --- a/web/src/SessionsView.tsx +++ b/web/src/SessionsView.tsx @@ -79,6 +79,12 @@ export default function SessionsView({ return () => window.clearTimeout(t); }, []); + useEffect(() => { + // real rows arriving end the skeleton window immediately: a later + // transient empty list must not flash skeletons again + if (sessions.length > 0) setLoaded(true); + }, [sessions.length]); + const open = useCallback( (id: string): void => { navigate(`/s/${id}`); diff --git a/web/src/SpawnView.test.tsx b/web/src/SpawnView.test.tsx index 4366159..a55151e 100644 --- a/web/src/SpawnView.test.tsx +++ b/web/src/SpawnView.test.tsx @@ -1,400 +1,503 @@ -import { fireEvent, render, screen, waitFor } from "@testing-library/react"; -import userEvent from "@testing-library/user-event"; -import { describe, expect, it } from "vitest"; -import type { ChatMessage, ToolState } from "./derive"; -import ChatStream, { Bubble, TypingIndicator } from "./ChatStream"; +import { act, fireEvent, render, screen } from "@testing-library/react"; +import { MemoryRouter, Route, Routes } from "react-router-dom"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { Repo, SessionListItem } from "./protocol"; +import { Route as ApiRoute } from "./protocol"; +import { fetchJson } from "./api"; +import SpawnView from "./SpawnView"; +import type { SessionsStore } from "./store"; +import { jsonResponse, mockFetchJson, seedSettings } from "./test/setup"; -function msg(partial: Partial): ChatMessage { +const repo = (path: string, branch = "main"): Repo => ({ + path, + name: path.split("/")[1] ?? path, + namespace: path.split("/")[0] ?? "g", + lastActivityAt: "2024-05-01T00:00:00Z", + webUrl: `https://gl/${path}`, + defaultBranch: branch, +}); + +const PUSH_TOAST = vi.fn(); + +function makeStore(over: Partial = {}): SessionsStore { return { - key: `k-${Math.random()}`, - role: "assistant", - text: "", - thinking: null, - toolCalls: [], - toolCallId: null, - streaming: false, - ...partial, + sessions: [], + state: "open", + spawnJobs: [], + refresh: async (): Promise => + fetchJson(ApiRoute.Sessions), + subscribe: (): (() => void) => () => undefined, + ...over, }; } -const tool = (p: Partial): ToolState => ({ - id: "c1", - name: "bash", - args: "", - running: false, - isError: false, - preview: "", - ...p, +function tree(store: SessionsStore): React.ReactElement { + return ( + + + } + /> + } /> + + + ); +} + +/** Flush pending microtasks + React effects (works under fake timers). */ +async function flush(ticks = 4): Promise { + for (let i = 0; i < ticks; i += 1) { + // eslint-disable-next-line no-await-in-loop + await act(async () => { + await Promise.resolve(); + }); + } +} + +beforeEach(() => { + PUSH_TOAST.mockClear(); + seedSettings(); }); -describe("Bubble", () => { - it("renders plain text per role class", () => { - const { container } = render( - , - ); - expect(container.querySelector(".bubble-row.user")).not.toBeNull(); - expect(container.textContent).toContain("hi there"); - }); +afterEach(() => { + vi.restoreAllMocks(); +}); - it("renders empty assistant text as nothing but shows nothing when empty", () => { - const { container } = render( - , - ); - expect(container.querySelector(".bubble")?.children).toHaveLength(0); - }); - - it("toolResult collapses to a one-line preview, expands to full text", async () => { - const long = `${"x".repeat(200)}`; - render( - , - ); - const details = screen - .getByText("result") - .closest("details") as HTMLDetailsElement; - expect(details.open).toBe(false); - expect(details.textContent).toContain("…"); - await userEvent.click(screen.getByText("result")); - expect(details.open).toBe(true); - expect(details.textContent).toContain(long); - }); - - it("toolResult with short text keeps full one-line preview", () => { - render( - , - ); - const details = screen - .getByText("result") - .closest("details") as HTMLDetailsElement; - expect(details.textContent).toContain("short out"); - expect(details.textContent).not.toContain("…"); - }); - - it("flattens whitespace in previews", () => { - render( - , - ); - expect(screen.getAllByText("a b c").length).toBeGreaterThan(0); - }); - - it("assistant tool calls attach tool cards", async () => { - const tools = new Map([ - [ - "c1", - tool({ - id: "c1", - name: "bash", - args: "ls -la", - running: false, - isError: false, - preview: "file", - }), - ], - ]); - render( - , - ); - expect(screen.getByText("🛠 bash")).toBeInTheDocument(); - expect(screen.getByText("finished")).toBeInTheDocument(); - - const summary = screen - .getByText("🛠 bash") - .closest("summary") as HTMLElement; - const card = summary.closest("details") as HTMLDetailsElement; - expect(card.open).toBe(false); - await userEvent.click(summary); - expect(card.open).toBe(true); - expect(card.textContent).toContain("ls -la"); - expect(card.textContent).toContain("file"); - }); - - it("tool card status variants: running, error, done", () => { - const tools = new Map([ - ["c1", tool({ id: "c1", running: true })], - ["c2", tool({ id: "c2", running: false, isError: true })], - ["c3", tool({ id: "c3", running: false, isError: false })], - ]); - render( - ({ - id, - name: `t-${id}`, - argsJson: "{}", - })), - })} - tools={tools} - />, - ); - expect(screen.getByText("working…")).toBeInTheDocument(); - expect(screen.getByText("error")).toBeInTheDocument(); - expect(screen.getByText("done")).toBeInTheDocument(); - }); - - it("tool call with no matching state renders no card", () => { - const { container } = render( - , - ); - expect(container.querySelectorAll(".tool-card")).toHaveLength(0); - }); - - it("thinking block only for non-empty thinking", async () => { - render(); - const details = screen - .getByText("thinking") - .closest("details") as HTMLDetailsElement; - await userEvent.click(screen.getByText("thinking")); - expect(details.open).toBe(true); - expect(details.textContent).toContain("because"); - - const { container } = render( - , - ); - expect(container.querySelector(".thinking")).toBeNull(); - }); - - it("streaming bubble shows the caret", () => { - const { container } = render( - , - ); - expect(container.querySelector(".stream-caret")).not.toBeNull(); +describe("SpawnView status", () => { + it("shows checking state, then error when gitlab status fails", async () => { + mockFetchJson((url) => { + if (url.includes("/api/gitlab/status")) + return jsonResponse({ error: "down" }, 500); + return []; + }); + render(tree(makeStore())); + expect(screen.getByText("checking gitlab…")).toBeInTheDocument(); + await flush(); + expect(screen.getByText(/gitlab status failed: down/)).toBeInTheDocument(); }); }); -describe("TypingIndicator", () => { - it("renders three dots with aria-live", () => { - const { container } = render(); - expect(container.querySelector('[aria-live="polite"]')).not.toBeNull(); - expect(container.querySelectorAll(".dot")).toHaveLength(3); +describe("SpawnView connect flow", () => { + it("requires a token", async () => { + mockFetchJson((url) => { + if (url.endsWith("/api/gitlab/status")) + return { connected: false, baseUrl: "https://gl" }; + return []; + }); + render(tree(makeStore())); + await flush(); + fireEvent.click(screen.getByRole("button", { name: "Connect" })); + await flush(); + expect(screen.getByText("token required")).toBeInTheDocument(); + }); + + it("connects, clears the PAT, loads repos and shows the picker", async () => { + let connected = false; + mockFetchJson((url, init) => { + if (url.endsWith("/api/gitlab/status")) + return { + connected, + baseUrl: "https://gl", + username: connected ? "alice" : undefined, + }; + if (url.endsWith("/api/gitlab/connect") && init?.method === "POST") { + connected = true; + return { username: "alice" }; + } + if (url.endsWith("/api/gitlab/repos")) + return [repo("g/one"), repo("g/two")]; + return []; + }); + render(tree(makeStore())); + await flush(); + expect(screen.getByText("Connect GitLab")).toBeInTheDocument(); + + const pat = screen.getByLabelText( + "GitLab personal access token", + ) as HTMLInputElement; + fireEvent.input(pat, { target: { value: "glpat-x" } }); + fireEvent.click(screen.getByRole("button", { name: "Connect" })); + await flush(); + + expect(screen.getByText("g/one")).toBeInTheDocument(); + expect(screen.getByText("g/two")).toBeInTheDocument(); + expect(screen.queryByLabelText("GitLab personal access token")).toBeNull(); + expect(screen.getByText("Repository")).toBeInTheDocument(); + }); + + it("connect failure shows the error and keeps the gate", async () => { + mockFetchJson((url, init) => { + if (url.endsWith("/api/gitlab/status")) + return { connected: false, baseUrl: "https://gl" }; + if (url.endsWith("/api/gitlab/connect") && init?.method === "POST") + return jsonResponse({ error: "bad pat" }, 401); + return []; + }); + render(tree(makeStore())); + await flush(); + fireEvent.input(screen.getByLabelText("GitLab personal access token"), { + target: { value: "glpat-bad" }, + }); + fireEvent.click(screen.getByRole("button", { name: "Connect" })); + await flush(); + expect(screen.getByText("bad pat")).toBeInTheDocument(); + expect(screen.getByText("Connect GitLab")).toBeInTheDocument(); + }); + + it("connected on load fetches repos immediately", async () => { + mockFetchJson((url) => { + if (url.endsWith("/api/gitlab/status")) + return { connected: true, baseUrl: "https://gl", username: "alice" }; + if (url.endsWith("/api/gitlab/repos")) return [repo("g/quick")]; + return []; + }); + render(tree(makeStore())); + await flush(); + expect(screen.getByText("g/quick")).toBeInTheDocument(); }); }); -describe("ChatStream", () => { - function stream(p: { - messages: ChatMessage[]; - busy: boolean; - hasOlder?: boolean; - loadingOlder?: boolean; - onOlder?: () => void; - }): React.ReactElement { - return ( - undefined)} - /> - ); +describe("SpawnView repo picker", () => { + function connectedMock(): void { + mockFetchJson((url) => { + if (url.endsWith("/api/gitlab/status")) + return { connected: true, baseUrl: "https://gl", username: "alice" }; + if (url.endsWith("/api/gitlab/repos")) + return [repo("g/alpha"), repo("g/beta", "dev")]; + return []; + }); } - it("renders messages and typing indicator while busy with no open stream", () => { - const { container, rerender } = render( - stream({ messages: [msg({ key: "a", text: "one" })], busy: true }), - ); - expect(container.querySelector(".typing")).not.toBeNull(); + it("filters repos by query, keyboard selects, empty filter message", async () => { + connectedMock(); + render(tree(makeStore())); + await flush(); + expect(screen.getByText("g/alpha")).toBeInTheDocument(); - rerender( - stream({ - messages: [ - msg({ key: "a", text: "one" }), - msg({ key: "b", streaming: true }), - ], - busy: true, - }), - ); - expect(container.querySelector(".typing")).toBeNull(); - rerender( - stream({ messages: [msg({ key: "a", text: "one" })], busy: false }), - ); - expect(container.querySelector(".typing")).toBeNull(); + const filter = screen.getByLabelText("Filter repositories"); + fireEvent.input(filter, { target: { value: "beta" } }); + expect(screen.queryByText("g/alpha")).toBeNull(); + expect(screen.getByText("g/beta")).toBeInTheDocument(); + + const item = screen + .getByText("g/beta") + .closest(".repo-item") as HTMLElement; + fireEvent.keyDown(item, { key: "Enter" }); + expect(screen.getByLabelText("Branch")).toHaveValue("dev"); + + fireEvent.input(filter, { target: { value: "zzz" } }); + expect(screen.getByText("no matching repos")).toBeInTheDocument(); }); - it("sticks to bottom on new messages, un-pins on user scroll up, re-pins near bottom", () => { - const { container, rerender } = render( - stream({ messages: [msg({ key: "a" })], busy: false }), - ); - const scroller = container.querySelector(".chat-scroll") as HTMLElement; - Object.defineProperty(scroller, "scrollHeight", { - configurable: true, - value: 1000, - }); - Object.defineProperty(scroller, "clientHeight", { - configurable: true, - value: 300, - }); + it("keyboard Enter and Space on a repo item are default-prevented (C)", async () => { + connectedMock(); + render(tree(makeStore())); + await flush(); + const item = screen + .getByText("g/alpha") + .closest(".repo-item") as HTMLElement; - scroller.dispatchEvent(new Event("scroll")); - // pinned: scrollTop at bottom - scroller.scrollTop = 700; - Object.defineProperty(scroller, "scrollTop", { - configurable: true, - writable: true, - value: 700, - }); - scroller.dispatchEvent(new Event("scroll")); - rerender( - stream({ - messages: [msg({ key: "a" }), msg({ key: "b" })], - busy: false, - }), - ); - expect(scroller.scrollTop).toBe(1000); + const enter = fireEvent.keyDown(item, { key: "Enter" }); + expect(enter).toBe(false); // preventDefault consumed: no page scroll + expect(screen.getByLabelText("Branch")).toHaveValue("main"); - // scroll far up -> unpin - Object.defineProperty(scroller, "scrollTop", { - configurable: true, - writable: true, - value: 0, - }); - scroller.dispatchEvent(new Event("scroll")); - rerender( - stream({ - messages: [msg({ key: "a" }), msg({ key: "b" }), msg({ key: "c" })], - busy: false, - }), - ); - expect(scroller.scrollTop).toBe(0); + const space = fireEvent.keyDown(item, { key: " " }); + expect(space).toBe(false); + expect(item.getAttribute("aria-pressed")).toBe("true"); // still picks - // scroll near bottom (within 80px) -> pinned again - Object.defineProperty(scroller, "scrollTop", { - configurable: true, - writable: true, - value: 940, - }); - scroller.dispatchEvent(new Event("scroll")); - rerender( - stream({ - messages: [ - msg({ key: "a" }), - msg({ key: "b" }), - msg({ key: "c" }), - msg({ key: "d" }), - ], - busy: false, - }), - ); - expect(scroller.scrollTop).toBe(1000); + const tab = fireEvent.keyDown(item, { key: "Tab" }); + expect(tab).toBe(true); // other keys keep their default behavior }); - it("load-older button renders only when an older page exists and fires on click", () => { - const onOlder = vi.fn(); - const { rerender } = render( - stream({ messages: [msg({ key: "a" })], busy: false }), - ); - expect( - screen.queryByRole("button", { name: "Load older messages" }), - ).toBeNull(); + it("shows loading repos while the list is pending", async () => { + const gate = { resolve: null as ((v: unknown) => void) | null }; + mockFetchJson((url) => { + if (url.endsWith("/api/gitlab/status")) + return { connected: true, baseUrl: "https://gl", username: "alice" }; + if (url.endsWith("/api/gitlab/repos")) + return new Promise((res) => { + gate.resolve = res; + }); + return []; + }); + render(tree(makeStore())); + await flush(); + expect(screen.getByText("loading repos…")).toBeInTheDocument(); + gate.resolve?.([repo("g/late")]); + await flush(); + expect(await screen.findByText("g/late")).toBeInTheDocument(); + }); - rerender( - stream({ - messages: [msg({ key: "a" })], - busy: false, - hasOlder: true, - onOlder, - }), - ); - const btn = screen.getByRole("button", { name: "Load older messages" }); - fireEvent.click(btn); - expect(onOlder).toHaveBeenCalledTimes(1); - - rerender( - stream({ - messages: [msg({ key: "a" })], - busy: false, - hasOlder: true, - loadingOlder: true, - onOlder, - }), - ); - expect( - screen.getByRole("button", { name: "Load older messages" }), - ).toBeDisabled(); + it("spawn without selection shows pick-a-repo error", async () => { + connectedMock(); + render(tree(makeStore())); + await flush(); + expect(screen.getByLabelText("Spawn container")).toBeDisabled(); + expect(screen.getByText("select a repo above")).toBeInTheDocument(); }); }); -describe("copy button", () => { - it("copy button writes message text and flashes copied", async () => { - const writeText = vi.fn(() => Promise.resolve()); - Object.assign(navigator, { clipboard: { writeText } }); - render( - , - ); - const btn = screen.getByRole("button", { name: "Copy message" }); - fireEvent.click(btn); - expect(writeText).toHaveBeenCalledWith("copy me"); - await waitFor(() => expect(screen.getByText("copied")).toBeInTheDocument()); +describe("SpawnView spawn+poll", () => { + beforeEach(() => { + vi.useFakeTimers(); }); - it("user bubbles get a copy button, toolResults do not", () => { - const { rerender } = render( - , + it("spawns, polls until online, then navigates to the chat", async () => { + const posts: Array<[string, RequestInit | undefined]> = []; + let sessionsOnline = false; // flipped after the first poll tick + const fetchMock = mockFetchJson((url, init) => { + if (init?.method === "POST" && url.endsWith("/api/spawn")) { + posts.push([url, init]); + return { sessionId: "new-1", containerId: "abc123def456" }; + } + if (url.endsWith("/api/sessions")) + return sessionsOnline + ? [ + { + id: "new-1", + name: "spawned", + cwd: "/w", + model: "m", + provider: "p", + agent: true, + repo: "g/proj", + startedAt: 1, + online: true, + lastEventAt: 1, + }, + ] + : []; + if (url.endsWith("/api/spawn/status")) return []; + if (url.endsWith("/api/gitlab/status")) + return { connected: true, baseUrl: "https://gl", username: "alice" }; + if (url.endsWith("/api/gitlab/repos")) return [repo("g/proj")]; + return []; + }); + const store = makeStore(); + const { unmount } = render(tree(store)); + await flush(); + fireEvent.click(screen.getByText("g/proj")); + fireEvent.click(screen.getByLabelText("Spawn container")); + await flush(); + + expect(posts).toHaveLength(1); + expect((posts[0]?.[1] as RequestInit).body).toBe( + JSON.stringify({ repo: "g/proj", branch: "main" }), ); + expect(screen.getByText("Spawning…")).toBeInTheDocument(); + expect(screen.getByText(/container abc123def456/)).toBeInTheDocument(); expect( - screen.getByRole("button", { name: "Copy message" }), + screen.getByText("waiting for session to come online…"), ).toBeInTheDocument(); - rerender( - , - ); - expect(screen.queryByRole("button", { name: "Copy message" })).toBeNull(); + + // first tick: session not online yet + await act(async () => { + vi.advanceTimersByTime(1500); + }); + await flush(); + expect(screen.queryByTestId("chat-route")).toBeNull(); + + // each poll tick issues exactly one sessions fetch (refresh reuse, S7) + const sessionsFetches = fetchMock.mock.calls.filter(([u]) => + String(u).endsWith("/api/sessions"), + ).length; + expect(sessionsFetches).toBe(1); + + // session comes online -> next tick navigates + sessionsOnline = true; + await act(async () => { + vi.advanceTimersByTime(1500); + }); + await flush(); + expect(screen.getByTestId("chat-route")).toBeInTheDocument(); + unmount(); + }); + + it("spawning view shows job line from status and spawn error text", async () => { + mockFetchJson((url, init) => { + if (init?.method === "POST" && url.endsWith("/api/spawn")) + return { sessionId: "sx", containerId: "" }; + if (url.endsWith("/api/gitlab/status")) + return { connected: true, baseUrl: "https://gl", username: "alice" }; + if (url.endsWith("/api/gitlab/repos")) return [repo("g/p")]; + return []; + }); + const store = makeStore({ + spawnJobs: [ + { sessionId: "sx", repo: "g/p", state: "cloning", containerId: "" }, + ], + }); + render(tree(store)); + await flush(); + fireEvent.click(screen.getByText("g/p")); + fireEvent.click(screen.getByLabelText("Spawn container")); + await flush(); + expect(screen.getByText("Spawning…")).toBeInTheDocument(); + expect(screen.getByText("g/p: cloning")).toBeInTheDocument(); + }); + + it("branch left blank sends repo only; poll refresh failure toasts", async () => { + const bodies: string[] = []; + mockFetchJson((url, init) => { + if (init?.method === "POST" && url.endsWith("/api/spawn")) { + bodies.push(String(init.body)); + return { sessionId: "new-2", containerId: "cccccccccccc" }; + } + if (url.endsWith("/api/spawn/status")) return []; + if (url.endsWith("/api/gitlab/status")) + return { connected: true, baseUrl: "https://gl", username: "alice" }; + if (url.endsWith("/api/gitlab/repos")) return [repo("g/blank")]; + return []; + }); + const failingRefresh = makeStore({ + refresh: (): Promise => + Promise.reject(new Error("boom")), + }); + render(tree(failingRefresh)); + await flush(); + fireEvent.click(screen.getByText("g/blank")); + fireEvent.change(screen.getByLabelText("Branch"), { + target: { value: "" }, + }); + fireEvent.click(screen.getByLabelText("Spawn container")); + await flush(); + expect(bodies).toEqual([JSON.stringify({ repo: "g/blank" })]); + + await act(async () => { + vi.advanceTimersByTime(1500); + }); + await flush(); + expect(PUSH_TOAST).toHaveBeenCalledWith("boom"); + }); + + it("spawn POST failure shows the error", async () => { + mockFetchJson((url, init) => { + if (init?.method === "POST" && url.endsWith("/api/spawn")) + return jsonResponse({ error: "no docker" }, 500); + if (url.endsWith("/api/gitlab/status")) + return { connected: true, baseUrl: "https://gl", username: "alice" }; + if (url.endsWith("/api/gitlab/repos")) return [repo("g/x")]; + return []; + }); + render(tree(makeStore())); + await flush(); + fireEvent.click(screen.getByText("g/x")); + fireEvent.click(screen.getByLabelText("Spawn container")); + await flush(); + expect(screen.getByText("no docker")).toBeInTheDocument(); + }); + + it("polling gives up after the tick cap and stays on the page", async () => { + let statusCalls = 0; + mockFetchJson((url) => { + if (url.endsWith("/api/spawn/status")) { + statusCalls += 1; + return []; + } + if (url.endsWith("/api/gitlab/status")) + return { connected: true, baseUrl: "https://gl", username: "alice" }; + if (url.endsWith("/api/gitlab/repos")) return [repo("g/slow")]; + if (url.endsWith("/api/spawn")) + return { sessionId: "slow-1", containerId: "d" }; + return []; + }); + const { unmount } = render(tree(makeStore())); + await flush(); + fireEvent.click(screen.getByText("g/slow")); + fireEvent.click(screen.getByLabelText("Spawn container")); + await flush(); + expect(screen.getByText("Spawning…")).toBeInTheDocument(); + + await act(async () => { + vi.advanceTimersByTime(1500 * 402); + }); + await flush(); + const afterCap = statusCalls; + await act(async () => { + vi.advanceTimersByTime(1500 * 10); + }); + await flush(); + expect(statusCalls).toBe(afterCap); + expect(screen.getByText("Spawning…")).toBeInTheDocument(); + unmount(); + }); + + it("spawn job line renders from store.spawnJobs", async () => { + mockFetchJson((url, init) => { + if (init?.method === "POST" && url.endsWith("/api/spawn")) + return { sessionId: "sj-1", containerId: "cid" }; + if (url.endsWith("/api/spawn/status")) return []; + if (url.endsWith("/api/gitlab/status")) + return { connected: true, baseUrl: "https://gl", username: "alice" }; + if (url.endsWith("/api/gitlab/repos")) return [repo("g/p")]; + return []; + }); + const store = makeStore(); + const { rerender } = render(tree(store)); + await flush(); + fireEvent.click(screen.getByText("g/p")); + fireEvent.click(screen.getByLabelText("Spawn container")); + await flush(); + expect(screen.getByText("Spawning…")).toBeInTheDocument(); + + store.spawnJobs = [{ repo: "g/p", state: "cloning", sessionId: "sj-1" }]; + act(() => { + rerender(tree(store)); + }); + expect(screen.getByText("g/p: cloning")).toBeInTheDocument(); + }); +}); + +describe("SpawnSteps", () => { + it("renders progress steps matching job state", async () => { + mockFetchJson((url, init) => { + if (init?.method === "POST" && url.endsWith("/api/spawn")) + return { sessionId: "sp1", containerId: "" }; + if (url.endsWith("/api/gitlab/status")) + return { connected: true, baseUrl: "https://gl", username: "a" }; + if (url.endsWith("/api/gitlab/repos")) return [repo("g/p")]; + return []; + }); + const store = makeStore({ + spawnJobs: [ + { sessionId: "sp1", repo: "g/p", state: "building", containerId: "" }, + ], + }); + render(tree(store)); + await flush(); + fireEvent.click(screen.getByText("g/p")); + fireEvent.click(screen.getByLabelText("Spawn container")); + await flush(); + const steps = document.querySelectorAll(".spawn-progress .step"); + expect(steps).toHaveLength(4); + expect(steps[0]?.className).toContain("done"); + expect(steps[1]?.className).toContain("current"); + expect(steps[2]?.className).toBe("step"); + }); + + it("progress element carries progressbar semantics for the current step", async () => { + mockFetchJson((url, init) => { + if (init?.method === "POST" && url.endsWith("/api/spawn")) + return { sessionId: "sp2", containerId: "" }; + if (url.endsWith("/api/gitlab/status")) + return { connected: true, baseUrl: "https://gl", username: "a" }; + if (url.endsWith("/api/gitlab/repos")) return [repo("g/p")]; + return []; + }); + const store = makeStore({ + spawnJobs: [ + { sessionId: "sp2", repo: "g/p", state: "building", containerId: "" }, + ], + }); + render(tree(store)); + await flush(); + fireEvent.click(screen.getByText("g/p")); + fireEvent.click(screen.getByLabelText("Spawn container")); + await flush(); + const bar = screen.getByRole("progressbar"); + expect(bar).toHaveAttribute("aria-valuemin", "1"); + expect(bar).toHaveAttribute("aria-valuemax", "4"); + expect(bar).toHaveAttribute("aria-valuenow", "2"); // building = step 2 }); }); diff --git a/web/src/SpawnView.tsx b/web/src/SpawnView.tsx index d32bd0e..f75de4f 100644 --- a/web/src/SpawnView.tsx +++ b/web/src/SpawnView.tsx @@ -270,7 +270,11 @@ export default function SpawnView({ store, pushToast }: Props) { aria-pressed={selected?.path === r.path} onClick={() => pick(r)} onKeyDown={(e) => { - if (e.key === "Enter" || e.key === " ") pick(r); + if (e.key === "Enter" || e.key === " ") { + // role=button: keep Space from scrolling the page + e.preventDefault(); + pick(r); + } }} >
diff --git a/web/src/TaskPanel.test.tsx b/web/src/TaskPanel.test.tsx index d2eb288..6f3d616 100644 --- a/web/src/TaskPanel.test.tsx +++ b/web/src/TaskPanel.test.tsx @@ -38,6 +38,22 @@ describe("TaskPanel", () => { expect(deleted.style.textDecoration).toContain("line-through"); }); + it("duplicate todo contents render as distinct rows (D)", () => { + const tasks: TaskDerivation = { + ...base, + todos: [ + { content: "same text", status: "pending", deleted: false }, + { content: "same text", status: "completed", deleted: false }, + ], + }; + const { container } = render(); + // identical content would collide on key={content}; index-composite keys + // keep both rows + expect(container.querySelectorAll(".todo-item")).toHaveLength(2); + expect(container.querySelectorAll(".todo-icon")).toHaveLength(2); + expect(container.querySelector(".todo-icon.completed")).not.toBeNull(); + }); + it("subagent rows: running spinner, done check, failed cross", () => { const tasks: TaskDerivation = { ...base, diff --git a/web/src/TaskPanel.tsx b/web/src/TaskPanel.tsx index 178649a..f445555 100644 --- a/web/src/TaskPanel.tsx +++ b/web/src/TaskPanel.tsx @@ -2,78 +2,87 @@ import type { SubagentRun, TodoItem, TodoStatus } from "./derive"; import type { TaskDerivation } from "./derive"; const STATUS_ICON: Record = { - pending: "○", - "in-progress": "◺", - completed: "●", + pending: "○", + "in-progress": "◺", + completed: "●", }; function TodoRow({ item }: { item: TodoItem }) { - return ( -
- - - {item.content} - -
- ); + return ( +
+ + + {item.content} + +
+ ); } function SubagentRow({ run }: { run: SubagentRun }) { - return ( -
- {run.running ? ( - - ) : ( - - )} - {run.name} - - {run.running ? "running" : run.isError ? "failed" : "done"} - -
- ); + return ( +
+ {run.running ? ( + + ) : ( + + )} + {run.name} + + {run.running ? "running" : run.isError ? "failed" : "done"} + +
+ ); } export default function TaskPanel({ tasks }: { tasks: TaskDerivation }) { - const hasTodos: boolean = tasks.todos.length > 0; - const hasSubagents: boolean = tasks.subagents.length > 0; - const hasWorking: boolean = tasks.workingTools.length > 0; - const empty: boolean = !hasTodos && !hasSubagents && !hasWorking; + const hasTodos: boolean = tasks.todos.length > 0; + const hasSubagents: boolean = tasks.subagents.length > 0; + const hasWorking: boolean = tasks.workingTools.length > 0; + const empty: boolean = !hasTodos && !hasSubagents && !hasWorking; - return ( -
- {empty &&

No tasks yet.

} - {hasTodos && ( -
-

Tasks

- {tasks.todos.map((t) => ( - - ))} -
- )} - {hasSubagents && ( -
-

Subagents

- {tasks.subagents.map((s) => ( - - ))} -
- )} - {hasWorking && ( -
-

Working

- {tasks.workingTools.map((w) => ( -
- - {w.name}… -
- ))} -
- )} -
- ); + return ( +
+ {empty &&

No tasks yet.

} + {hasTodos && ( +
+

Tasks

+ {tasks.todos.map((t, i) => ( + + ))} +
+ )} + {hasSubagents && ( +
+

Subagents

+ {tasks.subagents.map((s) => ( + + ))} +
+ )} + {hasWorking && ( +
+

Working

+ {tasks.workingTools.map((w) => ( +
+ + {w.name}… +
+ ))} +
+ )} +
+ ); } diff --git a/web/src/store.test.tsx b/web/src/store.test.tsx index 94afb1d..cb6457d 100644 --- a/web/src/store.test.tsx +++ b/web/src/store.test.tsx @@ -69,10 +69,28 @@ function listEvent(seq: number): EventFrame { describe("useSessions", () => { it("returns null when unconfigured", () => { const push = vi.fn(); - const { result } = renderHook(() => useSessions(push)); + const { result } = renderHook(() => useSessions(push, true)); expect(result.current).toBeNull(); }); + it("first-run: inactive -> active starts the ws manager (A)", async () => { + seedSettings(); + mockFetchJson(() => []); + const push = vi.fn(); + const { result, rerender } = renderHook( + ({ active }: { active: boolean }) => useSessions(push, active), + { initialProps: { active: false } }, + ); + // gate is up: settings exist but the hook must not dial yet + expect(FakeWebSocket.instances.length).toBe(0); + expect(result.current).not.toBeNull(); + + // gate saved: active flips true, the effect re-runs and starts the manager + rerender({ active: true }); + await waitFor(() => expect(FakeWebSocket.instances.length).toBe(1)); + expect(result.current?.state).toBe("connecting"); + }); + it("seeds via REST, updates from session_list and spawn_status frames, refresh works", async () => { seedSettings(); const sessions: SessionListItem[] = [ @@ -88,7 +106,7 @@ describe("useSessions", () => { return []; }); const push = vi.fn(); - const { result } = renderHook(() => useSessions(push)); + const { result } = renderHook(() => useSessions(push, true)); expect(result.current?.state).toBe("connecting"); await waitFor(() => expect(result.current?.sessions).toEqual(sessions)); @@ -131,7 +149,7 @@ describe("useSessions", () => { throw new Error("network down"); }); const push = vi.fn(); - const { result } = renderHook(() => useSessions(push)); + const { result } = renderHook(() => useSessions(push, true)); await waitFor(() => expect(push).toHaveBeenCalledWith("sessions: network down"), ); @@ -147,7 +165,7 @@ describe("useSessions", () => { seedSettings(); mockFetchJson(() => []); const push = vi.fn(); - const { result } = renderHook(() => useSessions(push)); + const { result } = renderHook(() => useSessions(push, true)); const sock = FakeWebSocket.last(); act(() => sock.serverOpen()); @@ -198,7 +216,7 @@ describe("useSessions", () => { mockFetchJson(() => []); const loc = stubReload(); const push = vi.fn(); - const { result } = renderHook(() => useSessions(push)); + const { result } = renderHook(() => useSessions(push, true)); const sock = FakeWebSocket.last(); act(() => sock.serverOpen()); act(() => sock.serverClose(1008)); @@ -219,7 +237,7 @@ describe("useSessions", () => { ); const loc = stubReload(); const push = vi.fn(); - renderHook(() => useSessions(push)); + renderHook(() => useSessions(push, true)); const failConnect = (): void => { act(() => FakeWebSocket.last().serverClose(1006)); }; @@ -254,7 +272,7 @@ describe("useSessions", () => { }); const loc = stubReload(); const push = vi.fn(); - renderHook(() => useSessions(push)); + renderHook(() => useSessions(push, true)); const failConnect = (): void => { act(() => FakeWebSocket.last().serverClose(1006)); }; @@ -293,7 +311,7 @@ describe("useSessions", () => { }); const loc = stubReload(); const push = vi.fn(); - renderHook(() => useSessions(push)); + renderHook(() => useSessions(push, true)); const failConnect = (): void => { act(() => FakeWebSocket.last().serverClose(1006)); }; @@ -336,7 +354,7 @@ describe("useSessions", () => { seedSettings(); mockFetchJson(() => []); const push = vi.fn(); - const { unmount } = renderHook(() => useSessions(push)); + const { unmount } = renderHook(() => useSessions(push, true)); await waitFor(() => expect(FakeWebSocket.instances.length).toBe(1)); const sock = FakeWebSocket.last(); unmount(); @@ -350,7 +368,7 @@ describe("useSessions", () => { const stable: SessionListItem[] = []; mockFetchJson(() => stable); const push = vi.fn(); - const { result, rerender } = renderHook(() => useSessions(push)); + const { result, rerender } = renderHook(() => useSessions(push, true)); await waitFor(() => expect(FakeWebSocket.instances.length).toBe(1)); // let the mount-time seed settle for (let i = 0; i < 4; i += 1) { @@ -370,7 +388,7 @@ describe("useSessions", () => { let list: SessionListItem[] = []; mockFetchJson(() => list); const push = vi.fn(); - const { result } = renderHook(() => useSessions(push)); + const { result } = renderHook(() => useSessions(push, true)); await waitFor(() => expect(result.current).not.toBeNull()); const first = result.current; const sock = FakeWebSocket.last(); diff --git a/web/src/store.ts b/web/src/store.ts index 3e6d9f9..8ca0f71 100644 --- a/web/src/store.ts +++ b/web/src/store.ts @@ -71,8 +71,13 @@ export interface SessionsStore { ) => () => void; } +/** + * `active` tells the hook the settings gate has been passed; it must be a + * dep below so saving settings (false -> true) starts the ws manager. + */ export function useSessions( pushToast: (text: string) => void, + active: boolean, ): SessionsStore | null { const [sessions, setSessions] = useState([]); const [spawnJobs, setSpawnJobs] = useState([]); @@ -80,6 +85,7 @@ export function useSessions( const [manager, setManager] = useState(null); useEffect(() => { + if (!active) return; const settings = getSettings(); if (settings === null) return; @@ -136,7 +142,7 @@ export function useSessions( m.close(); setManager(null); }; - }, [pushToast]); + }, [pushToast, active]); const refresh = useCallback(async (): Promise => { try {