Review of df6029c8 (instance-catchall). Two readers: Astra (GPT-6, via codex, high effort) independently from a brief, and Claude Opus reading the core code. Every code citation below was checked against the source. Nothing was run: the tests could not start in Astra's read-only sandbox.
The design is sound. Deriving ordinary alias rows means the edge, certificate and hibernation code needed no change, and old binaries skip the claim safely. Not mergeable yet: two P1s and three P2s.
P1: a team's name can be taken before the team exists (wildcard DNS)
The catch-all's guidance points *.<parent> at the edge, and attach needs no DNS proof. So any account can attach acme.letsjelly.dev before Acme signs up. Its exact claim beats the derived name for good. With DNS already pointing at the edge, it gets a real certificate and serves its own content on Jelly's domain. A deleted team's label opens up to the same thing.
The spec calls this first-come (design §6 (c)). But the feature makes this setup the recommended one, and Jelly no longer claims per team, so it never sees the 409 the spec tells it to watch for.
Suggested fix: once the parent holder has proven DNS control, refuse other accounts every one-label name under a live catch-all, not just live instance names. An unproven parent stays open, which keeps your review finding 3.
P1: the guard checks only the current instance domain (Astra)
catchallInstanceName (internal/console/domaincatchall.go:175) checks <label>.<current manifest domain>. After an operator changes the manifest domain, or removes the manifest, instances under the old domain still derive names. Those names lose rule 4's protection, so another account can attach them. Fix: check existing instance records using the same eligibility rules as derivation.
P2: a duplicate label routes differently per box (Astra)
deriveInstanceAliases (internal/domains/aliases.go:140) picks a winner from local routes only. Put acme.old.example on box A and acme.new.example on box B: each box maps acme.<parent> to its own instance. Which database a request reaches then depends on the box it lands on. The comment's "the same on every box" does not hold across boxes. Fix: pick one winner from the deployment-wide instance set, or refuse duplicate labels.
P2: an old owner's detach can delete the new owner's catch-all (Astra)
DetachClaim reads the claim, checks the owner, then deletes without a condition (internal/domains/claims.go:327). Sequence: the parent moves from A to B. A's detach reads A's catch-all. B replaces it through AttachInstanceCatchall. A's delete then removes B's catch-all, and all of B's derived names go with it. Fix: keep the ETag from the read and use release.DeleteIfMatch.
P2: a route catch-all on one box beats an instance name on another (Astra)
lookupUpstream's wildcard fallback runs in the local lookup at internal/edge/edge.go:1397, before the backhaul to a sibling box. If box A serves the parent's route catch-all (woodstar's shape) and box B serves an instance's derived name, requests landing on A go to the parent app, even after fleet publication converges. Fix: prefer a known remote exact host over a local wildcard, with a two-box routing test.
The asks
- Rules 1 and 4 and the parent refusals: the refusals are thorough. They cover parents above, at and under every reserved name, public suffixes and the sidecar suffix. The weak points are the two P1s above.
--platform catch-all on a deployment zone: keep refusing it until there is a real need.- CLI spelling: agreed. The star in the host is the mode, and
--catchall stays a plain on/off flag. - "Delete now" leftovers: please open it. A re-created instance at the same host restoring the old instance's parked data crosses the boundary between the old team and the new one. That makes it more than clutter, and worth more than a footnote to #219.