HandleWebhook triggers a deployment in a detached goroutine that keeps writing under the app data directory (the test's t.TempDir). The webhook tests slept 100ms and returned, racing Go's automatic TempDir cleanup and intermittently failing with RemoveAll: directory not empty.
Change: the webhook Service now tracks those goroutines in a sync.WaitGroup (started via WaitGroup.Go) and exposes WaitForDeployments. The tests wait on it instead of sleeping. Deploy joins its own sub-goroutines (the log writer) before returning, so the wait covers every temp-dir write. ExtractBranch coverage is unchanged; production behavior is unchanged apart from making completion observable.
Verification:
go test -race -run TestExtractBranch -count=20 ./internal/service/webhook — green, no RemoveAll errors (run twice: before and after the lint-driven refactor to WaitGroup.Go).
make check via the Docker gate (make docker): green — fmt-check, lint, and test all ran inside the pinned image.
Note: host make check panics in golangci-lint (file requires newer Go version go1.26) because the host has go1.26 while the host linter binary is older; the Docker gate pins go1.25 + golangci-lint v2.12.2 and is the authoritative check. Pre-existing, unrelated to this change.
Model: opus-4-8
Fixes https://git.eeqj.de/sneak/upaas/issues/198.
`HandleWebhook` triggers a deployment in a detached goroutine that keeps writing under the app data directory (the test's `t.TempDir`). The webhook tests slept 100ms and returned, racing Go's automatic `TempDir` cleanup and intermittently failing with `RemoveAll: directory not empty`.
Change: the webhook `Service` now tracks those goroutines in a `sync.WaitGroup` (started via `WaitGroup.Go`) and exposes `WaitForDeployments`. The tests wait on it instead of sleeping. `Deploy` joins its own sub-goroutines (the log writer) before returning, so the wait covers every temp-dir write. `ExtractBranch` coverage is unchanged; production behavior is unchanged apart from making completion observable.
Verification:
- `go test -race -run TestExtractBranch -count=20 ./internal/service/webhook` — green, no `RemoveAll` errors (run twice: before and after the lint-driven refactor to `WaitGroup.Go`).
- `make check` via the Docker gate (`make docker`): green — fmt-check, lint, and test all ran inside the pinned image.
Note: host `make check` panics in `golangci-lint` (`file requires newer Go version go1.26`) because the host has go1.26 while the host linter binary is older; the Docker gate pins go1.25 + golangci-lint v2.12.2 and is the authoritative check. Pre-existing, unrelated to this change.
Model: opus-4-8
HandleWebhook starts a deployment in a detached goroutine that keeps
writing under the app data dir (the test's t.TempDir). The webhook
tests slept 100ms and returned, racing Go's automatic TempDir cleanup
and intermittently failing with "RemoveAll: directory not empty".
Track those goroutines in a sync.WaitGroup on the webhook Service and
expose WaitForDeployments; the tests now wait on it instead of
sleeping. Deploy joins its own sub-goroutines before returning, so the
wait covers every temp-dir write. Production behavior is unchanged
apart from making completion observable.
Model: opus-4-8
PASS: tracking the async deployment goroutine in a sync.WaitGroup and having the webhook tests wait on WaitForDeployments instead of sleeping deterministically removes the t.TempDir cleanup race for #198.
Rebased onto current next (trivial TODO.md-only conflict; both Completed Steps entries kept) and gated the rebased tree, which is green; the PR is now mergeable.
Disclosure: WaitForDeployments is test-only, and its "every write under the data directory" wording is slightly broader than the code — the detached checkHealthAfterDelay health-check goroutine is not tracked — but that path is unreachable here (the test deploys fail at clone), so it does not affect the fix.
Model: opus-4-8
PASS: tracking the async deployment goroutine in a `sync.WaitGroup` and having the webhook tests wait on `WaitForDeployments` instead of sleeping deterministically removes the `t.TempDir` cleanup race for https://git.eeqj.de/sneak/upaas/issues/198.
Rebased onto current `next` (trivial `TODO.md`-only conflict; both Completed Steps entries kept) and gated the rebased tree, which is green; the PR is now mergeable.
Disclosure: `WaitForDeployments` is test-only, and its "every write under the data directory" wording is slightly broader than the code — the detached `checkHealthAfterDelay` health-check goroutine is not tracked — but that path is unreachable here (the test deploys fail at clone), so it does not affect the fix.
Model: opus-4-8
clawbot
merged commit f2e4be5eed into next2026-09-22 12:28:23 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #198.
HandleWebhooktriggers a deployment in a detached goroutine that keeps writing under the app data directory (the test'st.TempDir). The webhook tests slept 100ms and returned, racing Go's automaticTempDircleanup and intermittently failing withRemoveAll: directory not empty.Change: the webhook
Servicenow tracks those goroutines in async.WaitGroup(started viaWaitGroup.Go) and exposesWaitForDeployments. The tests wait on it instead of sleeping.Deployjoins its own sub-goroutines (the log writer) before returning, so the wait covers every temp-dir write.ExtractBranchcoverage is unchanged; production behavior is unchanged apart from making completion observable.Verification:
go test -race -run TestExtractBranch -count=20 ./internal/service/webhook— green, noRemoveAllerrors (run twice: before and after the lint-driven refactor toWaitGroup.Go).make checkvia the Docker gate (make docker): green — fmt-check, lint, and test all ran inside the pinned image.Note: host
make checkpanics ingolangci-lint(file requires newer Go version go1.26) because the host has go1.26 while the host linter binary is older; the Docker gate pins go1.25 + golangci-lint v2.12.2 and is the authoritative check. Pre-existing, unrelated to this change.Model: opus-4-8
c377eb2f5eto9c3fd1d9eaPASS: tracking the async deployment goroutine in a
sync.WaitGroupand having the webhook tests wait onWaitForDeploymentsinstead of sleeping deterministically removes thet.TempDircleanup race for #198.Rebased onto current
next(trivialTODO.md-only conflict; both Completed Steps entries kept) and gated the rebased tree, which is green; the PR is now mergeable.Disclosure:
WaitForDeploymentsis test-only, and its "every write under the data directory" wording is slightly broader than the code — the detachedcheckHealthAfterDelayhealth-check goroutine is not tracked — but that path is unreachable here (the test deploys fail at clone), so it does not affect the fix.Model: opus-4-8