fix(google-workspace): defer PEP 604 unions so setup.py runs on Py3.9 (#14688) by briandevans · Pull Request #15158 · NousResearch/hermes-agent

briandevans · 2026-04-24T13:22:45Z

What does this PR do?

Fixes #14688: on macOS, the agent's terminal tool invokes python3 scripts/setup.py --check (the google-workspace skill's auth entry-point) via the system interpreter /usr/bin/python3 = 3.9. The script crashes at import time on def get_optional_skills_dir(default: Path | None = None) with:

TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'

PEP 604 union syntax evaluates at function-def time on Py3.9 — the try/except ModuleNotFoundError fallback in setup.py never runs. The agent sees the crash, interprets it as "auth broken", and re-runs setup from scratch in a loop.

SKILL.md does tell agents to use $HERMES_HOME/hermes-agent/venv/bin/python, but smaller local models (e.g. Qwen3.5-35B-A3B-4bit) routinely simplify that down to plain python3, reproducing the bug on every call.

Fix

Add from __future__ import annotations (PEP 563) to the three affected files. This converts every annotation to a string so the PEP 604 unions never fire at module-load time. Py3.10+ behavior is unchanged.

Why this file set (grep'd exhaustively for PEP 604 syntax):

hermes_constants.py — shared infra with 5 X | None sites, including two module-level assignments (_wsl_detected: bool | None = None, _container_detected: bool | None = None) that evaluate eagerly even without the future import.
skills/productivity/google-workspace/scripts/setup.py — direct crash site; also uses tuple[str, str | None] at line 239.
skills/productivity/google-workspace/scripts/google_api.py — imported during setup.py --check; uses str | None and dict | None on _gws_binary/_run_gws (called out by @thedavidweng in the issue comments).

gws_bridge.py has no PEP 604 syntax — verified via AST-level scan, no pragma needed.

Related Issue

Fixes #14688

Type of Change

🐛 Bug fix (non-breaking change that fixes an issue)
✅ Tests (adding or improving test coverage)

Changes Made

Add from __future__ import annotations to hermes_constants.py, skills/productivity/google-workspace/scripts/setup.py, and skills/productivity/google-workspace/scripts/google_api.py with an inline comment linking back to google-workspace setup.py crashes under macOS system python3 (auto-reexec into venv) #14688 so a future refactor doesn't drop the pragma without thinking.
New regression test file tests/skills/test_py39_compat_google_workspace.py with 4 classes of assertion:
1. AST check that each file starts with from __future__ import annotations. Uses ast.parse walk rather than a substring scan — a docstring mentioning the pragma wouldn't pass.
2. PEP 604 usage check — each file still contains at least one X | None style annotation. If a future edit strips them all, the pragma becomes cosmetic and the pinned list should be re-evaluated; the test will flag that.
3. Smoke test that hermes_constants imports cleanly and the two functions whose annotations were problematic (get_optional_skills_dir, parse_reasoning_effort) still return expected values.
4. py_compile on each file — catches Py3.10+-only syntax beyond PEP 604 (e.g. match/case) that the pragma wouldn't cover.

All 8 new tests pass. All 14 existing tests/skills/test_google_oauth_setup.py + test_google_workspace_api.py tests still pass — no runtime behavior change under Py3.11+.

Verification

$ python3 -m pytest tests/skills/test_py39_compat_google_workspace.py tests/skills/test_google_oauth_setup.py tests/skills/test_google_workspace_api.py -v
...
================== 22 passed, 1 warning in 0.09s ==================

Proof that the pragma does what it claims (annotations become strings rather than being evaluated eagerly):

# WITH `from __future__ import annotations`
ann = get_optional_skills_dir.__annotations__
# {'default': 'Path | None', 'return': 'Path'} — strings, never evaluated

# WITHOUT the future import (the pre-fix state)
# __annotations__['default'] is a UnionType object, which requires
# Py3.10+ to even CONSTRUCT — hence the TypeError on 3.9.

Why not the reporter's second proposed fix (parents[4] path)?

The reporter diagnosed two root causes; I only applied the real one.

Their claim was that Path(__file__).resolve().parents[4] in setup.py's fallback resolves to ~/.hermes/ when hermes_constants.py actually lives at ~/.hermes/hermes-agent/hermes_constants.py, so the fallback sys.path.insert points at the wrong directory.

Verified locally this is not the case: for hermes-agent/skills/productivity/google-workspace/scripts/setup.py, parents[0..4] resolves to scripts/ → google-workspace/ → productivity/ → skills/ → hermes-agent/ — exactly the directory containing hermes_constants.py. The reporter's own traceback shows hermes_constants.py being found at the expected path before crashing with TypeError at line 60; that's inconsistent with a misrouted fallback. The crash is purely the Py3.9 type-evaluation failure.

Adding a needless / \"hermes-agent\" suffix would break the fallback on other installation layouts. Leaving that code path alone.

Out of scope for this PR (possible follow-ups)

Auto-reexec into the Hermes venv python when Py<3.10 is detected at script entry — ergonomic defense-in-depth. The current fix makes the crash go away for the reported case; reexec would also handle files that grow non-annotation Py3.10+ syntax in future (e.g. match/case). Separate change.
Broader sweep of other skills for the same X | None pattern — this PR ships the narrow google-workspace fix.

Test plan

New tests pass
Existing google-workspace tests pass (14 tests)
py_compile succeeds on all three modified files
Proved PEP 563 deferral is active: get_optional_skills_dir.__annotations__['default'] is the string 'Path | None', not a UnionType
hermes_constants.get_optional_skills_dir() and parse_reasoning_effort() still return expected values

briandevans · 2026-04-24T13:24:05Z

Heads-up on CI: the Scan PR for critical supply chain risks FAILURE here is the same false-positive from the scan-bug I wrote up in #13411 — the workflow uses a two-dot git diff ($BASE..$HEAD) which pulls in every file upstream main has drifted through since the branch was forked. On this PR that appears to include unrelated files like hermes_cli/setup.py that don't exist in this diff.

Actual files touched in this PR (git diff origin/main...HEAD):

hermes_constants.py
skills/productivity/google-workspace/scripts/google_api.py
skills/productivity/google-workspace/scripts/setup.py
tests/skills/test_py39_compat_google_workspace.py

None match any of the scanner's attack patterns (no .pth plant, no setup.py install_requires hook, no typosquat). The one-character fix for the scan is in #13411.

Other CI lanes (check-attribution, e2e) are already green; the three nix / test / check lanes were still running at comment-time.

Copilot

Pull request overview

This PR fixes a Python 3.9 import-time crash in the Google Workspace skill setup flow by deferring evaluation of PEP 604 (X | None) annotations, preventing macOS system /usr/bin/python3 (3.9) from failing at module import time.

Changes:

Add from __future__ import annotations to hermes_constants.py and the Google Workspace skill scripts to defer annotation evaluation on Py3.9.
Add a regression test ensuring the future-import guard is present and that the affected modules still contain PEP 604 unions.
Add a smoke test for hermes_constants behavior and a compile step intended to guard against newer syntax.

Reviewed changes

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

File	Description
`hermes_constants.py`	Adds deferred-annotations future import to prevent Py3.9 import-time crashes from PEP 604 unions.
`skills/productivity/google-workspace/scripts/setup.py`	Adds deferred-annotations future import to keep the auth entrypoint importable on Py3.9.
`skills/productivity/google-workspace/scripts/google_api.py`	Adds deferred-annotations future import to avoid Py3.9 import-time evaluation of unions.
`tests/skills/test_py39_compat_google_workspace.py`	Adds regression tests to ensure the pragma stays in place and basic behavior remains intact.

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

Copilot · 2026-04-24T13:26:02Z

+def test_google_workspace_setup_script_compiles() -> None:
+    """``py_compile`` proves the script is syntactically valid.  Combined
+    with the AST check above, this is the strongest proof we can give under
+    a Python 3.11 test runner that the #14688 crash is gone.
+
+    A real Python 3.9 regression run would fail at *import* (TypeError on
+    PEP 604 union), which py_compile does NOT catch — the syntax itself is
+    valid in 3.9, it's the runtime evaluation that fails.  The AST check
+    confirms the deferral pragma is in place; this confirms the file hasn't
+    grown OTHER Python 3.10+ syntax (e.g. match/case) that the pragma
+    doesn't help with.
+    """
+    import py_compile
+
+    for path in _PY39_COMPAT_FILES:
+        try:
+            py_compile.compile(str(path), doraise=True)
+        except py_compile.PyCompileError as e:
+            pytest.fail(f"{path.relative_to(REPO_ROOT)} failed to compile: {e}")


test_google_workspace_setup_script_compiles uses py_compile under the current interpreter (CI runs 3.11 per the docstring). That cannot detect Python 3.9-incompatible syntax like match/case—it will still compile successfully on 3.11—so the test/docstring currently give a false sense of Py3.9 safety. Consider replacing this with a parser-level compatibility check (e.g., ast.parse(..., feature_version=9) for 3.9 grammar) or adjusting the test name/docstring to reflect what it actually guarantees (3.11 syntax validity only).

Suggested change

def test_google_workspace_setup_script_compiles() -> None:

"""``py_compile`` proves the script is syntactically valid. Combined

with the AST check above, this is the strongest proof we can give under

a Python 3.11 test runner that the #14688 crash is gone.

A real Python 3.9 regression run would fail at *import* (TypeError on

PEP 604 union), which py_compile does NOT catch — the syntax itself is

valid in 3.9, it's the runtime evaluation that fails. The AST check

confirms the deferral pragma is in place; this confirms the file hasn't

grown OTHER Python 3.10+ syntax (e.g. match/case) that the pragma

doesn't help with.

"""

import py_compile

for path in _PY39_COMPAT_FILES:

try:

py_compile.compile(str(path), doraise=True)

except py_compile.PyCompileError as e:

pytest.fail(f"{path.relative_to(REPO_ROOT)} failed to compile: {e}")

def test_google_workspace_setup_script_parses_as_python39() -> None:

"""Parse each guarded file using Python 3.9 grammar.

Running ``py_compile`` under the active CI interpreter (3.11) only proves

the file is valid for *that* interpreter, so it would miss newer syntax

such as ``match/case`` that is invalid on Python 3.9. Parsing with

``feature_version=(3, 9)`` gives a parser-level compatibility check for

the macOS system-``python3`` constraint documented in this module.

This still does not simulate Python 3.9 runtime annotation evaluation;

``test_file_has_future_annotations_import`` remains the guard for the

original #14688 import-time failure.

"""

for path in _PY39_COMPAT_FILES:

source = path.read_text()

try:

ast.parse(source, filename=str(path), feature_version=(3, 9))

except SyntaxError as e:

pytest.fail(

f"{path.relative_to(REPO_ROOT)} is not valid Python 3.9 syntax: {e}"

)

briandevans · 2026-04-24T13:29:00Z

Thanks @copilot — fair catch. Pushed d20232f3 replacing the py_compile test with ast.parse(source, feature_version=(3, 9)) essentially per your suggestion.

Empirical proof the new test is strictly stronger:

match x: case 1: ...
py_compile (3.11):       PASS  ← old test would miss this
ast.parse(fv=(3,9)):     FAIL  ← new test correctly rejects

hermes_constants.py:     OK under 3.9 grammar
setup.py:                OK under 3.9 grammar
google_api.py:           OK under 3.9 grammar

Also parametrized the new test over the three guarded files to match the style of the other per-file tests in the module. All 10 tests pass locally. Kept test_file_has_future_annotations_import as a separate guard since PEP 604 unions are valid 3.9 grammar — the parser check won't flag them, only the runtime eval pragma would.

alt-glitch · 2026-04-24T13:40:24Z

Related to #13765 and #13789 — targeted subset of the same PEP 604 Py3.9 compat issue, scoped to google-workspace + hermes_constants.py. Fixes #14688.

briandevans · 2026-04-24T13:53:29Z

CI triage for the test FAILURE on run 24892064932:

Repro'd all 4 failures on clean origin/main (just checked out, ran the same 4 node IDs, 3 failed identically + the 4th (hindsight env-var) is xdist-ordering-sensitive and hits the same pattern):

tests/hermes_cli/test_custom_provider_model_switch.py::...test_saved_model_still_probes_endpoint
tests/hermes_cli/test_web_server.py::...test_no_single_field_categories
tests/plugins/memory/test_hindsight_provider.py::...test_local_embedded_setup_preserves_existing_key_when_input_left_blank
tests/tools/test_registry.py::...test_matches_previous_manual_builtin_tool_set

None touch hermes_constants.py or the google-workspace skill scripts this PR modifies. The focused suite on this branch (pytest tests/skills/test_py39_compat_google_workspace.py) is 10/10 green.

Happy to rebase once the main-branch failures are swept.

briandevans · 2026-04-24T13:53:30Z

Noting the cross-reference from @alt-glitch to #13765 and #13789 — framing this PR's scope vs theirs:

PR	Scope	Files	Status
#13765 (capt-marbles)	Broad Py3.9 compat sweep	~60 files	open, unclear activity
#13789 (vominh1919)	Broader compat + CLI MEDIA + model-switch	~80 files	open
#15158 (this PR)	Narrow: `hermes_constants.py` + google-workspace scripts	3 prod + 1 test = 4 files	open

Why the narrow scope:

Issue google-workspace setup.py crashes under macOS system python3 (auto-reexec into venv) #14688 specifically reports the google-workspace skill's setup.py --check crashing under macOS system /usr/bin/python3 = 3.9. That's the call-path the agent actually hits via its terminal tool.
The broader sweeps touch ~60–80 files, dropping module-level | unions into Optional[...] style across the codebase. Bigger surface, bigger review cost, bigger regression risk.
I think both can coexist — if the maintainers want to land a broader sweep, this PR's 3 files are already covered in fix: Python 3.9 PEP-604 compatibility via __future__ annotations #13765's larger diff and can be dropped from it cleanly (or this PR can be closed as superseded after the sweep merges).
Meanwhile, if the sweep is deferred for review-cost reasons, this narrow PR at least unblocks the specific google-workspace setup.py crashes under macOS system python3 (auto-reexec into venv) #14688 report.

No objection to either broader PR; just offering the scoped alternative for the one skill that's already generating support tickets.

…gle_api (NousResearch#14688) macOS ships /usr/bin/python3 = 3.9. When a local-model-backed agent uses the terminal tool to run `python3 scripts/setup.py --check`, it picks up that system interpreter rather than the Hermes venv's 3.11+. On Py3.9 expressions like `Path | None` in a function signature evaluate at def-time and raise `TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'` BEFORE the script's try/except ModuleNotFoundError fallback can run. Agents interpret the crash as "auth broken" and loop through setup. `from __future__ import annotations` (PEP 563) converts every annotation into a string so the PEP 604 unions never fire at import time. Py3.10+ users are unaffected. Files in this PR: - hermes_constants.py — shared infra, imported transitively by the skill scripts; has `Path | None`, `str | None`, `dict | None`, and two module-level `bool | None` annotations. - skills/productivity/google-workspace/scripts/google_api.py — imported by setup.py's `--check` path; carries `str | None` and `dict | None` on `_gws_binary`/`_run_gws`. Note: the equivalent fix for `scripts/setup.py` already landed on main via NousResearch#13038 ("ship google-workspace deps as [google] extra; make setup.py 3.9-parseable"). The test file pins all three files (hermes_constants, setup, google_api) so the regression guard catches future drift on any of them. Tests (tests/skills/test_py39_compat_google_workspace.py): - AST check that each file starts with `from __future__ import annotations` (substring scan would pass false on a docstring mention). - Verifies each file still contains PEP 604 unions — if a future edit strips them all, the pinned list should be reconsidered. - Smoke test that hermes_constants loads and the previously-crashing `get_optional_skills_dir` and `parse_reasoning_effort` return their expected values. - ast.parse(source, feature_version=(3, 9)) on each file — catches any Py3.10+-only syntax beyond PEP 604 (e.g. match/case). Replaces the prior `py_compile`-based check, which under the CI interpreter (3.11) happily compiled 3.10+-only syntax — Copilot's review on NousResearch#15158 flagged this and suggested ast.parse with feature_version. No runtime behavior change on Py3.10+. On Py3.9 the difference is an import-time TypeError becoming a clean module load. Closes NousResearch#14688 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

briandevans · 2026-04-30T08:16:00Z

Rebased onto current origin/main (was 923 commits behind). Setup.py portion was already salvaged to main via #13038 ("ship google-workspace deps as [google] extra; make setup.py 3.9-parseable"), so this PR is now narrower:

hermes_constants.py — adds from __future__ import annotations (still needed; main keeps Path | None, str | None, etc. without the pragma).
skills/productivity/google-workspace/scripts/google_api.py — adds the pragma (still uses str | None, dict | None).
tests/skills/test_py39_compat_google_workspace.py — regression guard pinning all three files (incl. the already-fixed setup.py) so future drift on any of them fails CI.

Squashed the original two-commit history into one commit on rebase. Empirical verification of the test-file change Copilot suggested (ast.parse + feature_version=(3,9)) was preserved in the new commit message.

Test run on rebased branch:

$ uv run --with pytest --with pytest-xdist python3 -m pytest tests/skills/test_py39_compat_google_workspace.py -v
======================= 10 passed, 10 warnings in 1.73s ========================

Sanity-imported hermes_constants on system python3 after rebase: get_optional_skills_dir() returns the expected ~/.hermes/optional-skills path.

Cross-PR positioning unchanged: still narrower scope than #13765 / #13789 (broad Py3.9 compat sweeps that are still open). This PR ships only the two files that are demonstrably reachable from the agent terminal-tool path that triggered the original report.

briandevans · 2026-05-18T23:22:04Z

Closing to keep the queue clean — happy to reopen if this is still useful. The setup.py portion was already salvaged via #13038, and the project now requires Python ≥3.11 (per pyproject.toml), so the PEP 604 deferral is no longer load-bearing.

Copilot AI review requested due to automatic review settings April 24, 2026 13:22

Copilot started reviewing on behalf of briandevans April 24, 2026 13:23 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent loop, run_agent.py, prompt builder labels Apr 24, 2026

This was referenced Apr 24, 2026

fix(honcho): pinPeerName opt-in keeps memory unified across platforms (#14984) #15162

Closed

test(model-switch): assert fetch_api_models kwargs match current signature (#15243) #15246

Closed

alt-glitch mentioned this pull request Apr 27, 2026

[Bug]: google_api.py crashes on Python 3.9 — missing from __future__ import annotations #16257

Open

1 task

briandevans force-pushed the fix/py39-compat-google-workspace-skill branch from d20232f to 85a1cb3 Compare April 30, 2026 08:15

alt-glitch mentioned this pull request May 8, 2026

Crash on Python <3.10: TypeError with X | None type hints #21798

Open

briandevans closed this May 18, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(google-workspace): defer PEP 604 unions so setup.py runs on Py3.9 (#14688)#15158

fix(google-workspace): defer PEP 604 unions so setup.py runs on Py3.9 (#14688)#15158
briandevans wants to merge 1 commit into
NousResearch:mainfrom
briandevans:fix/py39-compat-google-workspace-skill

briandevans commented Apr 24, 2026

Uh oh!

briandevans commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

briandevans commented Apr 24, 2026

Uh oh!

alt-glitch commented Apr 24, 2026

Uh oh!

briandevans commented Apr 24, 2026

Uh oh!

briandevans commented Apr 24, 2026

Uh oh!

briandevans commented Apr 30, 2026

Uh oh!

briandevans commented May 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

briandevans commented Apr 24, 2026

What does this PR do?

Fix

Related Issue

Type of Change

Changes Made

Verification

Why not the reporter's second proposed fix (parents[4] path)?

Out of scope for this PR (possible follow-ups)

Test plan

Uh oh!

briandevans commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

briandevans commented Apr 24, 2026

Uh oh!

alt-glitch commented Apr 24, 2026

Uh oh!

briandevans commented Apr 24, 2026

Uh oh!

briandevans commented Apr 24, 2026

Uh oh!

briandevans commented Apr 30, 2026

Uh oh!

briandevans commented May 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants