airbyte/secrets/env_vars.py (1)

66-71: Harden .env parsing and return a stable order (optional: rename to list_secret_names)

To keep behavior consistent with get_secret() (which swallows parse/missing-file errors) and to make outputs deterministic for tests/UI, could we wrap parsing in try/except and sort the keys? Also, would you consider the singular form list_secret_names for readability, then update callers, wdyt?

Apply this diff:

-    def list_secrets_names(self) -> list[str]:
-        """List all secrets available in the .env file."""
-        dotenv_vars: dict[str, str | None] = dotenv_values(
-            dotenv_path=self.dotenv_path,
-        )
-        return list(dotenv_vars.keys())
+    def list_secrets_names(self) -> list[str]:
+        """List all secret variable names available in the .env file (names only, no values)."""
+        try:
+            dotenv_vars: dict[str, str | None] = dotenv_values(
+                dotenv_path=self.dotenv_path,
+            )
+        except Exception:
+            # Can't locate or parse a .env file; align with get_secret() behavior
+            return []
+        return sorted(dotenv_vars.keys())

If you prefer the rename, here’s the alternative within this file:

-    def list_secrets_names(self) -> list[str]:
+    def list_secret_names(self) -> list[str]:

And the corresponding call-site update is proposed in airbyte/mcp/_local_ops.py below.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

airbyte/mcp/_local_ops.py (3)

188-200: Harden + make output deterministic (sort + guard errors).

Could we sort the names and guard against dotenv parsing errors so callers get stable, non-throwing results, wdyt?

Apply this diff:

 def list_dotenv_secrets() -> dict[str, list[str]]:
-    """List all environment variable names declared within declared .env files.
-
-    This returns a dictionary mapping the .env file name to a list of environment
-    variable names. The values of the environment variables are not returned.
-    """
+    """List environment variable names declared in configured .env files.
+
+    Returns: mapping of absolute .env file path -> sorted list of variable names (names only; no values).
+    """
     result: dict[str, list[str]] = {}
     for secrets_mgr in _get_secret_sources():
         if isinstance(secrets_mgr, DotenvSecretManager) and secrets_mgr.dotenv_path:
-            result[str(secrets_mgr.dotenv_path.resolve())] = secrets_mgr.list_secrets_names()
+            key = str(secrets_mgr.dotenv_path.resolve())
+            try:
+                names = secrets_mgr.list_secrets_names()
+            except Exception:
+                names = []
+            result[key] = sorted(names)
 
     return result

---

189-193: Docstring mismatch with behavior.

Doc says “file name” but the key is an absolute path; also phrasing repeats “declared”. Mind updating to reflect absolute paths and simplify wording, wdyt?

Apply this docstring-only diff if you prefer a minimal change:

-    """List all environment variable names declared within declared .env files.
-
-    This returns a dictionary mapping the .env file name to a list of environment
-    variable names. The values of the environment variables are not returned.
-    """
+    """List environment variable names declared in configured .env files.
+
+    Returns a mapping of absolute .env file path to a list of variable names (names only; no values).
+    """

---

194-199: Check comfort level with exposing absolute paths.

Using absolute paths as keys can surface local usernames or workspace structure in UIs/logs. Is that acceptable for this tool, or should we consider masking (e.g., Path.name, relative-to-CWD, or a “display_name” field) while keeping the absolute path in a separate “path” field, wdyt?

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro