Skip to content

fix: clean version stamps on release builds (no -dirty markers)#50

Merged
StefanSteiner merged 2 commits into
tableau:mainfrom
StefanSteiner:ssteiner/fix-release-dirty-markers
May 27, 2026
Merged

fix: clean version stamps on release builds (no -dirty markers)#50
StefanSteiner merged 2 commits into
tableau:mainfrom
StefanSteiner:ssteiner/fix-release-dirty-markers

Conversation

@StefanSteiner
Copy link
Copy Markdown
Contributor

Summary

The npm-published hyperdb-mcp@0.2.1 binary stamps its version as 0.2.1.rb57f0c37-dirty-20260527T015408Z — the -dirty-<timestamp> marker should never appear on a release build.

Root cause (verified end-to-end): release-please-config.json patches Cargo.toml and per-crate Cargo.tomls but NOT Cargo.lock. So the v0.2.1 tag points at a tree where Cargo.toml says 0.2.1 and Cargo.lock still pins workspace members at 0.1.3. When CI checks out the tag and runs cargo build --release, cargo silently rewrites the lockfile in-place to reconcile, dirtying the worktree. build.rs's git status --porcelain -uno then trips and stamps the dirty marker.

Fix — two-pronged

1. fix(build): exclude Cargo.lock from dirty-marker detection (hyperdb-mcp/build.rs, hyperdb-bootstrap/build.rs)

Add :(exclude,top)Cargo.lock to both git status --porcelain -uno invocations. A lockfile-only modification no longer trips the dirty marker. Any other modified tracked file (including a deliberately-edited Cargo.lock alongside a real code change) still trips it because git status will list the other file.

2. fix(ci): regenerate Cargo.lock in release-please PR (.github/workflows/release-please.yml)

Add follow-up steps that fire whenever a release PR exists (pr != ''). They check out the release-please branch, run cargo metadata --format-version=1 > /dev/null to reconcile the lockfile, verify the diff only touches workspace-member rows, and push the lockfile commit as github-actions[bot]. Plus a workflow-level concurrency: { group: release-please, cancel-in-progress: false } block to serialize concurrent runs.

The two prongs are deliberate defense-in-depth: the build.rs change ensures the released binary stamps cleanly even if the workflow ever misfires; the workflow change keeps the release-tag tree in sync so cargo doesn't have to rewrite at build time.

Why cargo metadata (not cargo update --workspace or cargo generate-lockfile)

Empirically tested on the live workspace with Cargo.toml@0.2.1 and Cargo.lock@0.1.3:

  • cargo metadata --format-version=1 > /dev/null flips exactly the 7 workspace-member version rows and touches no external dep rows. ✓
  • cargo generate-lockfile does a full re-resolve and bumps unrelated transitive deps (crypto-common 0.1.6 → 0.1.7, matchit 0.8.4 → 0.8.6, etc.) plus changes the lockfile format version. Noisy in a release commit. ✗
  • cargo update --workspace [--offline]'s --workspace flag does NOT prevent external dep re-resolution from the local cache. ✗

A two-layer sentinel verifies the lockfile diff post-sync: Layer 1 catches lockfile format-version flips (^[+-]version = [0-9]+$, unquoted), Layer 2 enumerates workspace members at runtime via cargo metadata --no-deps | jq (no hard-coded list to maintain) and asserts every changed package name is in that set. If either layer detects something unexpected, the workflow aborts and blocks the release.

Acknowledged residual risks

  • Force-push race. When release-please re-runs after a subsequent push to main, it force-pushes the release branch and clobbers our prior chore: sync Cargo.lock commit. The follow-up step re-fires on every re-run via the pr != '' gate, so the FINAL state of the release PR (when it merges) is always lockfile-synced. A maintainer who merges in a tiny window between release-please's force-push and our follow-up landing could still get a drifted tag — the build.rs Cargo.lock pathspec exclusion is the safety net for that case.
  • The already-published 0.2.1 npm packages stay dirty. Not retroactively fixed. The next release (this PR's fix: title triggers v0.2.2) will be the first clean one.

Test plan

  • CI green on the PR (workspace lint/build/test).
  • Inspect the next release-please PR after merge:
    • Diff includes a chore: sync Cargo.lock with bumped workspace versions commit.
    • Cargo.lock shows the workspace member rows bumped to v0.2.2.
    • No external dep version flips.
  • After merging the release PR, inspect the npm-build-publish workflow run for the new tag:
    • cargo build --release does NOT rewrite Cargo.lock (worktree stays clean).
    • Build emits cargo:rustc-env=HYPERDB_GIT_HASH=<hash> with NO -dirty substring.
  • After publish: npx -y hyperdb-mcp@0.2.2 --version returns 0.2.2.r<hash> with no -dirty-... suffix.

Empirical validation (this branch, this machine)

  • With Cargo.toml@0.2.1 and Cargo.lock@0.1.3, cargo metadata --format-version=1 > /dev/null rewrites the lockfile and git status --porcelain -uno reports M Cargo.lock.
  • The pathspec form git status --porcelain -uno -- ':(exclude,top)Cargo.lock' returns empty (clean).
  • A real source modification (echo "" >> hyperdb-mcp/src/main.rs) still trips the dirty marker — verified by inspecting the binary's --version output: 0.2.1.rac35ed73-dirty-20260527T055538Z.
  • cargo metadata exclusively flipped the 7 workspace member rows: hyperdb-api, hyperdb-api-core, hyperdb-api-node, hyperdb-api-salesforce, hyperdb-bootstrap, hyperdb-mcp, sea-query-hyperdb. No external deps changed.
  • The full sentinel logic ran end-to-end against the real lockfile drift in bash -c (matching the GitHub Actions runner default shell) and produced the expected workspace-only summary.
  • Both Phase 5 reviewers (parallel feature-dev:code-reviewer + aisuite:code-review) signed off; the deeper reviewer's MAJOR findings (hard-coded workspace list, awk attribution by proximity, missing protoc comment) are addressed.

release-please bumps `[workspace.package].version` in Cargo.toml but
does NOT update Cargo.lock — the lockfile isn't in the action's
extra-files list. When CI checks out a release tag and runs
`cargo build --release`, cargo silently reconciles the lockfile
in-place to match the bumped Cargo.toml. That dirties the worktree;
build.rs's `git status --porcelain -uno` then trips, and the released
binary gets stamped `<hash>-dirty-<timestamp>` even though the source
matches the tag perfectly.

Fix: add `:(exclude,top)Cargo.lock` to both build.rs git-status
invocations so a lockfile-only modification doesn't trip the dirty
marker. Any other modified tracked file (including a deliberately-
edited Cargo.lock alongside a code change) still trips it because
git status will list the other file.

The companion fix to release-please.yml that prevents the lockfile
from drifting in the first place lands in a follow-up commit; this
build.rs change is the safety net that ensures clean release binaries
even if the workflow change has rough edges.

Empirically validated: with Cargo.toml at 0.2.1 and Cargo.lock
pinning members at 0.1.3, `cargo metadata --format-version=1` rewrites
the lockfile and `git status --porcelain -uno` reports `M Cargo.lock`.
With the pathspec, `git status --porcelain -uno -- ':(exclude,top)Cargo.lock'`
returns empty. A real source-file modification still trips the marker.
Root cause of the `0.2.1.r<hash>-dirty-<timestamp>` markers on the
npm-published binaries: release-please bumps `[workspace.package].version`
in Cargo.toml on every release, but its `extra-files` config doesn't
include Cargo.lock. So the release tag points at a tree where Cargo.toml
is at the new version (e.g. 0.2.1) but Cargo.lock still pins workspace
members at the previous version (e.g. 0.1.3). When CI checks out that
tag and runs `cargo build --release`, cargo silently rewrites Cargo.lock
in-place to match — the worktree goes dirty, and `build.rs`'s
`git status --porcelain -uno` stamps `-dirty-<timestamp>` onto the
released binary.

Fix: add follow-up steps to release-please.yml that fire whenever a
release PR exists (created or updated) and reconcile Cargo.lock onto
the same release-please branch:

1. Checkout the release-please branch (`steps.release.outputs.pr` JSON
   carries `headBranchName`).
2. Setup Rust stable.
3. Run `cargo metadata --format-version=1 > /dev/null` — empirically
   verified to flip exactly the 7 workspace member version rows on
   this workspace and touch no external dep rows.
4. Sentinel step: parse the lockfile diff and fail loudly if any
   non-workspace package version changed. Defense against future
   cargo behavioral drift.
5. Commit Cargo.lock as `github-actions[bot]` and push.

Why `cargo metadata` and not `cargo update --workspace [--offline]`:
the `--workspace` flag does NOT prevent external dep re-resolution
from the local cache, and `--offline` makes that brittle.
`cargo metadata` is a more focused primitive — it just reconciles
the lockfile against current Cargo.tomls.

Also adds a workflow-level `concurrency: { group: release-please,
cancel-in-progress: false }` to serialize runs and avoid racing the
lockfile push against a parallel release-please invocation.

Pairs with the build.rs Cargo.lock exclusion in the prior commit:
that fix is the safety net for any case where this workflow
misbehaves; this fix is the real source-of-truth solution that
prevents the lockfile from drifting in the first place.
@StefanSteiner StefanSteiner merged commit 5962a4e into tableau:main May 27, 2026
11 checks passed
StefanSteiner added a commit that referenced this pull request May 27, 2026
The follow-up steps added in PR #50 reference
`fromJson(steps.release.outputs.pr).headBranchName` directly in `ref:`
(checkout) and `env:` (push) blocks. GitHub Actions evaluates every
`${{ }}` expression in the workflow at job-load time, BEFORE per-step
`if:` gates short-circuit. When release-please runs on a push that
doesn't produce a release PR (e.g. a PR merge whose conventional-
commit was already covered by an earlier release), `pr` is the empty
string, `fromJson('')` errors out, and the entire job fails with:

  ##[error]The template is not valid. .../release-please.yml
  (Line: 203, Col: 19): Error reading JToken from JsonReader.
  Path '', line 0, position 0.

This blocks release-please from ever opening the next release PR.
Observed in run 26494686197 on the merge of PR #50 itself.

Fix: extract the branch name in a new "Resolve release-please branch
name" step (id=`branch`) that does the parse via `jq` inside a `run:`
block — that block only runs when the step's `if:` gate passes, so
the parse is never attempted on empty input. The downstream
`actions/checkout` `ref:` and the final `git push` `env: BRANCH:`
both reference `steps.branch.outputs.name`, which evaluates safely
to the empty string when the step was skipped.

Add a doc comment explaining the GHA expression-evaluation timing
trap so the next person reading the workflow doesn't accidentally
re-introduce the same pattern.
This was referenced May 27, 2026
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.

1 participant