Skip to content

Fix panic in defang compose config#1901

Merged
KevyVo merged 3 commits intomainfrom
lio/panic
Feb 3, 2026
Merged

Fix panic in defang compose config#1901
KevyVo merged 3 commits intomainfrom
lio/panic

Conversation

@lionello
Copy link
Member

@lionello lionello commented Feb 3, 2026

Description

Compose config can continue without a stack. Interactively, you'd get prompted for a stack, or get a default stack from Fabric. Without auth, we don't know whether there is a default stack or not, so we return an error (expected).

Linked Issues

Fixes #1894

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

  • New Features

    • Added stack listing and configuration options management capabilities.
    • Enabled compose config command to work without authentication.
    • Improved session handling with automatic fallback mechanisms.
  • Bug Fixes

    • Enhanced login flow to properly return updated client state.
    • Better error recovery in compose operations.
  • Tests

    • Added tests for unauthenticated compose workflows.
    • Improved test environment isolation for cloud project configuration.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 3, 2026

📝 Walkthrough

Walkthrough

Changes refactor the client initialization and login flow by making InteractiveRequireLoginAndToS return a possibly-updated client along with an error, changing the client type from pointer to interface, and adding fallback session handling in compose commands to gracefully continue when initial session creation fails.

Changes

Cohort / File(s) Summary
Login flow signature change
src/pkg/login/login.go, src/cmd/cli/command/commands.go
InteractiveRequireLoginAndToS now returns (client.FabricClient, error) instead of error; all call sites updated to capture the returned client and assign to global.Client.
Client type updates
src/cmd/cli/command/globals.go, src/pkg/setup/setup.go
Client field changed from *client.GrpcClient pointer to client.FabricClient interface in GlobalConfig and SetupClient structs.
FabricClient interface expansion
src/pkg/cli/client/client.go
Added methods: GetFabricClient(), GetRequestedTenant(), ListStacks(), SetOptions(); GetDefaultStack signature retained.
Compose resilience
src/cmd/cli/command/compose.go
Introduced fallback session (sessionx) with Playground provider and default beta stack parameters when initial session creation fails, allowing continued execution instead of early termination.
Test coverage
src/cmd/cli/command/compose_test.go, src/pkg/cli/client/byoc/gcp/byoc_test.go
Added TestComposeConfig for unauthenticated scenarios; added environment variable cleanup in TestGetGcpProjectID.
Test data cleanup
src/cmd/cli/command/testdata/no-stack/.defang
Removed VALUE=from .defang line.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant Compose as Compose Command
    participant Session as Session Init
    participant Fallback as Fallback Session
    participant Provider as Fabric Provider
    participant Loader as Loader
    
    User->>Compose: Execute compose config
    Compose->>Session: Load command session
    Session-->>Compose: Session creation fails (unauthenticated)
    Compose->>Fallback: Create fallback sessionx with Playground provider
    Fallback-->>Compose: Return fallback session
    Compose->>Provider: Get provider from fallback session
    Compose->>Loader: Initialize loader with fallback provider
    Compose->>Loader: Load/create project using fallback session
    Loader-->>Compose: Project loaded
    Compose-->>User: Continue with fallback, no panic
Loading
sequenceDiagram
    participant CMD as Command Root
    participant Login as InteractiveRequireLoginAndToS
    participant Client as Fabric Client
    participant Setup as Setup Flow
    
    CMD->>Login: Call with context, fabric client, address
    Login->>Client: Check login/ToS status
    alt Login required
        Login->>Login: Prompt user interactively
        Login->>Client: Reconnect with new tenant if needed
        Client-->>Login: Updated fabric client
    end
    Login->>Login: Re-check login/ToS with current client
    Login-->>CMD: Return (updated fabric client, error)
    CMD->>Setup: Use returned fabric client for initialization
    Setup-->>CMD: Complete setup
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Suggested reviewers

  • jordanstephens

Poem

🐰 When clients fail to authenticate,
Our fallback rabbits don't deflate,
With Playground hops and beta stacks,
We gracefully take scenic tracks,
No panics hop, no nil to fright,
Just compose flows dancing right! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix panic in defang compose config' directly describes the main change—preventing a panic in the compose config command when unauthenticated, which aligns with the PR's primary objective and linked issue #1894.
Linked Issues check ✅ Passed The PR addresses issue #1894 by refactoring session handling in compose.go to use a fallback session when initial authentication fails, adding tests for unauthenticated scenarios, and updating login/client handling to prevent nil pointer dereference panics.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the panic: refactoring session/login handling in compose and related client files, adding unauthenticated tests, and updating function signatures to support the fallback flow without introducing unrelated modifications.
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
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch lio/panic

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

@lionello lionello mentioned this pull request Feb 3, 2026
3 tasks
@lionello lionello changed the title Lio/panic Fix panic in defang compose config` Feb 3, 2026
@lionello lionello changed the title Fix panic in defang compose config` Fix panic in defang compose config Feb 3, 2026
@KevyVo KevyVo merged commit 82fc99d into main Feb 3, 2026
14 checks passed
@KevyVo KevyVo deleted the lio/panic branch February 3, 2026 22:37
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.

Panic in Nightly

2 participants