Skip to content

Two thirds of canonical-review hubCommit stamps stop resolving after a squash merge #1222

Description

@ptr727

reports/canonical-review.json stamps each recorded pass with a hubCommit, read from HEAD at record time. For most rows that commit is no longer reachable from main or develop, because the branch that recorded it was squash-merged and its commits discarded.

Measured on the ledger as it stands: 40 of 59 recorded passes name a hubCommit that exists in the local object store but is not an ancestor of HEAD. A fresh clone resolves fewer still, since it fetches no unreachable objects at all, and GitHub answers 404 for a commit that no ref reaches once it is garbage-collected.

Why it matters

The field's purpose is to say which state of the repository a pass was recorded against. A reader auditing whether a unit's recorded pass is credible wants to resolve that commit and see what the unit held at the time. For two thirds of the ledger that resolution fails, so the field records provenance that cannot be checked.

It also makes one existing practice look wrong when it is right. Recording at the merge base, which this repository does deliberately so that a later amend does not strand the stamp, produces a hubCommit that IS reachable. Recording mid-branch produces one that stops being reachable at the squash merge. The two shapes are indistinguishable in the file, so a reader cannot tell a durable row from a transient one.

What needs deciding

  1. Stamp the merge base rather than HEAD. The merge base is reachable by construction and stays so after a squash. The cost is that the stamp no longer names the exact tree the reviewer read, which the digest already covers, so the field would become "the base this pass was measured from" rather than "the commit HEAD was at".
  2. Stamp the eventual squash commit. Accurate and durable, and impossible to write at record time, so it would need a post-merge rewrite of the ledger that nothing performs today.
  3. Drop the field. The digest already identifies the content the pass covered, and coverage is decided by digest match rather than by commit. If nothing reads hubCommit, its unresolvability costs nothing and the field is noise.
  4. Keep it and document it as best-effort. scripts/README.md's canonical_review.py section states that a stamp may become unreachable after a squash merge, so a reader stops expecting to resolve one.

Option 3 or 4 is likely right, and which depends on whether anything actually consumes the field. That is the part to check first.

Related

Found by an adversarial pass on #1215 while verifying that branch's own stamps resolve, which they do. Related to #1149, #1150 and #1151, the other follow-ups from the canonical-review gate's own change.

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