Skip to content

Comments

Skip userinfo call on CI#1937

Open
yuta519 wants to merge 1 commit intoDefangLabs:mainfrom
yuta519:skip-userinfocall-on-ci
Open

Skip userinfo call on CI#1937
yuta519 wants to merge 1 commit intoDefangLabs:mainfrom
yuta519:skip-userinfocall-on-ci

Conversation

@yuta519
Copy link
Contributor

@yuta519 yuta519 commented Feb 19, 2026

Description

Skip the userinfo call on the CI by using global.HasTty.

Linked Issues

#1834

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
    • Improved the whoami command's user information fetching to better handle different execution contexts, with optimized behavior for CI environments.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 19, 2026

📝 Walkthrough

Walkthrough

The whoami.go command now conditionally fetches user information only in interactive mode. In non-interactive (CI) environments, user info remains nil. FetchUserInfo errors result in warnings with graceful degradation. Variable userInfo is explicitly declared before use.

Changes

Cohort / File(s) Summary
Interactive Mode User Fetch
src/cmd/cli/command/whoami.go
Added conditional check to fetch user information only in interactive mode; non-interactive mode skips FetchUserInfo call and leaves userInfo nil; error handling via warning log preserved for interactive path.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A rabbit checks the terminal's glow—
Is someone here to watch? Does it show?
If interactive, fetch info with care,
If CI runs silent, skip the despair! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Skip userinfo call on CI' directly and specifically describes the main change: conditionally skipping the FetchUserInfo call in non-interactive/CI environments using interactive-mode checks.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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

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"


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.

@yuta519 yuta519 marked this pull request as ready for review February 19, 2026 13:07
Copy link
Contributor

@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.

Caution

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

⚠️ Outside diff range comments (1)
src/cmd/cli/command/whoami.go (1)

34-44: ⚠️ Potential issue | 🔴 Critical

Add nil-guard to ResolveWorkspaceName or guard the call in cli.Whoami.

The ResolveWorkspaceName function at src/pkg/cli/workspace.go:40 calls info.FindWorkspaceInfo(tenantSelection) without checking if info is nil. When userInfo is nil in CI mode (global.HasTty false), the call to ResolveWorkspaceName(userInfo, tenantSelection) at src/pkg/cli/whoami.go:42 will panic.

Either:

  1. Add a nil check at the start of ResolveWorkspaceName: if info == nil { return "" }
  2. Or guard the call in Whoami before line 42: only call ResolveWorkspaceName if userInfo != nil, otherwise set Workspace to an empty string or the tenant selection value
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/cmd/cli/command/whoami.go` around lines 34 - 44, Resolve the nil-deref by
guarding ResolveWorkspaceName against a nil user info: update
ResolveWorkspaceName(info, tenantSelection) to check if info == nil and return
an empty string (or sensible default) before calling
info.FindWorkspaceInfo(tenantSelection); alternatively, in cli.Whoami where
Whoami(ctx, global.Client, provider, userInfo, global.Tenant) is invoked, only
call ResolveWorkspaceName when userInfo != nil and set Workspace to "" or
tenantSelection when userInfo is nil. Ensure you reference ResolveWorkspaceName,
FindWorkspaceInfo, Whoami, and userInfo in the fix so the null path is handled
and no panic occurs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/cmd/cli/command/whoami.go`:
- Around line 34-44: Resolve the nil-deref by guarding ResolveWorkspaceName
against a nil user info: update ResolveWorkspaceName(info, tenantSelection) to
check if info == nil and return an empty string (or sensible default) before
calling info.FindWorkspaceInfo(tenantSelection); alternatively, in cli.Whoami
where Whoami(ctx, global.Client, provider, userInfo, global.Tenant) is invoked,
only call ResolveWorkspaceName when userInfo != nil and set Workspace to "" or
tenantSelection when userInfo is nil. Ensure you reference ResolveWorkspaceName,
FindWorkspaceInfo, Whoami, and userInfo in the fix so the null path is handled
and no panic occurs.

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