From 5e24bd31ff69b6f472eed4a72ff24a0a6bec1ce7 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 7 Sep 2026 13:02:16 -0700 Subject: [PATCH 1/7] Give the Verbatim Section Re-Vendor a Tested Tool Every resync hand-rolls the verbatim section re-vendor. `carry.py` owns whole trees and `spec/audit.py` names a stale section without writing anything, and between them sits the one apply step in `RESYNC.md` with no tool behind it, which is the step the AGENTS.md-overwrite incident happened on. Add `check-sections` and `apply-sections` to `carry.py`. They read `spec/files.json` for the sections a repository carries at verbatim fidelity, replace only those regions, and then re-read the file to assert that the level-two heading sequence is unchanged, that every line outside the replaced regions is byte-identical to what was there before, and that each declared section now matches the hub's canonical. The second assertion is the one that earns its keep. The 2.0.577 pilot's throwaway replacer dropped one blank line between the preamble and the first heading, which is invisible in a diff review, breaks no renderer, and no gate in this repository flags it. A mutation test proves each of the six new guards fails its own case when removed. Region boundaries come from `spec/audit.py`'s own fence reading rather than a second reading of CommonMark, so a heading shown inside a code sample cannot end a region for this tool and not for the check that judges the result. The replacement carries the terminator the section's own heading line carried, so a CRLF repository keeps its endings. Four states refuse rather than guess: a declared section the repository does not carry, an absent file, a file declaring both placeholders and verbatim sections, and a region holding content the fidelity comparison normalizes away. Verified read-only against three fleet repositories at their fetched origin/develop: 63 sections compared, zero disagreements with `spec/audit.py`'s own extractor, and the verdicts reproduce the known state, Utilities and PlexCleaner clean after their resyncs and HomeAutomation-Config ten sections behind on its frozen hub pin. Co-Authored-By: Claude Opus 5 (1M context) --- RESYNC.md | 1 + host-setup/menu.sh | 10 + scripts/README.md | 11 +- scripts/carry.py | 360 +++++++++++++++++++++++++++++++++++- scripts/tests/test_carry.py | 251 +++++++++++++++++++++++++ 5 files changed, 629 insertions(+), 4 deletions(-) diff --git a/RESYNC.md b/RESYNC.md index 2fb374bee..a65d8639c 100644 --- a/RESYNC.md +++ b/RESYNC.md @@ -75,6 +75,7 @@ The order is load-bearing. Each step below either changes the rules the later st 2. **Deletions second, before any re-vendor.** A `hub-only:` finding names a file the hub hosts rather than carries, and its remedy removes the file. Doing it after the re-vendors means refreshing a copy that is about to be deleted, which is wasted work that also reads as a deliberate update in the diff. See section 4, which is the whole of what deletion means here. 3. **Verbatim re-vendors.** Copy the current hub canonical down, whole file or the one named `## heading` region. A finding classified **stale** matches a past hub revision and needs no judgment. One classified **modified** matches no revision, so the repository changed fixed content and the change is read before it is overwritten, since it may be an improvement the hub should adopt instead. Run `python3 scripts/carry.py check --target /path/to/worktree` for manifest-owned trees. Read each modified-file diff and every extra path before running the same command with `apply`. The tree declaration supplies the prune authority, so the carried-instruction-file guard does not apply to this fully owned content. + Run `python3 scripts/carry.py check-sections --target /path/to/worktree` for the verbatim `## heading` regions inside a mixed file, then `apply-sections` once each reported region's diff has been read. It replaces only the regions the manifest declares verbatim and asserts afterwards that the preamble, the heading sequence, and every other region came through byte-identical, which is the assertion a hand-rolled replacer only gets when whoever wrote it happened to write one. It refuses rather than guessing where a declared section is absent from the repository, since that is a standup question rather than a drift question. It narrows what the `carried-instruction-file-guard` probe has to catch by hand without replacing it: a tool that can only touch declared verbatim regions cannot delete a repository's local additions, and the intent half is still nothing mechanical can judge. 4. **Interface workflows.** Honor the named contract (required jobs, the ruleset-bound check name, the artifact-name handoff) rather than copying bytes. The body is the repository's own. 5. **Settings, rulesets, and secrets.** From a hub checkout at `main`, run `repo-config/configure.sh check / release|operational`, then `apply` for what it reports. Pass the model explicitly. Delete each target-repository path under `repo-config/` that the audit classified `retire`. Inspect every untriaged path collision before deciding its disposition. 6. **Intent files last, and by hand.** See section 5, which states why these carry no mechanical signal at all. diff --git a/host-setup/menu.sh b/host-setup/menu.sh index 69ed63d82..746012c8a 100755 --- a/host-setup/menu.sh +++ b/host-setup/menu.sh @@ -443,6 +443,8 @@ print_menu() { log "Downstream, the repo this menu is run from:" log " 12 Check what the hub would change here, change nothing" log " 13 Pull the hub's verbatim-owned files into this repo" + log " 14 Check the hub's verbatim rule sections inside this repo's mixed files" + log " 15 Pull those rule sections in, leaving every other section alone" fi log "" log " q Quit" @@ -475,6 +477,14 @@ dispatch() { carry_action apply fi ;; + 14) carry_action check-sections ;; + 15) + if [[ $DRY_RUN == true ]]; then + carry_action check-sections + else + carry_action apply-sections + fi + ;; q | Q) QUIT=true ;; *) warn "Not one of the choices" diff --git a/scripts/README.md b/scripts/README.md index 47c19eef8..1ca22fecb 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -16,6 +16,7 @@ python3 scripts/tests/test_local_review.py python3 scripts/tests/test_canonical_review.py python3 scripts/tests/test_build_dist.py python3 scripts/tests/test_skills_install.py +python3 scripts/tests/test_carry.py python3 -m unittest discover -s scripts/tests # all of them, and exits 5 if the suite vanishes uvx coverage@latest run --source=scripts,spec,host-setup -m unittest discover -s scripts/tests && uvx coverage@latest report ``` @@ -280,15 +281,21 @@ A skill has to read whole in isolation and a rule has one home, so the text a sk ## `carry.py` -Copies manifest-owned trees from a freshly fetched ProjectTemplate `main` checkout into an isolated downstream feature worktree. `check` reports the canonical commit, repository identity, applicable declarations, path differences, and tree digests without changing the target. `apply` reports each write or removal and repeats the comparison before it succeeds. +Copies manifest-owned trees, and the verbatim `## heading` regions inside a mixed file, from a freshly fetched ProjectTemplate `main` checkout into an isolated downstream feature worktree. `check` reports the canonical commit, repository identity, applicable declarations, path differences, and tree digests without changing the target. `apply` reports each write or removal and repeats the comparison before it succeeds. ```shell python3 scripts/carry.py check PhotoCleaner --target /path/to/worktree python3 scripts/carry.py apply PhotoCleaner --target /path/to/worktree +python3 scripts/carry.py check-sections PhotoCleaner --target /path/to/worktree +python3 scripts/carry.py apply-sections PhotoCleaner --target /path/to/worktree ``` The tool validates the registry identity, target origin, feature-branch worktree, development-branch ancestry, unrelated changes, root containment, and symlink-free trees. A pruned declaration authorizes removal only beneath its target root. +The section modes close the gap between the two things that already existed: a tree mode owns a whole tree, and [`spec/audit.py`][audit] names a stale section precisely without writing anything, so the one apply step in [`RESYNC.md`][resync] with no tested tool behind it was the step the AGENTS.md-overwrite incident happened on. `check-sections` reports each declared verbatim section and which of them are stale, and exits `1` while any is. `apply-sections` replaces only those regions and then re-reads the file to assert three things: the level-two heading sequence is unchanged, every line outside the replaced regions is byte-identical to what was there before, and each declared section now matches the hub's canonical. The second of those is the one that earns its keep, since the defect it exists for, one blank line dropped between the preamble and the first heading, is invisible in a diff review, breaks no renderer, and no gate in this repository flags it. + +Region boundaries come from `spec/audit.py`'s own fence reading rather than a second reading of CommonMark, so a level-two heading marker shown inside a code sample cannot end a region for this tool and not for the check that judges the result. The replacement carries the terminator the section's own heading line carried, so a CRLF repository keeps its endings and the file is never rewritten to one ending it may not use throughout. Four states refuse rather than guess: a section the manifest declares that the repository does not carry (where it belongs is a standup question), a file absent from the target, a file declaring both placeholders and verbatim sections (a verbatim unit carries none, per [`spec/fidelity-model.md`][fidelity-model]), and a region holding content the fidelity comparison normalizes away, such as a Dependabot-owned action pin, whose bytes the repository rather than the hub owns. + ## `skills_install.py` Installs the fleet's Skills for the current machine, cross-platform and idempotent, mirroring [`host-setup/agent-safety/claude/install.py`][agent-safety-install]'s shape: `skills_install.sh` and `skills_install.ps1` are thin wrappers that locate a Python 3 interpreter and hand off, so every OS runs one tested code path. Two independent things happen on a run, since the three tools this fleet targets discover skills differently: `.agents/skills/` is materialized (not symlinked) to `$HOME/.agents/skills/`, so Codex and opencode's global scan covers every repo on the machine rather than only the one that happens to be open, and this repo's marketplace is registered with the `claude` CLI (`claude plugin marketplace add`, `claude plugin install`) so Claude Code loads the same content the other two read directly. The marketplace/plugin registration goes through the `claude` CLI's own commands rather than writing its internal `known_marketplaces.json` by hand, because that file's shape is the CLI's state, not a documented contract, and a hand-written copy risks drifting from what the CLI expects on its next release. @@ -307,6 +314,7 @@ Installs the fleet's Skills for the current machine, cross-platform and idempote [copilot-instructions]: ../.github/copilot-instructions.md [divergences]: ../spec/divergences.json [editorconfig]: ../.editorconfig +[fidelity-model]: ../spec/fidelity-model.md [files]: ../spec/files.json [fleet-skills-dist]: ../.claude-plugin/fleet-skills/ [github-skills-dist]: ../.github/skills/ @@ -323,6 +331,7 @@ Installs the fleet's Skills for the current machine, cross-platform and idempote [prose-gate-action]: ../.github/actions/prose-gate/action.yml [record-lock-issue]: https://github.com/ptr727/ProjectTemplate/issues/1151 [repos]: ../registry/repos.json +[resync]: ../RESYNC.md [section-model]: ../spec/section-model.md [tests]: ./tests/ [validate-hook]: ../.github/actions/validate/action.yml diff --git a/scripts/carry.py b/scripts/carry.py index 2fda08e82..d925249f8 100755 --- a/scripts/carry.py +++ b/scripts/carry.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -"""Check or apply manifest-owned trees from the hub to a fleet worktree.""" +"""Check or apply manifest-owned trees, and the verbatim sections inside a mixed file, from the hub to a fleet worktree.""" import argparse import fnmatch @@ -7,6 +7,7 @@ import json import os import pathlib +import re import subprocess import sys from dataclasses import dataclass @@ -14,6 +15,9 @@ ROOT = pathlib.Path(__file__).resolve().parent.parent +# The hub authors every canonical, so its own copies are the source a re-vendor reads rather than a target one writes. +HUB_NAME = "ProjectTemplate" + class CarryError(RuntimeError): """A state that prevents a safe carry decision.""" @@ -355,6 +359,354 @@ def verify_target( raise CarryError(f"target has unrelated changes: {', '.join(sorted(dirty))}") +def audit_module() -> Any: + """`spec/audit.py`, imported on first use rather than at module import. + + It is a script rather than a package module and imports its own sibling by bare name, so the + spec directory has to be importable before it is. Lazily, so a tree carry never pays to load + the audit for a reader it does not use. + """ + spec = str(ROOT / "spec") + if spec not in sys.path: + sys.path.insert(0, spec) + import audit + + return audit + + +# A line and its own terminator, splitting on LF alone. `str.splitlines` also breaks on a form feed and on several Unicode separators, which are content in a carried document rather than line boundaries, so splitting on it would move bytes this tool was asked to leave alone. +_LINE = re.compile(r"[^\n]*\n|[^\n]+$") + + +def split_lines(text: str) -> list[str]: + """`text` as lines that keep their own terminators, so joining them returns the original bytes.""" + return _LINE.findall(text) + + +def terminator(line: str) -> str: + """The line's own terminator, or the empty string for a final line that ends the file without one.""" + if line.endswith("\r\n"): + return "\r\n" + return "\n" if line.endswith("\n") else "" + + +def normalize_eol(text: str) -> str: + """`text` with every line ending as LF. + + Deliberately not `spec/audit.py`'s `normalize`, which also masks a Dependabot-owned action pin + and a per-repository `needs:` list. Those are governed drift for a comparison asking whether + two copies say the same thing, and this tool writes bytes, so masking either would let it + overwrite a value the repository owns. Line endings are the one exception, because + `.gitattributes` governs them separately and a re-vendor preserves whichever the target uses. + """ + return text.replace("\r\n", "\n").replace("\r", "\n") + + +def fence_step(line: str, marker: str | None, marker_len: int) -> tuple[str | None, int, bool]: + """`spec/audit.py`'s fence reading, so one CommonMark reading spans the fidelity checks and this. + + A second reading here could disagree with the audit's about where a section stops, and then a + `## ` shown inside a code sample would end a region for one tool and not the other, leaving + the audit reporting drift in text this tool never touched. + """ + return audit_module()._fence_step(line, marker, marker_len) + + +def h2_headings(lines: list[str]) -> list[str]: + """Every level-two heading's parsed text, in order, reading a fenced `## ` as content.""" + out = [] + marker: str | None = None + marker_len = 0 + for line in lines: + marker, marker_len, boundary = fence_step(line, marker, marker_len) + stripped = line.strip() + if not boundary and marker is None and stripped.startswith("## "): + out.append(stripped[2:].strip()) + return out + + +def section_span(lines: list[str], heading: str) -> tuple[int, int] | None: + """The `[start, end)` line range of the `## ` region, or None when the file has none. + + Boundaries are `spec/audit.py`'s `extract_section`: the heading line is inside the region, a + sibling level-two heading ends it, a fenced `## ` is content, and the match is on the parsed + heading text case-folded, so a re-cased heading is found rather than read as absent. The range + is positional rather than the text itself, which is what lets the caller replace the region + without also replacing an identical passage quoted elsewhere in the document. + """ + want = heading.strip().lower() + start = None + marker: str | None = None + marker_len = 0 + for index, line in enumerate(lines): + marker, marker_len, boundary = fence_step(line, marker, marker_len) + stripped = line.strip() + if boundary or marker is not None or not stripped.startswith("## "): + continue + if start is not None: + return start, index # a sibling H2 ends the region + if stripped[2:].strip().lower() == want: + start = index + return None if start is None else (start, len(lines)) + + +def region_text(lines: list[str], span: tuple[int, int]) -> str: + """The exact bytes of the line range, terminators included.""" + return "".join(lines[span[0] : span[1]]) + + +def outside_sections(lines: list[str], sections: list[str], path: str) -> str: + """Every line outside the named regions, joined, which is the text a re-vendor must not touch. + + Includes the preamble, every intent section, and every verbatim section this run left alone, + so comparing it before and after is what proves the write stayed inside the regions it named. + """ + covered: set[int] = set() + for section in sections: + span = section_span(lines, section) + if span is None: + raise CarryError( + f"{path} section '{section}' is absent, so the untouched text cannot be compared" + ) + covered.update(range(*span)) + return "".join(line for index, line in enumerate(lines) if index not in covered) + + +def guard_governed_drift(path: str, section: str, region: str, side: str) -> None: + """Refuse a section carrying content the fidelity comparison normalizes away. + + A `uses: @` pin with its `# vN` comment, and a job's `needs:` list, are owned per + repository (`spec/fidelity-model.md` "Normalization"), so writing the hub's bytes over either + would revert a bump or a prune the repository owns and the audit would report nothing. No + section declared verbatim carries either today, and this refuses rather than discovering it + silently the first time one does. + """ + if audit_module().normalize(region) != normalize_eol(region): + raise CarryError( + f"{path} section '{section}' carries content the fidelity comparison normalizes away" + f" ({side} copy), so re-vendoring its bytes could revert per-repository drift; re-vendor it by hand" + ) + + +def verbatim_section_names(item: dict[str, Any], selectors: set[str]) -> list[str]: + """The section names this repository carries at verbatim fidelity, in manifest order. + + A bare-string entry is `appliesTo` `*` at intent fidelity, matching `spec/audit.py`'s own + reading of the same manifest, so it is never a re-vendor target. + """ + out = [] + for element in item.get("sections", []): + if isinstance(element, str): + continue + if not isinstance(element, dict): + raise CarryError(f"section declaration must be a string or an object: {element!r}") + if element.get("fidelity") != "verbatim": + continue + if not applicable(element.get("appliesTo", "*"), selectors): + continue + name = element.get("name") + if not isinstance(name, str) or not name.strip(): + raise CarryError("section declaration name must be a non-empty string") + if name not in out: + out.append(name) + return out + + +def section_units(manifest: dict[str, Any], selectors: set[str]) -> list[tuple[str, list[str]]]: + """`(path, [section])` for every baseline file this repository carries verbatim sections of. + + One path declared by more than one baseline entry, which the manifest does for a file whose + applicability differs by repository type, merges into one unit, since the file is written once. + """ + baseline = manifest.get("baseline") + if not isinstance(baseline, list): + raise CarryError("manifest baseline must be an array") + units: dict[str, list[str]] = {} + for item in baseline: + if not isinstance(item, dict): + raise CarryError(f"baseline declaration must be an object: {item!r}") + if not applicable(item.get("appliesTo", "*"), selectors): + continue + names = verbatim_section_names(item, selectors) + if not names: + continue + path = item.get("path") + if not isinstance(path, str) or not path: + raise CarryError("baseline declaration path must be a non-empty string") + if not path.endswith(".md"): + raise CarryError( + f"a verbatim section is a Markdown heading region, so it cannot be declared on {path}" + ) + if item.get("placeholders"): + raise CarryError( + f"{path} declares both placeholders and verbatim sections, and a verbatim unit carries" + " no placeholder (spec/fidelity-model.md 'Normalization'), so re-vendoring one could" + " overwrite a substituted value" + ) + merged = units.setdefault(path, []) + merged.extend(name for name in names if name not in merged) + return list(units.items()) + + +def replace_sections( + target_lines: list[str], source_lines: list[str], sections: list[str], path: str +) -> list[str]: + """`target_lines` with each named region replaced by the hub's, every other line untouched. + + Spliced from the last region to the first, so an earlier replacement cannot move a later + region's indices. Each replacement carries the terminator the section's own heading line + carried, which is how a CRLF target keeps its endings without the file being rewritten to one + ending it may not use throughout. + """ + spans = [] + for section in sections: + target_span = section_span(target_lines, section) + source_span = section_span(source_lines, section) + if target_span is None or source_span is None: + raise CarryError(f"{path} section '{section}' could not be located to replace") + spans.append((target_span, source_span)) + out = list(target_lines) + for (start, end), source_span in sorted(spans, reverse=True): + ending = terminator(out[start]) or "\n" + region = [] + for line in source_lines[source_span[0] : source_span[1]]: + body = line[: len(line) - len(terminator(line))] + region.append(body + ending if terminator(line) else body) + # A region spliced ahead of more of the file has to end terminated, or its last line would run into the heading that follows it. + if region and end < len(out) and not terminator(region[-1]): + region[-1] += ending + out[start:end] = region + return out + + +def assert_sections( + path: str, + target_file: pathlib.Path, + source_lines: list[str], + replaced: list[str], + declared: list[str], + before_lines: list[str], +) -> None: + """Re-read the written file and prove the three things the re-vendor promised. + + Stated as an assertion the tool makes about its own output rather than as care taken while + writing it. The defect this exists for, a single blank line dropped between the preamble and + the first heading, is invisible in a review of the diff, breaks no renderer, and no linter in + the gate set flags it, so it would ship and the next audit would report drift nobody could + see. `apply` already re-compares tree digests after writing, and this is that discipline for a + section. + """ + after_lines = split_lines(decode_text(target_file, path)) + if h2_headings(after_lines) != h2_headings(before_lines): + raise CarryError( + f"post-apply check failed for {path}: the level-two heading sequence changed," + " so the section set or its order did not survive the write" + ) + if outside_sections(after_lines, replaced, path) != outside_sections( + before_lines, replaced, path + ): + raise CarryError( + f"post-apply check failed for {path}: text outside the re-vendored sections changed," + " so the preamble, an intent section, or a section this run left alone was modified" + ) + for section in declared: + after_span = section_span(after_lines, section) + source_span = section_span(source_lines, section) + if after_span is None or source_span is None: + raise CarryError(f"post-apply check failed for {path}: section '{section}' is absent") + if normalize_eol(region_text(after_lines, after_span)) != normalize_eol( + region_text(source_lines, source_span) + ): + raise CarryError( + f"post-apply check failed for {path} section '{section}':" + " it still does not match the hub's canonical" + ) + + +def selector_set(entry: dict[str, Any], defaults: dict[str, Any]) -> set[str]: + """The repository's applicability selectors: its types, workflow model, release trigger, and consumer model.""" + selectors = set(entry.get("types", [])) + selectors.add(entry.get("workflowModel") or defaults.get("workflowModel") or "release") + selectors.add(entry.get("releaseTrigger") or defaults.get("releaseTrigger") or "two-phase") + if entry.get("consumerModel"): + selectors.add(entry["consumerModel"]) + return selectors + + +def decode_text(path: pathlib.Path, rel: str) -> str: + """A carried file's text, refusing rather than guessing at bytes that are not UTF-8.""" + try: + return path.read_bytes().decode("utf-8") + except (OSError, UnicodeDecodeError) as exc: + raise CarryError(f"cannot read {rel}: {exc}") from exc + + +def run_sections(mode: str, name: str, target: pathlib.Path, hub: pathlib.Path = ROOT) -> int: + """Compare, and for `apply-sections` re-vendor, the verbatim sections inside a mixed file. + + The gap between the two things that already exist: `carry.py`'s tree modes own a whole tree, + and the audit names a stale section precisely without writing anything. A file such as + `AGENTS.md` is neither, holding fleet law in some sections and the repository's own content in + others, and re-vendoring the whole file over it is the overwrite the + `carried-instruction-file-guard` skill exists to stop. + """ + # Ahead of every other check, because which repository was named does not depend on the hub's state, and reporting a stale hub for an argument that could never be valid names the wrong problem. + if name == HUB_NAME: + raise CarryError("the hub's own copy is the canonical, so it is never a re-vendor target") + registry = load_json(hub / "registry/repos.json") + manifest = load_json(hub / "spec/files.json") + hub_commit = verify_hub(hub, registry) + entry = resolve_repo(name, registry) + selectors = selector_set(entry, registry.get("defaults", {})) + units = section_units(manifest, selectors) + owned_roots = [relative_root(target, path) for path, _ in units] + verify_target(target, entry, owned_roots) + print(f"hubCommit: {hub_commit}") + print(f"repository: {name}") + print(f"types: {','.join(entry.get('types', []))}") + print(f"files: {len(units)}") + clean = True + for path, declared in units: + source_file = relative_root(hub, path) + target_file = relative_root(target, path) + if not target_file.is_file(): + raise CarryError( + f"{path} is absent from the target, which is a standup question rather than drift" + " (the hub's STANDUP.md carries the baseline), so this refuses rather than creating it" + ) + source_lines = split_lines(decode_text(source_file, path)) + target_lines = split_lines(decode_text(target_file, path)) + stale = [] + for section in declared: + source_span = section_span(source_lines, section) + if source_span is None: + raise CarryError( + f"{path} section '{section}' is declared verbatim but absent from the hub's own copy" + ) + target_span = section_span(target_lines, section) + if target_span is None: + raise CarryError( + f"{path} section '{section}' is declared verbatim but absent from the target." + " Where a declared section belongs in a file that never carried it is a standup" + " question rather than a drift question, so this refuses rather than guessing" + ) + source_region = region_text(source_lines, source_span) + target_region = region_text(target_lines, target_span) + guard_governed_drift(path, section, source_region, "hub") + guard_governed_drift(path, section, target_region, "target") + if normalize_eol(source_region) != normalize_eol(target_region): + stale.append(section) + print(json.dumps({"path": path, "sections": declared, "stale": stale}, sort_keys=True)) + clean = clean and not stale + if mode == "apply-sections" and stale: + rewritten = replace_sections(target_lines, source_lines, stale, path) + target_file.write_bytes("".join(rewritten).encode("utf-8")) + print(f"write {path}") + assert_sections(path, target_file, source_lines, stale, declared, target_lines) + print(json.dumps({"path": path, "postApply": True, "stale": []}, sort_keys=True)) + return 0 if mode == "apply-sections" or clean else 1 + + def run(mode: str, name: str, target: pathlib.Path, hub: pathlib.Path = ROOT) -> int: registry = load_json(hub / "registry/repos.json") manifest = load_json(hub / "spec/files.json") @@ -450,12 +802,14 @@ def run(mode: str, name: str, target: pathlib.Path, hub: pathlib.Path = ROOT) -> def main() -> int: parser = argparse.ArgumentParser(description=__doc__) - parser.add_argument("mode", choices=("check", "apply")) + parser.add_argument("mode", choices=("check", "apply", "check-sections", "apply-sections")) parser.add_argument("repository") parser.add_argument("--target", required=True, type=pathlib.Path) args = parser.parse_args() + # The section modes are their own vocabulary rather than a scope flag on the tree modes, so an existing invocation keeps meaning exactly what it did and a caller names which of the two it wants. + dispatch = run_sections if args.mode.endswith("-sections") else run try: - return run(args.mode, args.repository, args.target.resolve()) + return dispatch(args.mode, args.repository, args.target.resolve()) except CarryError as exc: print(f"ERROR: {exc}", file=sys.stderr) return 2 diff --git a/scripts/tests/test_carry.py b/scripts/tests/test_carry.py index d21bcdfd9..06b6a38d1 100755 --- a/scripts/tests/test_carry.py +++ b/scripts/tests/test_carry.py @@ -323,5 +323,256 @@ def test_target_identity_and_unrelated_dirty_state(self) -> None: carry.verify_target(worktree, {"url": str(remote)}, [owned]) +HUB_DOC = """# Title + +Preamble that the hub and the target share. + +## Fleet Bootstrap + +The hub's current wording. + +```text +## Not A Heading +``` + +## Where the Rules Live + +The hub's current table. +""" + +TARGET_DOC = """# Title + +Preamble that the hub and the target share. + +## Fleet Bootstrap + +A wording three revisions behind. + +```text +## Not A Heading +``` + +## Local Addition + +A rule this repository wrote after a fault the fleet has never seen. + +## Where the Rules Live + +The hub's current table. +""" + + +class CarrySectionTests(unittest.TestCase): + """The verbatim-section re-vendor: what it replaces, what it must not touch, and what it refuses.""" + + def span(self, lines: list[str], heading: str) -> tuple[int, int]: + """The section's line range, failing the test rather than returning None, so a caller can unpack it.""" + found = carry.section_span(lines, heading) + if found is None: + self.fail(f"section '{heading}' was not found") + return found + + def test_split_lines_round_trips_every_ending(self) -> None: + for text in ("a\nb\n", "a\r\nb\r\n", "a\r\nb\n", "a\nb", "", "\n"): + with self.subTest(text=text): + self.assertEqual("".join(carry.split_lines(text)), text) + + def test_section_span_reads_a_fenced_heading_as_content(self) -> None: + lines = carry.split_lines(HUB_DOC) + + start, end = self.span(lines, "Fleet Bootstrap") + + self.assertEqual(lines[start], "## Fleet Bootstrap\n") + self.assertIn("## Not A Heading\n", lines[start:end]) + self.assertEqual(lines[end], "## Where the Rules Live\n") + + def test_section_span_folds_case_and_reports_an_absent_section(self) -> None: + lines = carry.split_lines(HUB_DOC) + + self.assertEqual( + carry.section_span(lines, "fleet bootstrap"), + carry.section_span(lines, "Fleet Bootstrap"), + ) + self.assertIsNone(carry.section_span(lines, "Release Model")) + + def test_replace_sections_leaves_every_other_byte_identical(self) -> None: + """The pilot's own defect: a rebuild that drops the blank line before the first heading.""" + target = carry.split_lines(TARGET_DOC) + source = carry.split_lines(HUB_DOC) + + result = "".join(carry.replace_sections(target, source, ["Fleet Bootstrap"], "AGENTS.md")) + + self.assertIn("The hub's current wording.", result) + self.assertNotIn("A wording three revisions behind.", result) + self.assertIn("A rule this repository wrote", result) + self.assertEqual( + carry.outside_sections(carry.split_lines(result), ["Fleet Bootstrap"], "AGENTS.md"), + carry.outside_sections(target, ["Fleet Bootstrap"], "AGENTS.md"), + ) + self.assertEqual(carry.h2_headings(carry.split_lines(result)), carry.h2_headings(target)) + + def test_replace_sections_preserves_the_targets_line_endings(self) -> None: + target = carry.split_lines(TARGET_DOC.replace("\n", "\r\n")) + source = carry.split_lines(HUB_DOC) + + result = "".join(carry.replace_sections(target, source, ["Fleet Bootstrap"], "AGENTS.md")) + + self.assertNotIn("\n", result.replace("\r\n", "")) + self.assertIn("The hub's current wording.\r\n", result) + + def test_replace_sections_terminates_a_region_the_hub_ended_unterminated(self) -> None: + source = carry.split_lines(HUB_DOC.rstrip("\n")) + target = carry.split_lines( + "# Title\n\n## Where the Rules Live\n\nStale.\n\n## Local Addition\n\nKept.\n" + ) + + result = "".join( + carry.replace_sections(target, source, ["Where the Rules Live"], "AGENTS.md") + ) + + self.assertIn("The hub's current table.\n## Local Addition\n", result) + self.assertEqual(carry.h2_headings(carry.split_lines(result)), carry.h2_headings(target)) + + def _write(self, root: pathlib.Path, text: str) -> pathlib.Path: + path = root / "AGENTS.md" + path.write_bytes(text.encode("utf-8")) + return path + + def test_assert_sections_passes_on_a_correct_re_vendor(self) -> None: + target = carry.split_lines(TARGET_DOC) + source = carry.split_lines(HUB_DOC) + rewritten = carry.replace_sections(target, source, ["Fleet Bootstrap"], "AGENTS.md") + + with tempfile.TemporaryDirectory() as temp: + path = self._write(pathlib.Path(temp), "".join(rewritten)) + carry.assert_sections( + "AGENTS.md", path, source, ["Fleet Bootstrap"], ["Fleet Bootstrap"], target + ) + + def test_assert_sections_catches_a_dropped_blank_line(self) -> None: + """Mutation test: the replacer's defect from the pilot, which review and the linters both miss.""" + target = carry.split_lines(TARGET_DOC) + source = carry.split_lines(HUB_DOC) + start, _ = self.span(target, "Fleet Bootstrap") + dropped = list(carry.replace_sections(target, source, ["Fleet Bootstrap"], "AGENTS.md")) + self.assertEqual(dropped[start - 1], "\n") + del dropped[start - 1] + + with tempfile.TemporaryDirectory() as temp: + path = self._write(pathlib.Path(temp), "".join(dropped)) + with self.assertRaisesRegex(carry.CarryError, "text outside the re-vendored sections"): + carry.assert_sections( + "AGENTS.md", path, source, ["Fleet Bootstrap"], ["Fleet Bootstrap"], target + ) + + def test_assert_sections_catches_a_deleted_local_section(self) -> None: + target = carry.split_lines(TARGET_DOC) + source = carry.split_lines(HUB_DOC) + + with tempfile.TemporaryDirectory() as temp: + path = self._write(pathlib.Path(temp), HUB_DOC) + with self.assertRaisesRegex(carry.CarryError, "heading sequence changed"): + carry.assert_sections( + "AGENTS.md", path, source, ["Fleet Bootstrap"], ["Fleet Bootstrap"], target + ) + + def test_assert_sections_catches_a_region_that_did_not_take(self) -> None: + target = carry.split_lines(TARGET_DOC) + source = carry.split_lines(HUB_DOC) + + with tempfile.TemporaryDirectory() as temp: + path = self._write(pathlib.Path(temp), TARGET_DOC) + with self.assertRaisesRegex(carry.CarryError, "does not match the hub's canonical"): + carry.assert_sections("AGENTS.md", path, source, [], ["Fleet Bootstrap"], target) + + def test_verbatim_section_names_skips_bare_strings_and_intent(self) -> None: + item = { + "sections": [ + "A bare string is intent", + {"name": "Verbatim Everywhere", "fidelity": "verbatim"}, + {"name": "Intent Section", "fidelity": "intent"}, + {"name": "Verbatim Elsewhere", "fidelity": "verbatim", "appliesTo": ["python"]}, + ] + } + + self.assertEqual(carry.verbatim_section_names(item, {"dotnet"}), ["Verbatim Everywhere"]) + + def test_section_units_refuses_a_placeholder_file(self) -> None: + manifest = { + "baseline": [ + { + "path": "COPILOT.md", + "placeholders": [""], + "sections": [{"name": "Runbook", "fidelity": "verbatim"}], + } + ] + } + + with self.assertRaisesRegex(carry.CarryError, "placeholders and verbatim sections"): + carry.section_units(manifest, {"dotnet"}) + + def test_section_units_refuses_a_non_markdown_file(self) -> None: + manifest = { + "baseline": [ + { + "path": ".vscode/tasks.json", + "sections": [{"name": "Tasks", "fidelity": "verbatim"}], + } + ] + } + + with self.assertRaisesRegex(carry.CarryError, "cannot be declared on"): + carry.section_units(manifest, {"dotnet"}) + + def test_section_units_merges_one_path_declared_twice(self) -> None: + manifest = { + "baseline": [ + { + "path": "AGENTS.md", + "appliesTo": ["dotnet"], + "sections": [{"name": "Fleet Bootstrap", "fidelity": "verbatim"}], + }, + { + "path": "AGENTS.md", + "appliesTo": ["dotnet"], + "sections": [{"name": "Where the Rules Live", "fidelity": "verbatim"}], + }, + ] + } + + self.assertEqual( + carry.section_units(manifest, {"dotnet"}), + [("AGENTS.md", ["Fleet Bootstrap", "Where the Rules Live"])], + ) + + def test_guard_refuses_a_region_the_fidelity_comparison_normalizes(self) -> None: + region = "## Job\n\n```yaml\nuses: actions/checkout@" + "a" * 40 + " # v4\n```\n" + + with self.assertRaisesRegex(carry.CarryError, "normalizes away"): + carry.guard_governed_drift("AGENTS.md", "Job", region, "hub") + + carry.guard_governed_drift("AGENTS.md", "Fleet Bootstrap", HUB_DOC, "hub") + + def test_run_sections_refuses_the_hub_as_a_target(self) -> None: + with ( + tempfile.TemporaryDirectory() as temp, + self.assertRaisesRegex(carry.CarryError, "never a re-vendor target"), + ): + carry.run_sections("check-sections", carry.HUB_NAME, pathlib.Path(temp), carry.ROOT) + + def test_the_manifest_declares_sections_this_hub_actually_carries(self) -> None: + """The tool reads the real manifest, so a renamed heading must surface here rather than downstream.""" + manifest = carry.load_json(carry.ROOT / "spec/files.json") + + units = carry.section_units(manifest, {"dotnet", "release", "two-phase"}) + + self.assertTrue(units) + for path, names in units: + lines = carry.split_lines(carry.decode_text(carry.ROOT / path, path)) + for name in names: + with self.subTest(path=path, section=name): + self.assertIsNotNone(carry.section_span(lines, name)) + + if __name__ == "__main__": unittest.main() From f21ccbda7c963a440917bb89b7e89583283e5afb Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 7 Sep 2026 13:19:41 -0700 Subject: [PATCH 2/7] Answer the Local Review's Findings on the Section Re-Vendor Ten findings from the pre-push adversarial pass, all on text this change wrote. Refuse a section that ends the file in one copy and not the other. A region runs to the next level-two heading, so the blank line before that heading is inside it and a section ending the file has none. The hub's `Where the Rules Live` is its last, so re-vendoring it into a repository carrying a local section after it would run that heading onto the region's last line, and padding the region would leave bytes the fidelity comparison reports as drift forever. Both are wrong, so this names the conflict rather than picking one. No fleet repository is in that state today, and `spec/section-model.md` describes exactly the migration that produces one. Fold case in the heading-sequence postcondition, which `section_span` deliberately does when it locates a region. A downstream heading differing only in case is drift this tool fixes, and the check read the corrected casing as the section set having changed and refused a file it had just made correct. Fold case when deduplicating a section name too, since the lookup folds. Two spellings of one heading survived an exact-string dedupe and resolved to one region, which spliced its replacement in twice against indices the first splice had already moved. `replace_sections` now refuses overlapping regions outright. Plan every unit before writing any of them, so a refusal reached partway through no longer leaves one file re-vendored and the next not, reported by the same exit code as a refusal that touched nothing. Exempt no owned root from the unrelated-changes check. A tree's root holds only hub-owned content, where these files hold the repository's own sections too, so the exemption would have let a write land on an uncommitted edit to a section the tool never touches. Refuse a bare carriage return, which `spec/audit.py`'s `normalize` reads as a line break and this tool does not, rather than misdiagnosing the file as missing a section the audit can see. Carry the two new menu entries into `menu.ps1`, verified with the PowerShell parser and PSScriptAnalyzer. Use `selector_set` and `HUB_NAME` in `run` rather than the inline copy and the string literal each replaced. Seventeen guards, each mutation-tested. The harness itself had to be fixed first: two mutations that delete equal-length text produce equal-sized files, and CPython validates a cached `.pyc` on modification time and size alone, so within one tick the second run imported the first mutation's bytecode and reported a guard as unguarded that is guarded. Co-Authored-By: Claude Opus 5 (1M context) --- host-setup/menu.ps1 | 7 ++ scripts/README.md | 6 +- scripts/carry.py | 195 +++++++++++++++++++++++++----------- scripts/tests/test_carry.py | 87 ++++++++++++++++ 4 files changed, 235 insertions(+), 60 deletions(-) diff --git a/host-setup/menu.ps1 b/host-setup/menu.ps1 index e299d885f..8a4ab2e68 100644 --- a/host-setup/menu.ps1 +++ b/host-setup/menu.ps1 @@ -497,6 +497,8 @@ function Show-Menu { log 'Downstream, the repo this menu is run from:' log ' 13 Check what the hub would change here, change nothing' log " 14 Pull the hub's verbatim-owned files into this repo" + log " 15 Check the hub's verbatim rule sections inside this repo's mixed files" + log ' 16 Pull those rule sections in, leaving every other section alone' } log '' log ' q Quit' @@ -525,6 +527,11 @@ function Invoke-Dispatch { if ($script:DRY_RUN) { return (Invoke-CarryAction 'check') } return (Invoke-CarryAction 'apply') } + '15' { return (Invoke-CarryAction 'check-sections') } + '16' { + if ($script:DRY_RUN) { return (Invoke-CarryAction 'check-sections') } + return (Invoke-CarryAction 'apply-sections') + } { $_ -in @('q', 'Q') } { $script:QUIT = $true; return 0 } default { warn 'Not one of the choices' diff --git a/scripts/README.md b/scripts/README.md index 1ca22fecb..8e4e53ed3 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -294,7 +294,11 @@ The tool validates the registry identity, target origin, feature-branch worktree The section modes close the gap between the two things that already existed: a tree mode owns a whole tree, and [`spec/audit.py`][audit] names a stale section precisely without writing anything, so the one apply step in [`RESYNC.md`][resync] with no tested tool behind it was the step the AGENTS.md-overwrite incident happened on. `check-sections` reports each declared verbatim section and which of them are stale, and exits `1` while any is. `apply-sections` replaces only those regions and then re-reads the file to assert three things: the level-two heading sequence is unchanged, every line outside the replaced regions is byte-identical to what was there before, and each declared section now matches the hub's canonical. The second of those is the one that earns its keep, since the defect it exists for, one blank line dropped between the preamble and the first heading, is invisible in a diff review, breaks no renderer, and no gate in this repository flags it. -Region boundaries come from `spec/audit.py`'s own fence reading rather than a second reading of CommonMark, so a level-two heading marker shown inside a code sample cannot end a region for this tool and not for the check that judges the result. The replacement carries the terminator the section's own heading line carried, so a CRLF repository keeps its endings and the file is never rewritten to one ending it may not use throughout. Four states refuse rather than guess: a section the manifest declares that the repository does not carry (where it belongs is a standup question), a file absent from the target, a file declaring both placeholders and verbatim sections (a verbatim unit carries none, per [`spec/fidelity-model.md`][fidelity-model]), and a region holding content the fidelity comparison normalizes away, such as a Dependabot-owned action pin, whose bytes the repository rather than the hub owns. +Region boundaries come from `spec/audit.py`'s own fence reading rather than a second reading of CommonMark, so a level-two heading marker shown inside a code sample cannot end a region for this tool and not for the check that judges the result. The replacement carries the terminator the section's own heading line carried, so a CRLF repository keeps its endings and the file is never rewritten to one ending it may not use throughout. + +Every refusal is raised while planning, before the first file is written, so a run that cannot finish leaves nothing half-re-vendored. Six states refuse rather than guess: a section the manifest declares that the repository does not carry (where it belongs is a standup question), a file absent from the target, a file declaring both placeholders and verbatim sections (a verbatim unit carries none, per [`spec/fidelity-model.md`][fidelity-model]), a region holding content the fidelity comparison normalizes away, such as a Dependabot-owned action pin, whose bytes the repository rather than the hub owns, a file carrying a bare carriage return, which the fidelity comparison reads as a line break and this tool does not, and a section that ends the file in one copy and not the other. That last one is a conflict rather than a choice: a region runs to the next level-two heading, so the blank line before that heading is inside it and a section ending the file has none, which means the hub's bytes and the target's own heading spacing cannot both be satisfied. + +The target is required to be clean, with no owned root exempted from the unrelated-changes check, unlike a tree carry. A tree's root holds nothing but hub-owned content, so exempting it gives up nothing, where these files hold the repository's own sections too and exempting one would let a write land on top of an uncommitted edit to a section the tool never touches. ## `skills_install.py` diff --git a/scripts/carry.py b/scripts/carry.py index d925249f8..8b64736df 100755 --- a/scripts/carry.py +++ b/scripts/carry.py @@ -4,6 +4,7 @@ import argparse import fnmatch import hashlib +import itertools import json import os import pathlib @@ -378,8 +379,19 @@ def audit_module() -> Any: _LINE = re.compile(r"[^\n]*\n|[^\n]+$") -def split_lines(text: str) -> list[str]: - """`text` as lines that keep their own terminators, so joining them returns the original bytes.""" +def split_lines(text: str, rel: str | None = None) -> list[str]: + """`text` as lines that keep their own terminators, so joining them returns the original bytes. + + A lone CR is refused rather than read as content, because `spec/audit.py`'s `normalize` treats + one as a line break and this does not, so the two would read a CR-terminated document as + different line sets: the audit would report a section stale that this tool cannot even locate, + and the refusal it would otherwise raise names an absent section rather than the real problem. + """ + if rel is not None and "\r" in text.replace("\r\n", ""): + raise CarryError( + f"{rel} carries a bare carriage return, which the fidelity comparison reads as a line" + " break and this tool does not, so the two would disagree about where a section stops" + ) return _LINE.findall(text) @@ -398,6 +410,8 @@ def normalize_eol(text: str) -> str: two copies say the same thing, and this tool writes bytes, so masking either would let it overwrite a value the repository owns. Line endings are the one exception, because `.gitattributes` governs them separately and a re-vendor preserves whichever the target uses. + `scripts/canonical_review.py`'s own `normalize` is this same reduction for the same reason, and + it is not shared, since that module imports this one and importing it back would be a cycle. """ return text.replace("\r\n", "\n").replace("\r", "\n") @@ -494,7 +508,7 @@ def verbatim_section_names(item: dict[str, Any], selectors: set[str]) -> list[st A bare-string entry is `appliesTo` `*` at intent fidelity, matching `spec/audit.py`'s own reading of the same manifest, so it is never a re-vendor target. """ - out = [] + out: list[str] = [] for element in item.get("sections", []): if isinstance(element, str): continue @@ -507,7 +521,8 @@ def verbatim_section_names(item: dict[str, Any], selectors: set[str]) -> list[st name = element.get("name") if not isinstance(name, str) or not name.strip(): raise CarryError("section declaration name must be a non-empty string") - if name not in out: + # Folded and stripped, matching `section_span`'s own lookup key, since two spellings of one heading resolve to one region and replacing that region twice would splice its tail in again. + if name.strip().lower() not in {seen.strip().lower() for seen in out}: out.append(name) return out @@ -544,7 +559,8 @@ def section_units(manifest: dict[str, Any], selectors: set[str]) -> list[tuple[s " overwrite a substituted value" ) merged = units.setdefault(path, []) - merged.extend(name for name in names if name not in merged) + seen = {name.strip().lower() for name in merged} + merged.extend(name for name in names if name.strip().lower() not in seen) return list(units.items()) @@ -565,8 +581,16 @@ def replace_sections( if target_span is None or source_span is None: raise CarryError(f"{path} section '{section}' could not be located to replace") spans.append((target_span, source_span)) + ordered = sorted(spans, reverse=True) + # Two names resolving to one region would splice that region's replacement in twice, the second time against indices the first splice already moved, so the file would gain a duplicated tail before any postcondition read it back. + for (later, _), (earlier, _) in itertools.pairwise(ordered): + if earlier[1] > later[0]: + raise CarryError( + f"{path} names two regions that overlap, lines {earlier} and {later}," + " so one replacement would splice over the other" + ) out = list(target_lines) - for (start, end), source_span in sorted(spans, reverse=True): + for (start, end), source_span in ordered: ending = terminator(out[start]) or "\n" region = [] for line in source_lines[source_span[0] : source_span[1]]: @@ -596,8 +620,12 @@ def assert_sections( see. `apply` already re-compares tree digests after writing, and this is that discipline for a section. """ - after_lines = split_lines(decode_text(target_file, path)) - if h2_headings(after_lines) != h2_headings(before_lines): + after_lines = split_lines(decode_text(target_file, path), path) + # Folded, because `section_span` locates a heading folded, so a re-cased heading is drift this tool fixes rather than a change of section set. + # Any other edit to a heading line is caught byte-for-byte by the two comparisons below. + if [name.lower() for name in h2_headings(after_lines)] != [ + name.lower() for name in h2_headings(before_lines) + ]: raise CarryError( f"post-apply check failed for {path}: the level-two heading sequence changed," " so the section set or its order did not survive the write" @@ -641,6 +669,74 @@ def decode_text(path: pathlib.Path, rel: str) -> str: raise CarryError(f"cannot read {rel}: {exc}") from exc +@dataclass(frozen=True) +class SectionPlan: + """One file's re-vendor, decided but not yet written.""" + + path: str + declared: list[str] + source_lines: list[str] + target_lines: list[str] + stale: list[str] + + +def plan_unit( + hub: pathlib.Path, target: pathlib.Path, path: str, declared: list[str] +) -> SectionPlan: + """Locate every declared section in both copies and decide which are stale, writing nothing.""" + source_file = relative_root(hub, path) + target_file = relative_root(target, path) + if not target_file.is_file(): + raise CarryError( + f"{path} is absent from the target, which is a standup question rather than drift" + " (the hub's STANDUP.md carries the baseline), so this refuses rather than creating it" + ) + source_lines = split_lines(decode_text(source_file, path), path) + target_lines = split_lines(decode_text(target_file, path), path) + stale = [] + for section in declared: + source_span = section_span(source_lines, section) + if source_span is None: + raise CarryError( + f"{path} section '{section}' is declared verbatim but absent from the hub's own copy" + ) + target_span = section_span(target_lines, section) + if target_span is None: + raise CarryError( + f"{path} section '{section}' is declared verbatim but absent from the target." + " Where a declared section belongs in a file that never carried it is a standup" + " question rather than a drift question, so this refuses rather than guessing" + ) + # A region runs to the next level-two heading, so the blank line before that heading is inside it and a section ending the file has none. + # Where one copy ends the file and the other does not, the hub's bytes and the target's document structure cannot both be satisfied: writing them verbatim runs the following heading onto the last line of the region, and padding the region leaves bytes the fidelity comparison would then report as drift forever. + # Both are wrong, so this names the conflict instead of picking one. + if (source_span[1] == len(source_lines)) != (target_span[1] == len(target_lines)): + raise CarryError( + f"{path} section '{section}' ends the file in one copy and not the other, so the" + " hub's bytes and this file's own heading spacing cannot both be satisfied;" + " reconcile the section order by hand and raise it against the manifest" + ) + source_region = region_text(source_lines, source_span) + target_region = region_text(target_lines, target_span) + guard_governed_drift(path, section, source_region, "hub") + guard_governed_drift(path, section, target_region, "target") + if normalize_eol(source_region) != normalize_eol(target_region): + stale.append(section) + return SectionPlan(path, declared, source_lines, target_lines, stale) + + +def plan_sections( + hub: pathlib.Path, target: pathlib.Path, units: list[tuple[str, list[str]]] +) -> list[SectionPlan]: + """Every unit's plan, so each refusal is raised before the first file is written. + + A refusal reached partway through the writes would leave one file re-vendored and the next + not, reported by the same exit code as a refusal that touched nothing, and the operator would + have no way to tell the two apart from the output. + """ + return [plan_unit(hub, target, path, declared) for path, declared in units] + + def run_sections(mode: str, name: str, target: pathlib.Path, hub: pathlib.Path = ROOT) -> int: """Compare, and for `apply-sections` re-vendor, the verbatim sections inside a mixed file. @@ -659,52 +755,40 @@ def run_sections(mode: str, name: str, target: pathlib.Path, hub: pathlib.Path = entry = resolve_repo(name, registry) selectors = selector_set(entry, registry.get("defaults", {})) units = section_units(manifest, selectors) - owned_roots = [relative_root(target, path) for path, _ in units] - verify_target(target, entry, owned_roots) + # No owned root, unlike a tree carry. + # A tree's root holds nothing but hub-owned content, so exempting it from the unrelated-changes check gives up nothing, where these files hold the repository's own sections too and exempting one would let this tool's writes land on top of somebody's uncommitted edit to a section it never touches. + # The cost is that a second apply needs the first one committed, which is the order RESYNC.md already gives. + verify_target(target, entry, []) + plans = plan_sections(hub, target, units) print(f"hubCommit: {hub_commit}") print(f"repository: {name}") print(f"types: {','.join(entry.get('types', []))}") - print(f"files: {len(units)}") - clean = True - for path, declared in units: - source_file = relative_root(hub, path) - target_file = relative_root(target, path) - if not target_file.is_file(): - raise CarryError( - f"{path} is absent from the target, which is a standup question rather than drift" - " (the hub's STANDUP.md carries the baseline), so this refuses rather than creating it" + print(f"files: {len(plans)}") + for plan in plans: + print( + json.dumps( + {"path": plan.path, "sections": plan.declared, "stale": plan.stale}, sort_keys=True ) - source_lines = split_lines(decode_text(source_file, path)) - target_lines = split_lines(decode_text(target_file, path)) - stale = [] - for section in declared: - source_span = section_span(source_lines, section) - if source_span is None: - raise CarryError( - f"{path} section '{section}' is declared verbatim but absent from the hub's own copy" - ) - target_span = section_span(target_lines, section) - if target_span is None: - raise CarryError( - f"{path} section '{section}' is declared verbatim but absent from the target." - " Where a declared section belongs in a file that never carried it is a standup" - " question rather than a drift question, so this refuses rather than guessing" - ) - source_region = region_text(source_lines, source_span) - target_region = region_text(target_lines, target_span) - guard_governed_drift(path, section, source_region, "hub") - guard_governed_drift(path, section, target_region, "target") - if normalize_eol(source_region) != normalize_eol(target_region): - stale.append(section) - print(json.dumps({"path": path, "sections": declared, "stale": stale}, sort_keys=True)) - clean = clean and not stale - if mode == "apply-sections" and stale: - rewritten = replace_sections(target_lines, source_lines, stale, path) - target_file.write_bytes("".join(rewritten).encode("utf-8")) - print(f"write {path}") - assert_sections(path, target_file, source_lines, stale, declared, target_lines) - print(json.dumps({"path": path, "postApply": True, "stale": []}, sort_keys=True)) - return 0 if mode == "apply-sections" or clean else 1 + ) + if mode != "apply-sections": + return 0 if all(not plan.stale for plan in plans) else 1 + for plan in plans: + if not plan.stale: + continue + target_file = relative_root(target, plan.path) + rewritten = replace_sections(plan.target_lines, plan.source_lines, plan.stale, plan.path) + target_file.write_bytes("".join(rewritten).encode("utf-8")) + print(f"write {plan.path}") + assert_sections( + plan.path, + target_file, + plan.source_lines, + plan.stale, + plan.declared, + plan.target_lines, + ) + print(json.dumps({"path": plan.path, "postApply": True, "stale": []}, sort_keys=True)) + return 0 def run(mode: str, name: str, target: pathlib.Path, hub: pathlib.Path = ROOT) -> int: @@ -712,12 +796,7 @@ def run(mode: str, name: str, target: pathlib.Path, hub: pathlib.Path = ROOT) -> manifest = load_json(hub / "spec/files.json") hub_commit = verify_hub(hub, registry) entry = resolve_repo(name, registry) - defaults = registry.get("defaults", {}) - selectors = set(entry.get("types", [])) - selectors.add(entry.get("workflowModel") or defaults.get("workflowModel") or "release") - selectors.add(entry.get("releaseTrigger") or defaults.get("releaseTrigger") or "two-phase") - if entry.get("consumerModel"): - selectors.add(entry["consumerModel"]) + selectors = selector_set(entry, registry.get("defaults", {})) all_declarations = manifest.get("trees") if not isinstance(all_declarations, list): raise CarryError("manifest trees must be an array") @@ -727,9 +806,7 @@ def run(mode: str, name: str, target: pathlib.Path, hub: pathlib.Path = ROOT) -> for declaration in all_declarations if applicable(declaration.get("appliesTo", "*"), selectors) ] - if name == "ProjectTemplate" and any( - not item.get("allowHubTarget", False) for item in declarations - ): + if name == HUB_NAME and any(not item.get("allowHubTarget", False) for item in declarations): raise CarryError("a declaration does not allow ProjectTemplate as its target") owned_roots = [relative_root(target, item["target"]) for item in declarations] verify_target(target, entry, owned_roots) diff --git a/scripts/tests/test_carry.py b/scripts/tests/test_carry.py index 06b6a38d1..aced63d82 100755 --- a/scripts/tests/test_carry.py +++ b/scripts/tests/test_carry.py @@ -433,6 +433,93 @@ def test_replace_sections_terminates_a_region_the_hub_ended_unterminated(self) - self.assertIn("The hub's current table.\n## Local Addition\n", result) self.assertEqual(carry.h2_headings(carry.split_lines(result)), carry.h2_headings(target)) + def _unit(self, hub_text: str, target_text: str | None, sections: list[str]) -> None: + """Plan one AGENTS.md unit from two documents written to temporary hub and target trees.""" + with tempfile.TemporaryDirectory() as temp: + root = pathlib.Path(temp) + hub, target = root / "hub", root / "target" + hub.mkdir() + target.mkdir() + (hub / "AGENTS.md").write_bytes(hub_text.encode("utf-8")) + if target_text is not None: + (target / "AGENTS.md").write_bytes(target_text.encode("utf-8")) + carry.plan_unit(hub, target, "AGENTS.md", sections) + + def test_plan_unit_refuses_a_file_the_target_does_not_carry(self) -> None: + with self.assertRaisesRegex(carry.CarryError, "absent from the target, which is a standup"): + self._unit(HUB_DOC, None, ["Fleet Bootstrap"]) + + def test_plan_unit_refuses_a_section_absent_from_the_hub(self) -> None: + with self.assertRaisesRegex(carry.CarryError, "absent from the hub's own copy"): + self._unit(HUB_DOC, TARGET_DOC, ["Release Model"]) + + def test_plan_unit_refuses_a_section_absent_from_the_target(self) -> None: + hub = HUB_DOC + "\n## Release Model\n\nHub only.\n" + with self.assertRaisesRegex(carry.CarryError, "absent from the target"): + self._unit(hub, TARGET_DOC, ["Release Model"]) + + def test_plan_unit_refuses_a_section_that_ends_one_file_and_not_the_other(self) -> None: + """The hub's last section carries no blank line before a heading, so its bytes and a + downstream local section that follows it cannot both be satisfied.""" + target = TARGET_DOC + "\n## Repository Local Rule\n\nWritten after a local fault.\n" + with self.assertRaisesRegex( + carry.CarryError, "ends the file in one copy and not the other" + ): + self._unit(HUB_DOC, target, ["Where the Rules Live"]) + + def test_plan_unit_refuses_a_bare_carriage_return(self) -> None: + with self.assertRaisesRegex(carry.CarryError, "bare carriage return"): + self._unit(HUB_DOC, TARGET_DOC.replace("\n", "\r"), ["Fleet Bootstrap"]) + + def test_plan_unit_reports_the_stale_sections_and_nothing_else(self) -> None: + with tempfile.TemporaryDirectory() as temp: + root = pathlib.Path(temp) + hub, target = root / "hub", root / "target" + hub.mkdir() + target.mkdir() + (hub / "AGENTS.md").write_bytes(HUB_DOC.encode("utf-8")) + (target / "AGENTS.md").write_bytes(TARGET_DOC.encode("utf-8")) + + plan = carry.plan_unit( + hub, target, "AGENTS.md", ["Fleet Bootstrap", "Where the Rules Live"] + ) + + self.assertEqual(plan.stale, ["Fleet Bootstrap"]) + self.assertEqual(plan.declared, ["Fleet Bootstrap", "Where the Rules Live"]) + + def test_replace_sections_refuses_two_names_resolving_to_one_region(self) -> None: + target = carry.split_lines(TARGET_DOC) + source = carry.split_lines(HUB_DOC) + + with self.assertRaisesRegex(carry.CarryError, "two regions that overlap"): + carry.replace_sections( + target, source, ["Fleet Bootstrap", "fleet bootstrap"], "AGENTS.md" + ) + + def test_verbatim_section_names_folds_case_when_deduplicating(self) -> None: + item = { + "sections": [ + {"name": "Fleet Bootstrap", "fidelity": "verbatim"}, + {"name": "fleet bootstrap ", "fidelity": "verbatim"}, + ] + } + + self.assertEqual(carry.verbatim_section_names(item, {"dotnet"}), ["Fleet Bootstrap"]) + + def test_assert_sections_accepts_a_heading_the_re_vendor_re_cased(self) -> None: + """A re-cased heading is the drift this tool fixes, so the postcondition must not read the + corrected casing as the section set having changed.""" + target = carry.split_lines(TARGET_DOC.replace("## Fleet Bootstrap", "## fleet bootstrap")) + source = carry.split_lines(HUB_DOC) + rewritten = carry.replace_sections(target, source, ["Fleet Bootstrap"], "AGENTS.md") + + with tempfile.TemporaryDirectory() as temp: + path = self._write(pathlib.Path(temp), "".join(rewritten)) + carry.assert_sections( + "AGENTS.md", path, source, ["Fleet Bootstrap"], ["Fleet Bootstrap"], target + ) + self.assertIn("## Fleet Bootstrap\n", path.read_text(encoding="utf-8")) + def _write(self, root: pathlib.Path, text: str) -> pathlib.Path: path = root / "AGENTS.md" path.write_bytes(text.encode("utf-8")) From d7b89324731d98384ca2f197c2fca61ab5216722 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 7 Sep 2026 13:36:53 -0700 Subject: [PATCH 3/7] Reconcile the Section Boundary Rather Than Refusing It The second review pass disproved the refusal the first pass produced. It claimed that padding a re-vendored region with the blank line a following heading needs would leave bytes the fidelity comparison reports as drift forever. It does not. `spec/audit.py`'s `extract_section` splits the document on LF and joins the region back with LF, so a file-final newline and a blank line before the next heading both render as one trailing empty element, and the two forms compare equal. Verified directly against that function. So the refusal goes, and with it the premise it rested on. The comparison now uses `comparable_region`, which reproduces `extract_section` exactly, and the write reconciles the boundary to what the target document needs: a section that ends the hub's file keeps a blank line before whatever follows it downstream, and one that ends the target's file gains no trailing blank line. Both render identically to the check that judges them, and only one of them leaves a document a Markdown linter accepts. The rest of that pass, all on text this branch wrote: Move the section modes to `RESYNC.md` step 1, where the instruction set's verbatim sections are already ordered. Step 3 was wrong twice over: it put the instruction set after the deletions and the tree carry, which step 1 exists to prevent, and it put a clean-worktree requirement after the step that dirties the worktree, so the command could not run. Extract the write loop as `apply_plans`, and report a write failure rather than letting an `OSError` out as a traceback. Sweep placeholders across every applicable entry for a path rather than only the entry that also carried the verbatim sections, since the two can be declared separately and the file is written once. Say in `scripts/README.md` what a refusal actually guarantees, which is that a refused run writes nothing, not that no run can fail partway. Name the section modes in the menu contract and in both menus' help text. Twenty guards, each mutation-tested. End to end against `HomeAutomation-Config`'s real ten stale sections in a scratch copy: all ten cleared, zero disagreements with `spec/audit.py` across the 21 declared sections, and the heading sequence and every line outside the declared regions byte-identical. Co-Authored-By: Claude Opus 5 (1M context) --- RESYNC.md | 2 +- host-setup/README.md | 2 +- host-setup/menu.ps1 | 5 +- host-setup/menu.sh | 6 +- scripts/README.md | 4 +- scripts/carry.py | 116 +++++++++++++++++++++------------ scripts/tests/test_carry.py | 124 +++++++++++++++++++++++++++++++++--- 7 files changed, 201 insertions(+), 58 deletions(-) diff --git a/RESYNC.md b/RESYNC.md index a65d8639c..3578c92de 100644 --- a/RESYNC.md +++ b/RESYNC.md @@ -72,10 +72,10 @@ python3 spec/fidelity_honesty.py --report # regenerate reports/divergences The order is load-bearing. Each step below either changes the rules the later steps are judged against, or removes something a later step would otherwise refresh. 1. **The instruction set first.** `CLAUDE.md`, then `AGENTS.md` and `GOVERNANCE.md` verbatim sections, then `CODESTYLE.md` and `WORKFLOW.md`. `CLAUDE.md` is the single `@AGENTS.md`-import file that gets `AGENTS.md` into a Claude Code session's context at all, so a repo carrying `AGENTS.md` without it is still a partial carry for that provider even though `spec/files.json` lists them as separate entries. `AGENTS.md`'s skill-dependency pointer paragraph, naming `scripts/skills_install.py` and where the fleet's Skills live, carries as one more verbatim unit in this same step, not a separate pass. These are the rules for producing every other file, so carrying them last means everything touched beforehand was judged against the previous revision. This is the same closing-window shape as [`STANDUP.md`][standup] section 1A, and the cost of getting it wrong is rework proportional to how much was changed first. **Run the `carried-instruction-file-guard` skill's distinctive-phrase probe before touching `AGENTS.md`, `GOVERNANCE.md`, `CODESTYLE.md`, or `WORKFLOW.md` in this step, every time, without exception, regardless of whether the update is a verbatim re-vendor or an intent-fidelity edit.** A diff that looks routine is exactly the shape the AGENTS.md-overwrite incident took, so the probe is not weighed against how routine the request sounds, it runs regardless. `CLAUDE.md` is outside that guard's scope entirely, the same as any file the guard was never written to cover: it carries no mixed or repo-specific content by design, so its own re-vendor is an ordinary verbatim-fidelity copy, no probe needed. `resync-a-repo` packages this whole step, and the rest of this section, as a Skill for a hub-context session driving the resync. + Run `python3 scripts/carry.py check-sections --target /path/to/worktree` for the `AGENTS.md` and `GOVERNANCE.md` verbatim sections, then `apply-sections` once each reported region's diff has been read. It replaces only the regions the manifest declares verbatim and asserts afterwards that the preamble, the heading sequence, and every other region came through byte-identical, which is the assertion a hand-rolled replacer only gets when whoever wrote it happened to write one. It refuses rather than guessing where a declared section is absent from the repository, since that is a standup question rather than a drift question. It narrows what the `carried-instruction-file-guard` probe has to catch by hand without replacing it: a tool that can only touch declared verbatim regions cannot delete a repository's local additions, and the intent half is still nothing mechanical can judge. It requires a clean worktree and exempts no path from that check, which this step's own position satisfies: nothing else has written to the tree yet. 2. **Deletions second, before any re-vendor.** A `hub-only:` finding names a file the hub hosts rather than carries, and its remedy removes the file. Doing it after the re-vendors means refreshing a copy that is about to be deleted, which is wasted work that also reads as a deliberate update in the diff. See section 4, which is the whole of what deletion means here. 3. **Verbatim re-vendors.** Copy the current hub canonical down, whole file or the one named `## heading` region. A finding classified **stale** matches a past hub revision and needs no judgment. One classified **modified** matches no revision, so the repository changed fixed content and the change is read before it is overwritten, since it may be an improvement the hub should adopt instead. Run `python3 scripts/carry.py check --target /path/to/worktree` for manifest-owned trees. Read each modified-file diff and every extra path before running the same command with `apply`. The tree declaration supplies the prune authority, so the carried-instruction-file guard does not apply to this fully owned content. - Run `python3 scripts/carry.py check-sections --target /path/to/worktree` for the verbatim `## heading` regions inside a mixed file, then `apply-sections` once each reported region's diff has been read. It replaces only the regions the manifest declares verbatim and asserts afterwards that the preamble, the heading sequence, and every other region came through byte-identical, which is the assertion a hand-rolled replacer only gets when whoever wrote it happened to write one. It refuses rather than guessing where a declared section is absent from the repository, since that is a standup question rather than a drift question. It narrows what the `carried-instruction-file-guard` probe has to catch by hand without replacing it: a tool that can only touch declared verbatim regions cannot delete a repository's local additions, and the intent half is still nothing mechanical can judge. 4. **Interface workflows.** Honor the named contract (required jobs, the ruleset-bound check name, the artifact-name handoff) rather than copying bytes. The body is the repository's own. 5. **Settings, rulesets, and secrets.** From a hub checkout at `main`, run `repo-config/configure.sh check / release|operational`, then `apply` for what it reports. Pass the model explicitly. Delete each target-repository path under `repo-config/` that the audit classified `retire`. Inspect every untriaged path collision before deciding its disposition. 6. **Intent files last, and by hand.** See section 5, which states why these carry no mechanical signal at all. diff --git a/host-setup/README.md b/host-setup/README.md index 2d318aa23..04e2f54be 100644 --- a/host-setup/README.md +++ b/host-setup/README.md @@ -88,7 +88,7 @@ powershell -ExecutionPolicy Bypass -File menu.ps1 host-setup\menu.ps1 # from a checkout of this repository, or of any other repo in the fleet ``` -It answers a different question than `bootstrap.sh`/`bootstrap.ps1` do. Those stand a host up and stop. The menu loops, because a person sitting down with it usually wants more than one thing done in a sitting, and it tells the hub apart from whichever repo it happens to be run from: a checkout of `ptr727/ProjectTemplate` gets the hub tasks (audit a cataloged repo, check the generated Skills distributions), a checkout of any other repo gets the downstream tasks too (check or pull the hub's verbatim-owned files into that repo's own worktree, per [`scripts/carry.py`][carry]), and every run gets the host tasks this directory's `linux/` or `windows/` tooling already provides. Only the downstream tasks need a repo to run from. The hub tasks fetch the hub themselves when there is no local checkout to reuse, so they still show and still work when the menu is run entirely standalone, off no checkout at all. +It answers a different question than `bootstrap.sh`/`bootstrap.ps1` do. Those stand a host up and stop. The menu loops, because a person sitting down with it usually wants more than one thing done in a sitting, and it tells the hub apart from whichever repo it happens to be run from: a checkout of `ptr727/ProjectTemplate` gets the hub tasks (audit a cataloged repo, check the generated Skills distributions), a checkout of any other repo gets the downstream tasks too (check or pull the hub's verbatim-owned files into that repo's own worktree, and the same pair again for the verbatim rule sections inside a file that repo also owns the rest of, per [`scripts/carry.py`][carry]), and every run gets the host tasks this directory's `linux/` or `windows/` tooling already provides. Only the downstream tasks need a repo to run from. The hub tasks fetch the hub themselves when there is no local checkout to reuse, so they still show and still work when the menu is run entirely standalone, off no checkout at all. Reaching `spec/audit.py` and `scripts/carry.py` from outside a hub checkout means fetching one, the same "hosted and reached, never carried" model [`scripts/README.md`][scripts-readme] states for those tools generally. The menu clones fresh rather than reusing the bootstrap loader's tarball, since `scripts/carry.py` itself checks that its hub argument is a real git checkout on a freshly fetched `origin/main` with no local changes, and a full clone rather than a shallow one, since `spec/audit.py` walks the hub's own commit history to judge whether a carried copy is trailing the file it was copied from. Run from inside the hub itself, that same freshness is confirmed against the local checkout before a hub task uses it, falling back to a fresh clone when the local checkout has moved on, so an audit or a Skills-distribution check never silently reads a stale or feature-branch tree. diff --git a/host-setup/menu.ps1 b/host-setup/menu.ps1 index 8a4ab2e68..885102d6a 100644 --- a/host-setup/menu.ps1 +++ b/host-setup/menu.ps1 @@ -62,8 +62,9 @@ Usage: menu.ps1 [options] An interactive menu over this fleet's host and repo tooling: update the host tools, upgrade the OS packages, install the fleet skills, audit a cataloged repo, and pull the hub's verbatim-owned -files into a downstream repo's own worktree. Run from a hub checkout or from any other repo. The -menu shows each the tasks that apply to it. +files, or just the verbatim rule sections inside a mixed file, into a downstream repo's own +worktree. Run from a hub checkout or from any other repo. The menu shows each the tasks that +apply to it. Options: -y, -Yes Pass -Yes to each tool this menu runs, so a tool does not prompt. The menu's diff --git a/host-setup/menu.sh b/host-setup/menu.sh index 746012c8a..ebfd250de 100755 --- a/host-setup/menu.sh +++ b/host-setup/menu.sh @@ -45,9 +45,9 @@ usage() { Usage: menu.sh [options] An interactive menu over this fleet's host and repo tooling: update the host tools, upgrade the -OS, install the fleet skills, audit a cataloged repo, and pull the hub's verbatim-owned files into -a downstream repo's own worktree. Run from a hub checkout or from any other repo. The menu shows -each the tasks that apply to it. +OS, install the fleet skills, audit a cataloged repo, and pull the hub's verbatim-owned files, or +just the verbatim rule sections inside a mixed file, into a downstream repo's own worktree. Run +from a hub checkout or from any other repo. The menu shows each the tasks that apply to it. Options: -y, --yes Pass --yes to each tool this menu runs, so a tool does not prompt. The menu's diff --git a/scripts/README.md b/scripts/README.md index 8e4e53ed3..605dbbac2 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -296,7 +296,9 @@ The section modes close the gap between the two things that already existed: a t Region boundaries come from `spec/audit.py`'s own fence reading rather than a second reading of CommonMark, so a level-two heading marker shown inside a code sample cannot end a region for this tool and not for the check that judges the result. The replacement carries the terminator the section's own heading line carried, so a CRLF repository keeps its endings and the file is never rewritten to one ending it may not use throughout. -Every refusal is raised while planning, before the first file is written, so a run that cannot finish leaves nothing half-re-vendored. Six states refuse rather than guess: a section the manifest declares that the repository does not carry (where it belongs is a standup question), a file absent from the target, a file declaring both placeholders and verbatim sections (a verbatim unit carries none, per [`spec/fidelity-model.md`][fidelity-model]), a region holding content the fidelity comparison normalizes away, such as a Dependabot-owned action pin, whose bytes the repository rather than the hub owns, a file carrying a bare carriage return, which the fidelity comparison reads as a line break and this tool does not, and a section that ends the file in one copy and not the other. That last one is a conflict rather than a choice: a region runs to the next level-two heading, so the blank line before that heading is inside it and a section ending the file has none, which means the hub's bytes and the target's own heading spacing cannot both be satisfied. +Every refusal is raised while planning, before the first file is written, so a refused run leaves nothing re-vendored. What refuses: a section the manifest declares that the repository does not carry (where it belongs is a standup question), a file absent from the target, a file that is not UTF-8 or that carries a bare carriage return, which the fidelity comparison reads as a line break and this tool does not, a file declaring both placeholders and verbatim sections (a verbatim unit carries none, per [`spec/fidelity-model.md`][fidelity-model]), a region holding content the fidelity comparison normalizes away, such as a Dependabot-owned action pin, whose bytes the repository rather than the hub owns, and two declared names resolving to one region. A failure after that point is the write itself or its assertion, and it names the file it was working on, because an earlier file may already hold its re-vendored content. + +A region runs to the next level-two heading, so the blank line before that heading sits inside the region and a section ending its file has none. `spec/audit.py`'s `extract_section` reads those two as the same content, since it splits the document on LF and joins the region back with LF, so a file-final newline and a blank line before a following heading both render as one trailing empty element. The comparison therefore uses that form, and the write reconciles the boundary to what the target document needs rather than copying it from the hub: a section that ends the hub's file keeps a blank line before whatever follows it downstream, and one that ends the target's file gains no trailing blank line. The target is required to be clean, with no owned root exempted from the unrelated-changes check, unlike a tree carry. A tree's root holds nothing but hub-owned content, so exempting it gives up nothing, where these files hold the repository's own sections too and exempting one would let a write land on top of an uncommitted edit to a section the tool never touches. diff --git a/scripts/carry.py b/scripts/carry.py index 8b64736df..21761df34 100755 --- a/scripts/carry.py +++ b/scripts/carry.py @@ -469,6 +469,23 @@ def region_text(lines: list[str], span: tuple[int, int]) -> str: return "".join(lines[span[0] : span[1]]) +def comparable_region(lines: list[str], span: tuple[int, int]) -> str: + """The region rendered the way `spec/audit.py`'s `extract_section` renders it. + + That function splits the whole document on LF and joins the region's lines back with LF, so a + file ending in a newline yields one final empty element and a region running to the end of the + file carries it. The consequence is worth stating, because it decides what this tool writes: a + section that ends the hub's file and the same section followed by a heading downstream, with + one blank line between, render to the identical string. The fidelity comparison therefore reads + them as the same content, and the raw bytes do not, so this is the form to compare and + `region_text` is the form to write. + """ + bodies = [line[: len(line) - len(terminator(line))] for line in lines[span[0] : span[1]]] + if span[1] == len(lines) and lines and terminator(lines[-1]): + bodies.append("") + return "\n".join(bodies) + + def outside_sections(lines: list[str], sections: list[str], path: str) -> str: """Every line outside the named regions, joined, which is the text a re-vendor must not touch. @@ -537,30 +554,34 @@ def section_units(manifest: dict[str, Any], selectors: set[str]) -> list[tuple[s if not isinstance(baseline, list): raise CarryError("manifest baseline must be an array") units: dict[str, list[str]] = {} + placeholder_paths = set() for item in baseline: if not isinstance(item, dict): raise CarryError(f"baseline declaration must be an object: {item!r}") if not applicable(item.get("appliesTo", "*"), selectors): continue - names = verbatim_section_names(item, selectors) - if not names: - continue path = item.get("path") if not isinstance(path, str) or not path: raise CarryError("baseline declaration path must be a non-empty string") + # Collected for every applicable entry rather than only the ones carrying verbatim sections, since the two can be declared on separate entries for one path and a file is written once whichever entry named the substitution. + if item.get("placeholders"): + placeholder_paths.add(path) + names = verbatim_section_names(item, selectors) + if not names: + continue if not path.endswith(".md"): raise CarryError( f"a verbatim section is a Markdown heading region, so it cannot be declared on {path}" ) - if item.get("placeholders"): - raise CarryError( - f"{path} declares both placeholders and verbatim sections, and a verbatim unit carries" - " no placeholder (spec/fidelity-model.md 'Normalization'), so re-vendoring one could" - " overwrite a substituted value" - ) merged = units.setdefault(path, []) seen = {name.strip().lower() for name in merged} merged.extend(name for name in names if name.strip().lower() not in seen) + for path in sorted(set(units) & placeholder_paths): + raise CarryError( + f"{path} declares both placeholders and verbatim sections, and a verbatim unit carries" + " no placeholder (spec/fidelity-model.md 'Normalization'), so re-vendoring one could" + " overwrite a substituted value" + ) return list(units.items()) @@ -597,8 +618,16 @@ def replace_sections( body = line[: len(line) - len(terminator(line))] region.append(body + ending if terminator(line) else body) # A region spliced ahead of more of the file has to end terminated, or its last line would run into the heading that follows it. - if region and end < len(out) and not terminator(region[-1]): + if region and not terminator(region[-1]): region[-1] += ending + # A region runs to the next level-two heading, so the blank line before that heading is inside it and a section ending its file has none. + # Where the two copies differ on that, `comparable_region` reads both as the same content, so the boundary is reconciled to what this document needs rather than copied from the other one: pad where a heading follows, and drop the trailing blank where nothing does. + source_ends_file = source_span[1] == len(source_lines) + target_ends_file = end == len(out) + if source_ends_file and not target_ends_file: + region.append(ending) + elif target_ends_file and not source_ends_file and region and not region[-1].strip(): + region.pop() out[start:end] = region return out @@ -642,8 +671,8 @@ def assert_sections( source_span = section_span(source_lines, section) if after_span is None or source_span is None: raise CarryError(f"post-apply check failed for {path}: section '{section}' is absent") - if normalize_eol(region_text(after_lines, after_span)) != normalize_eol( - region_text(source_lines, source_span) + if comparable_region(after_lines, after_span) != comparable_region( + source_lines, source_span ): raise CarryError( f"post-apply check failed for {path} section '{section}':" @@ -707,20 +736,11 @@ def plan_unit( " Where a declared section belongs in a file that never carried it is a standup" " question rather than a drift question, so this refuses rather than guessing" ) - # A region runs to the next level-two heading, so the blank line before that heading is inside it and a section ending the file has none. - # Where one copy ends the file and the other does not, the hub's bytes and the target's document structure cannot both be satisfied: writing them verbatim runs the following heading onto the last line of the region, and padding the region leaves bytes the fidelity comparison would then report as drift forever. - # Both are wrong, so this names the conflict instead of picking one. - if (source_span[1] == len(source_lines)) != (target_span[1] == len(target_lines)): - raise CarryError( - f"{path} section '{section}' ends the file in one copy and not the other, so the" - " hub's bytes and this file's own heading spacing cannot both be satisfied;" - " reconcile the section order by hand and raise it against the manifest" - ) - source_region = region_text(source_lines, source_span) - target_region = region_text(target_lines, target_span) + source_region = comparable_region(source_lines, source_span) + target_region = comparable_region(target_lines, target_span) guard_governed_drift(path, section, source_region, "hub") guard_governed_drift(path, section, target_region, "target") - if normalize_eol(source_region) != normalize_eol(target_region): + if source_region != target_region: stale.append(section) return SectionPlan(path, declared, source_lines, target_lines, stale) @@ -737,6 +757,35 @@ def plan_sections( return [plan_unit(hub, target, path, declared) for path, declared in units] +def apply_plans(target: pathlib.Path, plans: list[SectionPlan]) -> None: + """Write each plan's stale regions and assert the result, one file at a time. + + Every refusal has already been raised by `plan_sections`, so nothing here can decline to + proceed. What can still fail is the write itself or the assertion after it, and each names the + file it was working on, because by then an earlier file may already hold its re-vendored + content and the operator has to know which. + """ + for plan in plans: + if not plan.stale: + continue + target_file = relative_root(target, plan.path) + rewritten = replace_sections(plan.target_lines, plan.source_lines, plan.stale, plan.path) + try: + target_file.write_bytes("".join(rewritten).encode("utf-8")) + except OSError as exc: + raise CarryError(f"cannot write {plan.path}: {exc}") from exc + print(f"write {plan.path}") + assert_sections( + plan.path, + target_file, + plan.source_lines, + plan.stale, + plan.declared, + plan.target_lines, + ) + print(json.dumps({"path": plan.path, "postApply": True, "stale": []}, sort_keys=True)) + + def run_sections(mode: str, name: str, target: pathlib.Path, hub: pathlib.Path = ROOT) -> int: """Compare, and for `apply-sections` re-vendor, the verbatim sections inside a mixed file. @@ -757,7 +806,7 @@ def run_sections(mode: str, name: str, target: pathlib.Path, hub: pathlib.Path = units = section_units(manifest, selectors) # No owned root, unlike a tree carry. # A tree's root holds nothing but hub-owned content, so exempting it from the unrelated-changes check gives up nothing, where these files hold the repository's own sections too and exempting one would let this tool's writes land on top of somebody's uncommitted edit to a section it never touches. - # The cost is that a second apply needs the first one committed, which is the order RESYNC.md already gives. + # The cost is that a second apply needs the first one committed, and RESYNC.md's step 1 runs these modes before anything else writes to the tree. verify_target(target, entry, []) plans = plan_sections(hub, target, units) print(f"hubCommit: {hub_commit}") @@ -772,22 +821,7 @@ def run_sections(mode: str, name: str, target: pathlib.Path, hub: pathlib.Path = ) if mode != "apply-sections": return 0 if all(not plan.stale for plan in plans) else 1 - for plan in plans: - if not plan.stale: - continue - target_file = relative_root(target, plan.path) - rewritten = replace_sections(plan.target_lines, plan.source_lines, plan.stale, plan.path) - target_file.write_bytes("".join(rewritten).encode("utf-8")) - print(f"write {plan.path}") - assert_sections( - plan.path, - target_file, - plan.source_lines, - plan.stale, - plan.declared, - plan.target_lines, - ) - print(json.dumps({"path": plan.path, "postApply": True, "stale": []}, sort_keys=True)) + apply_plans(target, plans) return 0 diff --git a/scripts/tests/test_carry.py b/scripts/tests/test_carry.py index aced63d82..75425c8fd 100755 --- a/scripts/tests/test_carry.py +++ b/scripts/tests/test_carry.py @@ -10,6 +10,8 @@ import unittest sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent)) +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent.parent / "spec")) +import audit import carry @@ -430,7 +432,7 @@ def test_replace_sections_terminates_a_region_the_hub_ended_unterminated(self) - carry.replace_sections(target, source, ["Where the Rules Live"], "AGENTS.md") ) - self.assertIn("The hub's current table.\n## Local Addition\n", result) + self.assertIn("The hub's current table.\n\n## Local Addition\n", result) self.assertEqual(carry.h2_headings(carry.split_lines(result)), carry.h2_headings(target)) def _unit(self, hub_text: str, target_text: str | None, sections: list[str]) -> None: @@ -458,14 +460,118 @@ def test_plan_unit_refuses_a_section_absent_from_the_target(self) -> None: with self.assertRaisesRegex(carry.CarryError, "absent from the target"): self._unit(hub, TARGET_DOC, ["Release Model"]) - def test_plan_unit_refuses_a_section_that_ends_one_file_and_not_the_other(self) -> None: - """The hub's last section carries no blank line before a heading, so its bytes and a - downstream local section that follows it cannot both be satisfied.""" - target = TARGET_DOC + "\n## Repository Local Rule\n\nWritten after a local fault.\n" - with self.assertRaisesRegex( - carry.CarryError, "ends the file in one copy and not the other" - ): - self._unit(HUB_DOC, target, ["Where the Rules Live"]) + def test_a_section_ending_the_hub_file_keeps_a_blank_line_before_what_follows(self) -> None: + """The hub's last section carries no blank line before a following heading, because it has + none. `spec/audit.py` reads that and one blank line as the same content, so the boundary is + reconciled to what the target document needs rather than copied byte-for-byte.""" + hub = carry.split_lines(HUB_DOC) + target = carry.split_lines( + TARGET_DOC + "\n## Repository Local Rule\n\nWritten after a local fault.\n" + ) + + result = "".join(carry.replace_sections(target, hub, ["Where the Rules Live"], "AGENTS.md")) + + self.assertIn("The hub's current table.\n\n## Repository Local Rule\n", result) + self.assertIn("Written after a local fault.", result) + self.assertEqual( + audit.extract_section(result, "Where the Rules Live"), + audit.extract_section(HUB_DOC, "Where the Rules Live"), + ) + + def test_a_section_ending_the_target_file_keeps_no_trailing_blank_line(self) -> None: + """The mirror case: nothing follows the section downstream, so the hub's own trailing blank + line would leave the file ending on one.""" + hub = carry.split_lines(HUB_DOC + "\n## Release Model\n\nHub only.\n") + target = carry.split_lines("# Title\n\n## Where the Rules Live\n\nStale.\n") + + result = "".join(carry.replace_sections(target, hub, ["Where the Rules Live"], "AGENTS.md")) + + self.assertTrue(result.endswith("The hub's current table.\n")) + self.assertEqual( + audit.extract_section(result, "Where the Rules Live"), + audit.extract_section( + HUB_DOC + "\n## Release Model\n\nHub only.\n", "Where the Rules Live" + ), + ) + + def test_comparable_region_matches_the_fidelity_check_across_the_file_boundary(self) -> None: + """`region_text` and `extract_section` differ by one newline exactly when one copy's section + ends its file and the other's does not, which is the case this form exists to reconcile.""" + hub = "# T\n\n## X\n\nbody\n" + padded = "# T\n\n## X\n\nbody\n\n## Y\n\nlocal\n" + + hub_span = self.span(carry.split_lines(hub), "X") + padded_span = self.span(carry.split_lines(padded), "X") + + self.assertEqual( + carry.comparable_region(carry.split_lines(hub), hub_span), + carry.comparable_region(carry.split_lines(padded), padded_span), + ) + self.assertEqual( + audit.extract_section(hub, "X"), + carry.comparable_region(carry.split_lines(hub), hub_span), + ) + self.assertNotEqual( + carry.region_text(carry.split_lines(hub), hub_span), + carry.region_text(carry.split_lines(padded), padded_span), + ) + + def test_plan_sections_refuses_on_a_later_unit_before_anything_is_written(self) -> None: + """Every refusal is reachable while planning, so a bad second unit stops the run with the + first one still unwritten.""" + with tempfile.TemporaryDirectory() as temp: + root = pathlib.Path(temp) + hub, target = root / "hub", root / "target" + hub.mkdir() + target.mkdir() + for tree, text in ((hub, HUB_DOC), (target, TARGET_DOC)): + (tree / "AGENTS.md").write_bytes(text.encode("utf-8")) + (hub / "GOVERNANCE.md").write_bytes(HUB_DOC.encode("utf-8")) + before = (target / "AGENTS.md").read_bytes() + + with self.assertRaisesRegex(carry.CarryError, "absent from the target"): + carry.plan_sections( + hub, + target, + [("AGENTS.md", ["Fleet Bootstrap"]), ("GOVERNANCE.md", ["Fleet Bootstrap"])], + ) + + self.assertEqual((target / "AGENTS.md").read_bytes(), before) + + def test_apply_plans_writes_every_stale_unit_and_asserts_each(self) -> None: + with tempfile.TemporaryDirectory() as temp: + root = pathlib.Path(temp) + hub, target = root / "hub", root / "target" + hub.mkdir() + target.mkdir() + for name in ("AGENTS.md", "GOVERNANCE.md"): + (hub / name).write_bytes(HUB_DOC.encode("utf-8")) + (target / name).write_bytes(TARGET_DOC.encode("utf-8")) + units = [("AGENTS.md", ["Fleet Bootstrap"]), ("GOVERNANCE.md", ["Fleet Bootstrap"])] + + plans = carry.plan_sections(hub, target, units) + carry.apply_plans(target, plans) + + for name in ("AGENTS.md", "GOVERNANCE.md"): + written = (target / name).read_text(encoding="utf-8") + self.assertIn("The hub's current wording.", written) + self.assertIn("A rule this repository wrote", written) + self.assertEqual([plan.stale for plan in plans], [["Fleet Bootstrap"]] * 2) + + def test_section_units_refuses_a_placeholder_declared_on_a_sibling_entry(self) -> None: + """The two can sit on separate entries for one path, and the file is written once.""" + manifest = { + "baseline": [ + {"path": "AGENTS.md", "placeholders": [""]}, + { + "path": "AGENTS.md", + "sections": [{"name": "Fleet Bootstrap", "fidelity": "verbatim"}], + }, + ] + } + + with self.assertRaisesRegex(carry.CarryError, "placeholders and verbatim sections"): + carry.section_units(manifest, {"dotnet"}) def test_plan_unit_refuses_a_bare_carriage_return(self) -> None: with self.assertRaisesRegex(carry.CarryError, "bare carriage return"): From 00711fa14406d0d480ee43fb463cdd8570e59481 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 7 Sep 2026 13:39:47 -0700 Subject: [PATCH 4/7] Delete the Region Form the Boundary Fix Left Unused `comparable_region` replaced `region_text` in both comparisons, and the write path builds its own lines because it needs the target's own terminators, so nothing in the module called `region_text` any more. Its docstring reference, which claimed it was the form to write, was wrong for the same reason. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/carry.py | 10 +++------- scripts/tests/test_carry.py | 8 ++++---- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/scripts/carry.py b/scripts/carry.py index 21761df34..27af5a2f9 100755 --- a/scripts/carry.py +++ b/scripts/carry.py @@ -464,11 +464,6 @@ def section_span(lines: list[str], heading: str) -> tuple[int, int] | None: return None if start is None else (start, len(lines)) -def region_text(lines: list[str], span: tuple[int, int]) -> str: - """The exact bytes of the line range, terminators included.""" - return "".join(lines[span[0] : span[1]]) - - def comparable_region(lines: list[str], span: tuple[int, int]) -> str: """The region rendered the way `spec/audit.py`'s `extract_section` renders it. @@ -477,8 +472,9 @@ def comparable_region(lines: list[str], span: tuple[int, int]) -> str: file carries it. The consequence is worth stating, because it decides what this tool writes: a section that ends the hub's file and the same section followed by a heading downstream, with one blank line between, render to the identical string. The fidelity comparison therefore reads - them as the same content, and the raw bytes do not, so this is the form to compare and - `region_text` is the form to write. + them as the same content, and the raw bytes do not, so this is the form to compare. It is not + the form to write: `replace_sections` builds its own lines, because writing needs the target's + own terminators and this form has flattened them. """ bodies = [line[: len(line) - len(terminator(line))] for line in lines[span[0] : span[1]]] if span[1] == len(lines) and lines and terminator(lines[-1]): diff --git a/scripts/tests/test_carry.py b/scripts/tests/test_carry.py index 75425c8fd..aedeaaef7 100755 --- a/scripts/tests/test_carry.py +++ b/scripts/tests/test_carry.py @@ -495,8 +495,8 @@ def test_a_section_ending_the_target_file_keeps_no_trailing_blank_line(self) -> ) def test_comparable_region_matches_the_fidelity_check_across_the_file_boundary(self) -> None: - """`region_text` and `extract_section` differ by one newline exactly when one copy's section - ends its file and the other's does not, which is the case this form exists to reconcile.""" + """A region's raw bytes and `extract_section` differ by one newline exactly when one copy's + section ends its file and the other's does not, which is what this form reconciles.""" hub = "# T\n\n## X\n\nbody\n" padded = "# T\n\n## X\n\nbody\n\n## Y\n\nlocal\n" @@ -512,8 +512,8 @@ def test_comparable_region_matches_the_fidelity_check_across_the_file_boundary(s carry.comparable_region(carry.split_lines(hub), hub_span), ) self.assertNotEqual( - carry.region_text(carry.split_lines(hub), hub_span), - carry.region_text(carry.split_lines(padded), padded_span), + "".join(carry.split_lines(hub)[hub_span[0] : hub_span[1]]), + "".join(carry.split_lines(padded)[padded_span[0] : padded_span[1]]), ) def test_plan_sections_refuses_on_a_later_unit_before_anything_is_written(self) -> None: From 68f754041537568a6e8b114409f5d6b04650282d Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 7 Sep 2026 13:49:27 -0700 Subject: [PATCH 5/7] Answer Copilot's Three Findings on the Section Re-Vendor Recast the semicolon in the governed-drift refusal as two sentences. `GOVERNANCE.md` "Documentation Style Conventions" bans one in agent-authored text, and `prose_lint.py` reads Markdown rather than Python string literals, so nothing local caught it. The rest of this branch's added prose was swept for the same shape and carries none. Reword the help text both menus end on. "The menu shows each the tasks that apply to it" is pre-existing on `develop` in both files, and this change only rewrapped the paragraph around it, but it was raised against both files here and it sits in the sentence this change rewrote, so it is cheaper to fix than to file. Co-Authored-By: Claude Opus 5 (1M context) --- host-setup/menu.ps1 | 4 ++-- host-setup/menu.sh | 2 +- scripts/carry.py | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/host-setup/menu.ps1 b/host-setup/menu.ps1 index 885102d6a..9f8f6ccee 100644 --- a/host-setup/menu.ps1 +++ b/host-setup/menu.ps1 @@ -63,8 +63,8 @@ Usage: menu.ps1 [options] An interactive menu over this fleet's host and repo tooling: update the host tools, upgrade the OS packages, install the fleet skills, audit a cataloged repo, and pull the hub's verbatim-owned files, or just the verbatim rule sections inside a mixed file, into a downstream repo's own -worktree. Run from a hub checkout or from any other repo. The menu shows each the tasks that -apply to it. +worktree. Run from a hub checkout or from any other repo. Each is shown the tasks that apply +to it. Options: -y, -Yes Pass -Yes to each tool this menu runs, so a tool does not prompt. The menu's diff --git a/host-setup/menu.sh b/host-setup/menu.sh index ebfd250de..ae465653f 100755 --- a/host-setup/menu.sh +++ b/host-setup/menu.sh @@ -47,7 +47,7 @@ Usage: menu.sh [options] An interactive menu over this fleet's host and repo tooling: update the host tools, upgrade the OS, install the fleet skills, audit a cataloged repo, and pull the hub's verbatim-owned files, or just the verbatim rule sections inside a mixed file, into a downstream repo's own worktree. Run -from a hub checkout or from any other repo. The menu shows each the tasks that apply to it. +from a hub checkout or from any other repo. Each is shown the tasks that apply to it. Options: -y, --yes Pass --yes to each tool this menu runs, so a tool does not prompt. The menu's diff --git a/scripts/carry.py b/scripts/carry.py index 27af5a2f9..c9965c631 100755 --- a/scripts/carry.py +++ b/scripts/carry.py @@ -511,7 +511,8 @@ def guard_governed_drift(path: str, section: str, region: str, side: str) -> Non if audit_module().normalize(region) != normalize_eol(region): raise CarryError( f"{path} section '{section}' carries content the fidelity comparison normalizes away" - f" ({side} copy), so re-vendoring its bytes could revert per-repository drift; re-vendor it by hand" + f" ({side} copy), so re-vendoring its bytes could revert per-repository drift." + " Re-vendor that section by hand" ) From a5a969db497aba33503399dd9ba5303f8b046629 Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 7 Sep 2026 13:57:19 -0700 Subject: [PATCH 6/7] Read the Replacement's Line Ending From the File, Not From LF A replacement takes its ending from the section's own heading line, and that line carries no terminator when the heading is the last line of a file with no final newline. The fallback was LF, so a CRLF file in that shape came back mixed, which is the opposite of what this tool states it does with line endings. Reproduced before fixing: a CRLF target ending `## X` with no final newline came back as `# T\r\n\r\n## X\n\nhub body\n`, two CRLF lines and three LF. The file now answers for itself, read from its first terminated line, and LF is the answer only for a file that has no terminated line at all. Raised by Copilot as a suppressed finding, which opens no thread. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/carry.py | 17 ++++++++++++++++- scripts/tests/test_carry.py | 20 ++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/scripts/carry.py b/scripts/carry.py index c9965c631..dc4a8bc81 100755 --- a/scripts/carry.py +++ b/scripts/carry.py @@ -402,6 +402,20 @@ def terminator(line: str) -> str: return "\n" if line.endswith("\n") else "" +def file_ending(lines: list[str]) -> str: + """The terminator this file uses, read from its first terminated line. + + A replacement takes its ending from the section's own heading line, which carries none when + that heading is the last line of a file with no final newline. Falling back to LF there would + write LF into a CRLF file and leave it mixed, so the file answers for itself and LF is the + answer only for a file that has no terminated line at all. + """ + for line in lines: + if terminator(line): + return terminator(line) + return "\n" + + def normalize_eol(text: str) -> str: """`text` with every line ending as LF. @@ -608,8 +622,9 @@ def replace_sections( " so one replacement would splice over the other" ) out = list(target_lines) + default_ending = file_ending(target_lines) for (start, end), source_span in ordered: - ending = terminator(out[start]) or "\n" + ending = terminator(out[start]) or default_ending region = [] for line in source_lines[source_span[0] : source_span[1]]: body = line[: len(line) - len(terminator(line))] diff --git a/scripts/tests/test_carry.py b/scripts/tests/test_carry.py index aedeaaef7..a64840e9b 100755 --- a/scripts/tests/test_carry.py +++ b/scripts/tests/test_carry.py @@ -422,6 +422,26 @@ def test_replace_sections_preserves_the_targets_line_endings(self) -> None: self.assertNotIn("\n", result.replace("\r\n", "")) self.assertIn("The hub's current wording.\r\n", result) + def test_replace_sections_reads_the_ending_from_a_file_whose_heading_ends_it(self) -> None: + """A heading that is the last line of a file with no final newline carries no terminator, so + the ending has to come from the file rather than defaulting to LF and leaving it mixed.""" + hub = carry.split_lines("# T\n\n## X\n\nhub body\n") + target = carry.split_lines("# T\r\n\r\n## X") + + result = "".join(carry.replace_sections(target, hub, ["X"], "AGENTS.md")) + + crlf = result.count("\r\n") + self.assertEqual( + result.count("\n") - crlf, 0, f"bare LF written into a CRLF file: {result!r}" + ) + self.assertEqual(result, "# T\r\n\r\n## X\r\n\r\nhub body\r\n") + + def test_file_ending_falls_back_to_lf_only_when_no_line_is_terminated(self) -> None: + self.assertEqual(carry.file_ending(carry.split_lines("a\r\nb\r\n")), "\r\n") + self.assertEqual(carry.file_ending(carry.split_lines("a\nb\n")), "\n") + self.assertEqual(carry.file_ending(carry.split_lines("only line")), "\n") + self.assertEqual(carry.file_ending([]), "\n") + def test_replace_sections_terminates_a_region_the_hub_ended_unterminated(self) -> None: source = carry.split_lines(HUB_DOC.rstrip("\n")) target = carry.split_lines( From e8a90177b592f28b26c13a40c8dc12e84453dcba Mon Sep 17 00:00:00 2001 From: Pieter Viljoen Date: Mon, 7 Sep 2026 14:05:49 -0700 Subject: [PATCH 7/7] Hold the Lazily Imported Audit Module Rather Than Reaching for It Per Line `fence_step` calls `audit_module` once per line, and that function scanned `sys.path` and re-entered `import audit` on every one of them. Measured on this repository's own `GOVERNANCE.md`, 333 lines and 22 level-two sections: 28.8 ms to locate every section against 0.9 ms with the module held, over 7326 calls, and a re-vendor walks each file several times. `functools.cache` on the accessor, which keeps the import lazy while making "on first use" mean once rather than every time. Raised by Copilot as a suppressed finding, which opens no thread. Co-Authored-By: Claude Opus 5 (1M context) --- scripts/carry.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/scripts/carry.py b/scripts/carry.py index dc4a8bc81..101dd7b3f 100755 --- a/scripts/carry.py +++ b/scripts/carry.py @@ -3,6 +3,7 @@ import argparse import fnmatch +import functools import hashlib import itertools import json @@ -360,12 +361,16 @@ def verify_target( raise CarryError(f"target has unrelated changes: {', '.join(sorted(dirty))}") +@functools.cache def audit_module() -> Any: - """`spec/audit.py`, imported on first use rather than at module import. + """`spec/audit.py`, imported on first use rather than at module import, then held. It is a script rather than a package module and imports its own sibling by bare name, so the spec directory has to be importable before it is. Lazily, so a tree carry never pays to load - the audit for a reader it does not use. + the audit for a reader it does not use. Cached because `fence_step` reaches for it once per + line: locating this repository's own `GOVERNANCE.md` sections costs 28.8 ms of `sys.path` + scanning and `sys.modules` lookups uncached against 0.9 ms cached, over 7326 calls, and a + re-vendor walks each file several times. """ spec = str(ROOT / "spec") if spec not in sys.path: