buenosair/lvmh
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(review round 1): daemon PAT-header auth, job pruning, session-split batches, streaming tars, seed cleanup, ping/pong, byte caps, branch switch, slug --, ctx cancel, Init:true, pagination, latest/before events; web newest-window history + load-older, offline busy gate, staleness guards, memo store, auth probe, scroll key, focus-visible, draft restore, poll dedup; 106+181 tests, coverage 96.2%/95.1%+

Browse Source
This commit is contained in:
Raphael Westphal
2026-08-18 18:49:52 +02:00
parent 64e45e1a82
commit 6aac763563
25 changed files with 2564 additions and 959 deletions
Download Patch File Download Diff File Expand all files Collapse all files
daemon/spawner_test.go
+220 -20
View File
@@ -75,7 +75,7 @@ func TestSpawnerStartHappyPath(t *testing.T) {
t.Fatalf("missing env %v", wantEnv)
}
wantBinds := []string{
"lvmh-repo-group-project:" + workspaceMount,
"lvmh-repo-group--project:" + workspaceMount,
volumeSessions + ":" + sessionsMount,
volumePiCache + ":" + cacheMount,
}
@@ -90,6 +90,9 @@ func TestSpawnerStartHappyPath(t *testing.T) {
if c.HostConfig.NetworkMode != defaultNetwork {
t.Fatalf("network = %q", c.HostConfig.NetworkMode)
}
if c.HostConfig.Init == nil || !*c.HostConfig.Init {
t.Fatalf("agent container Init = %v, want true (tini zombie reaping)", c.HostConfig.Init)
}
// fresh repo volume seeded from the clone via CopyToContainer (tar)
seed := f.createsByName("lvmh-seed-")
@@ -99,10 +102,13 @@ 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{"lvmh-repo-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)
}
if seed[0].HostConfig.Init == nil || !*seed[0].HostConfig.Init {
t.Fatalf("seed container Init = %v, want true", seed[0].HostConfig.Init)
}
if len(f.archives) != 1 || f.archives[0] == 0 {
t.Fatalf("CopyToContainer archives = %v, want one non-empty tar", f.archives)
}
@@ -150,7 +156,7 @@ func TestSpawnerStartValidatesDockerAndDockerfile(t *testing.T) {
// docker reachable but image absent and no Dockerfile anywhere → clear error
t.Setenv(envWorkerDockerfile, filepath.Join(t.TempDir(), "missing.Dockerfile"))
sp2, err := NewSpawner(sp.store, sp.hub, "https://gitlab.example/")
sp2, err := NewSpawner(context.Background(), sp.store, sp.hub, "https://gitlab.example/")
if err != nil {
t.Fatalf("NewSpawner: %v", err)
}
@@ -250,7 +256,7 @@ func TestSpawnerCloneOrUpdatePullsExisting(t *testing.T) {
sp, _ := newTestSpawner(t, f)
// existing clone → pull --ff-only in that dir, no clone
dir := filepath.Join(sp.reposDir, "group-project")
dir := filepath.Join(sp.reposDir, repoSlug("group/project"))
if err := os.MkdirAll(filepath.Join(dir, ".git"), 0o755); err != nil {
t.Fatalf("mkdir .git: %v", err)
}
@@ -263,7 +269,7 @@ func TestSpawnerCloneOrUpdatePullsExisting(t *testing.T) {
}
}
func TestSpawnerCloneURLInjectsPAT(t *testing.T) {
func TestSpawnerCloneURLNeverCarriesPAT(t *testing.T) {
f := newFakeDocker()
sp, store := newTestSpawner(t, f)
@@ -273,12 +279,13 @@ func TestSpawnerCloneURLInjectsPAT(t *testing.T) {
if err := store.SetSetting(settingGitLabToken, "pat-1"); err != nil {
t.Fatalf("set token: %v", err)
}
// A stored PAT must never leak into the persisted clone URL.
u, err := sp.cloneURL("group/project")
if err != nil {
t.Fatalf("cloneURL: %v", err)
}
if u != "https://pat-1@gitlab.example/group/project.git" {
t.Fatalf("cloneURL with PAT = %q", u)
if u != "https://gitlab.example/group/project.git" || strings.Contains(u, "pat-1") {
t.Fatalf("cloneURL with stored PAT = %q, want clean URL", u)
}
}
@@ -305,8 +312,8 @@ func TestSpawnerRemoveSession(t *testing.T) {
t.Fatal("container row must be deleted")
}
for _, j := range sp.JobsSnapshot() {
if j.SessionID == "s1" && j.State != stateError {
t.Fatalf("job after removal = %+v, want error state", j)
if j.SessionID == "s1" {
t.Fatalf("job after removal = %+v, want entry deleted", j)
}
}
@@ -326,17 +333,17 @@ func TestSpawnerSeedVolumeCancel(t *testing.T) {
f := newFakeDocker()
f.archiveHang = true // server never answers the archive PUT
sp, _ := newTestSpawner(t, f)
if err := os.MkdirAll(filepath.Join(sp.reposDir, "group-project"), 0o755); err != nil {
if err := os.MkdirAll(filepath.Join(sp.reposDir, repoSlug("group/project")), 0o755); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(sp.reposDir, "group-project", "README.md"), []byte("x"), 0o644); err != nil {
if err := os.WriteFile(filepath.Join(sp.reposDir, repoSlug("group/project"), "README.md"), []byte("x"), 0o644); err != nil {
t.Fatal(err)
}
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
done := make(chan error, 1)
go func() { done <- sp.seedVolume(ctx, "group-project", "lvmh-repo-group-project") }()
go func() { done <- sp.seedVolume(ctx, repoSlug("group/project"), "lvmh-repo-group--project") }()
waitFor(t, 5*time.Second, func() bool { return f.hasCallSuffix(http.MethodPut, "/archive") })
cancel()
select {
@@ -379,9 +386,13 @@ func TestSpawnerSameRepoSpawnsSerialize(t *testing.T) {
}
func TestRepoSlugAndUUID(t *testing.T) {
if got := repoSlug("a/b/c"); got != "a-b-c" {
if got := repoSlug("a/b/c"); got != "a--b--c" {
t.Fatalf("repoSlug = %q", 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"))
}
id := newUUID()
if len(id) != 36 || id[8] != '-' || id[13] != '-' || id[18] != '-' || id[23] != '-' {
t.Fatalf("newUUID shape = %q", id)
@@ -391,6 +402,195 @@ func TestRepoSlugAndUUID(t *testing.T) {
}
}
func TestSpawnerCloneUsesHeaderAuthNotURLCredentials(t *testing.T) {
gitLog := useFakeGit(t, fakeGitModeOK)
f := newFakeDocker()
sp, store := newTestSpawner(t, f)
if err := store.SetSetting(settingGitLabToken, "pat-1"); err != nil {
t.Fatalf("set token: %v", err)
}
res, err := sp.Start(context.Background(), "group/project", "")
if err != nil {
t.Fatalf("Start: %v", err)
}
waitJobState(t, sp, res.SessionID, stateRunning)
calls := readGitLog(t, gitLog)
if len(calls) != 1 {
t.Fatalf("git calls = %v", calls)
}
call := calls[0]
// auth travels as a per-invocation -c http.extraHeader arg…
if !strings.HasPrefix(call, "-c http.extraHeader=Authorization: token pat-1 clone ") {
t.Fatalf("git call = %q, want -c http.extraHeader auth before clone", call)
}
// …and the URL recorded into .git/config stays credential-free.
if !strings.Contains(call, " -- https://gitlab.example/group/project.git ") {
t.Fatalf("git call = %q, want clean clone URL", call)
}
if strings.Contains(call, "pat-1@") {
t.Fatalf("git call = %q leaks the PAT into the URL", call)
}
}
func TestSpawnerJobsPrunedToCap(t *testing.T) {
f := newFakeDocker()
sp, _ := newTestSpawner(t, f)
for i := 0; i < maxSpawnJobs+10; i++ {
sp.setJob(fmt.Sprintf("s%d", i), "group/project", stateCloning, "", "")
}
jobs := sp.JobsSnapshot()
if len(jobs) != maxSpawnJobs {
t.Fatalf("jobs = %d, want capped at %d", len(jobs), maxSpawnJobs)
}
seen := map[string]bool{}
for _, j := range jobs {
seen[j.SessionID] = true
}
if seen["s0"] {
t.Fatal("oldest job must be pruned first")
}
for _, id := range []string{fmt.Sprintf("s%d", maxSpawnJobs), fmt.Sprintf("s%d", maxSpawnJobs+9)} {
if !seen[id] {
t.Fatalf("newest job %s pruned; kept = %v", id, seen)
}
}
}
func TestSpawnerFailedSeedRemovesRepoVolume(t *testing.T) {
useFakeGit(t, fakeGitModeOK)
f := newFakeDocker()
f.failArchive = true // CopyToContainer fails → seed fails
sp, _ := newTestSpawner(t, f)
res, err := sp.Start(context.Background(), "group/project", "")
if err != nil {
t.Fatalf("Start: %v", err)
}
job := waitJobState(t, sp, res.SessionID, stateError)
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") {
t.Fatal("failed seed must force-remove the repo volume for a fresh retry")
}
if f.volumeExists("lvmh-repo-group--project") {
t.Fatal("repo volume must not linger half-seeded")
}
}
func TestSpawnerFailedSeedSurvivesVolumeRemoveFailure(t *testing.T) {
useFakeGit(t, fakeGitModeOK)
f := newFakeDocker()
f.failArchive = true
f.failVolumeDelete = true // cleanup itself fails; seed error still surfaces
sp, _ := newTestSpawner(t, f)
res, err := sp.Start(context.Background(), "group/project", "")
if err != nil {
t.Fatalf("Start: %v", err)
}
job := waitJobState(t, sp, res.SessionID, stateError)
if !strings.Contains(job.Message, "seed") {
t.Fatalf("job message = %q, want seed failure despite cleanup failure", job.Message)
}
}
func TestSpawnerCloneOrUpdateBranchFetchFails(t *testing.T) {
useFakeGit(t, fakeGitModeFail) // branch probe and fetch both fail
f := newFakeDocker()
sp, _ := newTestSpawner(t, f)
slug := repoSlug("group/project")
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)
if err == nil || !strings.Contains(err.Error(), "git fetch") {
t.Fatalf("cloneOrUpdate branch fetch failure = %v, want git fetch error", err)
}
}
func TestSpawnerDeleteJobMissing(t *testing.T) {
sp, _ := newTestSpawner(t, newFakeDocker())
sp.deleteJob("never-existed") // no-op, must not panic
sp.setJob("s1", "group/project", stateRunning, "cid", "")
sp.deleteJob("s1")
for _, j := range sp.JobsSnapshot() {
if j.SessionID == "s1" {
t.Fatalf("job %s still present after deleteJob", j.SessionID)
}
}
}
func TestSpawnerCloneOrUpdateSwitchesBranch(t *testing.T) {
cases := []struct {
name string
branch string
headOut string
wantTail []string
}{
{
name: "same-branch-skips-fetch-checkout",
branch: "main",
headOut: "main\n",
wantTail: []string{"pull --ff-only"},
},
{
name: "different-branch-fetches-and-checks-out",
branch: "dev",
headOut: "main\n",
wantTail: []string{"fetch origin dev", "checkout dev", "pull --ff-only"},
},
{
name: "unknown-head-falls-through-to-fetch",
branch: "dev",
headOut: "",
wantTail: []string{"fetch origin dev", "checkout dev", "pull --ff-only"},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
gitLog := useFakeGit(t, fakeGitModeOK)
t.Setenv("FAKE_GIT_HEAD", tc.headOut)
f := newFakeDocker()
sp, store := newTestSpawner(t, f)
if err := store.SetSetting(settingGitLabToken, "pat-1"); err != nil {
t.Fatalf("set token: %v", err)
}
slug := repoSlug("group/project")
dir := filepath.Join(sp.reposDir, slug)
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 {
t.Fatalf("cloneOrUpdate: %v", err)
}
calls := readGitLog(t, gitLog)
if len(calls) != len(tc.wantTail)+1 { // +1: rev-parse probe
t.Fatalf("git calls = %v, want %v (+rev-parse)", calls, tc.wantTail)
}
if !strings.HasPrefix(calls[0], "-C ") || !strings.HasSuffix(calls[0], "rev-parse --abbrev-ref HEAD") {
t.Fatalf("first call = %q, want branch probe", calls[0])
}
authPrefix := "-c http.extraHeader=Authorization: token pat-1 "
for i, want := range tc.wantTail {
got := calls[i+1]
if want == "checkout dev" {
if got != want { // checkout needs no auth
t.Fatalf("call %d = %q, want %q", i+1, got, want)
}
continue
}
if got != authPrefix+want {
t.Fatalf("call %d = %q, want %q%q", i+1, got, authPrefix, want)
}
}
})
}
}
func TestEnvOr(t *testing.T) {
t.Setenv("LVMH_TEST_ENV_OR", " value ")
if got := envOr("LVMH_TEST_ENV_OR", "def"); got != "value" {
@@ -484,7 +684,7 @@ func TestGitRunSilentFailure(t *testing.T) {
func TestSpawnerNewBadDockerHost(t *testing.T) {
t.Setenv("DOCKER_HOST", "http://")
store := openTestStore(t)
if _, err := NewSpawner(store, NewHub(store), "https://gitlab.example"); err == nil {
if _, err := NewSpawner(context.Background(), store, NewHub(store), "https://gitlab.example"); err == nil {
t.Fatal("NewSpawner must fail on an unparseable DOCKER_HOST")
}
}
@@ -533,7 +733,7 @@ func TestSpawnerCloneOrUpdateErrors(t *testing.T) {
t.Fatalf("write file: %v", err)
}
sp.reposDir = file
if err := sp.cloneOrUpdate("group/project", "", "group-project"); err == nil {
if err := sp.cloneOrUpdate("group/project", "", repoSlug("group/project")); err == nil {
t.Fatal("cloneOrUpdate with file reposDir must fail")
}
}
@@ -570,7 +770,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["lvmh-repo-group--project"] = true // repo volume exists → skip seed
f.failVolumeCreate = true
sp, _ := newTestSpawner(t, f)
res, err := sp.Start(context.Background(), "group/project", "")
@@ -590,12 +790,12 @@ func TestSpawnerSeedVolumeCreateStartFail(t *testing.T) {
ctx := context.Background()
f.failCreate = true
if err := sp.seedVolume(ctx, "group-project", "lvmh-repo-group-project"); err == nil {
if err := sp.seedVolume(ctx, repoSlug("group/project"), "lvmh-repo-group--project"); err == nil {
t.Fatal("seedVolume with failing create must fail")
}
f.failCreate = false
f.failStart = true
if err := sp.seedVolume(ctx, "group-project", "lvmh-repo-group-project"); err == nil {
if err := sp.seedVolume(ctx, repoSlug("group/project"), "lvmh-repo-group--project"); err == nil {
t.Fatal("seedVolume with failing start must fail")
}
}
@@ -632,7 +832,7 @@ func TestSpawnerCloneOrUpdatePullFails(t *testing.T) {
useFakeGit(t, fakeGitModeFail)
f := newFakeDocker()
sp, _ := newTestSpawner(t, f)
dir := filepath.Join(sp.reposDir, "group-project")
dir := filepath.Join(sp.reposDir, repoSlug("group/project"))
if err := os.MkdirAll(filepath.Join(dir, ".git"), 0o755); err != nil {
t.Fatalf("mkdir .git: %v", err)
}
@@ -647,7 +847,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["lvmh-repo-group--project"] = true
sp, _ := newTestSpawner(t, f)
f.failCreate = true