Lio/stacks

[lionello]

Description

• Fixed a panic (no provider => GetRegionVarName panics)
• Could not ctrl-c from interactive stacks query, would still deploy
• Fill in missing fields from remote stack info (empty stack file)
• More consistent error messages

Linked Issues

Checklist

• I have performed a self-review of my code
• I have added appropriate tests
• I have updated the Defang CLI docs and/or README to reflect my changes, if necessary

Summary by CodeRabbit

• Bug Fixes

  • Fixed session-based stack handling to ensure per-session stack mode is used instead of global references, improving reliability in multi-session scenarios.
  • Improved error handling when default stacks are not configured.

• New Features

  • Stack creation is now mandatory for deployment operations.
  • Deployment lists now indicate "active" status when applicable.

• Improvements

  • Enhanced error messages for invalid provider and mode values.
  • Stack parameters are now automatically populated from remote sources when missing.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR refactors stack management across the codebase by shifting from global stack references to per-session stack handling, renames AllowCreate to AllowStackCreation in SelectStackOptions, introduces the ErrDefaultStackNotSet error constant, updates PlaygroundProvider with a constructor, improves error messages, and makes region extraction conditional on provider presence.

Changes

Cohort / File(s)	Summary
Session and Stack Context Migration src/cmd/cli/command/cd.go, src/cmd/cli/command/compose.go	Changed Mode source from global.Stack to session.Stack in preview/deployment flows; added mandatory stack requirement in compose command with per-session stack validation across debugger, deployment, tail, and monitor operations.
Stack Loading and Error Handling src/pkg/session/session.go, src/pkg/session/session_test.go	Modified LoadStack to return ErrDefaultStackNotSet when no default stack exists, enabling fallback behavior with beta mode instead of failing; updated test to use specific error type.
SelectStackOptions API Refactoring src/pkg/stacks/selector.go, src/pkg/stacks/selector_test.go, src/pkg/stacks/manager.go, src/pkg/agent/tools/provider.go	Renamed public field AllowCreate to AllowStackCreation throughout stack selection flows; introduced new exported error ErrDefaultStackNotSet in manager.
Stack Parameter Population and Parsing src/pkg/stacks/stacks.go	Made region extraction conditional on DEFANG_PROVIDER presence; simplified error messages by removing input values; updated Parameters population to use conditional region lookup.
Provider and Client Updates src/pkg/cli/client/playground.go, src/pkg/cli/client/provider_id.go, src/pkg/cli/connect.go	Added Stack field and NewPlaygroundProvider constructor to PlaygroundProvider; improved provider error message formatting; updated default provider instantiation to use new constructor.
Variable Naming Consistency src/pkg/agent/tools/current_stack.go, src/pkg/cli/common.go, src/pkg/cli/stacks.go, src/pkg/modes/modes.go	Renamed variables and parameters for consistency: stackFileContent → stackFile, s → str in Parse function; updated corresponding usage sites.
Deployment Display Messages src/pkg/cli/deploymentsList.go, src/pkg/cli/deploymentsList_test.go	Added conditional "active" prefix to no-deployments messages when ListType is DEPLOYMENT_TYPE_ACTIVE; updated both empty-result branches with formatted output.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• #1721 — Introduces ErrDefaultStackNotSet and the AllowStackCreation field rename that are central to this PR.
• #1780 — Modifies stack context handling from global to per-session scopes in deployment and command flows, aligning with the session-stack migration here.
• #1730 — Updates the same AllowCreate → AllowStackCreation rename in stack selection operations.

Suggested reviewers

• jordanstephens
• raphaeltm

Poem

🐰 From global stacks to sessions bright,
We hop through code with careful sight!
AllowCreate now AllowStackCreation flows,
ErrDefaultStackNotSet—our fallback shows,
Better errors help us find our way,
A refactored warren—hooray, hooray! 🥕

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Lio/stacks' is vague and does not clearly describe the main changes. It appears to be a branch name or identifier rather than a descriptive PR title about fixing panic, interactive workflow issues, or stack handling.	Revise the title to clearly describe the primary change, such as 'Fix stack handling panic and interactive workflow issues' or 'Improve session stack handling and Ctrl-C support'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

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

✨ Finishing touches

• 📝 Generate docstrings

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.5.0)

level=warning msg="[linters_context] running gomodguard failed: unable to read module file go.mod: current working directory must have a go.mod file: if you are not using go modules it is suggested to disable this linter"
level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"

---

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

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/pkg/session/session.go (1)

81-101: Add explicit ProviderID default for empty fallback case.

In the ErrDefaultStackNotSet fallback path (lines 95-101), if ProviderID remains empty (not set to "auto"), it bypasses the coercion logic and returns Provider: "". This causes NewProvider() to silently fall back to PlaygroundProvider (the default case in the switch statement at src/pkg/cli/connect.go:46).

Either add an explicit default here (e.g., coerce empty to ProviderDefang alongside the ProviderAuto case) or require callers to initialize ProviderID before creating SessionLoaderOptions.