Canonical merge of #882 - #883
Merged
Merged
Conversation
…he flag Two flags on `capture`, one reporter, one failure seen from both ends. `--out` is documented as "write the pending nonce to a file". The write was wired correctly and guarded on `result.nonce`, but the prompt-only branch returned `nonce: null` while prepare had already persisted a real transaction under a real nonce. Prompt-only is the step `--out` exists for -- get the nonce, hand the prompt to a model, come back with `--draft` -- so the guard was false on exactly the run that needed it. Three runs, three exit 0, three missing files, with and without `--diff`. The nonce is now reported, so `--out` writes it and `--json` carries it (#878). `--diff` was accepted, ignored by prepare, and honoured by verify. Prepare hashes `git diff --cached` itself into `source_hashes.diff`; verify hashed the file the caller named. Passing the same `--diff` to both steps therefore failed whenever that file was not byte-identical to the staged diff, and failed as `source-mismatch: diff hash does not match the prepared transaction` -- a statement about the draft's sources, for a fault belonging entirely to the flag. The reporter re-checked quotes and locators that were never wrong (#877). The refusal is correct and stays. A capture transaction binds to the staged diff, which prepare, verify and stage each recompute independently, because every binding is computed server-side and never from the caller (capture-stage.ts). Binding `--diff` at prepare would only move the same rejection to stage, and would let a record's diff evidence be checked against a diff that is not the one being committed. So `--diff` can assert what is staged; it cannot select something else, and the help text now says that instead of implying an override it never had. What changed is where the refusal happens. It is raised before prepare, as a usage error, naming the flag and naming the way out -- `git reset --soft`, which is the flow the reporter had to discover on their own after the message sent them elsewhere. Refusing before prepare also stops a doomed run leaving a transaction behind; the old path wrote two pending files per attempt for a capture that could not succeed. The `#359` help-text guard read the option out of a single source line, which r-diffdefault had already recorded as its known limit. The description outgrew one line here and all three of its assertions went from checking a string to checking `undefined` -- green either way. It now reads the whole `.option(...)` call, and asserts the description does not promise a diff other than the staged one, which is the #359 defect pointed the other way. Both issue repros were run against the built CLI. `--out` writes a 32-hex nonce naming a real file under .git/commitlore/pending (3/3, was 0/3), and a `--diff` that differs exits 2 naming the flag with zero pending files left (was `source-mismatch`, exit 0, two files). Negative controls: with `nonce: null` restored the three #878 tests fail; with the refusal disabled the two #877 refusal tests fail while the identical-`--diff` control still passes, which is what keeps the fix from degenerating into rejecting every `--diff`. Full suite 3192 passed, 0 failed; tsc clean. Closes #878 Closes #877 Record-Id: r-capflags877 Provenance: authored Ruled-out: make --diff bind the transaction at prepare | capture-stage.ts recomputes the staged diff and rejects a mismatch before writing, so the same failure would surface one step later, and the binding contract states every binding is computed server-side and never from the caller Ruled-out: drop --diff from capture entirely | every existing caller passes one, including this suite's own fixtures, and an assertion about what is staged is worth keeping Ruled-out: leave the refusal at verify and only reword its message | verify cannot name the flag without knowing a flag was passed, and the pending file is already written by then Limit: --out is still silently skipped when the run fails before a nonce exists; those paths already exit non-zero, so the silence is not the only signal, but it is not an error either Limit: the refusal compares whole file contents, so a --diff differing only in trailing whitespace is refused with no hint that the difference is invisible Warn: the #359 guard's teeth depend on the four-space indent of chained .option calls; reformatting that chain makes it match nothing again Blast: module Undo: easy Certainty: firm Verified: 23/23 in test/capture.test.ts, 7/7 in test/help-text-honesty.test.ts, full suite 3192 passed 4 skipped 0 failed, tsc exit 0 Verified: each fix seen failing with the defect restored and the source rebuilt, not only passing with it applied Unverified: whether the MCP capture tools reach the same prompt-only path and were losing the nonce the same way -- only the CLI was measured Co-Authored-By: Claude <noreply@anthropic.com>
`Claude-Session:` is neither a SPEC §3 key nor `X-<Name>`, so refusing it is
correct and it stays refused. What it cost is the point: Claude Code instructs
every session to end its commit message with that trailer, so the key arrives by
default, the hook refuses it, the commit is lost, and an agent that cannot see
the repository's earlier commits emits it again. Reported twice in one day in two
repositories by the same author, the second time after already knowing about the
first, because `X-` is not the kind of thing that stays in mind between
repositories. This repository's own history already carries the accepted form, so
the convention exists and nothing surfaced it at the point of refusal.
`X-<Name>` was in the answer and was not enough. The answer now names the key the
author could have written:
want "a key from SPEC §3, or X-Claude-Session if this is your own metadata"
SPEC §3 still leads the sentence, because for most unknown keys the protocol
vocabulary is the better answer. Nothing is accepted that was not accepted
before, and nothing is rewritten -- SPEC §6 says the message is not modified, and
silently renaming someone's trailer would be worse than refusing it.
Two keys are deliberately not offered the prefix. One whose value reads as a
sentence keeps the #647 answer: that author's problem is that their prose became
metadata, and `X-` is the wrong door for them -- pointing at it is what sent an
author to invent `X-Live:` the last time. One that differs from a SPEC §3 key
only by case keeps the plain answer too, because `limit:` is `Limit:` miswritten
and `X-limit` would be a valid record carrying the wrong key. That second guard
exists only because this change created the hazard; without it the fix would make
that case worse than it was.
There is no predicate separating `Claude-Session` from `Constraint` -- `X-` makes
almost any alphabetic key valid -- so the hint fires for both, and
spec/fixtures/invalid/03-unknown-key.expected.json moves with it. The violation
that fixture pins is unchanged and `Constraint:` is still refused; only the
advisory `want` differs, which test/validate.test.ts already records as prose
owned by src/core/schema.ts rather than part of the conformance contract, since
SPEC §9 pins the violation class. r-prosetrailer647 ruled the other way on a
conformance fixture once -- there the fixture caught a misclassification and the
heuristic changed. Nothing is misclassified here.
Verification ran the reporter's own case: the commit-msg refusal of
`Claude-Session:` now names `X-Claude-Session`, and still exits 1 with the
message unmodified. Negative control: with the constant restored, three of the
schema tests fail, including the conformance fixture comparison.
Closes #881
Record-Id: r-xprefixhint881
Provenance: authored
Ruled-out: hint only for hyphenated keys, to leave the fixture untouched | the predicate is a guess dressed as a rule -- a single-word vendor key would silently get no hint, and hyphenation describes protocol keys too
Ruled-out: carry the hint in a new Violation field instead of want | the conformance fixture is compared with toEqual, so an added field moves it exactly as changed text does, for a larger surface
Ruled-out: accept Claude-Session as a well-known foreign key | r-coauthoredcasing holds that exemption to a fixed pair of standardised trailers, and the reporter explicitly did not ask for the key to be accepted
Limit: `Constraint:` is now told about `X-Constraint` when `Limit:` is probably what it meant; the message names SPEC §3 first, but nothing points at the specific key it resembles
Limit: the case-insensitive guard covers SPEC §3 keys only, so `signed-off-by:` still gets the prefix offered even though the foreign-key exemption would have taken it
Warn: this is the second time #647's reasoning has been narrowed; the prose branch is what protects it, and folding the two branches together would undo both changes at once
Blast: module
Undo: easy
Certainty: firm
Verified: 57/57 in test/schema.test.ts, full suite 3192 passed 4 skipped 0 failed, tsc exit 0
Verified: seen failing with the old constant restored before it was seen passing
Unverified: whether any other advisory want in schema.ts describes a shape where it could name the value, as this one now does
Co-Authored-By: Claude <noreply@anthropic.com>
Two capture flags that did nothing and said nothing, and a refusal that knew the fix and did not say it: #877, #878, #881. Thirty-six version pins across eleven files. The JSON manifests were bumped by parsing the document rather than replacing text, because `"version": "1.2.4"` also matches `"rolldown": "~1.2.4"` in the lockfile -- a dependency genuinely at that version, which no test reads, so corrupting it would be silent. The READMEs and installers were anchored to the three install shapes rather than to the bare version string, because README.md also carries prose about the v1.0.2 release boundary that is a historical fact and must not move. server.json's installer one-liner and release URL are under the registry's publisher-provided `_meta` block, not under `packages`, which is where a path written from memory put them; the completeness check caught it before anything was written. The bump script re-reads every file it touched afterwards and reports every remaining occurrence of the old version, so the one that is supposed to remain is seen rather than assumed. Record-Id: r-release125 Provenance: authored Ruled-out: text-replacing the version across all manifests | it also matches four dependencies genuinely at that version, and no test reads them Ruled-out: bumping every v1.2.4 found in the READMEs | README.md's release-boundary prose is a historical statement, and moving it makes the document say something false that no test checks Limit: dist/ and installer/canonical-artifact.json are not in this commit, so artifact:verify fails on this tree by design -- canonical-merge.yml rebuilds them on linux/amd64, where a macOS esbuild output would not match Blast: system Undo: easy Certainty: firm Verified: 156/156 across manifest, readme, release-version, check-release-version and install-script tests Verified: the only remaining 1.2.4 in the eleven touched files is package-lock.json:2744 "rolldown": "~1.2.4" Unverified: the install one-liners cannot be exercised until the tag exists; install.sh clones a pinned tag, so any check before that reads the previous release Co-Authored-By: Claude <noreply@anthropic.com>
`build:canonical` on the merged tree, so the commit that lands matches the source it lands with. The pull request carried source only, which is what a contributor on a host that cannot run a linux/amd64 Docker build can produce (#720). Limit: this proves the bundle matches this tree; whether this tree is what a reviewer wants is what the pull request is for Blast: system Undo: easy Certainty: firm Record-Id: r-canonmerge882 Provenance: authored Verified: artifact:verify passed against the regenerated manifest in the same job, before any credential was available to it CommitLore-Version: 2.0.0
CommitLore — record lintTrailers: clean — 5 commits in Active constraints for the paths this PR touchesLimits (394)
Truncated: 838 lines omitted — the comment hit GitHub's 65000 character limit. Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR. |
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.
The commit that will land for #882:
mainplus that source plus a canonical rebuild, built together so all eleven required contexts run on the tree that merges rather than on one that resembles it.#882 carries source only, which is what a contributor on a host that cannot run a
linux/amd64Docker build can produce (#720). Nothing was rebuilt by hand.Merge this with a merge commit, not a squash. This branch merged #882 with
--no-ff, so its head commit is an ancestor here: a merge commit lands that commit onmain, and GitHub then records #882 as merged because its head is reachable -- which is what T-1502 asks for. A squash lands new bytes instead, and #882 stays open with nothing to point at.This body deliberately carries no closing keyword. GitHub binds one only to the number straight after it, and a pull request closed by keyword is recorded closed rather than merged -- the opposite of the line above. Reachability does the closing here.
Opened by
canonical-merge.ymlfor #719.