Reject a Deployment Environment Name or Branch That Carries Surrounding Whitespace - #1504
Merged
Conversation
…ng Whitespace `environment_errors_for_repo` tested a declared name and each custom branch name for emptiness through `.strip()` truthiness, which a padded value passes. `"pypi "` and `" main"` were therefore valid declarations that `repo-config/configure.sh` can never resolve, since it matches the environment with `select(.name == $n)` and compares each declared branch to the live policy names directly. The repository reports the environment as missing, or the branch set as drifted, while carrying both correctly. ## What changed `spec/validate.py` reports each case in its own branch rather than widening an existing message. - A name that survives the emptiness check but differs from its own stripped form is reported as padded, before the duplicate check rather than after, so a padded twin of an existing name is named for what is wrong with it rather than for colliding. - A custom branch set is checked for padding after the non-empty check, so a whitespace-only entry is still reported as empty and reported once. The message names every padded entry, since reporting the first alone would hide the rest behind a fix round. `repo-config/configure.sh`'s note that its own jq shape test is narrower than the validator's listed the two shapes that differed. This change adds two more, so the note states the category instead of a list that goes stale on every change that widens the gap. ## The schema was left alone, on measurement `registry/repos.schema.json` co-declares this shape with `minLength: 1`, so a padded value is schema-valid and validator-invalid. Tightening it with the `^\S(?:[^\n\r]*\S)?$` idiom that file already uses was tried here and reverted, because it made the disagreement worse rather than smaller. `registry/repos.json` names that schema and no CI job validates against it, so the only reader that ever applies the pattern is the author's editor, and its engine is ECMA-262. ECMA-262 `\s` includes U+FEFF where Python's does not, so the editor refuses a name the validator accepts. Under Python `re`, which is what a command-line schema checker uses, `$` matches before a trailing newline, so `"pypi\n"` satisfies the pattern and fails the validator. A one-directional gap where an advisory schema admits what the gate refuses became a two-directional one where each refuses something the other allows, and a false rejection in an editor is the worse half. Closing it needs a positive grammar both engines agree on, per field, which is a design change rather than a tightening. That is #1503. ## What this does not do Padding is not the whole class. `str.strip()` encodes Python's notion of whitespace, and a zero-width space or a byte-order mark is invisible, equally unmatchable by an exact comparison, and not whitespace to Python, so this check accepts one exactly as the truthiness check did. That gap is pre-existing rather than introduced here, and it is #1503 as well. ## Verification Four of the five added tests fail against the unfixed `environment_errors_for_repo`, measured by reverting only that file into a scratch tree: six subtest failures across the padded-name, padded-twin, identical-twins and padded-branches cases. The fifth, `test_an_empty_branch_is_reported_as_empty_rather_than_as_padded`, passes either way by design, guarding the ordering the fix introduces so that a later edit cannot let the new check swallow the empty case. `spec/validate.py` against the live registry, `prose_lint.py`, `repo_gate.py` across eol and eol-coverage, `spec/audit.py --selftest`, `spec/workflow_reuse.py --selftest`, `build_dist.py --check`, `ruff format --check`, `ruff check`, `mypy`, the `unittest` suite at 1313 passed, and `docker_lint.py` over shellcheck, shfmt, editorconfig-checker and cspell. `canonical_review.py check` reports no carried canonical unit changed. Three adversarial passes ran before this was pushed. Two of what they raised changed the code: the schema tightening above, reverted on the second pass's measurement, and the coverage gap that became #1503. The rest were defects in this change's own account of itself, including a comment claiming every padding shape reaches the consumer intact, where command substitution and the `jqr` helper eat a trailing newline before the comparison, and a count of failing tests wrong by one in the direction that understated the evidence. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 34 minutes. View limit detailsLimit 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. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
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. Comment |
Owner
Author
|
@coderabbitai review |
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The validator change is correct, is exercised by targeted tests, and the shell-side update is a comment-only clarification consistent with the new validation behavior.
Pull request overview
This pull request tightens the registry validator so deployment environment names and custom branch entries that include leading/trailing whitespace are rejected, preventing declarations that cannot match repo-config/configure.sh's exact comparisons.
Changes:
- Extend
spec/validate.py'senvironment_errors_for_repo()to reject environmentnamevalues and custombranchesentries that differ from their stripped forms. - Add unit tests covering padded names, padded-twin ordering, and padded branch lists (including the empty-vs-padded ordering guard).
- Update
repo-config/configure.shcommentary to describe the jq shape test as intentionally less strict thanspec/validate.py.
File summaries
| File | Description |
|---|---|
| spec/validate.py | Rejects leading/trailing whitespace in environment names and custom branch entries to match downstream exact-compare behavior. |
| scripts/tests/test_spec_validate.py | Adds regression tests for padded names/branches and error-reporting ordering. |
| repo-config/configure.sh | Refreshes the explanatory comment to remain accurate as spec/validate.py grows stricter. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ptr727
added a commit
that referenced
this pull request
Sep 10, 2026
…g, and the Release-Trigger Retirement (#1489) Promotes thirteen merged pull requests from `develop` to `main`. ## What this promotes - **#1486** State the `host-tools.json` carry rule and stop the empty-overlay warning. `spec/files.json` now requires the declaration unconditionally, and `host_gate.py` no longer warns on an empty one. - **#1484** Fan the validator's pytest leg out over a `python-versions` input, so a caller with more than one interpreter no longer silently loses its other legs to a pinned 3.13, with an entry guard that refuses a value `fromJson` would otherwise admit. - **#1476** Retire the publish-on-merge release trigger, which `spec/files.json` applied to an interface contract whose task has no publish-on-merge path. - **#1479** Ship the release caller snippet and retire the not-yet-released claim. - **#1474** Exclude `node_modules` from markdownlint and Docker lint discovery at any depth. - **#1468** Move the carried-surface cluster off decision, recording the measurements and what stays unmeasured on the issues that hold them. - **#1498** Correct the release-caller snippet's publish-on-merge claim, raised by CodeRabbit against this promotion and fixed on its own branch, since a promotion PR's head is `develop`. - **#1494** Settle the markdown lint exclusion seam as a nested config. - **#1492** Check declared deployment environments and fix the carried-payload rule. - **#1501** Move the session handoff to a chain of labeled issues. Two of its units are verbatim fidelity, `AGENTS.md` "Session Scope" and `GOVERNANCE.md` "Communicating with the User", so every fleet repo picks them up on its next resync. - **#1505** Refuse a bare handoff link rather than reading it as no successor, answering a finding raised against this promotion. - **#1504** Reject a deployment environment name or branch carrying surrounding whitespace, answering two more. - **#1506** Encode an environment name before it becomes a path segment, delete a branch that cannot fire, and dedupe a registry name by case, answering two more and declining one on a reproduction. ## Closing references A `develop`-targeted pull request registers no closing reference, so the issues those pull requests settled close with this promotion. #1470 is the one that needs a word. Two of its seven acceptance items are not met as literally written, and the maintainer's decision to close it here anyway, with the basis for both, is recorded on the issue itself. Closes #1483 Closes #1473 Closes #1487 Closes #1482 Closes #1475 Closes #1470 ## Left open deliberately - **#1499**, **#1500** follow-ups #1501 filed rather than fixed. - **#1503** the validator-looser-than-its-consumer class, filed from findings on this promotion and deliberately not closed by it. Five instances recorded, and the fix is a design decision rather than a check. - **#1497** the Docker delta list never says to remove the `push:` block. - **#1472** the fleet-level pin call, deliberately not pre-empted by #1479. - **#1321**, **#1464** carry #1468's decision record and remain in their own scope. ## Verification `origin/main..origin/develop` merges clean, no end-of-line conflict. Each promoted pull request ran the fleet gate set on its own branch before merging; this promotion adds no content of its own. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01817rckKywjfoaEV5WF9TKs
ptr727
added a commit
that referenced
this pull request
Sep 10, 2026
… an Editor Agree On (#1510) 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 - #1508 carries the sweep's remaining six findings, each local to a different consumer. - #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](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
ptr727
added a commit
that referenced
this pull request
Sep 10, 2026
… 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) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reject a Deployment Environment Name or Branch That Carries Surrounding Whitespace
environment_errors_for_repotested a declared name and each custombranch name for emptiness through
.strip()truthiness, which a paddedvalue passes.
"pypi "and" main"were therefore valid declarationsthat
repo-config/configure.shcan never resolve, since it matches theenvironment with
select(.name == $n)and compares each declared branchto the live policy names directly. The repository reports the environment
as missing, or the branch set as drifted, while carrying both correctly.
What changed
spec/validate.pyreports each case in its own branch rather thanwidening an existing message.
stripped form is reported as padded, before the duplicate check rather
than after, so a padded twin of an existing name is named for what is
wrong with it rather than for colliding.
so a whitespace-only entry is still reported as empty and reported
once. The message names every padded entry, since reporting the first
alone would hide the rest behind a fix round.
repo-config/configure.sh's note that its own jq shape test is narrowerthan the validator's listed the two shapes that differed. This change
adds two more, so the note states the category instead of a list that
goes stale on every change that widens the gap.
The schema was left alone, on measurement
registry/repos.schema.jsonco-declares this shape withminLength: 1,so a padded value is schema-valid and validator-invalid. Tightening it
with the
^\S(?:[^\n\r]*\S)?$idiom that file already uses was triedhere and reverted, because it made the disagreement worse rather than
smaller.
registry/repos.jsonnames that schema and no CI job validates againstit, so the only reader that ever applies the pattern is the author's
editor, and its engine is ECMA-262. ECMA-262
\sincludes U+FEFF wherePython's does not, so the editor refuses a name the validator accepts.
Under Python
re, which is what a command-line schema checker uses,$matches before a trailing newline, so
"pypi\n"satisfies the patternand fails the validator. A one-directional gap where an advisory schema admits what the
gate refuses became a two-directional one where each refuses something
the other allows, and a false rejection in an editor is the worse half.
Closing it needs a positive grammar both engines agree on, per field,
which is a design change rather than a tightening. That is #1503.
What this does not do
Padding is not the whole class.
str.strip()encodes Python's notion ofwhitespace, and a zero-width space or a byte-order mark is invisible,
equally unmatchable by an exact comparison, and not whitespace to Python,
so this check accepts one exactly as the truthiness check did. That gap
is pre-existing rather than introduced here, and it is #1503 as well.
Verification
Four of the five added tests fail against the unfixed
environment_errors_for_repo, measured by reverting only that file intoa scratch tree: six subtest failures across the padded-name, padded-twin,
identical-twins and padded-branches cases. The fifth,
test_an_empty_branch_is_reported_as_empty_rather_than_as_padded, passeseither way by design, guarding the ordering the fix introduces so that a
later edit cannot let the new check swallow the empty case.
spec/validate.pyagainst the live registry,prose_lint.py,repo_gate.pyacross eol and eol-coverage,spec/audit.py --selftest,spec/workflow_reuse.py --selftest,build_dist.py --check,ruff format --check,ruff check,mypy, theunittestsuite at 1313passed, and
docker_lint.pyover shellcheck, shfmt,editorconfig-checker and cspell.
canonical_review.py checkreports nocarried canonical unit changed.
Three adversarial passes ran before this was pushed. Two of what they
raised changed the code: the schema tightening above, reverted on the
second pass's measurement, and the coverage gap that became #1503. The
rest were defects in this change's own account of itself, including a
comment claiming every padding shape reaches the consumer intact, where
command substitution and the
jqrhelper eat a trailing newline beforethe comparison, and a count of failing tests wrong by one in the
direction that understated the evidence.
Co-authored-by: Claude Opus 5 (1M context) noreply@anthropic.com
Answers the two
spec/validate.pythreads Copilot raised against thedevelop -> mainpromotion PR #1489, whose head isdevelopand so cannot carry the fix itself.Part of the class filed as #1503, which this deliberately does not close.
🤖 Generated with Claude Code
https://claude.ai/code/session_01817rckKywjfoaEV5WF9TKs