CI boot tests deployed every Steam game from scratch, so each run downloaded the whole game from Steam again, from the runner's address. This gives CI the AI builder's Steam download cache, to keep that traffic low.
What changes
scripts/boot-test.sh runs the new gbrgames boot-test ID. That's the builder's own deploy path: the same payload, deployed by the same node-agent BootTest.
Steam games (installMethod: STEAMCMD) start from the cache volume (gbr-steamcache-<app>). The node-agent's install then only checks the files.
The cache is updated at most once a day. A timestamp in the cache volume, not copied into servers, records when. A repeat run within a day doesn't ask Steam for an update at all, and the node-agent's own install still fetches anything newer.
RUNNER.md explains the cache. Its prune command already keeps these volumes.
Together with gbreed-public/gbr-node-agent#31, which logs in once per install attempt, a repeat boot test of a Steam game now costs Steam one warm-up login and one file check, not a full download.
Tested
go test ./... passes, including a test for the cache timestamp's age.
Ran gbrgames boot-test twice locally for a Steam template, with the agent from #31. Both passed. The first run filled the cache. The second skipped the update ("updated in the last 24h"), seeded from the cache, and the agent's install only checked the files.
Merge after #31, or in either order: each works on its own.
CI boot tests deployed every Steam game from scratch, so each run downloaded the whole game from Steam again, from the runner's address. This gives CI the AI builder's Steam download cache, to keep that traffic low.
**What changes**
- `scripts/boot-test.sh` runs the new `gbrgames boot-test ID`. That's the builder's own deploy path: the same payload, deployed by the same node-agent BootTest.
- Steam games (`installMethod: STEAMCMD`) start from the cache volume (`gbr-steamcache-<app>`). The node-agent's install then only checks the files.
- The cache is updated at most **once a day**. A timestamp in the cache volume, not copied into servers, records when. A repeat run within a day doesn't ask Steam for an update at all, and the node-agent's own install still fetches anything newer.
- RUNNER.md explains the cache. Its prune command already keeps these volumes.
Together with gbreed-public/gbr-node-agent#31, which logs in once per install attempt, a repeat boot test of a Steam game now costs Steam one warm-up login and one file check, not a full download.
**Tested**
- `go test ./...` passes, including a test for the cache timestamp's age.
- Ran `gbrgames boot-test` twice locally for a Steam template, with the agent from #31. Both passed. The first run filled the cache. The second skipped the update ("updated in the last 24h"), seeded from the cache, and the agent's install only checked the files.
Merge after #31, or in either order: each works on its own.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
CI deployed every Steam game from scratch on each run, downloading the whole
game from Steam each time. scripts/boot-test.sh now runs the new
`gbrgames boot-test ID`, the builder's own deploy path: the same payload and
node-agent BootTest, but Steam games start from the cache volume
(gbr-steamcache-<app>, kept by RUNNER.md's prune).
The cache is updated at most once a day (a timestamp in the cache volume, not
copied into servers), so a repeat run within a day doesn't ask Steam for an
update at all; the node-agent's own install still checks the files and fetches
anything newer.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
But steamCacheUpdatedWithin runs docker run -v cache:/c:ro alpine cat …, and Docker auto-creates a named volume if it doesn't already exist. On the first-ever run for a given app ID, this creates the volume without the gbr.cache=steam label. The subsequent docker volume create --label gbr.cache=steam cache is then a no-op (the volume already exists), so the label is never set.
— will delete the cache volume, defeating the purpose of the cache and contradicting the PR description ("The prune above keeps these volumes").
Fix: Move (or duplicate) the labelled volume creation before the freshness check, so the volume always gets the label on first creation:
cache:="gbr-steamcache-"+appID// Ensure the volume exists with its protection label before any docker run auto-creates itrunCmd("docker","volume","create","--label","gbr.cache=steam",cache)if!steamCacheFresh[appID]&&steamCacheUpdatedWithin(cache,steamCacheMaxAge){...}if!steamCacheFresh[appID]{// … SteamCMD update …}
docker volume create is idempotent (no-op if the name already exists), so this is safe on every invocation.
2. Non-blocking suggestions
No timeout on the SteamCMD update container. If the network stalls mid-download, docker run … cm2network/steamcmd … will hang indefinitely and block the CI job. Consider wrapping the call in exec.CommandContext with a generous timeout (e.g. 20 min) or adding timeout to the shell script inside the container.
Timestamp write error is silently discarded. After a successful SteamCMD update:
If this fails (disk full, permissions), the cache is populated but the next run will re-download. The failure mode is safe (extra download), but a fmt.Printf on error would aid debugging.
The runAgentBootTest cleanup calls ignore errors intentionally, but the pattern is worth a comment.runCmd("docker", "rm", "-f", "-v", container) and the volume removal are best-effort; a brief // best-effort cleanup of any leftovers from a previous run would make the intent explicit for future readers (though the existing comment above partially covers it).
3. Tests
TestSteamCacheStampAge is a good unit test for the pure parsing logic. ✔
The decision tree in seedSteamApp (fresh → skip update; not fresh → update; copy always) is only exercised end-to-end against a real Docker daemon. A lightweight test that stubs runCmd (e.g. via an interface or function variable) and asserts the sequence of calls for the "fresh" vs "stale" paths would guard against regressions without needing Docker in CI.
No test covers the exit-code contract of runBootTest (exit 3 for missing secret, exit 1 for failure). This is hard to test in-process because of os.Exit, but extracting the verdict into a return value and testing that, with the actual os.Exit in a thin wrapper, would make the contract explicit.
4. Overall assessment
The design is sound and the implementation is clean: the cache logic is straightforward, the fallback paths are safe (agent does a full download on any failure), and the shell-script simplification is a nice win. The one blocking issue (volume label lost to docker run auto-creation) is a small code-ordering fix. Once that's addressed, the PR is ready to merge.
Automated review by AI Git Bot
## 🤖 AI Code Review
## 1. Blocking issues
**Cache volume loses its protection label on first creation, so the weekly prune will delete it.**
`steamCacheUpdatedWithin` is called before the volume is explicitly created:
```go
if !steamCacheFresh[appID] && steamCacheUpdatedWithin(cache, steamCacheMaxAge) { ... }
if !steamCacheFresh[appID] {
...
runCmd("docker", "volume", "create", "--label", "gbr.cache=steam", cache)
...
}
```
But `steamCacheUpdatedWithin` runs `docker run -v cache:/c:ro alpine cat …`, and Docker **auto-creates a named volume** if it doesn't already exist. On the first-ever run for a given app ID, this creates the volume *without* the `gbr.cache=steam` label. The subsequent `docker volume create --label gbr.cache=steam cache` is then a no-op (the volume already exists), so the label is never set.
Consequence: the weekly cleanup in RUNNER.md —
```bash
docker volume prune -af --filter "label!=gbr.cache"
```
— will delete the cache volume, defeating the purpose of the cache and contradicting the PR description ("The prune above keeps these volumes").
**Fix:** Move (or duplicate) the labelled volume creation *before* the freshness check, so the volume always gets the label on first creation:
```go
cache := "gbr-steamcache-" + appID
// Ensure the volume exists with its protection label before any docker run auto-creates it
runCmd("docker", "volume", "create", "--label", "gbr.cache=steam", cache)
if !steamCacheFresh[appID] && steamCacheUpdatedWithin(cache, steamCacheMaxAge) { ... }
if !steamCacheFresh[appID] {
// … SteamCMD update …
}
```
`docker volume create` is idempotent (no-op if the name already exists), so this is safe on every invocation.
---
## 2. Non-blocking suggestions
- **No timeout on the SteamCMD update container.** If the network stalls mid-download, `docker run … cm2network/steamcmd …` will hang indefinitely and block the CI job. Consider wrapping the call in `exec.CommandContext` with a generous timeout (e.g. 20 min) or adding `timeout` to the shell script inside the container.
- **Timestamp write error is silently discarded.** After a successful SteamCMD update:
```go
runCmd("docker", "run", "--rm", "-v", cache+":/server", "alpine", "sh", "-c",
fmt.Sprintf("date +%%s > /server/%s", steamCacheStamp))
```
If this fails (disk full, permissions), the cache is populated but the next run will re-download. The failure mode is safe (extra download), but a `fmt.Printf` on error would aid debugging.
- **The `runAgentBootTest` cleanup calls ignore errors intentionally, but the pattern is worth a comment.** `runCmd("docker", "rm", "-f", "-v", container)` and the volume removal are best-effort; a brief `// best-effort cleanup of any leftovers from a previous run` would make the intent explicit for future readers (though the existing comment above partially covers it).
---
## 3. Tests
- `TestSteamCacheStampAge` is a good unit test for the pure parsing logic. ✔
- The decision tree in `seedSteamApp` (fresh → skip update; not fresh → update; copy always) is only exercised end-to-end against a real Docker daemon. A lightweight test that stubs `runCmd` (e.g. via an interface or function variable) and asserts the sequence of calls for the "fresh" vs "stale" paths would guard against regressions without needing Docker in CI.
- No test covers the exit-code contract of `runBootTest` (exit 3 for missing secret, exit 1 for failure). This is hard to test in-process because of `os.Exit`, but extracting the verdict into a return value and testing that, with the actual `os.Exit` in a thin wrapper, would make the contract explicit.
---
## 4. Overall assessment
The design is sound and the implementation is clean: the cache logic is straightforward, the fallback paths are safe (agent does a full download on any failure), and the shell-script simplification is a nice win. The one blocking issue (volume label lost to `docker run` auto-creation) is a small code-ordering fix. Once that's addressed, the PR is ready to merge.
---
*Automated review by AI Git Bot*
greg
merged commit 1fcf05cc27 into main2026-10-26 23:28:24 +00: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.
CI boot tests deployed every Steam game from scratch, so each run downloaded the whole game from Steam again, from the runner's address. This gives CI the AI builder's Steam download cache, to keep that traffic low.
What changes
scripts/boot-test.shruns the newgbrgames boot-test ID. That's the builder's own deploy path: the same payload, deployed by the same node-agent BootTest.installMethod: STEAMCMD) start from the cache volume (gbr-steamcache-<app>). The node-agent's install then only checks the files.Together with gbreed-public/gbr-node-agent#31, which logs in once per install attempt, a repeat boot test of a Steam game now costs Steam one warm-up login and one file check, not a full download.
Tested
go test ./...passes, including a test for the cache timestamp's age.gbrgames boot-testtwice locally for a Steam template, with the agent from #31. Both passed. The first run filled the cache. The second skipped the update ("updated in the last 24h"), seeded from the cache, and the agent's install only checked the files.Merge after #31, or in either order: each works on its own.
🤖 Generated with Claude Code
🤖 AI Code Review
1. Blocking issues
Cache volume loses its protection label on first creation, so the weekly prune will delete it.
steamCacheUpdatedWithinis called before the volume is explicitly created:But
steamCacheUpdatedWithinrunsdocker run -v cache:/c:ro alpine cat …, and Docker auto-creates a named volume if it doesn't already exist. On the first-ever run for a given app ID, this creates the volume without thegbr.cache=steamlabel. The subsequentdocker volume create --label gbr.cache=steam cacheis then a no-op (the volume already exists), so the label is never set.Consequence: the weekly cleanup in RUNNER.md —
— will delete the cache volume, defeating the purpose of the cache and contradicting the PR description ("The prune above keeps these volumes").
Fix: Move (or duplicate) the labelled volume creation before the freshness check, so the volume always gets the label on first creation:
docker volume createis idempotent (no-op if the name already exists), so this is safe on every invocation.2. Non-blocking suggestions
No timeout on the SteamCMD update container. If the network stalls mid-download,
docker run … cm2network/steamcmd …will hang indefinitely and block the CI job. Consider wrapping the call inexec.CommandContextwith a generous timeout (e.g. 20 min) or addingtimeoutto the shell script inside the container.Timestamp write error is silently discarded. After a successful SteamCMD update:
If this fails (disk full, permissions), the cache is populated but the next run will re-download. The failure mode is safe (extra download), but a
fmt.Printfon error would aid debugging.The
runAgentBootTestcleanup calls ignore errors intentionally, but the pattern is worth a comment.runCmd("docker", "rm", "-f", "-v", container)and the volume removal are best-effort; a brief// best-effort cleanup of any leftovers from a previous runwould make the intent explicit for future readers (though the existing comment above partially covers it).3. Tests
TestSteamCacheStampAgeis a good unit test for the pure parsing logic. ✔seedSteamApp(fresh → skip update; not fresh → update; copy always) is only exercised end-to-end against a real Docker daemon. A lightweight test that stubsrunCmd(e.g. via an interface or function variable) and asserts the sequence of calls for the "fresh" vs "stale" paths would guard against regressions without needing Docker in CI.runBootTest(exit 3 for missing secret, exit 1 for failure). This is hard to test in-process because ofos.Exit, but extracting the verdict into a return value and testing that, with the actualos.Exitin a thin wrapper, would make the contract explicit.4. Overall assessment
The design is sound and the implementation is clean: the cache logic is straightforward, the fallback paths are safe (agent does a full download on any failure), and the shell-script simplification is a nice win. The one blocking issue (volume label lost to
docker runauto-creation) is a small code-ordering fix. Once that's addressed, the PR is ready to merge.Automated review by AI Git Bot