Skip to content

Commit 421e4e5

Browse files
Brett Heapclaude
andcommitted
Merge origin/main (3ef059c, #314) into fix/adopt-pin-writes-pin-file: plan asks check's manifest question of the plan it is about to write, whose pins: now holds each pin as the mapping plan writes since #273, and #314's key-for-key test pins openGlass@<commit> from a local stand-in, where this branch refuses a name alone
#314 asks the project's YAML reader and `validate-manifest.py` about the manifest's values before anything exists, in both tools. One conflict, `adopt-project.py`, one hunk: this branch added `_refuse_an_unrecordable_pin_source` after `_checked_plan_inputs`, and #314 added its section "The project must be able to hold what `execute` writes into it (#261)" (`MANIFEST_FLAGS`, `_flag_plan`, `_manifest_values`, `_manifest_findings`, `_refuse_unholdable_values`) at the same place. Both are kept, this branch's function first; neither calls the other. `scaffold-project.py`, `scripts/shape_materialize.py` and `docs/cli.md` merged with no conflict (`render-cli-reference.py --check` ok). Where the two meet, as lane 5 composed them before (implT's recipe, `pr314-composition.diff`): - `_flag_plan` built `"pins": list(pins)`. `pins` here is `declared_pin_values`' `{product: PIN_* values}`, so that list held the products, where `plan` writes each pin as a mapping of `PIN_FIELDS`. It now builds those mappings, so the plan it asks about is the plan it writes; `_manifest_values` still reads products through `Plan.pins`. The two annotations say `dict`. - `tests/test_adopt_manifest_values.py::test_plan_asks_what_check_asks_of_the_same_keys` planned `--pin openGlass`, a name alone, which `plan` refuses since #273 as `pin-malformed`. It now plans `--pin openGlass@<commit>` with `--pin-source` naming a stand-in repository the test makes, so it reads no network. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2 parents 4098b61 + 3ef059c commit 421e4e5

6 files changed

Lines changed: 2487 additions & 18 deletions

File tree

‎adopt-project.py‎

Lines changed: 247 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,17 @@
6262
relative url resolved again from the leg remotes it is about to create, which
6363
under `--local-remote-dir` are not the GitHub ones `check` resolves from.
6464
65+
THE NEW PROJECT MUST BE ABLE TO HOLD WHAT `execute` WRITES INTO IT (#261).
66+
`execute` puts the plan's values into the root's `project.yaml`, and the
67+
project's own `scripts/validate-manifest.py` judges them at its first
68+
bootstrap. So `plan` refuses, `check` finds and `execute` refuses a value that
69+
validator would refuse -- a blank `elected_by` or `reference`, an `elected_on`
70+
that is not `YYYY-MM-DD`, an organisation name it cannot hold, a `visibility`
71+
other than `private`, `public` or `internal` -- asking the validator itself,
72+
loaded from the template it ships, and not a copy of its rules. A name that
73+
cannot be a directory's (a `--project` that makes a repository name of 256
74+
bytes with its `.git`) is refused the same way.
75+
6576
WHY `git filter-repo` AND NOT A VENDORED COPY. Extracting history correctly is
6677
a solved problem with one correct implementation, and a vendored copy of it
6778
would be a second implementation that drifts. It is a hard REQUIREMENT here,
@@ -147,6 +158,13 @@
147158
CASE_ASIDE as _CASE_ASIDE, MOVE_THE_LEG, first_and_more as _first_and_more,
148159
path_overlap as _overlap, shape_path_collisions,
149160
)
161+
# What the project's own `validate-manifest.py` would say of the values written
162+
# into its manifest, and whether a repository's name can be a directory's:
163+
# asked of the plan's values by `plan`, `check` and `execute` (#261).
164+
from shape_materialize import ( # noqa: E402
165+
MANIFEST_INVALID_REMEDIATION, manifest_value_problems,
166+
misread_placeholders, refuse_manifest_values, refuse_unstorable_names,
167+
)
150168
# Whether a value this tool writes into a file is text a file can hold
151169
# (#255), asked beside `one_printable_line`, the trailer's question.
152170
from shape_materialize import unwritable_reason # noqa: E402
@@ -2064,6 +2082,187 @@ def _refuse_an_unrecordable_pin_source(source: Path, products) -> None:
20642082
"as it stands.")
20652083

20662084

2085+
# ---------------------------------------------------------------------------
2086+
# The project must be able to hold what `execute` writes into it (#261)
2087+
# ---------------------------------------------------------------------------
2088+
#
2089+
# `execute` puts the plan's values into the assembly root's `project.yaml`, and
2090+
# the project's own `scripts/validate-manifest.py` then judges them at its
2091+
# first bootstrap. Nothing here asked it first, so a blank `--elected-by` or
2092+
# `--reference`, `--elected-on 2026-W41-6` or `20261011` (which
2093+
# `date.fromisoformat` reads from Python 3.11), `--org test+org`, and a plan
2094+
# edited to `visibility: secret` or `elected_by: null`, each ended
2095+
# `adoption verified` over a project whose own gate was red, with both legs
2096+
# already pushed; the way back was an edit to `project.yaml`, which AGENTS.md
2097+
# tells an agent never to make to turn a row green.
2098+
#
2099+
# `plan` (a flag), `check` (a FINDING) and `execute` (a refusal, through
2100+
# `_entry_findings`, before any leg exists) ask ONE question of the values: the
2101+
# `project.yaml` `execute` would write from them, put through the validator the
2102+
# template ships, through `shape_materialize.manifest_value_problems`, which
2103+
# loads that validator rather than copying a rule of it. The table is built by
2104+
# `_template_table`, which `execute`'s `_template_values` also calls, so it is
2105+
# the one `execute` renders from. Only the validator's question is asked here:
2106+
# whether the project's YAML reader reads a value back as the text it is --
2107+
# `--id true`, `--tracking-branch 007` -- is #181's, and the scaffold asks
2108+
# both.
2109+
2110+
#: How `plan` names each value the manifest records: the flag a person typed.
2111+
MANIFEST_FLAGS = {
2112+
"PROJECT_ID": "--id", "ELECTED_BY": "--elected-by",
2113+
"ELECTED_ON": "--elected-on", "REFERENCE": "--reference",
2114+
"VISIBILITY": "--visibility", "TRACKING_BRANCH": "--tracking-branch",
2115+
"ORG": "--org",
2116+
}
2117+
2118+
#: The validator's findings about a leg's mount path, which `check` already
2119+
#: reports as `plan-bad-leg-path` and `plan-leg-path-collides` from the same
2120+
#: list (`_leg_findings`): said once, in the words of the rule that judges it.
2121+
LEG_PATH_CODES = ("manifest-leg-path", "manifest-leg-path-collision")
2122+
2123+
#: And how `check` and `execute` name the same value: its key in the plan.
2124+
MANIFEST_PLAN_KEYS = {
2125+
"PROJECT_ID": "id", "ELECTED_BY": "elected_by",
2126+
"ELECTED_ON": "elected_on", "REFERENCE": "reference",
2127+
"VISIBILITY": "visibility", "TRACKING_BRANCH": "tracking_branch",
2128+
"ORG": "org",
2129+
}
2130+
2131+
2132+
def _flag_plan(args, names: dict, pins: dict) -> "Plan":
2133+
"""The plan `plan` is about to write, as `Plan` reads one: the values it
2134+
records, under the keys `render_plan` writes them to. `pins` is
2135+
`declared_pin_values`' `{product: PIN_* values}`, recorded as `plan`
2136+
writes each pin since #273: one mapping of `PIN_FIELDS`.
2137+
2138+
Built from the values and not read back from the file, which lists every
2139+
path of the source: `plan` asks what `check` will ask of the same keys,
2140+
before a file exists to read. `tests/test_adopt_manifest_values.py` holds
2141+
the two to the same keys.
2142+
"""
2143+
return Plan(Path(args.out), {
2144+
"kind": PLAN_KIND, "mode": "in-place", "project": args.project,
2145+
"org": args.org, "id": args.id, "visibility": args.visibility,
2146+
"tracking_branch": args.tracking_branch,
2147+
"elected_by": args.elected_by, "elected_on": args.elected_on,
2148+
"reference": args.reference,
2149+
"legs": {**names, "spec_path": args.spec_path,
2150+
"code_path": args.code_path},
2151+
"pins": [{key: values[placeholder] for key, placeholder in PIN_FIELDS}
2152+
for values in pins.values()]})
2153+
2154+
2155+
#: The commit and the tree digest the manifest rendered FOR VALIDATION holds
2156+
#: in `shape.commit` and `shape.digests.tree_sha256`, in place of the real
2157+
#: ones `execute` reads and writes (`_template_values`). The validator the
2158+
#: template ships holds each to its FORM (`COMMIT_RE`, `SHA256_RE`) and
2159+
#: compares neither with anything, and no tool names either to a person, so
2160+
#: reading the real ones (`git rev-parse HEAD`, then `git ls-tree -r` of the
2161+
#: whole standard) bought `plan` and `check` nothing, and made them skip the
2162+
#: question silently from a tree with no git revision. Lowercase hex with a
2163+
#: letter in it, so that the project's YAML reader reads each as text and never
2164+
#: as a number (forty digits would be an integer), quoted or not.
2165+
_VALIDATION_SHAPE_COMMIT = "0123456789abcdef0123456789abcdef01234567"
2166+
_VALIDATION_SHAPE_DIGEST = ("0123456789abcdef0123456789abcdef"
2167+
"0123456789abcdef0123456789abcdef")
2168+
2169+
2170+
def _manifest_values(plan: Plan, naming: NamingPolicy) -> dict | None:
2171+
"""The substitution table `execute` renders the manifest from, as it
2172+
would build it from this plan, FOR VALIDATION AND NEVER FOR WRITING; None
2173+
where the plan's names are not ones the naming policy classifies, which
2174+
`check` and `execute` report in their own words (`_check_names`) and the
2175+
manifest cannot be rendered without.
2176+
2177+
`_template_table` is asked, not copied, and `execute`'s `_template_values`
2178+
is that same table with the real commit and digest, so the table cannot
2179+
differ from `execute`'s. The four leg commits and digests are not known
2180+
before the legs exist and are not in the manifest; the standard's own
2181+
commit and tree digest are replaced by the fixed pair above, so that no
2182+
`git` runs here and a checkout that cannot name its revision is asked the
2183+
same question as any other. A refusal of the plan's own names, id,
2184+
organisation or leg paths is raised here as it is by `check` and `execute`
2185+
a few lines later.
2186+
"""
2187+
if not isinstance(plan.legs, dict):
2188+
# The doctor's PLACEMENT plan writes `legs:` as a list (see
2189+
# `_plan_leg_path`): it names no repositories, so there is no
2190+
# manifest to render from it.
2191+
return None
2192+
names = plan.names()
2193+
pins = set(plan.pins)
2194+
try:
2195+
_check_names(naming, names, pins)
2196+
except Refusal:
2197+
return None
2198+
org = checked_value("org", plan.get("org"))
2199+
repositories = {role: f"{org}/{name}" for role, name in names.items()}
2200+
(_, spec_path), (_, code_path) = _plan_leg_paths(plan)
2201+
return _template_table(
2202+
plan, names, repositories, {"assembly": ""}, str(spec_path),
2203+
str(code_path), str(plan.get("tracking_branch", "main")), {}, {},
2204+
_VALIDATION_SHAPE_COMMIT, _VALIDATION_SHAPE_DIGEST, pins, naming)
2205+
2206+
2207+
def _manifest_findings(plan: Plan, misreads_elsewhere: bool = False
2208+
) -> tuple[list[str], list[str]]:
2209+
"""What the project's own validator would refuse of this plan's values,
2210+
as `check` prints them (one FINDING each), and what it prints once after
2211+
them. `[]` and `[]` when it would pass, or when the plan's names are
2212+
refused in their own words (`_manifest_values`).
2213+
2214+
`misreads_elsewhere` is for a caller whose list ALSO holds a misread
2215+
rule's finding for each value the project's reader reads back as another
2216+
type (`elected_on: "20261011"`, `visibility: "true"`, `id: "007"`: text in
2217+
the plan, a number or a boolean to the reader once the template writes it
2218+
bare), so that such a value is judged once and not twice. It is False
2219+
wherever nothing else asks that question, so that this rule alone still
2220+
refuses the value, with the validator's finding as the only one. Who is
2221+
left out is the reader's answer (`shape_materialize.manifest_misreads`,
2222+
the rule the scaffold applies) and not a list.
2223+
"""
2224+
naming = NamingPolicy.load(NAMING_POLICY)
2225+
values = _manifest_values(plan, naming)
2226+
if values is None:
2227+
return [], []
2228+
# A value the plan's reader read as a number, a boolean or a collection is
2229+
# a MISREAD (`id: 007` is 7), and what the template would then write is
2230+
# `str` of what was read, which the plan never said: it is not asked about
2231+
# here, in words that would echo that text.
2232+
not_text = tuple(
2233+
placeholder for placeholder, key in MANIFEST_PLAN_KEYS.items()
2234+
if plan.get(key) is not None
2235+
and not isinstance(plan.get(key), str))
2236+
if misreads_elsewhere:
2237+
not_text += misread_placeholders(values, MANIFEST_PLAN_KEYS)
2238+
problems = manifest_value_problems(values, naming, MANIFEST_PLAN_KEYS,
2239+
misreads=False, ignore=LEG_PATH_CODES,
2240+
not_text=not_text)
2241+
if not problems:
2242+
return [], []
2243+
return ([f"FINDING plan-manifest-invalid: {sentence}"
2244+
for _, sentence in problems],
2245+
[f"{MANIFEST_INVALID_REMEDIATION} `execute` writes the plan's "
2246+
"values into the project's `project.yaml` as they stand, so "
2247+
"correct each one named here in the plan, or re-run `plan` with "
2248+
"a value the validator accepts."])
2249+
2250+
2251+
def _refuse_unholdable_values(args, names: dict, pins: dict,
2252+
naming: NamingPolicy) -> None:
2253+
"""`plan` refuses, before it writes a file, what `check` would find in the
2254+
plan it is about to write (#261): the three repository names it derives,
2255+
held to `checked_value` as `check` holds `legs:` (a `--project` of 251 to
2256+
255 characters planned, then `check` refused `legs.spec`), and the
2257+
manifest's values, through the one question `check` asks of them."""
2258+
plan = _flag_plan(args, names, pins)
2259+
values = _manifest_values(plan, naming)
2260+
if values is not None:
2261+
refuse_manifest_values(
2262+
"adopt", values, naming, MANIFEST_FLAGS,
2263+
"No plan was written.", misreads=False)
2264+
2265+
20672266
def _print_triad_advisory(path: Path, commit: str | None = None) -> None:
20682267
"""The Triad advisory, once, about the repository this run reads.
20692268
@@ -2161,6 +2360,7 @@ def cmd_plan(args) -> int:
21612360
naming = NamingPolicy.load(NAMING_POLICY)
21622361

21632362
names, pins = _checked_plan_inputs(args, source, naming)
2363+
_refuse_unholdable_values(args, names, pins, naming)
21642364

21652365
tree = source.tree()
21662366
# WHAT THE RULES IMPLY is read from `walk`'s entries, BEFORE
@@ -3523,8 +3723,10 @@ def _entry_findings(plan: Plan, source: Source,
35233723
each entry's path, then coverage, then each entry's `leg:`, the two
35243724
mount paths and what the root keeps where the shape writes, then the
35253725
source's own submodules, then each value `execute` writes into a file
3526-
that no file can hold (#255), and last each relative url of a kept
3527-
submodule that names another repository from its leg (#190).
3726+
that no file can hold (#255), then what the project's own
3727+
`validate-manifest.py` would refuse of the values `execute` writes into
3728+
its manifest (#261), and last each relative url of a kept submodule
3729+
that names another repository from its leg (#190).
35283730
35293731
ONE LIST, read by `check` and by `execute`'s `_refuse_what_check_finds`.
35303732
#168 and #166 each added findings to `check` and a refusal to `execute`
@@ -3545,6 +3747,16 @@ def _entry_findings(plan: Plan, source: Source,
35453747
THE PATHS COME FIRST (#196), so that `_refuse_what_check_finds` wears
35463748
`plan-unsafe-path` for a plan that has one: a path that is a `git` option
35473749
is refused as that, and never as the uncovered file it also leaves.
3750+
3751+
A VALUE NO FILE CAN HOLD IS SAID BEFORE THE VALIDATOR'S WORD ON IT (#255
3752+
before #261). A lone surrogate, a line break or a bidi control in
3753+
`visibility:` or `elected_on:` is both, and `_refuse_what_check_finds`
3754+
then wears `plan-unwritable-value`, as `plan` refuses such a flag as
3755+
`adopt-unwritable-value` before it asks the validator anything. What the
3756+
validator says is said of the manifest the value would be written into,
3757+
which is the value's consequence; the byte, the break or the control is
3758+
what the person retypes. Both are listed, and both refuse before any leg
3759+
exists. The relative urls of #190 still come after both, last.
35483760
"""
35493761
tree = source.tree()
35503762
unsafe, unsafe_remediation = _unsafe_path_findings(plan)
@@ -3556,9 +3768,10 @@ def _entry_findings(plan: Plan, source: Source,
35563768
(split, split_remediation), (moved, moved_remediation), notes = \
35573769
_submodule_findings(plan, source, tree, local_remote_dir)
35583770
unwritable_findings, unwritable_remediation = _value_findings(plan)
3559-
return (findings + split + unwritable_findings + moved,
3771+
value_findings, value_remediation = _manifest_findings(plan)
3772+
return (findings + split + unwritable_findings + value_findings + moved,
35603773
unsafe_remediation + split_remediation + unwritable_remediation
3561-
+ moved_remediation, notes)
3774+
+ value_remediation + moved_remediation, notes)
35623775

35633776

35643777
def _topics_line(topic: str, local: bool) -> str:
@@ -4130,7 +4343,16 @@ def _repository_urls(args, plan: Plan, names: dict,
41304343
"""
41314344
org = checked_value("org", plan.get("org"))
41324345
repositories = {role: f"{org}/{name}" for role, name in names.items()}
4133-
return (args.local_remote_dir is not None, repositories,
4346+
local = args.local_remote_dir is not None
4347+
# Each name is a directory's too: the work directory's for all three, and
4348+
# under `--local-remote-dir` a bare `<name>.git` for the two new legs. A
4349+
# name of 247 to 250 characters passed every check and died in
4350+
# `Path.exists` with `File name too long`, a traceback, before any leg
4351+
# existed (#261).
4352+
refuse_unstorable_names(
4353+
"adopt-name-too-long", list(names.values()),
4354+
[names["spec"], names["code"]] if local else [])
4355+
return (local, repositories,
41344356
_remote_urls(org, names, args.local_remote_dir, source))
41354357

41364358

@@ -4793,6 +5015,22 @@ def _refuse_an_unwritable_clone_url(url: str) -> None:
47935015
def _template_values(plan: Plan, names, repositories, urls, spec_path,
47945016
code_path, tracking, leg_commits, leg_digests,
47955017
shape_commit, pins, naming) -> dict[str, str]:
5018+
"""The substitution table the new project's files are WRITTEN from: the
5019+
standard's real `shape_commit` and the digest of its tree, read here.
5020+
`execute` is the one caller. A table to ask a question about and not to
5021+
write is `_template_table`'s, which reads neither (`_manifest_values`)."""
5022+
return _template_table(plan, names, repositories, urls, spec_path,
5023+
code_path, tracking, leg_commits, leg_digests,
5024+
shape_commit, tree_digest(SHAPE_ROOT, shape_commit),
5025+
pins, naming)
5026+
5027+
5028+
def _template_table(plan: Plan, names, repositories, urls, spec_path,
5029+
code_path, tracking, leg_commits, leg_digests,
5030+
shape_commit, shape_digest, pins, naming
5031+
) -> dict[str, str]:
5032+
"""The substitution table, from the plan and the commit and digest it is
5033+
handed, running nothing: no `git`, no `gh`."""
47965034
project = names["assembly"]
47975035
project_id = plan.project_id
47985036
elected_on = str(plan.get("elected_on", _dt.date.today().isoformat()))
@@ -4824,7 +5062,7 @@ def _template_values(plan: Plan, names, repositories, urls, spec_path,
48245062
"CODE_REPOSITORY": repositories["code"],
48255063
"SHAPE_REPOSITORY": SHAPE_REPOSITORY,
48265064
"SHAPE_COMMIT": shape_commit,
4827-
"SHAPE_TREE_SHA256": tree_digest(SHAPE_ROOT, shape_commit),
5065+
"SHAPE_TREE_SHA256": shape_digest,
48285066
"DIGEST_DEFINITION": TREE_DIGEST_DEFINITION,
48295067
"CLONE_URL": urls["assembly"],
48305068
"ASSEMBLY_CLONE_URL": urls["assembly"],
@@ -5423,7 +5661,9 @@ def main(argv: list[str] | None = None) -> int:
54235661
plan.add_argument("--id", default=None)
54245662
plan.add_argument("--visibility", choices=VISIBILITY_CHOICES,
54255663
default="private")
5426-
plan.add_argument("--elected-by", default=None)
5664+
plan.add_argument("--elected-by", default=None,
5665+
help="the human electing the shape (default: `git "
5666+
"config user.name`); not blank")
54275667
plan.add_argument("--elected-on", default=None, help="YYYY-MM-DD")
54285668
plan.add_argument("--reference", default=None,
54295669
help="the document the election followed. Default: "

‎docs/cli.md‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,9 @@ options:
246246
-h, --help show this help message and exit
247247
--org ORG
248248
--project PROJECT the assembly-root name, e.g. Atlas
249-
--id ID lowercase project id
249+
--id ID lowercase project id (default: --project lowercased).
250+
One the project's YAML reader would read as another
251+
type -- `null`, `true`, `007` -- is refused.
250252
--name NAME display name
251253
--visibility {private,public,internal}
252254
default: private for a fresh scaffold; with --reuse-
@@ -262,6 +264,9 @@ options:
262264
(owner/openProduct@<sha>) to override it for one pin
263265
without changing this default for the others.
264266
--elected-by ELECTED_BY
267+
the human electing the shape (default:
268+
GIT_AUTHOR_NAME, then `git config user.name`); not
269+
blank
265270
--elected-on ELECTED_ON
266271
YYYY-MM-DD
267272
--reference REFERENCE
@@ -273,6 +278,10 @@ options:
273278
does not claim it followed a document that did not
274279
exist yet.
275280
--tracking-branch TRACKING_BRANCH
281+
the branch every repository is created on (default:
282+
main). One the project's YAML reader would read as
283+
another type -- `null`, `true`, `007`, `2026` -- is
284+
refused.
276285
--spec-path SPEC_PATH
277286
where the spec leg is mounted, as a path in the
278287
assembly root (default: spec). A path that is, holds
@@ -367,6 +376,8 @@ options:
367376
--id ID
368377
--visibility {private,public,internal}
369378
--elected-by ELECTED_BY
379+
the human electing the shape (default: `git config
380+
user.name`); not blank
370381
--elected-on ELECTED_ON
371382
YYYY-MM-DD
372383
--reference REFERENCE

0 commit comments

Comments
 (0)