fix: cli mode google functions by taylorwilsdon · Pull Request #450 · taylorwilsdon/google_workspace_mcp

taylorwilsdon · 2026-02-10T19:06:48Z

Closes #448

Copilot

Pull request overview

Fixes CLI invocation of FastAPI-based tool functions by unwrapping FastAPI Body/Query/... marker defaults so omitted CLI args resolve to real runtime defaults (not marker objects), addressing Gmail tools failing in CLI mode.

Changes:

Aliased FastAPI Body to BodyParam and updated affected Gmail tool signatures to use it consistently.
Added CLI argument normalization that resolves FastAPI param marker defaults (and produces clearer missing-required errors) before calling tools.
Recorded the bugfix in .beads/issues.jsonl.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
gmail/gmail_tools.py	Renames `Body` import to `BodyParam` and updates Gmail tool params to use it, aligning with CLI normalization behavior.
core/cli_handler.py	Adds FastAPI marker detection + default extraction to normalize CLI args before invoking tool functions.
.beads/issues.jsonl	Adds a closed issue entry describing the CLI FastAPI default-unwrapping fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-10T19:07:57Z

+    except Exception:
+        resolved_default = getattr(default_marker, "default", inspect.Parameter.empty)
+
+    return _is_required_marker_default(resolved_default), resolved_default


If _extract_fastapi_default falls back to inspect.Parameter.empty, _is_required_marker_default will currently treat that as not-required, and _normalize_cli_args_for_tool may pass inspect.Parameter.empty into the tool function as a real argument value. Consider treating inspect.Parameter.empty as 'required/unresolved' (or raising) so CLI calls fail fast with a clear missing-arg error instead of passing a sentinel to user code.

Copilot · 2026-02-10T19:07:58Z

+    return value is Ellipsis or type(value).__name__ == "PydanticUndefinedType"
+
+


Detecting required defaults by comparing type(value).__name__ to PydanticUndefinedType is brittle across Pydantic versions (and can miss other sentinel defaults like pydantic.fields.Undefined / UndefinedType). Consider broadening this check (e.g., handling inspect.Parameter.empty as required, and supporting both Pydantic v1/v2 undefined sentinels) to keep CLI behavior stable across dependency upgrades.

Suggested change

return value is Ellipsis or type(value).__name__ == "PydanticUndefinedType"

# Treat standard Python/FastAPI markers as "required"

if value is Ellipsis or value is inspect.Parameter.empty:

return True

# Handle Pydantic v1/v2 "undefined" sentinels without hard-coding imports.

# - v1: pydantic.fields.Undefined (type: UndefinedType)

# - v2: pydantic_core.PydanticUndefined (type: PydanticUndefinedType)

t = type(value)

type_name = getattr(t, "__name__", "")

type_module = getattr(t, "__module__", "")

if type_name in {"PydanticUndefinedType", "UndefinedType"} and type_module.startswith(

"pydantic"

):

return True

return False

Copilot · 2026-02-10T19:07:58Z

+    default_type = type(default)
+    return default_type.__module__ == "fastapi.params" and hasattr(
+        default, "get_default"
+    )


Checking FastAPI marker types via default_type.__module__ == \"fastapi.params\" is fairly fragile (module paths and wrappers can change). Prefer an isinstance check against FastAPI's param base type (if available) or a more direct duck-typing check (e.g., hasattr(default, \"in_\") + hasattr(default, \"get_default\")) without depending on an exact module string.

Suggested change

default_type = type(default)

return default_type.__module__ == "fastapi.params" and hasattr(

default, "get_default"

)

# Use duck-typing instead of fragile module-name checks; FastAPI param

# markers (Query, Body, etc.) expose both `in_` and `get_default`.

return hasattr(default, "in_") and hasattr(default, "get_default")

Copilot · 2026-02-10T19:07:58Z

+            raise TypeError(f"{fn.__name__}() missing 1 required argument: '{missing}'")
+
+        missing = ", ".join(f"'{name}'" for name in missing_required)
+        raise TypeError(
+            f"{fn.__name__}() missing {len(missing_required)} required arguments: {missing}"


The multi-argument missing-required TypeError message doesn't match Python's typical formatting (which includes an 'and' before the last argument, and often says 'positional arguments'). Consider formatting it closer to the built-in error style for readability and consistency with user expectations.

Suggested change

raise TypeError(f"{fn.__name__}() missing 1 required argument: '{missing}'")

missing = ", ".join(f"'{name}'" for name in missing_required)

raise TypeError(

f"{fn.__name__}() missing {len(missing_required)} required arguments: {missing}"

raise TypeError(

f"{fn.__name__}() missing 1 required positional argument: '{missing}'"

)

# Match CPython style for multiple missing positional arguments:

# e.g. "foo() missing 2 required positional arguments: 'a' and 'b'"

# "foo() missing 3 required positional arguments: 'a', 'b' and 'c'"

missing_names = [f"'{name}'" for name in missing_required]

if len(missing_names) == 2:

missing = " and ".join(missing_names)

else:

missing = ", ".join(missing_names[:-1]) + f" and {missing_names[-1]}"

raise TypeError(

f"{fn.__name__}() missing {len(missing_required)} required positional arguments: {missing}"

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-10T19:16:20Z

+def _is_required_marker_default(value: Any) -> bool:
+    """Check whether a FastAPI/Pydantic default represents a required field."""
+    return value is Ellipsis or type(value).__name__ == "PydanticUndefinedType"
+
+
+def _extract_fastapi_default(default_marker: Any) -> tuple[bool, Any]:
+    """
+    Resolve the runtime default from a FastAPI marker.
+
+    Returns:
+        Tuple of (is_required, resolved_default)
+    """
+    try:
+        resolved_default = default_marker.get_default(call_default_factory=True)
+    except TypeError:
+        # Compatibility path for implementations without call_default_factory kwarg
+        resolved_default = default_marker.get_default()
+    except Exception:
+        resolved_default = getattr(default_marker, "default", inspect.Parameter.empty)
+
+    return _is_required_marker_default(resolved_default), resolved_default
+
+
+def _normalize_cli_args_for_tool(fn, args: Dict[str, Any]) -> Dict[str, Any]:
+    """
+    Fill omitted CLI args for FastAPI markers with their real defaults.
+
+    When tools are invoked via HTTP, FastAPI resolves Body/Query/... defaults.
+    In CLI mode we invoke functions directly, so we need to do that resolution.
+    """
+    normalized_args = dict(args)
+    signature = inspect.signature(fn)
+    missing_required = []
+
+    for param in signature.parameters.values():
+        if param.kind in (
+            inspect.Parameter.VAR_POSITIONAL,
+            inspect.Parameter.VAR_KEYWORD,
+        ):
+            continue
+
+        if param.name in normalized_args:
+            continue
+
+        if param.default is inspect.Parameter.empty:
+            continue
+
+        if not _is_fastapi_param_marker(param.default):
+            continue
+
+        is_required, resolved_default = _extract_fastapi_default(param.default)
+        if is_required:
+            missing_required.append(param.name)
+        else:
+            normalized_args[param.name] = resolved_default


resolved_default can become inspect.Parameter.empty in _extract_fastapi_default() (line 59). Currently _is_required_marker_default() does not treat inspect.Parameter.empty as required, so _normalize_cli_args_for_tool() may inject inspect.Parameter.empty into normalized_args (line 95) and pass that sentinel into the tool, producing confusing downstream errors. Treat inspect.Parameter.empty as required (or avoid adding it to normalized_args) so the CLI raises a “missing required argument” error instead of passing the sentinel value through.

Copilot · 2026-02-10T19:16:21Z

+def _is_required_marker_default(value: Any) -> bool:
+    """Check whether a FastAPI/Pydantic default represents a required field."""
+    return value is Ellipsis or type(value).__name__ == "PydanticUndefinedType"


Detecting Pydantic “undefined” via type(value).__name__ == "PydanticUndefinedType" is brittle across Pydantic versions/implementations. Prefer checking against known sentinels/types (e.g., importing the undefined sentinel/type if available) and/or handling inspect.Parameter.empty explicitly, to make required-field detection stable over dependency upgrades.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-10T19:26:45Z

+except Exception:
+    _PYDANTIC_FIELDS_UNDEFINED = None
+
+try:
+    from pydantic.fields import Undefined as _PYDANTIC_V1_UNDEFINED
+except Exception:
+    _PYDANTIC_V1_UNDEFINED = None
+
+try:
+    from pydantic_core import PydanticUndefined as _PYDANTIC_CORE_UNDEFINED
+except Exception:


The module import fallbacks are catching Exception, which can inadvertently mask real runtime problems (e.g., environment/site-packages corruption, unexpected import-time errors) and silently change behavior. Prefer catching ImportError (or ModuleNotFoundError) for these optional imports so genuine errors still surface.

Suggested change

except Exception:

_PYDANTIC_FIELDS_UNDEFINED = None

try:

from pydantic.fields import Undefined as _PYDANTIC_V1_UNDEFINED

except Exception:

_PYDANTIC_V1_UNDEFINED = None

try:

from pydantic_core import PydanticUndefined as _PYDANTIC_CORE_UNDEFINED

except Exception:

except ImportError:

_PYDANTIC_FIELDS_UNDEFINED = None

try:

from pydantic.fields import Undefined as _PYDANTIC_V1_UNDEFINED

except ImportError:

_PYDANTIC_V1_UNDEFINED = None

try:

from pydantic_core import PydanticUndefined as _PYDANTIC_CORE_UNDEFINED

except ImportError:

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-10T21:42:56Z

+            raise TypeError(
+                f"{fn.__name__}() missing 1 required positional argument: '{missing}'"
+            )


The raised TypeError message says “required positional argument(s)”, but these tool calls are invoked via keyword arguments (fn(**call_args)), so the wording is misleading (and differs from Python’s typical “required keyword-only argument” phrasing when applicable). Consider emitting a message that matches Python’s conventions (e.g., “required keyword-only argument(s)” for keyword-only params, otherwise “required argument(s)”) or at least “required argument(s)” to avoid incorrect classification.

Copilot · 2026-02-10T21:42:57Z

+        raise TypeError(
+            f"{fn.__name__}() missing {len(missing_required)} required positional arguments: {missing}"
+        )


The raised TypeError message says “required positional argument(s)”, but these tool calls are invoked via keyword arguments (fn(**call_args)), so the wording is misleading (and differs from Python’s typical “required keyword-only argument” phrasing when applicable). Consider emitting a message that matches Python’s conventions (e.g., “required keyword-only argument(s)” for keyword-only params, otherwise “required argument(s)”) or at least “required argument(s)” to avoid incorrect classification.

Copilot · 2026-02-10T21:42:57Z

 {"id":"google_workspace_mcp-ic8","title":"enh: support writing hyperlink URLs in modify_sheet_values","description":"Issue #434 also requested hyperlink creation/writes. Current implementation reads hyperlinks in read_sheet_values but modify_sheet_values does not expose first-class hyperlink writes.","status":"open","priority":3,"issue_type":"task","owner":"tbarrettwilsdon@gmail.com","created_at":"2026-02-08T17:42:10.590658-05:00","created_by":"Taylor Wilsdon","updated_at":"2026-02-08T17:42:10.590658-05:00"}
 {"id":"google_workspace_mcp-jf2","title":"ci: make PyPI publish step rerun-safe with skip-existing","description":"GitHub Actions reruns on same tag fail because PyPI rejects duplicate file uploads. Add skip-existing=true to pypa/gh-action-pypi-publish so reruns proceed to MCP publish.","status":"closed","priority":2,"issue_type":"bug","owner":"tbarrettwilsdon@gmail.com","created_at":"2026-02-08T20:59:58.461102-05:00","created_by":"Taylor Wilsdon","updated_at":"2026-02-08T21:00:32.121469-05:00","closed_at":"2026-02-08T21:00:32.121469-05:00","close_reason":"Closed"}
 {"id":"google_workspace_mcp-qfl","title":"Fix stdio multi-account session binding","status":"in_progress","priority":1,"issue_type":"task","owner":"tbarrettwilsdon@gmail.com","created_at":"2026-02-07T13:27:09.466282-05:00","created_by":"Taylor Wilsdon","updated_at":"2026-02-07T13:27:22.857227-05:00"}
+{"id":"google_workspace_mcp-xia","title":"fix: CLI should unwrap FastAPI Body defaults when invoking tools","description":"CLI mode invokes tool functions directly and currently passes FastAPI Body marker objects as defaults for omitted args. This breaks gmail send/draft with errors like Body has no attribute lower/len. Update CLI invocation to normalize Param defaults and return clear missing-required errors.","status":"closed","priority":1,"issue_type":"bug","owner":"tbarrettwilsdon@gmail.com","created_at":"2026-02-10T12:33:06.83139-05:00","created_by":"Taylor Wilsdon","updated_at":"2026-02-10T12:36:35.051947-05:00","closed_at":"2026-02-10T12:36:35.051947-05:00","close_reason":"Implemented CLI FastAPI default normalization + regression tests","labels":["cli","gmail"]}


The close_reason claims “+ regression tests”, but this PR diff doesn’t include any test changes. Either add the referenced regression tests in this PR or adjust close_reason to avoid documenting tests that weren’t actually implemented here.

Suggested change

{"id":"google_workspace_mcp-xia","title":"fix: CLI should unwrap FastAPI Body defaults when invoking tools","description":"CLI mode invokes tool functions directly and currently passes FastAPI Body marker objects as defaults for omitted args. This breaks gmail send/draft with errors like Body has no attribute lower/len. Update CLI invocation to normalize Param defaults and return clear missing-required errors.","status":"closed","priority":1,"issue_type":"bug","owner":"tbarrettwilsdon@gmail.com","created_at":"2026-02-10T12:33:06.83139-05:00","created_by":"Taylor Wilsdon","updated_at":"2026-02-10T12:36:35.051947-05:00","closed_at":"2026-02-10T12:36:35.051947-05:00","close_reason":"Implemented CLI FastAPI default normalization + regression tests","labels":["cli","gmail"]}

{"id":"google_workspace_mcp-xia","title":"fix: CLI should unwrap FastAPI Body defaults when invoking tools","description":"CLI mode invokes tool functions directly and currently passes FastAPI Body marker objects as defaults for omitted args. This breaks gmail send/draft with errors like Body has no attribute lower/len. Update CLI invocation to normalize Param defaults and return clear missing-required errors.","status":"closed","priority":1,"issue_type":"bug","owner":"tbarrettwilsdon@gmail.com","created_at":"2026-02-10T12:33:06.83139-05:00","created_by":"Taylor Wilsdon","updated_at":"2026-02-10T12:36:35.051947-05:00","closed_at":"2026-02-10T12:36:35.051947-05:00","close_reason":"Implemented CLI FastAPI default normalization for Body defaults","labels":["cli","gmail"]}

fix cli mode

2bef6d0

taylorwilsdon requested a review from Copilot February 10, 2026 19:06

taylorwilsdon self-assigned this Feb 10, 2026

taylorwilsdon added the bug Something isn't working label Feb 10, 2026

Copilot AI reviewed Feb 10, 2026

View reviewed changes

refac

b099101

taylorwilsdon requested a review from Copilot February 10, 2026 19:15

Copilot AI reviewed Feb 10, 2026

View reviewed changes

taylorwilsdon added 2 commits February 10, 2026 14:20

pydantic

a4a3bdb

sentinel safeguard

487db5c

taylorwilsdon requested a review from Copilot February 10, 2026 19:25

Copilot AI reviewed Feb 10, 2026

View reviewed changes

garbage import fix

dd248f5

taylorwilsdon requested a review from Copilot February 10, 2026 21:41

Copilot AI reviewed Feb 10, 2026

View reviewed changes

taylorwilsdon merged commit bc2279b into main Feb 10, 2026
7 checks passed

		return value is Ellipsis or type(value).__name__ == "PydanticUndefinedType"

-    return value is Ellipsis or type(value).__name__ == "PydanticUndefinedType"
+    # Treat standard Python/FastAPI markers as "required"
+    if value is Ellipsis or value is inspect.Parameter.empty:
+        return True
+    # Handle Pydantic v1/v2 "undefined" sentinels without hard-coding imports.
+    # - v1: pydantic.fields.Undefined (type: UndefinedType)
+    # - v2: pydantic_core.PydanticUndefined (type: PydanticUndefinedType)
+    t = type(value)
+    type_name = getattr(t, "__name__", "")
+    type_module = getattr(t, "__module__", "")
+    if type_name in {"PydanticUndefinedType", "UndefinedType"} and type_module.startswith(
+        "pydantic"
+    ):
+        return True
+    return False

Uh oh!

Conversation

taylorwilsdon commented Feb 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants