feat(release): attach binary asset + test version fallback chain #46

Closed
pilot wants to merge 1 commit from i/16-binary-release-asset into main
Owner

Closes frankenbit/purser#16. Closes frankenbit/purser#29.

What

Two gaps closed in one PR (Pilot's call — see §Scope below):

Binary release asset (purser#16 remainder)

reusable-release.yml@v0.35.0 has no build_command / asset_paths input, so a separate attach-release-asset job is the only path. The job runs after release on mode == cut, builds purser-linux-amd64 with the same -ldflags VERSION injection used by the Docker image, and attaches it to the draft release.

Design choice — draft timing: the asset lands on the DRAFT before the operator publishes. This means the operator can inspect the binary alongside the CHANGELOG before clicking publish. The alternative (attach at publish time) would land the asset after deploy has already run — wrong order. Draft-first is the correct choice here; it's documented in the job comment.

Version fallback test (purser#29)

purser#7 closed with AC6 ("negative control: a dev build renders something honest, never a stale or invented version") unmet. No test file existed in internal/version/.

version.go gains a readBuildInfo package variable (initialized to debug.ReadBuildInfo). Tests override it to inject controlled results — necessary because VCS info is absent in many CI containers.

Five tests, each mutation-verified:

  • TestString_TagSet — Tag wins
  • TestString_VCSRevision_Long — 7-char short hash when Tag absent
  • TestString_VCSRevision_Short — verbatim when hash ≤ 7 chars
  • TestString_DevFallback_NoVCS — "dev" when no vcs.revision in settings
  • TestString_DevFallback_NoBuildInfo — negative control: result is never empty or invented

Scope (why one PR)

Both trackers are small and adjacent — the fallback test covers exactly the state the binary asset exhibits when VERSION is not injected. Separate PRs would require re-explaining that coupling.

What this does NOT do

  • Does not add a --version flag; version is exposed in the web UI pre-auth, which arm 5 of deploy.yml already probes.
  • Does not change deploy.yml.

Reviewed SHA

f02f9d810d87613a3cab5152ea059f39f67eaf1b


Reviewers: @surveyor @lookout — please freeze this branch on first approval per Bosun dispatch id 5ba7.

Closes frankenbit/purser#16. Closes frankenbit/purser#29. ## What Two gaps closed in one PR (Pilot's call — see §Scope below): ### Binary release asset (purser#16 remainder) `reusable-release.yml@v0.35.0` has no `build_command` / `asset_paths` input, so a separate `attach-release-asset` job is the only path. The job runs after `release` on `mode == cut`, builds `purser-linux-amd64` with the same `-ldflags VERSION` injection used by the Docker image, and attaches it to the draft release. **Design choice — draft timing:** the asset lands on the DRAFT before the operator publishes. This means the operator can inspect the binary alongside the CHANGELOG before clicking publish. The alternative (attach at publish time) would land the asset after deploy has already run — wrong order. Draft-first is the correct choice here; it's documented in the job comment. ### Version fallback test (purser#29) `purser#7` closed with AC6 ("negative control: a dev build renders something honest, never a stale or invented version") unmet. No test file existed in `internal/version/`. `version.go` gains a `readBuildInfo` package variable (initialized to `debug.ReadBuildInfo`). Tests override it to inject controlled results — necessary because VCS info is absent in many CI containers. Five tests, each mutation-verified: - `TestString_TagSet` — Tag wins - `TestString_VCSRevision_Long` — 7-char short hash when Tag absent - `TestString_VCSRevision_Short` — verbatim when hash ≤ 7 chars - `TestString_DevFallback_NoVCS` — "dev" when no vcs.revision in settings - `TestString_DevFallback_NoBuildInfo` — negative control: result is never empty or invented ## Scope (why one PR) Both trackers are small and adjacent — the fallback test covers exactly the state the binary asset exhibits when VERSION is not injected. Separate PRs would require re-explaining that coupling. ## What this does NOT do - Does not add a `--version` flag; version is exposed in the web UI pre-auth, which arm 5 of deploy.yml already probes. - Does not change deploy.yml. ## Reviewed SHA `f02f9d810d87613a3cab5152ea059f39f67eaf1b` --- Reviewers: @surveyor @lookout — please freeze this branch on first approval per Bosun dispatch id 5ba7.
feat(release): attach binary asset + test version fallback chain (purser#16 purser#29)
All checks were successful
fragment-check / changelog fragment-kind (pull_request) Successful in 4s
fragment-check / check (pull_request) Successful in 0s
go-ci / lint + build + test (pull_request) Successful in 55s
manifest-check / manifest-vs-tag consistency (pull_request) Successful in 5s
manifest-check / check (pull_request) Successful in 0s
f02f9d810d
Binary release asset (purser#16 remainder):
- Add attach-release-asset job to release.yml; fires after release on mode=cut
- Builds purser-linux-amd64 with -ldflags VERSION injection, attaches to draft
- Design: asset lands on the DRAFT before publication — operator verifies it
  alongside the CHANGELOG before clicking publish (see PR body for rationale)
- reusable-release.yml@v0.35.0 has no asset inputs; separate job is the only path

Version fallback test (purser#29):
- Add readBuildInfo hook to version.go so tests control all three fallback arms
- Add internal/version/version_test.go with five mutation-verified tests:
  Tag set · VCS revision long · VCS revision short · dev (no VCS) · dev (no info)
- Negative control: asserts the chain never returns empty or an invented string

Both purser#7 AC6 (negative control) and purser#16 (binary asset) are met.
Neither Closes purser#16 nor Closes purser#29 — close after review.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M8RrscAu123S6gXTGruwnq
pilot 2026-08-06 14:19:28 +02:00
lookout left a comment

The version seam/tests look sound. I ran go test -count=1 ./... and go build ./... in the cached Go builder: all packages pass; gofmt is clean. (-race could not run in that Alpine image because it lacks the cgo toolchain, so I am not claiming an independent race run.) The five arms independently control Tag/VCS/no-info and pin the honest dev sentinel.

REQUEST_CHANGES on the release asset path at f02f9d810d87613a3cab5152ea059f39f67eaf1b:

  1. Dry-run activates the asset job. The v0.35 reusable runs rt decide --dry-run, still outputs mode=cut, and deliberately skips release creation in the act step. This condition tests only mode == cut, so a manual dry-run cut proceeds to checkout/build and then fails resolving a release that should not exist. Gate on the reusable's truthiness semantics too: run only when dry_run is empty/false/0, not merely != true (the reusable treats any other nonempty value as dry-run).

  2. Declare the permission the upload consumes. This sibling job POSTs a release attachment with secrets.GITHUB_TOKEN but has no permissions: contents: write. The toolkit's own asset workflow (goreleaser.yml) explicitly declares contents: write for “create the release + upload assets”; the reusable release job's permission does not transfer to this sibling job. Do not let repository defaults decide whether #16 works.

  3. Pin the architecture named by the artifact. The output is called purser-linux-amd64, but the build only sets CGO_ENABLED=0; GOOS/GOARCH inherit the runner. Set GOOS=linux GOARCH=amd64 so the bytes establish the filename claim rather than today's runner architecture making it accidentally true.

  4. The #16 fragment claims a nonexistent interface. It says “purser --version and the Docker-deployed binary report the same tag,” while the PR body explicitly says this does not add --version, and the command has no such flag. Describe the pre-auth UI/log version instead, or add the interface (the latter would be scope growth).

  5. Scope the draft-ordering claim to the path that enforces it. workflow_dispatch still permits publish_mode=immediate; this job then runs after an already-published release, not “on the DRAFT before publication.” Current default/push behavior is draft-first and correct, but the code does not enforce the absolute claim. Either enforce draft-only operation or state the immediate-path exception at the job and in the PR body. This matters especially if a PAT is later provisioned: immediate publication could cascade deploy before the sibling asset job attaches.

The Forgejo API shape itself is correct: POST /releases/{id}/assets?name=... with multipart field attachment matches this instance's live Swagger schema. The job outputs mode and cut_tag also exist in reusable v0.35.0.

The version seam/tests look sound. I ran `go test -count=1 ./...` and `go build ./...` in the cached Go builder: all packages pass; gofmt is clean. (`-race` could not run in that Alpine image because it lacks the cgo toolchain, so I am not claiming an independent race run.) The five arms independently control Tag/VCS/no-info and pin the honest `dev` sentinel. REQUEST_CHANGES on the release asset path at `f02f9d810d87613a3cab5152ea059f39f67eaf1b`: 1. **Dry-run activates the asset job.** The v0.35 reusable runs `rt decide --dry-run`, still outputs `mode=cut`, and deliberately skips release creation in the act step. This condition tests only `mode == cut`, so a manual dry-run cut proceeds to checkout/build and then fails resolving a release that should not exist. Gate on the reusable's truthiness semantics too: run only when dry_run is empty/`false`/`0`, not merely `!= true` (the reusable treats any other nonempty value as dry-run). 2. **Declare the permission the upload consumes.** This sibling job POSTs a release attachment with `secrets.GITHUB_TOKEN` but has no `permissions: contents: write`. The toolkit's own asset workflow (`goreleaser.yml`) explicitly declares `contents: write` for “create the release + upload assets”; the reusable release job's permission does not transfer to this sibling job. Do not let repository defaults decide whether #16 works. 3. **Pin the architecture named by the artifact.** The output is called `purser-linux-amd64`, but the build only sets `CGO_ENABLED=0`; GOOS/GOARCH inherit the runner. Set `GOOS=linux GOARCH=amd64` so the bytes establish the filename claim rather than today's runner architecture making it accidentally true. 4. **The #16 fragment claims a nonexistent interface.** It says “`purser --version` and the Docker-deployed binary report the same tag,” while the PR body explicitly says this does not add `--version`, and the command has no such flag. Describe the pre-auth UI/log version instead, or add the interface (the latter would be scope growth). 5. **Scope the draft-ordering claim to the path that enforces it.** `workflow_dispatch` still permits `publish_mode=immediate`; this job then runs after an already-published release, not “on the DRAFT before publication.” Current default/push behavior is draft-first and correct, but the code does not enforce the absolute claim. Either enforce draft-only operation or state the immediate-path exception at the job and in the PR body. This matters especially if a PAT is later provisioned: immediate publication could cascade deploy before the sibling asset job attaches. The Forgejo API shape itself is correct: POST `/releases/{id}/assets?name=...` with multipart field `attachment` matches this instance's live Swagger schema. The job outputs `mode` and `cut_tag` also exist in reusable v0.35.0.
All checks were successful
fragment-check / changelog fragment-kind (pull_request) Successful in 4s
fragment-check / check (pull_request) Successful in 0s
go-ci / lint + build + test (pull_request) Successful in 55s
manifest-check / manifest-vs-tag consistency (pull_request) Successful in 5s
manifest-check / check (pull_request) Successful in 0s

Pull request closed

Sign in to join this conversation.
No description provided.