Repository navigation
feat: support static site builds and deployments - #282
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for static builds and deployments to the Hyphen CLI, allowing users to deploy static sites via the --type static and --sites flags. The changes include logic for preparing, gzipping, and uploading static files directly to a SiteRegistry, alongside new local development environment variables (HYPHEN_LOCAL_APIX and HYPHEN_LOCAL_HORIZON). Feedback on the implementation highlights two critical issues: a classic Go loop variable pointer capture bug in internal/build/static.go that could affect correctness in Go versions prior to 1.22, and the use of the omitzero JSON tag in internal/models/build.go, which was introduced in Go 1.24 and may break backward compatibility with older Go toolchains.
There was a problem hiding this comment.
Code Review
This pull request introduces support for static site builds and deployments in the Hyphen CLI, adding a --type static option to the build and deploy commands, handling gzip packaging, and uploading assets directly to a SiteRegistry. Feedback on these changes highlights several important issues: a potential serialization failure due to the unsupported omitzero JSON tag, a runtime panic risk when slicing short commit SHAs, a loop variable pointer anti-pattern, overly restrictive URL validation that rejects query parameters, the need to resolve symlinks for site directories using os.Stat, and a test assertion mismatch for the local Socket.IO base URL.
Summary
Enable hx to build and deploy static websites through Hyphen Cloud while preserving the default Docker workflow.
--type static|dockerto build/deploy, defaulting to Docker. Static builds require a final positional directory path.--sitesand--appsdeployment selection, including build selectors, previews, and no-build flows.sitesand the Hyphen Cloud organization integration for static builds; exclude Hyphen Cloud from Docker integration choices.Verification
go build ./...go vet ./...git diff --check origin/main...HEADgo test -count=1 ./...(all packages, with local mock-server access).The Socket.IO test/documentation mismatch is resolved in
a221e2d: local Socket.IO uses port 4100 and APIX uses port 4000.No live deployments, uploads, migrations, or database changes were performed.
Review disposition — Hemingway pass
Mode: reviewing (no new code edits).
Behavior contract: Preserve static uploads, Docker artifact JSON, supported Go 1.24.1 semantics, and intentional local endpoints.
Result: 0 cuts, 0 simplifications, 6 keeps — net 0 code lines in this review pass.
Cut
None.
Simplify
None.
Keep
internal/deployment/sites.go:51— fixed APIX origin and escaped path IDs; the cited URL input does not allow an attacker-selected host.internal/build/static.go:90— per-iteration registry value is correct with the module's Go version; no older-toolchain compatibility layer needed. Go loop semantics.internal/models/build.go:8—omitzeropreserves Docker empty ports versus omitted static ports; changing toomitemptybreaks the existing serialization test. Go JSON support.internal/build/static.go:41— retain the documented real-directory input requirement; changing only Lstat does not make the directory walker follow a symlink root.internal/build/static.go:300— retain current bearer-session upload URL validation; presigned cloud-storage support is a different protocol, not a required compatibility path.internal/build/service.go:313— full Git object ID or the existing long fallback precedes truncation; no short-success input was identified.Smallest Change
No additional runtime changes warranted. The valid Socket.IO test correction is already present in
a221e2d.Verification
All packages pass
go test -count=1 ./...;go build ./...,go vet ./..., and diff whitespace checks pass. Nine review comments received individual replies. No proposed behavior narrowings remain.