Add Manifest-Driven Verbatim Tree Propagation - #804
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a manifest-driven mechanism for carrying fully owned hub-generated directory trees (starting with .github/skills/) into downstream repositories, and wires that fidelity model into validation, audit, and the standup/resync procedures so drift can be detected and safely repaired.
Changes:
- Extend the fleet manifest schema and validation to support
trees[]entries withverbatim-treefidelity and prune semantics. - Add
scripts/carry.pywithcheckand guardedapplymodes to propagate manifest-owned trees into isolated downstream worktrees. - Integrate
verbatim-treeintospec/audit.pyand document the new workflow in STANDUP/RESYNC/AUDIT and scripts docs.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| STANDUP.md | Document applying manifest-owned whole-tree declarations during standup. |
| RESYNC.md | Add carry-tool step for manifest-owned trees in the resync procedure. |
| AUDIT.md | Define the new verbatim-tree audit dimension and expected findings. |
| scripts/README.md | Document carry.py usage and its safety checks. |
| scripts/carry.py | New carry engine for manifest-owned trees with check/apply and write-safety validation. |
| scripts/tests/test_carry.py | New unit tests covering inventory/compare/apply behavior and safety checks. |
| spec/files.schema.json | Add trees to the files manifest schema and require it. |
| spec/files.json | Declare .github/skills as the first manifest-owned verbatim-tree target. |
| spec/fidelity-model.md | Document verbatim-tree semantics and its no-normalization rule. |
| spec/validate.py | Validate trees[] shape, overlap rules, canonical roots, and Copilot skill references. |
| spec/audit.py | Audit downstream verbatim-tree targets for missing/modified/extra paths and staleness. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (1)
scripts/carry.py:123
- apply_tree() can create the declared target root (via target_root.mkdir(...)) without emitting a corresponding "create ..." line. That means an apply that only needs to materialize the root (for example, an empty included source inventory) produces no change output even though it created a path, which contradicts the tool's documented behavior of reporting created/updated/removed paths.
changes = []
target_root.mkdir(parents=True, exist_ok=True)
for relative in result["missingDirectories"]:
ptr727
commented
Aug 18, 2026
Suppressed findings response for Copilot review round 4957297201, count: 1.
Fixed in 4e3b62a. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (4)
spec/validate.py:473
- Validation currently treats a missing top-level "trees" key as an empty list (no error), even though files.schema.json now requires it and CI does not run JSON-schema validation. This can silently disable verbatim-tree declarations if someone forgets to add the key.
files = load("spec/files.json")
baseline = files.get("baseline", [])
if not isinstance(baseline, list):
errors.append("files.json: 'baseline' must be an array")
baseline = []
scripts/carry.py:49
- relative_root() allows paths containing ".." segments (for example "a/../b") as long as the resolved path stays within the repo root. The manifest validator rejects ".." outright, and carry.py should match that for defense in depth and to avoid surprising path normalization.
def relative_root(root: pathlib.Path, value: str) -> pathlib.Path:
resolved_root = root.resolve()
candidate = resolved_root / value
resolved_candidate = candidate.resolve(strict=False)
try:
scripts/tests/test_carry.py:9
- All other scripts/tests/test_*.py files in this repo are runnable scripts with a shebang, a module docstring describing how to run them, and
from __future__ import annotations. test_carry.py is missing that header, which makes the test suite inconsistent and harder to discover/execute the same way as the other script tests.
import pathlib
import subprocess
import sys
import tempfile
import unittest
sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent))
import carry
scripts/carry.py:232
- verify_target() relies on
git merge-base --is-ancestorfor the develop-ancestry check, but when it fails the raised error is the generic "git ... failed" message. This is a common user error, so the failure should be reported explicitly as an ancestry requirement.
git(target, "fetch", "origin", "develop")
git(target, "merge-base", "--is-ancestor", "origin/develop", "HEAD")
ptr727
commented
Aug 18, 2026
Suppressed findings response for Copilot review round 4957333510, count: 4.
Fixed in 758a5da. Validation reports a missing required
Fixed in 758a5da. The carry boundary rejects absolute declarations and every
Fixed in 758a5da. The test has the repository-standard shebang, module docstring, future annotations import, and executable mode.
Fixed in 758a5da. A dedicated ancestry reader distinguishes a normal false result from a Git execution failure and reports that the target branch must contain current |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (2)
scripts/carry.py:81
- inventory() currently adds every visited directory to Inventory.directories when
included(relative_dir + "/placeholder", patterns)is true. Whenpruneis enabled, carry.py inventories the target with patterns ["**/"], which makes that condition true for all target directories, including non-empty parent directories that exist only to hold included files. If the source include patterns are narrower (for example [".txt"]) the source inventory does not record those parent directories, and compare() will incorrectly report them as extraDirectories, causingcheckto fail andapplyto fail its post-apply comparison even when the file bytes are identical.
relative_dir = current_path.relative_to(root).as_posix()
if relative_dir != "." and included(relative_dir + "/placeholder", patterns):
directories.add(relative_dir)
scripts/tests/test_carry.py:123
- Add a regression test for pruned targets where the source include patterns are narrower than "**/" (for example [".txt"]). Without this, inventory()/compare() can mistakenly flag non-empty parent directories as extraDirectories under prune, even when file bytes match, and the behavior will be easy to regress.
def test_apply_reports_empty_target_root_creation(self) -> None:
with tempfile.TemporaryDirectory() as temp:
root = pathlib.Path(temp)
repository = root / "repo"
source_root = root / "source"
ptr727
commented
Aug 18, 2026
Suppressed findings response for Copilot review round 4957373199, count: 2.
Fixed in 5b4d498. Inventories track only genuinely empty included leaf directories. Structural parents containing carried files are implicit on both sides, while pruning removes extra empty leaves and any ancestors made empty by that removal.
Fixed in 5b4d498. The new regression compares identical |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (2)
scripts/carry.py:151
- Prune-mode apply can fail when extra files live under an extra directory. The initial inventory does not record non-empty directories as extraDirectories, so apply_tree() removes the extra files but leaves their now-empty parent directories behind. The post-apply compare then reports those directories as extraDirectories and apply aborts.
After deleting an extra file, walk upward removing newly-empty ancestor directories (stopping at target_root) unless the directory is required by the source inventory (explicit empty dir or parent of a source file).
for relative in result["extra"]:
destination = target_root / relative
destination.unlink()
changes.append(f"remove {destination.relative_to(repository_root)}")
scripts/tests/test_carry.py:156
- There is no test that covers pruning an extra directory that only becomes empty after its extra files are removed. This is the main failure mode for prune=true targets (e.g., extra skill directories), and it would have caught the post-apply compare failure described in carry.py.
Add a unit test where the target contains an extra file under an extra directory, assert that apply_tree() removes both the file and the now-empty directory, and that a follow-up compare reports clean.
def test_apply_prunes_empty_extra_directory_ancestors(self) -> None:
with tempfile.TemporaryDirectory() as temp:
root = pathlib.Path(temp)
source_root = root / "source"
repository = root / "repo"
ptr727
commented
Aug 18, 2026
Addressed both suppressed findings from Copilot review #804 (review) in b17e3d7: apply mode now prunes ancestor directories that become empty after extra-file deletion while preserving every source-required directory, and the regression suite covers the exact extra-file-inside-extra-directory case. Local verification passed, including ruff, mypy, 713 unit tests, repository/prose/spec gates, all three script self-tests, distribution freshness, and diff checks. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (1)
spec/validate.py:545
- The Copilot-instructions validation checks whether each referenced skill path is inside a carried tree by comparing against tree["source"]. For generic tree declarations where source and target differ, this will validate the wrong root. Since Copilot runs in the downstream repo, the check should be against the declaration's target root (tree["target"]).
if not any(
pathlib.PurePosixPath(tree["source"]) in pathlib.PurePosixPath(path).parents
for tree in validated_trees
):
ptr727
commented
Aug 18, 2026
Addressed both suppressed findings from Copilot review round 8 in 568818b: tree digests now include typed empty-directory records as well as file records, with a regression proving directory-only differences change the digest; and the fleet audit caches canonical blob hashes by source path so repeated repository comparisons reuse the first read/hash. Full local verification passes with 715 tests. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Suppressed comments (2)
spec/fidelity-model.md:18
- This description implies overlaps are only rejected across prune boundaries. Because overlapping targets can still cause incorrect apply behavior even when neither declaration prunes, the fidelity contract should state that overlapping target roots are rejected outright (or else document/define a safe disjointness model).
A declaration in `trees[]` uses **verbatim-tree**. Each included source file exists at the corresponding target path with identical bytes. A pruned target contains no extra path. The tree root exists even when the included source inventory is empty. The carry engine rejects overlapping prune boundaries because two declarations cannot own the same path safely.
scripts/tests/test_carry.py:215
- This test only covers the prune-boundary overlap case. Since overlapping targets are unsafe even when neither declaration prunes (later apply steps can overwrite earlier ones), add coverage for the non-pruned overlap case by setting both prune flags false and renaming the test accordingly.
def test_rejects_overlapping_pruned_targets(self) -> None:
declarations = [
{
"source": "source-a",
"target": ".github",
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
ptr727
commented
Aug 18, 2026
Addressed both suppressed findings from Copilot review round 9 in feec639: the fidelity model now states that target roots never overlap regardless of pruning, and the regression now proves two non-pruned overlapping declarations are rejected. The same commit fixes and resolves both inline validator findings. Full local verification passes with 715 tests. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (2)
spec/validate.py:548
- spec/validate.py only checks that a Copilot-referenced skill path lives under some carried tree's target root. It does not check whether that path is actually included by the tree's include patterns.
This can let .github/copilot-instructions.md reference a skill that will not be propagated by the verbatim-tree declaration (e.g., a future declaration with a narrower include), which will then fail downstream when Copilot tries to load it. Validate that the referenced skill path is both under the target root and matched by include[].
if not any(
pathlib.PurePosixPath(tree["target"]) in pathlib.PurePosixPath(path).parents
for tree in validated_trees
):
errors.append(
scripts/carry.py:256
- validate_declarations() assumes every manifest tree entry has the right shape (source/target/include/prune types). If spec/files.json is malformed, carry.py can raise KeyError/TypeError and emit a traceback instead of a controlled CarryError, which makes failures harder to interpret.
Harden validate_declarations() to validate required keys, basic types, and unknown fields up front, then run the overlap check on the validated targets.
def validate_declarations(declarations: list[dict[str, Any]], hub: pathlib.Path) -> None:
for index, left in enumerate(declarations):
if left.get("fidelity") != "verbatim-tree":
raise CarryError(f"tree declaration has unsupported fidelity: {left.get('fidelity')}")
relative_root(hub, left["source"])
ptr727
commented
Aug 18, 2026
Addressed both suppressed findings from Copilot review round 10 in 5b1ae85: Copilot-referenced skill paths must now be under a carried target and match its include patterns; and carry.py validates every tree declaration object, required/unknown fields, path and selector shapes, include patterns, and booleans before applicability filtering, producing controlled CarryError failures. Malformed declarations have focused regression coverage. Full local verification passes with 716 tests. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (1)
spec/validate.py:504
- spec/validate.py currently defaults a missing tree-declaration appliesTo to "" (via tree.get("appliesTo", "")). This means CI validation can pass even when a tree declaration is missing the required appliesTo field (and/or has a typo like "applyTo"), but scripts/carry.py will later refuse the manifest at runtime because it requires appliesTo. Emit a validation error when appliesTo is missing so the contract is enforced consistently and typos are caught early.
if tree.get("fidelity") != "verbatim-tree":
errors.append(f"files.json: tree {source} fidelity must be 'verbatim-tree'")
check_selector(f"tree {source}", tree.get("appliesTo", "*"))
include = tree.get("include")
ptr727
commented
Aug 18, 2026
Addressed the suppressed finding from Copilot review round 11 in 44ead73: spec validation now reports a missing required appliesTo field and only validates a selector when the field is present, matching the schema and carry runtime rather than defaulting to all repositories. Full local verification passes with 716 tests. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (2)
scripts/carry.py:328
- verify_target() only evaluates the destination path of a rename/copy status line (it takes the last token after " -> "), which can miss unrelated changes outside owned roots. Example:
git mv seed.txt owned/seed.txtmodifiesseed.txtoutside the owned tree, but this code only checksowned/seed.txtand would treat the worktree as safe/clean.
To preserve the write-safety contract, treat rename/copy lines as affecting both the source and destination paths and ensure all involved paths are within owned_roots.
for row in git(target, "status", "--porcelain", "--untracked-files=all").splitlines():
relative = row[3:].split(" -> ")[-1]
path = (target / relative).resolve(strict=False)
if not any(path == root or root in path.parents for root in owned_roots):
dirty.append(relative)
scripts/tests/test_carry.py:304
- Add a regression case covering a rename/copy that crosses the owned-root boundary (e.g.
git mv seed.txt owned/seed.txt). This ensures verify_target() rejects unrelated changes even when Git reports them as a single rename line rather than separate delete/add entries.
(worktree / "unrelated.txt").write_text("dirty", encoding="utf-8")
with self.assertRaisesRegex(carry.CarryError, "unrelated changes"):
carry.verify_target(worktree, {"url": str(remote)}, [owned])
ptr727
commented
Aug 18, 2026
Addressed both suppressed findings from Copilot review round 12 in 30574db: verify_target now parses NUL-delimited porcelain status and checks both source and destination paths for rename/copy entries against the owned roots; a regression moves a tracked file from outside into an owned tree and proves the worktree is rejected. Full local verification passes with 716 tests. |
Uh oh!
There was an error while loading. Please reload this page.
Summary
verbatim-treedeclarations to the fleet file manifestcheckandapplycarry engine for downstream worktrees.github/skills/distribution as the first consumerRoot Cause
GitHub Copilot reads skills from the pull request head tree. Host-global skills and hub-only
.agents/skills/content are unavailable when Copilot reviews a downstream repository.Impact
Downstream repositories can receive fully owned generated trees without a second hand-authored source. The audit detects missing, stale, modified, and extra content. The apply path validates repository identity, worktree isolation, ancestry, unrelated changes, containment, and symlinks before writing.
Validation
ruff check .ruff format --check .mypyscripts/repo_gate.pyspec/validate.pyscripts/build_dist.py --checkspec/audit.py --selftestCloses#797