fix(build): clear a killed boot test's container and volume first #16

2026-10-25T22:32:48Z

greg commented

2026-10-25 22:32:48 +00:00

A worker restarted mid-build (watchtower installing a new image) leaves the boot test's container and volume behind, and the next boot test of that game deploys under the same name.

bootCheck now removes boottest-<id> and its volume before each boot test.

Restarts that kill a running build stay as they are (latest code first); the job is simply rerun.

🤖 Generated with Claude Code

A worker restarted mid-build (watchtower installing a new image) leaves the boot test's container and volume behind, and the next boot test of that game deploys under the same name. `bootCheck` now removes `boottest-<id>` and its volume before each boot test. Restarts that kill a running build stay as they are (latest code first); the job is simply rerun. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

👀 1

greg added 1 commit 2026-10-25 22:32:48 +00:00

fix(build): clear a killed boot test's container and volume first

Validate game templates / validate (push) Successful in 20s

Details

Validate game templates / boot-test (push) Skipped

Details

Validate game templates / publish (push) Skipped

Details

e9e78610d6

When the worker is restarted mid-build (watchtower updating its image), the
boot test's container and volume are left behind, and the next boot test of
that game deploys under the same name. Remove them before each boot test.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

git_ai_bot reviewed

2026-10-25 22:33:16 +00:00

git_ai_bot left a comment

🤖 AI Code Review

1. Blocking issues

None.

2. Non-blocking suggestions

Ignored runCmd errors are fine, but a brief log on failure would help debugging. Both docker rm -f and docker volume rm -f will return an error when the resource doesn't exist (which is the common case). Ignoring those is correct. However, if they fail for a different reason (Docker daemon unreachable, permission error, container stuck in a bad state), you get no signal before the boot test proceeds and potentially fails in a confusing way. A one-line fmt.Println on error would make production debugging easier:
```
if out, err := runCmd("docker", "rm", "-f", "boottest-"+id); err != nil {
    fmt.Printf("[build] cleanup: docker rm: %v\n", out)
}
```
This is purely cosmetic for the happy path (the commands are no-ops when nothing exists) and only prints when something unexpected happened.
Verify the volume name suffix. The cleanup hard-codes boottest-<id>-data. This presumably matches what the Java BootTest application creates (likely via a Spring/Compose convention). If that naming is ever refactored on the agent side, this line will silently stop cleaning up. A short comment like // name must match the volume BootTest creates would make the coupling explicit for the next reader. (The container name boottest-<id> has the same coupling, but it's already referenced in the existing docker run path, so it's less surprising.)

3. Tests

The change is two best-effort shell commands at the top of bootCheck. The existing boottest_test.go / build_test.go presumably mock or skip the Docker calls. No new test is needed for "container doesn't exist → error is ignored," but if the test harness already stubs runCmd, it would be worth asserting that the two cleanup calls are made with the expected names (boottest-<id>, boottest-<id>-data) so a future rename is caught.

4. Overall assessment

Small, well-scoped operational fix. The problem (orphaned container + volume after a killed worker) is real, the solution (idempotent rm -f before each deploy) is the standard pattern, and the placement at the top of bootCheck is logical. No correctness, security, or compatibility concerns. Ship it.

Automated review by AI Git Bot

## 🤖 AI Code Review ## 1. Blocking issues None. ## 2. Non-blocking suggestions - **Ignored `runCmd` errors are fine, but a brief log on failure would help debugging.** Both `docker rm -f` and `docker volume rm -f` will return an error when the resource doesn't exist (which is the common case). Ignoring those is correct. However, if they fail for a *different* reason (Docker daemon unreachable, permission error, container stuck in a bad state), you get no signal before the boot test proceeds and potentially fails in a confusing way. A one-line `fmt.Println` on error would make production debugging easier: ```go if out, err := runCmd("docker", "rm", "-f", "boottest-"+id); err != nil { fmt.Printf("[build] cleanup: docker rm: %v\n", out) } ``` This is purely cosmetic for the happy path (the commands are no-ops when nothing exists) and only prints when something unexpected happened. - **Verify the volume name suffix.** The cleanup hard-codes `boottest-<id>-data`. This presumably matches what the Java `BootTest` application creates (likely via a Spring/Compose convention). If that naming is ever refactored on the agent side, this line will silently stop cleaning up. A short comment like `// name must match the volume BootTest creates` would make the coupling explicit for the next reader. (The container name `boottest-<id>` has the same coupling, but it's already referenced in the existing `docker run` path, so it's less surprising.) ## 3. Tests The change is two best-effort shell commands at the top of `bootCheck`. The existing `boottest_test.go` / `build_test.go` presumably mock or skip the Docker calls. No new test is needed for "container doesn't exist → error is ignored," but if the test harness already stubs `runCmd`, it would be worth asserting that the two cleanup calls *are* made with the expected names (`boottest-<id>`, `boottest-<id>-data`) so a future rename is caught. ## 4. Overall assessment Small, well-scoped operational fix. The problem (orphaned container + volume after a killed worker) is real, the solution (idempotent `rm -f` before each deploy) is the standard pattern, and the placement at the top of `bootCheck` is logical. No correctness, security, or compatibility concerns. Ship it. --- *Automated review by AI Git Bot*

openhands merged commit ef4d72621d into main

2026-10-25 22:33:31 +00:00

openhands deleted branch fix/worker-finish-job-on-stop

2026-10-25 22:33:31 +00:00

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: gbreed-public/gbr-games#16