Say the mirror went unread, instead of counting the rebuild as whole - #401
Merged
Conversation
`index --rebuild` reads two sources -- the commit trailers and `refs/notes/commitlore` -- and in a clone that never fetched the mirror it can open only one. It printed `rebuilt: scanned 1 commit, indexed 3 trailers in 90ms` and exited 0. `context`, in the same repository, already said the answer may be missing records that exist upstream. So the query path was honest and the build path was not, and `rebuild` is the command someone runs *because* they suspect the index is wrong -- the least useful moment to omit that one of the two sources was unreadable. This is `r-fetchowed` one command over, and it takes the same shape: no new state, no widened check, no exit code moved. `notesAvailability` already computes the answer; the command now says it, on stderr, where the rebuild reason and every other diagnostic already go, so `--json` stays parseable and a caller piping it still hears this. The exit code stays 0 on purpose rather than by inertia. `context` exits 3 on exactly this state, but 3 there marks an *answer* drawn from an incomplete store; this command's contract is 0 built, 2 could not run, and it did build the index git can support here. Moving it would fail `init` and every CI step in every unfetched clone over a cache that is correct for what the repository holds. The caveat covers the incremental build and `--no-index` as well as `--rebuild`. They read the same two sources and were equally silent, and a user who sees the sentence on one and not the next would reasonably read the silence as the mirror having arrived. `docs/cli.md` said the index "can always reconstruct" itself, in the one place a reader is told the file is disposable. It now says what it reconstructs from -- whatever git holds here -- and that the command says so when that is one source of two. Record-Id: r-mirrorunread Limit: the caveat can only fire where notesAvailability returns unfetched, so a refspec added after cloning and never fetched through still builds silently -- the distinction config alone cannot carry, recorded on that function under r-fetchowed Ruled-out: Exiting 3 the way context does | 3 marks an incomplete answer rather than a failed build, this command's documented codes are 0 and 2, and the change would fail init and CI in every unfetched clone over an index that is correct for what git holds Ruled-out: Carrying the state through IndexStats and reporting it from core | the data was already right and only the report was wrong, so widening a core type to move one sentence costs every caller of the index a field it must then decide about Ruled-out: Adding the same sentence to init and backfill in this change | neither is silent for want of a check -- init collapses its steps to check marks unless --verbose, and backfill prints its own count -- so each is a decision about that command's output contract, which issue 400 does not make Certainty: firm Blast: local Undo: easy Verified: a plain clone of an origin holding one record in a message and one only in the mirror now prints the caveat on --rebuild, on the incremental build and on --no-index, and still exits 0; silent when the mirror is present, when there is no remote, and once the refspec is added and fetched; notes-availability 17, and index-db, index-perf, notes, doctor, query, query-path-not-in-history, cli, dogfood, init, init-output, init-verbose, backfill, shallow-history, stale, validate, guard, help-text-honesty and source-guards pass at 591 with 1 skipped; typecheck clean, dist rebuilt, bench:verify 632 rows valid, check-readme-numbers ok Unverified: whether init and backfill should carry the same sentence, which is a question about what those two commands surface and not about this defect
CommitLore — record lintTrailers: clean — 1 commit in Active constraints for the paths this PR touchesLimits (86)
Ruled out (202)
Warnings (47)
Truncated: 20 lines omitted — the comment hit GitHub's 65000 character limit. Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR. |
This was referenced Aug 3, 2026
Closed
Closed
This was referenced Aug 3, 2026
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
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 #400
The defect, reproduced
An origin holding one record in a commit message and one only in
refs/notes/commitlore, cloned plainly so the mirror was never fetched:No mention that one of its two record sources was unreadable, and the note's
record is simply absent from the index it just built.
context, in the sameclone, is honest about exactly this:
So the query path said it and the build path did not — and
rebuildis thecommand someone runs because they suspect the index is wrong.
The fix
The same shape as
r-fetchowed, one command over: no new state, no widenedcheck, no exit code moved.
notesAvailabilityalready computes the answer; thecommand now says it, on stderr, next to the rebuild reason, so
--jsonstaysparseable and a caller piping it still hears this.
It covers the incremental build and
--no-indexas well as--rebuild: theyread the same two sources and were equally silent, and a user who saw the
sentence on one and not the next would reasonably read the silence as the mirror
having arrived.
The exit code stays 0 on purpose.
contextexits 3 on this same state, but 3there marks an answer drawn from an incomplete store; this command's contract
is 0 built, 2 could not run, and it did build the index git can support here.
Moving it would fail
initand every CI step in every unfetched clone over acache that is correct for what the repository holds. Recorded as a
Ruled-out:on the commit.
Documentation
docs/cli.mdsaid the index "can always reconstruct" itself, in the one place areader is told the file is disposable. It now says what it reconstructs from —
whatever git holds here — and that the command says so when that is one source of
two. Two lines, matching the trim that file just had.
The neighbours, checked and reported
Everything else that writes the index, run in the same unfetched clone:
index(no--rebuild)index --no-indexcontext/limits/ …stalethis scan is incomplete)validate -c HEADreferences not checked (notes mirror not fetched))guardinitbackfillTwo are genuinely the same defect and are deliberately left alone, because
neither is silent for want of a check:
initrunsdoctor --fixlast, sor-fetchowed's sentence is produced —ok notes fetch refspec — refs/notes/commitlore is now covered for origin — nothing has been fetched through it yetwithfix: git fetch origin. But thatis visible only under
--verbose. The default output collapses all four stepsto
✓ Hooks / ✓ Index / ✓ Agent integration / ✓ Final check, so an operatorrunning plain
initin a fresh clone sees nothing. Surfacing it means changingwhat a code-0 step is allowed to print by default, which is that command's
output contract (and pinned by
test/init-output.test.ts's ≤6-line promise),not this one's.
backfillwrites the index viaupdateIndexand reportsindex: 3 commit trailers, 0 note trailers from 1 commit. "0 note trailers"reads as an absence when it is a gap — and worse, its
targets:count isselected from that same index, so a commit whose record exists upstream can be
offered up for reconstruction. Same shape, its own report to design.
Both are recorded as
Unverified:on the commit rather than fixed in passing.Verification
test/notes-availability.test.tsfailedagainst the unfixed build (
expected '' to contain 'has not been fetched'),then passed. Two more (
stays quiet …,stops saying it once fetched) passedboth before and after — they pin the absence of a false alarm.
npx vitest runover index-db, index-perf, notes, notes-availability, doctor,query, query-path-not-in-history, cli, dogfood, init, init-output,
init-verbose, backfill, shallow-history, stale, validate, guard,
help-text-honesty and source-guards: 19 files, 591 passed, 1 skipped.
npm run typecheckclean;npm run buildrebuiltdist/(committed);npm run bench:verify— 632 rows in 9 files valid;node scripts/check-readme-numbers.mjs— ok.commitlore validate -c HEAD—shape ok · references ok.No test was weakened.
README.mdand the translations are untouched.