Repository navigation
Resolve environment alternate IDs before static builds - #285
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces environment resolution for static builds, allowing the use of either an environment ID or an alternate ID, and updates the CLI flag description and tests accordingly. A critical feedback item identifies a data race in the newly added TestStaticBuildEnvironmentFailureStopsBeforeUploading test, where the requests slice is accessed concurrently without synchronization, and suggests protecting it with a sync.Mutex.
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation consistently uses canonical IDs, fails before upload, preserves unscoped behavior, and includes comprehensive coverage.
Review effort: Balanced
Findings: None
What changed in this PR
Resolves static-build environment aliases to canonical IDs before upload and registration.
Changes:
- Adds environment resolution and early failure handling.
- Preserves unscoped builds for all environments.
- Expands static build and deploy-flow tests and clarifies CLI help.
| File | Description |
|---|---|
internal/build/static.go |
Adds environment lookup. |
internal/build/service.go |
Resolves static-build environments before upload. |
internal/build/static_test.go |
Tests resolution and failure behavior. |
cmd/deploy/static_flow_test.go |
Updates static deployment integration coverage. |
cmd/build/build.go |
Clarifies --env help text. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to resolve environment IDs or alternate IDs for static builds. It adds a new resolveSiteEnvironment method to the BuildService to fetch and resolve the environment ID from the API before proceeding with the build. Additionally, it updates the --env flag description, adds comprehensive unit tests for various environment resolution scenarios, and includes error handling tests for when environment resolution fails. There are no review comments, and I have no feedback to provide.
This change resolves project environment IDs for all build types, not just static builds. A shared environment lookup helper now handles environment validation before build execution, and the related tests cover both static and docker flows for failure cases such as missing environments, forbidden access, malformed responses, and missing IDs.
Resolve environment alternate IDs before static builds
Summary
hx build --env development --type static ./distnow sends the canonical environment ID required by the existing upload API.--envavailable to all environments.--envhelp text.Corresponding app instructions: https://github.com/Hyphen/hyphen-app/pull/1636
Validation
go test -race ./internal/build ./cmd/build ./cmd/deploypassed after synchronizing the captured-request assertions.go test ./.....hxconfirms the exit-code behavior but produces a file-permission error rather than the expected missing-credentials message. Credentials were not modified.gofmtandgit diff --checkpassed.