fix(google-workspace): restore required_credential_files in SKILL.md (#16452) by briandevans · Pull Request #16470 · NousResearch/hermes-agent

briandevans · 2026-04-27T09:15:29Z

Summary

Restores the required_credential_files frontmatter block removed in feat(google-workspace): add --from flag for custom sender display name #9931 from skills/productivity/google-workspace/SKILL.md
Adds a regression test that prevents this field from being silently dropped again

The bug

PR #9931 ("feat(google-workspace): add --from flag for custom sender display name") accidentally dropped the required_credential_files YAML header while reformatting the frontmatter. This header is the mechanism by which hermes registers google_token.json and google_client_secret.json for bind-mounting into Docker/Modal remote terminal backends at container creation time.

Without this header, register_credential_files() is never called for the skill, the session-scoped ContextVar is never populated, and get_credential_file_mounts() returns an empty list when DockerEnvironment.__init__ builds the container's -v arguments. The OAuth credential files are therefore never visible inside the sandbox — setup.py fails to find them even though they exist on the host.

The fix

Restores both google_token.json and google_client_secret.json to required_credential_files. The existing register_credential_file() implementation skips files that don't exist on the host (first-time setup, google_token.json absent) without error and adds them to the missing_cred_files list that drives setup_needed = True, so the setup prompt behaviour is correct.

Test plan

Before: required_credential_files absent → fm.get("required_credential_files") returns None → test_required_credential_files_present_in_skill_md fails
After: all 3 new tests pass; 141/141 existing skills + credential-files tests unchanged
Regression guard: manually stripped field from parsed content → confirmed test assertion fires with "required_credential_files missing from google-workspace SKILL.md"

Fixes [Bug]: google-workspace can't read oauth file in remote terminals #16452
Original feature: feat: mount skill credential files into remote terminal backends (Modal/Docker) #3671 (feat: mount skill credential files into remote terminal backends)
Regression introduced by: feat(google-workspace): add --from flag for custom sender display name #9931

🤖 Generated with Claude Code

…ousResearch#16452) PR NousResearch#9931 ("feat(google-workspace): add --from flag for custom sender display name") accidentally removed the required_credential_files frontmatter block that tells hermes to bind-mount google_token.json and google_client_secret.json into Docker and Modal remote terminals before running setup.py. Without this header the credential files are never registered in the session-scoped ContextVar, so get_credential_file_mounts() returns an empty list at container creation time and the OAuth files are invisible inside the sandbox. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Restores the required_credential_files YAML frontmatter in the Google Workspace skill so Hermes can register/mount OAuth credential files into remote terminal backends (Docker/Modal), and adds a regression test to prevent the header from being dropped again.

Changes:

Re-adds required_credential_files entries for google_token.json and google_client_secret.json to skills/productivity/google-workspace/SKILL.md.
Adds a new test suite validating the frontmatter presence and basic credential-file registration/mount behavior.

Reviewed changes

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

File	Description
`skills/productivity/google-workspace/SKILL.md`	Restores the `required_credential_files` frontmatter block needed for credential passthrough into remote sandboxes.
`tests/skills/test_google_workspace_credential_files.py`	Adds regression coverage to ensure the frontmatter field exists and registers/mounts expected credential files.

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

Copilot · 2026-04-27T09:19:36Z

+            with patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}):
+                missing = register_credential_files(entries)
+
+            assert missing == [], f"Unexpected missing files: {missing}"
+            mounts = get_credential_file_mounts()
+            container_paths = {m["container_path"] for m in mounts}
+            assert "/root/.hermes/google_token.json" in container_paths
+            assert "/root/.hermes/google_client_secret.json" in container_paths


This test calls get_credential_file_mounts() after only patching HERMES_HOME inside the register_credential_files() block. get_credential_file_mounts() loads terminal.credential_files via read_raw_config() on first use, so it can read the developer machine’s real ~/.hermes/config.yaml and add mounts (including google_token.json) that make the test environment-dependent/flaky. Patch HERMES_HOME for the entire section that calls get_credential_file_mounts(), and/or explicitly reset/disable tools.credential_files._config_files (similar to tests/tools/test_credential_files.py) so config-based mounts can’t leak into these assertions.

Copilot · 2026-04-27T09:19:36Z

+            with patch.dict(os.environ, {"HERMES_HOME": str(hermes_home)}):
+                missing = register_credential_files(entries)
+
+            assert "google_token.json" in missing
+            mounts = get_credential_file_mounts()
+            container_paths = {m["container_path"] for m in mounts}
+            assert "/root/.hermes/google_client_secret.json" in container_paths
+            assert "/root/.hermes/google_token.json" not in container_paths


Same config-leak risk here: HERMES_HOME is only patched around register_credential_files(), but get_credential_file_mounts() may still read the real ~/.hermes/config.yaml (terminal.credential_files) and include extra mounts. If a developer has google_token.json configured, the assertion that it is not mounted will fail even though registration behaved correctly. Keep HERMES_HOME patched while calling get_credential_file_mounts(), and/or clear tools.credential_files._config_files between tests to keep mounts deterministic.

Copilot · 2026-04-27T09:19:37Z

+            (e["path"] if isinstance(e, dict) else e)
+            for e in entries


The set comprehension uses e["path"] for dict entries, which will raise KeyError and produce a less-informative failure if the entry schema ever changes (note register_credential_files also supports dict entries with a "name" key). Consider using e.get("path")/e.get("name") (and skipping falsy values) so the test fails with the intended assertion message rather than crashing.

Suggested change

(e["path"] if isinstance(e, dict) else e)

for e in entries

path

for e in entries

for path in [((e.get("path") or e.get("name")) if isinstance(e, dict) else e)]

if path

Three Copilot findings addressed: 1. Config-cache leak: both integration tests called get_credential_file_mounts() outside the patch.dict(HERMES_HOME) context, so _load_config_files() could still read the real ~/.hermes/config.yaml. Move the call inside the with block and reset _config_files to None before each test to ensure a fresh read under the controlled env. 2. Set-comprehension KeyError: e["path"] raises if a dict entry has no "path" key. Use e.get("path") with a conditional filter instead. 3. Unused import: removed import pytest (no pytest.mark/raises/fixture usage). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

briandevans · 2026-04-27T10:09:23Z

@copilot-pull-request-reviewer All 3 findings addressed in commit 7d0553ac0c:

Finding 1 (line 71 — config-cache leak in test_entries_are_registered_when_files_exist): Moved get_credential_file_mounts() inside the patch.dict(HERMES_HOME) context so _load_config_files() reads from the temp directory, not ~/.hermes/config.yaml. Also reset _cf._config_files = None before the test and restore it in finally to avoid cross-test cache pollution.

Finding 2 (line 100 — same leak in test_missing_token_is_reported): Same fix applied — get_credential_file_mounts() now runs inside the with patch.dict(...) block, with matching cache reset/restore.

Finding 3 (line 40 — e["path"] KeyError risk): Changed set comprehension to e.get("path") with a filter predicate (if (isinstance(e, dict) and e.get("path")) or isinstance(e, str)) so entries without a path key are skipped rather than raising KeyError.

Also removed the unused import pytest (no pytest.mark, pytest.raises, or pytest.fixture in file).

teknium1 · 2026-05-04T19:43:39Z

Salvaged via #19886 onto current main. Thanks @briandevans!

Copilot AI review requested due to automatic review settings April 27, 2026 09:15

Copilot started reviewing on behalf of briandevans April 27, 2026 09:16 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists tool/skills Skills system (list, view, manage) backend/docker Docker container execution backend/modal Modal.com cloud execution labels Apr 27, 2026

teknium1 mentioned this pull request May 4, 2026

fix(google-workspace): restore required_credential_files in SKILL.md #19886

Merged

teknium1 closed this May 4, 2026

briandevans mentioned this pull request May 11, 2026

feat(google-workspace): add gmail attachment list/get CLI verbs (#22872) #23465

Closed

2 tasks

Navigation Menu

Uh oh!

fix(google-workspace): restore required_credential_files in SKILL.md (#16452) - #16470

fix(google-workspace): restore required_credential_files in SKILL.md (#16452)#16470
briandevans wants to merge 2 commits into
NousResearch:mainfrom
briandevans:fix/google-workspace-required-credential-files-16452

briandevans commented Apr 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

Copilot AI Apr 27, 2026

Uh oh!

briandevans commented Apr 27, 2026

Uh oh!

teknium1 commented May 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

briandevans commented Apr 27, 2026

Summary

The bug

The fix

Test plan

Related

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

Uh oh!

briandevans commented Apr 27, 2026

Uh oh!

teknium1 commented May 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants