avoid double stack loading

[jordanstephens]

Description

This PR moves the responsibility of setting the stack env vars out of the stacks manager and into the session

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

• Refactor
  • Stack environment loading is now centralized in the selection/session flow so variables are initialized during interactive selection.
  • Lower-level local and remote load paths no longer auto-import environment data; they return stack parameters directly.
  • Provider setup and interactive selection now explicitly load stack environments at runtime, surfacing environment load failures earlier for more consistent initialization.

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

[coderabbitai]

📝 Walkthrough

Walkthrough

Removed the exported StacksManager interface; provider preparer now accepts a concrete stacks.Manager. Stack environment loading was removed from manager LoadLocal/LoadRemote and is now invoked from session flows after a stack is selected or loaded.

Changes

Cohort / File(s)	Summary
Provider → stacks.Manager change src/pkg/agent/tools/provider.go	Removed exported StacksManager interface; providerPreparer.sm now uses stacks.Manager; updated NewProviderPreparer signature to accept stacks.Manager.
Session: env loading moved to session flows src/pkg/session/session.go	Removed unconditional env load in LoadSession; added loadStackInteractively; after interactive/specified/fallback selection, session now calls stacks.LoadStackEnv(...) and propagates errors. Attention: new helper and changed return values include origin/whence.
Stacks manager: removed env loader src/pkg/stacks/manager.go	Deleted LoadStackEnv method; LoadLocal and LoadRemote no longer call/return environment-loading and now return parameters directly.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Session
  participant StacksMgr as "stacks.Manager"
  participant ProviderPrep as "providerPreparer"

  User->>Session: start session / request stack
  alt Interactive selection
    Session->>StacksMgr: List / Read available stacks
    StacksMgr-->>Session: stacks list
    Session->>User: prompt & select stack
  else Specified or remote
    Session->>StacksMgr: LoadLocal / LoadRemote
    StacksMgr-->>Session: stack parameters
  end
  Session->>StacksMgr: LoadStackEnv(stack, false)
  StacksMgr-->>Session: env loaded / error
  Session->>ProviderPrep: pass selected stack (with env)
  ProviderPrep-->>Session: continue provider setup

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• ensure stack env vars are loaded #1790: Adds explicit stacks.LoadStackEnv calls after stack selection—overlaps session/env-loading changes.
• Stacks onboarding with "remote" stacks #1721: Refactor around stacks.Manager and stack-loading APIs that interacts with manager/provider signatures.
• Stack tools #1713: Changes to src/pkg/agent/tools/provider.go and manager-type updates touching the same types and calls.

Suggested reviewers

• lionello
• raphaeltm

Poem

🐰 I hopped through branches, tidy and spry,

Moved envs to sessions beneath the sky,
Manager hands parameters clean on the tray,
Sessions fetch variables to light the way,
Happy hops — new flows ready to try!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ 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%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'avoid double stack loading' directly and accurately captures the main objective of the PR: preventing duplicate stack environment loading by moving responsibility from the stacks manager to the session.

_{✏️ 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: 1

🤖 Fix all issues with AI agents

In `@src/pkg/agent/tools/provider.go`:
- Around line 44-49: The code calls stacks.LoadStackEnv(*stack, false) before
assigning *stack = *newStack, so it loads env from the old stack; move the
assignment *stack = *newStack to occur before calling stacks.LoadStackEnv and
then call stacks.LoadStackEnv(*stack, false) with the updated stack, preserving
the existing error handling (i.e., check err and return fmt.Errorf("failed to
load stack env: %w", err) if it fails).

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/pkg/session/session.go`:
- Around line 152-180: The loadFallbackStack function has multiple compile
issues: remove or use the unused local whence (set by DEFANG_PROVIDER check) —
simplest fix is to drop the whence variable entirely; when handling the
GetDefaultStack response, use params (not undefined stack) when calling
stacks.LoadStackEnv and do not redeclare err (use assignment: err =
stacks.LoadStackEnv(params, false) or err = stacks.LoadStackEnv(*params, false)
depending on signature); ensure you pass the correct params type expected by
stacks.LoadStackEnv and propagate the error if non-nil; also fix indentation to
match project style (tabs). Locate symbols: loadFallbackStack,
sl.client.GetDefaultStack, stacks.NewParametersFromContent, params, and
stacks.LoadStackEnv.