diff --git a/AGENTS.md b/AGENTS.md index 31fee71f..d661c883 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -152,6 +152,11 @@ order, and step 4 is the one you must not skip. and the split are pushed and verified: finish with the command it prints and never re-run into fresh legs, which abandons that pair. +`execute` also records the plan's answers about the source's submodules in +the split's `project.yaml`, under `submodules:` — per submodule a leg keeps, +its `path`, `leg`, `resolution:` and `source_commit`. It confers nothing, +nothing rewrites it, and the doctor holds each submodule to it. + `execute` opens a pull request, because these organisations are pull-request only. Never suggest a direct push to the default branch, and never `--admin`. @@ -242,7 +247,10 @@ which writes that file and nothing else — see 4. `review_required: true` is a question the policy is ASKING, not a defect found: answering it on their behalf puts a fact in their tree that nobody decided. The plan is `kind: placement-plan` and is not an adoption plan; - never hand one to `adopt-project.py`, which refuses it by name. + never hand one to `adopt-project.py`, which refuses it by name. A + submodule is judged by the adoption's `submodules:` record, not a rule + (`rule: null`, `confidence: recorded`), and is a `note` where no row names + it. One moved on purpose stays MISPLACED: its row is the human's to change. 5. **`NOT A SHAPE ROOT` is a question for the human, not a task.** There is no `project.yaml` and no `family.yaml`. The two ways in are `adopt-project.py plan` (a repository that already exists, converted in diff --git a/adopt-project.py b/adopt-project.py index 6b54ebae..9fa6e65a 100755 --- a/adopt-project.py +++ b/adopt-project.py @@ -46,7 +46,10 @@ `xf-project-` topic on all three (skipped for local remotes), and then VERIFIES by blob sha that every source path landed in exactly one place, and that the root's only submodules are the - two leg mounts, each at the commit pushed to its leg. + two leg mounts, each at the commit pushed to its leg. The split's + `project.yaml` records the leg each source submodule kept in a + leg was answered into, under `submodules:`, for `shape-doctor.py` + to check it against. THE PLAN IS AN ARTIFACT A HUMAN OR AN AI EDITS. That is why it is YAML with reasons in it rather than a pipe between two processes: the classifier is @@ -4901,6 +4904,100 @@ def _where_it_landed(assembly: Path, written: str) -> str | None: if landed.is_relative_to(root) else None +#: The key `execute` appends to the assembly's `project.yaml` (#189). The +#: doctor reads it by this name through the module it loads, and the +#: project's own `validate-manifest.py`, which imports nothing of this tool's, +#: spells it once more. +SUBMODULE_RECORD = "submodules" + +#: The comment above that block, in the manifest itself, because the +#: manifest is read by people who never saw the plan it came from. +SUBMODULE_RECORD_PREAMBLE = ( + "", + "# THE SOURCE'S OWN SUBMODULES, and the leg each one was ANSWERED into", + "# when this repository was adopted (#189). Written once, by", + "# `adopt-project.py execute`, from the adoption plan: one row per", + "# submodule the split kept in a leg, its path inside that leg, the", + "# `resolution:` the plan gave the entry that covered it, and the source", + "# commit the plan was answered at. `shape-doctor.py` checks each", + "# submodule a leg holds against it. Like every field here, it confers", + "# nothing.", + f"{SUBMODULE_RECORD}:", +) + + +def submodule_record(entries: list, tree: list, commit: str) -> str: + """The `submodules:` block `execute` appends to `project.yaml`, or "" + when the split keeps no submodule of the source's in a leg (#189). + + WHY THE ANSWER IS WRITTEN DOWN. Which leg holds a source submodule is a + human's answer under #166 -- `plan` asks it of every entry that covers a + gitlink, whatever rule its name matches -- and the plan that carries the + answer is not committed anywhere. Without a record, the assembly holds + `.cursor` in the code leg and nothing else, and `shape-doctor.py` cannot + tell that answer from a submodule added by hand, so the most it can say + is a note. With one, a submodule in the leg recorded is placed and one in + the other leg is MISPLACED, with the manifest as the evidence. + + ONE ROW PER GITLINK KEPT IN A LEG, from `_answered_legs`, the reading + `check` holds the plan to: by the time `execute` calls this, + `_refuse_what_check_finds` has refused a gitlink no entry covers, one + covered twice, one left unanswered and one answered `root`. A gitlink + answered `drop` is in no leg, so nothing can be checked against it, and + one added at that path later is a gitlink no record covers. The path is + the gitlink's path in the source, which `git filter-repo` keeps as its + path in the leg. `resolution:` is written when the plan's entry has one; + a directory entry's answer covers every gitlink under it, so each row + carries that entry's line. + + A SOURCE WITH NO SUBMODULE GETS "" and so the `project.yaml` -- and the + split -- it always got, byte for byte. + + EVERY PATH AND REASON IS DOUBLE-QUOTED, by `repo_shape.double_quoted`, + the writer of the escapes the reader resolves (#199). Always quoted, + unlike `y`: a submodule may sit at `null`, `true` or `0001`, which a + plain scalar reads back as something that is not a path. And a character + `str.isprintable` refuses is spelled by its code point: NEL and U+2028 + end a line for the reader, a no-break or zero-width space is another name + than the one it looks like, and the surrogate a byte that is not UTF-8 is + kept as cannot be written by `write_lf` at all, where `\\udce9` reads + back as that surrogate. So every row reads back as the name the doctor + holds the submodule at to it, and the manifest is written after both + legs exist. A name with a control character or a byte that is not UTF-8 + in it is one the doctor reads no further: it reports it as a name no + report can carry. + """ + gitlinks = [path for path, mode, _, _ in tree if mode == GITLINK_MODE] + kept = _kept_in_a_leg(_answered_legs(entries, gitlinks), gitlinks) + if not kept: + return "" + resolutions = {str(e.get("path")): _resolution_text(e.get("resolution")) + for e in entries} + lines = list(SUBMODULE_RECORD_PREAMBLE) + for path, leg, entry in kept: + lines.append(f" - path: {double_quoted(path)}") + lines.append(f" leg: {leg}") + if resolutions.get(entry): + lines.append( + f" resolution: {double_quoted(resolutions[entry])}") + lines.append(f' source_commit: "{commit}"') + return "\n".join(lines) + "\n" + + +def _resolution_text(resolution) -> str | None: + """A plan entry's `resolution:` as the text the record carries, or None. + + A scalar is written as the plan's reader returned it. A blank one is no + reason, and a mapping or a list under the key is not one line of text, + so neither is written: the record's `resolution:` is optional, and a row + without one still says where the submodule was answered into. + """ + if resolution is None or isinstance(resolution, (dict, list)): + return None + text = str(resolution) + return text if text.strip() else None + + def _commit_the_split(plan: Plan, source: Source, assembly: Path, names: dict, urls: dict, values: dict, work_root: Path, paths_for: dict, seeded: list, spec_path: str, @@ -4960,9 +5057,17 @@ def _commit_the_split(plan: Plan, source: Source, assembly: Path, spec_path, code_path) step = "committing the split" + # The plan's submodule answers outlive the plan only here (#189). + # An empty record appends nothing, so a source with no submodule + # gets the manifest it always got. + append = {"Makefile": ADOPT_MAKEFILE_BLOCK} + record = submodule_record(plan.entries, source.tree(), + source.commit) + if record: + append["project.yaml"] = record materialized = materialize_assembly_root( SHAPE_ROOT, assembly, values, collision_dir=COLLISION_DIR, - append={"Makefile": ADOPT_MAKEFILE_BLOCK}) + append=append) for intended, actual in materialized.collisions: print(f" beside {actual} (the source already has {intended}; " "nothing was overwritten)") diff --git a/scripts/repo_shape.py b/scripts/repo_shape.py index 2bcae52f..55fa5228 100644 --- a/scripts/repo_shape.py +++ b/scripts/repo_shape.py @@ -21,6 +21,8 @@ - `file_sha256` — the per-file digest used by the shape pin's `files:` block, mirroring `neutral-product-pin`'s per-file `sha256` rows. - `NamingPolicy` — the classifier over `contracts/repository-naming.yaml`. + - `path_spelling_fault` — whether a path is spelled the one way Git + records it, asked of a leg's mount path and of a recorded submodule's. - `same_repository` and the remote-url arithmetic under it — the ONE definition of whether two remote spellings name one repository, which `shape-doctor.py` reports by and `scripts/siblings.py` refuses by. @@ -348,10 +350,13 @@ def double_quoted(text: str) -> str: """`text` as a double-quoted scalar `_scalar` reads back as `text`. The subset's one writer quotes with this: `adopt-project.py`'s `y`, for - the adoption plan and for `shape-doctor.py`'s placement plan. It escapes - a backslash and a double quote, which would bend or end the quoting; - `\\n`, `\\r` and `\\t` by name; and every other character `str.isprintable` - refuses by its code point. PART OF THAT CLASS THE READER CANNOT TAKE + the adoption plan and for `shape-doctor.py`'s placement plan, and its + `submodule_record`, for the `submodules:` record `execute` appends to + `project.yaml`, which the doctor holds a submodule to only where a row + reads back as its path (#189). It escapes a backslash and a double + quote, which would bend or end the quoting; `\\n`, `\\r` and `\\t` by + name; and every other character `str.isprintable` refuses by its code + point. PART OF THAT CLASS THE READER CANNOT TAKE RAW. It reads a file line by line with `str.splitlines`, which ends a line at `\\n` and `\\r` -- the macOS folder icon is a file named `Icon\\r` -- and also at `\\x0b`, `\\x0c`, `\\x1c`-`\\x1e`, `\\x85`, @@ -1935,6 +1940,55 @@ def checked_value(what: str, value, pattern: re.Pattern = SAFE_ARG_RE) -> str: return text +# --------------------------------------------------------------------------- +# A path spelled the one way Git records it (#179, #189) +# --------------------------------------------------------------------------- +# +# Git records a path in exactly one spelling: relative to the top of its +# repository, `/` between segments, and no segment empty, `.` or `..`. +# `git update-index` refuses `./x`, `x/`, `a//b`, `a/./b`, `a/../b`, `/x` and +# `.` alike as `Invalid path`, so a record that names a path in any other +# spelling names one no tree holds, and nothing is ever found at it. +# +# TWO RULES ASK IT. The leg-path rule in `shape_materialize.py` asks it of a +# leg's mount path (#179), and a project's own `validate-manifest.py` asks it +# of each path `project.yaml`'s `submodules:` records (#189) -- which is why +# it lives HERE: a project carries this file and not `shape_materialize.py`. +# Each says the answer in its own sentence, about a leg or about a submodule. +# +# A BACKSLASH AND A DRIVE LETTER ARE NOT ASKED. On POSIX each is an ordinary +# character of a name -- Git records `vendor/a\b` and `C:/x` there, refusing +# them only on Windows -- and an adoption records a source submodule at +# either. They are faults only in a path a PERSON types for commands that +# Windows also runs, which is the leg-path rule's own question. + +#: The ways a path can fail to be spelled as Git records it, in the order +#: `path_spelling_fault` asks: empty, absolute, climbing out with `..`, +#: naming the top itself, and any other empty or `.` segment. +PATH_EMPTY = "empty" +PATH_ABSOLUTE = "absolute" +PATH_CLIMBS = "climbs" +PATH_ITSELF = "itself" +PATH_NOT_CANONICAL = "not-canonical" + + +def path_spelling_fault(text: str) -> str | None: + """Why `text` is not one path spelled as Git records it, as one of the + `PATH_*` codes above, or None when it is.""" + if not text: + return PATH_EMPTY + if text.startswith("/"): + return PATH_ABSOLUTE + segments = text.split("/") + if ".." in segments: + return PATH_CLIMBS + if all(segment in ("", ".") for segment in segments): + return PATH_ITSELF + if "" in segments or "." in segments: + return PATH_NOT_CANONICAL + return None + + def accepts_role(found, role: str) -> bool: """Is `found` an acceptable classification for a leg DECLARED as `role`? diff --git a/scripts/shape_materialize.py b/scripts/shape_materialize.py index 1e3a6cf5..38adc5e5 100644 --- a/scripts/shape_materialize.py +++ b/scripts/shape_materialize.py @@ -43,8 +43,9 @@ sys.path.insert(0, str(Path(__file__).resolve().parent)) from repo_shape import ( # noqa: E402 - CHAIN_RECORD_FIELD, SAFE_ARG_RE, NamingPolicy, Refusal, checked_value, - double_quoted, file_sha256, + CHAIN_RECORD_FIELD, PATH_CLIMBS, PATH_ITSELF, PATH_NOT_CANONICAL, + SAFE_ARG_RE, NamingPolicy, Refusal, checked_value, double_quoted, + file_sha256, path_spelling_fault, ) # The reader's own parse and its own comment rule, for "The two lines a pin # is moved by" below. `_strip_comment` is that module's private name and is @@ -2069,6 +2070,14 @@ def copy_out(src: Path, dst: Path) -> None: # asks (see "Commit trailers" above for the same argument). This module # is in no copy list and both tools already import it. # +# EXCEPT THE SEGMENTS, which a project's copy DOES ask (#189): its +# `validate-manifest.py` holds each path `project.yaml`'s `submodules:` +# records to the spelling Git records a path in, so +# `repo_shape.path_spelling_fault` answers that for both rules, and +# `_spelling_reason` below adds only what a TYPED path can get wrong -- a +# drive letter and a backslash, which on POSIX are characters of a name Git +# records -- and says every answer about a leg. +# # CANONICAL = a relative POSIX path of one or more segments, each non-empty # and neither `.` nor `..`: no leading `/` or `./` (nor a drive letter), no # trailing `/`, no `//`, no backslash. The two legs also take DIFFERENT @@ -2132,8 +2141,28 @@ def copy_out(src: Path, dst: Path) -> None: ) +#: What each way a path's SEGMENTS can be misspelled means for a leg's mount +#: path. `path_spelling_fault` asks the segments; the empty and absolute +#: answers it also gives are `_spelling_reason`'s own, asked first, beside +#: the drive letter and the backslash only a typed path can get wrong. +LEG_SEGMENT_REASONS = { + PATH_CLIMBS: ("it has a `..` segment, so it names a different place than " + "it spells and can climb out of the assembly root"), + PATH_ITSELF: ("it names the assembly root itself, and a leg is mounted " + "INSIDE the root, not at it"), + PATH_NOT_CANONICAL: ( + "Git records a mount without its `.` segments (a leading `./`), a " + "trailing `/` or a doubled `/`, so the mount and its `.gitmodules` " + "entry would name two different paths"), +} + + def _spelling_reason(text: str) -> str | None: - """Why `text` is not a CANONICAL spelling of a leg path, or None.""" + """Why `text` is not a CANONICAL spelling of a leg path, or None. + + The segments are `repo_shape.path_spelling_fault`'s, the rule a + project's own validator also holds a recorded submodule path to (#189). + """ if not text: return "it is empty" if text.startswith("/") or WINDOWS_ABSOLUTE_RE.match(text): @@ -2142,18 +2171,7 @@ def _spelling_reason(text: str) -> str | None: "its path is relative to it") if "\\" in text: return "it has a backslash, and a path here is written with `/`" - segments = text.split("/") - if ".." in segments: - return ("it has a `..` segment, so it names a different place than it " - "spells and can climb out of the assembly root") - if all(segment in ("", ".") for segment in segments): - return ("it names the assembly root itself, and a leg is mounted " - "INSIDE the root, not at it") - if "" in segments or "." in segments: - return ("Git records a mount without its `.` segments (a leading " - "`./`), a trailing `/` or a doubled `/`, so the mount and its " - "`.gitmodules` entry would name two different paths") - return None + return LEG_SEGMENT_REASONS.get(path_spelling_fault(text)) def _unmountable_reason(text: str) -> str | None: diff --git a/shape-doctor.py b/shape-doctor.py index f81db0ef..1d1b3331 100755 --- a/shape-doctor.py +++ b/shape-doctor.py @@ -158,21 +158,35 @@ `.github/**`, say -- is named all the same, because the row's promise is that no gitlink a leg keeps goes unreported. -THE NOTE SAYS WHICH KIND OF PLACEMENT IT FOUND (Codex review on #186, round -2). Which leg a submodule belongs in is the adoption plan's answer: -`adopt-project.py plan` asks it of every entry that holds a gitlink (#166). -But nothing in the assembly records that anyone answered it. A project adopted -before that question, or one with a submodule added by hand, has its gitlink -in whichever leg it landed in, and the doctor cannot tell those from an -answered one. So the row says what it CAN tell. A gitlink whose name no rule -names, a root rule names, or the rule of the leg it is in names, is `kept`: -the root keeps no source submodule, so a leg holds it. A gitlink whose name a -rule assigns to the OTHER leg -- `docs/theme` in the code leg, under -`spec-governance` -- is `unverifiable`: named with the rule and the leg, in -the row, in `--json` (`placement`, `rule`) and in the plan, with the words -"confirm against the plan, or move it". Both stay a `note`, never MISPLACED, -because a human's answer under #166 looks exactly like the second, and -MISPLACED would put back the false positive #176 removed. +A SUBMODULE IS CHECKED AGAINST THE ANSWER THE ADOPTION RECORDED (#189). Which +leg a submodule belongs in is the adoption plan's answer: `adopt-project.py +plan` asks it of every entry that holds a gitlink (#166), and `execute` writes +each answer for a submodule it keeps in a leg into `project.yaml`, under +`submodules:` -- its path in the leg, the leg, the plan's `resolution:` and +the source commit. A gitlink in the leg its row names is `recorded`: placed, +named in the row, and no reason to keep it off `ok`. One in the OTHER leg is +`misplaced`, and it is a path in the wrong leg like any other -- a FINDING, +counted in MISPLACED, an entry under `paths:` in the plan -- because the +manifest asserts where it belongs, and a FINDING is exactly something else +asserting it. The same record answers the policy's question about a leg's +`.gitmodules` -- which leg owns the submodules it registers -- where it +places every submodule that leg keeps there and the doctor could read every +name the leg holds, so that file is placed too, and an adoption whose record +places every submodule reads `ok`. + +WHERE NO ROW NAMES IT, THE NOTE SAYS WHICH KIND OF PLACEMENT IT FOUND (Codex +review on #186, round 2). A project adopted before the record, or one with a +submodule added by hand, has its gitlink in whichever leg it landed in, and +the doctor cannot tell those from an answered one. So the row says what it +CAN tell. A gitlink whose name no rule names, a root rule names, or the rule +of the leg it is in names, is `kept`: the root keeps no source submodule, so +a leg holds it. A gitlink whose name a rule assigns to the OTHER leg -- +`docs/theme` in the code leg, under `spec-governance` -- is `unverifiable`: +named with the rule and the leg, in the row, in `--json` (`placement`, +`rule`) and in the plan, with the words "confirm against the plan, or move +it". Both stay a `note`, never MISPLACED, because a human's answer under #166 +that nothing recorded looks exactly like the second, and MISPLACED would put +back the false positive #176 removed. AND THE DOCTOR STILL MOVES NOTHING. A path changing legs is a pull request on the leg it leaves, a pull request on the leg it joins, and one pin bump in the @@ -206,8 +220,9 @@ 0 COMPLIANT: every row ok and the shape is current 1 findings: the shape is behind, a copy has drifted, a validator is red, a leg is off its pin, a manifest is there and does not load, or a - tracked path sits in a leg the path policy puts elsewhere -- - COMPLIANT SHAPE BEHIND, DRIFTED, MISPLACED, INVALID + tracked path sits in a leg the path policy puts elsewhere -- or, for a + submodule, a leg other than the one `project.yaml` records for it + (#189) -- COMPLIANT SHAPE BEHIND, DRIFTED, MISPLACED, INVALID 2 NOT A SHAPE ROOT: neither `project.yaml` nor `family.yaml` is there 3 usage or environment -- nothing here is a statement about the tree that was pointed at: no such root, this file is not sitting in a @@ -458,20 +473,37 @@ def not_in_the_standard(detail: str) -> int: #: put back on every run, and the one its name draws from this policy is #: `default`'s, which offers `root` -- not an answer for a submodule. #: -#: TWO CLASSES, SAID APART (Codex review on #186, round 2). A submodule's leg -#: is the adoption plan's answer, and nothing in the assembly records that -#: anyone gave it, so the row says only what the policy can tell it. `kept` is -#: a submodule whose name no rule claims, a root rule claims, or the rule of -#: the leg it is in claims: the root keeps no source submodule, so a leg holds -#: it, and there is nothing to check it against. `unverifiable` is one whose -#: name a rule assigns to the OTHER leg: that may be the person adopting -#: having answered the question the other way, and may be nobody having -#: answered it, and the doctor cannot tell. Neither is MISPLACED -- that is the -#: false positive #176 removed. +#: FOUR CLASSES, SAID APART. A submodule's leg is the adoption plan's answer, +#: and `execute` records each one it keeps in a leg in `project.yaml`'s +#: `submodules:` (#189). `recorded` is a submodule in the leg its row names, +#: and `misplaced` one in the other leg: the manifest asserts where it +#: belongs, so the second is a FINDING. Where no row names its path -- a +#: project adopted before the record, a submodule added by hand -- the row +#: says only what the policy can tell it (Codex review on #186, round 2). +#: `kept` is a submodule whose name no rule claims, a root rule claims, or the +#: rule of the leg it is in claims: the root keeps no source submodule, so a +#: leg holds it, and there is nothing to check it against. `unverifiable` is +#: one whose name a rule assigns to the OTHER leg: that may be the person +#: adopting having answered the question the other way, and may be nobody +#: having answered it, and with no record the doctor cannot tell. Neither of +#: those two is MISPLACED -- that is the false positive #176 removed. +SUBMODULE_RECORDED = "recorded" +SUBMODULE_MISPLACED = "misplaced" SUBMODULE_KEPT = "kept" SUBMODULE_UNVERIFIABLE = "unverifiable" KEPT_SUBMODULE = ("a submodule; the assembly root keeps no source submodule " "(#166), so a leg holds it") +#: Where the record is, said the same way in the row, `--json` and the plan. +THE_RECORD = "project.yaml's `submodules:`" +#: The key naming the leg the record gives a submodule, in `--json` and the +#: plan alike; present only on a submodule a record names. +RECORDED_LEG = "recorded_leg" +#: The key that marks a `misplaced` entry as a submodule the record puts in +#: the other leg, rather than a file the policy judged. +AGAINST_THE_RECORD = "submodule" +#: The key of a leg's entry naming the paths whose policy question the record +#: answered -- its `.gitmodules` -- present only on a leg that has one. +ANSWERED = "answered" #: What belongs to EVERY repository, so the `placement` row never calls it #: misplaced in a leg. @@ -3194,38 +3226,60 @@ def named_offenders(rows: list) -> str: return "; ".join(named) + " (" + ", ".join(totals) + ")" -def named_submodules(kept: list) -> str: +def first_three(names: list) -> str: + """The first three of `names`, and how many more there are.""" + named = ", ".join(names[:3]) + if len(names) > 3: + named += f", and {len(names) - 3} more" + return named + + +def named_submodules(kept: list, with_rule: bool = True) -> str: """The first three submodules of one class, each with the rule that named - it where one did (Codex review on #186, round 2). + it where one did (Codex review on #186, round 2) -- and without it for a + submodule checked against the record, which no rule decided (#189). """ def one(item: dict) -> str: - if item["rule"] is None: + if not with_rule or item["rule"] is None: return item["path"] return f"{item['path']} (rule {item['rule']})" - named = ", ".join(one(item) for item in kept[:3]) - if len(kept) > 3: - named += f", and {len(kept) - 3} more" - return named + return first_three([one(item) for item in kept]) + + +def named_against_the_record(rows: list) -> str: + """The first three submodules in a leg other than the recorded one, each + with the leg the record names (#189).""" + return first_three([f"{row['path']} (recorded in the " + f"{row['classified_as']} leg)" for row in rows]) def kept_submodules(kept: list) -> str: - """The clauses naming the submodules the legs keep, one for each class, - and why none of them is judged (#176). + """The clauses naming the submodules the legs keep, one for each class + the row does not count as misplaced (#176, #189). ONE CLAUSE FOR EACH CLASS PRESENT, IN EVERY BRANCH THAT CAN MEET ONE, so that a submodule beside a misplaced file, or beside a question, is said - exactly as it is said on its own. The two classes are SAID APART because - they are not the same claim: `kept` is a submodule the root cannot hold, - and `unverifiable` is one in the leg a rule says it does not belong in, - which the doctor cannot check against the answer the adoption plan gave - (Codex review on #186, round 2). The second asks for a human's look in so - many words, and is still no finding. + exactly as it is said on its own. The classes are SAID APART because + they are not the same claim. `recorded` is a submodule in the leg the + adoption recorded for it: checked, and placed (#189). `kept` is a + submodule the root cannot hold, and `unverifiable` is one in the leg a + rule says it does not belong in, which with no record the doctor cannot + check against the answer the adoption plan gave (Codex review on #186, + round 2). The last asks for a human's look in so many words, and is + still no finding. `misplaced` has no clause here: it is a path in the + wrong leg, and it is named where those are. """ + recorded = [item for item in kept + if item["placement"] == SUBMODULE_RECORDED] placed = [item for item in kept if item["placement"] == SUBMODULE_KEPT] asked = [item for item in kept if item["placement"] == SUBMODULE_UNVERIFIABLE] clauses = [] + if recorded: + clauses.append( + f"{len(recorded)} submodule(s) in the leg {THE_RECORD} records " + f"for them (#189): {named_submodules(recorded, with_rule=False)}") if placed: clauses.append( f"{len(placed)} submodule(s) the path policy does not judge, " @@ -3240,28 +3294,87 @@ def kept_submodules(kept: list) -> str: return "; ".join(clauses) -def submodule_placement(policy, role: str, path: str) -> dict: - """What the policy can say about the leg a submodule is in, and no more - (Codex review on #186, round 2). +def answered_clause(legs: list) -> str: + """The clause naming each leg's `.gitmodules` whose policy question the + record answered, or "" where none was (#189). - THE RULE THAT NAMES THE PATH, AND WHICH LEG IT ASSIGNS. `rule_for` is the - policy's own first-match lookup and nothing else: a submodule is judged by - no extension majority and no `default`, which read what a FILE looks like. - A rule that assigns the OTHER leg makes the placement `unverifiable`; one - that assigns this leg, assigns the root, or leaves the leg to a human - (`ambiguous`) -- or no rule at all -- leaves it `kept`. Neither is a - finding: the leg of a submodule is the adoption plan's answer under #166, - the assembly records no such answer, and a gitlink in the other leg looks - the same whether somebody chose it or nobody did. + SAID, because the row no longer counts it among the paths the policy + will not call, and a reader who knows the policy asks about every + `.gitmodules` is owed where this one's answer came from. + """ + answered = [path for leg in legs for path in leg.get(ANSWERED) or []] + if not answered: + return "" + return (f"{THE_RECORD} answers the question the policy asks of " + f"{', '.join(answered)}: which leg owns the submodules it " + "registers") - The rule's id is in the result whenever one matched, in either class, so + +def held_clauses(detail: dict) -> str: + """`kept_submodules` and `answered_clause` as the tail of the row's + sentence: `; ` and their clauses, or nothing where neither has one -- + no submodule at all, or only misplaced ones, which `misplaced_clauses` + names -- so that no reason ends on a separator with nothing after it. + """ + clauses = (kept_submodules(detail.get("submodules") or []), + answered_clause(detail.get("legs") or [])) + said = "; ".join(clause for clause in clauses if clause) + return f"; {said}" if said else "" + + +def recorded_placement(role: str, rule, row: dict) -> dict: + """A submodule held to the row `project.yaml` records for its path (#189). + + THE RECORD DECIDES, NOT THE POLICY. The row is the adoption plan's answer, + written into the manifest by `execute`: in that leg the submodule is + `recorded`, and in the other it is `misplaced`. `rule` is the id of the + policy rule that names the path, or None, kept so that a reader is told + which rule the doctor did not apply here either. The row's `resolution` + travels with it, because it is the human's reason. + """ + leg = str(row["leg"]) + resolution = row.get("resolution") + found = {"rule": rule, RECORDED_LEG: leg, + "resolution": resolution if isinstance(resolution, str) else None} + if leg == role: + return {"placement": SUBMODULE_RECORDED, **found, + "note": (f"a submodule {THE_RECORD} records in the {leg} " + "leg, where it is (#189)")} + return {"placement": SUBMODULE_MISPLACED, **found, + "note": (f"a submodule {THE_RECORD} records in the {leg} leg, " + f"and it is in the {role} leg (#189)")} + + +def submodule_placement(policy, role: str, path: str, + recorded: dict | None = None) -> dict: + """What the record says about the leg a submodule is in, or failing a + record, what the policy can say about it, and no more (#189; Codex + review on #186, round 2). + + A ROW OF `project.yaml`'s `submodules:` NAMING THE PATH DECIDES IT, in + `recorded_placement`. + + WITHOUT ONE, THE RULE THAT NAMES THE PATH, AND WHICH LEG IT ASSIGNS. + `rule_for` is the policy's own first-match lookup and nothing else: a + submodule is judged by no extension majority and no `default`, which read + what a FILE looks like. A rule that assigns the OTHER leg makes the + placement `unverifiable`; one that assigns this leg, assigns the root, or + leaves the leg to a human (`ambiguous`) -- or no rule at all -- leaves it + `kept`. Neither is a finding: the leg of a submodule is the adoption + plan's answer under #166, an assembly adopted before the record or a + submodule added by hand records no such answer, and a gitlink in the + other leg looks the same whether somebody chose it or nobody did. + + The rule's id is in the result whenever one matched, in every class, so that a reader is told which rule the doctor did NOT apply to a submodule. """ rule = policy.rule_for(path) + name = None if rule is None else str(rule["id"]) + if recorded is not None: + return recorded_placement(role, name, recorded) if rule is None: return {"placement": SUBMODULE_KEPT, "rule": None, "note": KEPT_SUBMODULE} - name = str(rule["id"]) leg = rule["leg"] if leg in LEG_ROLES and leg != role: return {"placement": SUBMODULE_UNVERIFIABLE, "rule": name, @@ -3274,6 +3387,71 @@ def submodule_placement(policy, role: str, path: str) -> dict: "the doctor does not apply it to a submodule)")} +def recorded_submodules(adopt, manifest) -> dict: + """`{path in its leg: row}` for each row of `project.yaml`'s + `submodules:` a submodule can be held to (#189). + + ONLY A ROW THAT SAYS SOMETHING. A row that is not a mapping, has no text + `path:`, or names a leg that is not `spec` or `code` decides nothing + here: the project's own `validate-manifest.py` names each of those, in + the `manifest` row, and a submodule whose row cannot be read is one no + record covers. A path spelled as Git records no path (`./x`, `x/`) is + one the validator names too, and no gitlink is ever at it. + + WHERE TWO ROWS NAME ONE PATH, THE FIRST DECIDES, because the validator + accepts the first and names the second as `recorded twice` -- the same + reading, so no row it refuses decides anything here. That holds when it + refuses the first row's `leg:` as well: the path is that row's, it + decides nothing, and the second does not stand in for it. `execute` + writes one row per gitlink, so only an edit makes two, and whichever + row decides, the `manifest` row is a FINDING and the verdict red. + + Read by the key `adopt-project.py execute` writes, through the module + this row has already loaded, so that the writer and this reader spell it + once. + """ + rows = (manifest or {}).get(adopt.SUBMODULE_RECORD) + record: dict = {} + claimed: set = set() + for row in rows if isinstance(rows, list) else []: + path = row.get("path") if isinstance(row, dict) else None + if not isinstance(path, str) or path in claimed: + continue + claimed.add(path) + if row.get("leg") in LEG_ROLES: + record[path] = row + return record + + +def misplaced_submodule(item: dict) -> dict: + """The `misplaced` entry for a submodule in a leg its record does not + name (#189), in the keys a misplaced FILE's entry carries. + + IN THE SAME LIST, because it is the same claim: a tracked path sits in a + leg other than the one something asserts it belongs in. So it counts in + MISPLACED, is an entry under `paths:` in the plan with `leg:` the leg + recorded, and is resolved the way any entry there is. The record decided + it and no rule did, so `rule` is null, `confidence` is `recorded`, and + the reason says where the record is and carries the `resolution:` the + adoption plan gave, on one line, because it is the human's reason and a + report or plan line it split would break. `files` is the one gitlink and + `bytes` is 0, which is what `git ls-tree -l` reports for one. + `submodule` marks the entry so the row names it apart from the files the + policy judged. + """ + leg = item[RECORDED_LEG] + reason = f"{THE_RECORD} records this submodule in the {leg} leg (#189)" + if item.get("resolution"): + reason += ": " + " ".join(str(item["resolution"]).split()) + return {"path": item["path"], "leg": item["leg"], + "path_in_leg": item["path_in_leg"], "classified_as": leg, + "rule": None, "reason": reason, "confidence": SUBMODULE_RECORDED, + "question": None, "files": 1, "bytes": 0, + "review_required": False, + "direction": f"{leg} in the {item['leg']} leg", + AGAINST_THE_RECORD: True} + + def placement_row(status: str, reason: str, next_command=None, detail=None) -> Row: """One `placement` row with the `--json` keys ALWAYS present. @@ -3294,8 +3472,107 @@ def placement_row(status: str, reason: str, next_command=None, return Row("placement", "placement", status, reason, next_command, base) +def leg_submodules(policy, role: str, rel: str, held: list, + record: dict) -> tuple: + """The submodules one leg keeps, each placed by its record or, with + none, by what the policy can say; and the `misplaced` entries for those + in a leg other than the one recorded (#189). + + Split out of `audit_leg`, so that one reads as the walk of a leg's files + and this as what becomes of the gitlinks set aside from it. + """ + submodules = [{"path": f"{rel}/{path}", "leg": role, + "path_in_leg": path, + **submodule_placement(policy, role, path, record.get(path))} + for path in held] + wrong = [misplaced_submodule(item) for item in submodules + if item["placement"] == SUBMODULE_MISPLACED] + return submodules, wrong + + +def gitmodules_the_record_answers(adopt, submodules: list, + unread: list) -> str | None: + """The leg's own `.gitmodules`, when the record answers the policy's + question about it, or None (#189). + + `.gitmodules` IS THE POLICY'S ONE QUESTION ABOUT SUBMODULES: rule + `ambiguous-gitmodules` asks which leg owns the submodules it registers. + An adoption answered it with the leg of every one of them -- `check` + refuses a plan that sends `.gitmodules` to one leg and a submodule it + registers to another (#166) -- and `execute` recorded that leg for each + submodule it kept. So where the record places EVERY submodule this leg + keeps in this leg, the answer is in the record, and it is this leg: the + file is placed, and no question is left. + + EVERY ONE, NOT ANY ONE. Beside a recorded submodule a leg may keep one + added by hand, which `git submodule add` registers in this same file and + no row names, or one the record puts in the other leg. The record does + not say this leg owns that one, so it has not answered the question for + all the file registers: the file stays the question it was, beside the + note or the finding that names that submodule, and the row does not say + the record answered it. So does a leg that keeps no submodule at all. + + AND EVERY NAME IN THE LEG READ (#189's second review, F1). A name no + report or plan can carry -- a control character, or a byte that is not + UTF-8 -- is set aside by `leg_tracked_files` before its mode is read, so + a submodule kept under one is in no list here and no row was held to + it. `unread` is that residue, and a leg with any of it is not one whose + every submodule the record is known to place: the file stays the + question it was. + """ + if (submodules and not unread + and all(item["placement"] == SUBMODULE_RECORDED + for item in submodules)): + return adopt.GITMODULES + return None + + +def walked_row(found, role: str, rel: str) -> dict: + """One walked path of a leg, in the keys the row, `--json` and the plan + carry for it.""" + return { + "path": f"{rel}/{found.path}", + "leg": role, + "path_in_leg": found.path, + "classified_as": found.leg, + "rule": found.rule, + "reason": found.reason, + "confidence": found.confidence, + "question": found.question, + "files": found.files, + "bytes": found.bytes, + "review_required": found.leg is None, + "direction": (f"{found.leg} in the {role} leg" if found.leg + else f"unclassified in the {role} leg"), + } + + +def walked_findings(walked: list, role: str, rel: str, + answered: str | None) -> tuple: + """The walked paths of one leg in the wrong leg, the questions, and the + paths whose question the record `answered` (#189). + + Split out of `audit_leg`. A path the policy will not call is a question, + except `answered`: the leg's `.gitmodules` where + `gitmodules_the_record_answers` finds the answer recorded, and that + answer is this leg, so it is neither a question nor in the wrong leg. + """ + misplaced: list = [] + review: list = [] + settled: list = [] + for found in walked: + row = walked_row(found, role, rel) + if found.leg is None and found.path == answered: + settled.append(row["path"]) + elif found.leg is None: + review.append(row) + elif found.leg != role: + misplaced.append(row) + return misplaced, review, settled + + def audit_leg(adopt, policy, patterns: list, role: str, rel: str, - root: Path) -> tuple: + root: Path, record: dict) -> tuple: """ONE leg: its summary, the paths in the wrong leg, the questions, and the submodules it keeps. @@ -3327,11 +3604,25 @@ def audit_leg(adopt, policy, patterns: list, role: str, rel: str, `.github/actions` or `LICENSE` is dropped from the walk like a file there, and is still one a leg keeps, so it is still named. - AND EACH ONE IS CLASSIFIED AS WHAT THE DOCTOR CAN KNOW OF IT, NOT JUDGED - (Codex review on #186, round 2). `submodule_placement` reads the rule the - policy names its path by: a rule that assigns the OTHER leg makes the - submodule `unverifiable`, because its leg is the adoption plan's answer - and the assembly records none. The `note` is still a note. + AND EACH ONE IS HELD TO THE ANSWER THE ADOPTION RECORDED, WHERE THERE IS + ONE (#189). `record` is `recorded_submodules`, and `leg_submodules` holds + each gitlink to it: a row naming the path makes the submodule `recorded` + in that leg and `misplaced` in the other, and a + misplaced one is ALSO returned among the paths in the wrong leg, as + `misplaced_submodule` words it, and counted in the leg's `misplaced`. + Where no row names it, it is classified as what the doctor can know of + it, not judged (Codex review on #186, round 2): `submodule_placement` + reads the rule the policy names its path by, and a rule that assigns the + OTHER leg makes it `unverifiable`. That `note` is still a note. + + THE RECORD IS CONSULTED BEFORE THE LEG'S QUESTIONS ARE COUNTED (#189). + The policy asks which leg owns the submodules a `.gitmodules` registers, + and in a leg whose every submodule the record places there, and whose + every name could be read, the record is that answer + (`gitmodules_the_record_answers`): the file is placed, and the leg's + entry names it under `answered` instead of as a question. + Otherwise a real adoption, whose leg always holds the `.gitmodules` that + registers its submodules, could never read `ok`. """ entry = {"role": role, "path": rel} mount = root / rel @@ -3369,28 +3660,10 @@ def audit_leg(adopt, policy, patterns: list, role: str, rel: str, if path not in gitlinks and not any(regex.match(path) for regex in patterns)] walked = adopt.walk(policy, kept) - misplaced: list = [] - review: list = [] - for found in walked: - row = { - "path": f"{rel}/{found.path}", - "leg": role, - "path_in_leg": found.path, - "classified_as": found.leg, - "rule": found.rule, - "reason": found.reason, - "confidence": found.confidence, - "question": found.question, - "files": found.files, - "bytes": found.bytes, - "review_required": found.leg is None, - "direction": (f"{found.leg} in the {role} leg" if found.leg - else f"unclassified in the {role} leg"), - } - if found.leg is None: - review.append(row) - elif found.leg != role: - misplaced.append(row) + submodules, wrong = leg_submodules(policy, role, rel, held, record) + answered = gitmodules_the_record_answers(adopt, submodules, unwritable) + misplaced, review, settled = walked_findings(walked, role, rel, answered) + misplaced += wrong # FILES AND PATHS ARE DIFFERENT NUMBERS and the row prints both. `walk` # folds a directory whose files agree into ONE entry, so `src/` with two # hundred files under it is one PATH and two hundred FILES -- and a reason @@ -3400,10 +3673,9 @@ def audit_leg(adopt, policy, patterns: list, role: str, rel: str, "classified": len(kept), "paths": len(walked), "misplaced": len(misplaced), "review_required": len(review)}) - submodules = [{"path": f"{rel}/{path}", "leg": role, - "path_in_leg": path, - **submodule_placement(policy, role, path)} - for path in held] + if settled: + # Only when there is one, so that no other leg's entry changes. + entry[ANSWERED] = settled return entry, misplaced, review, submodules @@ -3471,6 +3743,50 @@ def leg_audit_summary(entry: dict) -> str: return f"{entry['path']}: {counted}" +def misplaced_clauses(misplaced: list) -> str: + """What the row says of the paths in the wrong leg: the files the policy + puts elsewhere, then the submodules the record puts elsewhere (#189). + + SAID APART, because what asserts each is different -- a named rule of the + path policy, or the leg `project.yaml` records for a submodule -- and a + reader disagrees with the one that decided. Each clause only when there + is one, so a project with no submodule gets the sentence it always got. + """ + held = [row for row in misplaced if row.get(AGAINST_THE_RECORD)] + files = [row for row in misplaced if not row.get(AGAINST_THE_RECORD)] + clauses = [] + if files: + clauses.append(f"{len(files)} path(s) sit in a leg the policy puts " + f"elsewhere: {named_offenders(files)}") + if held: + clauses.append(f"{len(held)} submodule(s) sit in a leg other than " + f"the one {THE_RECORD} records for them (#189): " + f"{named_against_the_record(held)}") + return "; ".join(clauses) + + +def submodules_verdict(detail: dict, audited: str) -> Row: + """The row when the legs keep submodules and nothing else is to say. + + `ok` ONLY WHEN THE RECORD PLACES EVERY ONE (#189). A submodule in the leg + `project.yaml`'s `submodules:` records for it is checked and placed, so + it does not keep the row off `ok`; the reason says "every tracked FILE", + because the policy classified the files and the record placed the + gitlinks, and the clause beside it names those -- and names a leg's + `.gitmodules` too, where the policy's question about it is the one the + record answered. One that no record names is the `note` it has been + since #176: the doctor did not check it. + """ + kept = detail["submodules"] + held = held_clauses(detail) + if all(item["placement"] == SUBMODULE_RECORDED for item in kept): + return placement_row( + OK, f"every tracked file classifies as the leg it is in{held} " + f"[{audited}]", None, detail) + return placement_row( + NOTE, f"nothing is in the wrong leg{held} [{audited}]", None, detail) + + def placement_verdict(detail: dict, audited: str, plan: str) -> Row: """What the audited legs add up to, in the order the answers outrank. @@ -3480,21 +3796,30 @@ def placement_verdict(detail: dict, audited: str, plan: str) -> Row: finding and never an implicit anything; then the legs that could not be read, which are a note because what is missing is a READ and not a fault of this repository's; then the submodules the legs keep, which are a note - because nothing is wrong with them (#176). `ok` is last and is the only - branch that claims every tracked path was classified, which is why - nothing above it may fall through to it. - - A SUBMODULE NEVER MOVES THIS ROW ON ITS OWN ACCOUNT. Beside a misplaced - path or a question it adds one clause per class to the sentence those - already make, and on its own it is the `note` that keeps the row off `ok`: - a gitlink is a tracked path the policy did not classify, so "every tracked - path classifies as the leg it is in" would not be true of it. That holds - for the `unverifiable` class too, which names the rule and asks for a - human's look (Codex review on #186, round 2): it is not MISPLACED, because - a person adopting under #166 answered that leg, and the doctor cannot tell - that answer from none. Its `next` is the plan's only where a misplaced - path or a question already offers one, because the confirmation it asks - for is against the ADOPTION plan, a document this row cannot name. + where no record names them, because nothing is wrong with them (#176), + and `ok` where the record places them all (#189). `ok` is last and is + the only branch that claims every tracked path was classified or placed, + which is why nothing above it may fall through to it. + + A SUBMODULE THE RECORD PUTS IN THE OTHER LEG IS A PATH IN THE WRONG LEG + (#189): it is in `misplaced`, it makes this row a FINDING, and the verdict + MISPLACED, because `project.yaml` asserts where it belongs. And a leg's + `.gitmodules` whose question the record answered is not in `review` + (`audit_leg`), so it no longer holds the row of an adoption at `note`. + + ANY OTHER SUBMODULE NEVER MOVES THIS ROW ON ITS OWN ACCOUNT. Beside a + misplaced path or a question it adds one clause per class to the + sentence those already make. On its own, one in its recorded leg leaves + the row `ok`, and one no record names is the `note` that keeps the row + off `ok`: a gitlink is a tracked path the policy did not classify, so + "every tracked path classifies as the leg it is in" would not be true of + it. That holds for the `unverifiable` class too, which names the rule and + asks for a human's look (Codex review on #186, round 2): it is not + MISPLACED, because a person adopting under #166 may have answered that + leg, and with no record the doctor cannot tell that answer from none. Its + `next` is the plan's only where a misplaced path or a question already + offers one, because the confirmation it asks for is against the ADOPTION + plan, a document this row cannot name. Reads what it needs out of `detail` rather than taking the same six numbers a second time: the row prints that dictionary, so a count this @@ -3506,7 +3831,7 @@ def placement_verdict(detail: dict, audited: str, plan: str) -> Row: review = detail["review_required"] counts = detail["counts"] kept = detail.get("submodules") or [] - held = f"; {kept_submodules(kept)}" if kept else "" + held = held_clauses(detail) if counts["unread_legs"] == len(detail["legs"]): # NOT `ok` AND NOT A FINDING. Every leg declined for a reason the # `legs` row already asserts or the manifest already carries, and a @@ -3518,9 +3843,7 @@ def placement_verdict(detail: dict, audited: str, plan: str) -> Row: extra = (f"; {len(review)} more path(s) need a human's reading" if review else "") return placement_row( - FINDING, - f"{len(misplaced)} path(s) sit in a leg the policy puts " - f"elsewhere: {named_offenders(misplaced)}{extra}{held} " + FINDING, f"{misplaced_clauses(misplaced)}{extra}{held} " f"[{audited}]", plan, detail) if review: @@ -3537,12 +3860,10 @@ def placement_verdict(detail: dict, audited: str, plan: str) -> Row: "everything could be") return placement_row( NOTE, - f"{said}{held} [{audited}]" if kept else f"{said}: {audited}", + f"{said}{held} [{audited}]" if held else f"{said}: {audited}", None, detail) if kept: - return placement_row( - NOTE, f"nothing is in the wrong leg{held} [{audited}]", None, - detail) + return submodules_verdict(detail, audited) return placement_row( OK, f"every tracked path classifies as the leg it is in: {audited}", None, detail) @@ -3574,12 +3895,16 @@ def check_placement(ctx: Context) -> Row: printed. A path the policy cannot CALL carries `review_required` and is a `note`: it is a question for a human, and this standard's rule is that an unanswered question is never a finding and never an implicit anything. - And a SUBMODULE a leg keeps is classified by nothing and is a `note` - naming it (#176): the policy reads names, a gitlink is not the file its - name looks like, and the assembly root keeps no source submodule (#166). - The note says which of two it is: `kept`, or `unverifiable` when a rule - assigns the path to the other leg and so the doctor cannot confirm the - adoption plan's answer. + And a SUBMODULE a leg keeps is classified by no rule (#176): the policy + reads names, a gitlink is not the file its name looks like, and the + assembly root keeps no source submodule (#166). It is held instead to + the leg `project.yaml`'s `submodules:` records for it, which + `adopt-project.py execute` writes from the adoption plan's answer + (#189): in that leg it is placed, and in the other it is a FINDING like + any path in the wrong leg. + One no record names is a `note` naming it, which says which of two it + is: `kept`, or `unverifiable` when a rule assigns the path to the other + leg and so the doctor cannot confirm the adoption plan's answer. AND NOTHING MOVES. The row's next command writes a plan; the plan is the human's to resolve and a later `--fix` is the thing that would carry it @@ -3617,6 +3942,7 @@ def check_placement(ctx: Context) -> Row: ignored = everywhere_patterns(ctx.shape) patterns = [glob_to_regex(pattern) for pattern in ignored] + record = recorded_submodules(adopt, ctx.manifest) per_leg = [] misplaced = [] review = [] @@ -3625,7 +3951,7 @@ def check_placement(ctx: Context) -> Row: role = str(leg.get("role") or "?") rel = str(leg.get("path") or role) entry, wrong, unsure, held = audit_leg(adopt, policy, patterns, role, - rel, ctx.root) + rel, ctx.root, record) per_leg.append(entry) misplaced.extend(wrong) review.extend(unsure) @@ -3810,6 +4136,38 @@ def placement_plan_paths(adopt, entries: list, unread: list) -> list: return lines +#: The comment above the plan's `submodules:` block where no record names +#: any of them (#176; Codex review on #186, round 2). +UNRECORDED_LEGEND = ( + "# Kept in a leg and NOT JUDGED: a submodule is a gitlink, which", + "# the path policy cannot tell from a file of the same name, and", + "# the assembly root keeps no source submodule (#166). Nothing", + "# here is to resolve, and no entry under `paths:` covers one.", + "# `placement: unverifiable` is a submodule at a path a rule", + "# assigns to the OTHER leg: its leg is the adoption plan's", + "# answer, which the doctor cannot verify here, so confirm it", + "# against the plan, or move it.", +) + +#: The same comment where `project.yaml`'s `submodules:` names at least one +#: of them (#189). A misplaced one is under `paths:` too, so this does not +#: say that none is. +RECORDED_LEGEND = ( + "# Kept in a leg, and judged by NO RULE: a submodule is a gitlink,", + "# which the path policy cannot tell from a file of the same name,", + "# and the assembly root keeps no source submodule (#166).", + "# `placement: recorded` is a submodule in the leg project.yaml's", + "# `submodules:` records for it (`recorded_leg:`), the answer the", + "# adoption plan gave (#189): placed, and nothing to resolve.", + "# `placement: misplaced` is one in the OTHER leg from the one", + "# recorded: it is an entry under `paths:` as well, resolved like", + "# any there. `placement: kept` is one no record names, which a", + "# leg holds; `placement: unverifiable` is one no record names at a", + "# path a rule assigns to the OTHER leg, so confirm it against the", + "# adoption plan, or move it.", +) + + def placement_plan_submodules(adopt, kept: list) -> list: """The `submodules:` block: what the legs keep and nobody is asked about. @@ -3832,25 +4190,27 @@ def placement_plan_submodules(adopt, kept: list) -> list: it cannot check the answer the adoption plan gave, so the file carries the instruction and nothing to fill in. + A SUBMODULE `project.yaml` RECORDS IS LISTED WITH THE LEG RECORDED (#189): + `placement: recorded` in that leg, `placement: misplaced` in the other, + each with `recorded_leg:`. A misplaced one is ALSO an entry under + `paths:`, because it is a path in the wrong leg, and the comment above + the block says so only where a record names one -- the comment a plan + with no record gets is #186's, word for word. + NOTHING AT ALL WHEN NO LEG KEEPS ONE, so the plan of every project without a submodule is the file it always was. """ if not kept: return [] - lines = ["", - "# Kept in a leg and NOT JUDGED: a submodule is a gitlink, which", - "# the path policy cannot tell from a file of the same name, and", - "# the assembly root keeps no source submodule (#166). Nothing", - "# here is to resolve, and no entry under `paths:` covers one.", - "# `placement: unverifiable` is a submodule at a path a rule", - "# assigns to the OTHER leg: its leg is the adoption plan's", - "# answer, which the doctor cannot verify here, so confirm it", - "# against the plan, or move it.", + recorded = any(RECORDED_LEG in item for item in kept) + lines = ["", *(RECORDED_LEGEND if recorded else UNRECORDED_LEGEND), "submodules:"] for item in kept: lines.append(f" - path: {adopt.y(item['path'])}") adopt.emit(lines, "in_leg", item["leg"], 4) adopt.emit(lines, "placement", item["placement"], 4) + if RECORDED_LEG in item: + adopt.emit(lines, RECORDED_LEG, item[RECORDED_LEG], 4) adopt.emit(lines, "rule", item["rule"], 4) adopt.emit(lines, "note", item["note"], 4) return lines diff --git a/templates/assembly-root/scripts/validate-manifest.py b/templates/assembly-root/scripts/validate-manifest.py index 6312dbf7..2263b58e 100755 --- a/templates/assembly-root/scripts/validate-manifest.py +++ b/templates/assembly-root/scripts/validate-manifest.py @@ -49,6 +49,24 @@ A link of the chain is VERIFIED against that link's own manifest when its tree is beside this root, and reported as `declared-unverified` — a WARNING, not a finding — when it is not. The whole check is OFFLINE. + - `submodules`, OPTIONAL: the record an adoption writes of the leg each + source submodule was answered into (#189). Absent is fine, because a + project with no source submodule has none, and nor does one adopted + before the record existed. Present, it is a list of mappings, one per + submodule a leg keeps: + `path` the submodule's path inside its leg, spelled the one + way Git records a path -- relative, `/` between + segments, none empty, `.` or `..` -- and named by one + row only. A backslash or a drive letter is a + character of the name, which Git records on POSIX. + `leg` `spec` or `code`: the assembly root keeps no + submodule but its legs. + `resolution` OPTIONAL text: the reason the adoption plan gave. + `source_commit` OPTIONAL, 40 hex: the source commit it was answered + at. + `adopt-project.py execute` writes it, once, from the adoption plan, and + `shape-doctor.py` holds each submodule a leg keeps to the row naming its + path. A row whose `path` or `leg` is refused here holds nothing there. EXIT CODES: 0 valid · 1 findings · 2 refusal (the file is missing or unreadable) """ @@ -62,10 +80,10 @@ sys.path.insert(0, str(Path(__file__).resolve().parent)) from repo_shape import ( # noqa: E402 - CHAIN_RECORD_FIELD, COMMIT_RE, PROJECT_ID_RE, SHA256_RE, - TREE_DIGEST_DEFINITION, VISIBILITY_CHOICES, NamingPolicy, Refusal, - accepts_role, find_repo_root, link_pins_from_trees, load_yaml, - repo_basename, + CHAIN_RECORD_FIELD, COMMIT_RE, PATH_NOT_CANONICAL, PROJECT_ID_RE, + SHA256_RE, TREE_DIGEST_DEFINITION, VISIBILITY_CHOICES, NamingPolicy, + Refusal, accepts_role, find_repo_root, link_pins_from_trees, load_yaml, + path_spelling_fault, repo_basename, ) DATE_RE = re.compile(r"^\d{4}-\d{2}-\d{2}$") @@ -647,6 +665,102 @@ def _leg_findings(leg, policy: NamingPolicy, pins: set[str], root, return out +#: The legs a source submodule may be recorded in (#189). The assembly is not +#: one of them: the root keeps no submodule but its two legs. +SUBMODULE_LEGS = ("spec", "code") + + +#: What a recorded `path` is, said beside one that is not. +SUBMODULE_PATH = ("it is the submodule's path inside its leg, relative and " + "without `..`") + + +def _submodule_path_problem(path, seen: set) -> str | None: + """Why one recorded `path` is not a submodule's path inside its leg, or + None. Records a good one in `seen`, so a second row for it is named. + + SPELLED THE ONE WAY GIT RECORDS A PATH (`repo_shape.path_spelling_fault`, + the rule a leg's mount path is held to as well). The doctor finds a + submodule by the path its leg's index records, and Git records none with + a leading `./`, a trailing `/`, a doubled `/` or a `.` segment, so a row + spelled that way would name nothing the doctor could ever hold to it. + A backslash or a drive letter is accepted: on POSIX each is a character + of a name Git records, and an adoption records a submodule at one. + """ + fault = path_spelling_fault(path) if isinstance(path, str) else None + if fault == PATH_NOT_CANONICAL: + return (f"path is {path!r}; Git records no path with a `.` segment, " + "a trailing `/` or a doubled `/`, so no submodule is ever at " + "it") + if fault is not None or not isinstance(path, str): + return f"path is {path!r}; {SUBMODULE_PATH}" + if path in seen: + return f"path {path!r} is recorded twice" + seen.add(path) + return None + + +def _submodule_row_findings(index: int, row, seen: set) -> list[str]: + """One row of the `submodules:` record. + + Split from `_submodules_findings` so that each reads as one question. + """ + where = f"submodules[{index}]" + if not isinstance(row, dict): + return [_finding( + "manifest-submodules", + f"{where} is {row!r}, expected a mapping with `path:` and " + "`leg:`")] + out = [] + problem = _submodule_path_problem(row.get("path"), seen) + if problem: + out.append(_finding("manifest-submodule-path", f"{where}: {problem}")) + if row.get("leg") not in SUBMODULE_LEGS: + out.append(_finding( + "manifest-submodule-leg", + f"{where}: leg is {row.get('leg')!r}, expected one of " + f"{list(SUBMODULE_LEGS)}: the assembly root keeps no submodule " + "but its legs")) + resolution = row.get("resolution") + if resolution is not None and not isinstance(resolution, str): + out.append(_finding( + "manifest-submodule-resolution", + f"{where}: resolution is {resolution!r}, expected the text the " + "adoption plan gave")) + commit = row.get("source_commit") + if commit is not None and not COMMIT_RE.match(str(commit)): + out.append(_finding( + "manifest-submodule-commit", + f"{where}: source_commit is {commit!r}, not 40 hex")) + return out + + +def _submodules_findings(manifest: dict) -> list[str]: + """`submodules`, OPTIONAL: the leg each source submodule was answered + into when this project was adopted (#189). + + A RECORD IS CHECKED BECAUSE SOMETHING READS IT. `shape-doctor.py` holds + every submodule a leg keeps to the row naming its path, and calls one in + the other leg MISPLACED; a row it could not read would leave that + submodule unchecked, and nothing would say so. Absent is fine: a project + with no submodule of its source's has nothing to record, and nor does + one adopted before the record was written. + """ + rows = manifest.get("submodules") + if rows is None: + return [] + if not isinstance(rows, list): + return [_finding( + "manifest-submodules", + f"submodules is {rows!r}, expected a list of `path:` and `leg:` " + "records")] + seen: set[str] = set() + out: list[str] = [] + for index, row in enumerate(rows): + out.extend(_submodule_row_findings(index, row, seen)) + return out + + def _owners_span_finding(owners: set[str]) -> list[str]: """Do the legs span more than one organisation? @@ -672,6 +786,7 @@ def _findings(manifest: dict, policy: NamingPolicy, root=None) -> list[str]: out.extend(_reference_finding(manifest)) out.extend(_visibility_finding(manifest)) out.extend(_shape_findings(manifest)) + out.extend(_submodules_findings(manifest)) legs = manifest.get("legs") if not isinstance(legs, list) or not legs: diff --git a/tests/test_adopt_e2e.py b/tests/test_adopt_e2e.py index b75efafe..2ded855c 100644 --- a/tests/test_adopt_e2e.py +++ b/tests/test_adopt_e2e.py @@ -268,6 +268,15 @@ def test_the_manifest_and_pins_describe_the_legs_that_were_created(adopted, assert gitlink == tip, "the gitlink and the pin move together or not" +def test_a_source_with_no_submodule_gets_no_submodule_record(adopted_clone): + """`execute` records the leg of each source submodule it keeps (#189), + and this source has none: its manifest gains no `submodules:` and no + comment about one, and is the manifest every adoption wrote before.""" + manifest = adopted_clone / "project.yaml" + assert "submodules" not in load_yaml(manifest) + assert "#189" not in manifest.read_text(encoding="utf-8") + + # --- verification ---------------------------------------------------------- def test_verification_accounts_for_every_source_path(adopted): diff --git a/tests/test_adopt_late_failure.py b/tests/test_adopt_late_failure.py index fc94d9ba..905c3bfe 100644 --- a/tests/test_adopt_late_failure.py +++ b/tests/test_adopt_late_failure.py @@ -253,6 +253,14 @@ def legs_arguments(tmp_path: Path, source=None) -> dict: branch=BRANCH, tracking="main") +class SplitPlan(dict): + """The plan `_commit_the_split` reads: `follow_ups` through `get`, for + the split message, and `entries`, which #189's `submodules:` record + is built from. These sources keep no submodule, so it has none.""" + + entries = () + + def split_arguments(tmp_path: Path, source) -> dict: """What `cmd_execute` hands `_commit_the_split` once both legs are pushed. The leg commits are stand-ins: every step that would read them @@ -261,7 +269,7 @@ def split_arguments(tmp_path: Path, source) -> dict: urls = leg_urls(tmp_path, names) urls[ROOT] = str(source.path) return dict( - plan={}, source=source, assembly=tmp_path / WORK / ASSEMBLY, + plan=SplitPlan(), source=source, assembly=tmp_path / WORK / ASSEMBLY, names=names, urls=urls, values={}, work_root=tmp_path / WORK, paths_for={SPEC: ["specs/"], CODE: ["src/"], "root": [README], "drop": []}, diff --git a/tests/test_adopt_submodules.py b/tests/test_adopt_submodules.py index 15c665cf..8dffb47f 100644 --- a/tests/test_adopt_submodules.py +++ b/tests/test_adopt_submodules.py @@ -23,7 +23,7 @@ resolve, run_script, write_plan) sys.path.insert(0, str(REPO / "scripts")) -from repo_shape import load_yaml # noqa: E402 +from repo_shape import load_yaml, parse_yaml # noqa: E402 #: What the end-to-end test needs and the mount tests below do not: they call @@ -104,6 +104,11 @@ def test_adopt_moves_existing_gitmodules_with_its_dependency(tmp_path, r"^submodule\..*\.path$", cwd=clone).stdout.splitlines() assert sorted(root_modules) == ["submodule.code.path code", "submodule.spec.path spec"] + # The answer outlives the plan (#189): the split's manifest records the + # leg the dependency was answered into, for the doctor to check it by. + assert load_yaml(clone / "project.yaml")["submodules"] == [{ + "path": "upstream/dependency", "leg": dependency_leg, + "resolution": "answered by the test", "source_commit": source_head}] assert git("rev-parse", "HEAD", cwd=source).stdout.strip() == source_head assert git("status", "--porcelain", cwd=source).stdout == "" @@ -1731,3 +1736,113 @@ def test_a_key_with_no_value_beside_no_submodule_adopts_and_clones(tmp_path): cloned = clone_the_split(thing, tmp_path) assert cloned.returncode == 0, cloned.stderr assert (clone / "code" / GITMODULES).read_bytes() == BARE_KEY.encode() + + +# --- the record of the answers, which the doctor reads (#189) --------------- +# +# `execute` appends to the split's `project.yaml` one row per source submodule +# it keeps in a leg: the path, the leg, the plan's `resolution:` and the +# source commit. These read the block `submodule_record` writes back with the +# reader every manifest goes through, so that what the doctor reads is the +# answer the plan gave. + +BLOB_MODE = "100644" +GITLINK = "160000" +#: The object every row below names: no row's record depends on it. +ANY_OID = "0" * 40 +SOURCE_COMMIT = "ab" * 20 + + +def tree_of(*rows: tuple) -> list: + """`Source.tree()`'s shape, sorted as it is: `(path, mode)` each.""" + return sorted((path, mode, ANY_OID, 0) for path, mode in rows) + + +def test_the_record_names_each_submodule_a_leg_keeps_with_its_answer( + adopter): + """ONE ROW PER GITLINK KEPT IN A LEG, AND NONE FOR ONE DROPPED. + + A directory entry's answer covers every gitlink under it, so both of + `upstream/`'s carry its `resolution:`; an entry with none gives none; a + dropped gitlink is in no leg, so nothing can be checked against it. + """ + entries = [ + {"path": GITMODULES, "leg": "code", "resolution": "with them"}, + {"path": UPSTREAM, "leg": "code", + "resolution": "what the code builds against"}, + {"path": "vendor/old", "leg": "drop", "resolution": "unused"}, + {"path": "docs/theme", "leg": "spec"}, + {"path": "src/", "leg": "code"}, + ] + tree = tree_of((GITMODULES, BLOB_MODE), ("docs/theme", GITLINK), + ("src/a.py", BLOB_MODE), (DEPENDENCY, GITLINK), + ("upstream/second", GITLINK), ("vendor/old", GITLINK)) + text = adopter.submodule_record(entries, tree, SOURCE_COMMIT) + assert text.startswith("\n# THE SOURCE'S OWN SUBMODULES"), text + answer = "what the code builds against" + assert parse_yaml(text) == {"submodules": [ + {"path": "docs/theme", "leg": "spec", + "source_commit": SOURCE_COMMIT}, + {"path": DEPENDENCY, "leg": "code", "resolution": answer, + "source_commit": SOURCE_COMMIT}, + {"path": "upstream/second", "leg": "code", "resolution": answer, + "source_commit": SOURCE_COMMIT}, + ]} + + +@pytest.mark.parametrize("tree, entries", [ + (tree_of((README, BLOB_MODE)), [{"path": README, "leg": "root"}]), + (tree_of(("vendor/old", GITLINK)), [{"path": "vendor/old", + "leg": "drop"}]), +], ids=["no-submodule", "every-submodule-dropped"]) +def test_a_source_with_no_submodule_kept_gets_no_record(adopter, tree, + entries): + """NOTHING TO APPEND, so the manifest and the split are what they were.""" + assert adopter.submodule_record(entries, tree, SOURCE_COMMIT) == "" + + +@pytest.mark.parametrize("name", [ + "null", "0001", "true", "a: b", "#hash", 'say "hi"', "back\\slash", + "tab\there", "- dash", "ends:", "caf\u00e9", "\\x41", + # Characters `str.isprintable` refuses, spelled by their code points: + "nb\u00a0sp", "ideo\u3000graphic", "zero\u200bwidth", "soft\u00adhyphen", + "bom\ufeff", "nel\u0085x", "line\u2028sep", "cr\rx", "bell\x07", +]) +def test_a_recorded_name_reads_back_as_that_name(adopter, name): + """A NAME YAML WOULD READ AS SOMETHING ELSE IS STILL THAT NAME. + + A submodule may sit at `null` or `0001`, which a plain scalar reads back + as nothing or as a number, and a path or a `resolution:` may hold a + colon, a hash, a quote, a backslash or a tab. Each is written quoted and + escaped, and reads back as itself. So does a name holding a character + that is not printable -- a no-break, ideographic or zero-width space, + NEL, U+2028, a carriage return -- which `repo_shape.double_quoted` + writes as YAML's escape for it, `\\r` by name and the rest by code + point, and the reader the doctor reads the record with resolves (#189, + #199); before, the reader kept the backslash, and the row named no + gitlink at all. A backslash already in a name is not read as the start + of one. + """ + entries = [{"path": name, "leg": "code", "resolution": name}] + text = adopter.submodule_record(entries, tree_of((name, GITLINK)), + SOURCE_COMMIT) + [row] = parse_yaml(text)["submodules"] + assert (row["path"], row["resolution"]) == (name, name), text + assert len(text.splitlines()) == text.count("\n"), ( + "every row is whole lines: no name may break one") + + +def test_a_name_that_is_not_utf8_is_spelled_and_still_written(adopter): + """A BYTE THAT IS NOT UTF-8 IS SPELLED, so `write_lf` can write the + manifest at all: the surrogate a path read with `surrogateescape` holds + for it would stop `execute` after both legs exist. It is spelled as that + surrogate's code point, `\\udce9`, and reads back as the name; `\\xe9` + would read back as U+00E9, which is another name (#189).""" + name = "caf\udce9" + text = adopter.submodule_record( + [{"path": name, "leg": "code"}], tree_of((name, GITLINK)), + SOURCE_COMMIT) + text.encode("utf-8") + assert ' - path: "caf\\udce9"\n' in text, text + [row] = parse_yaml(text)["submodules"] + assert row["path"] == name, text diff --git a/tests/test_manifest_validator.py b/tests/test_manifest_validator.py index 9aa2a803..15ec8cd9 100644 --- a/tests/test_manifest_validator.py +++ b/tests/test_manifest_validator.py @@ -9,7 +9,16 @@ import pytest -from conftest import run_script +from conftest import REPO, git, run_script + +sys.path.insert(0, str(REPO / "scripts")) +from repo_shape import parse_yaml, path_spelling_fault # noqa: E402 + +#: A name that holds a backslash or a drive letter is one Git records on +#: POSIX and refuses on Windows, where each is a separator or a drive. +POSIX_NAMES = pytest.mark.skipif( + os.name == "nt", + reason="Git for Windows records no path with a backslash or a drive") def validate(project): @@ -579,3 +588,150 @@ def test_an_install_form_is_a_leg_of_nothing(project): result = validate(project) assert result.returncode == 1 assert "naming-not-a-leg" in result.stderr + + +# --- the record of the source's own submodules (#189) ---------------------- +# +# `adopt-project.py execute` appends a `submodules:` block to the manifest, +# one row per source submodule it kept in a leg, and `shape-doctor.py` holds +# each submodule a leg keeps to the row naming its path. A row the doctor +# could not read would leave that submodule unchecked, so the project's own +# validator names it. + +#: A record as `execute` writes it: two rows, one with the plan's +#: `resolution:` and one without, and the source commit on each. +SUBMODULE_RECORD = ( + "\n# THE SOURCE'S OWN SUBMODULES (#189)\n" + "submodules:\n" + ' - path: ".cursor"\n' + " leg: code\n" + ' resolution: "editor rules the code leg reads"\n' + ' source_commit: "0123456789abcdef0123456789abcdef01234567"\n' + ' - path: "docs/theme"\n' + " leg: spec\n" + ' source_commit: "0123456789abcdef0123456789abcdef01234567"\n' +) + + +def append_record(project, text: str = SUBMODULE_RECORD) -> None: + manifest = project / "project.yaml" + manifest.write_bytes((manifest.read_text() + text).encode("utf-8")) + + +def test_a_submodule_record_is_accepted(project): + append_record(project) + result = validate(project) + assert result.returncode == 0, result.stderr + result.stdout + assert "manifest ok" in result.stdout + + +@pytest.mark.parametrize("old, new, code", [ + (" leg: code\n", " leg: root\n", "manifest-submodule-leg"), + (" leg: code\n", " leg: drop\n", "manifest-submodule-leg"), + (' - path: ".cursor"\n', ' - path: "/etc/.cursor"\n', + "manifest-submodule-path"), + (' - path: ".cursor"\n', ' - path: "../.cursor"\n', + "manifest-submodule-path"), + (' - path: ".cursor"\n', ' - path: ""\n', "manifest-submodule-path"), + (' - path: "docs/theme"\n', ' - path: ".cursor"\n', + "manifest-submodule-path"), + (' resolution: "editor rules the code leg reads"\n', + " resolution: 3\n", "manifest-submodule-resolution"), + (' source_commit: "0123456789abcdef0123456789abcdef01234567"\n', + ' source_commit: "main"\n', "manifest-submodule-commit"), + (' - path: ".cursor"\n leg: code\n' + ' resolution: "editor rules the code leg reads"\n' + ' source_commit: "0123456789abcdef0123456789abcdef01234567"\n', + ' - ".cursor"\n', "manifest-submodules"), + # Spellings Git records no path in, so no submodule is ever at one: + (' - path: ".cursor"\n', ' - path: "./.cursor"\n', + "manifest-submodule-path"), + (' - path: ".cursor"\n', ' - path: ".cursor/"\n', + "manifest-submodule-path"), + (' - path: "docs/theme"\n', ' - path: "docs//theme"\n', + "manifest-submodule-path"), + (' - path: "docs/theme"\n', ' - path: "docs/./theme"\n', + "manifest-submodule-path"), + (' - path: ".cursor"\n', ' - path: "./"\n', "manifest-submodule-path"), +], ids=["leg-root", "leg-drop", "absolute-path", "dotdot-path", "empty-path", + "path-twice", "resolution-not-text", "commit-not-hex", + "row-not-a-mapping", "leading-dot-slash", "trailing-slash", + "doubled-slash", "dot-segment", "the-leg-itself"]) +def test_a_submodule_record_row_that_says_nothing_is_a_finding(project, old, + new, code): + assert old in SUBMODULE_RECORD, f"fixture drift: {old!r}" + append_record(project, SUBMODULE_RECORD.replace(old, new, 1)) + result = validate(project) + assert result.returncode == 1, result.stderr + result.stdout + assert code in result.stderr, result.stderr + + +def test_a_submodule_record_that_is_not_a_list_is_a_finding(project): + append_record(project, "\nsubmodules: .cursor\n") + result = validate(project) + assert result.returncode == 1, result.stderr + result.stdout + assert "manifest-submodules" in result.stderr, result.stderr + + +def test_a_recorded_path_git_never_records_says_why(project): + """The finding says what is wrong with the spelling, not only that the + path is not one: `.cursor/` is the submodule at `.cursor` written as no + index ever writes it, so the doctor would never hold that one to it.""" + append_record(project, SUBMODULE_RECORD.replace( + ' - path: ".cursor"\n', ' - path: ".cursor/"\n', 1)) + result = validate(project) + assert result.returncode == 1, result.stderr + result.stdout + assert ("submodules[0]: path is '.cursor/'; Git records no path with a " + "`.` segment, a trailing `/` or a doubled `/`, so no submodule is " + "ever at it") in result.stderr, result.stderr + + +@pytest.mark.parametrize("spelled, path", [ + ('"vendor/a\\\\b"', "vendor/a\\b"), + ('"C:/vendor"', "C:/vendor"), + ('"vendor/caf\u00e9"', "vendor/caf\u00e9"), + ('"vendor/nb\\xa0sp"', "vendor/nb\u00a0sp"), + ('"vendor/my lib"', "vendor/my lib"), +], ids=["backslash", "drive-letter", "non-ascii", "code-point-escape", + "space"]) +def test_a_recorded_path_git_records_is_accepted(project, spelled, path): + """A NAME GIT RECORDS IS A NAME THE RECORD MAY HOLD (#189). + + On POSIX a backslash and a drive letter are characters of a name, which + Git records and an adoption of such a source records too; refusing them + would make `execute`'s own manifest INVALID. The leg-path rule refuses + both because a person TYPES a leg path, for commands Windows also runs, + and this is the other half of that rule only. A no-break space arrives + as the code-point escape `adopt-project.py` writes it with. + """ + assert parse_yaml(f"path: {spelled}\n")["path"] == path + append_record(project, SUBMODULE_RECORD.replace( + ' - path: ".cursor"\n', f" - path: {spelled}\n", 1)) + result = validate(project) + assert result.returncode == 0, result.stderr + result.stdout + assert "manifest ok" in result.stdout + + +@pytest.mark.parametrize("path", [ + "./x", "x/", "a//b", "a/./b", "a/../b", "/x", ".", "x", "vendor/x", + "my lib", "caf\u00e9", + pytest.param("a\\b", marks=POSIX_NAMES), + pytest.param("C:/x", marks=POSIX_NAMES), +]) +def test_the_recorded_path_rule_refuses_what_git_will_not_record(tmp_path, + path): + """`repo_shape.path_spelling_fault` IS GIT'S OWN RULE, asked of Git. + + A path the rule refuses is one `git update-index` refuses to record a + gitlink at, and a path it accepts is one Git records -- so a row the + validator accepts names a place a submodule can be, and a row it refuses + names none. + """ + repo = tmp_path / "r" + repo.mkdir() + git("init", "-q", ".", cwd=repo) + commit = "0123456789abcdef0123456789abcdef01234567" + recorded = git("update-index", "--add", "--cacheinfo", + f"160000,{commit},{path}", cwd=repo, check=False) + assert (path_spelling_fault(path) is None) == (recorded.returncode == 0), ( + path_spelling_fault(path), recorded.stderr) diff --git a/tests/test_repo_hygiene.py b/tests/test_repo_hygiene.py index a281a729..55b65674 100644 --- a/tests/test_repo_hygiene.py +++ b/tests/test_repo_hygiene.py @@ -600,9 +600,22 @@ def test_agents_md_is_short_enough_to_be_read(): branch, and re-run into a fresh `--work-dir`, or none. `tests/test_adopt_late_failure.py` holds the point to it, and `tests/test_adopt_github_exits.py` still holds the two exits after the - push.""" + push. + + 2026-10-12: 482 -> 490 — the adoption's `submodules:` record (#189). + Four lines and a blank one under "Adopting an existing repository" say + that `execute` now writes one more thing into the split: the plan's + answer for each source submodule a leg keeps, under `project.yaml`'s + `submodules:`, which nothing rewrites afterwards. Three lines end + "Checking"'s point 4, and they are the part an assistant would get + wrong: a MISPLACED submodule carries `rule: null` and `confidence: + recorded` because the record judged it and no rule did, so a row with + no rule id is not a broken row; and a submodule moved on purpose stays + MISPLACED against its row, which is the human's to change -- never a + manifest edit an assistant makes to turn the row green, the hand-edited + pin arriving through a new door.""" lines = (REPO / "AGENTS.md").read_text().splitlines() - assert len(lines) <= 482, f"AGENTS.md is {len(lines)} lines; the cap is 482" + assert len(lines) <= 490, f"AGENTS.md is {len(lines)} lines; the cap is 490" def test_claude_md_points_at_agents_md(): diff --git a/tests/test_shape_doctor.py b/tests/test_shape_doctor.py index fc1c74fc..7c38f621 100644 --- a/tests/test_shape_doctor.py +++ b/tests/test_shape_doctor.py @@ -5672,6 +5672,578 @@ def test_a_submodule_at_a_path_the_doctor_never_judges_is_still_named( assert data["submodules"][0]["in_leg"] == "code" +# --- placement: a submodule held to the leg the adoption recorded (#189) ---- +# +# Which leg holds a source submodule is a human's answer in the adoption plan +# (#166), and the plan is committed nowhere. So until `execute` wrote it into +# `project.yaml`, the row could only NOTE a submodule -- kept, or a placement +# it cannot verify -- and a submodule added by hand looked the same as one a +# person placed. With the record, one in the leg recorded is placed and one in +# the other leg is MISPLACED; one no record names is still #186's note. + +#: The source commit a hand-written record names. Any 40 hex: the doctor reads +#: the leg, not the commit. +RECORDED_AT = "0123456789abcdef0123456789abcdef01234567" + + +def record_submodules(project: Path, *rows: tuple) -> None: + """Append `project.yaml`'s `submodules:` block, as `adopt-project.py + execute` writes it, one `(path, leg)` or `(path, leg, resolution)` row + each. + + SPELT OUT HERE, not written by the adoption's own function: what is + under test is that the doctor reads the record a manifest carries, and + `test_an_adopted_submodule_is_recorded_and_placed` below proves the + writer's own output is that record. Written as BYTES, so the file keeps + the LF line endings it was written with on every platform. + """ + lines = ["", "submodules:"] + for path, leg, *why in rows: + lines += [f' - path: "{path}"', f" leg: {leg}"] + if why: + lines.append(f' resolution: "{why[0]}"') + lines.append(f' source_commit: "{RECORDED_AT}"') + manifest = project / "project.yaml" + manifest.write_bytes((manifest.read_text(encoding="utf-8") + + "\n".join(lines) + "\n").encode("utf-8")) + + +def recorded_note(leg: str) -> str: + """What a `recorded` submodule's note says, spelt out (see KEPT_NOTE).""" + return (f"a submodule project.yaml's `submodules:` records in the {leg} " + "leg, where it is (#189)") + + +def audited_legs(row: dict) -> str: + """The `[...]` the row's reason ends with, from the legs it printed.""" + return "; ".join( + f"{leg['path']}: {leg['paths']} path(s) over " + f"{leg['classified']} of {leg['tracked']} tracked file(s)" + for leg in row["detail"]["legs"]) + + +def test_a_submodule_in_the_leg_its_record_names_is_placed(standard, + project): + """THE ANSWER IS CHECKED, AND THE ROW IS `ok`. + + `.cursor` is rooted by a rule for a FILE, and a leg keeps the submodule + there because the root keeps none (#166). Recorded in the code leg, and + in the code leg, it is placed: the row is `ok` and not the note #186 had + to give it, it says the record placed it, and the project's own + validator accepts the record. + """ + stage_gitlink(project / "code", ".cursor") + record_submodules(project, (".cursor", "code", "editor rules")) + result = doctor(standard, project, "--json") + assert result.returncode == 0, result.stdout + result.stderr + assert json.loads(result.stdout)["verdict"] == "COMPLIANT" + rows = rows_of(result) + assert rows["manifest"]["status"] == "ok", rows["manifest"] + row = rows["placement"] + assert row["status"] == "ok", row + assert row["detail"]["misplaced"] == [] + assert row["detail"]["review_required"] == [] + assert row["detail"]["submodules"] == [{ + "path": "code/.cursor", "leg": "code", "path_in_leg": ".cursor", + "placement": "recorded", "rule": "root-assistant-instructions", + "recorded_leg": "code", "resolution": "editor rules", + "note": recorded_note("code")}] + assert row["reason"] == ( + "every tracked file classifies as the leg it is in; 1 submodule(s) " + "in the leg project.yaml's `submodules:` records for them (#189): " + f"code/.cursor [{audited_legs(row)}]"), row["reason"] + assert row["next"] is None, row + + text = doctor(standard, project) + assert text.returncode == 0, text.stdout + text.stderr + assert verdict_line(text).startswith("COMPLIANT (exit 0)"), text.stdout + + +def test_a_submodule_in_the_other_leg_from_its_record_is_misplaced( + standard, project, tmp_path): + """THE RECORD SAYS SPEC AND THE CODE LEG HOLDS IT: MISPLACED (1 path). + + The manifest asserts where it belongs, so it is a path in the wrong leg + like any other: in `misplaced`, counted in the verdict and the leg, named + with the leg recorded, and an entry under `paths:` in the plan that the + adoption's own reader accepts. Nothing else is in the wrong leg here, so + the reason is that one clause and no separator hangs after it. + """ + stage_gitlink(project / "code", ".cursor") + record_submodules(project, (".cursor", "spec", "rules the spec reads")) + out = tmp_path / "placement-plan.yaml" + result = doctor(standard, project, "--json", "--placement-plan", str(out)) + assert result.returncode == 1, result.stdout + result.stderr + assert json.loads(result.stdout)["verdict"] == "MISPLACED (1 path)" + row = rows_of(result)["placement"] + assert row["status"] == "FINDING", row + reason = ("project.yaml's `submodules:` records this submodule in the " + "spec leg (#189): rules the spec reads") + assert row["detail"]["misplaced"] == [{ + "path": "code/.cursor", "leg": "code", "path_in_leg": ".cursor", + "classified_as": "spec", "rule": None, "reason": reason, + "confidence": "recorded", "question": None, "files": 1, "bytes": 0, + "review_required": False, "direction": "spec in the code leg", + "submodule": True}] + [held] = row["detail"]["submodules"] + assert held["placement"] == "misplaced", held + assert held["recorded_leg"] == "spec", held + assert held["note"] == ( + "a submodule project.yaml's `submodules:` records in the spec leg, " + "and it is in the code leg (#189)"), held + code = [leg for leg in row["detail"]["legs"] if leg["role"] == "code"][0] + assert code["misplaced"] == 1, code + assert row["reason"] == ( + "1 submodule(s) sit in a leg other than the one project.yaml's " + "`submodules:` records for them (#189): code/.cursor (recorded in " + f"the spec leg) [{audited_legs(row)}]"), row["reason"] + assert "--placement-plan" in row["next"], row + + adopt = adoption_module(standard) + data = adopt.load_yaml(out) + [entry] = data["paths"] + assert entry == {"path": "code/.cursor", "in_leg": "code", "leg": "spec", + "confidence": "recorded", "rule": None, + "reason": reason, "files": 1, "bytes": 0, + "review_required": False, "resolution": ""}, entry + [listed] = data["submodules"] + assert listed == {"path": "code/.cursor", "in_leg": "code", + "placement": "misplaced", "recorded_leg": "spec", + "rule": "root-assistant-instructions", + "note": held["note"]}, listed + assert "`placement: misplaced`" in out.read_text(encoding="utf-8") + as_plan = dict(data) + as_plan["kind"], as_plan["mode"] = adopt.PLAN_KIND, "in-place" + assert adopt._leg_findings(adopt.Plan(out, as_plan), None) == [] + + text = doctor(standard, project) + assert text.returncode == 1, text.stdout + text.stderr + assert verdict_line(text).startswith("MISPLACED (1 path)"), text.stdout + + +def test_a_submodule_no_record_names_is_still_the_note(standard, project, + tmp_path): + """A SUBMODULE ADDED BY HAND, beside one the adoption recorded. + + `docs/theme` is in no row, so the doctor still cannot tell an answer + from none, and it is #186's `unverifiable` note, word for word, while + `.cursor` beside it is placed. A row with a note in it is not `ok`, and + nothing is MISPLACED. The plan lists both, the recorded one with the leg + recorded and the other without one. + """ + stage_gitlink(project / "code", ".cursor") + stage_gitlink(project / "code", "docs/theme") + record_submodules(project, (".cursor", "code")) + out = tmp_path / "placement-plan.yaml" + result = doctor(standard, project, "--json", "--placement-plan", str(out)) + assert result.returncode == 0, result.stdout + result.stderr + assert json.loads(result.stdout)["verdict"] == "COMPLIANT" + row = rows_of(result)["placement"] + assert row["status"] == "note", row + assert row["detail"]["misplaced"] == [] + by_path = {held["path"]: held for held in row["detail"]["submodules"]} + assert by_path["code/.cursor"]["placement"] == "recorded" + assert by_path["code/.cursor"]["resolution"] is None, ( + "a row with no `resolution:` gives none") + theme = by_path["code/docs/theme"] + assert theme == {"path": "code/docs/theme", "leg": "code", + "path_in_leg": "docs/theme", "placement": "unverifiable", + "rule": "spec-governance", + "note": unverifiable_note("spec-governance", "spec")} + recorded, apart, unverified = row["reason"].partition( + "; 1 submodule(s) at a path a rule assigns") + assert apart, row["reason"] + assert recorded == ( + "nothing is in the wrong leg; 1 submodule(s) in the leg " + "project.yaml's `submodules:` records for them (#189): code/.cursor") + assert "code/docs/theme (rule spec-governance)" in unverified + data = adoption_module(standard).load_yaml(out) + listed = {held["path"]: held for held in data["submodules"]} + assert listed["code/.cursor"]["recorded_leg"] == "code" + assert "recorded_leg" not in listed["code/docs/theme"] + + +def test_a_record_row_the_validator_refuses_decides_nothing(standard, + project): + """`leg: root` IS NO ANSWER FOR A SUBMODULE, and it places nothing. + + The project's own validator names the row, which makes the verdict + INVALID, and the placement row treats `.cursor` as one no record names: + a row nobody can hold a submodule to must not make it look checked. + """ + stage_gitlink(project / "code", ".cursor") + record_submodules(project, (".cursor", "root")) + result = doctor(standard, project, "--json") + assert result.returncode == 1, result.stdout + result.stderr + rows = rows_of(result) + assert rows["manifest"]["status"] == "FINDING", rows["manifest"] + assert "manifest-submodule-leg" in json.dumps(rows["manifest"]) + [held] = rows["placement"]["detail"]["submodules"] + assert held["placement"] == "kept", held + assert "recorded_leg" not in held, held + + +@pytest.mark.parametrize("legs, placement, recorded_leg", [ + (("code", "spec"), "recorded", "code"), + (("spec", "code"), "misplaced", "spec"), + (("root", "code"), "kept", None), +], ids=["code-then-spec", "spec-then-code", "refused-leg-then-code"]) +def test_where_two_rows_name_one_path_the_first_decides(standard, project, + legs, placement, + recorded_leg): + """TWO ROWS FOR ONE PATH: THE FIRST DECIDES, AND THE VERDICT IS RED. + + The project's own validator accepts the first row and names the second + as `recorded twice`, so the doctor holds `.cursor` to the first, as it + ignores every row the validator refuses -- and where it refuses the + first row's `leg: root`, nothing decides, and the second row does not + stand in for it. `execute` writes one row per gitlink, so only an edit + makes two; whichever row decides, the `manifest` row is a FINDING and + the doctor exits 1. + """ + stage_gitlink(project / "code", ".cursor") + record_submodules(project, *((".cursor", leg) for leg in legs)) + named = run_script(project / "scripts" / "validate-manifest.py", + cwd=project) + assert "submodules[1]: path '.cursor' is recorded twice" in ( + named.stderr), named.stderr + result = doctor(standard, project, "--json") + assert result.returncode == 1, result.stdout + result.stderr + rows = rows_of(result) + assert rows["manifest"]["status"] == "FINDING", rows["manifest"] + [held] = rows["placement"]["detail"]["submodules"] + assert held["placement"] == placement, held + assert held.get("recorded_leg") == recorded_leg, held + + +#: A leg's `.gitmodules` registering `.cursor`, as an adoption leaves one. +CURSOR_GITMODULES = ('[submodule ".cursor"]\n\tpath = .cursor\n' + "\turl = https://example.invalid/rules.git\n") + +#: What the row says of a `.gitmodules` whose question the record answered. +ANSWERED_CLAUSE = ("project.yaml's `submodules:` answers the question the " + "policy asks of code/.gitmodules: which leg owns the " + "submodules it registers") + + +@pytest.mark.parametrize("recorded, status, asked", [ + ("code", "ok", []), + ("spec", "FINDING", ["code/.gitmodules"]), + (None, "note", ["code/.gitmodules"]), +], ids=["recorded-in-this-leg", "recorded-in-the-other-leg", "no-record"]) +def test_the_record_answers_the_question_about_the_gitmodules_beside_it( + standard, project, recorded, status, asked): + """THE POLICY ASKS WHICH LEG OWNS THE SUBMODULES A `.gitmodules` + REGISTERS, AND THE RECORD SAYS (#189). + + An adoption's leg always holds the `.gitmodules` that registers its + submodules, and rule `ambiguous-gitmodules` asks that of every one. In a + leg that keeps a submodule the record places there, the record is the + answer, and it is this leg: the file is placed, the row is `ok`, and the + reason says where the answer came from. Where the record puts the + submodule in the other leg, or there is no record, it says nothing about + this file, which stays the question it was. + """ + stage(project / "code", ".gitmodules", CURSOR_GITMODULES) + stage_gitlink(project / "code", ".cursor") + if recorded: + record_submodules(project, (".cursor", recorded)) + result = doctor(standard, project, "--json") + assert result.returncode == (1 if status == "FINDING" else 0), ( + result.stdout + result.stderr) + row = rows_of(result)["placement"] + assert row["status"] == status, row + assert [item["path"] for item in row["detail"]["review_required"]] == ( + asked), row["detail"]["review_required"] + code = [leg for leg in row["detail"]["legs"] if leg["role"] == "code"][0] + if asked: + assert "answered" not in code, code + assert ANSWERED_CLAUSE not in row["reason"], row["reason"] + return + assert code["answered"] == ["code/.gitmodules"], code + assert row["reason"] == ( + "every tracked file classifies as the leg it is in; 1 submodule(s) " + "in the leg project.yaml's `submodules:` records for them (#189): " + f"code/.cursor; {ANSWERED_CLAUSE} [{audited_legs(row)}]"), ( + row["reason"]) + + +@pytest.mark.parametrize("other, rows, status, unread", [ + ("vendor/lib", (), "note", []), + ("docs/theme", (("docs/theme", "spec"),), "FINDING", []), + pytest.param("bad\x01name", (), "note", ["'bad\\x01name'"], + marks=pytest.mark.skipif( + os.name == "nt", reason="Git for Windows records no " + "path that holds a control character")), +], ids=["one-no-row-names", "one-recorded-in-the-other-leg", + "one-whose-name-cannot-be-read"]) +def test_the_question_stays_where_the_record_does_not_place_every_submodule( + standard, project, other, rows, status, unread): + """THE RECORD ANSWERS FOR A `.gitmodules` ONLY WHERE IT PLACES EVERY + SUBMODULE THE LEG KEEPS (#189). + + Beside the recorded `.cursor` the code leg keeps a second submodule, + registered in the same `.gitmodules` as `git submodule add` registers + one: added by hand, which no row names, or one the record puts in the + spec leg. The record does not say this leg owns everything the file + registers, so the file stays the policy's question, and the row never + says the record answered it; the note or the finding about the second + submodule is what it was. + + Or one whose name the doctor cannot read (#189's second review, F1). A + control character, like a byte that is not UTF-8, sets the name aside + before its mode is read, so no row was held to it: the leg reports the + name by `repr`, and the question stays. + """ + stage(project / "code", ".gitmodules", CURSOR_GITMODULES + ( + f'[submodule "{other}"]\n\tpath = {other}\n' + "\turl = https://example.invalid/other.git\n")) + stage_gitlink(project / "code", ".cursor") + stage_gitlink(project / "code", other) + record_submodules(project, (".cursor", "code"), *rows) + result = doctor(standard, project, "--json") + assert result.returncode == (1 if status == "FINDING" else 0), ( + result.stdout + result.stderr) + row = rows_of(result)["placement"] + assert row["status"] == status, row + assert [item["path"] for item in row["detail"]["review_required"]] == [ + "code/.gitmodules"], row["detail"]["review_required"] + code = [leg for leg in row["detail"]["legs"] if leg["role"] == "code"][0] + assert "answered" not in code, code + assert code["unwritable"] == unread, code + assert ANSWERED_CLAUSE not in row["reason"], row["reason"] + + +#: Names the record cannot write raw -- `str.isprintable` refuses a character +#: in each -- and the doctor still reads: none holds a control character or a +#: byte that is not UTF-8, the names it reports as ones no report can carry. +ESCAPED_NAMES = ("vendor/nb\u00a0sp", "vendor/ideo\u3000graphic", + "vendor/zero\u200bwidth", "vendor/soft\u00adhyphen", + "vendor/nel\u0085x", "vendor/line\u2028sep") + + +def held_placements(result) -> dict: + """`{path: placement}` of every submodule the `placement` row holds.""" + return {held["path"]: held["placement"] for held in + rows_of(result)["placement"]["detail"].get("submodules") or []} + + +def append_written_record(standard: Path, project: Path, names, + leg: str = "code") -> str: + """The record the adoption's OWN writer makes for `names` in `leg`, + appended to `project.yaml` as bytes, and returned.""" + adopt = adoption_module(standard) + block = adopt.submodule_record( + [{"path": name, "leg": leg} for name in names], + sorted((name, adopt.GITLINK_MODE, RECORDED_AT, 0) for name in names), + RECORDED_AT) + manifest = project / "project.yaml" + manifest.write_bytes((manifest.read_text(encoding="utf-8") + + block).encode("utf-8")) + return block + + +def test_a_name_the_record_spells_with_an_escape_still_names_its_submodule( + standard, project): + """THE ROW READS BACK AS THE NAME, so the submodule is held to it (#189). + + The adoption's own writer spells each of these names' unprintable + character by its code point -- `\\xa0`, `\\u3000`, `\\u2028` -- and the + reader resolves that escape. So in the leg recorded each submodule is + placed, and staged in the other leg instead each one is MISPLACED. When + the reader kept the backslash, no row named any of them: they were + `kept`, and the move was COMPLIANT. + """ + block = append_written_record(standard, project, ESCAPED_NAMES) + spelled = [line for line in block.splitlines() if "- path:" in line] + assert len(spelled) == len(ESCAPED_NAMES), block + assert all("\\" in line for line in spelled), ( + "each name must be one the record spells with an escape", block) + for name in ESCAPED_NAMES: + stage_gitlink(project / "code", name) + placed = doctor(standard, project, "--json") + assert placed.returncode == 0, placed.stdout + placed.stderr + assert rows_of(placed)["manifest"]["status"] == "ok" + assert held_placements(placed) == { + f"code/{name}": "recorded" for name in ESCAPED_NAMES} + + for name in ESCAPED_NAMES: + git("rm", "-q", "--cached", "--", name, cwd=project / "code") + stage_gitlink(project / "spec", name) + moved = doctor(standard, project, "--json") + assert moved.returncode == 1, moved.stdout + moved.stderr + assert json.loads(moved.stdout)["verdict"] == ( + f"MISPLACED ({len(ESCAPED_NAMES)} paths)") + assert held_placements(moved) == { + f"spec/{name}": "misplaced" for name in ESCAPED_NAMES} + + +@pytest.mark.skipif(os.name == "nt", + reason="Git for Windows records no path with a backslash") +def test_a_recorded_name_with_a_backslash_is_valid_and_placed(standard, + project): + """A BACKSLASH IS A CHARACTER OF THE NAME, and the record may hold it. + + On POSIX Git records `vendor/a\\b`, an adoption of a source with a + submodule there records it, and the doctor holds the gitlink to that + row. So the project's own validator accepts the row: the spelling rule + it holds a recorded path to is the one Git records paths by (#189), and + the leg-path rule's refusal of a backslash, which a person TYPING a path + for Windows gets wrong, is not part of it. + """ + name = "vendor/a\\b" + append_written_record(standard, project, [name]) + stage_gitlink(project / "code", name) + result = doctor(standard, project, "--json") + assert result.returncode == 0, result.stdout + result.stderr + assert rows_of(result)["manifest"]["status"] == "ok", ( + rows_of(result)["manifest"]) + assert held_placements(result) == {f"code/{name}": "recorded"} + + +# --- the record, written by a real adoption and read by the doctor (#189) --- + +#: `execute` REQUIRES `git filter-repo`; see tests/test_adopt_e2e.py. +needs_filter_repo = pytest.mark.skipif( + shutil.which("git-filter-repo") is None, + reason="git filter-repo is not installed: `pip install git-filter-repo`") + +#: Why the adoption below answered `.cursor` and its `.gitmodules` `code`. +CURSOR_ANSWER = "editor rules the code leg's tooling reads" + + +@pytest.fixture(scope="module") +def adopted_cursor(standard, tmp_path_factory) -> dict: + """A source whose `.cursor` is a SUBMODULE, adopted by the standard + under test with `.cursor` and `.gitmodules` answered `code`, and cloned + recursively from the split branch.""" + base = tmp_path_factory.mktemp("adopted-cursor") + dependency = base / "rules" + dependency.mkdir() + git("init", "-q", "-b", "main", ".", cwd=dependency) + (dependency / "rules.md").write_text("editor rules\n", encoding="utf-8") + commit_all(dependency, "rules") + source = base / "Thing" + source.mkdir() + git("init", "-q", "-b", "main", ".", cwd=source) + for name, body in (("README.md", "# Thing\n"), + ("src/app/util.py", "VALUE = 1\n"), + ("docs/index.md", "# Docs\n")): + (source / name).parent.mkdir(parents=True, exist_ok=True) + (source / name).write_text(body, encoding="utf-8") + commit_all(source, "init") + git(*FILE_PROTOCOL, "submodule", "add", "-q", str(dependency), ".cursor", + cwd=source) + commit_all(source, "vendor the editor rules") + adopt = standard / "adopt-project.py" + plan = base / "adoption-plan.yaml" + written = run_script(adopt, "plan", "--source", str(source), "--project", + "Northwind", "--org", ORG, "--elected-by", "T", + "--elected-on", "2026-09-02", "--out", str(plan)) + assert written.returncode == 0, written.stdout + written.stderr + text = plan.read_text(encoding="utf-8") + for path in (".cursor", ".gitmodules"): + needle = f" - path: {path}\n leg: null\n" + assert needle in text, (path, text) + text = text.replace(needle, f" - path: {path}\n leg: code\n" + f" resolution: \"{CURSOR_ANSWER}\"\n") + plan.write_bytes(text.encode("utf-8")) + done = run_script(adopt, "execute", "--plan", str(plan), "--yes", + "--local-remote-dir", str(base / "remotes"), + "--work-dir", str(base / "work")) + assert done.returncode == 0, done.stdout + done.stderr + clone = base / "clone" + git(*FILE_PROTOCOL, "clone", "-q", "--recurse-submodules", "-b", + "adopt/three-repo-shape", str(source), str(clone), cwd=base) + return {"clone": clone, + "commit": git("rev-parse", "main", cwd=source).stdout.strip()} + + +@pytest.fixture +def cursor_clone(adopted_cursor, tmp_path) -> Path: + """A private, mutable copy of that clone.""" + target = tmp_path / "Northwind" + shutil.copytree(adopted_cursor["clone"], target, symlinks=True) + return target + + +def cursor_placement(standard, root: Path) -> tuple: + """The verdict and the `.cursor` submodule's entry, from `--json`.""" + result = doctor(standard, root, "--json") + row = rows_of(result)["placement"] + held = {item["path"]: item for item in row["detail"]["submodules"]} + return result, json.loads(result.stdout)["verdict"], row, held + + +@needs_filter_repo +def test_an_adopted_submodule_is_recorded_and_placed(standard, + adopted_cursor, + cursor_clone): + """`execute` WRITES THE ANSWER, AND THE DOCTOR PLACES THE SUBMODULE BY IT. + + The split's `project.yaml` carries one row for `.cursor`: its path in + the leg, the leg answered, the plan's `resolution:` and the source + commit. The doctor holds the code leg's `.cursor` to it: recorded, and + COMPLIANT. And the row is `ok`: the code leg's `.gitmodules`, which + registers `.cursor`, is the policy's question about which leg owns that + submodule, and the record is its answer. + """ + manifest = adoption_module(standard).load_yaml( + cursor_clone / "project.yaml") + assert manifest["submodules"] == [{ + "path": ".cursor", "leg": "code", "resolution": CURSOR_ANSWER, + "source_commit": adopted_cursor["commit"]}], manifest["submodules"] + result, verdict, row, held = cursor_placement(standard, cursor_clone) + assert result.returncode == 0, result.stdout + result.stderr + assert verdict == "COMPLIANT" + assert rows_of(result)["manifest"]["status"] == "ok" + assert row["status"] == "ok", row + assert row["detail"]["review_required"] == [], row["detail"] + assert held["code/.cursor"]["placement"] == "recorded", held + assert held["code/.cursor"]["resolution"] == CURSOR_ANSWER + assert ("1 submodule(s) in the leg project.yaml's `submodules:` records " + "for them (#189): code/.cursor") in row["reason"], row["reason"] + assert ANSWERED_CLAUSE in row["reason"], row["reason"] + assert "keeps no source submodule" not in row["reason"], row["reason"] + + +@needs_filter_repo +def test_an_adopted_submodule_whose_record_is_edited_is_misplaced( + standard, cursor_clone): + """THE SAME ASSEMBLY, ITS RECORD EDITED TO SAY SPEC: MISPLACED (1 path).""" + manifest = cursor_clone / "project.yaml" + text = manifest.read_text(encoding="utf-8") + assert text.count(" leg: code\n") == 1, text + manifest.write_bytes( + text.replace(" leg: code\n", " leg: spec\n").encode("utf-8")) + result, verdict, row, held = cursor_placement(standard, cursor_clone) + assert result.returncode == 1, result.stdout + result.stderr + assert verdict == "MISPLACED (1 path)" + assert row["status"] == "FINDING", row + assert held["code/.cursor"]["placement"] == "misplaced", held + assert [entry["path"] for entry in row["detail"]["misplaced"]] == [ + "code/.cursor"] + + +@needs_filter_repo +def test_a_submodule_added_by_hand_after_adoption_is_the_note(standard, + cursor_clone): + """A SUBMODULE NOBODY ANSWERED, in the same leg as one somebody did. + + `vendor/lib` is in no row of the record, so it is #186's `kept` note; + `.cursor` is still recorded, and the verdict stays COMPLIANT. + """ + stage_gitlink(cursor_clone / "code", "vendor/lib") + result, verdict, row, held = cursor_placement(standard, cursor_clone) + assert result.returncode == 0, result.stdout + result.stderr + assert verdict == "COMPLIANT" + assert row["status"] == "note", row + assert held["code/.cursor"]["placement"] == "recorded", held + assert held["code/vendor/lib"]["placement"] == "kept", held + assert held["code/vendor/lib"]["note"] == KEPT_NOTE + assert "code/vendor/lib" in row["reason"], row["reason"] + + # --- the platform-aware quoter (#101) --------------------------------------- # # Copilot asked for shell quoting on PR #100, against the placement row. The diff --git a/tests/test_windows_paths.py b/tests/test_windows_paths.py index 7d325b5f..088ed3b0 100644 --- a/tests/test_windows_paths.py +++ b/tests/test_windows_paths.py @@ -371,6 +371,16 @@ def test_a_windows_path_survives_the_plan_it_was_written_into(adopt, tmp_path): (r'"a\\\\b"', "a\\\\b"), # two backslashes (r'"say \"hi\""', 'say "hi"'), (r'"a\qb"', "a\\qb"), # not an escape; the backslash survives + # YAML's code-point escapes, which `adopt-project.py` writes a recorded + # submodule's name with when a character in it is not printable (#189): + (r'"nb\xa0sp"', "nb\xa0sp"), # U+00A0, the character, not the byte + (r'"\u3000\u2028"', "\u3000\u2028"), + (r'"\U0001F600"', "\U0001F600"), + (r'"caf\udce9"', "caf\udce9"), # the surrogate a non-UTF-8 byte is kept as + (r'"\\x41"', "\\x41"), # an escaped backslash, then x41: never `A` + (r'"\x4"', "\\x4"), # too few digits; the backslash survives + (r'"\xzz"', "\\xzz"), # not hex; the backslash survives + (r'"\U00110000"', "\\U00110000"), # past U+10FFFF; so does this one ]) def test_a_double_quoted_scalar_resolves_one_escape_at_a_time( tmp_path, written, read_back):