Skip to content

fix(relay): accept production cells past c29 in the regional rehome operator - #22518

Merged
Jinwoo-H merged 1 commit into
mainfrom
relay-rehome-cell-id-cap
Sep 23, 2026
Merged

Jinwoo-H merged 1 commit into
mainfrom
relay-rehome-cell-id-cap

Conversation

@Jinwoo-H

@Jinwoo-H Jinwoo-H commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​24 0 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​24
Prod 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​3 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2

ELI5

The script that turns regional rehoming on checks that every cell name it is given looks right. That check only knew about cells numbered up to 29. Production now has cell 30, so the check refused a correct list and the enable step stopped.

What Changed

Before: running "Operate Relay Production Rehome" with mode=enable and the current general cell list failed at "Apply exact durable regional rehome enable" with selector membership is invalid. The monitor workflow accepted the same list, so the two workflows disagreed.

After: the operator script accepts any production-gce-c<N> from 1 to 99 written without a leading zero, so the list that includes c30 passes. It still refuses an empty list, duplicate cells, c0, c100, c09, and names from other environments.

Mechanism: the membership check in cloud/dev/scripts/operate-relay-regional-rehome.mjs used a pattern that spelled out 1-9 and 10-29. It now uses one named pattern for 1-99.

Why

A list of allowed numbers has to be edited every time a cell is added, and forgetting it blocks an audited production step. A two-digit bound still rejects typos and junk while leaving room for new cells. The director remains the real authority: the script compares the given list against the live selector before changing anything, so a looser shape check cannot enable a wrong membership.

Linked Issue

Fixes the failed enable in workflow run 35899848244.

Visual Proof

N/A. This is an operator script with no UI.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

A new test checks that c9, c29, c30 and c99 are accepted and that c0, c100, c09, a duplicate, a staging name, and an empty list are rejected. It fails against the old pattern and passes with the fix. The rehome operator, rehome workflow, and rehome trust probe test files all pass under node --test.

AI Disclosure

Review

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

No desktop, mobile, SSH, or cross-platform surface. The script only runs in the production rehome workflow.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

…perator

The selector membership check capped cell ids at c29, so enable failed
closed with "selector membership is invalid" once c30 went general.
Accept c1-c99 with no leading zero.

Claude-Session: ced32ebb-7155-4413-adad-1eccd14c2010

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

  • Widen the operator's cell shape check — operate-relay-regional-rehome.mjs replaces the inline c1–c29 regex with a named PRODUCTION_CELL_ID = /^production-gce-c[1-9][0-9]?$/, so production cells up to c99 parse while c0, c100, leading-zero, duplicate, empty, and non-production names still fail.
  • Pin the new boundary with a test — operate-relay-regional-rehome.test.mjs adds coverage that accepts c9/c29/c30/c99 and rejects c0/c100/c09/a duplicate/staging-gce-c30/an empty list.

I confirmed the PR's safety argument independently: the shape check is argument hygiene only, and operateRegionalRehome still requires exactMembership against the live selector before any mutation (operate-relay-regional-rehome.mjs:272-276), so the looser bound cannot enable a wrong membership. I also verified why the monitor accepted the same list and the operator did not — the monitor validates against its configured cell set rather than a shape regex (cloud/apps/relay-ops/src/incident-monitor-cli.ts:156-174). The new assertions do fail under the old pattern (c30/c99 would throw), and the file passes under node --test.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@coderabbitai

coderabbitai Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: f8a4ec2c-ad38-41d2-bcf0-f7ebd3af2e53

📥 Commits

Reviewing files that changed from the base of the PR and between 641a7f3 and b1c382b.

📒 Files selected for processing (2)
  • cloud/dev/scripts/operate-relay-regional-rehome.mjs
  • cloud/dev/scripts/operate-relay-regional-rehome.test.mjs

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The production cell validator now accepts IDs from production-gce-c1 through production-gce-c99, without leading zeroes. A new test checks accepted IDs and rejects zero, out-of-range, zero-padded, duplicate, staging, and empty membership values.

Priority: ⬇️ Low

Merge Risk: ⚪ Minimal · up to b1c38

The expanded production-cell range has no identified issue requiring a fix before merge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: allowing production cells beyond c29 in the regional rehome operator.
Description check ✅ Passed The description is complete and focused. It explains the problem, behavior before and after, implementation, rationale, testing, and compatibility considerations. The Linked Issue section names workfl…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

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.

@Jinwoo-H
Jinwoo-H merged commit 1116c54 into main Sep 23, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant