Skip to content

Eight canonical-review entries carry a hubCommit that resolves nowhere on the remote #1210

Description

@ptr727

Eight entries in reports/canonical-review.json carry a hubCommit that resolves nowhere on the remote, so the hub state those eight passes read cannot be re-derived by anyone who did not run them.

The value

df5e2493b48aa45d80f586db68af0c21be97ea21, on eight entries. It is the pre-squash tip of the deleted branch behind "Fix Carried Text a Downstream Repository Cannot Fix in Its Own Copy" (2026-08-31), and it was never pushed in that form.

  • git cat-file -t answers on the machine that recorded it, but only because a loose object survives there.
  • git branch -a --contains df5e2493 finds no ref.
  • gh api repos/ptr727/ProjectTemplate/commits/df5e2493... returns HTTP 422 No commit found for SHA.

A fresh clone, a second machine, or a git gc --prune on the recording machine each turn it into a dangling reference.

Why the field matters

scripts/canonical_review.py states the purpose of hubCommit as "a verdict carrying no commit cannot be re-run". A pass is recorded against the content a reviewer read, and the commit is what lets a later reader reconstruct the tree that content sat in. Eight passes currently have no such anchor.

How it happens

A pass records hubCommit from HEAD at record time. Recording after a commit and then amending that commit, or recording on a branch whose commits are later squashed and whose branch is deleted, both leave the recorded SHA unreachable. Neither is unusual: an amend after a review pass is the ordinary way to correct a commit message, and squash-merge with branch deletion is the fleet's merge method for a feature branch.

This was hit live and caught on #1209, where two freshly written entries carried a SHA an amend had just orphaned. That instance was fixed by resetting to the merge base before recording, so the entries name a commit that exists on develop. The eight above predate that branch and sit on develop today.

Candidate resolutions

The choice is a design decision rather than a cleanup:

  1. Record the merge base rather than HEAD, since a merge base is by construction already on the target branch and survives any rewrite of the branch under review.
  2. Keep recording HEAD and have the engine rewrite entries to the squash commit at merge time, which needs a hook the fleet does not have today.
  3. Accept unreachable values and have canonical_review.py report flag them, so a burn-down reader can see which passes have lost their anchor rather than trusting all of them equally.

Option 1 costs nothing and is what #1209 did by hand. Option 3 is worth having regardless, since it is the only one that surfaces the eight already recorded.

Related

Found by an adversarial diff pass on #1209. Related to #1149, #1150 and #1151, the other canonical_review.py follow-ups.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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