fix(skills): stop marking persisted env vars missing on remote backends by kentimsit · Pull Request #3452 · NousResearch/hermes-agent

kentimsit · 2026-03-27T21:08:27Z

What does this PR do?

Stops remote backends from marking persisted required environment variables as missing during skill readiness checks.

Today, skill_view(...) can report a skill as not ready on remote backends even when the required env var already exists in
HERMES_HOME/.env or os.environ. This PR makes readiness depend on whether the var is actually persisted, regardless of backend.

This is intentionally scoped to the maintainer-approved tools/skills_tool.py fix. It does not change remote backend command injection
behavior.

Related Issue

Fixes #3433

Type of Change

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

Changes Made

removed the remote-backend short-circuit from the initial missing env var computation in skill_view(...)
removed the remote-backend short-circuit from _remaining_required_environment_names(...)
kept the existing remote-backend guidance text for vars that are genuinely missing
updated regression tests so remotely loaded skills treat persisted vars as available
updated regression tests so remote secret capture no longer leaves setup_needed=true
added passthrough registration coverage for remotely loaded skills with locally available env vars

How to Test

Run:
uv run --extra dev python -m pytest tests/tools/test_skills_tool.py tests/tools/test_skill_env_passthrough.py -q
Set a remote backend, for example:
TERMINAL_ENV=docker
Persist a required env var in HERMES_HOME/.env
Load a skill that declares that var in required_environment_variables
Confirm:
- setup_needed is false
- missing_required_environment_variables is empty
- readiness_status is available

Test Results

81 passed in 5.63s

Platform Tested

macOS

kentimsit · 2026-03-27T21:08:46Z

Scoped to the agreed readiness fix for #3433; I’m handling the separate remote terminal passthrough gap in follow-up discussion, not in this PR.

Copilot

Pull request overview

Fixes skill readiness checks so remote terminal backends no longer mark already-persisted required environment variables as missing, ensuring skills correctly report as available when values exist in HERMES_HOME/.env or the host environment.

Changes:

Removed remote-backend short-circuits so missing required env vars are computed strictly from actual persisted/available values.
Updated readiness flow so secret-captured values clear setup_needed for remote backends when available locally.
Added/updated regression tests for remote readiness and env passthrough registration behavior.

Reviewed changes

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

File	Description
`tools/skills_tool.py`	Removes remote-only “always missing” behavior; readiness now depends on `_is_env_var_persisted(...)` results.
`tests/tools/test_skills_tool.py`	Adds regression coverage for remote backends treating persisted env vars as available; updates secret-capture expectations.
`tests/tools/test_skill_env_passthrough.py`	Adds coverage ensuring remote-backed skills still register locally available (persisted) env vars for passthrough.

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

Copilot · 2026-03-27T21:11:29Z

@@ -360,8 +360,6 @@ def _remaining_required_environment_names(
    if backend is None:
        backend = _get_terminal_backend_name()


backend is now effectively unused in _remaining_required_environment_names (it’s only defaulted but never referenced). Consider removing the backend kwarg and the if backend is None: ... block (and the call-site arg) to avoid dead parameters/assignments and future confusion (and to prevent potential unused-variable lint failures if enabled).

kentimsit · 2026-03-27T21:32:28Z

I created the follow-up stacked PR for the remote terminal passthrough fix with a clean diff against this branch:

kentimsit#1

It is marked as a draft because it depends on this PR. Once this PR merges, I’ll rebase that branch onto upstream/main so the upstream-ready diff contains only the second change.

kentimsit · 2026-03-27T21:33:30Z

@Mibayy

Referring to : #3433

I created the follow-up stacked PR for the remote terminal passthrough fix with a clean diff against this branch:

kentimsit#1

It is marked as a draft because it depends on this PR. Once this PR merges, I’ll rebase that branch onto upstream/main so the upstream-ready diff contains only the second change.

kentimsit · 2026-03-27T21:45:53Z

Good catch. That parameter became dead after removing the remote-backend short-circuit. I trimmed the unused backend plumbing in this PR without changing behavior.

…ds (#3650) Salvage of PR #3452 (kentimsit). Fixes skill readiness checks on remote backends — persisted env vars are no longer incorrectly marked as missing. Co-Authored-By: kentimsit <kentimsit@users.noreply.github.com>

teknium1 · 2026-03-29T00:52:43Z

Merged via PR #3650. Cherry-picked both commits onto current main with authorship preserved. All 81 skill tests passing.

Good catch on the readiness check — there's a related gap where the env_passthrough registry isn't automatically consulted by Docker/SSH/Modal backends when forwarding vars into the remote environment (they use a separate docker_forward_env config list). That's a separate enhancement we'll track. Thanks!

…ds (NousResearch#3650) Salvage of PR NousResearch#3452 (kentimsit). Fixes skill readiness checks on remote backends — persisted env vars are no longer incorrectly marked as missing. Co-Authored-By: kentimsit <kentimsit@users.noreply.github.com>

fix(skills): stop marking persisted env vars missing on remote backends

de924a7

Copilot AI review requested due to automatic review settings March 27, 2026 21:08

Copilot started reviewing on behalf of kentimsit March 27, 2026 21:08 View session

kentimsit mentioned this pull request Mar 27, 2026

[Bug]: required_environment_variables are not passed through correctly for remote-backed sessions (daytona, docker, etc.) #3433

Closed

1 task

Copilot AI reviewed Mar 27, 2026

View reviewed changes

refactor(skills): remove unused backend arg from readiness helper

b5f5238

teknium1 mentioned this pull request Mar 29, 2026

fix(skills): stop marking persisted env vars missing on remote backends #3650

Merged

teknium1 closed this in #3650 Mar 29, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(skills): stop marking persisted env vars missing on remote backends#3452

fix(skills): stop marking persisted env vars missing on remote backends#3452
kentimsit wants to merge 2 commits into
NousResearch:mainfrom
kentimsit:fix/skill-env-remote-readiness

kentimsit commented Mar 27, 2026

Uh oh!

kentimsit commented Mar 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 27, 2026

Uh oh!

kentimsit commented Mar 27, 2026

Uh oh!

kentimsit commented Mar 27, 2026

Uh oh!

kentimsit commented Mar 27, 2026

Uh oh!

teknium1 commented Mar 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -360,8 +360,6 @@ def _remaining_required_environment_names(
		if backend is None:
		backend = _get_terminal_backend_name()

Conversation

kentimsit commented Mar 27, 2026

What does this PR do?

Related Issue

Type of Change

Changes Made

How to Test

Test Results

Platform Tested

Uh oh!

kentimsit commented Mar 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

kentimsit commented Mar 27, 2026

Uh oh!

kentimsit commented Mar 27, 2026

Uh oh!

kentimsit commented Mar 27, 2026

Uh oh!

teknium1 commented Mar 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants