Normalize env value parsing for config loading

[shuofengzhang]

What changed

• Normalize environment variable values with strip().lower() before parsing.
• Make HEADLESS parsing more tolerant by supporting mixed case / surrounding whitespace and on/off aliases.
• Make TRANSPORT parsing tolerant to mixed case / surrounding whitespace while preserving validation for unsupported values.
• Added config loader tests covering:
  • HEADLESS=" TrUe "
  • HEADLESS="off"
  • TRANSPORT=" StReAmAbLe-HtTp "

Why

• Environment values are often injected with inconsistent casing or accidental whitespace in shell scripts, CI, and container setups.
• Normalizing inputs makes configuration parsing more robust without changing defaults or relaxing invalid-value checks.

Testing

• ✅ scripts/clone_and_test.sh stickerdaniel/linkedin-mcp-server
• ✅ source .venv/bin/activate && pytest tests/test_config.py -q
• ✅ source .venv/bin/activate && pytest -q

Greptile Summary

This PR improves the robustness of environment variable parsing in the config loader by introducing a _normalize_env() helper (strip().lower()) and applying it to the HEADLESS and TRANSPORT keys. It also adds "on"/"off" as boolean aliases. The changes are fully backward-compatible — all previously accepted values still work because the old mixed-case variants ("True", "False", "Yes", "No") are handled naturally by the lowercasing step. The LOG_LEVEL path correctly opts out of _normalize_env() and instead uses .strip().upper() directly, which avoids the redundant lower→upper round-trip.

• New tests cover all normalization paths: whitespace + mixed-case HEADLESS (both truthy and falsy), the on/off aliases, a normalised LOG_LEVEL, and both TRANSPORT values (stdio and streamable-http) with whitespace and mixed case.
• All gaps previously flagged in review threads (symmetric on alias, stdio transport normalisation, whitespace falsy HEADLESS) are addressed.
• Error messages for invalid TRANSPORT values retain the original, un-normalised value for clear user feedback — a nice touch.

Confidence Score: 5/5

• This PR is safe to merge — changes are minimal, backward-compatible, and backed by thorough tests.
• All logic changes are additive normalisation steps that preserve existing behavior for previously accepted values. No defaults were altered, no validation was relaxed, and the test suite comprehensively covers the new paths including all previously flagged gaps.
• No files require special attention.

Important Files Changed

Filename	Overview
linkedin_mcp_server/config/loaders.py	Adds _normalize_env() helper and applies it to HEADLESS and TRANSPORT parsing; LOG_LEVEL correctly uses .strip().upper() directly. Backward-compatible changes with no logic regressions found.
tests/test_config.py	Comprehensive new test cases added for all normalization paths: mixed-case/whitespace HEADLESS (truthy + falsy, including on/off aliases), LOG_LEVEL, and both TRANSPORT values (stdio and streamable-http). All previously flagged gaps are now covered.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[os.environ.get] --> B{Value present?}
    B -- No --> Z[Use default]
    B -- Yes --> C[_normalize_env: strip + lower]

    C --> D{Which key?}

    D -- HEADLESS --> E{in FALSY_VALUES?}
    E -- Yes --> F[headless = False]
    E -- No --> G{in TRUTHY_VALUES?}
    G -- Yes --> H[headless = True]
    G -- No --> Z

    D -- TRANSPORT --> I{== 'stdio'?}
    I -- Yes --> J[transport = 'stdio']
    I -- No --> K{== 'streamable-http'?}
    K -- Yes --> L[transport = 'streamable-http']
    K -- No --> M[raise ConfigurationError]

    D -- LOG_LEVEL --> N[strip + upper only]
    N --> O{in DEBUG/INFO/WARNING/ERROR?}
    O -- Yes --> P[log_level = value]
    O -- No --> Z

Loading

Comments Outside Diff (1)

1. tests/test_config.py, line 100-105 (link)

   No normalization test for LOG_LEVEL

   The load_from_env function now routes LOG_LEVEL through _normalize_env() before calling .upper(), which means whitespace-padded values like LOG_LEVEL=" debug " or LOG_LEVEL=" WARNING " would silently work. However, there is no test exercising this normalization path for LOG_LEVEL, unlike the explicit whitespace/case tests added for HEADLESS and TRANSPORT.

   Consider adding a test such as:

   def test_load_from_env_log_level_with_whitespace_and_case(self, monkeypatch):
       monkeypatch.setenv("LOG_LEVEL", "  dEbUg  ")
       from linkedin_mcp_server.config.loaders import load_from_env
   
       config = load_from_env(AppConfig())
       assert config.server.log_level == "DEBUG"

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: tests/test_config.py
   Line: 100-105
   
   Comment:
   **No normalization test for `LOG_LEVEL`**
   
   The `load_from_env` function now routes `LOG_LEVEL` through `_normalize_env()` before calling `.upper()`, which means whitespace-padded values like `LOG_LEVEL=" debug "` or `LOG_LEVEL="  WARNING  "` would silently work. However, there is no test exercising this normalization path for `LOG_LEVEL`, unlike the explicit whitespace/case tests added for `HEADLESS` and `TRANSPORT`.
   
   Consider adding a test such as:
   
   ```python
   def test_load_from_env_log_level_with_whitespace_and_case(self, monkeypatch):
       monkeypatch.setenv("LOG_LEVEL", "  dEbUg  ")
       from linkedin_mcp_server.config.loaders import load_from_env
   
       config = load_from_env(AppConfig())
       assert config.server.log_level == "DEBUG"
   ```
   
   How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: c486d8c}

[shuofengzhang]

Addressed both review points in this PR:

• added coverage for HEADLESS="on" truthy alias
• added normalization coverage for TRANSPORT=" StDiO "

Validation run:

• uv run pytest -q tests/test_config.py (26 passing)

[shuofengzhang]

Good call — I removed the redundant transformation in the LOG_LEVEL path.

Changes:

• switched LOG_LEVEL normalization from _normalize_env(...).upper() to log_level_env.strip().upper()
• added regression coverage for mixed-case + whitespace (" dEbUg ")

Validation:

• uv run pytest -q tests/test_config.py (27 passed)
• uv run ruff check linkedin_mcp_server/config/loaders.py tests/test_config.py

[shuofengzhang]

Added the missing symmetric falsy coverage:

• new test for HEADLESS=" FaLsE " to verify whitespace/case normalization on the falsy path

Validation:

• uv run pytest -q tests/test_config.py (28 passed)
• uv run ruff check tests/test_config.py

[greptile-apps]

Greptile Summary

This PR improves robustness of environment variable parsing by introducing a _normalize_env helper (strip().lower()) and applying it to HEADLESS and TRANSPORT parsing, while handling LOG_LEVEL with an inline strip().upper(). The TRUTHY_VALUES/FALSY_VALUES tuples are simplified to lowercase-only since inputs are now normalized before comparison, and new "on"/"off" aliases are added. The accompanying tests cover whitespace+case normalization, both transport values, "on"/"off" aliases, and LOG_LEVEL whitespace handling.

Key changes:

• _normalize_env helper centralises strip().lower() normalization for boolean-style env vars
• TRUTHY_VALUES/FALSY_VALUES de-duplicated to lowercase-only entries; "on"/"off" added as new aliases
• LOG_LEVEL normalisation correctly uses inline strip().upper() (avoiding the unnecessary round-trip through lower() before upper())
• HEADLESS block refactored to read the env var once using walrus operator, eliminating the previous double os.environ.get call
• Nine new tests added covering normalization edge cases across all three env vars

Confidence Score: 5/5

• This PR is safe to merge — changes are purely additive normalization with no breaking behavior and excellent test coverage.
• All prior behaviors are preserved (existing lowercase and title-case values still resolve correctly after normalization), the _normalize_env helper is simple and correct, the walrus-operator refactor eliminates a pre-existing double os.environ.get call, and nine targeted tests confirm every code path. No regressions or logic errors detected.
• No files require special attention.

Important Files Changed

Filename	Overview
linkedin_mcp_server/config/loaders.py	Adds _normalize_env helper for strip().lower(), applies it to HEADLESS and TRANSPORT parsing, uses inline strip().upper() for LOG_LEVEL, and removes mixed-case duplicates from TRUTHY_VALUES/FALSY_VALUES. Logic is correct; minor inconsistency in normalization approach between branches.
tests/test_config.py	Adds thorough normalization tests: whitespace+case for HEADLESS (true/false), on/off aliases, LOG_LEVEL with whitespace/case, and both transport values with whitespace/case. All tests are well-structured and cover the new behavior.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[os.environ.get] --> B{Value present?}
    B -- No --> Z[Keep default]
    B -- Yes --> C["_normalize_env: strip + lower"]

    C --> D{Which env var?}

    D -- HEADLESS --> E{In FALSY_VALUES?}
    E -- Yes --> F[headless = False]
    E -- No --> G{In TRUTHY_VALUES?}
    G -- Yes --> H[headless = True]
    G -- No --> Z

    D -- TRANSPORT --> I[transport_explicitly_set = True]
    I --> J{Normalized value?}
    J -- stdio --> K[transport = stdio]
    J -- streamable-http --> L[transport = streamable-http]
    J -- other --> M[raise ConfigurationError]

    D -- LOG_LEVEL --> N["Inline: strip + upper"]
    N --> O{In DEBUG/INFO/WARNING/ERROR?}
    O -- Yes --> P[log_level = value]
    O -- No --> Z

Loading

_{Last reviewed commit: 3619a31}