Revert "feat(controller): pair with controller for channel/device pairing"#1179
Revert "feat(controller): pair with controller for channel/device pairing"#1179evanjacobson wants to merge 1 commit intomainfrom
Conversation
|
We will roll-forward if anything comes up in the post-review for #1095 |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| @@ -588,23 +586,6 @@ export class KiloClawInstance extends DurableObject<KiloClawEnv> { | |||
| // ── Lifecycle ─────────────────────────────────────────────────────── | |||
|
|
|||
| async start(userId?: string): Promise<void> { | |||
There was a problem hiding this comment.
WARNING: Concurrent start() calls can create duplicate machines
This method no longer has any in-flight guard, so startAsync() and a direct RPC start() can overlap after the first await. If both calls observe flyMachineId === null, they can each fall through to machine creation and orphan a duplicate Fly machine.
| } | ||
| } | ||
|
|
||
| await flyMachines.ensureVolume(flyConfig, this.ctx, this.s, this.env, 'start'); |
There was a problem hiding this comment.
WARNING: Missing metadata recovery reintroduces duplicate-machine starts
If DO storage loses flyMachineId but the user's machine still exists on Fly, start() now goes straight into volume/machine creation. The removed attemptMetadataRecovery(...) call was the safeguard that reattached to the existing machine instead of provisioning a second one.
| // so teardown wins. We bypass loadState() because it no-ops when already loaded. | ||
| const currentStatus = await this.ctx.storage.get('status'); | ||
| if (!currentStatus || currentStatus === 'destroying') { | ||
| if (currentStatus === 'destroying') { |
There was a problem hiding this comment.
WARNING: Deleted instances can be resurrected here
finalizeDestroyIfComplete() clears storage with deleteAll(), which makes ctx.storage.get('status') return undefined. This guard used to bail out on both undefined and 'destroying'; with only the 'destroying' check left, a background start() can repersist status: 'running' after the instance was fully deleted.
| }) | ||
| ); | ||
| } else { | ||
| // Transient Fly API error — leave in 'starting', alarm will retry. |
There was a problem hiding this comment.
WARNING: Timed-out starts can get stuck in starting forever
After STARTING_TIMEOUT_MS, a transient getMachine() failure now just logs and returns. That leaves startingAt intact and never transitions back to stopped, so the user cannot retry until Fly happens to respond again.
| sql`(${kiloclaw_subscriptions.status} IS NULL OR ${kiloclaw_subscriptions.status} NOT IN ('active', 'trialing'))`, | ||
| isNull(kiloclaw_subscriptions.suspended_at) | ||
| ) | ||
| sql`(${kiloclaw_subscriptions.status} IS NULL OR ${kiloclaw_subscriptions.status} NOT IN ('active', 'trialing'))` |
There was a problem hiding this comment.
WARNING: Suspended users are now included in earlybird warning emails
Dropping the suspended_at IS NULL filter means accounts already in the suspension/destruction flow also match this query. They'll get a claw_earlybird_* warning even though they're already suspended, which is a user-visible regression from the previous lifecycle behavior.
Code Review SummaryStatus: 5 Issues Found | Recommendation: Address before merge Overview
Fix these issues in Kilo Cloud Issue Details (click to expand)WARNING
Other Observations (not in diff)N/A Files Reviewed (17 files)
Reviewed by gpt-5.4-20260305 · 2,144,195 tokens |
1 participant
Reverts #1095
Auto-merge was configured and the approval triggered merge before Florian could review.