fix: a commit only an unpushed local tag reaches now counts as unpushed - #521
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Reviewer's GuideThe PR fixes tag-only data-loss detection by comparing each workspace tag’s exact object with the repository’s bare mirror, then feeding only unvouched local tags into the existing unpushed-commit guard. It threads repository-specific mirror paths through rm, prune, and listing flows, conservatively keeps clones when comparison is unavailable, and adds unit, domain, and end-to-end regressions without changing the public Unsaved result vocabulary. Sequence diagram for tag-aware clone deletion guardsequenceDiagram
participant Lifecycle as rm_or_prune
participant State as workspace_state
participant Clone as WorkspaceClone
participant Bare as BareMirror
participant Git as Git
Lifecycle->>State: holds_unsaved_work(clone, bare)
State->>Git: tags_in_clone(clone)
Git-->>State: tag refs with object IDs
alt clone has tags and bare is available
State->>Git: tags_in_bare(bare)
Git-->>State: mirror tag refs with object IDs
State->>State: filter tags not vouched_for by exact name and object
else no bare or bare refuses
State->>State: treat every clone tag as local
end
State->>Git: unpushed_commits(clone, local_tags)
Git-->>State: Unsaved result
State-->>Lifecycle: keep clone or allow removal
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
blooop
force-pushed
the
fix/487-unpushed-local-tag
branch
2 times, most recently
from
August 29, 2026 19:59
56e7bc9 to
37242b5
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
blooop
force-pushed
the
fix/487-unpushed-local-tag
branch
from
August 29, 2026 20:05
37242b5 to
149ae23
Compare
blooop
force-pushed
the
fix/487-unpushed-local-tag
branch
2 times, most recently
from
August 29, 2026 20:28
6489e36 to
8570653
Compare
…to lose The delete guard excludes `refs/tags/*` from the unpushed question, because a tag the remote carries on a branch it no longer has would otherwise read as hundreds of unpushed commits on every clone of that repository forever (#485, fixed in #486). The exclusion was right about that case and silently wrong about its neighbour: tag before a rewrite, move the branch off the tag, and the commit under it exists in one place on earth while `dl rm` deletes the clone without asking and `--prune` without printing. Nothing inside a clone can tell those two apart, because no remote-tracking ref carries a tag. What can is the bare mirror the clone was made from, which fetches `+refs/tags/*:refs/tags/*` forced and pruned: a tag it holds at the same object is a tag the remote had at the last sweep, and a tag it has not got, or holds at another object, was typed here. Those come back into the query by name, so the commits only they reach are counted like any other unpushed work, and a local tag pointing at a commit the remote already has still costs nothing. The bare path is threaded through the resolver that already answers which directory a record's clone is in: `ClonePathResolver::bare_path`, answered in production by `resolve_bare_path` beside `resolve_clone_path`, and named off the record's repository rather than off the clone directory. `--prune` passes the same mirror it already computes while walking the cache. Where there is none to ask, every tag counts, which keeps the clone: `BareCache` is two arms rather than an `Option<&Path>` so every caller that cannot name one has to write the word. No fourth `Unsaved` arm: its three arms are the three documented keys of `dl --ls --json`. What changed is what counts as unpushed, not the vocabulary. Closes #487.
The refusal the tag comparison produces was "holds 1 unpushed commit(s). Push or
commit it, or run: dl <ws> rm --force", and for every tag-driven instance of it
that advice cannot be taken: the commit is already committed, and where the
mirror is merely behind the remote it is already pushed too. What is missing is
the tag in dl's cache, which no amount of pushing supplies and which the sentence
did not mention. That is the state docs/cleanup.md argues against two paragraphs
above the one this adds: a guard that has to be --force'd past to clear, until
--force is what gets typed without reading.
So the answer now carries what makes the sentence actionable. A second local
query, asked only once a refusal exists and only when a local tag was named in
the one that produced it, isolates the commits nothing but those tags reach: the
same ref algebra with the sides swapped. Its result is a subset of the first by
construction, so both counts can be said at once without either being a second
measurement:
holds 2 unpushed commit(s), 1 reachable only from local tag(s) (backup)
Both numbers, because they answer different questions: how much would be lost,
and how much of it pushing cannot clear. A commit on a branch is not blamed on a
tag, and a tag reaching nothing a branch does not reach is named nowhere, so the
plain sentence survives for the case where it was right.
`ByLocalTags` holds the tags and the commits owed to them, both `NonEmpty`, so
"some tags reached no commits" has no representation and the absent attribution
is an absent value. A refusal from this query is *not* a `CouldNotTell` and it is
the one place this module bends that rule: by then the loss is established and
the clone is kept either way, so no failed question here can be read as
permission. Only the naming half is lost.
Also pins `TagRef`'s equality on both name and object, since `vouched_for` is
that equality: derived on the name alone the whole comparison would degrade to
"the mirror has heard of this name", which fails in the deleting direction and
which every fixture where the two agree would pass.
blooop
force-pushed
the
fix/487-unpushed-local-tag
branch
from
August 29, 2026 20:45
8570653 to
53dd4dc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #487.
The lie
dl <ws> rmanddl --pruneask one question before destroying a clone: is thereanything in here that exists nowhere else? Since #486 that question excludes
refs/tags/*wholesale, and it had to: a tag your remote carries on a branch itno longer has reaches commits no
refs/remotes/*reaches, so a repository thattags releases on branches it then deletes reported hundreds of unpushed commits on
every clone, forever (#485, 265 of them on one real repository, six of eight
workspaces on a host refusing to be deleted, and not one commit of it real).
The exclusion is right about that case and silently wrong about its neighbour:
The commit
backup-before-rebasereaches is now the only copy of itself, and theguard answers
NothingToLose, whichflows/lifecycle.rsreads asMayRemove.That is the backup habit an agent rewrite workflow actively encourages, so this
was the highest-priority data-loss path left in the repo.
The fix
Nothing inside a clone can tell the two cases apart: no remote-tracking ref
carries a tag, so
refs/tags/has no mark saying which name arrived in a fetch.What knows is dl's own bare mirror at
repos/<owner>/<repo>/.bare, which fetches+refs/tags/*:refs/tags/*forced and pruned (#476) and is what every workspaceclone is made from. So the guard compares the two, offline:
The tags that come back counted are named as ordinary positive refs in the
existing query, between the
--allthey were excluded from and the--notthatwould flip them:
so a local tag pointing at a commit the remote already has still costs nothing.
No fourth
Unsavedarm: the three arms are the three documented keys ofdl --ls --json, which wf consumes (#485's lesson). What changed is what countsas unpushed, not the answer vocabulary.
The refusal says which tag
Naming the tags is the difference between a refusal you can act on and one you
cannot. "Push or commit it" is advice already taken for a commit under a tag, and
where the mirror is merely behind the remote the commit is already pushed too:
Both counts, because they answer different questions: how much would be lost, and
how much of it pushing cannot clear. A second local query, asked only once a
refusal exists and only when a local tag was named in the one that produced it,
isolates the commits nothing but those tags reach (the same ref algebra with the
sides swapped, so its result is a subset of the first by construction). A commit
on a branch is not blamed on a tag, and a tag reaching nothing a branch does not
reach is named nowhere.
ByLocalTagsholds both halves asNonEmpty, so "sometags reached no commits" has no representation. A refusal from that query is not
a
CouldNotTelland it is the one place this module bends that rule: by then theloss is established and the clone is kept either way, so nothing here can read a
failed question as permission, and only the naming half is lost.
This also covers a case worth naming: a tag pushed from a workspace and not yet
swept into the cache, on a commit no remote branch reaches, reads as unpushed.
That is #485's own repository shape, and it is narrow, but "reachable only from
local tag(s) (v0.26.0)" is what tells the reader the cache is stale rather than
leaving them to
--forcepast a refusal they cannot otherwise explain.Why #486's false positive does not come back
The mirror and the clone agree by construction for everything that came off the
remote:
dlfetches tags into the bare andgit clonecopies them from thereinto the workspace, same objects on both sides. #485's own fixture is pinned as a
test in both places, at the seam
(
a_tag_no_remote_branch_reaches_any_more_is_not_unsaved_work) and at the binaryboundary (
a_clone_whose_last_tag_the_remote_carries_too_is_deleted_like_any_other),and both still say
NothingToLose.a_local_tag_on_a_commit_the_remote_already_has_is_not_a_losscovers the narrower way it could have come back, where naming a tag makes a clone
refuse for commits the remote does have.
The comparison is exact only up to the last sweep, which is the trade the ticket
picked: a tag pushed straight from a workspace and not yet fetched into the cache
reads as local until the next fetch, and keeps the clone.
Where the bare path comes from
read_clone(&Git, clone)knew only a directory. The seam chosen is the resolverthat already answers "which directory is this record's clone in":
ClonePathResolvergainsbare_path(record), and the production implementationanswers
resolve_bare_path, besideresolve_clone_pathon the clone manager. Itis named off the record's repository, not off the clone directory, so a record
whose
local_pathwas written by an older layout still finds the mirror itsoriginpoints at.--prunegets it from the samebare_dir(owner, repo)italready computes while walking the cache.
The absent case is a two-armed
BareCacherather than anOption<&Path>, becauseit is an answer with a direction:
Unknowncounts every tag as local, and everycaller that cannot name a mirror has to write the word. Principle 1 of map #444: a
check that cannot prove safety fails towards keeping.
Cost
One extra local
for-each-refper clone, and a second only when the clone has atag at all (
a_clone_with_no_tags_never_asks_the_barepins that). No network.Tests
Red before green: with the comparison stubbed back to "no local tags", exactly the
four new behaviour tests fail and the two anti-regression ones stay green.
a_commit_only_an_unpushed_local_tag_reaches_is_unsaved(was..._is_given_up, which asserted the bug)a_local_tag_the_bare_holds_at_another_object_is_unsavedwith_no_bare_to_compare_against_every_tag_countsa_bare_that_is_not_a_repository_counts_every_tag_tooa_local_tag_on_a_commit_the_remote_already_has_is_not_a_lossa_clone_with_no_tags_never_asks_the_barethe_two_tag_queries_ask_the_same_question_of_each_side,a_tag_listing_is_read_as_the_pairs_it_is(client layer)a_commit_only_a_local_tag_reaches_stops_the_delete(binary boundary, new--local-tagworld)the_mirror_is_named_off_the_repository_wherever_the_clone_was_recordedanda_record_whose_repository_the_name_rules_refuse_names_no_mirror(the resolver)For the message (each red without the attribution, green with it):
a_branch_commit_is_not_blamed_on_a_tag_that_happens_to_be_there(two unpushedcommits, one tag's: both counts said separately)
a_tag_that_reaches_nothing_of_its_own_is_not_named_in_the_refusala_long_list_of_tags_is_cut_short_like_every_other_list(one truncation rule,shared with the changed-paths list rather than written twice)
the_attribution_is_given_up_rather_than_the_refusal_when_git_will_not_sayandno_local_tags_is_no_question_askedthe_attribution_query_is_the_unpushed_one_with_the_sides_swappeda_tag_is_its_object_as_well_as_its_name(pins the equalityvouched_forrestson, which no fixture where the two sides agree could catch)
On the checked-in
public-apisnapshotThe rows for
ClonePathResolver::bare_path,Loss::Unpushed's new shape andByLocalTagsare hand-written, because this host carries neither nightly nor thepinned
cargo-public-api. That is not a second hand-maintained copy of a fact:CI's
public-apijob runsscripts/public-api-snapshots.shinto a scratch treeand diffs the regenerated output against the file, so a hand-written row that is
wrong fails the job rather than being trusted. It is green here, which is the
verification.
🤖 Generated with Claude Code
Summary by Sourcery
Prevent cleanup from deleting clones whose only remaining commits are reachable through local tags, while retaining the existing protection against false positives from remote-backed tags.
Bug Fixes:
Enhancements:
Documentation:
Tests: