Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 62 additions & 10 deletions scripts/check-chart-freeze.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@
MeshWeaver#7127) and may do nothing else to them. A file the diff adds, modifies, retypes, copies or
renames INTO a frozen path is refused, naming the file and where the chart lives now. A frozen
DIRECTORY matches its own path as well as everything below it, so replacing it with a symlink (its
files deleted, the directory path added) is refused too.
files deleted, the directory path added) is refused too — and so is a non-deletion AT any directory
ABOVE a frozen path (MeshWeaver#7391), which would replace the whole frozen subtree the same way.
A saved listing is read strictly: the whole status token, the number of path fields it needs and
their non-emptiness are validated before a record counts.

The build-host deployment followed (MeshWeaver.Deploy#5): its runner `.github/scripts/build-run.py`,
`deploy/build-host/**`, the announcement script, the declaration `.github/build-hosts.json` and the
Expand Down Expand Up @@ -40,6 +43,7 @@
from __future__ import annotations

import argparse
import re
import subprocess
import sys

Expand Down Expand Up @@ -82,8 +86,21 @@
"deploy/aks/INSTANCE-LIFECYCLE.md",
)

# The status letters `git diff --name-status` prints (C and R do not occur with --no-renames).
KNOWN_STATUS = set("ADMTUXBCR")
# The status TOKENS `git diff --name-status` prints (C and R do not occur with --no-renames). The
# WHOLE token is validated, not its first letter (MeshWeaver#7391): `Dgarbage` once read as a
# deletion and passed. A copy or rename carries a similarity score (`R100`); a modification may
# carry a dissimilarity score under `-B`. No other status is scored — a scored deletion (`D100`)
# is not something git prints, and reading it as a deletion would exempt it — and a score is a
# percentage, so it never exceeds 100.
STATUS_RE = re.compile(r"(?:[ADTUXB]|M(?P<m>\d{1,3})?|[CR](?P<cr>\d{1,3}))")


def _valid_status(status: str) -> bool:
m = STATUS_RE.fullmatch(status)
if not m:
return False
score = m.group("m") or m.group("cr")
return score is None or int(score) <= 100


def _matches(path: str, prefixes: tuple[str, ...]) -> bool:
Expand All @@ -97,27 +114,40 @@ def is_frozen(path: str) -> bool:
return _matches(path, FROZEN)


def is_frozen_ancestor(path: str) -> bool:
"""`path` is a directory ABOVE a frozen path (MeshWeaver#7391). A name-status listing names
files, so a non-deletion AT a directory's own path is a file or symlink replacing that whole
directory: moving `deploy/aks` aside and adding the symlink `deploy/aks -> ../editable-aks`
lists the frozen children as deletions and an addition at `deploy/aks`, which no frozen prefix
matches — and re-opens every frozen path below it for editing through the link."""
prefix = path.rstrip("/") + "/"
return bool(path) and any(f.startswith(prefix) for f in FROZEN)


def violations(listing: str) -> list[tuple[str, str]]:
"""Every (status, path) in a NUL-delimited name-status listing that touches a frozen path with
anything but a deletion. The listing is a flat sequence of fields: status, path, status, path, …
A field this gate cannot read — an unknown status, or a status with no path after it — is itself
a violation: a change it could not read is a change it did not check."""
A field this gate cannot read — an unknown or malformed status token, a status with fewer path
fields than it needs, or an empty path — is itself a violation: a change it could not read is a
change it did not check."""
out: list[tuple[str, str]] = []
fields = listing.split("\0")
if fields and fields[-1] == "":
fields.pop()
i = 0
while i < len(fields):
status = fields[i]
if not status or status[0] not in KNOWN_STATUS or i + 1 >= len(fields):
# A C or R entry carries two paths; --no-renames should never produce one, but read both.
width = 3 if status[:1] in ("C", "R") else 2
paths = fields[i + 1:i + width]
if (not _valid_status(status) or len(paths) != width - 1
or any(not p for p in paths)):
# Past an unreadable field the status/path pairing is lost, so nothing after it can be
# read either: one violation names where reading stopped.
out.append(("?", status))
break
# A C or R entry carries two paths; --no-renames should never produce one, but read both.
width = 3 if status[0] in "CR" else 2
for path in fields[i + 1:i + width]:
if is_frozen(path) and status[0] != "D":
for path in paths:
if status[0] != "D" and (is_frozen(path) or is_frozen_ancestor(path)):
out.append((status[0], path))
i += width
return out
Expand Down Expand Up @@ -201,6 +231,28 @@ def self_test() -> int:
"M\0", 1),
("an empty listing is no change",
"", 0),
# #7391: the ancestor replacement and the strict record reader.
("a frozen path's ANCESTOR replaced by a symlink is refused (deploy/aks -> elsewhere)",
_z("D\tdeploy/aks/values.aks.yaml\nD\tdeploy/aks/manifests/x.yaml\nA\tdeploy/aks\n"
"A\teditable-aks/values.aks.yaml"), 1),
("…at any depth (a file replacing .github/scripts)",
_z("A\t.github/scripts"), 1),
("…but deleting at an ancestor path is allowed, and a sibling of it is not an ancestor",
_z("D\tdeploy/aks\nM\tdeploy/aksx\nM\tdeploy/aks-notes.md"), 0),
("a rename with a MISSING destination is a violation, never a silent pass",
"R100\0docs/x.yaml\0", 1),
("a status token with trailing garbage is not a deletion",
"Dgarbage\0deploy/helm/values.yaml\0", 1),
("an EMPTY path field is a violation",
"M\0\0", 1),
("a scored modification (-B) still reads",
"M073\0deploy/helm/values.yaml\0", 1),
("a SCORED deletion is not a deletion (git never scores one)",
"D100\0deploy/helm/values.yaml\0", 1),
("a score above 100 is malformed, never a silent pass",
"R101\0docs/x.yaml\0docs/y.yaml\0", 1),
("…and a rename scored at the 100 bound between unfrozen paths stays clean",
"R100\0docs/x.yaml\0docs/y.yaml\0", 0),
]
failed = 0
for name, text, expected in cases:
Expand Down
56 changes: 50 additions & 6 deletions scripts/check-cross-repo-pair.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,11 @@
It consumes the surface-removal REPORT that `check-type-forwards.py --surface-json` already
computes — one detector, proven by one set of self-tests, never a second one — and:

* removals empty -> PASS. Measured on this repo: `main~25 -> main` removes ZERO public types,
so an ordinary pull request never meets this gate at all. `main~100 -> main` removes 116, all
* removals empty -> PASS, UNLESS the body DECLARES a counterpart (MeshWeaver#7397): a
declared `Pairs-with:` is resolved whatever the detector found, because the author is asserting
a coupling the detector cannot see — a PackageReference or PackageVersion leaving core. Measured
on this repo: `main~25 -> main` removes ZERO public types, so an ordinary pull request (which
declares nothing) never meets this gate at all. `main~100 -> main` removes 116, all
of them the Maps/Indexing carve-out (#2941) — the exact wave that produced #2689's incident
shapes 4 and 5.
* removals non-empty -> the pull-request body MUST carry a `Pairs-with:` declaration, and every
Expand Down Expand Up @@ -445,10 +448,21 @@ def evaluate(surface: dict, body: str, fleet: set[str], resolve,
f"Base tree holds {projects_at_base} `{PROJECT_SUFFIX}` file(s); "
f"this diff deletes or renames {len(removed_projects)} of them."
)
declarations = parse_declarations(body)
if not removed and not removed_projects:
out.append("No public type or member left src/ and no project path moved in this diff — "
"no cross-repo pair to gate.")
return 0, out
# 🚨 A DECLARED pair is resolved even when the detector found nothing (MeshWeaver#7397).
# The detector sees public types, public members and project paths — not a
# PackageReference or PackageVersion leaving core, which a satellite can depend on just as
# hard (#7205 dropped YamlDotNet; #7179 dropped pins a satellite read versionless). An
# author who writes `Pairs-with:` is asserting a coupling the detector cannot see, and a
# gate that returned before reading it held nothing while the body claimed it did.
if not any(isinstance(d, (PairRef, Unparseable)) for d in declarations):
out.append("No public type or member left src/ and no project path moved in this "
"diff — no cross-repo pair to gate.")
return 0, out
out.append("No public type or member left src/ and no project path moved, but the body "
"DECLARES a counterpart — resolving it, because the declaration is the author "
"asserting a coupling the detector cannot see (a dependency or pin removal).")

for entry in removed[:40]:
out.append(f" [{entry.get('category')}] {entry.get('assembly')} :: {entry.get('fullName')}")
Expand All @@ -468,7 +482,6 @@ def evaluate(surface: dict, body: str, fleet: set[str], resolve,
f"a counterpart may live in: {', '.join(sorted(fleet))}."
)

declarations = parse_declarations(body)
bad = [d for d in declarations if isinstance(d, Unparseable)]
if bad:
for d in bad:
Expand Down Expand Up @@ -912,6 +925,37 @@ def resolve(repo: str, number: int) -> dict:
"", _resolver(_pr()), {"projectsAtBase": 2, "removed": []}, _FLEET, False,
("below the floor", "green on zero"),
),
(
# 🚨 THE #7397 SHAPE. #7205 dropped a package dependency from core (no public type, no
# project path) and DECLARED its satellite counterpart; before the fix this row PASSED
# with "no cross-repo pair to gate" while the counterpart was still open.
"a DECLARED counterpart is resolved though the detector found no removal — OPEN holds it",
{"publicTypesAtBase": _HEALTHY, "removed": []},
"Drops a package dependency.\n\nPairs-with: Systemorph/MeshWeaver.Plugins#4227\n",
_resolver(_pr(merged=False, state="open")), _NO_PROJECTS, _FLEET, False,
("DECLARES a counterpart", "still OPEN", "must land LAST"),
),
(
"…and once that declared counterpart has merged, the change may follow",
{"publicTypesAtBase": _HEALTHY, "removed": []},
"Pairs-with: Systemorph/MeshWeaver.Plugins#4227\n",
_resolver(_pr()), _NO_PROJECTS, _FLEET, True,
("ok Systemorph/MeshWeaver.Plugins#4227", "may follow"),
),
(
"…a malformed declaration on a no-removal diff FAILS rather than reading as none",
{"publicTypesAtBase": _HEALTHY, "removed": []},
"Pairs-with: MeshWeaver.Plugins 4227\n",
_resolver(_pr()), _NO_PROJECTS, _FLEET, False,
("Unparseable declaration",),
),
(
"…and `Pairs-with: none` on a no-removal diff still passes",
{"publicTypesAtBase": _HEALTHY, "removed": []},
"Pairs-with: none — nothing outside core consumed the dropped package\n",
_raising_resolver("must not be called"), _NO_PROJECTS, _FLEET, True,
("no cross-repo pair to gate",),
),
(
"an ordinary diff that moves no project still never meets the gate",
{"publicTypesAtBase": _HEALTHY, "removed": []},
Expand Down
162 changes: 161 additions & 1 deletion scripts/check-record-signatures.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,120 @@ def _strip_bom(text: str) -> str:
return text.lstrip("\ufeff")


# 🚨 A COMMENT is not a parameter (MeshWeaver#7366). `split_params` splits on top-level commas
# and `parse_param` takes the last token as the name, so a `//` line between two parameters
# whose prose holds a comma became a PHANTOM parameter, and a comment ending in an identifier
# was swallowed into the next parameter's TYPE. Measured on core#5077: `UserOnboardingRequest`
# read as 8 -> 10 parameters for a real 7 -> 8, with five lines of prose printed as the type of
# `Locale`. Rewording such a comment then read as a type change on a record whose signature did
# not move — and, worse, a comment GAINING a comma while a real parameter left kept the count
# equal. So comments go before anything is counted, on BOTH sides of the comparison, because both
# pass through here.
#
# String and char literals are copied verbatim (a `//` inside `"https://…"` is not a comment);
# `@"…"` verbatim strings use `""` as their only escape, and a raw literal opened by N >= 3
# quotes runs to the next N quotes (a four-quote literal may hold `"""`). An interpolated `$"…"`
# string's `{…}` holes are scanned as CODE, so a nested literal such as
# `$"{string.Concat("/*", "x")}"` neither ends the outer string nor opens a comment. A literal
# the scanner mis-bounds is not harmless: text it leaves outside a string can open a phantom
# `/*` that swallows every record after it, and an arity change there would pass. Newlines inside
# a block comment are kept so nothing downstream sees two tokens fused into one.
def _skip_hole(text: str, j: int) -> int:
"""`j` is just past an interpolation hole's `{`; returns the index just past its `}`."""
n, depth = len(text), 0
while j < n:
c = text[j]
if c in "\"'":
j = _skip_literal(text, j)
continue
Comment on lines +210 to +212

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changing this in round 3. It is a real lexer gap, but you only reach it with a block comment that holds a } INSIDE an interpolation hole and is followed by a literal /* in that same hole. That is far outside how this tree writes C#. Extending the hand lexer to cover comments in holes is hardening beyond this PR's scope, which is the comment stripping for ordinary code that #7366 asked for. I filed it to triage together with the raw-literal case on the sibling thread (rbuergi/Feedback/20261029-record-gate-lexer-comments-and-raw-literals-in-holes), with both repro shapes and a regression-case plan.

if c in "{([":
depth += 1
elif c in "})]":
if depth == 0 and c == "}":
return j + 1
depth -= 1
j += 1
return n


def _skip_literal(text: str, i: int) -> int:
"""`text[i]` opens a string or char literal; returns the index just past its end."""
n = len(text)
if text[i] == "'":
j = i + 1
while j < n:
c = text[j]
if c == "\\":
j += 2
continue
if c in "'\n":
return j + 1
j += 1
return n
p = i
while p > 0 and text[p - 1] in "$@":
p -= 1
prefix = text[p:i]
interpolated, verbatim = "$" in prefix, "@" in prefix
run = 0
while i + run < n and text[i + run] == '"':
run += 1
if run >= 3 and not verbatim:
close = text.find('"' * run, i + run)
return n if close == -1 else close + run
Comment on lines +245 to +247

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same answer as the sibling thread. A raw interpolated string whose hole holds another raw literal that contains /* is valid C#, but nothing in this tree is written that way, and following raw-interpolation holes recursively (dollar count included) is a lexer extension rather than a fix to what this PR changes. It is in the same triage item as the comment-in-hole case: rbuergi/Feedback/20261029-record-gate-lexer-comments-and-raw-literals-in-holes.

j = i + 1
while j < n:
c = text[j]
if verbatim:
if c == '"':
if j + 1 < n and text[j + 1] == '"':
j += 2
continue
return j + 1
else:
if c == "\\":
j += 2
continue
if c in '"\n':
return j + 1
if interpolated and c == "{":
if j + 1 < n and text[j + 1] == "{":
j += 2
continue
j = _skip_hole(text, j + 1)
continue
j += 1
return n


def _strip_comments(text: str) -> str:
out: list[str] = []
i, n = 0, len(text)
while i < n:
ch = text[i]
if ch == "/" and i + 1 < n and text[i + 1] == "/":
end = text.find("\n", i)
i = n if end == -1 else end
continue
if ch == "/" and i + 1 < n and text[i + 1] == "*":
end = text.find("*/", i + 2)
stop = n if end == -1 else end + 2
out.append(" " + "\n" * text.count("\n", i, stop))
i = stop
continue
if ch in "\"'":
stop = _skip_literal(text, i)
out.append(text[i:stop])
i = stop
continue
out.append(ch)
i += 1
return "".join(out)


def records_in(source: str) -> dict[str, list[Param]]:
"""Every public record's primary-constructor parameter list, by record name."""
source = _strip_bom(source)
source = _strip_comments(_strip_bom(source))
found: dict[str, list[Param]] = {}
for m in RECORD_RE.finditer(source):
open_idx = source.index("(", m.end() - 1)
Expand Down Expand Up @@ -376,6 +487,55 @@ def check(label: str, cond: bool) -> None:
check("an attribute is stripped, not counted", len(a) == 2 and a[0].name == "X")
check("a params modifier is stripped", a[1].name == "Rest")

# ── comments inside a parameter list are not parameters (#7366) ─────────────────────
# The core#5077 shape: line comments between parameters, holding commas, an em dash, a
# trailing identifier and a `//` inside a string default that must SURVIVE.
commented = (
"public sealed record Onboarding(\n"
" string Username,\n"
" string Email,\n"
" // The viewer's language (\"de\"), from the onboarding form's language picker —\n"
" // which itself defaults to the browser's, so the point of use reads it anyway.\n"
" string? Locale = null,\n"
" /* a block comment, with a comma,\n"
" spanning lines */ string Home = \"https://example.com/a,b\",\n"
" // not learning it, and is pinned to no learning path.\n"
" bool IsPlatformBootstrap = false);\n"
)
oc = records_in(commented).get("Onboarding") or []
check("comment text is not counted as parameters (#7366: real arity is 5)",
[p.name for p in oc] == ["Username", "Email", "Locale", "Home", "IsPlatformBootstrap"])
check("…and a comment is not swallowed into the next parameter's TYPE",
[p.type for p in oc] == ["string", "string", "string?", "string", "bool"])
reworded = commented.replace("so the point of use reads it anyway", "so callers, again, read it")
check("rewording a comment inside a parameter list is NOT a signature change",
not compare("Onboarding", oc, records_in(reworded).get("Onboarding") or [], "x.cs"))
dropped = commented.replace(" string Email,\n", " // Email moved, see the profile record,\n")
check("a real parameter leaving while a comment gains a comma is still an arity change",
[x.kind for x in compare("Onboarding", oc,
records_in(dropped).get("Onboarding") or [], "x.cs")] == ["arity"])
check("a `//` inside a string literal is not a comment",
records_in('public record U(string Url = "http://x", int N = 1);').get("U") is not None
and [p.name for p in records_in('public record U(string Url = "http://x", int N = 1);')["U"]]
== ["Url", "N"])
check("a commented-OUT record is not a record",
"Gone" not in records_in("// public record Gone(int A);\npublic record Here(int A);"))
# A mis-bounded literal leaves text outside any string, where a `/*` becomes a phantom
# comment that swallows every record after it — and an arity change there would pass.
four_quote = 'var s = """"\nhas """ /* inside\n"""";\npublic record After(int A, int B);\n'
check("a four-quote raw literal holding three quotes does not hide the record after it",
[p.name for p in records_in(four_quote).get("After") or []] == ["A", "B"])
nested = ('var s = $"{string.Concat("/*", "x")}";\npublic record After(int A, int B);\n'
'var t = "*/";\n')
check("a literal nested in an interpolation hole does not open a comment",
[p.name for p in records_in(nested).get("After") or []] == ["A", "B"])
check("…and `{{` in an interpolated string is an escaped brace, not a hole",
[p.name for p in records_in('var s = $"{{ /* }}";\npublic record After(int A);\n')
.get("After") or []] == ["A"])
check("a verbatim string opening with an escaped quote is not a raw literal",
[p.name for p in records_in('var s = @"""x"" /*";\npublic record After(int A);\n')
.get("After") or []] == ["A"])

# ── the transitional allowance, end to end through run() ─────────────────────────
#
# 🚨 These call run() on a real repository rather than the classifier in isolation. A guard
Expand Down
Loading
Loading