diff --git a/CHANGES.md b/CHANGES.md index 182a90e..6548d12 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -59,11 +59,13 @@ concern - readable here, never shipped as something to parse. --- -## 8.0.0-beta.9 - 2026-09-30 - bug-report instruction: step 1 no longer calls every bundle unpseudonymised +## 8.0.0-beta.10 - 2026-09-30 - publish: the gate lists the staged state; a missing or unreachable remote stops before the commit **Author:** Torben Nehmer -**Breaking Change:** Page titles must form valid, unique file names on Windows and macOS: new and rename refuse forbidden characters, reserved names (including INDEX and COLLECTION), a trailing dot or space, and titles that collide with another page by case or Unicode normalization; lint reports existing violations as hard errors - rename each affected page with tools/wikitool rename +**Breaking Change:** +- Page titles must form valid, unique file names on Windows and macOS: new and rename refuse forbidden characters, reserved names (including INDEX and COLLECTION), a trailing dot or space, and titles that collide with another page by case or Unicode normalization; lint reports existing violations as hard errors - rename each affected page with tools/wikitool rename +- publish without --no-push now exits 1 before committing when the remote is unreachable or not configured, where it used to commit locally and fail at the push - an offline session or a local-only instance must pass --no-push **Migration:** none required - No page format changes; the rule only refuses titles, and each affected page is renamed individually with tools/wikitool rename @@ -82,6 +84,7 @@ concern - readable here, never shipped as something to parse. - Live tracker suite: WIKITOOL_TASKS_CONFIG override, real-tracker tests for Super Productivity and CalDAV, nightly workflow and test image - Bug-report collector: tools/bugreport.py and instructions/bug-report.md - Bug-report collector can pseudonymise identities, in two stages +- publish: the gate lists the staged state; a missing or unreachable remote stops before the commit **Low impact** - version bump no longer points at version release in its output @@ -116,6 +119,46 @@ concern - readable here, never shipped as something to parse. - bug-report instruction: step 1 no longer calls every bundle unpseudonymised +### publish: the gate lists the staged state; a missing or unreachable remote stops before the commit (Gitea #159) + +**The Mass-Update Gate counted a path twice.** `collect_changes` read `git status --porcelain`, which +reports the index and the working tree separately. A path staged as deleted that sits in the working +tree again (`git rm -r raw`, then `git restore --source=HEAD -- raw/CONTRACT.md`) appears as `D ` and +`??`; the gate counted both, while `git add -A` cancels them out and the commit held neither. The list +a human approved therefore named a deletion and a new file that were never committed. `collect_changes` +now stages into a scratch copy of the index (`GIT_INDEX_FILE`) and reads `git diff --cached --no-renames` +from it, so the list is the state the commit will hold. The real index and the working tree stay +byte-identical, also when the computation fails; `git add` writes the new blobs into the object store, +where `gc` collects the unreferenced ones. + +- The digest in the `--confirm` token is now the blob id of the staged content instead of a sha256 over + the working-tree file, so the token binds to exactly what is committed. A deletion still has none. +- A rename is listed as its old path deleted plus its new path added, which is what the commit holds and + what the "deletions by name" note has to see. The `renamed` status is gone from the scale line. +- `_numstat`, `_untracked_stat`, `_changed_files`, `parse_porcelain_entries` and `parse_porcelain_z` are + removed; nothing else called them. The "Files changed:" list in the commit message comes from the same + list and is correct for the same reason. +- With `--path`, the list is restricted to that subtree even when more is staged, as the commit is. + +**One message for two states became three.** A failed fetch was reported as "No remote configured, or +origin could not be reached". It is now `no-remote`, `remote-lacks-branch` (the remote answers and has no +such branch yet - the first publish of an instance) or `unreachable`, told apart by `git remote get-url` +and the exit code of `git ls-remote --exit-code`, each with its own message. `sync` exits 0 in all three. + +**`publish` stops before the commit when it cannot publish.** Without `--no-push`, `unreachable` and +`no-remote` end the call with exit 1 at the reconcile - before the gate, `git add` and the commit, and +also on a clean tree, where it used to say "Nothing to commit". It used to commit and fail at the push, +which left a commit that only a hand-made `git push` could send. The messages name `--no-push` as the +way to a local commit; the next `publish` that reaches the remote sends that commit along. `remote-lacks-branch` +is unaffected, so the first publish of an instance still commits and pushes. The Publish-Remote Gate, +which runs first when `.wikitool-remotes.json` exists, is unchanged, and so is the retry after a rejected +push: a remote that has become unreachable by then reports the original push error. + +This is a **breaking** change in the sense of the version model: an offline session, or an instance that +stays local, has to pass `--no-push` on every `publish`. No content changes, so `**Migration:**` stays +"none required". `instructions/publish-cycle.md` has the new decision point, `setup-instance.md` step 4 +and `INSTALL.md` say what a local-only instance now sees without the flag. + ### bug-report instruction: step 1 no longer calls every bundle unpseudonymised Step 1 told the agent to announce that the bundle "is not pseudonymised" and then, one paragraph diff --git a/INSTALL.md b/INSTALL.md index 2f95a76..83f0002 100644 --- a/INSTALL.md +++ b/INSTALL.md @@ -67,7 +67,8 @@ Zwei Schritte, von denen nur der erste rein menschlich ist: anderen Repo übernommen, und ist zugleich der Autorname jeder künftig angelegten Wiki-Seite (`$WIKI_AUTHOR` überschreibt dies bei Bedarf). - **Remote** (optional) - eine URL, wenn du das Repo auf einen Server pushen willst; sonst - bleibt die Instanz lokal, und jedes `publish` läuft mit `--no-push`. + bleibt die Instanz lokal, und jedes `publish` läuft mit `--no-push` - ohne das Flag + bricht `publish` mit Exit 1 ab, bevor es committet. - **Autorenkonventionen** - Sprache, Abschnittsnamen, Namensformen, Ton, Beziehungslabels und Hedging-Regel stehen in `kb/CONVENTIONS.md`, dazu je Collection die Regeln in `kb//COLLECTION.md`. Die Distribution bringt davon nur die `.template`-Dateien mit: diff --git a/VERSION b/VERSION index c5f6633..d3d1a59 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -8.0.0-beta.9 +8.0.0-beta.10 diff --git a/instructions/publish-cycle.md b/instructions/publish-cycle.md index 6e5751a..b0eec12 100644 --- a/instructions/publish-cycle.md +++ b/instructions/publish-cycle.md @@ -47,6 +47,12 @@ consistent. - **Ten or more files changed?** `publish` exits 42. Show the user its output and stop; see [gates.md](gates.md). +- **Remote unreachable or not configured?** `publish` ends with exit 1 before it commits: + nothing is staged, committed or pushed, and the message names the remote. Ask the user whether + to commit locally with `--no-push`, and run that only on their answer. Never push by hand + (AGENTS.md invariant 5): the next `publish` that reaches the remote sends the local commit + together with whatever is new. A local-only instance, which has no remote at all, passes + `--no-push` on every call ([setup-instance.md](setup-instance.md), step 4). - **Query or lint pass?** Neither auto-publishes. Run `publish` only if asked to. - **Nothing under `kb/` changed?** Skip steps 1 and 2; a change to `tools/` or `instructions/` does not affect the catalog. diff --git a/instructions/setup-instance.md b/instructions/setup-instance.md index e1e54b4..088ccda 100644 --- a/instructions/setup-instance.md +++ b/instructions/setup-instance.md @@ -66,7 +66,8 @@ and ready for its first ingest. end state: - Given: `git remote add origin ` - Not given: stay local - then **every** later `tools/wikitool publish` needs a `--no-push` - (which also drops its branch check, see step 2). + (which also drops its branch check, see step 2). Without it, `publish` ends with exit 1 + before it commits anything, because there is no remote to publish to. 5. **Decision point - authoring conventions.** The distribution ships no filled-in conventions, only `kb/CONVENTIONS.md.template` and one `kb//COLLECTION.md.template` per collection. diff --git a/tools/CONTRACT.md b/tools/CONTRACT.md index 7c062e8..0a14480 100644 --- a/tools/CONTRACT.md +++ b/tools/CONTRACT.md @@ -1640,7 +1640,8 @@ Fetch `/` and bring the local branch up to date with it. **EXIT STATUS** - 0 success -- 0 No remote configured, or the remote cannot be reached - reported and skipped, not a failure +- 0 No remote configured - reported and skipped, not a failure +- 0 The remote cannot be reached - reported and skipped, not a failure - 1 The automatic rebase hit a real conflict (git failed); it is aborted cleanly - 42 Rebase-review gate: `/` moved and both sides changed the same file; the output lists the upstream commits, the overlapping files and their diff @@ -1660,6 +1661,7 @@ Fetch `/` and bring the local branch up to date with it. - A refused call performs no rebase attempt and leaves the branch where it was. - The `--confirm-rebase` token covers the exact upstream state and the set of files touched on both sides; either one moving makes it stale. - Makes no commit, no push, and no forced operation of any kind. +- Three messages for a fetch that fails: no remote of that name, a remote that answers but has no such branch yet (a new, empty repository), and a remote that cannot be reached. All three exit 0 here; `publish` stops on the first and the last. - Run it once at the start of a writing session. **SEE ALSO** @@ -1696,6 +1698,7 @@ Reconcile with `/`, then stage all changes, commit, and push. - 0 success - 1 git failed - `git add`, `git commit`, `git push`, or the reconcile's automatic rebase - 1 The push target (`--branch`) is not the checked-out branch, or HEAD is detached; the unborn branch of a fresh `git init` is not this case +- 1 No `--no-push`, and the remote is not configured or cannot be reached; nothing was committed - 1 `--yes`/`-y` was passed - the flag does not exist and fails with an explicit error - 1 `.wikitool-remotes.json` is unreadable or has no usable `allowed_push_urls` list - 42 Mass-Update Gate: `--threshold` (default 10) or more counted files would be committed, or the `--confirm` token does not match this changeset @@ -1706,6 +1709,7 @@ Reconcile with `/`, then stage all changes, commit, and push. - git failed - `git add`, `git commit`, `git push`, or the reconcile's automatic rebase -> Do not retry and do not force - report and ask the user. `publish` has already made its one retry of a rejected push itself, where a reconcile resolved the rejection - The push target (`--branch`) is not the checked-out branch, or HEAD is detached; the unborn branch of a fresh `git init` is not this case -> Check out the branch you mean to publish, or pass `--branch `, then retry once +- No `--no-push`, and the remote is not configured or cannot be reached; nothing was committed -> Show the message to the user and ask whether to commit locally with `--no-push`. Never push by hand - the next `publish` that reaches the remote sends that commit - `--yes`/`-y` was passed - the flag does not exist and fails with an explicit error -> Drop it. The Mass-Update Gate is cleared only with `--confirm ` from the gate's own refusal output - `.wikitool-remotes.json` is unreadable or has no usable `allowed_push_urls` list -> Show the error to the user and stop - a malformed file is not permission, and fixing or deleting it is theirs to do - Mass-Update Gate: `--threshold` (default 10) or more counted files would be committed, or the `--confirm` token does not match this changeset -> Show the user the command's full output verbatim and stop. Once they have approved it, run the re-run line the output prints, which carries `--confirm `. Without that token, or with a wrong, invented or superseded one, it exits 42 again with the current state @@ -1721,14 +1725,15 @@ Reconcile with `/`, then stage all changes, commit, and push. **NOTES** - Order: branch check and Publish-Remote Gate, then the reconcile with `/`, then the Mass-Update Gate, then `git add -A`, commit and push. `--no-push` skips all but the Mass-Update Gate and the commit. +- Without `--no-push`, a remote that is not configured or cannot be reached ends the call with exit 1 at the reconcile - before the gate, `git add` and the commit, and also on a clean tree. Nothing is committed, the index and the working tree are unchanged, and the message names `--no-push` as the way to a local commit. The next `publish` that reaches the remote pushes that commit along with whatever is new. A remote that answers but has no `` yet (a new, empty repository) is not this case: the first publish of an instance commits and pushes as before. - Reconcile: fetches `/`, fast-forwards when only the remote moved, rebases the local commits on top when both sides moved but touched disjoint files, and exits 42 (rebase-review gate) when both sides touched the same file. A refused reconcile performs no rebase attempt. The `--confirm-rebase` token covers the exact upstream state and the set of files touched on both sides. - The push target must be the checked-out branch; this is checked before anything is staged. The unborn branch of a fresh `git init -b main` counts as checked out, so the first publish of a new instance works; a real detached HEAD is refused. - With nothing new to stage, a local commit the remote lacks is still pushed: one left behind by an earlier publish whose push failed, or every commit when the remote answers but does not have the branch yet (a new, empty remote repository). -- A remote that cannot be reached is not read as lacking the branch: on a clean tree `publish` reports "Nothing to commit" and attempts no push. +- A rejected push that finds the remote unreachable on its one retry reports the original push error. - A rejected push gets exactly one more reconcile-and-push; never more than one. -- Mass-Update Gate: counts the files that would be committed, refuses with exit 42 at `--threshold` (default 10) or more, and prints a review report - a scale line (file count, total lines added/removed, status breakdown), attention notes where they apply (deletions by name, control-plane and harness-config touches, published pages, the largest single change, binaries), and every counted path grouped by area with its status and churn. The gate is evaluated before anything is staged, so a refused publish leaves the working tree untouched. +- Mass-Update Gate: counts the files that would be committed, refuses with exit 42 at `--threshold` (default 10) or more, and prints a review report - a scale line (file count, total lines added/removed, status breakdown), attention notes where they apply (deletions by name, control-plane and harness-config touches, published pages, the largest single change, binaries), and every counted path grouped by area with its status and churn. The list is what the commit will hold: it is computed from a scratch copy of the index after `git add -A`, so a path that is staged as deleted and back in the working tree is not counted twice, and a rename counts as its old path deleted plus its new path added. The real index and the working tree are not touched, so a refused publish leaves both byte-identical. - Never counted and never shown for approval, but committed like everything else: anything under `work/`, and the files `wikitool` generates itself (`kb/index.md`, `kb/log.md`, `kb/provenance.md`, every `INDEX.md`). The refusal line accounts for both, by reason. -- The `--confirm` token covers each counted path, its contents and the publish target: a different file list or edited contents need a new clearance. +- 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. - 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. diff --git a/tools/chemenu/commands/git_publish.py b/tools/chemenu/commands/git_publish.py index 3782e1d..185f235 100644 --- a/tools/chemenu/commands/git_publish.py +++ b/tools/chemenu/commands/git_publish.py @@ -49,9 +49,13 @@ from __future__ import annotations import hashlib import json +import os import shlex +import shutil import subprocess +import tempfile from dataclasses import dataclass, field +from pathlib import Path from typing import NamedTuple, Optional import typer @@ -160,48 +164,17 @@ def publish_remote_refusal(remote: str, branch: str) -> Optional[str]: ) -def parse_porcelain_entries(stdout: str) -> list[tuple[str, str]]: - """Parse `git status --porcelain -z` output into (status_code, path) pairs. - - NUL-delimited output is used instead of line-splitting because it is the - only form that survives paths containing spaces, quotes, or newlines - (line mode quotes and escapes them instead). Rename/copy entries carry a - second NUL-separated field with the original path; the new path is what - gets committed, so the original is consumed and discarded. - """ - fields = [f for f in stdout.split("\0") if f != ""] - entries: list[tuple[str, str]] = [] - index = 0 - while index < len(fields): - entry = fields[index] - index += 1 - if len(entry) < 4: - continue - status_code, path = entry[:2], entry[3:] - if status_code[0] in ("R", "C") or status_code[1] in ("R", "C"): - index += 1 # skip the original path of a rename/copy - entries.append((status_code, path)) - return entries +# The status letters `git diff --cached --raw` emits under `--no-renames` (renames and copies +# are switched off, so neither R nor C can appear). Anything else - M, and T for a type change - +# is a modification. +_STATUS_WORDS = {"A": "added", "D": "deleted"} -def parse_porcelain_z(stdout: str) -> list[str]: - """Just the paths - what the gate counts and what `git add -A` would stage.""" - return [path for _, path in parse_porcelain_entries(stdout)] - - -def describe_status(status_code: str) -> str: - """Porcelain XY code -> the word a reviewer needs. Deletions and additions - are what a reader scans for first, so they must not be flattened into a - generic "changed".""" - if status_code == "??": - return "added" - if "D" in status_code: - return "deleted" - if "R" in status_code or "C" in status_code: - return "renamed" - if "A" in status_code: - return "added" - return "modified" +def describe_status(letter: str) -> str: + """`git diff --raw` status letter -> the word a reviewer needs. Deletions and additions + are what a reader scans for first, so they must not be flattened into a generic + "changed".""" + return _STATUS_WORDS.get(letter, "modified") class FileChange(NamedTuple): @@ -230,123 +203,96 @@ class FileChange(NamedTuple): return f"+{self.added}/-{self.removed}" -def _numstat(paths: list[str]) -> dict[str, tuple[int, int]]: - """Lines added/removed per tracked file, against HEAD. - - `git diff HEAD` covers staged and unstaged changes together, which is what - `publish` is about to commit. Untracked files are absent from it and are - measured by reading them instead. A repository with no commits yet has no - HEAD to diff against - that is the first-commit case from - `instructions/setup-instance.md`, where everything is untracked anyway - so - a failure here is normal and yields no entries rather than an error. - - `-z` is required, not a preference: without it git renders a path with any - non-ASCII byte in quoted form ("kb/W\\303\\266rterbuch.md"), while - `_changed_files` reads raw paths from `git status --porcelain -z`. The two - then never match, the caller's lookup misses, and the file is measured as - though git had never seen it - every line an addition, no removals. A - rewritten page reported as a pure insertion hides exactly what a reviewer - is being asked to approve. - """ - result = _run(["git", "diff", "--numstat", "-z", "HEAD", "--", *paths]) +def _git_path(*args: str) -> str: + result = _run(["git", "rev-parse", "--git-path", *args]) if result.returncode != 0: - return {} - stats: dict[str, tuple[int, int]] = {} - # With -z each record is "added\tremoved\tpath" terminated by NUL. A rename - # or copy leaves the path empty and follows with two more records, the old - # and the new path; the new one is what `git status` reports. - records = result.stdout.split("\0") - index = 0 - while index < len(records): - record = records[index] - index += 1 - if not record: + fail(f"git rev-parse failed:\n{result.stderr}") + return result.stdout.strip() + + +def _parse_raw(stdout: str) -> list[tuple[str, str, str]]: + """`git diff --raw -z --no-renames --no-abbrev` -> (status letter, new blob id, path). + + Each record is ": " and, after a NUL, the + path. NUL-delimited output is the only form that survives a path with a space, a quote or + a non-ASCII byte, which line mode quotes and escapes instead. + """ + fields = [f for f in stdout.split("\0") if f != ""] + entries: list[tuple[str, str, str]] = [] + for index in range(0, len(fields) - 1, 2): + meta = fields[index].lstrip(":").split(" ") + if len(meta) < 5: continue + entries.append((meta[4][0], meta[3], fields[index + 1])) + return entries + + +def _parse_numstat(stdout: str) -> dict[str, tuple[int, int]]: + """`git diff --numstat -z --no-renames` -> path -> (added, removed); (-1, -1) for a binary + file, for which git writes "-" in both columns.""" + stats: dict[str, tuple[int, int]] = {} + for record in stdout.split("\0"): fields = record.split("\t") if len(fields) < 3: continue added, removed, path = fields[0], fields[1], fields[2] - if not path: - if index + 1 >= len(records): - continue - path = records[index + 1] - index += 2 - # git writes "-" for both counts on a binary file. stats[path] = (-1, -1) if added == "-" else (int(added), int(removed)) return stats -def _untracked_stat(path: str) -> tuple[int, int, str]: - """(added, removed, digest) for a file git has never seen: every line is an - addition. Anything unreadable as UTF-8 counts as binary rather than - guessing at a line count.""" - full = config.ROOT / path - try: - data = full.read_bytes() - except OSError: - return (-1, -1, "") - digest = hashlib.sha256(data).hexdigest()[:16] - try: - text = data.decode("utf-8") - except UnicodeDecodeError: - return (-1, -1, digest) - return (len(text.splitlines()), 0, digest) - - -def _digest_of(path: str) -> str: - """A short content digest of the working-tree file, or "" if it is gone. - - This is what binds a clearance to file *contents* and not merely to file - *names*: without it, approving a list and then rewriting one of those files - before confirming would still publish, which is the same "approved A, - published B" hole the token exists to close. - """ - try: - return hashlib.sha256((config.ROOT / path).read_bytes()).hexdigest()[:16] - except OSError: - return "" - - def collect_changes(paths: list[str]) -> list[FileChange]: - """Every path `git add -A` would stage, with its status, churn and content - digest. Ordered by path so the result is stable.""" - result = _run(["git", "status", "--porcelain", "-z", "-uall", "--", *paths]) - if result.returncode != 0: - fail(f"git status failed:\n{result.stderr}") - entries = parse_porcelain_entries(result.stdout) - numstat = _numstat(paths) + """Every path `git add -A -- ` would put into the commit, with its status, churn + and content digest. Ordered by path so the result is stable. + + The list is computed from the state *after* staging, which is the only state the commit + sees: `git status` reports the index and the working tree separately, so a path that is + staged as deleted and sits in the working tree again appears twice and cancels out at + `git add -A`. The staging happens in a copy of the index, so a refused publish leaves the + real index - and the working tree - byte-identical. `git add` does write the new blobs into + the object store; unreferenced ones are `gc`'s to collect. + + The digest is the blob id of the staged content, so a clearance token is bound to exactly + what is committed. A deletion has no content and therefore no digest. `--no-renames` lists + a rename as its old path (deleted) plus its new one (added), which is what the commit holds + and what the attention note on deletions has to see. + """ + index_path = config.ROOT / _git_path("index") + scratch = Path(tempfile.mkdtemp(prefix="wikitool-index-")) + try: + temp_index = scratch / "index" + if index_path.is_file(): + shutil.copyfile(index_path, temp_index) + env = {**os.environ, "GIT_INDEX_FILE": str(temp_index)} + + def run(args: list[str]) -> str: + result = subprocess.run( + ["git", *args], cwd=config.ROOT, capture_output=True, text=True, env=env, + ) + if result.returncode != 0: + fail(f"git {args[0]} failed:\n{result.stderr}") + return result.stdout + + run(["add", "-A", "--", *paths]) + diff = ["diff", "--cached", "--no-renames", "-z"] + raw = _parse_raw(run([*diff, "--raw", "--no-abbrev", "--", *paths])) + numstat = _parse_numstat(run([*diff, "--numstat", "--", *paths])) + finally: + shutil.rmtree(scratch, ignore_errors=True) changes: list[FileChange] = [] - for status_code, path in entries: - status = describe_status(status_code) + for letter, blob, path in raw: + status = describe_status(letter) + added, removed = numstat.get(path, (0, 0)) if status == "deleted": - # A deletion's churn is every line the file had, and git already - # knows it. Short-circuiting to 0/0 here (the first version of this - # code) silently understated the headline: one 718-line file went - # out reported as `-174` against git's own `-891`, hiding four - # fifths of the removals in the one direction a reviewer most needs - # not understated. The digest stays empty - there is no content - # left to fingerprint - which is itself what moves the token. - _, removed = numstat.get(path, (0, 0)) + # A deletion's churn is every line the file had, and git already knows it: + # reporting 0 here once hid four fifths of the removals in the one direction a + # reviewer most needs them not understated. changes.append(FileChange(path, status, 0, max(removed, 0), "")) - elif path in numstat: - added, removed = numstat[path] - changes.append(FileChange(path, status, added, removed, _digest_of(path))) else: - added, removed, digest = _untracked_stat(path) - changes.append(FileChange(path, status, added, removed, digest)) + changes.append(FileChange(path, status, added, removed, blob)) return sorted(changes, key=lambda change: change.path) -def _changed_files(paths: list[str]) -> list[str]: - """Every path `git add -A` would stage, including untracked files, - optionally restricted to a pathspec.""" - result = _run(["git", "status", "--porcelain", "-z", "-uall", "--", *paths]) - if result.returncode != 0: - fail(f"git status failed:\n{result.stderr}") - return parse_porcelain_z(result.stdout) - - def current_branch() -> Optional[str]: """The checked-out branch, or None in a detached HEAD / non-checkout. @@ -549,7 +495,7 @@ def scale_line(changes: list[FileChange]) -> str: by_status[change.status] = by_status.get(change.status, 0) + 1 breakdown = ", ".join( f"{by_status[status]} {status}" - for status in ("added", "modified", "renamed", "deleted") + for status in ("added", "modified", "deleted") if by_status.get(status) ) return ( @@ -753,12 +699,20 @@ def remote_ref_exists(remote: str, branch: str) -> bool: def fetch_remote(remote: str, branch: str) -> bool: """`git fetch `, true on success. A failure here (no remote configured, - network/auth, or a branch that does not exist on the remote yet) is never fatal on its own - - every caller falls back to today's behaviour and lets the eventual `git push` report the - real error, so an offline or brand-new instance sees no new failure mode.""" + network/auth, or a branch that does not exist on the remote yet) is not an answer by itself: + `reconcile` tells those three apart, and only `publish` treats two of them as a reason to + stop.""" return _run(["git", "fetch", remote, branch]).returncode == 0 +def remote_url(remote: str) -> Optional[str]: + """The fetch URL of `remote`, or None when no such remote is configured.""" + result = _run(["git", "remote", "get-url", remote]) + if result.returncode != 0: + return None + return result.stdout.strip() or None + + def divergence(local_ref: str, remote_ref: str) -> str: """Where `local_ref` stands relative to `remote_ref`: "up-to-date", "ff-possible" (remote only, local can fast-forward), "local-ahead" (local only, nothing to pull), or "diverged" @@ -811,8 +765,9 @@ def rebase_review_token(remote: str, branch: str, local_before: str, remote_tip: @dataclass class ReconcileOutcome: """What happened when the local branch was brought up to date with the remote before a - publish/sync. `status` is one of: "no-remote-or-fetch-failed", "up-to-date", - "fast-forwarded", "local-ahead", "rebased", "needs-review", "conflict".""" + publish/sync. `status` is one of: "no-remote", "remote-lacks-branch", "unreachable", + "up-to-date", "fast-forwarded", "local-ahead", "rebased", "needs-review", "conflict". + The first three are the ways the fetch can fail; `detail` then carries the remote's URL.""" status: str pulled_commits: list[str] = field(default_factory=list) overlap_files: list[str] = field(default_factory=list) @@ -835,7 +790,14 @@ def reconcile(remote: str, branch: str, confirm_rebase: Optional[str] = None) -> job) and `publish` (proactively before staging, and once more if the eventual push is rejected - the real, narrow race this whole module exists to close).""" if not fetch_remote(remote, branch) or not remote_ref_exists(remote, branch): - return ReconcileOutcome(status="no-remote-or-fetch-failed") + url = remote_url(remote) + if url is None: + return ReconcileOutcome(status="no-remote") + # `remote_lacks_branch` reads the ls-remote exit code: 2 means the remote answered and + # has no such branch (a new, empty repository), anything else that it did not answer. + if remote_lacks_branch(remote, branch): + return ReconcileOutcome(status="remote-lacks-branch", detail=url) + return ReconcileOutcome(status="unreachable", detail=url) remote_ref = f"{remote}/{branch}" state = divergence(branch, remote_ref) @@ -916,9 +878,9 @@ def _local_ahead_of_remote(remote: str, branch: str) -> bool: # No tracking ref, which `remote_ref_exists` cannot tell apart from an unreachable # remote - and the very first publish of an instance lands here. A remote that answers # and simply has no such branch yet means every local commit is unpushed, which is - # precisely the stranded state above; an unreachable one keeps the old answer, so an - # offline or local-only instance sees no new behaviour and the eventual `git push` - # (when there is something to stage) still reports the real error. + # precisely the stranded state above. An unreachable or missing remote never gets this + # far in `publish`, which stops on it before the commit; the answer here stays exact + # for a remote that goes away between the reconcile and this call. return remote_lacks_branch(remote, branch) and _has_commits(branch) result = _run(["git", "rev-list", "--count", f"{remote}/{branch}..{branch}"]) return result.returncode == 0 and result.stdout.strip() not in ("", "0") @@ -962,11 +924,39 @@ def _reconcile_summary(outcome: ReconcileOutcome, remote: str, branch: str) -> s return f"Rebased onto {len(outcome.pulled_commits)} new commit(s) from {remote}/{branch}{reviewed}." if outcome.status == "up-to-date": return f"Already up to date with {remote}/{branch}." - if outcome.status == "no-remote-or-fetch-failed": - return f"No remote configured, or {remote} could not be reached - continuing without a pull." + if outcome.status == "no-remote": + return f"No remote '{remote}' configured - nothing to pull." + if outcome.status == "remote-lacks-branch": + return f"{remote} has no branch '{branch}' yet - nothing to pull." + if outcome.status == "unreachable": + return f"{remote} ({outcome.detail}) could not be reached - nothing pulled." return "" +def publish_stop_message(outcome: ReconcileOutcome, remote: str, branch: str) -> Optional[str]: + """The ERROR `publish` ends with, before gate, staging and commit, when the remote it was + asked to publish to is missing or cannot be reached - else None. + + `publish` without `--no-push` means "publish", and that is not possible. Committing anyway + left a commit that only a hand-made `git push` could send, which invariant 5 rules out, so + the stop comes first and names the way to a local commit. + """ + if outcome.status == "unreachable": + return ( + f"Cannot publish: {remote} ({outcome.detail}) could not be reached. Nothing was " + "committed or pushed. To commit locally in the meantime, run the same `publish` " + f"with `--no-push`; the next `publish` that reaches {remote} pushes that commit " + "together with whatever is new." + ) + if outcome.status == "no-remote": + return ( + f"Cannot publish: this checkout has no remote named '{remote}'. Nothing was " + "committed or pushed. A local-only instance calls every `publish` with `--no-push` " + "(instructions/setup-instance.md, step 4); to publish, add the remote first." + ) + return None + + def apply_reconcile(outcome: ReconcileOutcome, remote: str, branch: str, command: str) -> str: """Turn a `ReconcileOutcome` into this module's fail/needs_clearance contract: raises via `fail()` on `conflict`, raises via `needs_clearance()` on `needs-review` (emitting the same @@ -1021,12 +1011,19 @@ def apply_reconcile(outcome: ReconcileOutcome, remote: str, branch: str, command "The `--confirm-rebase` token covers the exact upstream state and the set of files " "touched on both sides; either one moving makes it stale.", "Makes no commit, no push, and no forced operation of any kind.", + "Three messages for a fetch that fails: no remote of that name, a remote that answers " + "but has no such branch yet (a new, empty repository), and a remote that cannot be " + "reached. All three exit 0 here; `publish` stops on the first and the last.", "Run it once at the start of a writing session.", ), failures=( cli_contract.Failure( - cause="No remote configured, or the remote cannot be reached - reported and " - "skipped, not a failure", + cause="No remote configured - reported and skipped, not a failure", + reaction="", + code=0, + ), + cli_contract.Failure( + cause="The remote cannot be reached - reported and skipped, not a failure", reaction="", code=0, ), @@ -1099,6 +1096,13 @@ def sync_command( "Order: branch check and Publish-Remote Gate, then the reconcile with " "`/`, then the Mass-Update Gate, then `git add -A`, commit and push. " "`--no-push` skips all but the Mass-Update Gate and the commit.", + "Without `--no-push`, a remote that is not configured or cannot be reached ends the " + "call with exit 1 at the reconcile - before the gate, `git add` and the commit, and " + "also on a clean tree. Nothing is committed, the index and the working tree are " + "unchanged, and the message names `--no-push` as the way to a local commit. The next " + "`publish` that reaches the remote pushes that commit along with whatever is new. A " + "remote that answers but has no `` yet (a new, empty repository) is not this " + "case: the first publish of an instance commits and pushes as before.", "Reconcile: fetches `/`, fast-forwards when only the remote moved, " "rebases the local commits on top when both sides moved but touched disjoint files, " "and exits 42 (rebase-review gate) when both sides touched the same file. A refused " @@ -1110,22 +1114,25 @@ def sync_command( "With nothing new to stage, a local commit the remote lacks is still pushed: one left " "behind by an earlier publish whose push failed, or every commit when the remote " "answers but does not have the branch yet (a new, empty remote repository).", - "A remote that cannot be reached is not read as lacking the branch: on a clean tree " - "`publish` reports \"Nothing to commit\" and attempts no push.", + "A rejected push that finds the remote unreachable on its one retry reports the " + "original push error.", "A rejected push gets exactly one more reconcile-and-push; never more than one.", "Mass-Update Gate: counts the files that would be committed, refuses with exit 42 at " "`--threshold` (default 10) or more, and prints a review report - a scale line (file " "count, total lines added/removed, status breakdown), attention notes where they apply " "(deletions by name, control-plane and harness-config touches, published pages, the " "largest single change, binaries), and every counted path grouped by area with its " - "status and churn. The gate is evaluated before anything is staged, so a refused " - "publish leaves the working tree untouched.", + "status and churn. The list is what the commit will hold: it is computed from a scratch " + "copy of the index after `git add -A`, so a path that is staged as deleted and back in " + "the working tree is not counted twice, and a rename counts as its old path deleted " + "plus its new path added. The real index and the working tree are not touched, so a " + "refused publish leaves both byte-identical.", "Never counted and never shown for approval, but committed like everything else: " "anything under `work/`, and the files `wikitool` generates itself (`kb/index.md`, " "`kb/log.md`, `kb/provenance.md`, every `INDEX.md`). The refusal line accounts for " "both, by reason.", - "The `--confirm` token covers each counted path, its contents and the publish target: " - "a different file list or edited contents need a new clearance.", + "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 " @@ -1154,6 +1161,13 @@ def sync_command( reaction="Check out the branch you mean to publish, or pass `--branch `, then retry once", ), + cli_contract.Failure( + cause="No `--no-push`, and the remote is not configured or cannot be reached; " + "nothing was committed", + reaction="Show the message to the user and ask whether to commit locally with " + "`--no-push`. Never push by hand - the next `publish` that reaches the remote " + "sends that commit", + ), cli_contract.Failure( cause="`--yes`/`-y` was passed - the flag does not exist and fails with an " "explicit error", @@ -1292,6 +1306,9 @@ def publish_command( # a deliberate local-only commit. if push: outcome = reconcile(remote, branch, confirm_rebase) + stop = publish_stop_message(outcome, remote, branch) + if stop: + fail(stop) summary = apply_reconcile(outcome, remote, branch, "publish") if summary: typer.echo(summary) diff --git a/tools/chemenu/tests/test_git_publish.py b/tools/chemenu/tests/test_git_publish.py index e4d7c78..ab97ba5 100644 --- a/tools/chemenu/tests/test_git_publish.py +++ b/tools/chemenu/tests/test_git_publish.py @@ -24,8 +24,6 @@ from chemenu.commands.git_publish import ( format_changes, group_of, is_generated, - parse_porcelain_entries, - parse_porcelain_z, publish_command, reconcile, remote_lacks_branch, @@ -50,21 +48,6 @@ def test_default_threshold_matches_farzas_rule(): assert DEFAULT_MASS_UPDATE_THRESHOLD == 10 -def test_porcelain_parsing_handles_paths_with_spaces(): - stdout = " M kb/concepts/Hybrid Search.md\0?? kb/Lint Report 2026-08-13.md\0" - assert parse_porcelain_z(stdout) == [ - "kb/concepts/Hybrid Search.md", - "kb/Lint Report 2026-08-13.md", - ] - - -def test_porcelain_parsing_reports_the_new_path_of_a_rename(): - """Rename entries carry the original path in a second NUL field; only the - new path is what actually gets committed.""" - stdout = "R kb/concepts/New Name.md\0kb/concepts/Old Name.md\0 M AGENTS.md\0" - assert parse_porcelain_z(stdout) == ["kb/concepts/New Name.md", "AGENTS.md"] - - def test_branch_mismatch_names_both_branches_and_the_fix(): """Regression guard: `git push origin main` from a feature branch pushes the ref named `main` - an unrelated, usually unchanged commit - and exits 0, so @@ -81,10 +64,6 @@ def test_branch_mismatch_handles_detached_head(): assert "--branch None" not in message -def test_porcelain_parsing_of_empty_status_is_empty(): - assert parse_porcelain_z("") == [] - - def _msg(changed, threshold=10, token="tok123456789", stale=None): """`changed` may be paths (convenience) or FileChange records.""" records = [fc(c) if isinstance(c, str) else c for c in changed] @@ -509,20 +488,11 @@ def test_the_clearance_request_emits_a_matchable_token(repo): # --- grouping and review hints --- -def test_status_words_come_from_the_porcelain_code(): - assert describe_status("??") == "added" - assert describe_status("A ") == "added" - assert describe_status(" D") == "deleted" - assert describe_status("D ") == "deleted" - assert describe_status("R ") == "renamed" - assert describe_status(" M") == "modified" - - -def test_porcelain_entries_keep_the_status_alongside_the_path(): - stdout = " M kb/a.md\0?? kb/b.md\0 D kb/c.md\0" - assert parse_porcelain_entries(stdout) == [ - (" M", "kb/a.md"), ("??", "kb/b.md"), (" D", "kb/c.md"), - ] +def test_status_words_come_from_the_raw_status_letter(): + assert describe_status("A") == "added" + assert describe_status("D") == "deleted" + assert describe_status("M") == "modified" + assert describe_status("T") == "modified" def test_generated_files_are_recognised_wherever_they_sit(): @@ -1056,10 +1026,8 @@ def test_rebase_review_gate_emits_matchable_telemetry(repo): def test_numstat_survives_a_non_ascii_filename(repo): - """`git status --porcelain -z` emits raw paths, but `git diff --numstat` - quotes non-ASCII ones ("ausw\\303\\274rfeln"). When the two disagree the - numstat lookup misses and the file falls through to the untracked path, - which reports every line as an addition - a rewrite shown as a pure + """`git diff --numstat -z` and `--raw -z` emit raw paths, which is what lets the churn of + a path with a non-ASCII byte be matched to its status: a rewrite must not read as a pure insertion, hiding the removals a reviewer most needs to see.""" name = "kb/Wörterbuch.md" (repo / name).write_text("eins\nzwei\ndrei\n", encoding="utf-8") @@ -1073,6 +1041,246 @@ def test_numstat_survives_a_non_ascii_filename(repo): assert (change.added, change.removed) == (1, 2) +# --- the gate lists the staged state, from a scratch index -------------------- + + +def _index_bytes(root): + path = root / _git(root, "rev-parse", "--git-path", "index").stdout.strip() + return path.read_bytes() if path.is_file() else None + + +def _status(root): + return _git(root, "status", "--porcelain", "-uall").stdout + + +def _stage_a_deletion_and_restore_the_file(repo): + """The state a `git rm -r raw` followed by `git restore --source=HEAD -- raw/CONTRACT.md` + leaves: one path staged as deleted *and* present in the working tree.""" + (repo / "raw").mkdir() + (repo / "raw/CONTRACT.md").write_text("c\n", encoding="utf-8") + (repo / "raw/a.md").write_text("x\n", encoding="utf-8") + _git(repo, "add", "-A") + _git(repo, "commit", "-m", "raw") + _git(repo, "rm", "-rq", "raw") + _git(repo, "restore", "--source=HEAD", "--", "raw/CONTRACT.md") + + +def test_the_gate_counts_a_path_once_when_it_is_staged_deleted_and_back_in_the_tree(repo): + _stage_a_deletion_and_restore_the_file(repo) + # The premise: status really does list raw/CONTRACT.md twice. + assert _status(repo).count("raw/CONTRACT.md") == 2 + + changes = collect_changes([]) + + assert [(c.path, c.status) for c in changes] == [("raw/a.md", "deleted")] + + +def test_the_listed_paths_are_the_paths_the_commit_holds(repo): + _stage_a_deletion_and_restore_the_file(repo) + (repo / "README.md").write_text("init\nmore\n", encoding="utf-8") + (repo / "kb/new.md").write_text("new\n", encoding="utf-8") + (repo / "kb/moved.md").write_text("moved\n", encoding="utf-8") + _git(repo, "add", "-A") + _git(repo, "commit", "-m", "moved") + (repo / "kb/moved.md").rename(repo / "kb/renamed.md") + + listed = sorted(c.path for c in collect_changes([])) + _publish(message="check the list") + + committed = _git(repo, "show", "--name-only", "--no-renames", "--format=", "HEAD").stdout + assert listed == sorted(committed.split()) + assert "kb/moved.md" in listed and "kb/renamed.md" in listed + + +def test_a_rename_is_listed_as_its_old_path_deleted_and_its_new_path_added(repo): + (repo / "kb/old.md").write_text("content\n", encoding="utf-8") + _git(repo, "add", "-A") + _git(repo, "commit", "-m", "old") + (repo / "kb/old.md").rename(repo / "kb/new.md") + _git(repo, "add", "-A") # staged, so git status would report an R entry + + by_path = {c.path: c.status for c in collect_changes([])} + + assert by_path == {"kb/old.md": "deleted", "kb/new.md": "added"} + + +def test_the_digest_is_the_blob_id_of_what_gets_committed(repo): + (repo / "kb/page.md").write_text("page\n", encoding="utf-8") + digest = {c.path: c.digest for c in collect_changes([])}["kb/page.md"] + _publish(message="page") + assert digest == _git(repo, "rev-parse", "HEAD:kb/page.md").stdout.strip() + + +def test_a_path_scope_lists_only_that_subtree_even_when_more_is_staged(repo): + (repo / "kb/in.md").write_text("in\n", encoding="utf-8") + (repo / "other.md").write_text("out\n", encoding="utf-8") + _git(repo, "add", "other.md") + + assert [c.path for c in collect_changes(["kb"])] == ["kb/in.md"] + + +def test_a_refused_publish_leaves_index_and_status_byte_identical(repo): + _stage_a_deletion_and_restore_the_file(repo) + _write_files(repo, 10) + index_before, status_before = _index_bytes(repo), _status(repo) + + with pytest.raises(typer.Exit) as excinfo: + _publish(message="big change") + + assert excinfo.value.exit_code == EXIT_NEEDS_CLEARANCE + assert _index_bytes(repo) == index_before + assert _status(repo) == status_before + + +def test_a_refused_first_publish_leaves_a_missing_index_missing(fresh_instance): + """No index file at all yet: the scratch index starts empty and the real one must not + appear as a side effect.""" + _write_files(fresh_instance, 10) + assert _index_bytes(fresh_instance) is None + + with pytest.raises(typer.Exit) as excinfo: + _publish(message="first") + + assert excinfo.value.exit_code == EXIT_NEEDS_CLEARANCE + assert _index_bytes(fresh_instance) is None + + +def test_a_failing_computation_leaves_the_index_alone_and_removes_its_scratch_copy( + repo, tmp_path, monkeypatch +): + scratch_root = tmp_path / "scratch" + scratch_root.mkdir() + monkeypatch.setattr("tempfile.tempdir", str(scratch_root)) + _write_files(repo, 2) + index_before = _index_bytes(repo) + + with pytest.raises(typer.Exit): + collect_changes(["no/such/path"]) # `git add` refuses a pathspec that matches nothing + + assert _index_bytes(repo) == index_before + assert list(scratch_root.iterdir()) == [] + + +# --- a missing or unreachable remote: three messages, and publish stops early -- + + +def _break_the_remote(repo): + _git(repo, "remote", "set-url", "origin", str(repo.parent / "nope.git")) + + +def _outcomes(repo, mutate): + mutate(repo) + return reconcile("origin", "main", None) + + +def test_the_three_ways_a_fetch_fails_get_three_statuses_and_three_messages(repo): + no_remote = _outcomes(repo, lambda r: _git(r, "remote", "remove", "origin")) + _git(repo, "remote", "add", "origin", str(_remote_for(repo))) + lacks = reconcile("origin", "never-pushed", None) + _break_the_remote(repo) + unreachable = reconcile("origin", "main", None) + + assert (no_remote.status, lacks.status, unreachable.status) == ( + "no-remote", "remote-lacks-branch", "unreachable", + ) + messages = { + git_publish._reconcile_summary(no_remote, "origin", "main"), + git_publish._reconcile_summary(lacks, "origin", "never-pushed"), + git_publish._reconcile_summary(unreachable, "origin", "main"), + } + assert len(messages) == 3 + assert str(repo.parent / "nope.git") in git_publish._reconcile_summary( + unreachable, "origin", "main" + ) + + +def test_sync_ends_with_exit_0_in_all_three_cases(repo, capsys): + _sync(branch="never-pushed") # the remote answers and lacks the branch + _break_the_remote(repo) + _sync() # unreachable + _git(repo, "remote", "remove", "origin") + _sync() # no remote at all + assert capsys.readouterr().out.count("\n") >= 3 + + +def test_publish_without_no_push_stops_before_the_commit_on_an_unreachable_remote(repo): + _break_the_remote(repo) + (repo / "kb/page.md").write_text("page\n", encoding="utf-8") + head, index, status = (_git(repo, "rev-parse", "HEAD").stdout, _index_bytes(repo), _status(repo)) + + with pytest.raises(typer.Exit) as excinfo: + _publish(message="offline") + + assert excinfo.value.exit_code == 1 + assert _git(repo, "rev-parse", "HEAD").stdout == head + assert _index_bytes(repo) == index and _status(repo) == status + + +def test_the_unreachable_stop_names_the_remote_its_url_and_no_push(repo, capsys): + _break_the_remote(repo) + with pytest.raises(typer.Exit): + _publish(message="offline") + err = capsys.readouterr() + text = err.out + err.err + assert "origin" in text and str(repo.parent / "nope.git") in text and "--no-push" in text + + +def test_the_stop_holds_on_a_clean_tree_too(repo): + _break_the_remote(repo) + assert _status(repo) == "" + with pytest.raises(typer.Exit) as excinfo: + _publish(message="nothing to do") + assert excinfo.value.exit_code == 1 + + +def test_publish_without_a_remote_stops_before_the_commit_and_points_at_no_push(repo, capsys): + _git(repo, "remote", "remove", "origin") + (repo / "kb/page.md").write_text("page\n", encoding="utf-8") + head, index, status = (_git(repo, "rev-parse", "HEAD").stdout, _index_bytes(repo), _status(repo)) + + with pytest.raises(typer.Exit) as excinfo: + _publish(message="local") + + assert excinfo.value.exit_code == 1 + assert _git(repo, "rev-parse", "HEAD").stdout == head + assert _index_bytes(repo) == index and _status(repo) == status + captured = capsys.readouterr() + assert "--no-push" in captured.out + captured.err + + +def test_the_stop_holds_without_a_remote_on_a_clean_tree_too(repo): + _git(repo, "remote", "remove", "origin") + with pytest.raises(typer.Exit) as excinfo: + _publish(message="nothing to do") + assert excinfo.value.exit_code == 1 + + +def test_no_push_still_commits_locally_when_the_remote_is_unreachable(repo): + _break_the_remote(repo) + (repo / "kb/page.md").write_text("page\n", encoding="utf-8") + _publish(message="offline", push=False) + assert "kb/page.md" in _git(repo, "show", "--name-only", "HEAD").stdout + + +def test_no_push_still_commits_locally_when_there_is_no_remote(repo): + _git(repo, "remote", "remove", "origin") + (repo / "kb/page.md").write_text("page\n", encoding="utf-8") + _publish(message="local", push=False) + assert "kb/page.md" in _git(repo, "show", "--name-only", "HEAD").stdout + + +def test_a_commit_left_behind_while_offline_goes_out_with_the_next_reachable_publish(repo): + good_url = git_publish.remote_url("origin") + _break_the_remote(repo) + (repo / "kb/offline.md").write_text("offline\n", encoding="utf-8") + _publish(message="offline", push=False) + _git(repo, "remote", "set-url", "origin", good_url) + + _publish(message="back online") + + assert "offline" in _remote_log(_remote_for(repo)) + + # --- Publish-Remote Gate -----------------------------------------------------