Validate a Registry Value by a Positive Grammar Both Its Consumer and an Editor Agree On by ptr727 · Pull Request #1510 · ptr727/ProjectTemplate

ptr727 · 2026-09-10T17:58:10Z

Closes #1503.

A validator is only as strict as the comparison its value eventually
meets, and three defects of that shape were found in three files in one
day. #1504 fixed the environment name and branch for whitespace, and the
issue then measured why that cannot close the class: str.strip()
removes the 29 characters Python calls whitespace and leaves U+200B,
U+200C, U+200D, U+2060, U+FEFF, U+00AD and U+180E standing. Each of those
is invisible, and each is exactly as unmatchable by select(.name == $n)
as a trailing space. A padded name is refused and its zero-width twin is
accepted, and both fail in the same place for the same reason.

A grammar per field, stated positively

spec/validate.py now declares three patterns and applies them instead
of a .strip() comparison. Stated positively a grammar admits nothing
invisible in either direction, needs no notion of whitespace, and cannot
drift when a Python release changes what str.isspace() answers.

The fields need separate grammars rather than one, because their
consumers differ. A deployment environment name is one line of
printable ASCII with no leading or trailing space, an interior space
admitted since GitHub documents no character restriction beyond length
and uniqueness. A deployment branch policy name is a ref pattern, so
releases/* is legitimate and the grammar admits every visible ASCII
character rather than the alphabet the three names the registry declares
today happen to use: a grammar fitted to pypi, production and
staging would pass every test and every live value, and refuse the
first adopter declaring a release line. A groundTruthBranch is
concatenated raw into a request path and a ?ref= query value, so it
admits only the RFC 3986 unreserved characters plus /.

The schema is advisory, so it answers to the validator

No gate runs registry/repos.schema.json. .github/actions/validate
runs spec/validate.py and nothing else, and the only thing resolving
the schema is an editor, in ECMA-262. Its description pattern carried a
$comment claiming it was kept in sync with description_errors(), and
it was out of sync in two directions at once, measured rather than
predicted:

pattern            ^\S(?:[^\n\r]*\S)?$
"desc\n"           Python re: matches   ECMA-262: does not
"\ufeffdesc" (a BOM)  Python re: matches   ECMA-262: does not
"desc\ufeff" (a BOM)  Python re: matches   ECMA-262: does not

So the pattern admitted a trailing newline the validator refuses, while
an editor refused a value the gate allows. The three patterns the schema
now copies are the exact strings spec/validate.py declares, and a test
fails if they ever stop being identical. Every pattern in the file is
written in what the two engines share: (?![\s\S]) rather than $,
which is end-of-string in only one of them, and literal ASCII ranges
rather than \s or \S, which name a different set in each. Measured
across all four patterns and 33 values, including every invisible
character above, the two engines disagree on nothing.

The description pattern stays the schema's own statement rather than a
copy, deliberately looser than the validator, because a description may
legitimately carry a tier-2 or tier-3 non-ASCII character that no
portable positive grammar enumerates. A test asserts that direction over
a corpus. exclusionReason loses its \S pattern for the same reason
and keeps minLength. #1504 reverted an editor-side copy for leaving the
editor stricter than the gate, and agreement measured in both engines is
what makes this one safe.

The sweep, and the two findings it turned up that are fixed here

The issue asks which declared fields are consumed by an exact comparison
or concatenated into a URL or a path, and whether each is validated to
the strictness that consumer requires. The name-into-path shape is the
highest-consequence one, and both instances are fixed:

url reached spec/audit.py's repo_slug(), which took the last two
path segments of the raw value. The gate matched on .strip() and made a
trailing .git optional, stripping it for the identity only, so
https://github.com/<owner>/<repo>.git passed and then addressed
repos/<owner>/<repo>.git/... in fifteen reads, plus a Docker Hub URL
whose 404 is skipped by design and so silently stops checking anything.
Rather than adding a second check, repo_slug() now calls the
validator's own github_identity(). One parse means the validator cannot
be looser than the consumer by construction.

groundTruthBranch had no validation at all, in the validator or the
schema, while spec/audit.py, spec/fidelity_honesty.py and
spec/workflow_reuse.py each concatenate it into a path segment and a
?ref= value. Every reader defaults on absence rather than falsiness, so
a declared "" survives and reads the branch list, and main?ref=x
sends a different request rather than failing. The url field's own
grammar already excludes ? and #; the field landing in the same URLs
had nothing.

Also fixed, since it is three lines inside the loop being rewritten:
environment_errors_for_repo accepted a branch declared twice inside one
custom set. configure.sh sorts and joins both sides, so the duplicate
makes the declaration longer than any live set can be and reports as
drift on an environment that has none. The issue lists this as out of
scope, and it is named here rather than left in a function this change
rewrites around it.

The sweep's lower-ranked findings are filed rather than fixed here.

What the local strict review pass changed

It raised twelve findings and every one that held is answered here rather
than filed. Three are worth naming, because the first draft was wrong
about each:

repo_slug() raised on a url the grammar does not parse. spec/audit.py
wraps audit_repo in except Exception and survives that, but
spec/fidelity_honesty.py and spec/workflow_reuse.py call it on every
entry with no handler, so one malformed url aborted a whole fleet report
that previously 404'd and bucketed it. It now falls back to the old
segment split, because this module is not the gate: spec/validate.py
refuses such a url, nothing runs it before an audit, and the finding
belongs on the gate rather than on the report.

GITHUB_URL_RE still ended in $, the exact trap the rest of this change
adds a test to forbid, so a newline-padded url parsed while a
space-padded one did not. It ends in (?![\s\S]) now.

repo_identity() held a byte-identical second copy of that regex behind
a comment claiming the two were in sync. It calls github_identity()
instead, and it keeps its .strip(), deliberately and one-directionally:
this is the function that decides whether a live repo has an entry at
all, so a padded url refusing to resolve here would report the repo as
having no entry, a DEFECT naming the wrong problem. The selftest case
that pins this is what caught the first attempt to remove it.

Also from that pass: --branch reaches the same path segment and ?ref=
value the registry field does, and had no check, so --branch 'main?per_page=1' retargeted every read. The branch grammar's first
character was alphanumeric for no reason a consumer has, which refused
the legal _wip while the error message listed _ as allowed, so both
ends now admit everything except a . and a /, and the message says
exactly that. The two groundTruthBranch fields reach one $def rather
than two literals. And four tests were passing on code they did not
constrain, each now proven by mutation: dropping the check's wiring into
main(), restoring the .strip(), widening the branch grammar to admit
?, and making repo_slug() raise again each fail at least one test,
where before all four left the suite green.

Verified: ruff format and check, mypy, 1350 tests, spec/validate.py,
spec/audit.py --selftest, the prose gate, and the EOL gate. All five
patterns the tree now holds agree across Python re and ECMA-262 on 43
values, including every invisible character named above.

What the second pass changed

GROUND_TRUTH_BRANCH_PATTERN admitted .., and the live API resolves it.
Measured: gh api "repos/ptr727/ProjectTemplate/branches/a/../../../../../zen"
returns 200 with GitHub's /zen body, and
branches/x/../../../../../repos/ptr727/PlexCleaner/branches/main returns
PlexCleaner's head, which audit_repo would then take as this
repository's ground-truth head. A value the first draft accepted read a
different repository. A positive character class cannot say "no two of
these adjacent", so the grammar now opens with (?![\s\S]*\.\.), a
whole-string negation both engines read identically. The --branch
override had the same hole and the same fix.

~ was admitted on the strength of RFC 3986 calling it unreserved, and
git check-ref-format refs/heads/~x rejects it, so the grammar promised a
name no repository can hold and a test asserted it as valid. It is
excluded, and the grammar is now stated as the intersection of the two
rule sets rather than as one of them. Checked against git directly:
_wip, wip- and a.b are legal and pass, ~x, a..b and a/../x are
rejected by git and refused here.

The shape sentence the error messages print was a hand-written literal in
two modules, in a change whose thesis is one definition. It is
GROUND_TRUTH_BRANCH_SHAPE, and a test fails if the two messages ever
stop quoting it.

defaults.groundTruthBranch got the schema pattern and no gate check, so
the advisory schema was briefly the stricter of the two, the direction
this change forbids everywhere else. The gate checks that key now.

repo_slug() still raised for an absent or non-string url, which
spec/workflow_reuse.py reaches by constructing {"name": HUB_NAME} as
its own fallback and calling this with no handler. It answers with the
entry's name instead, which 404s like any other unparseable slug and names
the entry in the message.

import audit at module scope ran git config while importing, so a host
with no git on PATH failed all of this file's cases including the ones
that never touch audit. It is a local import in the one class that needs
it.

assert_portable refused the [\s\S] union in a lookahead body, which is
the portable idiom rather than a divergent construct, and it only ever
checked three constructs by shape. It strips that union first, and a new
test executes every schema pattern in node against a 45-value corpus and
asserts Python and ECMA-262 return the same verdict for each, so a future
\d, \w or (?P<name>...) is caught by measurement rather than by
enumeration.

Reverted rather than kept: an attempt to hold the registry entry name to
the same printable-ASCII grammar. spec/validate.py's own dedupe tests
deliberately declare Straße to pin the casefold normalizer, GitHub
allows such a repository name, and an ASCII floor there refuses a legal
value to fix an invisible-character case that has a filed issue instead.

Six mutations, each of which left the suite green before this round, now
fail at least one test: admitting .., re-admitting ~, deleting the
--branch guard, deleting the defaults check, raising on a missing url,
and letting the two shape sentences drift.

Deferred, filed rather than fixed here

The Remaining Six Registry Fields Whose Validation Is Looser Than Their Consumer, or Whose Declaration Has No Consumer #1508 carries the sweep's remaining six findings, each local to a different consumer.
AUDIT.md Section 1 Does Not Describe the Third Outcome --branch Now Has #1509 carries the one documentation gap the --branch guard opens, in a carried canonical unit whose edit owes its own whole-unit review pass.

Verification

ruff format --check, ruff check, mypy, 1356 tests, python3 spec/validate.py, python3 spec/audit.py --selftest, the prose gate, the EOL gate, and canonical_review.py check are all clean on this head. Every schema pattern is executed in both Python re and ECMA-262 by a test in the suite, not only measured by hand.

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features
- Added validation for printable environment names, deployment branches, and ground-truth branches.
- Added duplicate-branch detection and checks for invalid branch values in registry defaults and repositories.
- Added consistent GitHub repository URL and identity validation.
Bug Fixes
- Improved handling of padded, malformed, or non-string repository URLs.
- Added clearer validation messages for invalid names and branches.
- Updated schema rules to match validation behavior across supported engines.

… an Editor Agree On Closes #1503. A validator is only as strict as the comparison its value eventually meets, and three defects of that shape were found in three files in one day. #1504 fixed the environment name and branch for whitespace, and the issue then measured why that cannot close the class: `str.strip()` removes the 29 characters Python calls whitespace and leaves U+200B, U+200C, U+200D, U+2060, U+FEFF, U+00AD and U+180E standing. Each of those is invisible, and each is exactly as unmatchable by `select(.name == $n)` as a trailing space. A padded name is refused and its zero-width twin is accepted, and both fail in the same place for the same reason. ## A grammar per field, stated positively `spec/validate.py` now declares three patterns and applies them instead of a `.strip()` comparison. Stated positively a grammar admits nothing invisible in either direction, needs no notion of whitespace, and cannot drift when a Python release changes what `str.isspace()` answers. The fields need separate grammars rather than one, because their consumers differ. A deployment environment `name` is one line of printable ASCII with no leading or trailing space, an interior space admitted since GitHub documents no character restriction beyond length and uniqueness. A deployment branch policy name is a ref *pattern*, so `releases/*` is legitimate and the grammar admits every visible ASCII character rather than the alphabet the three names the registry declares today happen to use: a grammar fitted to `pypi`, `production` and `staging` would pass every test and every live value, and refuse the first adopter declaring a release line. A `groundTruthBranch` is concatenated raw into a request path and a `?ref=` query value, so it admits only the RFC 3986 unreserved characters plus `/`. ## The schema is advisory, so it answers to the validator No gate runs `registry/repos.schema.json`. `.github/actions/validate` runs `spec/validate.py` and nothing else, and the only thing resolving the schema is an editor, in ECMA-262. Its `description` pattern carried a `$comment` claiming it was kept in sync with `description_errors()`, and it was out of sync in two directions at once, measured rather than predicted: pattern ^\S(?:[^\n\r]*\S)?$ "desc\n" Python re: matches ECMA-262: does not "\ufeffdesc" (a BOM) Python re: matches ECMA-262: does not "desc\ufeff" (a BOM) Python re: matches ECMA-262: does not So the pattern admitted a trailing newline the validator refuses, while an editor refused a value the gate allows. The three patterns the schema now copies are the exact strings `spec/validate.py` declares, and a test fails if they ever stop being identical. Every pattern in the file is written in what the two engines share: `(?![\s\S])` rather than `$`, which is end-of-string in only one of them, and literal ASCII ranges rather than `\s` or `\S`, which name a different set in each. Measured across all four patterns and 33 values, including every invisible character above, the two engines disagree on nothing. The `description` pattern stays the schema's own statement rather than a copy, deliberately looser than the validator, because a description may legitimately carry a tier-2 or tier-3 non-ASCII character that no portable positive grammar enumerates. A test asserts that direction over a corpus. `exclusionReason` loses its `\S` pattern for the same reason and keeps `minLength`. #1504 reverted an editor-side copy for leaving the editor stricter than the gate, and agreement measured in both engines is what makes this one safe. ## The sweep, and the two findings it turned up that are fixed here The issue asks which declared fields are consumed by an exact comparison or concatenated into a URL or a path, and whether each is validated to the strictness that consumer requires. The name-into-path shape is the highest-consequence one, and both instances are fixed: `url` reached `spec/audit.py`'s `repo_slug()`, which took the last two path segments of the raw value. The gate matched on `.strip()` and made a trailing `.git` optional, stripping it for the identity only, so `https://github.com/<owner>/<repo>.git` passed and then addressed `repos/<owner>/<repo>.git/...` in fifteen reads, plus a Docker Hub URL whose 404 is skipped by design and so silently stops checking anything. Rather than adding a second check, `repo_slug()` now calls the validator's own `github_identity()`. One parse means the validator cannot be looser than the consumer by construction. `groundTruthBranch` had no validation at all, in the validator or the schema, while `spec/audit.py`, `spec/fidelity_honesty.py` and `spec/workflow_reuse.py` each concatenate it into a path segment and a `?ref=` value. Every reader defaults on absence rather than falsiness, so a declared `""` survives and reads the branch *list*, and `main?ref=x` sends a different request rather than failing. The `url` field's own grammar already excludes `?` and `#`; the field landing in the same URLs had nothing. Also fixed, since it is three lines inside the loop being rewritten: `environment_errors_for_repo` accepted a branch declared twice inside one `custom` set. `configure.sh` sorts and joins both sides, so the duplicate makes the declaration longer than any live set can be and reports as drift on an environment that has none. The issue lists this as out of scope, and it is named here rather than left in a function this change rewrites around it. The sweep's lower-ranked findings are filed rather than fixed here. ## What the local strict review pass changed It raised twelve findings and every one that held is answered here rather than filed. Three are worth naming, because the first draft was wrong about each: `repo_slug()` raised on a url the grammar does not parse. `spec/audit.py` wraps `audit_repo` in `except Exception` and survives that, but `spec/fidelity_honesty.py` and `spec/workflow_reuse.py` call it on every entry with no handler, so one malformed url aborted a whole fleet report that previously 404'd and bucketed it. It now falls back to the old segment split, because this module is not the gate: `spec/validate.py` refuses such a url, nothing runs it before an audit, and the finding belongs on the gate rather than on the report. `GITHUB_URL_RE` still ended in `$`, the exact trap the rest of this change adds a test to forbid, so a newline-padded url parsed while a space-padded one did not. It ends in `(?![\s\S])` now. `repo_identity()` held a byte-identical second copy of that regex behind a comment claiming the two were in sync. It calls `github_identity()` instead, and it keeps its `.strip()`, deliberately and one-directionally: this is the function that decides whether a live repo has an entry at all, so a padded url refusing to resolve here would report the repo as having no entry, a DEFECT naming the wrong problem. The selftest case that pins this is what caught the first attempt to remove it. Also from that pass: `--branch` reaches the same path segment and `?ref=` value the registry field does, and had no check, so `--branch 'main?per_page=1'` retargeted every read. The branch grammar's first character was alphanumeric for no reason a consumer has, which refused the legal `_wip` while the error message listed `_` as allowed, so both ends now admit everything except a `.` and a `/`, and the message says exactly that. The two `groundTruthBranch` fields reach one `$def` rather than two literals. And four tests were passing on code they did not constrain, each now proven by mutation: dropping the check's wiring into `main()`, restoring the `.strip()`, widening the branch grammar to admit `?`, and making `repo_slug()` raise again each fail at least one test, where before all four left the suite green. Verified: ruff format and check, mypy, 1350 tests, `spec/validate.py`, `spec/audit.py --selftest`, the prose gate, and the EOL gate. All five patterns the tree now holds agree across Python `re` and ECMA-262 on 43 values, including every invisible character named above. ## What the second pass changed `GROUND_TRUTH_BRANCH_PATTERN` admitted `..`, and the live API resolves it. Measured: `gh api "repos/ptr727/ProjectTemplate/branches/a/../../../../../zen"` returns 200 with GitHub's `/zen` body, and `branches/x/../../../../../repos/ptr727/PlexCleaner/branches/main` returns PlexCleaner's head, which `audit_repo` would then take as this repository's ground-truth head. A value the first draft accepted read a different repository. A positive character class cannot say "no two of these adjacent", so the grammar now opens with `(?![\s\S]*\.\.)`, a whole-string negation both engines read identically. The `--branch` override had the same hole and the same fix. `~` was admitted on the strength of RFC 3986 calling it unreserved, and `git check-ref-format refs/heads/~x` rejects it, so the grammar promised a name no repository can hold and a test asserted it as valid. It is excluded, and the grammar is now stated as the intersection of the two rule sets rather than as one of them. Checked against git directly: `_wip`, `wip-` and `a.b` are legal and pass, `~x`, `a..b` and `a/../x` are rejected by git and refused here. The shape sentence the error messages print was a hand-written literal in two modules, in a change whose thesis is one definition. It is `GROUND_TRUTH_BRANCH_SHAPE`, and a test fails if the two messages ever stop quoting it. `defaults.groundTruthBranch` got the schema pattern and no gate check, so the advisory schema was briefly the stricter of the two, the direction this change forbids everywhere else. The gate checks that key now. `repo_slug()` still raised for an absent or non-string url, which `spec/workflow_reuse.py` reaches by constructing `{"name": HUB_NAME}` as its own fallback and calling this with no handler. It answers with the entry's name instead, which 404s like any other unparseable slug and names the entry in the message. `import audit` at module scope ran `git config` while importing, so a host with no git on `PATH` failed all of this file's cases including the ones that never touch audit. It is a local import in the one class that needs it. `assert_portable` refused the `[\s\S]` union in a lookahead body, which is the portable idiom rather than a divergent construct, and it only ever checked three constructs by shape. It strips that union first, and a new test executes every schema pattern in node against a 45-value corpus and asserts Python and ECMA-262 return the same verdict for each, so a future `\d`, `\w` or `(?P<name>...)` is caught by measurement rather than by enumeration. Reverted rather than kept: an attempt to hold the registry entry `name` to the same printable-ASCII grammar. `spec/validate.py`'s own dedupe tests deliberately declare `Straße` to pin the casefold normalizer, GitHub allows such a repository name, and an ASCII floor there refuses a legal value to fix an invisible-character case that has a filed issue instead. Six mutations, each of which left the suite green before this round, now fail at least one test: admitting `..`, re-admitting `~`, deleting the `--branch` guard, deleting the defaults check, raising on a missing url, and letting the two shape sentences drift.

coderabbitai · 2026-09-10T17:58:18Z

Warning

Review limit reached

Next included review available in 11 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 7c266d14-5d21-40c3-97e8-9bb6df5791bf

📥 Commits

Reviewing files that changed from the base of the PR and between b510ac8 and d0a4787.

📒 Files selected for processing (3)

scripts/tests/test_spec_validate.py
spec/audit.py
spec/validate.py

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: fdf7381b-1e38-4768-8a9a-e97ad6df17bc

📥 Commits

Reviewing files that changed from the base of the PR and between 6dcce49 and b510ac8.

📒 Files selected for processing (4)

registry/repos.schema.json
scripts/tests/test_spec_validate.py
spec/audit.py
spec/validate.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

Changes

Registry validation

Layer / File(s)	Summary
Portable field grammars `registry/repos.schema.json`, `spec/validate.py`, `scripts/tests/test_spec_validate.py`	The schema and validator now share portable patterns for environment names, branches, and ground-truth branches. Tests verify pattern parity across regex engines.
Registry field validation `spec/validate.py`, `scripts/tests/test_spec_validate.py`	Validation now rejects invalid environment names and branches, detects duplicate branches, and checks ground-truth branches for defaults and repositories.
GitHub identity integration `spec/validate.py`, `spec/audit.py`, `scripts/tests/test_spec_validate.py`	GitHub URL parsing is centralized in `github_identity()`. Audit identity and slug handling use it, while `--branch` overrides use the ground-truth branch grammar.

Estimated code review effort: 4 (Complex) | ~45 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to b510a

The updated registry validation and audit behavior are consistently covered and no merge-blocking risk is evident.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 65.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 60 functions across 3 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: positive registry grammars shared by the consumer and editor.
Linked Issues check	✅ Passed	The changes satisfy the coding objectives in [`#1503`]. They add positive field-specific grammars, align schema behavior across Python and ECMA-262, validate ground-truth branches, centralize URL parsin…
Out of Scope Changes check	✅ Passed	The changes remain within [`#1503`]. Schema updates, validator changes, audit integration, and related tests support the stated grammar, consumer-alignment, URL, branch, and duplicate-detection objectiv…

Full details: Docstring Coverage

Explanation

Docstring coverage is 65.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 60 functions across 3 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feature/1503-positive-grammar-validators

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

ptr727 · 2026-09-10T17:59:44Z

@coderabbitai review

coderabbitai · 2026-09-10T17:59:50Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copilot

🟡 Changes recommended

spec/audit.py repo_slug() can still form gh api paths that include '?' or '#', which can retarget requests and break audit behavior for malformed URLs.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR tightens registry validation by replacing whitespace-based checks with positive, portable grammars that match how downstream consumers actually compare or concatenate values, and then mirrors those same regex strings into the advisory JSON schema with tests that pin cross-engine agreement.

Changes:

Introduces field-specific positive regex grammars in spec/validate.py (environment name, deployment branch policy patterns, and groundTruthBranch) and wires them into the validator.
Updates spec/audit.py to reuse spec/validate.py URL parsing (avoiding drift between validator and consumer) and validates --branch against the same groundTruthBranch grammar.
Adds tests that (1) prove the validator wiring via scratch registries and (2) assert schema/validator regex byte-identity and Python-vs-ECMA-262 agreement, while updating registry/repos.schema.json to reference the shared $defs.

File summaries

File	Description
spec/validate.py	Adds portable positive grammars and applies them to registry validation (including `groundTruthBranch` and URL identity parsing).
spec/audit.py	Reuses validator URL parsing for slugs and validates `--branch` with the same grammar as `groundTruthBranch`.
scripts/tests/test_spec_validate.py	Expands tests to pin validator wiring, schema mirroring/portability, and Python-vs-Node regex agreement.
registry/repos.schema.json	Mirrors the validator’s portable regex strings via `$defs` and updates field patterns/comments to match the new contract.

Review details

Files reviewed: 4/4 changed files
Comments generated: 1
Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot, on #1510: repo_slug()'s fallback built its value from a url the grammar refused, so a `?` or a `#` in that url survived into the slug. Raw, `repos/owner/Fixture?tab=readme/branches/main` is a request to `repos/owner/Fixture` with the rest as a query string, which reads a different resource rather than failing. That is the one shape the grammars in this branch exist to remove, and the fallback was the last place still producing it. Each fallback segment is percent-encoded now, so the same value is a 404 that names itself, which is what the fallback was for. The absent and non-string url case is encoded the same way. This is the fix #1506 already applied to an environment name before it became a path segment, reached from the other direction. A test asserts no fallback slug carries a `?`, a `#` or a space, over the four url shapes that produced one.

…hat Did Not Parse The local pass over the previous commit found that the same defect sat on the accepted path, not only the fallback, and that percent-encoding cannot close it. `GITHUB_URL_RE`'s two segments were `[^/\s?#]+`, which admits a segment that is nothing but dots. Measured against the live API, `https://github.com/../rate_limit` parsed, the gate printed "Spec validation OK", and `gh api repos/../rate_limit` returned 200 with the rate-limit document rather than failing. `repos/../user` returns the authenticated user and `repos/../..` returns the API root. Encoding is no answer: `repos/%2e%2e/rate_limit` returns 200 the same way, because GitHub decodes and then normalizes a dot segment. The segments now name GitHub's own character sets, an owner being letters, digits and hyphens and a repository name adding `.`, `_` and `-`, which refuses no url that can exist since GitHub replaces anything else at creation time. A segment that is nothing but dots is refused in `github_identity` rather than in the pattern, because a character class cannot say "not only dots". Only `.` and `..` normalize, so `.github`, `v1.0` and `a..b` still resolve. `repo_slug`'s fallback no longer builds anything out of the url at all. Percent-encoding its last two segments fixed the `?` and `#` shapes and left the worse one standing: `https://gitlab.test/owner/Repo` became `owner/Repo` and read that repository on github.com, so a url the gate refuses addressed a real and possibly unrelated repository. The fallback is the entry's percent-encoded name, which is one segment resolving to no repository, so the read 404s and the message names the entry that caused it. Also corrected from that pass: the previous commit's test comment claimed a space in a slug "would end an argument", which is false, since `gh()` runs a list-form subprocess with no shell and `gh` percent-encodes a space itself before the wire, traced with `GH_DEBUG=api`. And the docstring claimed every fallback was encoded per segment and always 404'd, neither of which described the name path or the resolving `http://` and non-GitHub shapes. Three mutations that leave the suite green without this commit now fail it: admitting a dot segment, widening the url classes back, and restoring the segment-split fallback.

ptr727 · 2026-09-10T18:32:24Z

@coderabbitai review

coderabbitai · 2026-09-10T18:32:29Z

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copilot

🟡 Changes recommended

The new repo_slug() fallback in spec/audit.py is not guaranteed to 404 and can still accidentally address real repositories for certain entry names; the fallback should be made structurally unresolvable.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 4/4 changed files
Comments generated: 1
Review effort level: Lite

… a Repository Copilot, on #1510: the previous commit's fallback was the entry's percent-encoded name, and that is neither guaranteed to 404 nor safe from the traversal it was meant to close. A one-segment slug shifts every later path component up one, so `repos/Fixture/git/trees/<sha>` reads owner `Fixture` and repository `git` rather than failing. And `urllib.parse.quote` leaves a dot untouched, so a name of `..` came through verbatim and `repos/<owner>/../branches/main` normalizes to `repos/branches/main`, which is the same traversal the commit before it closed on the url, re-entering through the name. A registry name is checked only for padding, so `"name": ".."` passes the gate. The fallback is now `_unresolved/<name>`, and each half answers one of those. Two segments, so nothing shifts. An owner segment carrying an underscore, which GitHub allows in a repository name and not in an owner name, so it cannot collide with a real owner: `gh api repos/_unresolved/Fixture` and `gh api users/_unresolved` both return non-200. And a name that is nothing but dots is replaced by `_unnamed` rather than encoded, because encoding does not help, `%2e%2e` normalizes the same way. Every consumer is better off than with a single segment. `docker_hub_description` unpacks two names and would have raised ValueError on one; it now builds a hub URL that 404s and is skipped by design. Three mutations that leave the suite green without this commit now fail it: returning one segment, admitting a dot-only name, and using an owner GitHub could actually hold.

Copilot

🟢 Approval recommended

The changes consistently tighten validation to match consumer behavior, close the identified retargeting/mismatch risks, and add comprehensive tests that pin both validator/schema parity and cross-regex-engine agreement.

Review details

Files reviewed: 4/4 changed files
Comments generated: 0 new
Review effort level: Lite

ptr727 · 2026-09-10T18:48:00Z

@coderabbitai review

coderabbitai · 2026-09-10T18:48:05Z

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copilot, on #1514: the citation named CODESTYLE.md, and that file carries no such heading. The rule is GOVERNANCE.md "Documentation Style Conventions" and its "Character Set" subsection, verified by heading search rather than by memory: CODESTYLE.md matches nothing for it. Corrected on both surfaces that state it, not only the one the finding landed on. spec/validate.py carried the same wrong citation from #1510, and it is the file the schema comment points at, so leaving it would have sent a reader from a correct reference to an incorrect one.

…ts Below Its Reference (#1514) Copilot, on the #1513 promotion pull request: the environmentName $comment said a zero-width space "is already trimmed", which reads as str.strip() removing it, while the tests in the same change assert the opposite. The first attempt at that sentence was longer and wrong in two new ways, found by the local pass over it. "A name carrying one is already in trimmed form" is false in general, since " <zws>pypi" is not, and it holds only for the narrower case the tests assert, where the zero-width character is the whole padding. And inserting "a padding check" gave "that comparison" a nearer antecedent than the one it meant, so the sentence read as saying a check accepts the value and then that the value fails that same check. The sentence is shorter instead of more qualified. It says str.strip() does not remove a zero-width space, that a name ending in one survives a padding test, and that it then matches no live environment, which is the consequence named rather than referred to. It also attributes the ASCII floor to CODESTYLE.md "Character set", which is where spec/validate.py locates it, rather than to the zero-width case, which argues for a grammar over a negative test and not for the ASCII bound. Two defects from #1510 in the same file go with it, since both are stated here and nowhere else. The sibling $comment said "the two patterns below" after that change moved the name pattern into $defs, so the reference below it is a $ref rather than a pattern, and its first sentence duplicated the one now in $defs/environmentName. Wording only. No pattern, no check and no test changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

… an Editor Agree On (#1513) Promotes `develop` to `main`. Closes #1503. ## Validate a Registry Value by a Positive Grammar Both Its Consumer and an Editor Agree On (#1510) A validator is only as strict as the comparison its value eventually meets. #1504 fixed a deployment environment name and its branch set for whitespace, and #1503 then measured why that cannot close the class: `str.strip()` removes the 29 characters Python calls whitespace and leaves U+200B, U+200C, U+200D, U+2060, U+FEFF, U+00AD and U+180E standing. Each is invisible and each is exactly as unmatchable by an exact comparison as a trailing space, so refusing padding admits the zero-width twin that fails in the same place for the same reason. Three grammars replace those tests, stated positively so they admit nothing invisible in either direction, need no notion of whitespace, and cannot drift when a Python release changes `str.isspace()`. They differ because their consumers differ: a deployment environment name is one line of printable ASCII, a deployment branch policy name is a ref *pattern* so `releases/*` is legitimate, and a `groundTruthBranch` is concatenated raw into a request path and a `?ref=` value so it admits only what is both unreserved in RFC 3986 and legal in a git ref name. `registry/repos.schema.json` is advisory, since no gate runs it, so it carries the validator's exact pattern strings through `$def`s and a test fails if they stop being identical. Its `description` pattern claimed a sync it did not have, in two directions at once: `"desc\n"` matched in Python and not in ECMA-262, and a BOM-prefixed value matched in Python and not in ECMA-262, so the pattern admitted a trailing newline the validator refuses while an editor refused a value the gate allows. Every pattern is now written in what the two engines share, and a test executes each one in both and asserts they agree. ## What the review rounds found, which is most of the value here Five rounds, three local adversarial passes and two Copilot rounds, each finding something the round before it had introduced or missed. The two that matter most were both proven against the live API: `GROUND_TRUTH_BRANCH_PATTERN` admitted `..`. A branch read whose ref traverses upward returns another repository's head with a 200, and `spec/audit.py` would take that as this repository's ground-truth head. Percent-encoding is no defense, since the encoded form normalizes the same way, so the grammar refuses the sequence outright. The same hole sat on the url field, and there it passed the gate: `GITHUB_URL_RE`'s segments were `[^/\s?#]+`, which admits a segment that is nothing but dots, so a traversing url parsed, `spec/validate.py` printed "Spec validation OK", and the audit's first read returned a non-repository document with a 200 instead of failing. The two segments now name GitHub's own character sets, and a dot-only segment is refused in `github_identity`, which leaves `.github`, `v1.0` and `a..b` resolving. `spec/audit.py`'s `repo_slug()` and `repo_identity()` each held a looser parse of their own, which is how the url field came to be validated by one rule and addressed by another. Both call `github_identity()` now. Its fallback for a url that does not parse went through three shapes before landing: taking the url's last two path segments produced a plausible slug that read an unrelated repository, percent-encoding those segments left a one-segment value that shifted every later path component up one, and a dots-only entry name re-entered the traversal through the name. It answers with two segments that cannot name a repository. Also from those rounds: the `--branch` override reaches the same path segment and `?ref=` value the declared field does and had no check; the grammar's first character was alphanumeric for no reason a consumer has, refusing a legal `_wip` while the error message listed `_` as allowed; the shape sentence was a hand-written literal in two modules in a change whose thesis is one definition; `defaults.groundTruthBranch` briefly had a schema pattern and no gate check, making the advisory schema the stricter of the two; and `import audit` at module scope ran a git command while importing, so a host without git failed every case in the test file. An attempt to hold the registry entry `name` to the same printable-ASCII grammar was reverted on measurement: `spec/validate.py`'s own dedupe tests deliberately declare a non-ASCII name to pin the casefold normalizer, GitHub allows such a repository name, and an ASCII floor there refuses a legal value. It is #1508 instead. ## Verification Thirteen mutations that leave the suite green without this change now fail it, including admitting a dot segment, widening the url classes back, deleting the `--branch` guard, and every earlier shape of the fallback slug. `ruff format --check`, `ruff check`, `mypy`, 1359 tests, `spec/validate.py`, `spec/audit.py --selftest`, the prose gate, the EOL gate, and `canonical_review.py check` are clean. ## Follow-ups filed rather than fixed - #1508: the sweep's remaining six findings, each local to a different consumer, plus the missing unknown-key check. - #1509: the one `AUDIT.md` clause the `--branch` guard leaves incomplete, in a carried canonical unit whose edit owes its own whole-unit review pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code)  ## Summary by CodeRabbit * **New Features** * Added validation for environment names, deployment branches, and ground-truth branches. * Added clearer checks for GitHub repository URLs and repository identity. * Added safe fallback identifiers for audit entries with unrecognized or missing repository URLs. * **Bug Fixes** * Improved handling of repository descriptions, exclusion reasons, and environment branch names. * Added validation for branch overrides used during audits. * Improved audit reporting for padded URLs and invalid repository names. * **Tests** * Expanded coverage for schema validation, branch rules, URL parsing, duplicate branches, and diagnostic messages.

Copilot AI lite review requested due to automatic review settings September 10, 2026 17:58

Copilot started reviewing on behalf of ptr727 September 10, 2026 17:58 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Comment thread spec/audit.py Outdated

ptr727 added 2 commits September 10, 2026 11:13

Copilot AI review requested due to automatic review settings September 10, 2026 18:31

Copilot started reviewing on behalf of ptr727 September 10, 2026 18:32 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Comment thread spec/audit.py Outdated

Copilot AI review requested due to automatic review settings September 10, 2026 18:42

Copilot started reviewing on behalf of ptr727 September 10, 2026 18:43 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

ptr727 merged commit e51dccb into develop Sep 10, 2026
9 checks passed

ptr727 deleted the feature/1503-positive-grammar-validators branch September 10, 2026 18:49

This was referenced Sep 10, 2026

Validate a Registry Value by a Positive Grammar Both Its Consumer and an Editor Agree On #1513

Merged

Name the Real Reason for the ASCII Floor and Stop Saying a Pattern Sits Below Its Reference #1514

Merged

Navigation Menu

Validate a Registry Value by a Positive Grammar Both Its Consumer and an Editor Agree On - #1510

Validate a Registry Value by a Positive Grammar Both Its Consumer and an Editor Agree On#1510
ptr727 merged 4 commits into
developfrom
feature/1503-positive-grammar-validators

ptr727 commented Sep 10, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 10, 2026 •

edited

Loading

Review limit reached

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

ptr727 commented Sep 10, 2026

Uh oh!

coderabbitai Bot commented Sep 10, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

ptr727 commented Sep 10, 2026

Uh oh!

coderabbitai Bot commented Sep 10, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

ptr727 commented Sep 10, 2026

Uh oh!

coderabbitai Bot commented Sep 10, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ptr727 commented Sep 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

A grammar per field, stated positively

The schema is advisory, so it answers to the validator

The sweep, and the two findings it turned up that are fixed here

What the local strict review pass changed

What the second pass changed

Deferred, filed rather than fixed here

Verification

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

ptr727 commented Sep 10, 2026

Uh oh!

coderabbitai Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

ptr727 commented Sep 10, 2026

Uh oh!

coderabbitai Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

ptr727 commented Sep 10, 2026

Uh oh!

coderabbitai Bot commented Sep 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ptr727 commented Sep 10, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 10, 2026 •

edited

Loading

coderabbitai Bot commented Sep 10, 2026 •

edited

Loading

coderabbitai Bot commented Sep 10, 2026 •

edited

Loading

coderabbitai Bot commented Sep 10, 2026 •

edited

Loading