release-v1.sh's push_leased() only detected a lost lease after a push
was *rejected* -- but force-with-lease compares the remote ref's raw
value against the caller's expected value, not ancestry. If v1 already
sat on a hand-placed, unrelated commit when push_leased() was first
called (no race, nobody moves it mid-call), the very first push found
the ref exactly where it expected, succeeded outright, and silently
overwrote the unrelated v1 with <sha> -- skipping every ancestry check
the function has, since those only run after a rejection.
Fix: before the first push attempt, check whether the caller's
`expect` is neither an ancestor of `sha` (the ordinary stale-v1 case)
nor already covering it (nothing to do) -- and go red naming both SHAs
if so. `expect` is always a peeled commit (fetch_v1() reads
`refs/tags/v1^{commit}`), so this doesn't add a second failure mode
for an annotated v1; that tag form's existing "not a lost lease"
behavior on the first rejected push is untouched.
Surfaced by PR #28's final review. New selftest scenario 11 in
release-v1-selftest.sh, red-proven against the unfixed script (v1 was
silently moved off the stray commit); green after the fix, with the
full 7-suite gate (shellcheck + selftest.sh) passing.
Ride-alongs from the same review:
- ci.yaml:120-123 claimed the README's Versioning section documented
what's verified about the release token's write access; it said
nothing. Added an accurate sentence there (the grant is unobserved
until the first merge, capped by repo/owner token-permission maxima
and unreadable tag protections) and pointed the comment at it.
- Deleted two comment-as-decision-history paragraphs per
comments-are-not-exposition: ci.yaml's "no job-level concurrency"
rationale (kept one line of intent) and
prune-cache-selftest.sh scenario 9's account of how an
existence-only assertion used to pass with the pass-2 guard removed
(kept a one-line statement of what it checks).
- README's release-v1-selftest.sh table row now names the new
hand-placed-v1 scenario.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UkSjXXtU6JYcN2vPntWhfb
The release guard in 17d87b0 was safe but not live. Gitea 1.27.2 calls
CancelPreviousJobsByJobConcurrency whenever a job's `needs` resolve
(services/actions/clear_tasks.go:91, models/actions/run_job.go:641), so
a job's place in the `release-tag-v1` group followed when its own
selftest finished, not merge order. A newer merge C2 finishing selftest
first queued behind the older C1, C1 cancelled it, saw tip = C2, and
deferred: nobody pushed, and if merges then stopped v1 stayed stale
indefinitely behind a Skipped and a Cancelled job. The "always catches
up once merges pause" claim in ci.yaml and README was false.
What now holds:
- release-sweep.yaml runs on `schedule` every 15 minutes, in its own
workflow and concurrency group, so nothing in ci.yaml can cancel it.
When v1 already covers main's tip it stops after a checkout and one
merge-base. Otherwise it checks out the tip, runs the same shellcheck
and selftest.sh as ci.yaml's selftest job, and tags the tip only if
they pass; a failing main therefore turns the sweep red on every tick
while v1 lags, which is #27's AC1 loud-failure half. It reads the tip
itself because a scheduled run's github.sha is the CommitSHA recorded
when the schedule was registered on the last push to main
(services/actions/notifier_helper.go:569-580,
services/actions/schedule_tasks.go:126-141), and ref is the default
branch: schedules are registered only from it
(notifier_helper.go:120, :531, :603-604). event_name is "schedule"
(context.go:71 reads TriggerEvent, set at schedule_tasks.go:136).
Cron is 5-field robfig in UTC (models/actions/schedule_spec.go:38-41).
- 15 minutes, not 10: the sweep is the fallback, not the release path,
and every tick is a run on gitdan-ci's shared slots and a row in the
Actions list. 96 no-op runs a day of a few seconds each is the cost;
the lag bound it buys is one interval plus one selftest run.
- Both writers go through scripts/release-v1.sh and push with
--force-with-lease=refs/tags/v1:<v1 as read>, so v1 cannot move
backwards when the sweep and a merge job race. A lost lease re-reads
v1: at or ahead of this run's gated commit is a clean skip (the other
writer released something at least as new); still behind it is a
retry leased on the new value, up to three attempts, since the other
writer may have tagged an older commit and giving up there would leave
v1 short of a commit this run did gate; anything else goes red. A
rejection with v1 unmoved is diagnosed as a non-lease failure and goes
red at once.
- release-tag loses its job-level concurrency group. The lease already
gives the ordering the group was there for, and the group was what
cancelled the one job that could have released the newest merge.
Without it each merge's job runs, and the one whose commit is still
the tip when it checks releases it.
The shell moves out of ci.yaml into scripts/release-v1.sh so shellcheck
and selftest.sh cover it. release-v1-selftest.sh runs it against a
scratch bare origin: sweep no-op at and ahead of the tip, tag on a
green gate, no tag and a failing sweep on a red one, the stranded trace
above followed by a catching-up sweep, and each lost-lease outcome, with
a control showing an unleased push does step v1 back. Red-proved by
seven mutations of release-v1.sh, each failing a named assertion: plain
--force, accepting any lost lease, a merge job that never defers,
ancestry reduced to equality, no non-lease diagnosis, a sweep that never
needs to run, and a retry that does not re-lease.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UkSjXXtU6JYcN2vPntWhfb