Skip to content

Stop refusing an amend as a duplicate of the commit it replaces - #668

Merged
MongLong0214 merged 2 commits into
mainfrom
fix-638-amend-marker
Aug 14, 2026
Merged

Stop refusing an amend as a duplicate of the commit it replaces#668
MongLong0214 merged 2 commits into
mainfrom
fix-638-amend-marker

Conversation

@MongLong0214

Copy link
Copy Markdown
Owner

Closes #638.

git commit --amend was refused with duplicate-id, because the commit it
replaces is still HEAD when the record is checked and carries the same
Record-Id.

How it decides

commit-msg cannot tell an amend from an ordinary commit — git hands it
nothing that differs. prepare-commit-msg can, so it writes a marker and
commit-msg consumes it:

amend  =  source == "commit"
          && the sha argument resolves to HEAD
          && no multi-commit operation in progress

Anything unrecognised is not an amend. The two mistakes are not symmetric:
calling a non-amend an amend drops HEAD from the duplicate check and lets a
real identity collision through, while the reverse is only the inconvenience
this issue describes. A list of operations to exclude would break the day
git adds one, so the default has to be the safe direction.

Only duplicate-id ignores that commit. Follows: and Supersedes: still
resolve against it — removing it from the prior stream outright turned every
reference to a record declared there into a dangling one, which two existing
tests caught.

What it does not reach

--amend -m and --amend -F supply the message on the command line, so git
reports the message's origin rather than the commit's and the arguments are
byte-identical to an ordinary commit. Those stay refused. This is pinned as a
test rather than left as a comment, because measured against a -m fixture
the working code reads as broken.

Regressions

test/amend-marker.test.ts pins what git passes for each operation — ordinary
commit, amend, rebase reword, cherry-pick, revert, merge, and the -m/-F
boundary above. Reword is the case that matters: its arguments are identical
to an amend's, and only the rebase directory separates them.

Verified end to end: --amend --no-edit now reports references ok and exits
0 where it previously reported duplicate-id.

Not caused by this branch

A negative control written for this work — an ordinary commit reusing HEAD's
Record-Id — is accepted here and on main. That is #667, filed separately.

Work in progress: the decision logic is measured and its regressions pass, but
the marker is not being found at read time. Pushed unfinished because it lives
in a temporary worktree and losing it would cost the whole investigation, not
just the code.

`prepare-commit-msg` is the only hook git tells whether this is an amend — it
gets `commit` as the source and HEAD as the sha, where `commit-msg` gets
nothing that differs. It records that; `commit-msg` reads it, consumes it, and
only then does `duplicate-id` ignore HEAD.

Three conditions, all required, and anything unrecognised is not an amend:
`rebase -i` reword produces arguments identical to an amend and is separated
only by a rebase being in progress. The two mistakes are not symmetric — taking
a non-amend for an amend drops HEAD from the duplicate check and lets a real
identity collision through, while the reverse is only the inconvenience #638
describes. So the default is the safe direction rather than a list of
exclusions, which would break the day git adds an operation.

test/amend-marker.test.ts pins what git actually passes for six operations,
because the reword case is the kind of fact that stops being true without
anyone noticing.

Blast: module
Undo: easy
Certainty: tentative
Ruled-out: option A, not counting HEAD at all | it reverses what #430 built — its two assertions fail directly, and the loss is not narrow because HEAD is the position every commit passes through
Ruled-out: a heuristic on whether the new message extends the old | it would decide from a guess whether a record directs an agent, and no real case for it exists
Limit: the amend path does not pass yet; `--git-path` returns a relative path, so the hook and validate may resolve the marker against different working directories — that is the next measurement, not a guess to code around
Provenance: authored
Record-Id: r-amendmarker638
Verified: four cases in test/amend-marker.test.ts covering ordinary commit, amend, rebase reword, cherry-pick, revert and merge; the three conditions measured true against a real amend; tsc --noEmit clean
`git commit --amend -m` and `--amend -F` supply the message on the command
line, so git reports the message's origin rather than the commit's: the
arguments reaching prepare-commit-msg are byte-identical to an ordinary
commit. No marker can be written for them and the amend stays refused.

That is a limit of the signal git offers, not of this implementation, and it
is worth a test rather than a sentence: measured against a `-m` fixture the
working code reads as broken, which is exactly what happened here and cost
hours of debugging the wrong side.

Verified on the path git does mark: `--amend --no-edit` now passes where it
was refused as a duplicate.

Limit: git reports the message's origin, not the commit's
Blast: local
Undo: easy
Certainty: firm
Provenance: authored
Record-Id: r-638bnd
@MongLong0214 MongLong0214 added the release:0.9.0 Must close before 0.9.0: an unaware user gets a wrong result, or the docs already promise it label Aug 14, 2026
@MongLong0214

Copy link
Copy Markdown
Owner Author

The negative control, and what this PR does not prove

A negative control was written for this work: an ordinary commit reusing the
Record-Id already on HEAD must still be refused. It is not. Measured on a
fresh repository that was never amended, against a build of main:

SEED-LANDED=[1] commits=[1]
shape ok · references ok

The fixture was asserted valid before the result was read — the seed commit
landed and HEAD carried the id.

So the control passes on main too, which means it was never a valid control
for this change
, and it does not indicate a hole opened here. It is a
separate defect, filed as #667.

What that costs this PR is worth stating plainly: the safe direction — that an
unrecognised operation is not an amend, so HEAD stays in the duplicate check —
is enforced by construction and by test/amend-marker.test.ts, but it cannot
be demonstrated end to end until #667 is fixed, because the check it would fall
back on does not currently fire. The six-path argument table and the verified
--amend --no-edit pass are what this PR rests on.

Rebase

Rebased onto main (was on #661, before #664 · #666 · #662 landed). The only
conflicts were the generated artifact and its manifest — #648 — resolved by
rebuilding rather than by merging: canonical artifact a9927fa6.

TRACE instrumentation used while debugging is absent from src/ and from
dist/, checked on the branch rather than assumed.

The amend that produced this push was itself performed through the built CLI
carrying this fix, on a commit whose record declares Record-Id: r-638bnd. It
was not refused.

@github-actions

Copy link
Copy Markdown

CommitLore — record lint

Trailers: clean — 2 commits in origin/main..2607bae9a1f8c63a734187bb74aa0586a116c469
Active constraints: not read — commitlore: git log --follow accepts exactly one pathspec, so renames are not followed for 10 paths; query one path at a time to follow its rename chain (10 changed paths)

Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR.

@MongLong0214
MongLong0214 merged commit 2efe185 into main Aug 14, 2026
12 checks passed
@MongLong0214

Copy link
Copy Markdown
Owner Author

Correction: the comment above saying #667 blocks demonstrating this end to end is wrong. #667 was not a defect — the reproduction used byte-identical trailer payloads, which SPEC §3.2 treats as one record re-declared. A same-id collision with a different payload is refused (references failed), so the fallback this PR's safe direction relies on is present and working. #667 is closed with the measurement.

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

Labels

release:0.9.0 Must close before 0.9.0: an unaware user gets a wrong result, or the docs already promise it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

amend that fixes a malformed trailer is refused as a duplicate Record-Id

1 participant