Retention can delete a pinned version out from under the pin #141

New issue

Closed

opened 2026-08-30 19:15:57 +02:00 by qwc · 0 comments

qwc commented

2026-08-30 19:15:57 +02:00

Owner

Found while implementing #127 (see PR #140). Pre-existing, not introduced there.

What happens

enforceRetentionPolicy (internal/handler/retention.go) decides what to delete purely from the keep-rule and the version's age. It never consults projects.pinned_version, and nothing else guards the delete path either — grep PinnedVersion finds no reference in retention or in VersionStore.Delete.

So a version pinned as latest is deleted like any other once it ages past the retention period and doesn't match the keep rule. The project is left with pinned_version naming a version that no longer exists.

Reproducing

A project with retention set (per-project, or the instance retention.nonsemver_days)
Upload a build with a tag the keep rule doesn't cover — pre-#140 that means any non-semver tag, e.g. nightly-2026-01-01
Pin it as latest (Admin > Projects > Edit, or the pin control on the project page)
Wait out the retention period, or backdate versions.created_at

The retention worker deletes it on its next hourly pass. project.go:102 then falls through to the newest version, so the pin is quietly ignored rather than erroring — which is why this can go unnoticed.

Why it matters

Pinning is how you say "this is the version people should land on". That's the same statement the keep pattern makes, so the two disagreeing is surprising. It's most likely to bite exactly where pinning is most useful: pinning a dated or branch-named build that the keep rule was never written to match.

Possible fixes

Skip the pinned version in enforceRetentionPolicy — smallest change, matches the intuition that pinning protects. PinPermanent may deserve a distinction: a permanent pin is clearly protected, while a transient pin (cleared on the next upload, upload.go:221) is arguable.
Refuse to pin a version the keep rule would expire, and say so in the UI — pushes the decision to pin time, but leaves an already-pinned version exposed when the pattern later changes.
Leave as is and document it — what PR #140 does for now, in how-to/version-retention.md.

Option 1 seems right, with permanent pins definitely protected; the transient-pin case is a product call.

Reported by Claude Opus 5.

Found while implementing #127 (see PR #140). Pre-existing, not introduced there. ## What happens `enforceRetentionPolicy` (`internal/handler/retention.go`) decides what to delete purely from the keep-rule and the version's age. It never consults `projects.pinned_version`, and nothing else guards the delete path either — `grep PinnedVersion` finds no reference in retention or in `VersionStore.Delete`. So a version pinned as latest is deleted like any other once it ages past the retention period and doesn't match the keep rule. The project is left with `pinned_version` naming a version that no longer exists. ## Reproducing 1. A project with retention set (per-project, or the instance `retention.nonsemver_days`) 2. Upload a build with a tag the keep rule doesn't cover — pre-#140 that means any non-semver tag, e.g. `nightly-2026-01-01` 3. Pin it as latest (**Admin > Projects > Edit**, or the pin control on the project page) 4. Wait out the retention period, or backdate `versions.created_at` The retention worker deletes it on its next hourly pass. `project.go:102` then falls through to the newest version, so the pin is quietly ignored rather than erroring — which is why this can go unnoticed. ## Why it matters Pinning is how you say "this is the version people should land on". That's the same statement the keep pattern makes, so the two disagreeing is surprising. It's most likely to bite exactly where pinning is most useful: pinning a dated or branch-named build that the keep rule was never written to match. ## Possible fixes 1. **Skip the pinned version in `enforceRetentionPolicy`** — smallest change, matches the intuition that pinning protects. `PinPermanent` may deserve a distinction: a permanent pin is clearly protected, while a transient pin (cleared on the next upload, `upload.go:221`) is arguable. 2. **Refuse to pin a version the keep rule would expire**, and say so in the UI — pushes the decision to pin time, but leaves an already-pinned version exposed when the pattern later changes. 3. **Leave as is and document it** — what PR #140 does for now, in `how-to/version-retention.md`. Option 1 seems right, with permanent pins definitely protected; the transient-pin case is a product call. --- Reported by Claude Opus 5.

qwc referenced this issue from a commit

2026-08-30 20:02:13 +02:00

Keep permanently pinned versions out of retention (fixes #141)

qwc referenced this issue from a pull request that will close it,

2026-08-30 20:02:22 +02:00

Keep permanently pinned versions out of retention (fixes #141) #143

qwc referenced this issue from a pull request that will close it,

2026-08-30 20:02:33 +02:00

Let a project name the versions worth keeping (fixes #127) #140

qwc closed this issue

2026-08-30 20:06:48 +02:00

qwc referenced this issue from a commit