feat: allow pass hostname in docker env

[tomerqodo]

Benchmark PR from agentic-review-benchmarks#6

[greptile-apps]

Greptile Overview

Greptile Summary

Added support for configurable SMTP local hostname via SMTP_LOCAL_HOSTNAME environment variable, addressing scenarios where servers are behind NAT or have problematic default hostnames. The implementation adds the configuration field, passes it to SMTP constructors, and updates all relevant environment files and tests.

Key changes:

• Added SMTP_LOCAL_HOSTNAME config field in MailConfig with proper type annotations
• Modified SMTP client to use configurable hostname for HELO/EHLO commands
• Refactored SMTP class selection using ternary operator
• Updated environment examples and docker-compose configuration
• Updated unit tests to verify local_hostname parameter is passed

Issues found:

• Critical logic bug: When SMTP_LOCAL_HOSTNAME is not configured, ehlo("") is called with an empty string (lines 34, 36 in smtp.py), which sends an invalid hostname in the SMTP protocol. The constructor correctly handles this by converting empty strings to None, but the direct ehlo() calls do not apply the same pattern.
• Minor style issues: Unnecessary assertion and complex one-liner that reduces readability

Confidence Score: 2/5

• This PR has a critical logic bug that will cause SMTP failures when the new config option is not set
• The PR introduces a critical bug where ehlo("") is called with an empty string when SMTP_LOCAL_HOSTNAME is not configured. This violates SMTP protocol and will likely cause mail server rejections in the default configuration. While the feature implementation is otherwise sound, this bug affects the primary use case (default/unconfigured state) and must be fixed before merge.
• Critical attention needed for api/libs/smtp.py lines 34 and 36 - the ehlo() calls must use local_host or None pattern to avoid sending empty hostname

Important Files Changed

Filename	Overview
api/libs/smtp.py	Added SMTP_LOCAL_HOSTNAME support with ternary refactor, but contains a bug where ehlo("") is called with empty string when config is unset
api/configs/feature/init.py	Added SMTP_LOCAL_HOSTNAME config field with proper type annotations and description
api/tests/unit_tests/libs/test_smtp_client.py	Updated test assertions to expect local_hostname parameter using ANY matcher

Sequence Diagram

sequenceDiagram
    participant App as Application
    participant SMTPClient as SMTPClient
    participant Config as dify_config
    participant SMTP as smtplib.SMTP/SMTP_SSL
    participant Server as SMTP Server

    App->>SMTPClient: send(mail)
    SMTPClient->>Config: Get SMTP_LOCAL_HOSTNAME
    Config-->>SMTPClient: hostname or None
    SMTPClient->>SMTPClient: local_host = SMTP_LOCAL_HOSTNAME or ""
    
    alt use_tls=True AND opportunistic_tls=False
        SMTPClient->>SMTP: SMTP_SSL(server, port, local_hostname=local_host or None)
    else use_tls=True AND opportunistic_tls=True OR use_tls=False
        SMTPClient->>SMTP: SMTP(server, port, local_hostname=local_host or None)
    end
    
    SMTP-->>SMTPClient: smtp instance
    
    alt use_tls=True AND opportunistic_tls=True
        SMTPClient->>SMTP: ehlo(local_host)
        SMTP->>Server: EHLO command
        Server-->>SMTP: 250 OK
        SMTPClient->>SMTP: starttls()
        SMTP->>Server: STARTTLS
        Server-->>SMTP: 220 Ready
        SMTPClient->>SMTP: ehlo(local_host)
        SMTP->>Server: EHLO command
        Server-->>SMTP: 250 OK
    end
    
    alt username and password present
        SMTPClient->>SMTP: login(username, password)
        SMTP->>Server: AUTH
        Server-->>SMTP: 235 Authenticated
    end
    
    SMTPClient->>SMTP: sendmail(from, to, message)
    SMTP->>Server: MAIL FROM / RCPT TO / DATA
    Server-->>SMTP: 250 OK
    SMTP-->>SMTPClient: Success
    SMTPClient->>SMTP: quit()
    SMTP->>Server: QUIT
    SMTPClient-->>App: Email sent

Loading

[greptile-apps]

_{4 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

When SMTP_LOCAL_HOSTNAME is not configured, local_host will be an empty string "". Passing an empty string to ehlo("") sends an invalid/empty hostname in the SMTP EHLO command, which violates SMTP protocol and may cause mail server rejections.

The constructor correctly handles this at line 29 with local_hostname=local_host or None, but the direct ehlo() calls should use the same pattern.

Suggested change

	smtp.ehlo(local_host)
	smtp.ehlo(local_host or None)

Prompt To Fix With AI

This is a comment left during a code review.
Path: api/libs/smtp.py
Line: 34:34

Comment:
When `SMTP_LOCAL_HOSTNAME` is not configured, `local_host` will be an empty string `""`. Passing an empty string to `ehlo("")` sends an invalid/empty hostname in the SMTP EHLO command, which violates SMTP protocol and may cause mail server rejections.

The constructor correctly handles this at line 29 with `local_hostname=local_host or None`, but the direct `ehlo()` calls should use the same pattern.

```suggestion
                smtp.ehlo(local_host or None)
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Same issue - when SMTP_LOCAL_HOSTNAME is not configured, local_host="" and ehlo("") sends an empty hostname.

Suggested change

	smtp.ehlo(local_host)
	smtp.ehlo(local_host or None)

Prompt To Fix With AI

This is a comment left during a code review.
Path: api/libs/smtp.py
Line: 36:36

Comment:
Same issue - when `SMTP_LOCAL_HOSTNAME` is not configured, `local_host=""` and `ehlo("")` sends an empty hostname.

```suggestion
                smtp.ehlo(local_host or None)
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

The ternary expression and class instantiation are combined on one line, making it harder to read and debug. The assertion on line 32 is also unnecessary since the ternary always returns a valid SMTP object.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review.
Path: api/libs/smtp.py
Line: 28:30

Comment:
The ternary expression and class instantiation are combined on one line, making it harder to read and debug. The assertion on line 32 is also unnecessary since the ternary always returns a valid SMTP object.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

This assertion is unnecessary - the ternary on line 28 always returns either SMTP_SSL or SMTP instance, never None.

Prompt To Fix With AI

This is a comment left during a code review.
Path: api/libs/smtp.py
Line: 32:32

Comment:
This assertion is unnecessary - the ternary on line 28 always returns either `SMTP_SSL` or `SMTP` instance, never `None`.

How can I resolve this? If you propose a fix, please make it concise.