v1.2.0 push line claims unsent records when no notes ref exists (regression from #656)

[MongLong0214]

Follow-up to #632, on the wording that #656 (aafd5ab) introduced. Field use of v1.2.0 on MongLong0214/logic-pro-mcp.

#632 asked one question directly: "Is there a local note waiting?" The new line answers it unconditionally with yes, without checking. When the answer is no, the line reports data that does not exist as unsent.

What happened

$ git push --force-with-lease=... origin HEAD:docs/adr009-popup-measured
commitlore: notes mirror (origin) failed: the 2s this hook waits for the remote ran out.
The branch was pushed; the records for these commits are still only local. The next push
retries this automatically, or run "commitlore sync" to send them now.
 + 9761c86d...4bd280dc HEAD -> docs/adr009-popup-measured (forced update)

So I ran what it named:

$ commitlore sync
origin       nothing-to-do  no notes mirror on either side
pr270-fork   nothing-to-do  no notes mirror on either side
pr54-fork    nothing-to-do  no notes mirror on either side

There is nothing to send. Checked three ways, all agreeing:

$ git for-each-ref refs/notes/          # (empty — no notes ref exists at all)
$ git notes --ref=commitlore show 4bd280dc   # (empty)
$ git log -1 --format='%B' 4bd280dc | grep -iE '^[A-Za-z-]+:'   # (no trailers)

"the records for these commits are still only local" is false: there are no records for these commits, in any location.

Why this is worse than what #632 reported

The v0.8.2 line was spawnSync git ETIMEDOUT; branch push continues — vague, but it only claimed a timeout. The v1.2.0 line is specific and asserts a fact about my repository's contents that it did not measure. I acted on it (ran sync, then went looking for the hook), which is the cost #632 was filed to remove.

The timeout is on reaching the remote. Whether local notes exist is a purely local question — git for-each-ref refs/notes/ — and does not depend on the remote answering. The message can be accurate at zero network cost.

Suggestion

Branch on the local check, which cannot time out:

• no local notes ref → "Could not reach the remote to check the notes mirror. Nothing is waiting locally, so nothing was lost."
• local notes present → the current sentence, which would then be true.

Second, smaller thing

commitlore sync iterates every remote and lets a plain git failure through for an unreachable one:

$ commitlore sync
fatal: repository 'https://github.com/narukijima/logic-pro-mcp.git/' not found
origin       nothing-to-do  no notes mirror on either side

That remote is a deleted fork, so failing is correct; surfacing it as a bare fatal: above the table reads as though sync itself failed. A row in the table (fork unreachable remote not found) would keep the report in one shape.

[MongLong0214]

Both fixed on main (c7debaf9 / 2f495b5b, shipping in 1.2.1).

The push line

The hook now measures before it claims. When this clone has no notes ref, the line is:

commitlore: notes mirror (origin) failed: <why>. The branch was pushed. Nothing is waiting locally, so nothing was lost.

and when records do exist it keeps the cautious sentence. "Unknown" is not reported as "none": if git will not answer at all, the cautious sentence stands, and that case is tested.

One correction to the issue's reading. The check is ref-scoped, not commit-scoped, and that was not the first attempt. The sentence's subject is "the records for these commits", so the first version enumerated the pushed commits and asked whether any carried a record. Three existing tests in test/pre-push-hook.test.ts rejected it, and they were right to: they write a record against HEAD, then advance the branch and push. The mirror publishes refs/notes/commitlore whole, so a record written against a commit the remote already has is still unsent — and the commit-scoped check calls that "nothing waiting". That is wrong in the more dangerous direction, so the ref is the right granularity.

Worth knowing for anyone reading the original report: on this repository the notes ref does exist while git ls-remote shows the remote already holds the identical OID. Nothing was waiting there either, but no local check can see that — what the remote holds is unknowable once the remote has stopped answering. What is knowable locally is whether this clone holds anything at all, which is exactly what the suggestion asked for.

The smaller thing

Reproduced before the fix, on a repository with a deleted remote:

deadfork     failed         remote: Repository not found.
fatal: repository 'https://github.com/…/does-not-exist.git/' not found
origin       fetched        collected the remote mirror

It was not a fatal: leaking past the table — it was inside the row. SyncResult.detail promises one line in its own doc comment and git does not, so the row broke in half and the second line read as a bare error standing above the next remote's result. Collapsing happens in failure(), the one place a git diagnostic becomes a detail, so the table, the JSON and the hook all inherit the promise. After:

deadfork     failed         remote not found
origin       in-sync        in-sync

Pre-tag review caught a defect in that repair, which is also fixed: the classifier matched repository .*not found, which takes repository metadata not found too — and since describeSync reads detail for ETIMEDOUT while execGit appends timeouts to partial stderr, the relabel could have erased the only part anything parses and reported a missing fork for a remote that was merely slow. It is bounded to git's two phrasings now, a timeout outranks the classification, and there is a classification test rather than a shape-only one.