[131-3] feat: CI pipeline — build and push opencode Docker image on tag#3
[131-3] feat: CI pipeline — build and push opencode Docker image on tag#3andreiships-bot merged 1 commit intodevfrom
Conversation
Adds .github/workflows/build-push.yml that triggers on v* tags to: - Build the multi-stage Dockerfile (Bun builder → Alpine runtime) - Push to registry.fly.io/pistachiorama-opencode with version + latest tags - Use GitHub Actions build cache for faster subsequent builds Depends on Dockerfile from PR #2 (131-phase-2 branch).
|
Hey! Your PR title Please update it to start with one of:
Where See CONTRIBUTING.md for details. |
Claude Single-Pass ReviewSummaryThis PR adds a straightforward GitHub Actions workflow to build and push a Docker image to Fly.io's registry on version tag pushes. The workflow is well-structured with minimal permissions, but has two issues worth addressing: Findings[FINDING-1] issue: P1 | .github/workflows/build-push.yml:35 | The [FINDING-2] nit: P2 | .github/workflows/build-push.yml:31 | [FINDING-3] nit: P2 | .github/workflows/build-push.yml:31 | No Code Quality
Recommendation[ ] Approve | [x] Approve with changes | [ ] Request changes |
andreiships-bot
left a comment
andreiships-bot
left a comment
There was a problem hiding this comment.
Claude Review
See inline comments for details.
| uses: docker/build-push-action@v6 | ||
| with: | ||
| context: . | ||
| push: true |
There was a problem hiding this comment.
[FINDING-1] issue: P1 | The :latest tag is pushed unconditionally on every v* tag push, including pre-releases like v1.0.0-alpha or v1.0.0-rc1. Verified via diff: registry.fly.io/pistachiorama-opencode:latest. Fix: Either omit the :latest tag entirely and let Fly.io deployments reference explicit version tags, or add a condition to only push :latest for non-pre-release tags (e.g., filter out tags containing - in the tag name using a separate step with if: !contains(steps.version.outputs.tag, '-')).
| id: version | ||
| run: echo "tag=${GITHUB_REF#refs/tags/}" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: Build and push |
There was a problem hiding this comment.
[FINDING-2] nit: P2 | docker/build-push-action@v6 defaults to provenance: true, which generates OCI attestation manifests. On Fly.io's registry, this can cause fly deploy to resolve the wrong digest (the attestation index rather than the image manifest). Fix: Add provenance: false to the build-push step unless attestation is explicitly needed.
| id: version | ||
| run: echo "tag=${GITHUB_REF#refs/tags/}" >> "$GITHUB_OUTPUT" | ||
|
|
||
| - name: Build and push |
There was a problem hiding this comment.
[FINDING-3] nit: P2 | No platforms: specified — the image will only be built for the runner's architecture (linux/amd64 on Blacksmith). If Fly.io machines use ARM, this will silently pull the wrong arch. Fix: Add platforms: linux/amd64 (or linux/amd64,linux/arm64 for multi-arch) to make the intent explicit.
Fixes issues found during retroactive review of PRs #1, #2, #3. tool-call.ts: - Agent.defaultAgent() returns a name string, not Agent.Info — resolve with Agent.get() before passing to ToolRegistry.tools() - Fix agent: agent.id (undefined on string) → agent: agentName - Use agent's configured model instead of hardcoded opencode/default Dockerfile: - Quote $(find ...) and add existence check to prevent cryptic failures when binary is missing - Remove BUN_RUNTIME_TRANSPILER_CACHE_PATH env var — irrelevant for a compiled native binary that does not use Bun's transpiler fly.toml: - min_machines_running: 0 → 1 to avoid cold starts on interactive sessions scripts/ci/test-opencode-integration.sh: - Replace hardcoded /tmp/ paths with mktemp tmpdir + EXIT trap to avoid collisions in parallel CI runs .github/workflows/build-push.yml: - Gate :latest push on stable tags only (no pre-release suffix like -rc1) - Add provenance: false to prevent OCI attestation manifests from breaking fly deploy digest resolution - Explicitly set platforms: linux/amd64 to avoid silent arch mismatches
…, CI) (#4) * fix(131): address P1 review findings from retroactive review Fixes issues found during retroactive review of PRs #1, #2, #3. tool-call.ts: - Agent.defaultAgent() returns a name string, not Agent.Info — resolve with Agent.get() before passing to ToolRegistry.tools() - Fix agent: agent.id (undefined on string) → agent: agentName - Use agent's configured model instead of hardcoded opencode/default Dockerfile: - Quote $(find ...) and add existence check to prevent cryptic failures when binary is missing - Remove BUN_RUNTIME_TRANSPILER_CACHE_PATH env var — irrelevant for a compiled native binary that does not use Bun's transpiler fly.toml: - min_machines_running: 0 → 1 to avoid cold starts on interactive sessions scripts/ci/test-opencode-integration.sh: - Replace hardcoded /tmp/ paths with mktemp tmpdir + EXIT trap to avoid collisions in parallel CI runs .github/workflows/build-push.yml: - Gate :latest push on stable tags only (no pre-release suffix like -rc1) - Add provenance: false to prevent OCI attestation manifests from breaking fly deploy digest resolution - Explicitly set platforms: linux/amd64 to avoid silent arch mismatches * ci: replace blacksmith runners with ubicloud-standard-2 All blacksmith-* runner labels replaced: - blacksmith-4vcpu-ubuntu-2404 → ubicloud-standard-2 - blacksmith-8vcpu-ubuntu-2404-arm → ubicloud-standard-8-arm - blacksmith-4vcpu-ubuntu-2404-arm → ubicloud-standard-2-arm - blacksmith-4vcpu-windows-2025 → windows-latest (no Ubicloud Windows runner) --------- Co-authored-by: Andrei Cojocaru <andrei@pistachiorama.ai> Co-authored-by: andreiships-bot <andreiships-bot@users.noreply.github.com>
2 participants
Adds build-push.yml workflow triggered on v* tags. Builds multi-stage Dockerfile and pushes to registry.fly.io/pistachiorama-opencode. Depends on Dockerfile from PR #2.