api: add /builders CRUD and prune endpoints - #341
Conversation
✱ Stainless preview builds for hypemanThis PR will update the Edit this comment to update it. It will appear in the SDK's changelogs. ✅ hypeman-openapi studio · code · diff
✅ hypeman-typescript studio · code · diff
✅ hypeman-go studio · code · diff
This comment is auto-generated by GitHub Actions and is automatically kept up to date as you push. |
8820a02 to
ac772e4
Compare
ac772e4 to
a6f1763
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Prune misses not-found race
- PruneBuilder now maps ResetDisk returning ErrNotFound to a 404 not_found response, matching the documented race handling used by DeleteBuilder.
Or push these changes by commenting:
@cursor push d305f20bd3
Preview (d305f20bd3)
diff --git a/cmd/api/api/builders.go b/cmd/api/api/builders.go
--- a/cmd/api/api/builders.go
+++ b/cmd/api/api/builders.go
@@ -149,6 +149,13 @@
log := logger.FromContext(ctx)
if err := s.BuilderManager.ResetDisk(ctx, b.ID); err != nil {
+ if errors.Is(err, builders.ErrNotFound) {
+ // Deleted between resolution and this call (e.g. idle reaper)
+ return oapi.PruneBuilder404JSONResponse{
+ Code: "not_found",
+ Message: "builder not found",
+ }, nil
+ }
if errors.Is(err, builders.ErrInUse) {
return oapi.PruneBuilder409JSONResponse{
Code: "conflict",You can send follow-ups to the cloud agent here.
a6f1763 to
7ae12ff
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Oversized disk create returns 500
- Added a dedicated invalid-disk-size sentinel error and mapped it to a 400 response so oversized
disk_size_gbrequests no longer fall through to 500.
- Added a dedicated invalid-disk-size sentinel error and mapped it to a 400 response so oversized
Or push these changes by commenting:
@cursor push 08c1fedb0b
Preview (08c1fedb0b)
diff --git a/cmd/api/api/builders.go b/cmd/api/api/builders.go
--- a/cmd/api/api/builders.go
+++ b/cmd/api/api/builders.go
@@ -71,6 +71,11 @@
Code: "invalid_request",
Message: err.Error(),
}, nil
+ case errors.Is(err, builders.ErrInvalidDiskSize):
+ return oapi.CreateBuilder400JSONResponse{
+ Code: "invalid_request",
+ Message: err.Error(),
+ }, nil
case errors.Is(err, tags.ErrInvalidTags):
return oapi.CreateBuilder400JSONResponse{
Code: "invalid_request",
diff --git a/cmd/api/api/builders_test.go b/cmd/api/api/builders_test.go
--- a/cmd/api/api/builders_test.go
+++ b/cmd/api/api/builders_test.go
@@ -6,6 +6,7 @@
"github.com/kernel/hypeman/lib/builders"
"github.com/kernel/hypeman/lib/oapi"
+ "github.com/kernel/hypeman/lib/paths"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@@ -97,6 +98,30 @@
assert.True(t, ok, "expected 400 response")
}
+func TestCreateBuilder_DiskSizeExceedsMaximum(t *testing.T) {
+ t.Parallel()
+ svc := newTestService(t)
+
+ builderMgr, err := builders.NewManager(
+ paths.New(svc.Config.DataDir),
+ builders.Config{MaxDiskSizeGb: 10},
+ svc.VolumeManager,
+ svc.InstanceManager,
+ nil,
+ nil,
+ )
+ require.NoError(t, err)
+ svc.BuilderManager = builderMgr
+
+ sizeGb := 11
+ resp, err := svc.CreateBuilder(ctx(), oapi.CreateBuilderRequestObject{
+ Body: &oapi.CreateBuilderRequest{DiskSizeGb: &sizeGb},
+ })
+ require.NoError(t, err)
+ _, ok := resp.(oapi.CreateBuilder400JSONResponse)
+ assert.True(t, ok, "expected 400 response")
+}
+
func TestGetBuilder(t *testing.T) {
t.Parallel()
svc := newTestService(t)
diff --git a/lib/builders/errors.go b/lib/builders/errors.go
--- a/lib/builders/errors.go
+++ b/lib/builders/errors.go
@@ -19,4 +19,8 @@
// ErrQuotaExceeded is returned when a create would exceed the configured
// builder count limit
ErrQuotaExceeded = errors.New("builder quota exceeded")
+
+ // ErrInvalidDiskSize is returned when the requested disk size is invalid
+ // for the current configuration
+ ErrInvalidDiskSize = errors.New("invalid builder disk size")
)
diff --git a/lib/builders/manager.go b/lib/builders/manager.go
--- a/lib/builders/manager.go
+++ b/lib/builders/manager.go
@@ -156,7 +156,7 @@
sizeGb = m.config.DefaultDiskSizeGb
}
if m.config.MaxDiskSizeGb > 0 && sizeGb > m.config.MaxDiskSizeGb {
- return nil, fmt.Errorf("disk_size_gb %d exceeds maximum of %d", sizeGb, m.config.MaxDiskSizeGb)
+ return nil, fmt.Errorf("%w: disk_size_gb %d exceeds maximum of %d", ErrInvalidDiskSize, sizeGb, m.config.MaxDiskSizeGb)
}
m.mu.Lock()
diff --git a/lib/builders/manager_test.go b/lib/builders/manager_test.go
--- a/lib/builders/manager_test.go
+++ b/lib/builders/manager_test.go
@@ -115,6 +115,7 @@
mgr, _, _, _ := setupTestManager(t, Config{MaxCount: 1, MaxDiskSizeGb: 60})
_, err := mgr.CreateBuilder(context.Background(), CreateBuilderRequest{DiskSizeGb: 61})
+ assert.ErrorIs(t, err, ErrInvalidDiskSize)
assert.ErrorContains(t, err, "exceeds maximum")
_, err = mgr.CreateBuilder(context.Background(), CreateBuilderRequest{DiskSizeGb: 60})You can send follow-ups to the cloud agent here.
fb3c065 to
4c29267
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Misleading in-use conflict message
- Delete and prune now map builders.ErrInUse to the generic message “builder is in use” so conflicts are accurate across all in-use states.
Or push these changes by commenting:
@cursor push 96e8ff4ffd
Preview (96e8ff4ffd)
diff --git a/cmd/api/api/builders.go b/cmd/api/api/builders.go
--- a/cmd/api/api/builders.go
+++ b/cmd/api/api/builders.go
@@ -129,7 +129,7 @@
if errors.Is(err, builders.ErrInUse) {
return oapi.DeleteBuilder409JSONResponse{
Code: "conflict",
- Message: "builder is in use by a build",
+ Message: "builder is in use",
}, nil
}
log.ErrorContext(ctx, "failed to delete builder", "error", err)
@@ -164,7 +164,7 @@
if errors.Is(err, builders.ErrInUse) {
return oapi.PruneBuilder409JSONResponse{
Code: "conflict",
- Message: "builder is in use by a build",
+ Message: "builder is in use",
}, nil
}
log.ErrorContext(ctx, "failed to prune builder", "error", err)You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 4c29267. Configure here.
00abce8 to
8495083
Compare
hiroTamada
left a comment
There was a problem hiding this comment.
reviewed — overall this looks good. a few suggestions worth considering:
API shape
openapi.yaml:1836-1888— consider modeling this as a build-cache resource (/build-caches,cache_id) since the durable resource is the cache disk while builder VMs are ephemeral. alternatively, nesting the cache configuration undercachewould leave room for future builder settings.
Implementation
openapi.yaml:1882-1885/cmd/api/api/builders.go:51-52— an explicitdisk_size_gbof zero or a negative value is treated as omission and replaced with the default. consider addingminimum: 1and returning400for non-positive values.cmd/api/api/builders.go:156-178—ResetDiskstarts its background worker before the handler re-reads the builder, so the worker may finish first and the202response may containready. returning the accepted transition snapshot directly would make the response and test deterministic.openapi.yaml:1836/lib/oapi/oapi.go:145-150— addingBuilderStatusrenames the existingImageStatusReady,ImageStatusFailed, etc. constants to package-globalReady,Failed, and so on. consider pinning stable enum names so adding this schema does not change the existing generated API.
Nits
cmd/api/api/api_test.go:137-146— thectxWithVolumecomment now documentsctxWithBuilder; add the builder comment and move the volume comment back.cmd/api/config/config_test.go:549-555— consider adding a negativeMaxDiskSizeGbcase for the corresponding validation branch.
8495083 to
4dffe91
Compare
4dffe91 to
4e83fe9
Compare
sjmiller609
left a comment
There was a problem hiding this comment.
Some in-line comments for clean up of wording in API
bugs
stainless.yaml— never updated;/buildersendpoints missing from SDK mapping stack-widebuilders.go:58— oversizedisk_size_gbreturns 500; needs sentinel error + 400 arm (mentioned in #340 already)
abstract builder tooling
- optionally add
typefield (default"buildkit") now - schema descriptions hardcode "BuildKit cache disk"; keep wording generic
findings
openapi.yaml—idclaims "idempotency" but retry returns 409builders.go:161— prune re-reads builder;ResetDiskshould return it (race → spurious 500)
nits
openapi.yaml—disk_size_gblacksminimum: 1; could protect against in schema- 409 message "in use by a build" wrong when builder is mid-prune (
ErrInUseconflation) idexample"bldr-abc123"— generated IDs have no prefix, make example look like it actually looks- generic ptr helpers (
valueOrEmptyetc.) stranded inbuilders.go idle_ttlparsed twice (Validate + provider); can driftBuilderManager.Starterror branch is dead code; reconcile failures swallowed- default disk size defaulted in two places (config + manager)
4e83fe9 to
d69245d
Compare
|
@hiroTamada @sjmiller609 addressed the API/config feedback in d69245d:
I kept the |
Expose the Builder resource over the API. POST /builders creates a
builder with an optional caller-supplied ID (validated, conflicts on
replay for control-plane idempotency), optional non-unique name, tags,
and disk size; the cache disk is provisioned eagerly and 201 returns the
builder. GET /builders lists with tag filtering and always returns a
non-nil array; GET /builders/{id} returns one builder; DELETE
/builders/{id} removes builder and disk (409 while in use); POST
/builders/{id}/prune resets the cache disk asynchronously and returns
202 with status pruning. Responses carry id, name, disk_size_gb, status,
tags, created_at, and last_used_at only. Every builder route documents
the 401 bearer-auth failure response.
Builders resolve by opaque ID through the resource resolver middleware;
builder:read/write/delete scopes gate the routes. The builders manager
is wired through config (builders.max_count, default_disk_size_gb,
max_disk_size_gb, idle_ttl), providers, and the generated injector, and
started with the server so startup reconciliation runs. Regenerating
oapi.go renamed the ImageStatus enum constants (oapi-codegen conflict
resolution with the new BuilderStatus enum); the one referencing test is
updated.
Handler tests cover create (defaults, custom ID, invalid ID, idempotent
replay), get, list with tag filtering, delete, prune, and 409s while a
build holds the builder. Config validation tests cover the new
builders.* settings.
PruneBuilder mapped ErrInUse to 409 but not ErrNotFound, so a builder removed by the idle reaper between resolution and ResetDisk surfaced as 500 instead of the documented 404, unlike DeleteBuilder.
The reset runs in a background goroutine; the test returned while the disk was still being recreated, racing TempDir cleanup on darwin (directory not empty).
The manager now returns ErrDiskSizeExceeded; map it to a client error instead of falling through to 500.
builders.ErrInUse also covers pruning, deleting, and an attached disk, so the 409 from delete/prune no longer claims the conflict is always a build.
d69245d to
a41df03
Compare


Expose the Builder resource over the API. POST /builders creates a
builder with an optional caller-supplied ID (validated; replay conflicts), optional
non-unique name, tags,
and disk size; the cache disk is provisioned eagerly and 201 returns the
builder. GET /builders lists with tag filtering and always returns a
non-nil array; GET /builders/{id} returns one builder; DELETE
/builders/{id} removes builder and disk (409 while in use); POST
/builders/{id}/prune resets the cache disk asynchronously and returns
202 with status pruning. Responses carry id, name, disk_size_gb, status,
tags, created_at, and last_used_at only. Every builder route documents
the 401 bearer-auth failure response.
Builders resolve by opaque ID through the resource resolver middleware;
builder:read/write/delete scopes gate the routes. The builders manager
is wired through config (builders.max_count, default_disk_size_gb,
max_disk_size_gb, idle_ttl), providers, and the generated injector, and
started with the server so startup reconciliation runs. OpenAPI pins generated enum constant names so adding BuilderStatus does not
rename the existing ImageStatus or BuildStatus API.
stainless.yamlmaps everyBuilder endpoint and model into SDK previews.
Handler tests cover create (defaults, positive-size enforcement, custom ID,
invalid ID, conflict replay), get, list with tag filtering, delete, deterministic
prune acceptance, and 409s while a build holds the builder. Config validation
parses idle TTL once, tests negative limits, and startup now propagates
reconciliation failures.
Stack created with GitHub Stacks CLI • Give Feedback 💬
Note
Medium Risk
New API manages persistent disks and background prune/delete behavior; misconfiguration of idle_ttl or quota limits could surprise operators, but patterns match existing volume/build resources.
Overview
Adds a Builder HTTP API for persistent build-cache disks: list/create, get/delete by opaque ID, and POST prune (202, async disk reset). Handlers delegate to
builders.Manager, map domain errors to 400/409/404, and use tag filtering on list like other resources.Wiring:
BuilderManageris injected intoApiService, Wire, andProvideBuilderManager(limits from newbuildersconfig: max count, default/max disk GB, optional destructiveidle_ttl). Startup callsBuilderManager.Startfor reconciliation/reaper. Middleware resolves/builders/{id}viaBuilderResolver; scopesbuilder:read/write/deletegate routes.OpenAPI/Stainless/oapi codegen adds models, routes, and clients; enum
x-enum-varnamesavoids renaming existing image/build status constants. Config validation and handler/integration tests cover quotas, in-use conflicts, and races with the idle reaper.Reviewed by Cursor Bugbot for commit a41df03. Bugbot is set up for automated code reviews on this repo. Configure here.