Exit 1 must mean the record is wrong, and twice it did not - #362
Merged
Conversation
CommitLore — record lintTrailers: clean — 2 commits in Active constraints for the paths this PR touchesLimits (73)
Ruled out (163)
Warnings (47)
Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR. |
The contract at the top of this file is one sentence: exit 1 means "the record is wrong" and nothing else, because a commit-msg hook branches on it. Anything else that reaches 1 refuses a commit for a reason its author cannot fix by fixing the record. Two things did. A shallow clone does not carry the ancestor that declares the referenced id, so `Supersedes: r-old001` reports `dangling-ref` and the commit is blocked -- while the same message against a full clone of the same origin reads `references ok`. The record was never the problem; the history is truncated. `action/lint/lint.mjs` already refuses to lint a shallow checkout, because a green over fewer commits than the pull request contains is a lie. The same fact reaches the local hook from the other side, and it must equally refuse to report red. Only `dangling-ref` is withdrawn, and only when the clone is shallow. It is the one reference answer that reads straight off the ancestors. `duplicate-id` between a message's own blocks, or against a note on the same commit, is answered from the message alone -- truncation cannot reach it, and it is exactly the multi-block squash shape this command is most useful for. Withdrawing the whole class, which is the shape the `unfetched` gate at the top of the loop uses, would be a skip with no cause: a real red dropped in order to fix a false one. Reproduced through the built binary -- a two-block message sharing a `Record-Id` still exits 1 on the shallow clone. The skip is stated rather than taken. When a reference is withheld the check reports `not-checked`, naming the boundary and the fix, instead of `ok`: the status is what a machine branches on and the green is what a person carries away, and neither should read as a verdict this clone cannot give. The second was identity. `indexedHeadRecords` keyed rows on `(sha, source)` while the index's own unique key is `(commit_sha, source, block, seq)`, so every block of a multi-block commit folded into a single record and `trailerValue` read only the first `Record-Id` in it. `query.ts` keys on all four specifically to stop this. Within one message the answer is in the spec rather than the index: SPEC 2.4 closes by saying `Follows:`/`Supersedes:` resolve against `Record-Id`s regardless of which block declared them, so the comment calling a sibling carve-out future work was describing a deviation, not a boundary, and it is gone. Multi-block messages are what this project's own squash inheritance and GitHub's squash button both produce, so neither path was an edge. Record-Id: r-exitonemeans Limit: a shallow clone cannot tell a reference that resolves below the boundary from one that resolves nowhere, so neither verdict is available and the check can only name the question it could not answer Ruled-out: Skipping the whole reference check on a shallow clone, the shape the unfetched gate already uses | it would also withdraw duplicate-id, which is answered from the message alone and is the multi-block squash shape the hook is most useful for -- a real red dropped to fix a false one Ruled-out: Letting a block resolve its own Record-Id along with its siblings | a record that follows itself still resolves to nothing, and the truncation argument does not cover it, so it stays reported Ruled-out: Reporting the shallow skip as ok with the caveat attached | ok is what a machine branches on and what a reader remembers, and this clone has no verdict to offer on that reference Certainty: firm Blast: module Undo: easy Verified: three tests written red first and confirmed failing -- a shallow clone reported dangling-ref for Supersedes, a two-block message reported dangling-ref for a Follows: naming its own sibling, and a message reported dangling-ref for a Follows: naming block 1 of a commit in history; all three green after the change; validate, squash, hooks, stale and query pass at 322, and action-lint, capture-stage, cli, demo-fixture, doctor, dogfood, hooks-entry-resolution, schema, mcp-capture, gate-a-e2e, shallow-history, index-db, source-guards and help-text-honesty pass at 273; through the built binary a shallow clone now exits 0 saying not checked with the boundary named, a full clone of the same origin exits 0 saying references ok, and on that same shallow clone a duplicate Record-Id and a bad enum both still exit 1; typecheck clean and dist rebuilt Unverified: whether a reference that is genuinely dangling and also below a shallow boundary is still caught downstream -- the CI lint refuses shallow checkouts outright, so it is, but no test here walks that path
`hasShallowHistory` spawns `git rev-parse --git-path shallow`. Put first in the conjunction, that spawn happens on every `validate` -- and `validate` is what the commit-msg hook runs, so it is a process per commit in every repository that installed the gate. The overwhelmingly common case is a record with no violations at all, which has nothing for the shallow answer to decide. Testing the violation list first is free, and short-circuits before the spawn in exactly that case. Both operands are pure with respect to each other, so the verdict is unchanged; only the cost moves. Record-Id: r-shallowlast Limit: the spawn still happens once per validate that has a dangling ref, which is the case where the answer is actually needed Ruled-out: Caching the shallow answer for the process | validate is a short-lived process and a cache would outlive the fetch --unshallow that changes the answer Certainty: firm Blast: local Undo: easy Verified: typecheck clean; dist rebuilt; validate, squash and shallow-history pass at 83 with the suppression behaviour unchanged Unverified: the size of the saving on a real commit -- it was not measured, only removed from the common path
MongLong0214
force-pushed
the
fix-issue-352
branch
from
August 1, 2026 04:36
bf31c5b to
0913821
Compare
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 #352
validatepromises that exit 1 means "the record is wrong" and nothing else, because the commit-msg hook branches on it. Two inputs reached 1 for reasons an author cannot fix by fixing the record.(a) Shallow clone
A
--depth 1clone does not carry the ancestor that declares the referenced id, so a validSupersedes:reporteddangling-refand blocked the commit. The same message against a full clone of the same origin readreferences ok.action/lint/lint.mjs:128-139already refuses to lint a shallow checkout, because a green over fewer commits than the PR contains is a lie. The same fact reaches the local hook from the other side, so it now equally refuses to report red.Narrow, not the whole check. Only
dangling-refis withdrawn, and only whenhasShallowHistoryis true. It is the one reference answer that reads straight off the truncated ancestors.duplicate-idbetween a message's own blocks, or against a note on the same commit, is answered from the message alone — truncation cannot reach it, and it is exactly the multi-block squash shape the hook is most useful for. Withdrawing the whole class (the shape theunfetchedgate uses) would drop a real red in order to fix a false one.The skip is reported, never silently passed. When a reference is withheld the check reports
not-checkedwith the boundary named and the fix, rather thanok:On that same shallow clone, a duplicate
Record-Idand a bad enum both still exit 1.(b) Multi-block messages
Two halves, both fixed:
indexedHeadRecordskeyed rows on(sha, source)while the index's own unique key is(commit_sha, source, block, seq). Every block of a multi-block commit folded into one record, andtrailerValuereads only the firstRecord-Idin it — so block 1's identity never reached the declared set.query.ts:469keys on all four specifically to stop this class of bug.Follows:/Supersedes:resolve againstRecord-Ids "regardless of which block declared them". A block's declared set is nowpriorplus its siblings — not itself, so a record that follows its own id still reportsdangling-ref. The comment calling a sibling carve-out "future work" was describing a deviation from the spec, so it is gone.Multi-block messages come from this project's own squash inheritance and from GitHub's squash button, so neither path was an edge case.
Tests
Three tests written red first and confirmed failing before any fix:
dangling-ref Supersedes — got "r-old352"Follows:to an earlier block of the same messagedangling-ref Follows — got "r-blocka352"Follows:to aRecord-Idin block 1 of a commit in historydangling-ref Follows — got "r-histb352"The shallow test carries a full-clone control against the same origin, so the fixture is proven to be a valid record that only truncation hides, and it configures the notes refspec so the
unfetchedgate cannot make it pass for the wrong reason.No existing test was modified or weakened.
Verification
validate,squash,hooks,stale,query— 322 passedaction-lint,capture-stage,cli,demo-fixture,doctor,dogfood,hooks-entry-resolution,schema,mcp-capture,gate-a-e2e,shallow-history,index-db,source-guards,help-text-honesty— 273 passednpm run typecheckclean,npm run buildclean,distrebuilt