Skip to content

refactor: remove basin creating state#333

Merged
shikhar merged 4 commits intomainfrom
codex/remove-basin-creating-state
Mar 19, 2026
Merged

refactor: remove basin creating state#333
shikhar merged 4 commits intomainfrom
codex/remove-basin-creating-state

Conversation

@shikhar
Copy link
Member

@shikhar shikhar commented Mar 19, 2026

  • remove Creating from the basin state enums in common, api, and sdk
  • update CLI/TUI basin state rendering and basin creation messaging to treat created basins as active
  • add assertions that created basins return Active in lite backend and SDK tests

@shikhar shikhar marked this pull request as ready for review March 19, 2026 18:24
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 19, 2026

Greptile Summary

This PR removes the Creating basin state across the entire stack — common, api, sdk, CLI, and TUI — reflecting a backend change where basins are now immediately Active upon creation. Complementary assertions are added to the lite backend and SDK integration tests to verify this new contract.

  • BasinState::Creating is removed from common/src/types/basin.rs, api/src/v1/basin.rs, and sdk/src/types.rs, along with all associated From conversion arms
  • CLI output for create_basin is updated so a successful creation always shows "✓ Basin created" (green) instead of the previous "✓ Basin creation requested" (yellow) message
  • TUI's BADGE_PENDING color constant and the Creating badge render arm are removed from cli/src/tui/ui.rs
  • lite/tests/backend/control_plane/basin.rs now captures the return value of create_basin and asserts it is CreatedOrReconfigured::Created with BasinState::Active for both the initial and idempotent creation calls
  • sdk/tests/account_ops.rs adds assert_eq!(basin_info.state, BasinState::Active) to the create_list_and_delete_basin integration test
  • A grep over the entire repo confirms zero remaining references to BasinState::Creating — the removal is complete

Confidence Score: 4/5

  • This PR is safe to merge; it is a clean, complete removal of a deprecated state with no leftover references and good test coverage.
  • All references to Creating are removed consistently across every layer (common, api, sdk, cli, tui). New test assertions confirm the Active-on-creation contract in both the lite backend and SDK tests. The only minor concern is that BasinState in sdk/src/types.rs is missing #[non_exhaustive] while the surrounding types all have it, which could lead to breaking SDK consumers when the next state is added.
  • sdk/src/types.rsBasinState is the only public enum without #[non_exhaustive]

Important Files Changed

Filename Overview
common/src/types/basin.rs Removes the Creating variant from the BasinState enum; clean and complete.
api/src/v1/basin.rs Removes Creating from the API-layer BasinState enum and its From conversion; clean.
sdk/src/types.rs Removes Creating variant from SDK's BasinState enum; BasinState is the only public enum in this file without #[non_exhaustive], which risks breaking SDK consumers if new states are added later.
lite/tests/backend/control_plane/basin.rs Adds assertions that create_basin returns CreatedOrReconfigured::Created with BasinState::Active for both initial and idempotent creates; good coverage.
sdk/tests/account_ops.rs Adds assert_eq!(basin_info.state, BasinState::Active) after creation to verify the new contract at the SDK integration-test level.

Sequence Diagram

sequenceDiagram
    participant Client
    participant SDK
    participant API
    participant Backend

    Client->>SDK: create_basin(name, config)
    SDK->>API: POST /basins (CreateOrReconfigureBasinRequest)
    API->>Backend: create_basin(name, config, CreateMode)
    Note over Backend: Basin created atomically,<br/>immediately Active
    Backend-->>API: CreatedOrReconfigured::Created(BasinInfo { state: Active })
    API-->>SDK: BasinInfo { state: "active" }
    SDK-->>Client: BasinInfo { state: BasinState::Active }
    Note over Client: ✓ Basin created (green)
Loading

Comments Outside Diff (1)

  1. sdk/src/types.rs, line 1124-1131 (link)

    P2 BasinState missing #[non_exhaustive]

    Nearly every other public type in this file is annotated with #[non_exhaustive] (e.g., BasinInfo at line 1143 directly below), but BasinState is not. This means any external SDK consumer that exhaustively matches on BasinState will have a compile error the next time a new state variant is added. Since this PR is already touching this type (removing Creating), this is a good moment to add the attribute.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: sdk/src/types.rs
    Line: 1124-1131
    
    Comment:
    **`BasinState` missing `#[non_exhaustive]`**
    
    Nearly every other public type in this file is annotated with `#[non_exhaustive]` (e.g., `BasinInfo` at line 1143 directly below), but `BasinState` is not. This means any external SDK consumer that exhaustively matches on `BasinState` will have a compile error the next time a new state variant is added. Since this PR is already touching this type (removing `Creating`), this is a good moment to add the attribute.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: sdk/src/types.rs
Line: 1124-1131

Comment:
**`BasinState` missing `#[non_exhaustive]`**

Nearly every other public type in this file is annotated with `#[non_exhaustive]` (e.g., `BasinInfo` at line 1143 directly below), but `BasinState` is not. This means any external SDK consumer that exhaustively matches on `BasinState` will have a compile error the next time a new state variant is added. Since this PR is already touching this type (removing `Creating`), this is a good moment to add the attribute.

```suggestion
#[derive(Debug, Clone, PartialEq, Eq)]
#[non_exhaustive]
/// Current state of a basin.
pub enum BasinState {
    /// Active
    Active,
    /// Deleting
    Deleting,
}
```

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: "Merge branch 'main' ..."

@shikhar shikhar merged commit 7871287 into main Mar 19, 2026
11 checks passed
@shikhar shikhar deleted the codex/remove-basin-creating-state branch March 19, 2026 18:45
@s2-release-plz s2-release-plz bot mentioned this pull request Mar 19, 2026
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