Skip to content

The notes refspec doctor --fix writes is forced, so an ordinary git fetch silently destroys unpublished records #417

Description

@MongLong0214

The defect

src/core/notes.ts:

export const NOTES_REFSPEC = '+refs/notes/*:refs/notes/*';

The leading + forces the update. A clone that ran doctor --fix therefore overwrites its local refs/notes/commitlore with the remote's on every git fetch, whether or not the two have diverged.

A note is not a cache. The note is the record. Overwriting one destroys a record rather than invalidating a derived file, and nothing reports that it happened.

Reproduction

Two clones of one remote, both with the refspec doctor --fix writes. Bob publishes a record; Alice writes one of her own on a different commit and has not pushed yet. Alice then runs nothing but git fetch:

alice note before fetch: Warn: alice unpublished
alice note after  fetch: GONE
bob's note now visible : Warn: from bob

Alice's record is gone from the ref. No prompt, no warning, exit 0.

This needs no unusual sequence — write a record, pull before pushing. backfill, squash inheritance, and any capture that lands in the mirror all produce exactly this state.

Why forced was chosen, and why it is still wrong

A forced refspec makes remote notes appear automatically, which is the behaviour the mirror wants. Unforced gets the same result in the common case: a fast-forward still applies without ceremony. The only time the two differ is when the local ref has commits the remote does not — which is precisely the case where forcing destroys something.

So the cost of unforcing is a visible ! [rejected] on a diverged fetch, and the benefit is that a record is never silently lost. That trade is not close.

Fix

  1. NOTES_REFSPEC drops the +.
  2. doctor detects an already-configured forced notes refspec and reports it; --fix rewrites it.
  3. A diverged fetch is what commitlore sync (The notes mirror is written locally and never leaves the machine, so a teammate's clone cannot see a record it holds #416) is for, and doctor should say so rather than leaving ! [rejected] unexplained.

Note for #416

git fetch <remote> <refspec> applies the configured refspecs in addition to the one on the command line — measured, not assumed:

 * [new ref]  refs/notes/commitlore -> refs/notes/commitlore-remote   (command line)
 * [new ref]  refs/notes/commitlore -> refs/notes/commitlore          (configured)

So any code fetching the mirror to a scratch ref must pass --refmap= to isolate itself, or the configured refspec will move the working ref underneath it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions