fix(github): show queued ready_to_complete shards as ready for cutover

[Kiran01bm]

Why this matters

On Vitess, some operations are ready to cut over before they ever start copying rows. CREATE TABLE and DROP TABLE are "immediate" operations — Vitess marks them ready_to_complete=1 while their migration status still says queued. The overall deploy state already understood this: it counted those shards as waiting for cutover. But the per-shard display only honored the readiness flag when the shard's status was running.

The result was a progress comment that disagreed with itself: the table line said ready to cut over, while the shard line right under it still said "queued". An operator deciding whether it's safe to issue the cutover command shouldn't have to guess which line is telling the truth.

What it does

Extracts an effectiveShardState helper in the PlanetScale progress aggregation and folds ready_to_complete into the displayed shard state for all pre-terminal Vitess statuses (running, queued, requested, ready). Terminal statuses (complete, failed, cancelled) keep precedence, since ready_to_complete can remain set after a shard finishes or is cancelled. The table-level state inherits the fix, since it is derived from the resolved shard states.

queued immediate op (CREATE TABLE), ready_to_complete=1:

BEFORE                                      AFTER
──────────────────────────────────          ──────────────────────────────────
deploy state:  pending_cutover              deploy state:  pending_cutover
table line:    ✅ ready to cut over          table line:    ✅ ready to cut over
shard line:    queued          ✗ disagrees  shard line:    ready_to_complete  ✓

ready_to_complete is the authoritative cutover-readiness signal, and the deploy-state derivation already treats queued-but-ready shards as waiting for cutover. Resolving the displayed shard state from the same signal keeps the shard rows, the table state, and the deploy request state in agreement — no readiness or safety logic changes, display only.

🤖 Generated with Claude Code

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[Copilot]

Pull request overview

Aligns PlanetScale/Vitess per-shard progress display with the ready_to_complete cutover-readiness signal so shard rows, table state, and deploy request state don’t disagree for immediate operations that remain queued.

Changes:

• Extracts effectiveShardState to derive a display state that folds ready_to_complete into all pre-terminal Vitess statuses.
• Updates shard aggregation to use the derived state consistently.
• Adds/extends tests to cover queued-but-ready shards and the new helper.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
pkg/engine/planetscale/progress.go	Adds effectiveShardState and uses it during shard aggregation to ensure queued/requested/ready shards with ready_to_complete=1 display as ready_to_complete.
pkg/engine/planetscale/progress_test.go	Adds coverage for queued shards with/without ready_to_complete and unit-tests effectiveShardState.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[aparajon]

🤖 Adversarial correctness review, requested by Armand and performed by his agent. Reviewed at head ed3cb9d.

Verdict: correct and display-only — approving.

Findings

The fixture for the "stays queued" subtest feeds a raw status that Vitess never emits. ready_to_complete is a SchemaBot-derived state, not a raw migration_status; Status: "running", ReadyToComplete: true would exercise the same path with a realistic input.

Action items

1. (optional) Swap the "stays queued" fixture to Status: "running", ReadyToComplete: true so the test input matches what Vitess actually returns.

Verified (tried to break, couldn't)

effectiveShardState is an exact allowlist over the four non-terminal Vitess states (Running/Queued/Requested/Ready), which together with the three terminal states exhaust the status enum — unknown statuses fall through unchanged, so the display fails safe rather than promoting to ready; the change is strictly display-level: cutover authority remains the deploy-request state and the cutover trigger acts at deploy-request level, so a shard can't be shown ready where cutover would be rejected; ready→not-ready flapping is only possible in running, where the fold pre-existed; no tests were deleted (additive only); head already merges main and the only main-side overlap since merge-base touches a test file in another package; both Copilot terminology comments were fixed on a prior commit; no internal details in title/body/diff; all 32 CI checks pass on head.

This review was generated by Claude Code (claude-fable-5).

[aparajon]

🤖 Approving on Armand's behalf after the adversarial correctness review above (no blocking findings). This stamp was left by Claude Code (claude-fable-5).