Skip to content

fix(deploy): populate build provenance on dev (#958)#968

Merged
vybe merged 1 commit into
devfrom
fix/958-build-info-dev-deploy
May 28, 2026
Merged

fix(deploy): populate build provenance on dev (#958)#968
vybe merged 1 commit into
devfrom
fix/958-build-info-dev-deploy

Conversation

@dolho

@dolho dolho commented May 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes two independent bugs that left GET /api/version (and the NavBar Build Info dialog) showing unknown for every git/build field on the dev deployment.

  1. docker-compose.prod.yml backend service was missing its build.args block. Base docker-compose.yml:11-15 declares GIT_COMMIT / GIT_BRANCH / GIT_COMMIT_SUBJECT / GIT_COMMIT_TIMESTAMP / BUILD_DATE build args, but the dev workflow builds with -f docker-compose.prod.yml only — not chained with base — so the Dockerfile ARGs never received any values. Added the equivalent block to prod compose.

  2. .github/workflows/deploy-dev.yml didn't compute the git provenance env vars before docker compose build. scripts/deploy/start.sh:175-181 does this for local runs; the workflow was missing the equivalent. Added a block that reads the values off the dev VM's checked-out tree and passes them inline on the sudo docker compose build invocation.

The sudo VAR=val … inline form is intentional rather than sudo -E because Ubuntu's default sudoers env_reset does not preserve GIT_* through -E.

Local runs were always fine — start.sh did both halves of the work (export + base compose carries the args). Only the CI deploy path was broken.

Related to #958

Test plan

  • Merge → dev deploy runs → SSH check the trinity-backend container env after the build:
    docker exec trinity-backend env | grep -E 'GIT_|BUILD_DATE'
    
    …shows the dev commit/branch/timestamp, not unknown.
  • Hit /api/version on dev (authenticated) — all fields populated.
  • Open NavBar → Build Info — no unknown rows.

Out of scope (follow-up in #958)

The issue also asks for a dialog UX fallback when all fields are still unknown (true e.g. for ad-hoc docker build without args) and an explanatory blurb under the dialog title. Frontend-only — not blocked on this deploy fix and would expand the PR's blast radius. Happy to send a second PR once this lands.

🤖 Generated with Claude Code

Two independent bugs caused `GET /api/version` (and the NavBar Build
Info dialog) to return `unknown` for every git/build field on the dev
deployment:

1. `docker-compose.prod.yml` backend service had no `build.args` block,
   so the Dockerfile's `GIT_COMMIT` / `GIT_BRANCH` / `BUILD_DATE` etc.
   ARGs never received values during compose builds. Base
   `docker-compose.yml` carries the block, but the dev workflow builds
   with `-f docker-compose.prod.yml` only, not chained with base.

2. `.github/workflows/deploy-dev.yml` didn't compute and pass the git
   provenance env vars before invoking `docker compose build`.
   `scripts/deploy/start.sh` does this for local runs; the workflow
   was missing the equivalent block.

Fix:
- Add `args:` block to prod compose backend service mirroring base.
- In the workflow, compute git vars from the dev VM's checkout and
  pass them inline on the `sudo docker compose build` invocation.
  Inline `sudo VAR=val …` is used rather than `sudo -E` because the
  default Ubuntu sudoers does not preserve `GIT_*` through `env_reset`.

After deploy, dev's Build Info dialog will render the real commit and
build timestamp instead of `unknown`.

Related to #958

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@dolho
dolho requested a review from AndriiPasternak31 as a code owner May 28, 2026 12:00
@dolho

dolho commented May 28, 2026

Copy link
Copy Markdown
Contributor Author

/review report — fine to merge

Critical findings: 0
Informational findings: 2 (both acknowledged below, no action required)

Pass 1 (critical / blockers)

  • SQL & data safety — no DB code touched
  • Race / concurrency — workflow is sequential, no new concurrency
  • Auth boundary — no endpoints, no auth code
  • Credential exposure — all five vars are public git metadata; workflow already echoes git log -1 --oneline at line 63, so no new disclosure

Pass 2 (informational)

[I1] sudo VAR=val … has a hardening edge case. Works against Ubuntu defaults: sudo's inline env-var form sets vars in the command's environment regardless of env_reset. On hardened sudoers configs where setenv is denied or these specific vars sit in env_delete, the build args would silently fall back to compose's ${VAR:-unknown}. Acceptable risk — dev VM uses stock Ubuntu sudoers; PR test plan verifies the first post-merge deploy.

[I2] Workflow passes env vars to all four builds; only backend.build.args declares them. frontend, mcp-server, scheduler ignore the extras (no matching ARG declarations). No regression. Not worth expanding scope to add identical args blocks to the other three when only the backend serves /api/version.

Scope check

PARTIAL DRIFT — disclosed and intentional. Issue #958 title says "local development"; this PR fixes the CI dev deployment. The local path was already fixed in #931 (start.sh exports + base docker-compose.yml args + Dockerfile). PR body explicitly defers ACs #4 and #5 (dialog all-unknown fallback + explanatory blurb) as a separate frontend-only PR.

Other categories — clean

Conditional side effects · magic numbers · dead code · error handling (set -e covers git failures, compose covers empty values) · test gaps (CI-only change, manual verification in test plan is right) · frontend (none) · performance (one extra git log is negligible) · enum completeness (no new enums; "unknown" sentinel already handled at main.py:1081)

🤖 Generated by /review (Claude Opus 4.7)

@dolho
dolho requested a review from vybe May 28, 2026 12:16

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — clean two-line infra fix with right scoping. Approved per /validate-pr.

@vybe
vybe merged commit f4c04f9 into dev May 28, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants