Skip to content

CLI deploy: random default names and taken-name suggestion#129

Merged
antidmg merged 1 commit intomainfrom
cli-deploy-name-conflict-suggestion
Mar 9, 2026
Merged

CLI deploy: random default names and taken-name suggestion#129
antidmg merged 1 commit intomainfrom
cli-deploy-name-conflict-suggestion

Conversation

@antidmg
Copy link
Copy Markdown
Contributor

@antidmg antidmg commented Mar 9, 2026

Summary

  • use a generated name when deploy has no explicit name and no local sync state
  • keep upsert behavior when local sync state exists
  • return a clear error with a suggested name when the requested name is already taken

Summary by CodeRabbit

  • New Features

    • Auto-generate and cache application names for first-time creates; subsequent syncs reuse the cached name.
    • Deployment now explicitly differentiates Create vs Update flows for clearer outcomes.
  • Improvements

    • Improved error messages for name conflicts with actionable suggestions.
    • More consistent API error reporting for clearer failure details.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 9, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 593376e0-d018-4df3-8bca-4ae3a2a25ca1

📥 Commits

Reviewing files that changed from the base of the PR and between 7ff28b7 and bf072ff.

📒 Files selected for processing (4)
  • binaries/statespace-cli/src/args.rs
  • binaries/statespace-cli/src/commands/sync.rs
  • binaries/statespace-cli/src/error.rs
  • binaries/statespace-cli/src/gateway/client.rs

Walkthrough

Refactors CLI sync flow to support explicit Create vs Upsert paths via a new SyncGateway trait, adds target resolution and name-handling (including random/default name behavior), enriches API error parsing with ApiErrorCode, and updates gateway response parsing to attach error codes.

Changes

Cohort / File(s) Summary
CLI args doc
binaries/statespace-cli/src/args.rs
Updated AppSyncArgs.name documentation to reflect default: randomly generated on first run and reused from .statespace/state.json.
Sync command + tests
binaries/statespace-cli/src/commands/sync.rs
Replaced Upserter with SyncGateway (adds create_application), added DeployMode, SyncTarget, SyncResult, resolve_target, map_create_error, updated run_sync control flow to Create vs Upsert, and extended tests / mock to record create vs upsert and simulate name-taken scenarios.
API error types & parsing
binaries/statespace-cli/src/error.rs
Introduced ApiErrorCode enum and parsing helpers; changed GatewayError::Api to include ApiErrorCode; added parsing helpers (parse_api_error_fields, is_name_taken_message) and display formatting.
Gateway client response handling
binaries/statespace-cli/src/gateway/client.rs
On JSON parse or data-deserialize failures, attach ApiErrorCode::Unknown("invalid_response") to GatewayError::Api so invalid/unknown responses include a code.

Sequence Diagram(s)

sequenceDiagram
    participant CLI as CLI (run_sync)
    participant State as Cached State (.statespace/state.json)
    participant Gateway as Gateway (SyncGateway)
    CLI->>State: read cached state
    State-->>CLI: cached name / none
    CLI->>CLI: resolve_target(explicit_name, cached)
    CLI->>Gateway: (Create) create_application(name, files)
    Gateway-->>CLI: DeployResult / name-taken error
    alt name-taken
        CLI->>CLI: map_create_error(name-taken -> suggestion)
    else created
        CLI->>State: write SyncResult (name,id,url,auth_token)
    end
    alt DeployMode == Upsert
        CLI->>Gateway: upsert_application(files)
        Gateway-->>CLI: UpsertResult
        CLI->>State: write SyncResult
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 23.53% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The pull request description lacks the required structure from the template, including missing 'Changes' section details, 'Testing' checklist, and 'Docs updated' confirmation. Expand the description to follow the template: add detailed 'Changes' section explaining the implementation, complete the 'Testing' checklist with verification results, and confirm whether documentation was updated.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: introducing random default names for CLI deploy and adding taken-name suggestions.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch cli-deploy-name-conflict-suggestion

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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@binaries/statespace-cli/src/args.rs`:
- Around line 134-136: The doc comment for the CLI option represented by the
struct field `name` is misleading; update the doc string above `pub name:
Option<String>` (the `name` arg definition) to state that the application name
is randomly generated on first run and then cached in `.statespace/state.json`
for subsequent deploys (i.e., "default: randomly generated on first run, then
reused from .statespace/state.json"), so the help text accurately reflects
steady-state behavior.

In `@binaries/statespace-cli/src/commands/sync.rs`:
- Around line 187-193: The fallback name generation currently only ensures
suggestion != name but never verifies availability; update the logic around
generate_name/suggestion to loop generating new suggestions until a proper
availability check (reuse the existing name-validation/gateway method used
elsewhere in this module—e.g., the function that checks name availability or the
same RPC used for initial validation) returns true, then use that verified
suggestion in the Error::cli message so the CLI suggests a name guaranteed to be
free.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 48156217-b37b-49ee-a82b-bd9045c1a40f

📥 Commits

Reviewing files that changed from the base of the PR and between 569722f and 7ff28b7.

📒 Files selected for processing (2)
  • binaries/statespace-cli/src/args.rs
  • binaries/statespace-cli/src/commands/sync.rs

Comment thread binaries/statespace-cli/src/args.rs Outdated
Comment thread binaries/statespace-cli/src/commands/sync.rs
@antidmg antidmg marked this pull request as draft March 9, 2026 23:25
@antidmg antidmg force-pushed the cli-deploy-name-conflict-suggestion branch from 3035606 to bf072ff Compare March 9, 2026 23:30
@antidmg antidmg marked this pull request as ready for review March 9, 2026 23:32
@antidmg antidmg merged commit ea93315 into main Mar 9, 2026
5 of 6 checks passed
@antidmg antidmg deleted the cli-deploy-name-conflict-suggestion branch March 9, 2026 23:32
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