fix: publish keeps a closing trailer block of --message last, so git reads Co-Authored-By again (#149)
Files changed: - CHANGES.md - VERSION - tools/CONTRACT.md - tools/chemenu/commands/git_publish.py - tools/chemenu/tests/test_git_publish.py Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SnAJ7Z3CpVD3PRbN73QtU2
This commit is contained in:
1 parent
c65d559690
commit
80488bee38
5 files changed
+168
-8
No files matched your search
@@ -1746,6 +1746,7 @@ Reconcile with `<remote>/<branch>`, then stage all changes, commit, and push.
|
||||
- The `--confirm` token covers each counted path, the blob id of its contents and the publish target: a different file list or edited contents need a new clearance.
|
||||
- Publish-Remote Gate: when the checkout carries `.wikitool-remotes.json` and the push URL of `--remote` is not listed in it, exits 42 before the reconcile fetches anything. The URL is read with `git remote get-url --push`, so a repointed remote does not pass on its name. An absent file means unrestricted; a malformed one is an error, not permission.
|
||||
- `--path` (repeatable) scopes the whole operation - gate count, staging and commit - to that subtree.
|
||||
- Commit message: `--message`, then a `Files changed:` paragraph listing every committed path. When `--message` ends in a paragraph git reads as a trailer block (`Co-Authored-By:` and the like), that block stays the last paragraph and the list goes in front of it, since git reads trailers from the last paragraph only. `git interpret-trailers` decides whether there is such a block, and the list moves only when git reads the same trailers from the result; otherwise it is appended at the end. `--message` is not part of any gate token.
|
||||
- After a successful commit or push whose changed files include `tools/`, `types/`, `instructions/`, `AGENTS.md` or a path ending in `CONTRACT.md`, prints one reminder line: the phase past this point (an issue-body rewrite, `docs/` staleness, a changelog entry's accuracy) is not covered by `docs verify`, `instructions verify` or `pytest`. It is not a gate: no exit code change, nothing to clear, and silent for an ordinary content publish.
|
||||
|
||||
**SEE ALSO**
|
||||
|
||||
@@ -80,11 +80,12 @@ DEFAULT_MASS_UPDATE_THRESHOLD = 10
|
||||
GATE_EXEMPT_PREFIXES = ("work/",)
|
||||
|
||||
|
||||
def _run(args: list[str]) -> subprocess.CompletedProcess:
|
||||
def _run(args: list[str], input: Optional[str] = None) -> subprocess.CompletedProcess:
|
||||
"""Run a `git ...` argument list, starting git from its recorded path."""
|
||||
if args and args[0] == "git":
|
||||
args = [toolpaths.git(), *args[1:]]
|
||||
return subprocess.run(args, cwd=config.ROOT, capture_output=True, text=True, encoding="utf-8")
|
||||
return subprocess.run(args, cwd=config.ROOT, capture_output=True, text=True, encoding="utf-8",
|
||||
input=input)
|
||||
|
||||
|
||||
# --- Publish-Remote Gate -----------------------------------------------------
|
||||
@@ -1077,6 +1078,43 @@ def sync_command(
|
||||
success(summary or f"Nothing to reconcile against {remote}/{branch}.")
|
||||
|
||||
|
||||
def _trailers(message: str) -> str:
|
||||
"""The trailers git reads from `message`, one normalized `Key: value` per line, or "".
|
||||
|
||||
`--no-divider` because that is how `git log --format=%(trailers)` reads a commit: a `---`
|
||||
line in the body does not end the message there. Run in `config.ROOT`, so a `trailer.*`
|
||||
setting of this checkout counts the same way it does for `git log`."""
|
||||
result = _run(["git", "interpret-trailers", "--parse", "--no-divider"], input=message)
|
||||
return result.stdout if result.returncode == 0 else ""
|
||||
|
||||
|
||||
def commit_message(message: str, changed_files: list[str]) -> str:
|
||||
"""`message` plus the "Files changed:" list - before a closing trailer block, not after it.
|
||||
|
||||
git reads trailers (`Co-Authored-By:` and the like) only from the last paragraph of a
|
||||
message, so appending the list behind them would hide them from every tool that reads
|
||||
them. Whether `message` ends in a trailer block is git's call, not a regex's: continuation
|
||||
lines, `(cherry picked from ...)`, the title rule and the share of trailer lines a block
|
||||
needs all decide it there. The list moves in front of the last paragraph only when git
|
||||
reads exactly the same trailers from the result as from `message`; in every other case
|
||||
the message is the plain append it always was."""
|
||||
file_list = "\n".join(f"- {f}" for f in changed_files)
|
||||
plain = f"{message}\n\nFiles changed:\n{file_list}"
|
||||
trailers = _trailers(message)
|
||||
if not trailers:
|
||||
return plain
|
||||
lines = message.rstrip().split("\n")
|
||||
blank = [i for i, line in enumerate(lines) if not line.strip()]
|
||||
if not blank:
|
||||
return plain
|
||||
head = "\n".join(lines[:blank[-1]]).rstrip()
|
||||
tail = "\n".join(lines[blank[-1] + 1:])
|
||||
if not head:
|
||||
return plain
|
||||
candidate = f"{head}\n\nFiles changed:\n{file_list}\n\n{tail}"
|
||||
return candidate if _trailers(candidate) == trailers else plain
|
||||
|
||||
|
||||
@cli_contract.record(cli_contract.CommandRecord(
|
||||
path="publish",
|
||||
summary="Reconcile with `<remote>/<branch>`, then stage all changes, commit, and push.",
|
||||
@@ -1143,6 +1181,13 @@ def sync_command(
|
||||
"permission.",
|
||||
"`--path` (repeatable) scopes the whole operation - gate count, staging and commit - "
|
||||
"to that subtree.",
|
||||
"Commit message: `--message`, then a `Files changed:` paragraph listing every committed "
|
||||
"path. When `--message` ends in a paragraph git reads as a trailer block "
|
||||
"(`Co-Authored-By:` and the like), that block stays the last paragraph and the list "
|
||||
"goes in front of it, since git reads trailers from the last paragraph only. "
|
||||
"`git interpret-trailers` decides whether there is such a block, and the list moves "
|
||||
"only when git reads the same trailers from the result; otherwise it is appended at "
|
||||
"the end. `--message` is not part of any gate token.",
|
||||
"After a successful commit or push whose changed files include `tools/`, `types/`, "
|
||||
"`instructions/`, `AGENTS.md` or a path ending in `CONTRACT.md`, prints one reminder "
|
||||
"line: the phase past this point (an issue-body rewrite, `docs/` staleness, a "
|
||||
@@ -1373,8 +1418,7 @@ def publish_command(
|
||||
if add_result.returncode != 0:
|
||||
fail(f"git add failed:\n{add_result.stderr}")
|
||||
|
||||
file_list = "\n".join(f"- {f}" for f in changed_files)
|
||||
full_message = f"{message}\n\nFiles changed:\n{file_list}"
|
||||
full_message = commit_message(message, changed_files)
|
||||
|
||||
# With a pathspec, `git commit -- <paths>` commits exactly those paths and
|
||||
# ignores anything else that happens to be staged, so batches stay disjoint.
|
||||
|
||||
@@ -969,11 +969,11 @@ def test_publish_reactive_retry_survives_a_genuine_race(repo, monkeypatch):
|
||||
raced = {"done": False}
|
||||
real_run = git_publish._run
|
||||
|
||||
def racy_run(args):
|
||||
def racy_run(args, **kwargs):
|
||||
if args[:2] == ["git", "push"] and not raced["done"]:
|
||||
raced["done"] = True
|
||||
_push_from_writer(writer, "mid-race.md", "landed during the push\n")
|
||||
return real_run(args)
|
||||
return real_run(args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(git_publish, "_run", racy_run)
|
||||
|
||||
@@ -1391,3 +1391,98 @@ def test_gate_has_no_flag_that_opens_it(repo):
|
||||
|
||||
params = inspect.signature(publish_command).parameters
|
||||
assert not any("remote" in name and "confirm" in name for name in params)
|
||||
|
||||
|
||||
# --- The commit message: "Files changed:" stays in front of a closing trailer block ---
|
||||
|
||||
ATTRIBUTION = "Co-Authored-By: A <a@x>\nClaude-Session: https://claude.ai/code/session_x"
|
||||
|
||||
|
||||
def _plain(message, files):
|
||||
"""The message `publish` has always written - the layout every message without a trailer
|
||||
block must keep, byte for byte."""
|
||||
return message + "\n\nFiles changed:\n" + "\n".join(f"- {f}" for f in files)
|
||||
|
||||
|
||||
def _git_trailers(root, message):
|
||||
return subprocess.run(
|
||||
["git", "interpret-trailers", "--parse", "--no-divider"],
|
||||
cwd=root, input=message, capture_output=True, text=True, check=True,
|
||||
).stdout
|
||||
|
||||
|
||||
@pytest.mark.parametrize("message", [
|
||||
"fix: something",
|
||||
"fix: something\n\nA body paragraph.",
|
||||
# A colon in prose is not a trailer: git needs every line of the last paragraph to be one
|
||||
# (or a quarter of them, alongside a git-generated one).
|
||||
"fix: something\n\nSee: this free paragraph has a colon in it\nand carries on in prose.",
|
||||
# The first paragraph is the title and is never a trailer block, whatever it looks like.
|
||||
"Co-Authored-By: A <a@x>",
|
||||
# Trailers that are not in the last paragraph are not trailers for git either.
|
||||
"fix: something\n\n" + ATTRIBUTION + "\n\nA closing remark.",
|
||||
])
|
||||
def test_a_message_without_a_trailer_block_is_byte_identical_to_before(repo, message):
|
||||
assert _git_trailers(repo, message) == ""
|
||||
assert git_publish.commit_message(message, ["kb/a.md", "kb/b.md"]) == _plain(message, ["kb/a.md", "kb/b.md"])
|
||||
|
||||
|
||||
def test_the_file_list_goes_in_front_of_a_closing_trailer_block(repo):
|
||||
message = "fix: something\n\nA body.\n\n" + ATTRIBUTION
|
||||
assert git_publish.commit_message(message, ["kb/a.md", "kb/b.md"]) == (
|
||||
"fix: something\n\nA body.\n\nFiles changed:\n- kb/a.md\n- kb/b.md\n\n" + ATTRIBUTION
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("message", [
|
||||
"fix: something\n\n" + ATTRIBUTION,
|
||||
# A continuation line belongs to the trailer above it.
|
||||
"fix: something\n\nA body.\n\nCo-Authored-By: A\n <a@x>\nClaude-Session: x",
|
||||
# A git-generated trailer lets a block carry non-trailer lines (the 25% rule).
|
||||
"fix: something\n\nA body.\n\nfree text\n(cherry picked from commit abc123)\nSigned-off-by: A <a@x>",
|
||||
# `git log` reads past a `---` line, and so does the check.
|
||||
"fix: something\n\nA body.\n---\nmore body\n\n" + ATTRIBUTION,
|
||||
# Trailing whitespace after the block, which `git commit` strips anyway.
|
||||
"fix: something\n\nA body.\n\n" + ATTRIBUTION + "\n\n \n",
|
||||
])
|
||||
def test_git_reads_the_same_trailers_after_the_file_list_is_added(repo, message):
|
||||
"""Whatever git reads as the message's trailers, it still reads from the commit message -
|
||||
and the file list is in there in full."""
|
||||
expected = _git_trailers(repo, message)
|
||||
assert expected
|
||||
result = git_publish.commit_message(message, ["kb/a.md", "kb/b.md"])
|
||||
assert _git_trailers(repo, result) == expected
|
||||
assert "\n\nFiles changed:\n- kb/a.md\n- kb/b.md\n\n" in result
|
||||
|
||||
|
||||
def test_publish_commits_trailers_git_log_can_read(repo):
|
||||
_write_files(repo, 3)
|
||||
_publish(message="fix: something\n\nA body.\n\n" + ATTRIBUTION)
|
||||
assert _git(
|
||||
repo, "log", "-1", "--format=%(trailers:key=Co-Authored-By,valueonly)"
|
||||
).stdout.strip() == "A <a@x>"
|
||||
body = _git(repo, "log", "-1", "--format=%B").stdout
|
||||
assert "Files changed:\n- kb/page0.md\n- kb/page1.md\n- kb/page2.md\n" in body
|
||||
|
||||
|
||||
def test_publish_without_trailers_commits_the_same_message_as_before(repo):
|
||||
_write_files(repo, 2)
|
||||
_publish(message="fix: something\n\nA body.")
|
||||
assert _git(repo, "log", "-1", "--format=%B").stdout.rstrip("\n") == _plain(
|
||||
"fix: something\n\nA body.", ["kb/page0.md", "kb/page1.md"]
|
||||
)
|
||||
|
||||
|
||||
def test_a_mass_update_token_does_not_depend_on_the_message(repo):
|
||||
"""The token binds the changeset, never `--message`: one issued while publishing a plain
|
||||
message clears the same changeset carrying a trailer block, and the trailers survive."""
|
||||
_write_files(repo, 10)
|
||||
with pytest.raises(typer.Exit) as excinfo:
|
||||
_publish(message="big change")
|
||||
assert excinfo.value.exit_code == EXIT_NEEDS_CLEARANCE
|
||||
|
||||
_publish(message="big change\n\n" + ATTRIBUTION, confirm=_token_for(repo))
|
||||
assert _git(repo, "status", "--porcelain", "-uall").stdout == ""
|
||||
assert _git(
|
||||
repo, "log", "-1", "--format=%(trailers:key=Co-Authored-By,valueonly)"
|
||||
).stdout.strip() == "A <a@x>"
|
||||
Reference in new issue
Block a user