diff --git a/AGENTS.md b/AGENTS.md index d5ef52b..3cefa4e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -238,7 +238,8 @@ Every `tools/wikitool` call has exactly four outcomes: produced. After the single allowed retry - or immediately, for the non-idempotent commands `new`, -`log append`, and `publish` - stop and report the exact command and error text to the user. +`log append`, `publish`, and `upstream merge` - stop and report the exact command and error +text to the user. Per-command detail (what exit 1 means, whether the command is atomic, whether a retry is safe) is in [tools/CONTRACT.md](tools/CONTRACT.md). A gate refusal is not a validation error - diff --git a/CHANGES.md b/CHANGES.md index ab5c5a9..4851ebd 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -35,12 +35,13 @@ dev-checkout concern - readable here, never shipped as something to parse. --- -## 4.4.1-beta.1 - 2026-09-03 - Faktenkorrekturen in der 4.4.0-Prosa; DEVELOPMENT.md ohne zweite Kommandobeschreibung +## 4.5.0-beta.1 - 2026-09-04 - wikitool upstream merge/verify: code procedure for taking a stack update, ownership.py as the shared stack/instance boundary **Author:** Torben Nehmer - Faktenkorrekturen in der 4.4.0-Prosa; DEVELOPMENT.md ohne zweite Kommandobeschreibung +- wikitool upstream merge/verify: code procedure for taking a stack update, ownership.py as the shared stack/instance boundary Die Prosa zu 4.4.0 - Changelog-Eintrag, `docs/version-model.md`, `instructions/dev/version-parts.md`, @@ -70,6 +71,52 @@ Kein Verhalten geändert, nur Prosa - und bewusst als laufender Kandidat gelasse CIs Version-Gate verlangt die `VERSION`-Bewegung, ein Release verlangt sie nicht. Genau dafür gibt es das Modell aus 4.4.0. +**Zweiter Bump auf demselben Kandidaten (#30):** `git merge upstream/main` behandelt einen +bewegten Korpus asymmetrisch - eine gelöschte, upstream-geänderte Seite meldet sich als Konflikt, +eine neu angelegte Seite wird still gestaged, nur eine beidseitig gelöschte Seite ist harmlos. Die +Prosa-Prozedur in `private-instance.md` § "Taking a stack update" hat das geschlossen, aber mit +vier eigenen Fehlern: der Pfadsatz stand dreifach (im Skript, im Kontroll-Grep, implizit in +`dist_cmd.py`); eine vom Upstream **gelöschte** Maschinerie-Datei wurde von `git checkout +MERGE_HEAD -- ` still ignoriert, weil das Skript kein `set -e` hatte; ein echter Konflikt in +`tools/`/`types/`/`instructions/` endete in einem von der Prosa nirgends erwähnten offenen Merge; +und ein *neuer* Maschinerie-Pfad unter einer Content-Stage hätte die Literal-Liste nie erreicht. + +Die Eigentumsgrenze ist jetzt ein Prädikat statt einer Liste: `chemenu/ownership.py`, +`is_stack_owned(relative)`, wahr für `/CONTRACT.md` direkt unter einer Content-Stage +(`kb`, `raw`, `work`, `reports`) und für jeden Pfad, der dort auf `.template` endet - nicht +rekursiv, `kb//COLLECTION.md` bleibt seit #39 instanzeigen. `dist_cmd.py` liest das +Modul jetzt statt einer eigenen `_CONTENT_ALLOWED_NAMES`-Liste zu pflegen, und +`CONTRACT_ONLY_STAGES` leitet sich aus `ownership.CONTENT_STAGES` ab statt die drei Stage-Pfade +ein zweites Mal aufzuschreiben - ein Test hält fest, dass beide Sichten nicht auseinanderlaufen +können. + +Neu: `wikitool upstream merge [--remote upstream] [--branch main] [--no-fetch]` und `wikitool +upstream verify --since [--until HEAD]`. `merge` prüft Vorbedingungen (sauberer Baum, kein +laufendes Merge, Remote löst auf), warnt statt zu blockieren, wenn `.wikitool-remotes.json` fehlt, +hält den Merge offen (`--no-commit --no-ff`), zwingt jede Content-Stage auf die lokale Seite +zurück, holt dann über die Vereinigungsmenge der Bäume von `MERGE_HEAD` und `HEAD` genau die +stack-eigenen Pfade zurück - inklusive einer Löschung, falls der Upstream einen Maschinerie-Pfad +entfernt hat -, verweigert bei verbliebenen unaufgelösten Pfaden ohne zu committen, committet +sonst und verifiziert den entstandenen Commit mit derselben Logik wie `verify` - ein Fund dort +wird laut gemeldet und **nicht** automatisch zurückgerollt. Nicht idempotent (AGENTS.md § Tool +error contract), nicht budget-exempt; `verify` liest nur und ist wie `migrate verify` von der +Budget-Gate ausgenommen. Die Mass-Update-Gate greift bei einem Merge-Commit strukturell nicht - +das steht jetzt als eigener Absatz in `instructions/gates.md`, mit `upstream merge`s eigener +Nachkontrolle als der Sicherung, die hier tatsächlich trägt. + +`private-instance.md` § "Taking a stack update" verweist jetzt auf den Befehl statt das Skript +auszuschreiben; die Pfadtabelle bleibt als Erklärung stehen, ist aber nicht mehr die operative +Liste. Vorschlag B (eigenes Demo-Repo) bleibt zurückgestellt, siehe #30. + +18 neue Tests unter `test_upstream_cmd.py` (30 Fälle mit der parametrisierten +`is_stack_owned`-Tabelle) decken die Fälle aus der Spezifikation ab: +gelöschte vs. geänderte vs. neu angelegte Seiten, `kb/CONTRACT.md`- und Template-Änderungen, +`kb/entities/COLLECTION.md` bleibt lokal, eine gelöschte `raw/CONTRACT.md` landet, ein neuer +Template-Pfad landet, ein offener `work/`-Lauf landet nicht, ein echter `tools/`-Konflikt lässt +den Merge offen, ein schmutziger Baum wird unberührt abgewiesen, "bereits aktuell" ist ein No-op, +die Publish-Remote-Gate-Warnung, `verify` gegen einen von Hand verpfuschten Merge, und die +`dist_cmd`/`ownership`-Konsistenz. + --- ## 4.4.0 - 2026-09-03 - Versionskandidat statt Bump-pro-Release: VERSION traegt -beta.N, version release fixiert diff --git a/VERSION b/VERSION index 6e5db82..31e7ebb 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -4.4.1-beta.1 +4.5.0-beta.1 diff --git a/instructions/gates.md b/instructions/gates.md index 17d7720..b3021cd 100644 --- a/instructions/gates.md +++ b/instructions/gates.md @@ -103,6 +103,24 @@ The setup this gate exists for - a private instance that takes stack updates fro upstream - is [private-instance.md](private-instance.md). Step 4 there arms it, deliberately *before* the first `publish`: added afterwards it leaves open exactly the window it closes. +### Mass-Update Gate blind spot: `upstream merge` + +`upstream merge` (a private instance taking a stack update - see +[private-instance.md](private-instance.md)) can update or delete dozens of stack-owned paths in +one commit, and the Mass-Update Gate does not see any of it. The gate counts *working-tree* +changes before `publish` stages them; by the time `upstream merge` commits, the change is +already history, and the commit it made is not what a later `publish` would be staging - that +publish sees only whatever this session adds on top. A merge touching 200 files therefore goes +out ungated the moment it is pushed. + +This is not a hole to patch by making `upstream merge` route through the gate: the gate's +question ("is this too much to publish?") does not apply to a change that only ever touches +stack-owned paths that are, by definition, not this instance's own content. The check that +actually matters here is `upstream merge`'s own postcheck - it re-verifies the merge commit +against `upstream verify`'s logic immediately after committing, and refuses loudly (uncommitted +state is not rolled back; the commit is left for a human to look at) if anything landed outside +a stack-owned path. That is the safeguard for this command, not the Mass-Update Gate. + ## Iteration Budget Gate and loop-breaker Every `wikitool` call is counted per session. Calls are refused past **60 in a session**, or diff --git a/instructions/private-instance.md b/instructions/private-instance.md index 078530d..3c72c2d 100644 --- a/instructions/private-instance.md +++ b/instructions/private-instance.md @@ -108,69 +108,47 @@ So the merge has to be scoped. That is the procedure below, and it is not option ## Taking a stack update -Take the machinery, never the content. The merge is held open, the content stages are forced -back to your own state, and only then does it close. +```bash +tools/wikitool upstream merge --remote upstream --branch main +``` -**Three files under those stages are machinery, not content**, and forcing them back is how an -upstream contract change gets silently discarded: +Take the machinery, never the content. This is the command form of the same idea a hand-rolled +merge would need: hold the merge open, force the content stages back to your own state, restore +only the paths that are machinery, and only then let it close. Which paths those are is not a +short literal list any more (see below) - it is `chemenu.ownership.is_stack_owned`, the same +predicate `dist_cmd.py`'s export reads, so a stack change that adds a new machinery path under a +content stage is recognised automatically rather than needing this document edited first. -| Path | Why it must take the upstream side | +**What counts as machinery under a content stage**, for readers who want the shape rather than +the code: + +| Path | Why it takes the upstream side | |---|---| -| `kb/CONTRACT.md` | The stack's own knowledge-layer contract. Every rule in it is enforced by `wikitool`; an instance never edits it | -| `kb/CONVENTIONS.md.template` | The template your `kb/CONVENTIONS.md` was filled from. The filled file is yours; the template is the stack's | -| `raw/CONTRACT.md` | The raw stage's contract, for the same reason as the first row | +| `/CONTRACT.md` (`kb/CONTRACT.md`, `raw/CONTRACT.md`, `work/CONTRACT.md`, `reports/CONTRACT.md`) | The stack's own stage contract. Every rule in it is enforced by `wikitool`; an instance never edits it | +| any `*.template` under a content stage (`kb/CONVENTIONS.md.template`, each `kb//COLLECTION.md.template`, and any later one) | The template your filled file was adopted from. The filled file is yours; the template is the stack's | -Everything else under `kb/` and `raw/` is yours, `kb/CONVENTIONS.md` and each -`kb//COLLECTION.md` included - they bind your corpus, and they are exactly what the -restore below is protecting. +Everything else under `kb/`, `raw/`, `work/` and `reports/` is yours, `kb/CONVENTIONS.md` and +each `kb//COLLECTION.md` included - they bind your corpus, and they are exactly what +`upstream merge` protects. -```bash -BEFORE=$(git rev-parse HEAD) -git fetch upstream - -# --no-commit holds the merge open; it may report conflicts under kb/ or raw/, -# which the next four lines are about to make irrelevant. -git merge --no-commit --no-ff upstream/main || true - -# Whatever the merge did to the content stages, undo it. HEAD is still your -# pre-merge commit while the merge is open, so this restores exactly your side. -git rm -rq --cached --ignore-unmatch kb raw -rm -rf kb raw -git checkout HEAD -- kb raw - -# ...then take the upstream side back for the machinery that lives among it. -# MERGE_HEAD is still resolvable while the merge is open. -git checkout MERGE_HEAD -- kb/CONTRACT.md kb/CONVENTIONS.md.template raw/CONTRACT.md - -git commit --no-edit -``` - -Then **check that it worked**, rather than trusting that it did. The same three paths are -excluded here, spelled out rather than held in a variable so that the check can be read on its -own and copied on its own: - -```bash -git diff --name-only "$BEFORE" HEAD -- kb raw \ - | grep -vE '^(kb/CONTRACT\.md|kb/CONVENTIONS\.md\.template|raw/CONTRACT\.md)$' -``` - -Must print nothing. - -An empty result is the proof that the update touched machinery only. A non-empty one means a -path slipped through - inspect it before going further. - -**The exclusion is not cosmetic.** Without it the check reports *empty* for an update that just -ate a `kb/CONTRACT.md` change - it would be confirming the failure it exists to catch. If one of -the three paths does not appear in the diff at all, that is fine: it means upstream did not -touch it. +The command itself checks its own result the same way `upstream verify` would, immediately +after committing, and refuses loudly - without rolling the commit back - if anything landed +outside a stack-owned path. A refusal there is a bug report, not something to work around by +hand; see [tools/CONTRACT.md](../tools/CONTRACT.md) for the full error contract, including what +a real conflict in `tools/`/`types/`/`instructions/` leaves behind. Then, as after any stack change: `doctor`, `docs verify`, `instructions verify`, `migrate status`, `lint`. A `migrate status` with outstanding links means the update crossed a compatibility boundary - follow [migrate-corpus.md](migrate-corpus.md) before doing anything else. -**Why not just `git merge upstream/main`?** Because of the table above: a page the upstream -*adds* arrives with no conflict and no message. You would find out when `lint` starts reporting -pages you never wrote - if you noticed at all. +**Why not just `git merge upstream/main`?** A page the upstream *adds* arrives with no conflict +and no message under a plain merge - measured in the table further up this document. You would +find out when `lint` starts reporting pages you never wrote, if you noticed at all. `upstream +merge` closes exactly that gap: the content stages never see the upstream's version at all. + +**Checking a merge you resolved by hand instead** (or auditing a past one): `tools/wikitool +upstream verify --since --until ` runs the same check `upstream merge` +runs on itself, without doing the merge. ## Where stack development happens @@ -186,17 +164,21 @@ merge above. Nothing is lost by the detour: the fix has to pass that CI either w ## Decision points -- **Merge conflict in `kb/` or `raw/`?** Expected, and already handled: the update procedure - above overwrites those stages with your own afterwards, so the conflict resolves itself. - Never resolve one by hand with `git add -A` - that is exactly how the upstream version, which - git left sitting in your working tree, gets committed into your instance. -- **`git diff` after the merge shows something under `kb/` or `raw/`?** Stop - unless it is one - of the three machinery paths the check excludes, which is the update working as intended. For - anything else the scoping step did not take: do not publish; find out which path came through - and where from. +- **Merge conflict in `kb/`, `raw/`, `work/` or `reports/`?** Expected, and already handled: + `upstream merge` overwrites those stages with your own afterwards, so the conflict resolves + itself. Never resolve one by hand with `git add -A` in a merge you are running yourself + instead - that is exactly how the upstream version, which git left sitting in your working + tree, gets committed into your instance. +- **`upstream merge` exits 1 after committing?** Read the message: its own postcheck found + content outside a stack-owned path in the commit it just made. The commit is **not** rolled + back - inspect it (`git show`, or `tools/wikitool upstream verify --since --until + HEAD`) and decide by hand whether to revert it, fix forward, or report it as a stack bug. This + should not happen; if it does, `chemenu.ownership.is_stack_owned` disagreed with itself between + the restore and the check, which is exactly what the shared predicate is meant to prevent. - **Conflict in `tools/`, `types/` or `instructions/`?** You changed the stack locally, which - step "Where stack development happens" says not to do. Take the upstream side and re-file the - change as an issue there. + step "Where stack development happens" says not to do. `upstream merge` leaves the merge open + rather than guessing - take the upstream side for the named paths and re-file the change as an + issue there, or resolve deliberately and finish the commit yourself. - **...but you changed how *your pages* are written?** That is not a stack change and the rule above does not apply to it. Language, section headings, naming forms, tone, relationship labels and the confidence rubric live in `kb/CONVENTIONS.md`, and each collection's authoring diff --git a/tools/CONTRACT.md b/tools/CONTRACT.md index 022a5d6..7c115fc 100644 --- a/tools/CONTRACT.md +++ b/tools/CONTRACT.md @@ -82,6 +82,8 @@ tools/wikitool --help | `migrate verify --from [--path P ...] [--expect-body-change] [--json] [--fail-on-error]` | Compare `kb/` against a git revision on the invariants a content migration must not change: wikilink and citation **counts** (not sets), footnote definitions, H1, structural frontmatter, and the **count of generated-region marker pairs** - a page that went from one links region to two has the same set of region names and a different count, and a lost marker turns a generated region into prose the next write appends a second one beside. Reports added/removed pages without failing on them. `--expect-body-change` additionally flags a page whose body did not change at all. Not migration-specific - worth running after any bulk rewrite, and the one question `lint` cannot answer, since it reads a single revision and so cannot see that something went missing. Read-only and exempt from the budget gate | | `migrate done [--pages N] [--dry-run]` | Record one migration as applied, advancing `kb_version` in `.wikitool-kb.json` to its target. **Refuses any version that is not the next link in the chain** - skipping one leaves the corpus in a shape no version describes, and an interrupted multi-step upgrade has to be resumable rather than guessable. An `offered` migration is recorded in the applied ledger *without* moving `kb_version` and with no ordering rule applied: it is not a link in the chain, so there is nothing to skip, and requiring the chain first would make an unrelated file upgrade wait on it. Re-recording one already in the ledger is a no-op, not an error | | `migrate baseline [--force]` | Declare `kb_version` once, for an instance predating `.wikitool-kb.json`. Refuses to overwrite an existing declaration without `--force`: advancing after a migration is `done`, which checks the chain, and this command must not become the quiet way around it | +| `upstream merge [--remote upstream] [--branch main] [--no-fetch]` | Take a stack update into a private instance's branch, machinery only - the code procedure behind `instructions/private-instance.md` § "Taking a stack update". Refuses on a dirty working tree, a merge already in progress, or a remote that does not resolve; WARNs (does not block) when `.wikitool-remotes.json` is absent, pointing at the setup step that arms it. Fetches `/` (unless `--no-fetch`) and reports "already up to date" if nothing new exists. Otherwise opens `git merge --no-commit --no-ff /`, forces every content stage (`kb/`, `raw/`, `work/`, `reports/`) back to the local side, then restores from the upstream side exactly the paths `chemenu.ownership.is_stack_owned` recognises as machinery (`/CONTRACT.md`, and anything ending `.template` under a content stage) - including a deletion, if the upstream removed one. A real conflict left in `tools/`, `types/` or `instructions/` after that leaves the merge open, uncommitted, and exits 1 rather than guessing. Commits with `git commit --no-edit`, then re-checks the resulting range with the same logic as `upstream verify`; a finding there is a loud, uncommitted-nothing-rolled-back error, because the merge commit already exists and needs a human's eyes, not an automatic repair. Never pushes. Not idempotent - see the tool error contract below | +| `upstream verify --since [--until HEAD]` | Compare two revisions: did anything under a content stage (`kb/`, `raw/`, `work/`, `reports/`) change except through a stack-owned path? Shares its check with `upstream merge`'s own postcheck, so a hand-resolved merge conflict, or a future `dist upgrade` (#7), can be verified the same way. Exit 1 with the offending paths if anything leaked; otherwise reports which stack-owned paths legitimately moved. Read-only and exempt from the Iteration Budget Gate, like `migrate verify` | | `doctor [--json]` | Check that this instance is correctly configured: dependencies (Python, ripgrep), author resolution, stack version, git identity/branch/remote, published skills, kb/raw/reports/work/instructions structure, personalization (`USER.md`/`SOUL.md` present **and** filled - a file still carrying the template's sentinel is a `FAIL`, since a renamed template is not a filled one), the KB conventions (`kb/CONVENTIONS.md` present, unsentinelled, and naming all three tool-owned section headings - a `FAIL` on any of the three, because `xref`/`cite` write out of it), the environment note (`ENVIRONMENT.md` - optional, so absent is `OK`; a still-templated one is a `WARN`), generated files, and `WIKITOOL_SESSION_ID`. Read-only, exit 1 only on a `FAIL` (a missing remote, session id, or `VERSION` is a `WARN`, not a fault). Exempt from the Iteration Budget Gate | ## Design notes @@ -199,6 +201,8 @@ is atomic, and whether a retry is safe. | `migrate verify` | Only with `--fail-on-error`: an invariant changed. Also exits 1 if `--from` is not a revision in this repository | Read-only | Exit 1 from `--fail-on-error` means "act on the findings", not "the tool is broken". A finding is never fixed by re-running - it names a page and what changed on it | | `migrate done` | Unknown version, no `.wikitool-kb.json`, nothing outstanding, or a *required* version that is not the next link in the chain | Yes - single file write | **Not idempotent** for a required migration: it advances the chain. For "not the next link", run `migrate status` and apply them in the order it prints - never force the order. Recording an `offered` migration *is* idempotent and safe to repeat | | `migrate baseline` | Unparseable version, or a declaration already exists and `--force` was not passed | Yes - single file write | Safe to re-run with the same version. If a declaration exists, it is almost always `migrate done` that was wanted | +| `upstream merge` | Dirty working tree, a merge already in progress, the remote does not resolve, or a real conflict remains in `tools/`/`types/`/`instructions/` after the content stages and stack-owned paths were restored | **No** - can leave an open, uncommitted merge behind on refusal after fetching | **Not idempotent, and not safe to retry unchanged.** For a dirty tree or an in-progress merge: fix the named precondition and retry once. For a real conflict: **do not retry, do not force** - resolve the named paths by hand (take the upstream side, or re-file the local change as an issue against the public repo per `instructions/private-instance.md`) and either `git commit --no-edit` yourself or `git merge --abort`. If the postcheck after commit finds a leak, the merge commit already exists and is **not** rolled back automatically - inspect it by hand; this is a bug report, not a retry | +| `upstream verify` | A leak was found (content changed under a content stage through a path that is not stack-owned), or `--since`/`--until` is not a revision in this repository | Read-only | A finding is not fixed by re-running - it names the paths that leaked. Fix the revision argument and retry for the second case | | `doctor` | At least one check reported `FAIL` (a `WARN`, e.g. no remote or no `WIKITOOL_SESSION_ID`, does not exit 1) | Read-only | Each finding names its own fix command; re-run after applying it | | `budget status` / `budget reset` | `reset` without `--yes`; `status` never fails | Read/rewrite of one JSON file | `status` is safe to retry. For `reset`: get the user's approval, then re-run with `--yes` | | `eval sessions` | Never fails; an empty list is a valid answer | Read-only | - | diff --git a/tools/README.md b/tools/README.md index f2baab9..a6a1fe8 100644 --- a/tools/README.md +++ b/tools/README.md @@ -43,6 +43,7 @@ tools/ links.py labelled edges in `related:` - the graph's semantics as data, not prose kb_collections.py collection discovery (a directory with COLLECTION.md), and what one declares about itself conventions.py kb/CONVENTIONS.md: what this instance decided about authoring, as opposed to what the stack enforces + ownership.py the stack-vs-instance boundary under a content stage - one predicate, read by `dist_cmd.py` and `commands/upstream_cmd.py` so the two cannot answer it differently type_resolver.py type-spec loading and schema resolution lint_core.py the lint checks and the report, with no CLI attached types_core.py type-spec listing/description, with no CLI attached diff --git a/tools/chemenu/cli.py b/tools/chemenu/cli.py index c49e86b..939f21e 100644 --- a/tools/chemenu/cli.py +++ b/tools/chemenu/cli.py @@ -31,6 +31,7 @@ try: search as search_module, touch as touch_module, types_cmd, + upstream_cmd, version_cmd, work_cmd, xref, @@ -69,6 +70,7 @@ app.add_typer(eval_cmd.app, name="eval") app.add_typer(dist_cmd.app, name="dist") app.add_typer(version_cmd.app, name="version") app.add_typer(migrate_cmd.app, name="migrate") +app.add_typer(upstream_cmd.app, name="upstream") app.command("new")(new_page.new_page_command) app.command("touch")(touch_module.touch_command) app.command("rename")(page_ops.rename_command) diff --git a/tools/chemenu/commands/dist_cmd.py b/tools/chemenu/commands/dist_cmd.py index d15836d..1f8ebf6 100644 --- a/tools/chemenu/commands/dist_cmd.py +++ b/tools/chemenu/commands/dist_cmd.py @@ -39,7 +39,7 @@ from typing import Callable, NamedTuple, Optional, Union import typer -from chemenu import config, conventions, kb_collections, kb_state, version as version_mod +from chemenu import config, conventions, kb_collections, kb_state, ownership, version as version_mod from chemenu.commands._util import fail, rel_path, success, today_iso app = typer.Typer(help="Build a distributable copy of the wiki machinery.") @@ -132,8 +132,15 @@ INSTRUCTIONS_EXCLUDE_DIRS = {"dev"} RAW_SUBDIRS = ("articles", "documents", "notes", "assets") # Stage contracts that are not collections and carry no pages: copied as a -# single file each, nothing else from their directory. -CONTRACT_ONLY_STAGES = ("raw/CONTRACT.md", "reports/CONTRACT.md", "work/CONTRACT.md") +# single file each, nothing else from their directory. `kb/` is excluded here +# - it is a content stage too, but it has collections underneath it, so its +# contract is handled by `build_plan` alongside them rather than as a bare +# stage copy. Derived from `ownership.CONTENT_STAGES` rather than listed +# again, so the set this loop copies and the set `upstream merge` restores +# cannot name a different stage without one of them failing its own test. +CONTRACT_ONLY_STAGES = tuple( + f"{stage}/CONTRACT.md" for stage in ownership.CONTENT_STAGES if stage != "kb" +) # Single tracked files copied out of an otherwise-untouched, partially-ignored # directory. `.claude/` holds the harness's own session-tracing config @@ -439,19 +446,20 @@ def build_plan(origin: Optional[Origin] = None) -> dict[str, PlannedFile]: # appears in INSTALL.md and version.py, so such a scan would either whitelist # the very string it is looking for or cry wolf on every export. # -# `COLLECTION.md` and `CONVENTIONS.md` are deliberately *not* on the allowed -# list any more. Both bind, and both are the instance's to write, so they cross -# the boundary as `.template` and are adopted by a rename - a plan carrying the +# `COLLECTION.md` and `CONVENTIONS.md` are deliberately *not* allowed through +# any more. Both bind, and both are the instance's to write, so they cross the +# boundary as `.template` and are adopted by a rename - a plan carrying the # filled name would hand a new instance this one's authoring conventions as # though they were the stack's. +# +# What counts as machinery under kb/ or raw/ is no longer a second list here: +# it is `ownership.is_stack_owned`, the same predicate `upstream merge` and +# `upstream verify` restore/check against. Only the export-only stubs +# (`ownership.EXPORT_STUB_NAMES`) are allowed here without also being +# stack-owned - a merge keeps the *local* copy of those, while export writes a +# fresh one regardless of either side, so the two callers genuinely disagree +# about them and each keeps its own allowance for that one case. _CONTENT_PREFIXES = ("kb/", "raw/") -_CONTENT_ALLOWED_NAMES = ( - "CONTRACT.md", - f"{kb_collections.CONTRACT_NAME}.template", - conventions.CONVENTIONS_TEMPLATE, - "log.md", - ".gitkeep", -) _INSTANCE_OWNED_KB_FILES = (kb_collections.CONTRACT_NAME, conventions.CONVENTIONS_FILENAME) @@ -473,7 +481,11 @@ def find_leaks(plan: dict[str, PlannedFile]) -> list[str]: leaks.append(f"{relative} (this instance's page type-spec; ship the .template)") elif relative.startswith("instructions/dev/"): leaks.append(f"{relative} (stack-development only)") - elif relative.startswith(_CONTENT_PREFIXES) and name not in _CONTENT_ALLOWED_NAMES: + elif ( + relative.startswith(_CONTENT_PREFIXES) + and not ownership.is_stack_owned(relative) + and not ownership.is_export_stub(name) + ): leaks.append(f"{relative} (wiki content, not machinery)") return leaks diff --git a/tools/chemenu/commands/run_budget.py b/tools/chemenu/commands/run_budget.py index 6f7b791..01e5e35 100644 --- a/tools/chemenu/commands/run_budget.py +++ b/tools/chemenu/commands/run_budget.py @@ -101,6 +101,13 @@ SKIP_COMMAND_PATHS = { ("migrate", "list"), ("migrate", "status"), ("migrate", "verify"), + # `upstream verify` only reads two git revisions and reports what changed - + # the same argument as `migrate verify`: a check that costs budget is one + # an agent starts skipping. `upstream merge` stays counted: it mutates the + # branch and can leave an open merge behind on refusal, so it belongs on + # the non-idempotent list (AGENTS.md's tool error contract) rather than + # the exempt one. + ("upstream", "verify"), } # Commands exempt regardless of their first argument, because that argument is diff --git a/tools/chemenu/commands/upstream_cmd.py b/tools/chemenu/commands/upstream_cmd.py new file mode 100644 index 0000000..84bbf84 --- /dev/null +++ b/tools/chemenu/commands/upstream_cmd.py @@ -0,0 +1,317 @@ +"""`wikitool upstream` - take a stack update from a public upstream into a +private instance's `main` without letting the upstream's own content (a demo +corpus, a workshop run) ride along. + +`git merge upstream/main` on its own treats a moved corpus dangerously +asymmetrically: a page the instance deleted and the upstream edited reports as +a conflict, a page the upstream *added* stages silently, and a page both sides +deleted is the only harmless case. `instructions/private-instance.md`'s prose +procedure closes that, by holding the merge open, forcing the content stages +(`ownership.CONTENT_STAGES`) back to the local side, and then restoring only +the paths `ownership.is_stack_owned` recognises as machinery. `upstream merge` +is that procedure in code, so the path set it acts on cannot drift from the +one `dist_cmd.py` ships - both read `chemenu.ownership` - and so a conflict in +the machinery layers, or a machinery file the upstream deleted, gets an +explained stop instead of a silently wrong commit. + +`upstream verify` is the other half: given two revisions, did anything change +under a content stage except through a stack-owned path? It shares +`_content_leaks` with the postcheck `upstream merge` runs on itself, so a +hand-resolved merge or a future `dist upgrade` (Gitea #7) can be checked the +same way. +""" +from __future__ import annotations + +import shutil +from pathlib import Path +from typing import Optional + +import typer + +from chemenu import config, ownership +from chemenu.commands import git_publish +from chemenu.commands._util import console, fail, success + +app = typer.Typer(help="Take a stack update from a public upstream, machinery only.") + + +def _run(args: list[str]): + import subprocess + + return subprocess.run(args, cwd=config.ROOT, capture_output=True, text=True) + + +def _rev_parse(rev: str) -> Optional[str]: + result = _run(["git", "rev-parse", "--verify", "-q", rev]) + return result.stdout.strip() if result.returncode == 0 else None + + +def _git_dir() -> Optional[Path]: + result = _run(["git", "rev-parse", "--git-dir"]) + if result.returncode != 0: + return None + path = Path(result.stdout.strip()) + return path if path.is_absolute() else config.ROOT / path + + +def _working_tree_dirty() -> bool: + result = _run(["git", "status", "--porcelain"]) + return bool(result.stdout.strip()) + + +def _merge_in_progress() -> bool: + git_dir = _git_dir() + return git_dir is not None and (git_dir / "MERGE_HEAD").exists() + + +def _remote_resolves(remote: str) -> bool: + return _run(["git", "remote", "get-url", remote]).returncode == 0 + + +def _is_ancestor(ancestor: str, of: str) -> bool: + return _run(["git", "merge-base", "--is-ancestor", ancestor, of]).returncode == 0 + + +def _tree_has_path(rev: str, path: str) -> bool: + return _run(["git", "rev-parse", "--verify", "-q", f"{rev}:{path}"]).returncode == 0 + + +def _tree_paths(rev: str) -> set[str]: + result = _run(["git", "ls-tree", "-r", "--name-only", "-z", rev]) + if result.returncode != 0: + return set() + return {p for p in result.stdout.split("\0") if p} + + +def _content_leaks(since: str, until: str) -> list[str]: + """Paths under a content stage that changed between `since` and `until` + through something other than a stack-owned path. Shared by `upstream + merge`'s own postcheck and `upstream verify`, so the two cannot disagree + about what a clean update looks like.""" + result = _run(["git", "diff", "--name-only", "-z", since, until, "--", *ownership.CONTENT_STAGES]) + if result.returncode != 0: + fail( + f"`git diff {since} {until}` failed - is {since} a revision in this repository?\n" + f"{result.stderr}" + ) + return [] + changed = [p for p in result.stdout.split("\0") if p] + return sorted(p for p in changed if not ownership.is_stack_owned(p)) + + +def _stack_paths_changed(since: str, until: str) -> list[str]: + """The subset of the same diff that *is* a stack-owned path - the paths + that legitimately moved, for the success message.""" + result = _run(["git", "diff", "--name-only", "-z", since, until, "--", *ownership.CONTENT_STAGES]) + changed = [p for p in result.stdout.split("\0") if p] + return sorted(p for p in changed if ownership.is_stack_owned(p)) + + +# --- upstream merge --------------------------------------------------------- + + +def _remote_gate_warning() -> None: + if git_publish.read_allowed_push_urls() is not None: + return + console.print( + "[bold yellow]WARN[/bold yellow] No .wikitool-remotes.json in this checkout - the " + "Publish-Remote Gate is unarmed, so a future `publish` to the wrong remote would not " + "be caught. `upstream merge` never pushes and proceeds regardless, but a checkout that " + "takes stack updates from a public upstream should arm the gate before its next publish " + "- see instructions/private-instance.md step 4." + ) + + +def _precondition_failure(remote: str) -> Optional[str]: + if _working_tree_dirty(): + return ( + "Working tree is not clean (`git status --porcelain` printed something). " + "`upstream merge` refuses to start on a dirty tree so a refusal never has to " + "guess which changes were already there. Commit or stash first." + ) + if _merge_in_progress(): + return ( + "A merge is already in progress (.git/MERGE_HEAD exists). Resolve or abort it " + "(`git merge --abort`) before running `upstream merge`." + ) + if not _remote_resolves(remote): + return f"Remote '{remote}' does not resolve (`git remote get-url {remote}` failed)." + return None + + +def _unresolved_conflict_message(unresolved: list[str], remote: str, branch: str) -> str: + listed = "\n".join(f" - {p}" for p in unresolved) + return ( + f"A real conflict remains in the machinery layers after restoring the content stages " + f"and the stack-owned paths from {remote}/{branch}:\n{listed}\n\n" + "The merge is left open, uncommitted - nothing was written to the branch. Per " + "instructions/private-instance.md's decision points: this means the checkout changed " + "the stack locally, which private instances do not do. Take the upstream side for " + "these paths (`git checkout --theirs -- ` then `git add`) and re-file the local " + "change as an issue against the public repo, or resolve deliberately and " + "`git commit --no-edit` yourself. `git merge --abort` gives up the merge entirely." + ) + + +def _postcheck_failure_message(leaks: list[str], before: str) -> str: + listed = "\n".join(f" - {p}" for p in leaks) + return ( + f"The merge commit exists (content stages are not what they were before this ran), " + f"but it changed content outside of a stack-owned path:\n{listed}\n\n" + f"This was NOT rolled back - the state belongs in front of you, not behind an automatic " + f"repair the command applies to itself. Compare against the pre-merge commit ({before}) " + "and decide by hand whether to revert the merge commit, cherry-pick around it, or fix " + "forward. This is a bug in `upstream merge` or in `ownership.is_stack_owned` if it " + "reproduces - please report it rather than working around it silently." + ) + + +def _merge_success_message( + updated: list[str], deleted: list[str], remote: str, branch: str +) -> str: + lines = [f"Merged {remote}/{branch}. Content stages ({', '.join(ownership.CONTENT_STAGES)}) are unchanged."] + if updated: + lines.append(f"Stack paths updated ({len(updated)}):") + lines += [f" - {p}" for p in updated] + if deleted: + lines.append(f"Stack paths removed, following the upstream ({len(deleted)}):") + lines += [f" - {p}" for p in deleted] + if not updated and not deleted: + lines.append("No stack-owned path changed.") + return "\n".join(lines) + + +@app.command("merge") +def merge_command( + remote: str = typer.Option("upstream", "--remote", help="Remote to merge from"), + branch: str = typer.Option("main", "--branch", help="Branch to merge"), + no_fetch: bool = typer.Option( + False, "--no-fetch", help="Skip `git fetch ` - use whatever is already fetched" + ), +): + """Merge `/` into the current branch, machinery only: + every path under a content stage (kb/, raw/, work/, reports/) is forced + back to the local side except a stack-owned path (`/CONTRACT.md`, + or anything ending `.template` under a content stage), which is taken + from the upstream - including a deletion, if the upstream removed one. A + real conflict elsewhere (tools/, types/, instructions/) leaves the merge + open and unresolved rather than guessing. Not idempotent: it can leave an + open merge behind on refusal. See instructions/private-instance.md.""" + problem = _precondition_failure(remote) + if problem: + fail(problem) + return + + _remote_gate_warning() + + before = _rev_parse("HEAD") + if before is None: + fail("HEAD does not resolve - is this a git repository with at least one commit?") + return + + if not no_fetch: + fetch_result = _run(["git", "fetch", remote, branch]) + if fetch_result.returncode != 0: + fail(f"`git fetch {remote} {branch}` failed:\n{fetch_result.stderr}") + return + + remote_ref = f"{remote}/{branch}" + if _rev_parse(remote_ref) is None: + fail(f"'{remote_ref}' does not resolve - fetch it first, or check --remote/--branch.") + return + + if _is_ancestor(remote_ref, "HEAD"): + success(f"Already up to date with {remote_ref}.") + return + + _run(["git", "merge", "--no-commit", "--no-ff", remote_ref]) + + for stage in ownership.CONTENT_STAGES: + if not _tree_has_path("HEAD", stage): + continue + _run(["git", "rm", "-rq", "--cached", "--ignore-unmatch", stage]) + stage_dir = config.ROOT / stage + if stage_dir.exists(): + shutil.rmtree(stage_dir) + _run(["git", "checkout", "HEAD", "--", stage]) + + merge_head_paths = _tree_paths("MERGE_HEAD") + head_paths = _tree_paths("HEAD") + stack_paths = sorted( + p for p in (merge_head_paths | head_paths) if ownership.is_stack_owned(p) + ) + + updated: list[str] = [] + deleted: list[str] = [] + for relative in stack_paths: + if relative in merge_head_paths: + checkout = _run(["git", "checkout", "MERGE_HEAD", "--", relative]) + if checkout.returncode != 0: + fail( + f"`git checkout MERGE_HEAD -- {relative}` failed even though it is listed " + f"in MERGE_HEAD's own tree:\n{checkout.stderr}\nThe merge is left open." + ) + return + updated.append(relative) + else: + _run(["git", "rm", "-q", "--cached", "--ignore-unmatch", relative]) + target = config.ROOT / relative + if target.exists(): + target.unlink() + deleted.append(relative) + + unresolved = [p for p in _run(["git", "diff", "--name-only", "--diff-filter=U"]).stdout.splitlines() if p] + if unresolved: + fail(_unresolved_conflict_message(unresolved, remote, branch)) + return + + commit_result = _run(["git", "commit", "--no-edit"]) + if commit_result.returncode != 0: + fail(f"`git commit --no-edit` failed:\n{commit_result.stderr}") + return + + leaks = _content_leaks(before, "HEAD") + if leaks: + fail(_postcheck_failure_message(leaks, before)) + return + + success(_merge_success_message(updated, deleted, remote, branch)) + + +# --- upstream verify --------------------------------------------------------- + + +def _verify_failure_message(leaks: list[str], since: str, until: str) -> str: + listed = "\n".join(f" - {p}" for p in leaks) + return ( + f"Content under a content stage (kb/, raw/, work/, reports/) changed between {since} " + f"and {until} through a path that is not stack-owned:\n{listed}\n\n" + "That is upstream content (or an equivalent local change) that reached this range " + "outside of a stack-owned path - inspect it before trusting this range as machinery-only." + ) + + +def _verify_success_message(stack_moved: list[str], since: str, until: str) -> str: + if not stack_moved: + return f"No content changed between {since} and {until} under kb/, raw/, work/, reports/." + listed = "\n".join(f" - {p}" for p in stack_moved) + return ( + f"Clean: only stack-owned paths changed under kb/, raw/, work/, reports/ between " + f"{since} and {until}:\n{listed}" + ) + + +@app.command("verify") +def verify_command( + since: str = typer.Option(..., "--since", help="Git revision to compare from"), + until: str = typer.Option("HEAD", "--until", help="Git revision to compare to"), +): + """Check that nothing under a content stage changed between --since and + --until except through a stack-owned path. Read-only, and exempt from the + Iteration Budget Gate - the same treatment `migrate verify` gets, for the + same reason: a check an agent has to ration is a check that gets skipped.""" + leaks = _content_leaks(since, until) + if leaks: + fail(_verify_failure_message(leaks, since, until)) + return + success(_verify_success_message(_stack_paths_changed(since, until), since, until)) diff --git a/tools/chemenu/ownership.py b/tools/chemenu/ownership.py new file mode 100644 index 0000000..52e327c --- /dev/null +++ b/tools/chemenu/ownership.py @@ -0,0 +1,64 @@ +"""The ownership boundary for a path under a content stage: does it belong to +the *stack* (ships with every distribution, wins over local content when a +private instance merges from a public upstream) or to the *instance* (never +ships filled, wins over the upstream's version)? + +One predicate, so `dist_cmd.py` (export) and `upstream_cmd.py` (merge/verify) +answer the same question about the same paths instead of each keeping its own +literal list that can drift out of sync with the other - see AGENTS.md +invariant 8, and Gitea #30 for the incident that made the drift concrete +(the private-instance merge procedure hardcoded a three-path list that +`dist_cmd.py` had already outgrown). +""" +from __future__ import annotations + +# The stages whose content belongs to *this instance*, not the stack. Mirrors +# the sentence .gitignore already makes about raw/, kb/ and work/ being the +# repo's content, plus reports/ - only reports/CONTRACT.md is tracked there, +# the rest is gitignored, so restoring it is a no-op today. It stays in the +# set anyway: a set that is "almost" this one is the beginning of the same +# drift this module exists to end. +CONTENT_STAGES = ("kb", "raw", "work", "reports") + +# Bare filenames `dist export` overwrites with a fresh stub rather than +# shipping the stack's own copy. Not stack-owned: an upstream merge takes the +# *local* side for these (they are the instance's own log/placeholder), +# while `dist export` writes a brand-new one regardless of either side. +EXPORT_STUB_NAMES = ("log.md", ".gitkeep") + +# The single machinery filename directly under a content stage's own root. +_STAGE_CONTRACT_NAME = "CONTRACT.md" + + +def is_stack_owned(relative: str) -> bool: + """Whether `relative` - a path under a content stage, e.g. "kb/CONTRACT.md" + or "kb/entities/COLLECTION.md.template" - is machinery: it ships with + every distribution, and it is the side an upstream merge keeps. + + True for exactly two shapes: + + - `/CONTRACT.md`, directly under a content stage's own root. Not + recursive: `kb//COLLECTION.md` sits one level deeper and is + instance-owned (see kb/CONTRACT.md's collection-ownership split). + - Any path under a content stage ending in `.template` - by construction + the stack's own copy of something the instance adopts by renaming + (`kb/CONVENTIONS.md.template` and every `kb//COLLECTION.md.template` + today; a future stack-owned template under a content stage falls under + this rule automatically, with no code change here). + + False for everything else under a content stage, `EXPORT_STUB_NAMES` + included - those are handled separately by whichever caller cares about + them, because the two callers disagree about which side wins for a stub. + """ + parts = relative.split("/") + if len(parts) < 2 or parts[0] not in CONTENT_STAGES: + return False + if relative.endswith(".template"): + return True + return len(parts) == 2 and parts[1] == _STAGE_CONTRACT_NAME + + +def is_export_stub(name: str) -> bool: + """Whether `name` (a bare filename, not a path) is one `dist export` + overwrites with a fresh stub of its own rather than shipping verbatim.""" + return name in EXPORT_STUB_NAMES diff --git a/tools/chemenu/tests/test_upstream_cmd.py b/tools/chemenu/tests/test_upstream_cmd.py new file mode 100644 index 0000000..86b8557 --- /dev/null +++ b/tools/chemenu/tests/test_upstream_cmd.py @@ -0,0 +1,345 @@ +"""Tests for `wikitool upstream merge`/`upstream verify` - the code procedure +that replaces private-instance.md's prose merge script (Gitea #30). + +Two real git repos stand in for a private instance (`repo`, remote name +`upstream`) and the public repo it takes updates from (`upstream`, a plain +repo committed to directly - a fetch-only remote does not need to be bare for +`git fetch` to work against it). Each scenario diverges the two by committing +independently on each side, exactly like a real fetch-only upstream would. +""" +from __future__ import annotations + +import subprocess + +import pytest +import typer + +from chemenu import config, ownership +from chemenu.commands import git_publish, upstream_cmd + + +def _git(root, *args): + result = subprocess.run(["git", *args], cwd=root, capture_output=True, text=True) + assert result.returncode == 0, result.stderr + return result + + +def _write(root, relative, content): + path = root / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(content, encoding="utf-8") + + +def _commit(root, message): + _git(root, "add", "-A") + _git(root, "commit", "-m", message) + + +@pytest.fixture +def two_repos(tmp_path, monkeypatch): + """`repo`, a private instance, with a fetch-only `upstream` remote pointing + at a second, independent repo. Both start from the same seed commit - + kb/CONTRACT.md, kb/CONVENTIONS.md(.template), kb/entities/COLLECTION.md, + raw/CONTRACT.md, work/CONTRACT.md, reports/CONTRACT.md, and one tools/ + file - which is what a private instance looks like right after the + private-instance.md setup: the tracked machinery, plus its own filled + instance files layered on top. + """ + seed = tmp_path / "seed" + seed.mkdir() + _git(seed, "init", "-b", "main") + _git(seed, "config", "user.name", "Seed") + _git(seed, "config", "user.email", "seed@example.com") + # .wikitool-remotes.json is gitignored in the real repo (it is per-checkout, + # see config.PUBLISH_REMOTES_FILENAME) - without this, dropping one into the + # fixture during a test would show up as an untracked file and trip the + # dirty-working-tree precondition for a reason that has nothing to do with + # what that test is checking. + _write(seed, ".gitignore", f"{config.PUBLISH_REMOTES_FILENAME}\n") + _write(seed, "kb/CONTRACT.md", "stack kb contract v1\n") + _write(seed, "kb/CONVENTIONS.md.template", "template v1\n") + _write(seed, "kb/CONVENTIONS.md", "instance conventions v1\n") + _write(seed, "kb/entities/COLLECTION.md", "instance collection contract v1\n") + _write(seed, "kb/Both.md", "page both sides delete\n") + _write(seed, "kb/ToDelete.md", "page the instance will delete\n") + _write(seed, "raw/CONTRACT.md", "raw contract v1\n") + _write(seed, "work/CONTRACT.md", "work contract v1\n") + _write(seed, "reports/CONTRACT.md", "reports contract v1\n") + _write(seed, "tools/wikitool.py", "line one\nline two\nline three\n") + _commit(seed, "seed") + + upstream = tmp_path / "upstream" + subprocess.run(["git", "clone", str(seed), str(upstream)], check=True, capture_output=True) + _git(upstream, "config", "user.name", "Upstream") + _git(upstream, "config", "user.email", "upstream@example.com") + + # Cloned from `upstream`, not from `seed` directly: the remote (renamed + # below) must resolve to the path this fixture actually commits new + # upstream state into, or a later `git fetch upstream main` silently + # fetches from `seed` instead and never sees anything new. + repo = tmp_path / "repo" + subprocess.run(["git", "clone", str(upstream), str(repo)], check=True, capture_output=True) + _git(repo, "config", "user.name", "Test") + _git(repo, "config", "user.email", "test@example.com") + _git(repo, "remote", "rename", "origin", "upstream") + + monkeypatch.setattr(config, "ROOT", repo) + monkeypatch.setenv("WIKITOOL_SESSION_ID", "test-session") + return upstream, repo + + +def _merge(**overrides): + kwargs = dict(remote="upstream", branch="main", no_fetch=False) + kwargs.update(overrides) + upstream_cmd.merge_command(**kwargs) + + +# --- the four restbefund regressions, plus the baseline table from the issue --- + + +def test_upstream_edit_of_a_page_the_instance_deleted_does_not_land(two_repos): + upstream, repo = two_repos + _git(repo, "rm", "-q", "kb/ToDelete.md") + _commit(repo, "instance deletes ToDelete") + + _write(upstream, "kb/ToDelete.md", "upstream edited it after the instance deleted it\n") + _commit(upstream, "upstream edits ToDelete") + + _merge() + + assert not (repo / "kb/ToDelete.md").exists() + + +def test_upstream_new_page_does_not_land(two_repos): + upstream, repo = two_repos + _write(upstream, "kb/NewPage.md", "a demo page the upstream added\n") + _commit(upstream, "upstream adds NewPage") + + _merge() + + assert not (repo / "kb/NewPage.md").exists() + + +def test_page_deleted_on_both_sides_is_a_noop(two_repos): + upstream, repo = two_repos + _git(repo, "rm", "-q", "kb/Both.md") + _commit(repo, "instance deletes Both") + _git(upstream, "rm", "-q", "kb/Both.md") + _commit(upstream, "upstream deletes Both") + + _merge() # must not raise + + assert not (repo / "kb/Both.md").exists() + + +def test_kb_contract_change_lands(two_repos): + upstream, repo = two_repos + _write(upstream, "kb/CONTRACT.md", "stack kb contract v2\n") + _commit(upstream, "upstream changes kb/CONTRACT.md") + + _merge() + + assert (repo / "kb/CONTRACT.md").read_text(encoding="utf-8") == "stack kb contract v2\n" + + +def test_conventions_template_change_lands_local_conventions_untouched(two_repos): + upstream, repo = two_repos + _write(upstream, "kb/CONVENTIONS.md.template", "template v2\n") + _commit(upstream, "upstream changes the conventions template") + + _merge() + + assert (repo / "kb/CONVENTIONS.md.template").read_text(encoding="utf-8") == "template v2\n" + assert (repo / "kb/CONVENTIONS.md").read_text(encoding="utf-8") == "instance conventions v1\n" + + +def test_collection_contract_change_does_not_land(two_repos): + """A COLLECTION.md is instance-owned since #39 - one level deeper than + `/CONTRACT.md`, so `is_stack_owned` must say no to it.""" + upstream, repo = two_repos + _write(repo, "kb/entities/COLLECTION.md", "instance collection contract v2 (local)\n") + _commit(repo, "instance rewrites its own collection contract") + + _write(upstream, "kb/entities/COLLECTION.md", "upstream collection contract v2\n") + _commit(upstream, "upstream changes the default collection contract") + + _merge() + + assert (repo / "kb/entities/COLLECTION.md").read_text(encoding="utf-8") == ( + "instance collection contract v2 (local)\n" + ) + + +def test_upstream_deletion_of_a_contract_file_lands(two_repos): + """Restbefund 2: a machinery file the upstream deleted must not silently + survive because `git checkout MERGE_HEAD -- ` has nothing to check + out.""" + upstream, repo = two_repos + _git(upstream, "rm", "-q", "raw/CONTRACT.md") + _commit(upstream, "upstream drops raw/CONTRACT.md") + + _merge() + + assert not (repo / "raw/CONTRACT.md").exists() + + +def test_new_stack_template_under_a_content_stage_lands(two_repos): + """Restbefund 4: a brand-new stack-owned path the local tree has never + seen must still be recognised by the predicate, not by a literal list.""" + upstream, repo = two_repos + _write(upstream, "kb/GLOSSARY.md.template", "a stack-owned template that never existed before\n") + _commit(upstream, "upstream adds a new template") + + _merge() + + assert (repo / "kb/GLOSSARY.md.template").read_text(encoding="utf-8") == ( + "a stack-owned template that never existed before\n" + ) + + +def test_open_workshop_run_files_do_not_land(two_repos): + upstream, repo = two_repos + _write(upstream, "work/some-run/README.md", "an in-progress workshop run\n") + _commit(upstream, "upstream ships an open work/ run") + + _merge() + + assert not (repo / "work/some-run").exists() + + +def test_real_conflict_in_tools_leaves_the_merge_open(two_repos): + upstream, repo = two_repos + + _write(repo, "tools/wikitool.py", "line one\nLOCAL CHANGE\nline three\n") + _commit(repo, "local edits tools/wikitool.py") + before = _git(repo, "rev-parse", "HEAD").stdout.strip() + + _write(upstream, "tools/wikitool.py", "line one\nUPSTREAM CHANGE\nline three\n") + _commit(upstream, "upstream edits the same line") + + with pytest.raises(typer.Exit) as excinfo: + _merge() + assert excinfo.value.exit_code == 1 + + assert _git(repo, "rev-parse", "HEAD").stdout.strip() == before + assert (repo / ".git" / "MERGE_HEAD").exists() + + +def test_dirty_working_tree_is_refused_untouched(two_repos): + upstream, repo = two_repos + before = _git(repo, "rev-parse", "HEAD").stdout.strip() + (repo / "kb/CONTRACT.md").write_text("uncommitted local edit\n", encoding="utf-8") + + _write(upstream, "kb/CONTRACT.md", "stack kb contract v2\n") + _commit(upstream, "upstream changes kb/CONTRACT.md") + + with pytest.raises(typer.Exit) as excinfo: + _merge() + assert excinfo.value.exit_code == 1 + + assert _git(repo, "rev-parse", "HEAD").stdout.strip() == before + assert (repo / "kb/CONTRACT.md").read_text(encoding="utf-8") == "uncommitted local edit\n" + + +def test_already_up_to_date_is_a_noop(two_repos): + upstream, repo = two_repos + before = _git(repo, "rev-parse", "HEAD").stdout.strip() + + _merge() # nothing new upstream at all + + assert _git(repo, "rev-parse", "HEAD").stdout.strip() == before + + +def test_merge_warns_when_the_publish_remote_gate_is_unarmed(two_repos, capsys): + upstream, repo = two_repos + assert git_publish.read_allowed_push_urls() is None # no .wikitool-remotes.json in this repo + + _write(upstream, "kb/CONTRACT.md", "stack kb contract v2\n") + _commit(upstream, "upstream changes kb/CONTRACT.md") + + _merge() + + captured = capsys.readouterr() + assert "WARN" in captured.out + assert ".wikitool-remotes.json" in captured.out + + +def test_merge_stays_silent_when_the_publish_remote_gate_is_armed(two_repos, capsys): + upstream, repo = two_repos + (repo / config.PUBLISH_REMOTES_FILENAME).write_text( + '{"schema": 1, "allowed_push_urls": ["ssh://example/test.git"]}\n', encoding="utf-8" + ) + + _write(upstream, "kb/CONTRACT.md", "stack kb contract v2\n") + _commit(upstream, "upstream changes kb/CONTRACT.md") + + _merge() + + captured = capsys.readouterr() + assert "WARN" not in captured.out + + +def test_dist_cmd_contract_only_stages_agree_with_ownership(two_repos): + """Consistency guard for the ownership refactor: `dist_cmd`'s own list of + stage-contract paths and `ownership.is_stack_owned` must not be able to + name a different set of stages - both are sourced from + `ownership.CONTENT_STAGES` now, so a stage added to one and not the other + fails this rather than only surfacing in a real merge.""" + from chemenu.commands import dist_cmd + + assert dist_cmd.CONTRACT_ONLY_STAGES # sanity: the derivation still yields entries + for relative in dist_cmd.CONTRACT_ONLY_STAGES: + assert ownership.is_stack_owned(relative) + + +# --- upstream verify -------------------------------------------------------- + + +def test_verify_is_clean_on_a_stack_owned_only_change(two_repos): + upstream, repo = two_repos + since = _git(repo, "rev-parse", "HEAD").stdout.strip() + + _write(repo, "kb/CONTRACT.md", "stack kb contract v2\n") + _commit(repo, "advance kb/CONTRACT.md") + + upstream_cmd.verify_command(since=since, until="HEAD") # must not raise + + +def test_verify_fails_on_a_hand_botched_merge(two_repos, capsys): + upstream, repo = two_repos + since = _git(repo, "rev-parse", "HEAD").stdout.strip() + + _write(repo, "kb/SneakedIn.md", "content that arrived outside a stack-owned path\n") + _commit(repo, "a hand-resolved merge that let content through") + + with pytest.raises(typer.Exit) as excinfo: + upstream_cmd.verify_command(since=since, until="HEAD") + assert excinfo.value.exit_code == 1 + + captured = capsys.readouterr() + assert "kb/SneakedIn.md" in captured.out + + +# --- ownership predicate, exercised directly --------------------------------- + + +@pytest.mark.parametrize( + "relative,expected", + [ + ("kb/CONTRACT.md", True), + ("raw/CONTRACT.md", True), + ("work/CONTRACT.md", True), + ("reports/CONTRACT.md", True), + ("kb/CONVENTIONS.md.template", True), + ("kb/entities/COLLECTION.md.template", True), + ("kb/GLOSSARY.md.template", True), + ("kb/CONVENTIONS.md", False), + ("kb/entities/COLLECTION.md", False), + ("kb/concepts/Some Page.md", False), + ("raw/notes/x.md", False), + ("tools/CONTRACT.md", False), # not a content stage + ("kb/log.md", False), # export stub, not stack-owned + ], +) +def test_is_stack_owned(relative, expected): + assert ownership.is_stack_owned(relative) == expected