Remove tailnet option

[lucacome]

Summary by CodeRabbit

• Refactor

  • Removed Tailnet configuration option across CLI, config, and client setup.

• Bug Fixes

  • Eliminated redundant local status retrieval during initialization.

• New Features

  • Added explicit dry-run handling with clearer user-facing messages.

• Documentation

  • Updated README examples to remove the Tailnet field.

[coderabbitai]

📝 Walkthrough

Walkthrough

Removed the Tailnet configuration field and its CLI flags/env reference across the codebase and tsapi.Client initializations; added centralized dry-run error handling and messaging in instance creation; reordered local Tailscale status retrieval in status handling.

Changes

Cohort / File(s)	Summary
CI & Docs /.github/workflows/ci.yaml, README.md	Removed TAILOUT_TAILSCALE_TAILNET env reference from CI and deleted example tailnet: entries in README YAML snippets.
CLI Flag removals cmd/connect.go, cmd/create.go, cmd/init.go, cmd/status.go, cmd/stop.go	Removed registration/binding of --tailscale-tailnet flag from multiple CLI commands.
Config struct tailout/config/config.go	Removed Tailnet string field from TailscaleConfig.
tsapi.Client initializations tailout/connect.go, tailout/disconnect.go, tailout/init.go, tailout/stop.go, tailout/ui.go	Omitted the Tailnet: assignment when constructing tsapi.Client instances (now only APIKey and BaseURL).
Create command changes tailout/create.go	Added public error ErrDryRun, wrapped ErrUserAborted in var block, adjusted user-facing messages to lowercase, and propagate/handle dry-run by returning ErrDryRun after successful create when dry-run is set. Removed Tailnet from client init.
Status handling tailout/status.go	Moved local Tailscale status retrieval to before tsapi.Client construction and removed a duplicated local status retrieval block.

Sequence Diagram(s)

sequenceDiagram
  participant UI as User/UI
  participant App as App (create)
  participant Spinner as Spinner
  participant EC2 as EC2 API
  participant TSAPI as tsapi.Client

  UI->>App: request create (dryRun?)
  App->>Spinner: start spinner and createInstance
  Spinner->>EC2: CreateInstance (DryRun if set)
  EC2-->>Spinner: success (or DryRun success)
  Spinner-->>App: createInstance returned
  alt dryRun == true
    App-->>App: return ErrDryRun
    App->>UI: print "dry run successful" message
  else
    App->>TSAPI: initialize client (APIKey, BaseURL)
    App->>TSAPI: register device / post-creation steps
    App->>UI: normal completion
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

Changes are consistent removals of a config field/flag across many files (low individual complexity) plus several logic edits (dry-run handling and status retrieval reorder) that require focused review.

Possibly related PRs

• Replace promptui with huh #82 — modifies the same Tailnet-related client initialization and CLI flag areas; likely overlapping changes and potential conflict.

Poem

🐰 small paws tap the code tonight,
tailnet lines step out of sight,
dry-run whispers, tests run true,
one less flag — a lighter view.

Pre-merge checks and finishing touches

❌ 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%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "Remove tailnet option" accurately reflects the primary change throughout the changeset. The PR systematically removes the tailnet functionality across multiple layers: removing the --tailscale-tailnet CLI flag from five commands (connect, create, init, status, stop), removing the Tailnet field from the TailscaleConfig struct, and removing Tailnet assignments from tsapi.Client initialization in seven files. The title is concise, specific, and clear enough that a developer reviewing PR history would immediately understand the main objective. While there are secondary changes (like refactored status retrieval and enhanced dry-run handling in create.go), the pervasive tailnet removal is undoubtedly the dominant change.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/tailnet

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1e553b3 and f3abbad.

📒 Files selected for processing (1)

• tailout/status.go (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• tailout/status.go

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: binary

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

tailout/create.go (1)

128-143: Consider consolidating dry-run error handling.

The dry-run flow works but involves multiple layers:

1. AWS SDK returns a DryRunOperation error
2. createInstance catches it and returns nil error (lines 316-319)
3. The spinner action then checks the dryRun flag and returns ErrDryRun (lines 128-130)
4. Finally, ErrDryRun is caught and converted to a success message (lines 140-143)

Consider having createInstance return ErrDryRun directly when it detects the AWS dry-run error. This would eliminate the need to check the dryRun flag in the spinner action:

In createInstance function (around line 316-319):

 	if runErr != nil {
 		var dryRunErr *smithy.GenericAPIError
 		if errors.As(runErr, &dryRunErr) && dryRunErr.Code == "DryRunOperation" {
-			return instance, nil
+			return instance, ErrDryRun
 		}
 		return instance, fmt.Errorf("failed to create EC2 instance: %w", runErr)
 	}

Then remove the dry-run check from the spinner action:

 	errSpin := s.Context(ctx).ActionWithErr(func(context.Context) error {
 		instance, createErr := createInstance(ctx, cfg, runInput, s)
 		if createErr != nil {
 			return createErr
 		}
-		if dryRun {
-			return ErrDryRun
-		}
 		if instance.InstanceID == "" {
 			return errors.New("no instance ID returned")
 		}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9e063da and 1e553b3.

📒 Files selected for processing (15)

• .github/workflows/ci.yaml (0 hunks)
• README.md (0 hunks)
• cmd/connect.go (0 hunks)
• cmd/create.go (0 hunks)
• cmd/init.go (0 hunks)
• cmd/status.go (0 hunks)
• cmd/stop.go (0 hunks)
• tailout/config/config.go (0 hunks)
• tailout/connect.go (0 hunks)
• tailout/create.go (3 hunks)
• tailout/disconnect.go (1 hunks)
• tailout/init.go (0 hunks)
• tailout/status.go (1 hunks)
• tailout/stop.go (0 hunks)
• tailout/ui.go (0 hunks)

💤 Files with no reviewable changes (12)

• README.md
• cmd/connect.go
• cmd/status.go
• tailout/config/config.go
• .github/workflows/ci.yaml
• tailout/ui.go
• tailout/stop.go
• tailout/init.go
• cmd/init.go
• cmd/stop.go
• tailout/connect.go
• cmd/create.go

🔇 Additional comments (4)

tailout/disconnect.go (1)

18-21: LGTM!

The removal of the Tailnet field from the API client initialization is clean and aligns with the PR objective to remove the Tailnet option.

tailout/create.go (3)

27-30: LGTM!

The addition of ErrDryRun improves error handling by providing a distinct error type for successful dry-run operations, allowing for more specific messaging and control flow.

---

44-47: LGTM!

The removal of the Tailnet field from the API client initialization is consistent with the PR objective and aligns with similar changes across other files.

---

109-109: LGTM!

The lowercase change improves consistency in user-facing error messages.