Skip to content

feat(release): concurrency lock to prevent racing ferrflow release runs - #517

Merged
BryanFRD merged 1 commit into
feat/quick-wins-perf-securityfrom
feat/release-concurrency-lock
Jun 3, 2026
Merged

BryanFRD merged 1 commit into
feat/quick-wins-perf-securityfrom
feat/release-concurrency-lock

Conversation

@BryanFRD

Copy link
Copy Markdown
Contributor

Stacked on #516. First half of #514 (the lock); checkpoint/resume stays open as a heavier follow-up.

Problem

Two concurrent `ferrflow release` invocations on the same repo (manually-triggered racing the cron-driven `auto-release` workflow, or two CI runners on the same commit) competed on git refs. Observed symptoms: non-fast-forward rejects, half-pushed tag sets, draft releases created twice.

Fix

New `src/monorepo/run/lock.rs`: RAII lock guard backed by `.git/ferrflow.lock` created via O_CREAT|O_EXCL — atomic, released on drop including panic unwind. Acquired at the top of `run_release_logic` for non-dry-run only. Read-only commands (`check`, `status`, `version`, `tag`) are unaffected.

Stale lock recovery

A lockfile older than 30 min is treated as orphaned (process crashed without releasing) and taken over with a warning. Beyond that, the `acquire_force` path exists for an eventual `--force-unlock` CLI flag (kept private for now).

Test plan

  • 6 unit tests: clean acquire, drop removes, busy second-acquire fails, force-unlock takes over, missing .git errors out, lockfile starts with PID
  • 521 lib + 635 bin tests pass overall
  • `cargo clippy --features cli -- -D warnings` clean
  • Manual: trigger two `ferrflow release` in parallel against the same repo — second exits with "another release is already running" error code GIT_LOCKED (2011)

First half of #514. Closes the lock concern; the checkpoint/resume
piece stays open (heavier design, separate PR).

## Problem

Two concurrent ferrflow release invocations on the same repo (typical
scenario: manually-triggered release racing the cron-driven
auto-release workflow, or two CI runners on the same commit) competed
on git refs. Observed symptoms: non-fast-forward rejects, half-pushed
tag sets, draft releases created twice.

## Fix

New src/monorepo/run/lock.rs: RAII lock guard backed by
.git/ferrflow.lock created via O_CREAT|O_EXCL. Atomic. Released on
drop, including panic unwind.

Acquired at the top of run_release_logic for non-dry-run only.
Read-only commands (check, status, version, tag) are not affected.

## Stale lock recovery

A lockfile older than STALE_LOCK_TTL (30 min, longer than any realistic
release) is treated as orphaned (process crashed without releasing)
and taken over with a warning. Beyond TTL the user can also
force-unlock manually by deleting the file; future PR may expose this
via --force-unlock CLI flag.

## Tests

- 6 unit tests in src/monorepo/run/lock.rs::tests:
  - acquire on clean repo succeeds
  - drop removes lockfile
  - second acquire fails while first held
  - force unlock takes over active lock
  - missing .git dir errors out cleanly
  - lockfile content starts with PID
- 521 lib + 635 bin tests pass overall
- cargo clippy -D warnings clean

## Out of scope

The checkpoint/resume mechanism from #514 (write release-state.json at
each step, resume if interrupted) is deferred — needs separate design
review around atomic write + invalidation rules. Issue stays open.
Copilot AI review requested due to automatic review settings May 24, 2026 12:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@BryanFRD
BryanFRD merged commit 386e5d0 into feat/quick-wins-perf-security Jun 3, 2026
2 of 3 checks passed
@BryanFRD
BryanFRD deleted the feat/release-concurrency-lock branch June 3, 2026 20:02
BryanFRD added a commit that referenced this pull request Jun 5, 2026
…ns (#517)

First half of #514. Closes the lock concern; the checkpoint/resume
piece stays open (heavier design, separate PR).

## Problem

Two concurrent ferrflow release invocations on the same repo (typical
scenario: manually-triggered release racing the cron-driven
auto-release workflow, or two CI runners on the same commit) competed
on git refs. Observed symptoms: non-fast-forward rejects, half-pushed
tag sets, draft releases created twice.

## Fix

New src/monorepo/run/lock.rs: RAII lock guard backed by
.git/ferrflow.lock created via O_CREAT|O_EXCL. Atomic. Released on
drop, including panic unwind.

Acquired at the top of run_release_logic for non-dry-run only.
Read-only commands (check, status, version, tag) are not affected.

## Stale lock recovery

A lockfile older than STALE_LOCK_TTL (30 min, longer than any realistic
release) is treated as orphaned (process crashed without releasing)
and taken over with a warning. Beyond TTL the user can also
force-unlock manually by deleting the file; future PR may expose this
via --force-unlock CLI flag.

## Tests

- 6 unit tests in src/monorepo/run/lock.rs::tests:
  - acquire on clean repo succeeds
  - drop removes lockfile
  - second acquire fails while first held
  - force unlock takes over active lock
  - missing .git dir errors out cleanly
  - lockfile content starts with PID
- 521 lib + 635 bin tests pass overall
- cargo clippy -D warnings clean

## Out of scope

The checkpoint/resume mechanism from #514 (write release-state.json at
each step, resume if interrupted) is deferred — needs separate design
review around atomic write + invalidation rules. Issue stays open.
BryanFRD added a commit that referenced this pull request Jun 5, 2026
…tion, markdown escape, ureq Agent reuse

* perf(alloc): switch to mimalloc for the cli binary

Default allocator (glibc malloc on Linux, HeapAlloc on Windows) is
suboptimal for alloc-heavy short-lived CLIs. mimalloc consistently
shaves 5-15% wall time on workloads that match ours (TagIndex::build,
revwalk + commit message decode, regex captures during conventional-
commit parsing).

Gated behind the cli feature so the wasm build doesn't pull it in.
Adds ~200 KB to the release binary; net positive on perf benches.

First item from #507.

* security(deps): ban git2/libgit2-sys/openssl in cargo-deny

cargo-deny already runs in CI (security job), but the bans section was
empty. Add explicit denials for:
- git2 / libgit2-sys: just migrated off in #487, prevent regression
- openssl-sys / openssl-src: vendored via libgit2's old chain, the gix
  migration moved us to rustls. Reintroducing would double binary size
  and inherit OpenSSL's CVE cadence

Closes #511.

* security: markdown-escape preview PR comments + validate ref names

Closes #512.

## Preview PR comment markdown escaping

format_preview_comment was interpolating pkg.name / pkg.current_version
/ pkg.next_version / pkg.bump_type directly into a markdown table —
all of which come from user-controlled .ferrflow + version files on
the PRs HEAD. A package name like foo|<script> broke the table on
github.com (rendered fine but visible) and triggered actual HTML on
custom forge installs (Gitea, Forgejo with permissive markdown).

Added escape_md_cell: encodes |, newline, <, >, backslash, backtick.
6 unit tests covering pipe-break, HTML injection, link injection,
backtick code, newline-row-break.

## Refname validation

New src/git/validate.rs::ensure_safe_refname_fragment rejects:
- empty
- leading - (flag confusion: --exec=ls etc.)
- NUL byte
- newline / carriage return
- control characters except tab

Called from create_tag, create_or_move_tag, push_branch, push_tags,
verify_remote_branch, reset_branch_to_remote.

The git tag invocations also gained a -- separator before the tag
name so an exotic value cant be re-interpreted as a flag even if the
validator misses something.

## Tests

- 514 lib + 629 bin tests pass (was 514 + 616)
- clippy -D warnings clean
- New tests: 7 in validate.rs, 6 in preview.rs::tests

* perf(forge): share one ureq::Agent across all HTTP calls

The bare ureq::get / ureq::post helpers create a fresh Agent (and TLS
handshake) per call. A 50-pkg release does ~150 HTTPS round-trips
against api.github.com (create_release × N, find_draft_release × N,
publish_release × N, plus comment + auto-merge for PR mode) — each
paying a fresh handshake.

Build one Agent in build_forge() and store it on GitHubForge /
GitLabForge. Subsequent calls reuse it via HTTP keep-alive. Expected
2-8 seconds saved on a 50-pkg release; smaller wins on single-pkg.

Closes #509.

* feat(release): concurrency lock to prevent racing ferrflow release runs (#517)

First half of #514. Closes the lock concern; the checkpoint/resume
piece stays open (heavier design, separate PR).

## Problem

Two concurrent ferrflow release invocations on the same repo (typical
scenario: manually-triggered release racing the cron-driven
auto-release workflow, or two CI runners on the same commit) competed
on git refs. Observed symptoms: non-fast-forward rejects, half-pushed
tag sets, draft releases created twice.

## Fix

New src/monorepo/run/lock.rs: RAII lock guard backed by
.git/ferrflow.lock created via O_CREAT|O_EXCL. Atomic. Released on
drop, including panic unwind.

Acquired at the top of run_release_logic for non-dry-run only.
Read-only commands (check, status, version, tag) are not affected.

## Stale lock recovery

A lockfile older than STALE_LOCK_TTL (30 min, longer than any realistic
release) is treated as orphaned (process crashed without releasing)
and taken over with a warning. Beyond TTL the user can also
force-unlock manually by deleting the file; future PR may expose this
via --force-unlock CLI flag.

## Tests

- 6 unit tests in src/monorepo/run/lock.rs::tests:
  - acquire on clean repo succeeds
  - drop removes lockfile
  - second acquire fails while first held
  - force unlock takes over active lock
  - missing .git dir errors out cleanly
  - lockfile content starts with PID
- 521 lib + 635 bin tests pass overall
- cargo clippy -D warnings clean

## Out of scope

The checkpoint/resume mechanism from #514 (write release-state.json at
each step, resume if interrupted) is deferred — needs separate design
review around atomic write + invalidation rules. Issue stays open.
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