fix: upstream merge - preserve gitignored local data, refuse a merge git never opened (4.5.0-beta.3, #30)
Files changed: - CHANGES.md - VERSION - docs/ownership-and-templates.md - instructions/gates.md - instructions/private-instance.md - tools/CONTRACT.md - tools/chemenu/commands/upstream_cmd.py - tools/chemenu/tests/test_upstream_cmd.py
This commit is contained in:
+37
-2
@@ -35,7 +35,7 @@ dev-checkout concern - readable here, never shipped as something to parse.
|
|||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## 4.5.0-beta.2 - 2026-09-04 - upstream merge: combined-commit regression test (edit+add+delete+contract+template+contract-delete in one commit)
|
## 4.5.0-beta.3 - 2026-09-04 - upstream merge: keep gitignored local data under a content stage, refuse a merge git never opened, report what actually changed
|
||||||
|
|
||||||
**Author:** Torben Nehmer
|
**Author:** Torben Nehmer
|
||||||
|
|
||||||
@@ -43,6 +43,7 @@ dev-checkout concern - readable here, never shipped as something to parse.
|
|||||||
- Faktenkorrekturen in der 4.4.0-Prosa; DEVELOPMENT.md ohne zweite Kommandobeschreibung
|
- 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
|
- wikitool upstream merge/verify: code procedure for taking a stack update, ownership.py as the shared stack/instance boundary
|
||||||
- upstream merge: combined-commit regression test (edit+add+delete+contract+template+contract-delete in one commit)
|
- upstream merge: combined-commit regression test (edit+add+delete+contract+template+contract-delete in one commit)
|
||||||
|
- upstream merge: keep gitignored local data under a content stage, refuse a merge git never opened, report what actually changed
|
||||||
<!-- /wikitool:bumps -->
|
<!-- /wikitool:bumps -->
|
||||||
|
|
||||||
Die Prosa zu 4.4.0 - Changelog-Eintrag, `docs/version-model.md`, `instructions/dev/version-parts.md`,
|
Die Prosa zu 4.4.0 - Changelog-Eintrag, `docs/version-model.md`, `instructions/dev/version-parts.md`,
|
||||||
@@ -109,7 +110,7 @@ Nachkontrolle als der Sicherung, die hier tatsächlich trägt.
|
|||||||
auszuschreiben; die Pfadtabelle bleibt als Erklärung stehen, ist aber nicht mehr die operative
|
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.
|
Liste. Vorschlag B (eigenes Demo-Repo) bleibt zurückgestellt, siehe #30.
|
||||||
|
|
||||||
19 neue Tests unter `test_upstream_cmd.py` (31 Fälle mit der parametrisierten
|
23 neue Tests unter `test_upstream_cmd.py` (35 Fälle mit der parametrisierten
|
||||||
`is_stack_owned`-Tabelle) decken die Fälle aus der Spezifikation ab:
|
`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,
|
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
|
`kb/entities/COLLECTION.md` bleibt lokal, eine gelöschte `raw/CONTRACT.md` landet, ein neuer
|
||||||
@@ -124,6 +125,40 @@ Contract-Änderung, eine Template-Änderung und eine Contract-Löschung gleichze
|
|||||||
als `test_one_upstream_commit_mixing_every_case_at_once` nachgetragen; alle sechs Erwartungen in
|
als `test_one_upstream_commit_mixing_every_case_at_once` nachgetragen; alle sechs Erwartungen in
|
||||||
einem `upstream merge`-Aufruf verifiziert.
|
einem `upstream merge`-Aufruf verifiziert.
|
||||||
|
|
||||||
|
**Vierter Bump: zwei Fehler, die ein Review-Durchgang nach dem Publish gefunden hat.** Beide
|
||||||
|
waren in der ersten Fassung enthalten, beide hätten Daten vernichtet, und keiner der bestehenden
|
||||||
|
Tests hat sie berührt.
|
||||||
|
|
||||||
|
*Erstens: die Content-Stage wurde als Ganzes gelöscht.* `_restore_stage_to_local` hieß in der
|
||||||
|
ersten Fassung `shutil.rmtree(stage_dir)` — die wörtliche Übersetzung des `rm -rf kb raw` aus der
|
||||||
|
Prosa-Prozedur. Für `kb/` und `raw/` ist das harmlos, weil dort nichts Ignoriertes liegt. Für die
|
||||||
|
beiden Stages, die dieses Issue *neu* in den Satz aufgenommen hat, ist es das nicht: `reports/`
|
||||||
|
ist bis auf seinen Contract komplett gitignored und trägt genau die Daten, die nirgends sonst
|
||||||
|
existieren — die Telemetrie-Traces, aus denen `eval score` liest, gespeicherte Eval-Berichte,
|
||||||
|
alte Lint-Reports. In dieser Instanz standen zum Zeitpunkt des Fundes 497 Trace-Verzeichnisse
|
||||||
|
unter `reports/telemetry/`; ein einziger `upstream merge` hätte sie alle gelöscht, und zwar
|
||||||
|
stillschweigend, weil git von ignorierten Dateien nichts meldet. Die Stage wird jetzt über die
|
||||||
|
**getrackten** Pfade beider Bäume zurückgesetzt statt über das Verzeichnis; ignorierte lokale
|
||||||
|
Daten bleiben unberührt. Leergewordene Verzeichnisse werden aufgeräumt, aber nur wirklich leere.
|
||||||
|
|
||||||
|
*Zweitens: ein Merge, den git nie eröffnet hat, hätte die Maschinerie gelöscht.* Der Exit-Code
|
||||||
|
von `git merge --no-commit --no-ff` wird bewusst ignoriert (Konflikte unter den Content-Stages
|
||||||
|
sind erwartet). Nur: wenn git das Merge gar nicht erst eröffnet — unverwandte Historien, eine
|
||||||
|
ignorierte Datei im Weg —, gibt es kein `MERGE_HEAD`, `_tree_paths("MERGE_HEAD")` liefert die
|
||||||
|
leere Menge, und **jeder** stack-eigene Pfad in `HEAD` fällt damit in den Zweig „der Upstream hat
|
||||||
|
ihn gelöscht": `kb/CONTRACT.md`, `raw/CONTRACT.md` und sämtliche Templates werden entfernt. Der
|
||||||
|
Kommando-Ablauf prüft jetzt nach dem Merge-Aufruf, dass tatsächlich ein Merge offen ist, und
|
||||||
|
bricht sonst ab, ohne den Baum angefasst zu haben. Beide Fehler haben je einen Regressionstest,
|
||||||
|
und beide Tests wurden gegen die alte Fassung laufen gelassen, um zu zeigen, dass sie sie
|
||||||
|
tatsächlich fangen.
|
||||||
|
|
||||||
|
Dazu eine Ehrlichkeitskorrektur an der Erfolgsmeldung: sie zählte die *wiederhergestellten*
|
||||||
|
Pfade, nicht die geänderten — ein Merge, der eine Datei bewegt, meldete vier oder fünf. Sie fragt
|
||||||
|
jetzt `git diff` zwischen Vor- und Nach-Commit, kennzeichnet Löschungen, und stimmt damit mit dem
|
||||||
|
überein, was ein Leser nachprüfen würde. `docs/ownership-and-templates.md` hat einen Abschnitt
|
||||||
|
bekommen, warum die Grenze ein Prädikat und keine Liste ist — die Begründung, die dieses Issue
|
||||||
|
erarbeitet hat, gehörte in die Hintergrunddoku und nicht nur in einen Changelog-Eintrag.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## 4.4.0 - 2026-09-03 - Versionskandidat statt Bump-pro-Release: VERSION traegt -beta.N, version release fixiert
|
## 4.4.0 - 2026-09-03 - Versionskandidat statt Bump-pro-Release: VERSION traegt -beta.N, version release fixiert
|
||||||
|
|||||||
@@ -42,6 +42,25 @@ Keeping the boundary at the file level, rather than trying to merge changes with
|
|||||||
file, means an upgrade never has to guess which lines are "stack" and which are "instance" -
|
file, means an upgrade never has to guess which lines are "stack" and which are "instance" -
|
||||||
the file itself already answers that.
|
the file itself already answers that.
|
||||||
|
|
||||||
|
## Why the boundary is a predicate rather than a list
|
||||||
|
|
||||||
|
For a while the boundary was written down as a list of paths - once in `dist_cmd.py`, once in
|
||||||
|
the merge procedure a private instance was told to run by hand, and once in the check that
|
||||||
|
procedure ended with. Three copies of one fact, which is the shape [AGENTS.md](../AGENTS.md)
|
||||||
|
invariant 8 exists to forbid, and they drifted exactly as predicted: the hand-run procedure was
|
||||||
|
still naming three paths after the collection contracts had moved to the instance's side of the
|
||||||
|
line, so it discarded upstream changes to files it had never heard of, while its own final check
|
||||||
|
excluded the same three paths and therefore reported success.
|
||||||
|
|
||||||
|
`chemenu/ownership.py` replaced the lists with one question - is this path, under a content
|
||||||
|
stage, the stack's or the instance's? - answered by shape rather than by enumeration:
|
||||||
|
`<stage>/CONTRACT.md`, and anything ending `.template`. Both consumers ask it, so `dist export`
|
||||||
|
and `wikitool upstream merge` cannot disagree, and a machinery file added under a content stage
|
||||||
|
tomorrow is recognised by both without either being edited. The deeper point is not the
|
||||||
|
deduplication: a list has to be maintained by whoever remembers it exists, and the failure mode
|
||||||
|
when nobody does is silence, because a path the list has never heard of simply looks like
|
||||||
|
content.
|
||||||
|
|
||||||
## Why a `.template`, not just an absent file
|
## Why a `.template`, not just an absent file
|
||||||
|
|
||||||
The mechanism for instance-owned content is a `.template` file the distribution ships instead
|
The mechanism for instance-owned content is a `.template` file the distribution ships instead
|
||||||
|
|||||||
@@ -117,9 +117,11 @@ This is not a hole to patch by making `upstream merge` route through the gate: t
|
|||||||
question ("is this too much to publish?") does not apply to a change that only ever touches
|
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
|
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
|
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
|
against `upstream verify`'s logic immediately after committing, and exits 1 with the offending
|
||||||
state is not rolled back; the commit is left for a human to look at) if anything landed outside
|
paths if anything landed outside a stack-owned one. **The merge commit is deliberately left in
|
||||||
a stack-owned path. That is the safeguard for this command, not the Mass-Update Gate.
|
place** rather than reverted: it exists, a human has to look at it, and a command that quietly
|
||||||
|
repaired its own mistake would hide the one event worth seeing. That postcheck is the safeguard
|
||||||
|
for this command, not the Mass-Update Gate.
|
||||||
|
|
||||||
## Iteration Budget Gate and loop-breaker
|
## Iteration Budget Gate and loop-breaker
|
||||||
|
|
||||||
|
|||||||
@@ -131,6 +131,12 @@ Everything else under `kb/`, `raw/`, `work/` and `reports/` is yours, `kb/CONVEN
|
|||||||
each `kb/<name>/COLLECTION.md` included - they bind your corpus, and they are exactly what
|
each `kb/<name>/COLLECTION.md` included - they bind your corpus, and they are exactly what
|
||||||
`upstream merge` protects.
|
`upstream merge` protects.
|
||||||
|
|
||||||
|
**Your local, uncommitted-by-design files under those stages survive.** Forcing a content stage
|
||||||
|
back to your own state removes only what git tracks, never the directory wholesale - which
|
||||||
|
matters because `reports/` is gitignored apart from its contract, so it holds data that is in no
|
||||||
|
commit and cannot be recomputed: the telemetry traces `eval score` reads, saved eval reports,
|
||||||
|
past lint reports. A merge has no business touching any of it, and does not.
|
||||||
|
|
||||||
The command itself checks its own result the same way `upstream verify` would, immediately
|
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
|
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
|
outside a stack-owned path. A refusal there is a bug report, not something to work around by
|
||||||
|
|||||||
+2
-2
@@ -82,7 +82,7 @@ tools/wikitool <command> --help
|
|||||||
| `migrate verify --from <rev> [--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 verify --from <rev> [--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 <version> [--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 done <version> [--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 <version> [--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 |
|
| `migrate baseline <version> [--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 `<remote>/<branch>` (unless `--no-fetch`) and reports "already up to date" if nothing new exists. Otherwise opens `git merge --no-commit --no-ff <remote>/<branch>`, 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 (`<stage>/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 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 `<remote>/<branch>` (unless `--no-fetch`) and reports "already up to date" if nothing new exists. Otherwise opens `git merge --no-commit --no-ff <remote>/<branch>` - and stops, untouched, if git refused to open a merge at all (unrelated histories), since without a `MERGE_HEAD` every stack path would read as "the upstream deleted it". Then forces every content stage (`kb/`, `raw/`, `work/`, `reports/`) back to the local side by removing **only the paths tracked in either tree** and checking `HEAD`'s back out - never the stage directory wholesale, because `reports/` is gitignored apart from its contract and holds local, non-recomputable data (telemetry traces `eval score` reads, saved eval and lint reports) that no merge has business deleting. Then restores from the upstream side exactly the paths `chemenu.ownership.is_stack_owned` recognises as machinery (`<stage>/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 <rev> [--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` |
|
| `upstream verify --since <rev> [--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 |
|
| `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 |
|
||||||
|
|
||||||
@@ -201,7 +201,7 @@ 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 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 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 |
|
| `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 merge` | Dirty working tree, a merge already in progress, the remote does not resolve, git refused to open the merge at all (unrelated histories), 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 |
|
| `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 |
|
| `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` |
|
| `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` |
|
||||||
|
|||||||
@@ -22,7 +22,6 @@ same way.
|
|||||||
"""
|
"""
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import shutil
|
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Optional
|
from typing import Optional
|
||||||
|
|
||||||
@@ -110,6 +109,52 @@ def _stack_paths_changed(since: str, until: str) -> list[str]:
|
|||||||
# --- upstream merge ---------------------------------------------------------
|
# --- upstream merge ---------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def _prune_empty_dirs(stage: str) -> None:
|
||||||
|
"""Remove directories left empty under `stage` after tracked files were
|
||||||
|
deleted. git tracks no directories, so an emptied one is invisible to
|
||||||
|
`git status` and would otherwise linger in the working tree as litter -
|
||||||
|
an empty `kb/<area>/` that only ever existed in the upstream's corpus.
|
||||||
|
Never touches a directory that still holds anything, ignored files
|
||||||
|
included."""
|
||||||
|
stage_dir = config.ROOT / stage
|
||||||
|
if not stage_dir.is_dir():
|
||||||
|
return
|
||||||
|
for path in sorted(stage_dir.rglob("*"), key=lambda p: len(p.parts), reverse=True):
|
||||||
|
if path.is_dir() and not any(path.iterdir()):
|
||||||
|
path.rmdir()
|
||||||
|
|
||||||
|
|
||||||
|
def _restore_stage_to_local(stage: str, tracked_paths: set[str]) -> None:
|
||||||
|
"""Force one content stage back to the local (HEAD) side, whatever the
|
||||||
|
merge did to it.
|
||||||
|
|
||||||
|
Deletes **only what git tracks on either side** - never the stage
|
||||||
|
directory wholesale. That distinction is the whole point of this function:
|
||||||
|
`reports/` is gitignored except its contract (see .gitignore), so a
|
||||||
|
content stage's working tree legitimately holds local data that is not in
|
||||||
|
any tree and not recomputable - the telemetry traces `eval score` reads,
|
||||||
|
saved eval reports, past lint reports. A blanket `rm -rf` of the stage
|
||||||
|
takes all of it out as collateral for a merge that was never about it.
|
||||||
|
|
||||||
|
Handles a stage that exists only in MERGE_HEAD too (the upstream
|
||||||
|
introduced it): what the merge wrote is removed, and there is simply
|
||||||
|
nothing to check out from HEAD afterwards.
|
||||||
|
"""
|
||||||
|
prefix = f"{stage}/"
|
||||||
|
stage_paths = [p for p in tracked_paths if p.startswith(prefix)]
|
||||||
|
if not stage_paths:
|
||||||
|
return
|
||||||
|
|
||||||
|
_run(["git", "rm", "-rq", "--cached", "--ignore-unmatch", stage])
|
||||||
|
for relative in stage_paths:
|
||||||
|
target = config.ROOT / relative
|
||||||
|
if target.is_file() or target.is_symlink():
|
||||||
|
target.unlink()
|
||||||
|
_prune_empty_dirs(stage)
|
||||||
|
if _tree_has_path("HEAD", stage):
|
||||||
|
_run(["git", "checkout", "HEAD", "--", stage])
|
||||||
|
|
||||||
|
|
||||||
def _remote_gate_warning() -> None:
|
def _remote_gate_warning() -> None:
|
||||||
if git_publish.read_allowed_push_urls() is not None:
|
if git_publish.read_allowed_push_urls() is not None:
|
||||||
return
|
return
|
||||||
@@ -167,16 +212,28 @@ def _postcheck_failure_message(leaks: list[str], before: str) -> str:
|
|||||||
|
|
||||||
|
|
||||||
def _merge_success_message(
|
def _merge_success_message(
|
||||||
updated: list[str], deleted: list[str], remote: str, branch: str
|
changed: list[str], deleted: list[str], remote: str, branch: str
|
||||||
) -> str:
|
) -> str:
|
||||||
lines = [f"Merged {remote}/{branch}. Content stages ({', '.join(ownership.CONTENT_STAGES)}) are unchanged."]
|
"""What the merge actually did, measured against the pre-merge commit
|
||||||
if updated:
|
rather than against what was restored.
|
||||||
lines.append(f"Stack paths updated ({len(updated)}):")
|
|
||||||
lines += [f" - {p}" for p in updated]
|
`changed` is the real diff - restoring every stack-owned path from
|
||||||
if deleted:
|
MERGE_HEAD touches each of them whether or not the upstream moved any, so
|
||||||
lines.append(f"Stack paths removed, following the upstream ({len(deleted)}):")
|
reporting the restore list would claim seven updates for a merge that
|
||||||
lines += [f" - {p}" for p in deleted]
|
changed one file, and a reader who checks would find the report wrong.
|
||||||
if not updated and not deleted:
|
"""
|
||||||
|
deleted_set = set(deleted)
|
||||||
|
lines = [
|
||||||
|
f"Merged {remote}/{branch}. Content stages "
|
||||||
|
f"({', '.join(ownership.CONTENT_STAGES)}) are unchanged."
|
||||||
|
]
|
||||||
|
if changed:
|
||||||
|
lines.append(f"Stack paths changed ({len(changed)}):")
|
||||||
|
lines += [
|
||||||
|
f" - {p}" + (" (deleted, following the upstream)" if p in deleted_set else "")
|
||||||
|
for p in changed
|
||||||
|
]
|
||||||
|
else:
|
||||||
lines.append("No stack-owned path changed.")
|
lines.append("No stack-owned path changed.")
|
||||||
return "\n".join(lines)
|
return "\n".join(lines)
|
||||||
|
|
||||||
@@ -224,24 +281,36 @@ def merge_command(
|
|||||||
success(f"Already up to date with {remote_ref}.")
|
success(f"Already up to date with {remote_ref}.")
|
||||||
return
|
return
|
||||||
|
|
||||||
_run(["git", "merge", "--no-commit", "--no-ff", remote_ref])
|
# The exit code is deliberately not the test - conflicts under the content
|
||||||
|
# stages are expected here and are exactly what the next steps undo. What
|
||||||
for stage in ownership.CONTENT_STAGES:
|
# *is* load-bearing is that a merge actually opened: without MERGE_HEAD,
|
||||||
if not _tree_has_path("HEAD", stage):
|
# `_tree_paths("MERGE_HEAD")` is empty, and every stack-owned path in HEAD
|
||||||
continue
|
# would then read as "the upstream deleted it" and be removed. A merge git
|
||||||
_run(["git", "rm", "-rq", "--cached", "--ignore-unmatch", stage])
|
# refused to start (unrelated histories, an ignored file in the way) must
|
||||||
stage_dir = config.ROOT / stage
|
# therefore stop here, with the tree untouched.
|
||||||
if stage_dir.exists():
|
merge_result = _run(["git", "merge", "--no-commit", "--no-ff", remote_ref])
|
||||||
shutil.rmtree(stage_dir)
|
if not _merge_in_progress():
|
||||||
_run(["git", "checkout", "HEAD", "--", stage])
|
fail(
|
||||||
|
f"`git merge --no-commit --no-ff {remote_ref}` did not open a merge, so there is "
|
||||||
|
f"nothing to scope - the working tree is unchanged:\n"
|
||||||
|
f"{merge_result.stdout}{merge_result.stderr}"
|
||||||
|
)
|
||||||
|
return
|
||||||
|
|
||||||
merge_head_paths = _tree_paths("MERGE_HEAD")
|
merge_head_paths = _tree_paths("MERGE_HEAD")
|
||||||
head_paths = _tree_paths("HEAD")
|
head_paths = _tree_paths("HEAD")
|
||||||
|
tracked_paths = merge_head_paths | head_paths
|
||||||
|
|
||||||
|
for stage in ownership.CONTENT_STAGES:
|
||||||
|
_restore_stage_to_local(stage, tracked_paths)
|
||||||
|
|
||||||
stack_paths = sorted(
|
stack_paths = sorted(
|
||||||
p for p in (merge_head_paths | head_paths) if ownership.is_stack_owned(p)
|
p for p in (merge_head_paths | head_paths) if ownership.is_stack_owned(p)
|
||||||
)
|
)
|
||||||
|
|
||||||
updated: list[str] = []
|
# Only the deletions are recorded: what was *restored* is every stack-owned
|
||||||
|
# path in MERGE_HEAD, which is not the same question as what changed - the
|
||||||
|
# success message asks git for that instead.
|
||||||
deleted: list[str] = []
|
deleted: list[str] = []
|
||||||
for relative in stack_paths:
|
for relative in stack_paths:
|
||||||
if relative in merge_head_paths:
|
if relative in merge_head_paths:
|
||||||
@@ -252,7 +321,6 @@ def merge_command(
|
|||||||
f"in MERGE_HEAD's own tree:\n{checkout.stderr}\nThe merge is left open."
|
f"in MERGE_HEAD's own tree:\n{checkout.stderr}\nThe merge is left open."
|
||||||
)
|
)
|
||||||
return
|
return
|
||||||
updated.append(relative)
|
|
||||||
else:
|
else:
|
||||||
_run(["git", "rm", "-q", "--cached", "--ignore-unmatch", relative])
|
_run(["git", "rm", "-q", "--cached", "--ignore-unmatch", relative])
|
||||||
target = config.ROOT / relative
|
target = config.ROOT / relative
|
||||||
@@ -275,7 +343,9 @@ def merge_command(
|
|||||||
fail(_postcheck_failure_message(leaks, before))
|
fail(_postcheck_failure_message(leaks, before))
|
||||||
return
|
return
|
||||||
|
|
||||||
success(_merge_success_message(updated, deleted, remote, branch))
|
success(
|
||||||
|
_merge_success_message(_stack_paths_changed(before, "HEAD"), deleted, remote, branch)
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
# --- upstream verify ---------------------------------------------------------
|
# --- upstream verify ---------------------------------------------------------
|
||||||
|
|||||||
@@ -55,7 +55,16 @@ def two_repos(tmp_path, monkeypatch):
|
|||||||
# fixture during a test would show up as an untracked file and trip 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
|
# dirty-working-tree precondition for a reason that has nothing to do with
|
||||||
# what that test is checking.
|
# what that test is checking.
|
||||||
_write(seed, ".gitignore", f"{config.PUBLISH_REMOTES_FILENAME}\n")
|
# Mirrors the real .gitignore in the two ways that matter here:
|
||||||
|
# `.wikitool-remotes.json` is per-checkout (dropping one in during a test
|
||||||
|
# must not read as a dirty tree), and `reports/` is derived output that is
|
||||||
|
# ignored except for its contract - which is what makes a content stage
|
||||||
|
# able to hold local, non-recomputable data a merge must not touch.
|
||||||
|
_write(
|
||||||
|
seed,
|
||||||
|
".gitignore",
|
||||||
|
f"/{config.PUBLISH_REMOTES_FILENAME}\n/reports/*\n!/reports/CONTRACT.md\n",
|
||||||
|
)
|
||||||
_write(seed, "kb/CONTRACT.md", "stack kb contract v1\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.template", "template v1\n")
|
||||||
_write(seed, "kb/CONVENTIONS.md", "instance conventions v1\n")
|
_write(seed, "kb/CONVENTIONS.md", "instance conventions v1\n")
|
||||||
@@ -208,6 +217,47 @@ def test_open_workshop_run_files_do_not_land(two_repos):
|
|||||||
assert not (repo / "work/some-run").exists()
|
assert not (repo / "work/some-run").exists()
|
||||||
|
|
||||||
|
|
||||||
|
def test_merge_keeps_ignored_local_data_under_a_content_stage(two_repos):
|
||||||
|
"""`reports/` is gitignored except its contract, so a content stage's
|
||||||
|
working tree holds local data that is in no git tree and is not
|
||||||
|
recomputable - the telemetry traces `eval score` reads, saved eval
|
||||||
|
reports, past lint reports. Forcing the stage back to the local side must
|
||||||
|
not take those out as collateral: this instance had 497 trace directories
|
||||||
|
under reports/telemetry/ when the first version of this command wiped the
|
||||||
|
stage wholesale."""
|
||||||
|
upstream, repo = two_repos
|
||||||
|
_write(repo, "reports/telemetry/session-a/trace.jsonl", '{"event": "local"}\n')
|
||||||
|
_write(repo, "reports/Lint Report 2026-09-04.md", "a local lint report\n")
|
||||||
|
assert _git(repo, "status", "--porcelain").stdout == "" # ignored, so the tree is clean
|
||||||
|
|
||||||
|
_write(upstream, "kb/CONTRACT.md", "stack kb contract v2\n")
|
||||||
|
_commit(upstream, "upstream changes kb/CONTRACT.md")
|
||||||
|
|
||||||
|
_merge()
|
||||||
|
|
||||||
|
assert (repo / "reports/telemetry/session-a/trace.jsonl").read_text(encoding="utf-8") == (
|
||||||
|
'{"event": "local"}\n'
|
||||||
|
)
|
||||||
|
assert (repo / "reports/Lint Report 2026-09-04.md").exists()
|
||||||
|
assert (repo / "reports/CONTRACT.md").read_text(encoding="utf-8") == "reports contract v1\n"
|
||||||
|
|
||||||
|
|
||||||
|
def test_upstream_content_under_a_stage_absent_from_head_does_not_land(two_repos):
|
||||||
|
"""The stage guard must not rest on the local side happening to track
|
||||||
|
something under that stage: an instance whose `work/` holds no tracked
|
||||||
|
file at all must still not receive the upstream's open run."""
|
||||||
|
upstream, repo = two_repos
|
||||||
|
_git(repo, "rm", "-q", "work/CONTRACT.md")
|
||||||
|
_commit(repo, "instance has nothing tracked under work/")
|
||||||
|
|
||||||
|
_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_one_upstream_commit_mixing_every_case_at_once(two_repos):
|
def test_one_upstream_commit_mixing_every_case_at_once(two_repos):
|
||||||
"""The acceptance test from the issue: a single upstream commit that edits
|
"""The acceptance test from the issue: a single upstream commit that edits
|
||||||
a page the instance deleted, adds a new page, deletes an untouched page,
|
a page the instance deleted, adds a new page, deletes an untouched page,
|
||||||
@@ -255,6 +305,51 @@ def test_real_conflict_in_tools_leaves_the_merge_open(two_repos):
|
|||||||
assert (repo / ".git" / "MERGE_HEAD").exists()
|
assert (repo / ".git" / "MERGE_HEAD").exists()
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_merge_git_refuses_to_open_deletes_nothing(two_repos, tmp_path):
|
||||||
|
"""The failure mode with the worst blast radius if it is not guarded:
|
||||||
|
without MERGE_HEAD, every stack-owned path in HEAD reads as "the upstream
|
||||||
|
deleted it", and the restore loop would remove kb/CONTRACT.md,
|
||||||
|
raw/CONTRACT.md and every template. A merge git refuses to start must stop
|
||||||
|
before that, with the tree untouched."""
|
||||||
|
upstream, repo = two_repos
|
||||||
|
unrelated = tmp_path / "unrelated"
|
||||||
|
unrelated.mkdir()
|
||||||
|
_git(unrelated, "init", "-b", "main")
|
||||||
|
_git(unrelated, "config", "user.name", "Unrelated")
|
||||||
|
_git(unrelated, "config", "user.email", "unrelated@example.com")
|
||||||
|
_write(unrelated, "somefile.md", "no shared history with the instance\n")
|
||||||
|
_commit(unrelated, "unrelated root commit")
|
||||||
|
|
||||||
|
_git(repo, "remote", "set-url", "upstream", str(unrelated))
|
||||||
|
before = _git(repo, "rev-parse", "HEAD").stdout.strip()
|
||||||
|
|
||||||
|
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") == "stack kb contract v1\n"
|
||||||
|
assert (repo / "raw/CONTRACT.md").exists()
|
||||||
|
assert (repo / "kb/CONVENTIONS.md.template").exists()
|
||||||
|
assert _git(repo, "status", "--porcelain").stdout == ""
|
||||||
|
|
||||||
|
|
||||||
|
def test_success_message_reports_what_changed_not_what_was_restored(two_repos, capsys):
|
||||||
|
"""Restoring every stack-owned path from MERGE_HEAD touches all of them
|
||||||
|
whether or not the upstream moved any, so the report has to ask git what
|
||||||
|
changed - otherwise a one-file update is announced as five."""
|
||||||
|
upstream, repo = two_repos
|
||||||
|
_write(upstream, "kb/CONTRACT.md", "stack kb contract v2\n")
|
||||||
|
_commit(upstream, "upstream changes exactly one stack path")
|
||||||
|
|
||||||
|
_merge()
|
||||||
|
|
||||||
|
out = capsys.readouterr().out
|
||||||
|
assert "Stack paths changed (1)" in out
|
||||||
|
assert "kb/CONTRACT.md" in out
|
||||||
|
assert "kb/CONVENTIONS.md.template" not in out
|
||||||
|
|
||||||
|
|
||||||
def test_dirty_working_tree_is_refused_untouched(two_repos):
|
def test_dirty_working_tree_is_refused_untouched(two_repos):
|
||||||
upstream, repo = two_repos
|
upstream, repo = two_repos
|
||||||
before = _git(repo, "rev-parse", "HEAD").stdout.strip()
|
before = _git(repo, "rev-parse", "HEAD").stdout.strip()
|
||||||
|
|||||||
Reference in New Issue
Block a user