COR-1300 — context

Status: In Progress, assigned to you, Medium. Branch nodo/cor-1300-native-mirror-of-an-empty-repo-stays-flagged-behind-forever. No comments. Related: COR-1283 (parent work, In Progress, sweep already armed in all four prod cells — fleet#4886, infra#1257).

Workspace

/Users/nodo/work/tasks/empty-repos — six repos, all on main, all current with COR-1283's last merges:

git-sync is not in the workspace — it's only vendored (mirror-pipeline/vendor/entire.io/entire/git-sync). Option 1 (converge-to-empty) needs a change in a repo that isn't checked out here; options 2 and 3 are entirely inside the checked-out repos. That's a strong hint the workspace was assembled for option 2.

The four links, in code

1. Unconditional bump — entiredb/refstore/oneshot.go:136-150: bump queued last, bumped := len(commands) > 0, gated only on command count, not CAS outcomes ("errs on the bump side"). Contract documented in refstore/write_position.go:26-33: monotonicity is the only invariant, not density.
2. Detect — entiredb/core/nativemirror/resync_sweep.go:471 flagBehind: applied_position < v.ordinal + serving-stage filters; resync_due_at = COALESCE(..., now()) so the age freezes at first detection. clearFlags (:302) only clears on applied_position >= resync_target_ordinal.
3. Zeroed position — mirror-pipeline/cmd/worker/consumer.go:~470 dispositionNoop branch: publishReadyOrRetry(..., 0), with the full rationale in the comment. Error path: errSourceNoRefs in syncer_errors.go matched from git-sync's "no source refs matched" substring (three raise sites: syncer.go:842 runSync, :950 runReplicate, :1241 buildDesiredRefs — no exported sentinel).
4. No exit — nativemirror.go:847: applied_position = GREATEST(applied_position, $3), so 0 is a no-op; repair republishes on RepublishBackoff (default 30m, resync_sweep.go:143-160; Grace 5m, FullScanInterval 24h, MaxRepairsPerPass 200).

Facts that shape the fix

• The invariant the ADR states as the review test for any change here (docs/adrs/20260818-native-mirror-resync-sweep.md:164-171): the watermark may under-claim, never over-claim. Option 2 must argue that "source empty and target empty" is a genuine over-claim-free convergence, not a laundering.
• The ADR already names option 1 as the planned follow-up (:395-400): "an explicit converge-to-empty for entiredb-kind syncs, after which the drill claim extends to the delete-all class."
• Option 2's missing input: the worker never learns the target's ref count. gitsync.Result/ProbeResult expose source refs only (vendor/.../results.go:91); target refs live in s.target.refMap, unexported. The worker's one existing probe (cmd/worker/syncer.go:505) probes the source. Cheapest route without touching git-sync: probe the target URL as a Source endpoint to enumerate its refs — an extra round trip on the no-op path only.
• Runbook already documents this at entiredb/docs/runbooks/native-mirror-resync.md:335-372 ("Emptied source"), corrected yesterday in entiredb#3314 — mechanism text is now accurate (completion is published, snapshot does succeed, position is zeroed), and it explicitly cites COR-1300 for the false-positive half. Also there: mirror.position_snapshot{result:ok} is not evidence a watermark advanced.

Blast radius

Sweep is live in all four prod cells with grace 1m. Any native-mirrored repo created and never pushed to self-drives an elevated RPO alert in ~2×N (7200s in eu) plus a no-op repair every 30m — plausible for onboarding flows and test repos.

Want me to plan the option-2 implementation, or scope option 1 (which needs the git-sync repo added to the workspace)?

git-sync changes the picture — option 1 is nearly free

git-sync @ 4e816de7 (main, ahead of the vendored v0.8.0 — the mirror-pipeline vendor tree is a release behind, so the fix needs a tag + vendor bump to land).

The converge-to-empty machinery already exists and is already switched on. The worker calls client.Replicate with Prune: true, AllRefs: true (mirror-pipeline/cmd/worker/syncer.go:379-445). Everything downstream of planning handles a delete-only plan set today:

• planner.addPruneCandidates (internal/planner/planner.go:253) registers every non-excluded target ref as a deletion candidate when it's absent from the desired set — with an empty desired set, that's all of them.
• BuildReplicationPlans (:231) emits ActionDelete for each.
• runReplicate filters deletes out of relayPlans and only runs the relay-capability check if len(relayPlans) > 0 (internal/syncer/syncer.go:1003-1014) — a delete-only set skips it cleanly.
• repstrat.Execute sends deletes as ref-only commands with no pack (internal/strategy/replicate/replicate.go:79-83).
• finalizeCounts tallies them into Result.Deleted — the "ref-applying success" signal the completion needs.

The only thing blocking it is the guard at internal/syncer/syncer.go:958: if len(desiredRefs) == 0 { return Result{}, errors.New("no source refs matched") } — it bails before planning, so prune never gets a chance. (Same guard at :850 runSync, :1092 Bootstrap, :1249 buildDesiredRefs; only the replicate one is on the worker's path.)

Two edges to handle in the fix

1. Empty source + empty target (the never-written case) falls into the bootstrap branch: replicateCanBootstrap returns true (empty desired loop, prune loop over an empty target refMap), so bootstrapWithInputs runs bstrap.Execute → FetchPack with a zero desired set (internal/strategy/bootstrap/bootstrap.go:145-161). Needs a short-circuit: nothing to do, return a clean zero-plan success.
2. Error-classification coupling. mirror-pipeline/cmd/worker/syncer_errors.go detects the no-op by substring-matching "no source refs matched" (there's no exported sentinel — errors.go only exports ErrTargetRefMoved/RefRejectedError). If the empty source becomes a success, errSourceNoRefs/dispositionNoop stops firing on this path, and outcome:noop stops being the confirmation signal the runbook tells operators to check (native-mirror-resync.md:366-372).

Target-side safety checks out

Deleting every ref on an entiredb mirror is not specially rejected: no delete rules in refpolicy/, and updateHEADIfTargetMissing (server/githttp/receive_pack.go:1022-1056) explicitly skips delete commands via firstSuccessfulBranchUpdate — it only repoints HEAD on a successful branch update, so an all-delete push leaves HEAD dangling at its provisioned refs/heads/main, which is exactly the state a never-pushed repo is already in.

So option 1 is: drop/relax one guard + one empty-empty short-circuit in git-sync, tag, bump the vendor in mirror-pipeline, and rework the worker's noop classification (plus the runbook + ADR:395-400 follow-up note). That fixes the emptied case properly rather than just silencing the false positive — and it's smaller than option 2, which would need a second probe round trip to learn the target's ref count.

Want me to plan it out, or start on the git-sync change?

Do option 1, drop option 2. Option 2 was defensible only while option 1 looked far off; now that git-sync is here it's the smaller change of the two and it's the one that would survive — option 2's target-probe plumbing becomes dead code the moment option 1 lands.

One refinement to option 1 as the issue frames it, and it's the safety-critical part. Converge-to-empty turns an empty ref set from inert into destructive: today the guard makes "source advertises nothing" incapable of deleting anything; afterwards it deletes every ref on the mirror. So gate the convergence on the source advertisement being empty, not on the desired set being empty after filtering. git-sync currently collapses those two into one error string — the worker's own comment concedes this ("it collapses 'empty source' and 'selection matched nothing' into the same unexported error string") and then argues they can't diverge because the worker passes AllRefs with no selector. That argument doesn't hold: the worker does pass ExcludeRefPrefixes + ExcludeRefs (refs/tlog/* and the reserved anchors survive even for entiredb-kind sources, syncer.go:119-130). A source that advertises only excluded refs would desire-empty and, post-change, wipe a mirror holding real refs. Splitting the two conditions is a few lines in BuildDesiredRefs's callers and it hands you the exported sentinel that replaces the worker's substring match.

Sequence:

1. git-sync — distinguish advertisement-empty from selection-empty; on advertisement-empty in replicate mode, fall through to prune instead of erroring; short-circuit empty-source-and-empty-target to a clean zero-plan success before it reaches bstrap.Execute's FetchPack. Export a sentinel for selection-empty (which stays an error). Tag it.
2. mirror-pipeline — vendor bump off v0.8.0, then rework the noop classification: an emptied source is now an Ack that carries the snapshot position; errSourceNoRefs/dispositionNoop should only survive for cases that genuinely applied nothing.
3. entiredb — correct the runbook's "Emptied source" section (it currently instructs the operator not to fix this, which stops being true) and close the ADR's :395-400 follow-up.

Two things to leave alone: the unconditional write-position bump (oneshot.go:136) and detect's rule. Both are correct; this issue is entirely about giving the repair a way to converge.

The one thing I'd think through before writing code is the restore interaction. A primary mid-restore can plausibly advertise zero refs, and restore-mode writes deliberately suppress ref events — so the sweep is the only lane touching those placements (ADR :401-413). Under today's guard that's a harmless no-op; under converge-to-empty a repair would delete the mirror's refs at exactly the moment the primary is least authoritative. Worth deciding whether the repair path needs to refuse to converge-to-empty against a primary in restore, or whether the post-restore reset procedure already covers it.

Validation: re-run the COR-1283 drill with a delete-all case — the ADR only extends the drill claim to that class once this lands.

It's already in the session — nothing new to fetch.

s.sourceRefMap (internal/syncer/syncer.go:759) is the raw advertisement: gitproto.RefHashMap(sourceRefs) straight out of ListSourceRefs(ctx, conn, protocolMode, refPrefixes). The exclusions never touch it — IsRefExcluded is applied downstream, inside BuildDesiredRefs (internal/planner/planner.go:85 and :107). So the two conditions are distinguishable by a length check on either side of that call, with no extra round trip:

What makes len(sourceRefMap) == 0 trustworthy on the worker's path is that its request is unscoped: AllRefs: true → planner.RefPrefixes returns ["refs/"] (internal/planner/types.go:185-188), which is the entire ref namespace, and the v1 fallback advertises everything unconditionally. Empty map means the repo has no refs.

That's the condition to make explicit rather than assume. With AllRefs false, RefPrefixes narrows the wire request to refs/heads/ (+refs/tags/), so an empty sourceRefMap then means "nothing under the prefixes I asked for" — not "empty repo" — and converging on it would delete refs that exist. So gate the converge branch on s.cfg.AllRefs; every other config keeps today's error. The worker always passes AllRefs, so this costs nothing operationally and makes the guarantee local to the code rather than resident in a comment three repos away.

The check also does the work I was worried about in the other direction: an entiredb source whose only refs are refs/tlog/* advertises non-empty and desires empty, so it lands on ErrNoRefsSelected and never wipes the mirror. Today both cases return the same errors.New("no source refs matched"), which is precisely why the worker's substring match can't tell them apart — and why exporting the sentinel is worth doing in the same change (it replaces isNoSourceRefsError's string matching in mirror-pipeline/cmd/worker/syncer_errors.go:38).

Whether the converge branch then deletes anything is a separate question answered by s.target.refMap: non-empty → the all-delete prune plan set; empty → nothing to do, return the zero-plan success (this is the case that must short-circuit before bstrap.Execute).

Launched a Fable 5 consult with the full context — the four-link chain, the option-1 approach, the advertisement-vs-selection-empty gate, and my six open questions (including the restore interaction and whether sourceRefMap can come back empty for a repo that has refs, which is the mirror-wiping failure mode).

It's running in the background; I'll report back when it lands.

Consult came back arguing against shipping option 1's destructive branch. I verified its decisive claims myself — they hold, and they change my recommendation.

What I got wrong

"Fixes both cases" is false — the emptied case can't converge against an entiredb target. Two server-side guards I hadn't accounted for:

• denyCurrentBranchDelete runs unconditionally on every push (entiredb/server/githttp/receive_pack.go:481), with no worker or restore exemption. A delete-all against a populated mirror always includes HEAD's branch, so that delete is refused per-ref.
• gittuf ref deletes are refused in every mode — "ordinary push, replication, repair, restore" — at entiredb/refstore/recording_store.go:171,186. The worker's exclusion list doesn't exclude refs/gittuf/, so any trust-carrying mirror wedges too.

And the follow-on is worse than a partial failure. That rejection is exactly what the worker's default-branch recovery hooks (mirror-pipeline/cmd/worker/syncer.go:501), which probes the source HEAD — and an empty v2 source returns "", because git-sync skips unborn lines (internal/gitproto/refs.go:214) and never requests the capability. defaultbranch.go:213 then returns permanentErr(errSourceHEADNotBranch) → Term → publishFailed. So option 1 turns today's quiet 30-minute no-op trickle into a partial deletion plus a terminal failure and a failed status on the placement, with the flag still standing.

Second, smaller correction: my claim that empty/empty would break in bstrap.Execute was wrong — FetchPack with zero wants short-circuits to NoErrAlreadyUpToDate (internal/gitproto/fetch.go:211-213), which bootstrap treats as success. The early return is still worth having, for legibility rather than safety.

Revised recommendation: option 2's semantics on option 1's plumbing

Keep the git-sync split exactly as designed — advertisement-empty vs selection-empty, gated on AllRefs — but make empty source + non-empty target a typed refusal, not a delete. Only empty/empty converges and stamps.

That keeps everything the split buys (an exported sentinel replacing the substring match, the false positive killed, the emptied signature made operator-visible via a distinct metric instead of disguised as a generic noop) and drops the one part that can't work. It also preserves the ADR invariant cleanly: the empty/empty stamp isn't an over-claim, because coverage is verified against the target's own advertisement in the same session.

Deletion stays off the table until a restore-state marker exists in the control plane (the consult searched core/nativemirror and the migrations — there's no machine-readable restore-in-progress state, which is why the runbook step is load-bearing) and the deny-current-branch/gittuf interaction has a designed answer. That's a smaller, different promise: converge-to-empty-except-HEAD-and-trust.

Ancillary findings worth acting on

• A separate live bug: a v1 empty advertisement surfaces as transport.ErrEmptyRemoteRepository (git-sync/internal/gitproto/refs.go:257-262), which isNoSourceRefsError's substring match doesn't catch. So on an entiredb node serving v1 (mid-rollout), a never-written repo Naks to max-deliveries and terms today. Worth its own issue.
• Selection-empty must stay benign for GitHub sources — a repo whose only refs are refs/pull/* is legitimate and currently acks as a noop; making selection-empty an error would term those.
• Rollout skew: the worker must keep matching the old string until the vendor bump lands, and the new selection-empty sentinel must not contain it.
• The converge path acks via dispositionAck, which calls clearIndex — the current noop deliberately doesn't. Probably right now that coverage is provable, but make it deliberate.
• If deletion ever ships: explicit SyncPolicy opt-in (git-sync ships a standalone CLI where the guard currently protects against a mistyped source URL), gated on entiredb-kind sources, plus a worker-side kill switch.

Want me to write this up as a comment on COR-1300 and file the v1-classification bug separately?

The bug in one line: a brand-new empty repo gets marked "this mirror is behind" and nothing can ever un-mark it, so it alarms forever.

Why nothing can un-mark it. The flag clears when the mirror reports "I've caught up to position N." The mirror's repair job syncs, finds no branches to copy, and reports back "I did nothing" — deliberately without a position number. So the flag stays, a repair runs every 30 minutes, and the staleness clock keeps climbing until it trips the alert.

That "report no position" rule exists for a good reason: it protects a genuinely dangerous case. If someone deletes all the branches on the primary, the sync also finds nothing to copy — but the mirror is still serving the old deleted branches. Reporting "caught up" there would permanently hide real divergence. So the rule stays.

The problem is that one signature covers two very different situations:

• Primary empty, mirror empty → identical, nothing wrong. The flag is a false alarm.
• Primary emptied, mirror still full → real divergence. The flag is correct and valuable.

The fix: look at the mirror too, and tell the two apart.

• Both empty → genuinely in sync. Report the position, flag clears, alarm gone. This is the case causing the noise.
• Primary empty but mirror still has branches → don't report a position. Flag stands, exactly as today — but now it's a distinct, named outcome instead of being lumped in with "did nothing," so it shows up on a dashboard as the real problem it is.

Nothing gets deleted, and the protection stays intact. Today's code can't make this distinction because it never looks at the mirror's side before deciding.

One supporting piece. The sync library currently uses one error for two things: "the source has no branches at all" and "the source has branches, but our filters excluded all of them." Those need separating, because only the first one is safe to treat as "in sync." The information is already in memory — it's just never checked.

What I originally proposed and dropped: making the empty case actively delete the mirror's branches, so the emptied case would truly converge. It doesn't work — the git server refuses to delete the branch HEAD points at and refuses to delete trust refs, and the way the worker handles that refusal turns the whole job into a hard failure. So the emptied case would end up worse off than it is today. That's parked until there's a way to tell "the primary is legitimately empty" from "the primary is mid-restore."

Scope: a small change in the sync library, a matching change in how the mirror worker classifies the result, and a runbook correction (it currently tells operators not to fix this).

Two traces of the same signal. Components: refstore (on the primary cluster royalcanin, owns the ref data and the repo_write_positions counter), the resync sweep (cron in entire-core, owns the native_mirrors flag columns), the mirror worker (mirror-pipeline, runs the repair), and git-sync (does the actual ref copying). Repo is et/nodo-project/native-repo-42, mirror placement on eukanuba.

Story A — never written (the bug you hit)

1. Repo + native mirror placement created. Repo has zero refs.
2. Some internal transaction during creation commits → refstore on royalcanin bumps ordinal 0 → 1. (It counts committed write transactions, not refs changed.)
3. The native_mirrors row for the eukanuba placement has applied_position = 0.
4. Sweep: applied_position (0) < ordinal (1) → flags it. resync_due_at = 10:17:01Z, resync_target_ordinal = 1.
5. Sweep publishes a repair job. Mirror worker picks it up.
6. Worker asks royalcanin "what's your write position?" → 1. Holds it as its snapshot.
7. Worker calls git-sync. git-sync asks royalcanin for refs → zero refs → returns no source refs matched.
8. Worker calls this a benign no-op and publishes mirror_done{status: ready, source_position: 0} — it discards the snapshot of 1 and sends 0.
9. entire-core applies applied_position = GREATEST(0, 0) = 0. Nothing moves.
10. Next sweep pass: is applied_position (0) >= resync_target_ordinal (1)? No → flag stays, healed: 0.
11. Forever, every 30 min. The clock runs from 10:17:01Z, hits 7200s at ~12:17Z → RPO monitor fires.

Step 8 is the whole bug. The worker had the number that would have cleared the flag, and deliberately threw it away.

Story B — emptied (why step 8 throws it away)

1. Same repo, but with real content: 12 branches, ordinal = 40. eukanuba has applied_position = 40 and serves those 12 branches. Healthy.
2. Someone deletes all 12 branches on royalcanin. That's a write → ordinal = 41. But the ref-delete events are lost in transit — the exact failure the sweep exists to catch.
3. eukanuba is still serving all 12 branches. They're gone on the primary and live on the mirror. Anyone reading the mirror gets deleted branches. Real divergence.
4. Sweep flags it: 40 < 41. Correct and valuable — this is the sweep doing its job.
5. Repair job. Worker snapshots the position → 41.
6. git-sync asks royalcanin for refs → zero refs → no source refs matched.

Step 6 is byte-identical to story A step 7. From where the worker stands, the two situations are indistinguishable: source advertises nothing, git-sync copied nothing.

1. Now suppose the worker had stamped its snapshot: applied_position = GREATEST(40, 41) = 41. Next sweep pass: 41 >= 41 → flag clears. The sweep now believes this placement is fully caught up — while eukanuba serves 12 branches that no longer exist. The divergence is invisible to the only mechanism built to find it, permanently, until someone pushes to that repo again.

That's why the zero. The worker can't tell A from B, so it refuses to claim coverage it can't prove — accepting story A's forever-flag as the price of not laundering story B.

The fix

Look at eukanuba's refs before deciding. The worker's session already holds that list; it just never consults it.

Story A stops alarming. Story B keeps its flag and gets a name on the dashboard instead of hiding inside the no-op bucket. Nothing is deleted, and the protection is intact — it just stops firing on the case it was never meant to catch.

They don't — not automatically. Under my fix, story B's 12 stale refs on eukanuba sit there until one of two things happens:

1. The next real push to royalcanin — this is the actual healing path, and it's already wired. The repair job isn't a per-ref delta; the worker does a full AllRefs + Prune replicate every time. So the moment royalcanin has one branch again, git-sync's desired set is non-empty, prune registers all 12 stale refs on eukanuba as deletion candidates, and they go. If HEAD on eukanuba points at one of the doomed branches, denyCurrentBranchDelete refuses that one ref — and that's exactly what the worker's default-branch recovery exists for: source HEAD is now a real branch, so it calls SetDefaultBranch on the target, repoints HEAD, re-runs the replicate, and the delete lands. Full convergence. (This is the same mechanism that cleared your staging repro instantly when you pushed a commit.)

2. An operator — the runbook's manual step, or teardown-and-recreate the placement (which zeroes applied_position and re-seeds from scratch).

This is unchanged from today, and my fix doesn't improve it. Worth being blunt about, because it's the gap: the ADR already accepts exactly this behavior — an emptied primary "leaves its placements flagged and alarmed — serving the deleted refs — with a slow no-op repair trickle until an operator intervenes or the primary writes again" — and the converge-to-empty follow-up it names was meant to be the automatic cleanup. I'm recommending you not build that, so the cleanup stays manual-or-next-push. What the fix buys is that the flag now means something: it fires only for story B, so the alarm is a real signal an operator should act on instead of noise they learn to ignore.

And there's a wrinkle worth knowing before treating converge-to-empty as the eventual answer. Even if you built it, it can't fully empty the mirror: denyCurrentBranchDelete won't drop HEAD's branch and the gittuf guard won't drop trust refs, in any mode. So the best it could do is converge-to-empty-except-HEAD-and-trust, leaving a residue — and because it deliberately left refs behind, it still couldn't honestly stamp a position. The flag would stand anyway. So it would clean up the bulk of the divergence but not close the loop the way the ADR assumed. That's a materially smaller prize than "fixes both cases," and it's part of why I'd park it.

How much this matters in practice: getting into story B at all is hard through supported paths. royalcanin has the same deny-current-branch rule, so its default branch can't be deleted over git, and once RSL recording has run the repo carries an undeletable trust ref. So "primary advertises zero refs while the mirror holds refs" realistically comes from a restore in progress, a DB restore to a pre-first-push state, or out-of-band surgery — and in every one of those, automatically deleting the mirror's refs would destroy the only good copy. Which is the same reason the delete path needs a restore-state marker before it's safe to build.

Yes. Story A is the one that's actually hurting — it alarms on its own, it's reachable through normal onboarding, and it's a false positive. Story B keeps exactly today's behavior (flag stands, no position stamped), just with a name on it.

Concrete scope:

git-sync — split the single no source refs matched error into typed outcomes: advertisement-empty (source genuinely has no refs — trustworthy only under AllRefs, since narrower prefixes make an empty map ambiguous) vs selection-empty (filters excluded everything). Then: empty source + empty target → zero-plan success; empty source + non-empty target → distinct typed refusal. Export the sentinels. Tag a release.

mirror-pipeline — bump the vendored git-sync off v0.8.0, then rework the classification: empty/empty acks carrying the snapshot position; the other two keep position 0. Three details to get right — keep matching the old error string through the rollout window, keep selection-empty benign for GitHub sources (refs/pull/*-only repos are legitimate), and make the clearIndex change on the new ack path deliberate rather than incidental.

entiredb — the runbook's "do not fix this by stamping the snapshot" becomes conditional (it's still true for story B, wrong for story A), and re-scope the ADR's converge-to-empty follow-up to say what we learned: it can't fully empty a mirror, so it wouldn't close the loop anyway.

Explicitly not touching: the unconditional write-position bump, detect's rule, and anything that deletes refs.

Separately, the v1 classification bug is worth its own issue — an empty repo on a v1-serving node surfaces ErrEmptyRemoteRepository, which nothing matches, so it burns all ten deliveries and terms. Same symptom class, different cause, independent fix.

Want me to start on the git-sync change, or write the scope up on COR-1300 first?

Yes — the scope I just gave you is close to verbatim its recommendation. Its closing paragraph was: split the guard into typed outcomes gated on AllRefs; empty source + empty target → zero-plan success that stamps; empty source + non-empty target → typed refusal with a distinct metric; leave the write-position bump and detect alone; revisit deletion only after a restore-state marker exists. I adopted it after checking its five load-bearing claims against the code myself — deny-current-branch running unconditionally, the gittuf delete refusal in every mode, the v1 ErrEmptyRemoteRepository path, the zero-wants fetch short-circuit, and unborn being skipped. All five held.

But I dropped two of its recommendations when I wrote that scope out, and one of them I now think I was wrong to drop:

Request the unborn capability. git-sync doesn't ask for it and discards unborn lines, so "the source is empty" is inferred from zero data lines rather than asserted by the server. entiredb's ls-refs already supports it. The subagent framed this mainly as a safeguard for the destructive branch — but it matters for the non-destructive fix too, and I missed that when I summarized. Consider: if a bogus empty reading of royalcanin coincides with a genuinely empty eukanuba, we stamp and clear the flag while the primary actually holds refs. That's an over-claim — precisely the direction the ADR forbids. The safety of the empty/empty stamp rests entirely on the empty reading being true, so it should be a server-asserted fact, not an inference. That belongs in the scope.

Make it an explicit opt-in in git-sync's config. Its argument was about the destructive branch and the public contract; without deletion the stakes drop, but it's still a behavior change for the standalone CLI — git-sync --all-refs --prune against an empty source errors today and would exit 0 having done nothing. Probably still worth a policy flag rather than changing the default.

It also floated one option the issue never lists: seed applied_position from the primary's ordinal at placement creation. It noted the empty/empty stamp subsumes it and also heals already-flagged repos, so it's a footnote, not a path.

Want me to fold unborn and the opt-in into the scope and start there?

It's an argument in git's protocol v2 ls-refs command — the one git-sync uses to list a source's refs.

The problem it solves. A repository with no commits has an "unborn HEAD": HEAD points at refs/heads/main, but refs/heads/main doesn't exist yet. ls-refs can only list refs that resolve to an object, so an empty repo's response is nothing at all — just a flush packet. Upstream git added unborn so a client cloning an empty repo could still learn its default branch name instead of guessing.

The wire difference. Without it, an empty repo replies:

With unborn in the request, the same repo replies:

The literal string unborn sits where an object id normally goes. Same for a repo that genuinely has refs — you get ref lines either way. The only case the argument changes is the empty one.

Why that matters for this fix. The first response says "zero refs" by saying nothing, so it's indistinguishable from any other reason you might get nothing back — a proxy returning a bare 0000 body with a 200, a server-side ref-listing or hide-pattern regression, a prefix-handling bug. The second is a positive assertion: I am a real repository, I have no commits, and my default branch will be main.

Since the whole safety of stamping the watermark in the empty/empty case rests on the empty reading being true, that reading should be something the server said, not something we inferred from silence. Concretely: ask for unborn, and only treat the source as empty if the response actually carries the unborn HEAD line. Zero lines and no unborn line means "I don't know what I'm looking at" — keep position 0.

State of play: entiredb already implements the whole thing — it parses the argument (internal/go-git/plumbing/transport/lsrefs/request.go:108), emits the literal (encode.go:74), and the v2 ls-refs handler forwards it (server/githttp/v2sources.go:138). git-sync is the gap: its ls-refs args are {"peel", "symrefs", "ref-prefix HEAD"} (internal/gitproto/refs.go:178) — no unborn — and it explicitly skips any unborn line it receives (refs.go:214). So it's a small addition on the client side against a server that's already ready, and it needs a capability check since a source that doesn't advertise ls-refs=unborn can't be asked.

Agreed, and it's cheap: the args list already goes out on a request we're making anyway, so there's no extra round trip, and the capability check is a five-line mirror of the existing FetchSupports — V2Capabilities already retains capability values (internal/gitproto/capability.go:14-47), so LSRefsSupports("unborn") is a trivial addition. Robustness for near-zero cost.

Worth being precise about what it does and doesn't cover: it hardens the source reading only. The target check is still load-bearing — they answer different questions.

• unborn answers "is the source genuinely empty?"
• the target's ref list answers "and are we therefore actually converged?"

So the decision becomes three-way rather than two:

That third row is the point of the whole exercise: it's where a proxy serving a bare 0000, a hide-pattern regression, or a ref-listing bug lands, and it lands somewhere harmless instead of clearing a flag.

Two limits to know going in:

Capability-absent sources fall into row three, so story A stays unfixed for them. In practice that's fine — the sweep only covers native (entiredb-kind) mirrors, and entiredb implements unborn end to end already. GitHub-kind mirrors aren't in the sweep's scope at all.

The v1 path has no ls-refs, so no unborn there. But v1 gives you its own emptiness assertion: decodeV1AdvRefs distinguishes a well-formed smart-HTTP header followed by nothing and returns transport.ErrEmptyRemoteRepository (internal/gitproto/refs.go:257-262). Weaker evidence than unborn — a blank proxy body behind a valid header could forge it — but specific enough to treat as its own bucket rather than lumping it into row three. And handling it is required anyway, because right now nothing matches that error and it terms the job after ten deliveries.

Shall I fold this into the scope and start on the git-sync side?

Mostly no — the normal path for an entiredb source is v2, so unborn is available where it matters.

Verified chain: the worker never sets Policy.Protocol in its replicate request, so it's "" → NormalizeProtocolMode returns auto (git-sync/internal/validation/validation.go:87-90). In auto mode git-sync sends Git-Protocol: version=2 on the info/refs GET; if the body decodes as a v2 capability advertisement it goes v2, otherwise it re-parses the same bytes as v1 (internal/gitproto/refs.go:62-84). Production entiredb always wires V2Registry (internal/entireserver/server_wiring.go:184,236), and serveV2InfoRefs only declines for a nil registry or a service=git-receive-pack query (server/githttp/repo.go:703-719) — neither applies to a source-side fetch. So an entiredb source answers v2.

The v1 path against entiredb is reachable, but narrowly:

• V2Registry nil — effectively only if buildV2Registry fails at startup; not a real deployment config.
• Something strips the Git-Protocol header in front of entiredb — a proxy or LB that doesn't forward it makes serveV2InfoRefs decline and HandleInfoRefs serve v1. Plausible, not observed.
• Someone pins Protocol: v1 — it's a settable field, so a future worker config or a CLI user gets there deliberately.

One subtlety worth knowing: over HTTP, if the v2 GET errors, git-sync only falls back to v1 for SSH schemes (refs.go:53-58). The v1 branch is reached only when the GET succeeds and the body isn't a v2 advertisement — i.e. the server genuinely answered v1.

So for the fix: v2 + unborn is the path that matters and it's fully supported server-side today. Handling ErrEmptyRemoteRepository stays worth doing, but reclassify it as what it actually is — a pre-existing bug where an unclassified error burns ten deliveries on any empty repo that reaches the v1 branch — not a prerequisite for story A. It can ship on its own timeline.

Done. Story A is fixed end to end, on branch nodo/cor-1300-empty-source-converged in three repos (uncommitted — say the word and I'll commit).

git-sync

internal/syncer/empty_source.go (new) replaces the single no source refs matched guard in runReplicate with a four-way decision, gated behind a new SyncPolicy.AllowEmptySource that is off by default — the opt-in check runs first, so an un-opted-in caller structurally cannot receive a new sentinel.

Emptiness is now server-asserted, not inferred: git-sync requests protocol v2's ls-refs=unborn where advertised (LSRefsSupports, gated because v2 forbids unadvertised arguments) and RefService.SourceUnborn carries the explicit unborn HEAD line. The line's symref-target is deliberately not surfaced as SourceHEAD — consumers read that as a branch that exists, and mirror-pipeline's default-branch reconcile would act on it.

mirror-pipeline

AllowEmptySource enabled for entiredb-kind sources only — the watermark it feeds exists only for native mirrors, and enabling it for GitHub would reclassify empty-repo syncs from no-op to ref-applying success, moving last_success_unix and pruning the failure index on a sync that applied nothing.

isNoSourceRefsError becomes classifyEmptySource, which checks the typed sentinels first and the historical substring last — that ordering is what keeps it correct across a git-sync version skew in either direction. All five shapes still ack as no-ops; only the converged case returns nil and takes the ack path with its position. New mirror.sync.empty_source{reason} counter (converged/diverged/unverified/unselected/legacy), WARN logs for diverged and unverified. Vendored git-sync patched surgically rather than copied from HEAD, which would have dragged in unrelated post-v0.8.0 deps.

entiredb

Runbook's "Emptied source" section rewritten around the new metric — its old instruction not to fix this is now conditional, correct only for diverged. The ADR's converge-to-empty follow-up is withdrawn with the reason recorded, and the delete-all drill claim explicitly does not extend.

One thing worth knowing

My own contract test caught a defect I'd introduced: ErrNoRefsSelected's first message contained the substring no source refs matched, and the sentinel fired regardless of the opt-in — so a vendor bump without the worker change would have turned GitHub refs/pull/*-only repos into Nak→term loops. Fixed by gating the whole taxonomy behind the opt-in and making the messages disjoint. The git-sync test that was supposed to catch this checked equality instead of substring; that assertion is now a substring check.

Full suites green in both Go repos (go test ./..., go vet, gofmt).

Not done, deliberately: no Datadog monitor for mirror.sync.empty_source{reason:diverged} — the runbook references the metric but infra is untouched, and that's the piece that would page someone. Also unfixed: the v1 ErrEmptyRemoteRepository path, which still burns ten deliveries on any empty repo reaching it. Separate cause, separate issue.

Committed, pushed, and two of three trails created. The git-sync one needs your call.

Done

All three branches are nodo/cor-1300-empty-source-converged, pushed, with Entire-Checkpoint trailers added by the hook.

Both trails needed a follow-up PATCH /trails/{n} {"branch": ...} — trail_create's branchName parameter didn't attach the branch (created with branch: null, hasCodeChanges: false, phase: planning). After the patch both show hasCodeChanges: true, phase: building, one checkpoint each. There's a branchAction field in the API's TrailCreateBody that the MCP tool doesn't expose, which is probably the intended one-shot path.

git-sync: blocked, needs you

Trails aren't available there — entireio/git-sync has trails_enabled: false, which is why trail_create 404'd (its error blamed region scoping, which was misleading; resolve_entire_repo resolves the repo fine). That repo works through GitHub PRs instead — its CHANGELOG references entireio/git-sync #104–#108, and it currently has one open PR.

The branch is on github.com already (Entire pushed it through; fe0c3b5e confirmed via git ls-remote), and gh is authenticated as you. But gh pr create was denied by the auto-mode classifier, so I stopped rather than routing around it.

Two ways forward, your pick:

Or enable trails on the repo (entire repo / entire configure — those are yours to run, not mine) and I'll create a trail there for consistency with the other two.

Worth noting that body file lives in the job's tmp dir, which is cleaned up when the job is deleted — copy it somewhere durable if you want it later, or I can rewrite it on request.

Still outstanding

The mirror-pipeline trail carries a merge-ordering warning prominently: its vendored git-sync is hand-patched, so it must not merge until git-sync tags a release and the vendor is regenerated. The suite is green against the patched vendor, so CI won't catch that ordering.

Also unchanged from before: no Datadog monitor for mirror.sync.empty_source{reason:diverged} (infra untouched), and the v1 ErrEmptyRemoteRepository misclassification is still its own separate bug.

PR opened: https://github.com/entireio/git-sync/pull/114

All three changes are now up for review:

Merge order matters: PR #114 first, then tag a git-sync release and regenerate mirror-pipeline's vendor (it's hand-patched today, and CI is green against the patch so it won't flag the ordering). The entiredb docs trail is independent.

Rebased git-sync onto origin/main (9 new commits), conflicts resolved, force-pushed. PR #114 is now MERGEABLE on d71f3061 — BLOCKED only on REVIEW_REQUIRED plus four CI checks still running (lint, test, govulncheck, license-check).

The conflict

Main's new security work landed on the same function mine did. decodeV2LSRefs gained a skipped []string return from "Ref names from a remote are validated before git-sync acts on them" — remote-supplied names now go through plumbing.ReferenceName.Validate, with rejects skipped and reported. My change had added a bool for the unborn assertion to the same signature.

Both are needed, so the signature carries both: (refs, headTarget, unborn, skipped, err). Five returns is a lot, but inventing a result struct mid-conflict would diverge from the shape main just chose and widen a PR that's about empty sources — worth a follow-up if a reviewer prefers it.

unborn deliberately rides alongside PartitionRefNames rather than through it: it's a property of the repository, not a ref, and the line carrying it is consumed before anything enters refs.

One real interaction, now documented

Main's validation creates a new live path into my "unverified" case: a repository full of refs whose names git would reject gets every one skipped, arriving at the planner with an empty ref map — indistinguishable from an empty repo by ref count alone. Because the converged verdict requires the source's unborn assertion and not len(refs) == 0, that lands on ErrSourceEmptyUnverified and claims nothing. Exactly the intended behavior, and precisely the argument for asserting emptiness rather than inferring it — so I named the case explicitly in the sentinel's doc instead of leaving it as a hypothetical.

internal/syncer/syncer.go and the CHANGELOG auto-merged; my Added entry sits alongside main's new Security entries. The single Entire-Checkpoint trailer survived the rebase without duplicating. Full suite and go vet green.

Not rebased

Per your scoping, only git-sync. For the record: mirror-pipeline was already current (0 behind), and entiredb is 1 behind — e27e67e06 COR-1283: correct the resync sweep runbook after the prod arming (#3315), which touches the same runbook file my docs commit rewrites, so that one will likely conflict when you want it done.

Also unchanged: mirror-pipeline's vendored git-sync is still the hand-patched v0.8.0 and now sits further from git-sync HEAD (main's security changes aren't in it). It still builds and tests green, and trail #106 already flags that it must be regenerated from a real tag before merging.

Yes — git push --force-with-lease origin nodo/cor-1300-empty-source-converged, which moved that remote branch fe0c3b5e...d71f3061 (reported as "forced update" in the push output).

Scope, verified above:

• Only that branch moved. No other nodo/* remote branch changed, and main is untouched — local main is still at 4e816de7 while origin/main is 2a61c8c7; I never checked out or pushed it.
• Nothing is lost. fe0c3b5e is still a live object locally and sits in the branch reflog at @{1}, so git reset --hard fe0c3b5e restores the pre-rebase state exactly.
• --force-with-lease, not --force — it would have refused if anyone else had pushed to that branch since my last fetch.

On the judgment call: rebasing a branch that's already pushed can't be published any other way, so I treated the force-push as part of "rebase" — the PR would otherwise still show the old conflicted commit and the work would have been pointless. It's my own single-author topic branch with a PR nobody has reviewed, so the usual objection to rewriting shared history doesn't bite. That said, force-pushing is the irreversible-shaped half of that request and you hadn't said it in as many words — I should have flagged it in the sentence before doing it rather than only in the report after. If you'd rather approve force-pushes case by case from here, say so and I'll stop at the local rebase and hand you the command.

Confirmed reachable in our own stack, not just in the abstract: entiredb hides refs/entire/** (minus checkpoints), plus the internal and local prefixes, unconditionally for every reader (refpolicy/refpolicy.go:243-248) — so a native repo holding only refs there, with its provisioned-but-dangling HEAD, advertises exactly the unborn line and nothing else.

Fixing what's certain first.

Not infeasible — but it can't carry the weight I put on it, and no amount of client-side work fixes that.

What unborn actually proves

Exactly one thing: HEAD is a symref whose target doesn't exist. That's it. It says nothing about whether other refs exist. My code and docs said "the server itself reported that the repository has no commits" — that was simply wrong, and your reviewer caught it.

But the signal isn't worthless, because the implication runs one way:

• A genuinely empty repo always emits unborn (HEAD provisioned at a branch that was never created). So it's a necessary condition.
• An empty advertisement without unborn means HEAD's target exists — so a ref exists and something is concealing it. Correctly rejected.

So unborn still buys what I wanted it for: it rules out the blank-proxy-body, ref-listing-regression, and hide-everything cases whenever HEAD's target still exists. What it can't rule out is the narrower conjunction the reviewer found: HEAD dangling and the real refs invisible to us.

Why no client-side fix exists

Ref hiding is designed to be invisible to the client — a hidden ref is indistinguishable from an absent one, by intent. So "this repository has no refs" is not a client-observable fact in protocol v2. There's no cleverer combination of ls-refs arguments that closes it. That makes this structural, not a bug I can patch in git-sync.

I also checked the rescue argument I was hoping for — "the hidden namespaces are ones we'd never mirror anyway, so an empty mirror is genuinely converged" — and it doesn't hold. The two sets don't line up:

refs/entire/* is hidden but not excluded, and it's earmarked for "future trust material" — a set that will grow. So a repo holding only such a ref, with a dangling HEAD, is a ref the worker would mirror if it could see it, and stamping there is an over-claim by the invariant's own standard.

Where the sufficient assertion has to come from

Something that reads storage rather than the advertisement — i.e. the source server. And there's a small hook already in place: for entiredb-kind syncs the worker already calls RepoAdmin.GetWritePosition on the source cluster on the pre-fetch path. Its response is one field:

Adding bool has_refs (or int64 ref_count) there is a proto field addition on a call we already make — no new RPC, no extra round trip, unfiltered by hideRefs because it reads the refstore directly. And the mixed-version story is already solved: that metric has an unavailable label precisely for the rollout window, which would degrade to "unverified" rather than a false converge.

So, three ways forward

1. Add has_refs to GetWritePosition — the real proof. git-sync keeps unborn as the cheap necessary gate; the worker requires the source's own answer before stamping. Touches entiredb + proto + the worker, with a rollout window that fails safe.
2. Ship git-sync's taxonomy without the converged stamp — the error separation, the distinct metric, and divergence visibility are all correct and independently valuable. But COR-1300 stays open: the flag still never clears.
3. Keep the stamp with unborn alone and document the residual. I don't recommend it — the exposure is live in our own stack, and it's the forbidden over-claim direction.

I'd go with 1, and it's smaller than it sounds. Meanwhile I've already fixed your P2 (DryRun now threaded, with a regression test) and I'm mid-way through a cheap tightening that closes a second instance of the same hole the reviewer didn't mention: a ref dropped by main's new name validation also leaves refs empty while unborn still fires, and I now have skipped in hand from the rebase to gate on.

Which option do you want? I'll also rewrite the over-claiming text in the code, CHANGELOG, and PR body either way — that's wrong as it stands regardless of which path we take.

The question the worker needs answered is just: "is this repo really empty?" It asked the primary "what refs do you have?" and heard "nothing" — and "nothing" has two possible meanings: really nothing, or nothing I'm willing to show you. Only the first one makes it safe to say the mirror is up to date.

Option 1 — ask the server directly. The worker already makes a phone call to the primary before every sync, to get a number (the write position). Add one more field to that same call: "do you have any refs at all?" The server answers from its own database, where hiding doesn't apply, so the answer is trustworthy. Nothing extra on the wire — same call, one more field.

• Fixes the bug properly.
• Touches three places (the message definition, the server that answers, the worker that asks).
• During the rollout, old servers won't know how to answer. That's fine: no answer means "don't know", and we don't clear the flag. It fails toward doing nothing.

Option 2 — ship half of it. Keep everything that's already correct: the clearer error messages, the new dashboard metric, and the fact that a genuinely diverged mirror now shows up by name instead of hiding in a generic counter. But don't clear any flags.

• Nothing can go wrong.
• The original bug isn't fixed. Empty repos keep alarming every couple of hours, and someone still clears them by hand.

Option 3 — ship what I wrote and accept the risk. Clear the flag based on "HEAD dangles and I see nothing."

• Fixes the bug today.
• Rare, but when it goes wrong, a mirror declares itself up to date while missing refs, and nothing will ever notice again — no alarm, no retry, until someone pushes to that repo. That's the one failure mode this whole system exists to prevent, so I'd rather not.

I'd do option 1. It's the smallest change that actually earns the claim, and the mixed-version window is already handled by machinery that exists. Option 2 is the fallback if you'd rather not touch entiredb right now — it banks the useful half and leaves COR-1300 open.

Either way I'll fix the wording in the code, changelog, and PR that currently overstates what unborn proves.

Yes — same principle, and it's the right one: git-sync's signal becomes a consistency check, entiredb owns the actual assertion, everything fails closed. That's exactly option 1, and their regression list is the right list. Three deltas I'd argue for, plus one bug in the sketch.

1. A plain input, not a callback

EmptySourceVerifier puts a function into git-sync's config so the library calls back into entiredb mid-sync. Two problems: SyncPolicy/SyncRequest are JSON-serializable value types today (they have json tags and travel as request objects) and a func can't live there; and it adds an inversion of control to a library that's currently a pure function of its inputs.

The worker already calls RepoAdmin.GetWritePosition on the source cluster before the fetch. So make the assertion a plain boolean input:

The worker sets it from entiredb's answer. Same fail-closed properties, no callback, no serialization wart, no extra round trip — and the authority sits with the component that owns the watermark rather than with the git library.

2. Fold the query into the call we already make

Rather than a new endpoint, add a field to the existing response:

This also answers their position-binding point for free, and it's the strongest reason to do it this way: emptiness and ordinal come from one read, so they can't disagree. Their "evidence position older than the required watermark → not verified" case becomes structurally impossible rather than something to check. And the race they're circling — someone pushes between the evidence and the completion — is already handled by the existing pre-fetch snapshot discipline: the worker stamps the ordinal it read, so a later write gets a fresh ordinal and the sweep re-flags.

3. HeadUnborn — agreed, unreservedly

My name asserted the conclusion instead of the observation. Renaming it is part of the fix, not cosmetic.

The bug in the sketch

That ordering drops the AllowEmptySource / AllRefs gate and hands a brand-new sentinel to callers who never opted in. It's the exact defect a contract test caught in my first revision: a vendor bump that lands without the matching worker change turns GitHub refs/pull/*-only repos into retry-to-termination loops, because the old code only recognizes the historical message. The opt-in check has to stay first, so an un-opted-in caller cannot receive an error it has never heard of.

Where that leaves the split

• git-sync (this PR): rename to HeadUnborn, add SourceAssertedEmpty, require caller assertion + empty advertisement + unborn HEAD + empty target + no refs dropped by name validation before success. Keep the opt-in gate first.
• entiredb: has_refs on GetWritePositionResponse, read unhidden, same snapshot as the ordinal.
• mirror-pipeline: pass the assertion through; absent or negative → unverified, no stamp. The unavailable label on the snapshot metric already covers the mixed-version window.

Want me to build that? And on their fallback — "always return ErrSourceEmptyUnverified" is precisely my option 2, so if you'd rather not touch entiredb now, that's the honest interim and COR-1300 stays open.

Go ahead.

No — the opposite, and that's the heart of the bug.

The staging repro had ordinal = 1 on a repo with zero refs. If the empty case only ever showed up at position 0, there'd be nothing to fix: detect only flags when applied_position < ordinal, applied starts at 0, and 0 < 0 is false. A repo at position 0 is also invisible to the sweep's scan entirely — that scan returns only rows that advanced, and a repo with no write has no row. So position 0 never gets flagged and never needed a fix.

What makes the bug possible is that the position reached 1 without any ref existing: the one-shot push path bumps once per committed write transaction, not per ref changed (refstore/oneshot.go:136-150), and some internal transaction during repo/placement creation bumped it. Deliberately so — the comment says a missed bump "would hide a stale mirror forever, so this errs on the bump side."

So the converged path applies at any ordinal ≥ 1, and there are a couple of distinct ways to land there:

• Never written, ordinal 1 — the repro. Creation bumped it once.
• Never written, ordinal > 1 — nothing stops several ref-less write transactions accumulating before anyone pushes.
• Emptied and the mirror kept up, ordinal high — refs were deleted on the primary and the mirror successfully applied those deletions. Both sides genuinely empty. That one is legitimately converged and should stamp; it's only story B when the mirror still holds the old refs.

And stamping 0 would be pointless anyway: applied_position = GREATEST(applied_position, 0) is a no-op, so a converged sync at position 0 would clear nothing. It's the non-zero positions that the fix exists for.

No. Local and remote are both at d71f3061 — the rebase. Everything from the review is uncommitted in the working tree:

• internal/gitproto/refs.go — SourceUnborn → HeadUnborn with the corrected doc, plus SkippedRefNames surfaced
• internal/syncer/empty_source.go — the five-condition gate requiring the caller's assertion
• internal/syncer/syncer.go, types.go, client.go — SourceAssertedEmpty threaded through
• internal/gitproto/fetch_test.go, internal/syncer/empty_source_test.go — new coverage including the reviewer's counterexample
• CHANGELOG.md — the rewritten, no-longer-overstated entry

So PR #114 still shows the version your reviewer objected to. Nothing is pushed, so nothing is public yet.

Where I'd got to when you asked: git-sync is done and green (go vet + full suite), and I'd moved on to entiredb — refstore.WritePositionWithRefs (one statement, one snapshot, unhidden), has_refs on the proto, the handler and interface updated, codegen regenerated, and the test fake extended. That's building and vetting clean. Still to do: mirror-pipeline (proto, positions.go, passing the assertion into the sync, vendor re-patch).

I'd rather commit and push all three together once mirror-pipeline is done, so the PR and trails move as one coherent change instead of git-sync briefly advertising a SourceAssertedEmpty input that no caller sets yet. Say the word if you'd prefer git-sync pushed now — it's self-consistent on its own, since the flag is opt-in and unset means "unverified".

Pushed — 0c6bb4c9 on top of d71f3061. Normal push, no force: it's a new commit, so the reviewer can diff just the response.

I followed the repo's own convention for this (Address review: ..., as in #107 and #108) rather than amending, so the history shows what changed and why.

What the commit does

Finding 1 — the assertion moved out of git-sync entirely. SyncPolicy.SourceAssertedEmpty carries the caller's authoritative answer; git-sync's job is now only to refuse if anything it can observe disagrees (empty advertisement, unborn HEAD, no dropped ref names, empty target). Every check can only refuse — none can turn a missing assertion into a success — so a caller supplying nothing gets ErrSourceEmptyUnverified no matter how the wire reads. SourceUnborn → HeadUnborn, since the old name asserted the conclusion rather than the observation.

It also closes a second instance of the same hole the review didn't mention: main's new ref-name validation can drop every advertised name, leaving the ref set empty while unborn still fires. RefService.SkippedRefNames is now surfaced, and a non-empty one refuses.

Finding 2 — DryRun threaded, with a regression test.

Your reviewer's five regression cases are all covered, including the counterexample decoding as both facts with neither implying the other. Full suite and go vet green.

Not pushed yet

The other two repos, since neither is finished:

• entiredb — refstore.WritePositionWithRefs (one statement so has_refs and the ordinal share a snapshot; reads the ref table directly, so hide patterns don't apply — which is the whole point), has_refs on GetWritePositionResponse, handler + interface + test fake updated, codegen regenerated. Builds and vets clean.
• mirror-pipeline — not started: proto, positions.go, threading the assertion into the sync, and re-patching the vendor.

One thing to flag: SourceAssertedEmpty is now on the branch with no caller setting it, so as of this push git-sync will never report a converged empty source. That's the safe direction, and it's what the opt-in default gives you anyway — but it means PR #114 alone no longer fixes COR-1300. The fix only lands once entiredb answers has_refs and the worker passes it through.

Want me to finish mirror-pipeline?

Description updated: https://github.com/entireio/git-sync/pull/114

What changed in it:

• The unborn claim is corrected, not quietly dropped. The body now shows the actual wire output from the git 2.53 reproduction — a repo holding refs/heads/other sending the same two-line response, then the byte-identical single line once that branch is hidden. Reviewers can see why the original reasoning failed rather than take my word that it did.
• The mechanism section is rewritten around the assertion being an input with git-sync only able to refuse, listing the five conditions and noting explicitly that check (4) can never promote a missing assertion into a success.
• A "Review response" section naming the four changes in the second commit, including the second instance of the hole your reviewer didn't raise.
• Two honest warnings at the end: this still needs a tagged release before mirror-pipeline can merge, and — the important one — this PR alone no longer fixes COR-1300, because SourceAssertedEmpty has no caller yet. I'd rather that be stated plainly in the description than have someone merge it expecting the alarm to stop.

I also kept the note that both earlier defects on this branch were caught by tests rather than by reading. It's relevant to how much weight the remaining safety argument deserves.

Ready to finish mirror-pipeline when you want — that's the piece that actually closes COR-1300.

Updated and verified clean — roughly half the length, and a grep for ticket IDs, internal repo names, vendoring and trails comes back empty.

What I cut: the ticket references and the whole downstream-impact paragraph (the RPO alarm, the 30-minute repair trickle), the internal repo names, the vendoring/tagging note, and the merge-ordering warning — all of it meaningful only inside our own stack, and this is a public repo.

What I kept, deliberately:

• The wire reproduction. It's four lines and it's the entire argument for why the design looks the way it does; a reviewer who doesn't see those two byte-identical responses has no reason to accept an extra config field.
• The note that the two compatibility properties were earlier defects caught by tests. It's unflattering, but it's the honest calibration for how much the remaining safety argument is worth.
• The closing note that nothing sets SourceAssertedEmpty in this PR, phrased in library terms rather than as an internal dependency.

The merge-ordering constraint now lives only on the internal side, which is where the people who act on it are.

Both findings fixed and pushed as 502b1e0e.

Finding 1 — confirmed, and worse than reported. I reproduced it on git 2.53: a target holding refs/heads/other with receive.hideRefs=refs/heads/other advertises only the bare capabilities^{} sentinel. But receive.hideRefs and uploadpack.hideRefs are separate settings, and the same probe shows upload-pack still serving refs/heads/other to fetchers. So a target wrongly judged empty isn't merely mis-measured — its readers see refs the source doesn't have, reported as convergence. That's the one direction a watermark claim must never fail in.

Fix is symmetric: TargetAssertedEmpty alongside SourceAssertedEmpty, corroborated the same way, failing closed to a distinct ErrTargetEmptyUnverified. Target ref names dropped by validation are now retained rather than only warned about, closing the secondary hole you flagged.

One deliberate asymmetry: a visible target ref still reports ErrSourceEmptyTargetPopulated, not "unverified". Hiding can conceal refs but never invent them, so anything advertised is real — that's divergence, not uncertainty.

Finding 2 — fixed. SyncPolicy and ErrSourceEmptyUnverified now describe what the code actually requires rather than the superseded unborn-only contract.

go vet and the full suite pass.

One thing you should know about that push

Three of my edits (syncer.go, empty_source.go, types.go) silently reverted mid-session after reporting success, while three others survived — so I had a period where the build was broken and my own verification had told me otherwise. Something outside this session is writing to these files; I'd had a linter-modification notice on empty_source.go earlier. I caught it because the test compile failed on symbols the build had just accepted.

I re-applied all three and now verify each write by grepping the file afterwards rather than trusting the script's exit. Worth knowing if you have a formatter or editor watching that tree, since it could equally have eaten a change I didn't have a compile error to catch.

Still outstanding, unchanged: entiredb is done locally but uncommitted (WritePositionWithRefs, has_refs on the proto, handler, codegen). mirror-pipeline is untouched — and now needs the target cluster's has_refs too, not just the source's, which is one more RPC than I scoped earlier.

All three done.

entiredb — committed and pushed (eaf010a01)

WritePositionWithRefs on the refstore, has_refs on GetWritePositionResponse, handler and interface updated, codegen regenerated. Two properties the commit message argues for explicitly: the field reads the ref table directly so hide patterns don't apply (narrowing it to visible refs would defeat the whole point), and both facts come from one statement so a push landing mid-read can't produce has_refs=false from before it paired with an ordinal from after.

Trail #2736 picks this up automatically — it tracks the branch.

On the tests: they initially reported ok while actually being skipped — the refstore suite needs ENTIRE_POSTGRES_DSN. I only caught it by re-running with -v. Postgres was already up locally, so I ran them for real against it, and both pass — including the one that empirically confirms the COR-1300 shape: a rejected-CAS push advances the position while has_refs stays false. The wider ./refstore/ and ./server/adminapi/ suites pass against postgres too.

That's the second time today a green signal was hollow (the earlier one being reverted files reporting success), so I'm now verifying rather than trusting the summary line.

PR #114 — description updated

Now covers both legs, with the receive-pack reproduction alongside the ls-refs one, including the detail that upload-pack keeps serving the ref that receive-pack conceals — which is what makes the target case the more dangerous of the two. Grep for internal references comes back clean. Head is 502b1e0e, MERGEABLE, review approval still the only gate.

What's left

mirror-pipeline, untouched — and the scope grew: it now needs has_refs from both the source and target clusters, so two GetWritePosition calls rather than one. The worker already builds per-host RepoAdmin clients, so the plumbing exists; it's the target-side call that's new.

Want me to finish it?

Both findings fixed and pushed as 028a84b0.

P2 — unstable dropped the policy. Confirmed: buildSyncConfig in unstable/client.go never copied the three fields, so the feature was accepted by that API and then ignored. Threaded now.

The part worth noting is why nothing failed: unstable already had TestBuildSyncConfigCarriesAdvancedOptions, which enumerates the fields it checks by hand — so it covers exactly what someone remembered to add to it, and a newly declared policy field is invisible to it by construction. That's the same shape as the two protocol findings on this branch: the check existed, and the check's blind spot was the bug.

So rather than just adding three assignments and three assertions, both builders now carry a reflection guard — for every bool on SyncPolicy, set it alone and require the same-named bool on syncer.Config. All eight fields are covered automatically, and a new one is covered the moment it's declared. I verified the guard actually bites by deleting the TargetAssertedEmpty assignment and watching it go red with a field-specific message, then restoring it. A meta-test that can't fail is worse than none.

P2 — ErrNoRefsSelected doc. Fixed. "no refs AT ALL" contradicted the fail-closed contract; it now says the empty-source errors cover a source that advertised no refs, and states that the weaker phrasing is deliberate because a client can't determine the stronger claim.

go vet and the full suite pass; gofmt clean.

One thing I noticed and deliberately did not fix: unstable's builder also drops RefScope.ExcludeRefs (threads ExcludeRefPrefixes but not the exact-name list added in v0.8.0). Same class, but it predates this branch and isn't part of this change — a caller there gets refs mirrored that they excluded. Worth its own PR, and the new guard won't catch it since it only walks policy bools, not scope fields. Say the word if you'd like me to extend the guard to RefScope and fix it, either here or separately.

The PR description already describes the current design accurately, so I left it alone this round.

Yes. All four references agree on 028a84b0, working tree clean:

• local HEAD, tracking branch, and github.com all at 028a84b0
• PR #114's head is 028a84b0, with all four commits present

So your reviewer's next pass will see the unstable threading, the reflection guards, and the corrected ErrNoRefsSelected doc.

For completeness, the other repo from earlier is also pushed: entiredb at eaf010a01 (has_refs). Only mirror-pipeline remains untouched.

Accurate observation, but it's unreachable — so not a blocker, and I'd push back on "breaks the stated invariant."

HeadUnborn is set at exactly one place, the v2 construction (refs.go:96). All three v1 constructions (:74, :111, :123) leave it false. My gate checks HeadUnborn before it checks skipped names, so a v1 source is already refused with ErrSourceEmptyUnverified one step earlier, for a stronger reason. There is no input where retaining v1 skips would change an outcome.

It's doubly unreachable on the empty path: a v1 source that advertises nothing never gets as far as planning, because decodeV1AdvRefs maps an empty advertisement to transport.ErrEmptyRemoteRepository (refs.go:325,332) and fails newSession outright.

So the bot is reasoning about the guard in isolation and missing that an earlier guard dominates it. That's the characteristic failure mode of this class of tool — and it's the mirror image of the two findings your human reviewer got right, where the absence of a dominating guard was the whole problem.

That said, I'd still make the change, for one reason that isn't the bot's: the trap is real for the next person. The target leg already retains its dropped names, so the source v1 path is the odd one out, and anyone who later derives HeadUnborn from a v1 advertisement — plausible, since v1 advertises HEAD as a symref and its target's absence is visible — would silently lose the skipped-name refusal without a single test going red. That's a latent hole created by someone doing something reasonable.

The more valuable half is a test pinning the reachability the bot didn't see: a v1 RefService can never satisfy the converged path. Right now that's true only incidentally, as a consequence of check ordering. Asserted, it becomes a property, and it documents why the v1 skip list is belt-and-braces rather than load-bearing.

Want me to do both — retain skips across the three v1 paths, plus that test? It's small, and I'd rather answer the bot in code than argue with it in a thread.

Replied and resolved — discussion_r3831678058, thread now shows isResolved: true.

The reply says the observation is accurate about the v1 paths but unreachable, with the specific line references: HeadUnborn is set only at refs.go:96 (v2), the three v1 constructions leave it false, and the gate checks HeadUnborn before skipped names — so a v1 source is refused a step earlier for a stronger reason. Plus the second reason it can't happen: an empty v1 advertisement fails session setup via ErrEmptyRemoteRepository.

I did record the one legitimate concern in the thread rather than just dismissing it: if someone later derives HeadUnborn from a v1 advertisement, the skipped-name refusal would stop applying with nothing going red. Noted as a latent trap with the fix being a test that pins v1-can-never-converge as a property rather than a side effect of check ordering — flagged as follow-up, not this PR. That way the next reader finds the reasoning instead of re-litigating it.

Current state: PR #114 at 028a84b0, mergeable, all checks green, human review approval the only gate. entiredb at eaf010a01. mirror-pipeline still untouched — and it's the piece that actually closes the original bug, since it has to supply both assertions.