Hold a deploy suite key to an alphabet the key list reads back as

README.md

@@ -101,10 +101,15 @@ Suites are a **registry the abstract iterates**, not a chain of `else if`.
 Adding a suite is adding an array entry. A mistyped `DEPLOYMENT_SUITE` reports
 the valid keys built from that same array, so the error cannot fall behind the
 suites it describes, and keys are checked unique because the key is what selects
-what gets broadcast. They are checked **non-empty** for the same reason from the
-other side: the empty string is what an unset `DEPLOYMENT_SUITE` arrives as, so
-leaving it declarable would let a dispatch with the suite input blank select
-something instead of reporting that it was told nothing.
+what gets broadcast. They are also checked against an **alphabet**: kebab case,
+optionally followed by `@` and a release tag, which is the shape a repo writes
+by hand and the shape a cut release generates. A key outside it is one the
+reported list cannot be read back as — a comma or a space in a key renders as
+two keys and sends a reader after one that is declared nowhere — and the empty
+key falls to the same rule, which matters most: the empty string is what an
+unset `DEPLOYMENT_SUITE` arrives as, so leaving it declarable would let a
+dispatch with the suite input blank select something instead of reporting that
+it was told nothing.
 
 Every suite is individually selectable, including a frozen release — which is
 how a snapshot from before a network existed reaches that network.

src/abstract/RainDeploySuitesBase.sol

@@ -7,24 +7,46 @@ pragma solidity ^0.8.25;
 /// @param suite The key declared more than once.
 error DuplicateDeploySuite(string suite);
 
-/// Thrown when a suite declares an EMPTY key.
+/// Thrown when a suite declares a key outside the key alphabet.
 ///
-/// The empty string is the absent-sentinel: `RainDeployBroadcast.run()` reads
-/// `DEPLOYMENT_SUITE` through `vm.envOr` with `string("")` as the default, so a
-/// dispatch that left the suite input blank asks the registry for exactly this
-/// key. A declaration free to answer it turns "told nothing" into a selection,
-/// and `CREATE2` under a zero salt puts those bytes at their own permanent
-/// address on every chain that dispatch reached.
+/// A key is a NAME, or a name and a release TAG joined by an at sign: the name
+/// lowercase letters and hyphens, the tag those plus digits and underscores,
+/// neither half empty and at most one at sign. Digits and underscores after the
+/// at sign only, which is where a tag needs them and where nothing else does.
 ///
-/// Reserved on the DECLARATION rather than beside the substitution, for the
-/// reason `NoDeployCandidates` is: a rule bound in the consumer is a rule most
+/// That is the kebab case a repo declares by hand, and it is what
+/// `LibRainDeploySnapshot` emits a released entry as — the candidate key the
+/// release was cut from, the at sign, and the record directory's tag, which
+/// `isTag` already holds to `X_Y_Z` — so a generated declaration needs no
+/// exemption from the rule a hand written one is held to. The at sign cannot
+/// appear in a name, so that emission carries exactly one however many releases
+/// a repo cuts.
+///
+/// The key list is why an alphabet exists at all. `suiteNames()` joins the
+/// declared keys on `", "` for `UnknownDeploymentSuite`, which exists so a
+/// caller who does NOT already know the valid keys is told them; a key free to
+/// carry either of those characters renders as two, so the reader is told a
+/// different number of suites exist than do and is sent after a key that is
+/// declared nowhere. A list that cannot be read back as the set it names is the
+/// hardcoded string the registry exists to replace, spelled differently.
+///
+/// The EMPTY key is refused by the same rule, and is the one refusal with a
+/// broadcast behind it: it is the value `RainDeployBroadcast.run()` substitutes
+/// for an absent `DEPLOYMENT_SUITE`, so a declaration allowed to answer it
+/// turns "told nothing" into a selection, and `CREATE2` under a zero salt puts
+/// those bytes at their own permanent address on every chain that dispatch
+/// reached. An alphabet admitting no zero byte key already says that; a length
+/// check beside it would be a second rule for one property, free to drift.
+///
+/// Held on the DECLARATION rather than beside the substitution, for the reason
+/// `NoDeployCandidates` is: a rule bound in the consumer is a rule most
 /// consumers do not run, and here every `suiteByName` caller pays for it rather
-/// than only `run()`. Uniqueness does not already cover it — a lone empty key
-/// collides with nothing.
+/// than only `run()`.
 /// @param index Position in `allSuites()`: the released suites in declaration
-/// order, then the candidates. The key itself names nothing, so the position is
+/// order, then the candidates. An empty key names nothing, so the position is
 /// the only thing that can.
-error EmptyDeploySuiteKey(uint256 index);
+/// @param suite The refused key.
+error InvalidDeploySuiteKey(uint256 index, string suite);
 
 /// Thrown when `DEPLOYMENT_SUITE` names no declared suite. Carries the valid
 /// keys, because the whole point of a registry is that the answer is not one
@@ -79,11 +101,11 @@ error CandidateSourceMismatch(string suite, bytes32 storedCreationCodeHash, byte
 /// make that comparison derived-against-derived, and a guard that compares a
 /// value to itself is not a guard.
 struct DeploySuite {
-    /// The key. Unique across every suite a repo declares, and never empty: it
-    /// is what `DEPLOYMENT_SUITE` selects for broadcasting, and the label every
-    /// verification error names. The empty string is the value an unset
-    /// `DEPLOYMENT_SUITE` arrives as, so it is reserved rather than declarable
-    /// — see `EmptyDeploySuiteKey`.
+    /// The key. Unique across every suite a repo declares, and held to the key
+    /// alphabet — see `InvalidDeploySuiteKey`. It is what
+    /// `DEPLOYMENT_SUITE` selects for broadcasting, the label every
+    /// verification error names, and what the valid-key list an unknown suite
+    /// reports has to read back as.
     ///
     /// A repo with one contract and several frozen releases gives each release
     /// its own key, because each is separately deployable — a chain added after
@@ -253,6 +275,43 @@ abstract contract RainDeploySuitesBase {
         }
     }
 
+    /// Refuses a key the registry cannot carry — see `InvalidDeploySuiteKey`.
+    ///
+    /// Held against the WHOLE key rather than against a name a released key is
+    /// derived from, because `releasedSuites()` declares finished keys: a rule
+    /// that only knew the name half could not be asked about a released entry
+    /// at all, and this is the one place every key a repo declares is read.
+    /// @param index Position in `allSuites()`, for the refusal to name.
+    /// @param suite The key to check.
+    function checkSuiteKey(uint256 index, string memory suite) internal pure {
+        bytes memory key = bytes(suite);
+        // Where the tag starts, and zero until an `@` is seen. Zero is not a
+        // position a tag can start at, because an `@` opening the key leaves
+        // the name half empty and is refused below.
+        uint256 tagStart = 0;
+
+        for (uint256 i = 0; i < key.length; i++) {
+            bytes1 char = key[i];
+            if (char == "@") {
+                if (i == 0 || tagStart != 0) {
+                    revert InvalidDeploySuiteKey(index, suite);
+                }
+                tagStart = i + 1;
+            } else {
+                bool nameAlphabet = (char >= "a" && char <= "z") || char == "-";
+                bool tagAlphabet = (char >= "0" && char <= "9") || char == "_";
+                if (!(nameAlphabet || (tagStart != 0 && tagAlphabet))) {
+                    revert InvalidDeploySuiteKey(index, suite);
+                }
+            }
+        }
+
+        // The empty key and a trailing at sign: the loop only refuses bytes that are there.
+        if (tagStart == key.length) {
+            revert InvalidDeploySuiteKey(index, suite);
+        }
+    }
+
     /// Every suite this repo declares: the released ones followed by the
     /// candidates. This is the verification set and the deploy registry, which
     /// are the same set because they are the same declaration.
@@ -261,13 +320,13 @@ abstract contract RainDeploySuitesBase {
     /// pay for the check and neither can be handed a registry that is ambiguous
     /// or that answers the absent-sentinel. One pass over the whole set, so a
     /// candidate colliding with another candidate is caught by the same code
-    /// that catches a candidate colliding with a release, and an empty key is
-    /// refused wherever in the declaration it was spelled — there is no second
-    /// rule to keep in step.
+    /// that catches a candidate colliding with a release, and a key the
+    /// alphabet refuses is refused wherever in the declaration it was spelled —
+    /// there is no second rule to keep in step.
     ///
-    /// Unique AND non-empty, because neither implies the other: a lone empty
-    /// key collides with nothing, and it is the one key `run()` can be handed
-    /// by accident.
+    /// Unique AND in the alphabet, because neither implies the other: a lone
+    /// unreadable key collides with nothing, and two keys can be spelled
+    /// perfectly and still be the same key.
     /// @return Every declared suite.
     function allSuites() internal pure returns (DeploySuite[] memory) {
         DeploySuite[] memory released = releasedSuites();
@@ -282,9 +341,7 @@ abstract contract RainDeploySuitesBase {
         }
 
         for (uint256 i = 0; i < suites.length; i++) {
-            if (bytes(suites[i].suite).length == 0) {
-                revert EmptyDeploySuiteKey(i);
-            }
+            checkSuiteKey(i, suites[i].suite);
             for (uint256 j = i + 1; j < suites.length; j++) {
                 if (keccak256(bytes(suites[i].suite)) == keccak256(bytes(suites[j].suite))) {
                     revert DuplicateDeploySuite(suites[i].suite);
@@ -296,6 +353,10 @@ abstract contract RainDeploySuitesBase {
     }
 
     /// Every declared key, comma separated, for the unknown-suite error.
+    ///
+    /// Splitting the result on `", "` recovers exactly the declared keys,
+    /// because the alphabet `allSuites` holds them to carries neither of those
+    /// two characters anywhere but between two keys.
     /// @return The declared keys.
     function suiteNames() internal pure returns (string memory) {
         DeploySuite[] memory suites = allSuites();

test/abstract/ExampleDeploySuites.sol

@@ -46,7 +46,7 @@ abstract contract ExampleDeploySuites is RainDeploySuitesBase {
     function releasedSuites() internal pure override returns (DeploySuite[] memory suites) {
         suites = new DeploySuite[](2);
         suites[0] = DeploySuite({
-            suite: "address-registry-0-0-1",
+            suite: "address-registry@0_0_1",
             creationCode: ADDRESS_REGISTRY_CREATION_CODE,
             storedDeployedAddress: ADDRESS_REGISTRY_DEPLOYED_ADDRESS,
             storedBytecodeHash: ADDRESS_REGISTRY_BYTECODE_HASH,

test/abstract/ExternalDeploySuites.sol

@@ -30,6 +30,10 @@ abstract contract ExternalDeploySuites is RainDeploySuitesBase {
         return suiteNames();
     }
 
+    function externalCheckSuiteKey(uint256 index, string memory suite) external pure {
+        checkSuiteKey(index, suite);
+    }
+
     /// @return The declared candidates, refusing an empty list.
     function externalCheckedCandidateSuites() external pure returns (DeployCandidate[] memory) {
         return checkedCandidateSuites();

test/concrete/EmptyKeyDeploySuites.sol

@@ -34,7 +34,7 @@ contract EmptyKeyDeploySuites is ExternalDeploySuites {
     function releasedSuites() internal pure override returns (DeploySuite[] memory) {
         DeploySuite[] memory suites = new DeploySuite[](1);
         suites[0] = DeploySuite({
-            suite: "address-registry-0-0-1",
+            suite: "address-registry@0_0_1",
             creationCode: ADDRESS_REGISTRY_CREATION_CODE,
             storedDeployedAddress: ADDRESS_REGISTRY_DEPLOYED_ADDRESS,
             storedBytecodeHash: ADDRESS_REGISTRY_BYTECODE_HASH,

test/concrete/NoCandidateDeploySuites.sol

@@ -24,7 +24,7 @@ contract NoCandidateDeploySuites is ExternalDeploySuites {
     function releasedSuites() internal pure override returns (DeploySuite[] memory suites) {
         suites = new DeploySuite[](1);
         suites[0] = DeploySuite({
-            suite: "address-registry-0-0-1",
+            suite: "address-registry@0_0_1",
             creationCode: ADDRESS_REGISTRY_CREATION_CODE,
             storedDeployedAddress: ADDRESS_REGISTRY_DEPLOYED_ADDRESS,
             storedBytecodeHash: ADDRESS_REGISTRY_BYTECODE_HASH,

test/concrete/SeparatorKeyDeploySuites.sol

@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: LicenseRef-DCL-1.0
+// SPDX-FileCopyrightText: Copyright (c) 2020 Rain Open Source Software Ltd
+pragma solidity =0.8.25;
+
+import {DeployCandidate, DeploySuite, RainDeploySuitesBase} from "../../src/abstract/RainDeploySuitesBase.sol";
+import {ExternalDeploySuites} from "../abstract/ExternalDeploySuites.sol";
+import {MockDeployableV2} from "./MockDeployableV2.sol";
+import {LibRainDeploy} from "../../src/lib/LibRainDeploy.sol";
+import {
+    BYTECODE_HASH as ADDRESS_REGISTRY_BYTECODE_HASH,
+    CREATION_CODE as ADDRESS_REGISTRY_CREATION_CODE,
+    DEPLOYED_ADDRESS as ADDRESS_REGISTRY_DEPLOYED_ADDRESS,
+    RUNTIME_CODE as ADDRESS_REGISTRY_RUNTIME_CODE
+} from "../../src/generated/candidate/AddressRegistry.sol";
+
+contract SeparatorKeyDeploySuites is ExternalDeploySuites {
+    /// @inheritdoc RainDeploySuitesBase
+    function releasedSuites() internal pure override returns (DeploySuite[] memory) {
+        DeploySuite[] memory suites = new DeploySuite[](1);
+        suites[0] = DeploySuite({
+            suite: "a,b",
+            creationCode: ADDRESS_REGISTRY_CREATION_CODE,
+            storedDeployedAddress: ADDRESS_REGISTRY_DEPLOYED_ADDRESS,
+            storedBytecodeHash: ADDRESS_REGISTRY_BYTECODE_HASH,
+            storedRuntimeCode: ADDRESS_REGISTRY_RUNTIME_CODE,
+            artifactPath: "src/concrete/AddressRegistry.sol:AddressRegistry",
+            dependencies: new address[](0)
+        });
+        return suites;
+    }
+
+    /// @inheritdoc RainDeploySuitesBase
+    function candidateSuites() internal pure override returns (DeployCandidate[] memory) {
+        DeployCandidate[] memory candidates = new DeployCandidate[](1);
+        candidates[0] = DeployCandidate({
+            snapshot: DeploySuite({
+                suite: "c",
+                creationCode: type(MockDeployableV2).creationCode,
+                storedDeployedAddress: LibRainDeploy.zoltuAddress(type(MockDeployableV2).creationCode),
+                storedBytecodeHash: keccak256(type(MockDeployableV2).runtimeCode),
+                storedRuntimeCode: type(MockDeployableV2).runtimeCode,
+                artifactPath: "test/concrete/MockDeployableV2.sol:MockDeployableV2",
+                dependencies: new address[](0)
+            }),
+            sourceCreationCode: type(MockDeployableV2).creationCode
+        });
+        return candidates;
+    }
+}

test/src/abstract/RainDeployBroadcast.t.sol

@@ -148,7 +148,7 @@ contract RainDeployBroadcastTest is Test {
             abi.encodeWithSelector(
                 UnknownDeploymentSuite.selector,
                 "",
-                "address-registry-0-0-1, second-address, address-registry-candidate, second-address-candidate"
+                "address-registry@0_0_1, second-address, address-registry-candidate, second-address-candidate"
             )
         );
         sDeploy.run();
@@ -159,7 +159,7 @@ contract RainDeployBroadcastTest is Test {
             abi.encodeWithSelector(
                 UnknownDeploymentSuite.selector,
                 "address-registry",
-                "address-registry-0-0-1, second-address, address-registry-candidate, second-address-candidate"
+                "address-registry@0_0_1, second-address, address-registry-candidate, second-address-candidate"
             )
         );
         sDeploy.run();
@@ -431,11 +431,11 @@ contract RainDeployBroadcastTest is Test {
     /// make that comparison derived-against-derived.
     function testSelectedSuiteCarriesTheRecordedPins() external view {
         assertEq(
-            sDeploy.externalSuiteByName("address-registry-0-0-1").storedDeployedAddress,
-            LibRainDeploy.zoltuAddress(sDeploy.externalSuiteByName("address-registry-0-0-1").creationCode)
+            sDeploy.externalSuiteByName("address-registry@0_0_1").storedDeployedAddress,
+            LibRainDeploy.zoltuAddress(sDeploy.externalSuiteByName("address-registry@0_0_1").creationCode)
         );
         assertEq(
-            sDeploy.externalSuiteByName("address-registry-0-0-1").artifactPath,
+            sDeploy.externalSuiteByName("address-registry@0_0_1").artifactPath,
             "src/concrete/AddressRegistry.sol:AddressRegistry"
         );
     }