Skip to content

fix: _is_unattended_context requires positive cron/subagent signal (#1554) - #1556

Open
Lexus2016 wants to merge 2 commits into
mainfrom
evolution/issue-1554-ci-fix
Open

fix: _is_unattended_context requires positive cron/subagent signal (#1554)#1556
Lexus2016 wants to merge 2 commits into
mainfrom
evolution/issue-1554-ci-fix

Conversation

@Lexus2016

Copy link
Copy Markdown
Owner

Summary

Fixes the CI failures on PR #1550 (accepted issue #1542 — suppress pending_approval retries, 44% of terminal failures).

Problem

PR #1550 introduced _is_unattended_context() to convert pending_approval into non-retryable blocked in cron/subagent contexts. But the implementation defaulted to True whenever no interactive CLI, gateway, or cron session was detected — a catch-all "nobody's home" default. In the non-interactive CI test environment, this evaluates to True, breaking every test that expects pending_approval behavior (slices 2/8 and 8/8).

Fix (#1554)

_is_unattended_context() now requires a positive signal of an unattended context:

  1. HERMES_CRON_SESSION env var (already set in cron/scheduler.py), OR
  2. A subagent contextvar (_hermes_subagent_ctx), set in delegate_tool._run_single_child

Why a contextvar instead of HERMES_SUBAGENT?

Issue #1554 proposed checking a HERMES_SUBAGENT env var, but that env var does not exist on main (0 grep matches). Subagents are identified by the _delegate_depth attribute on the agent instance, which module-level tools/approval.py cannot access. The correct mechanism is a contextvars.ContextVar — mirroring the existing _hermes_interactive_ctx pattern (introduced for the same thread-safety reason: concurrent ACP/gateway sessions on a shared ThreadPoolExecutor). The contextvar is set in _run_single_child (the single entry point for both single and batch delegation) and reset in its finally block.

Validation

  • All 11 test_approval_cron_block.py tests pass (6 existing + 5 new)
  • 392 approval/delegation tests pass
  • 338 subagent/delegation tests pass
  • ruff check + format clean
  • The 3 test_approval.py failures (tee/sensitive-redirect patterns) are pre-existing — confirmed by running them against the pre-fix commit c25748edf; they fail identically and are caused by running as root (~/.bashrc path resolution)

Size note

236 insertions across 3 files — over the 200-line autonomous-merge cap. The three files are tightly coupled (contextvar declared in approval.py, set in delegate_tool.py, tested in the test file); no smaller coherent slice exists. Holding for human review.

Closes #1554

Co-Authored-By: Hermes Evolution evolution@hermes.ai

Hermes Evolution added 2 commits August 1, 2026 03:18
)

Adds _is_unattended_context() and _cron_blocked_result() to tools/approval.py.
Both fallback approval paths (terminal + execute_code) now check for
unattended context and return a non-retryable 'blocked' status with 'Do NOT
retry' message, preventing the 44% retry spirals.

Closes #1542

Co-Authored-By: Hermes Evolution <evolution@hermes.ai>
…1554)

The catch-all 'nobody's home' default in _is_unattended_context() returned
True in the non-interactive CI test environment, turning every test's
pending_approval into non-retryable blocked and breaking slices 2/8 and 8/8.

Fix: require a POSITIVE signal (HERMES_CRON_SESSION or subagent contextvar)
instead of defaulting to unattended. Subagents are detected via a new
thread-local contextvar (_hermes_subagent_ctx), set in _run_single_child,
mirroring the existing _hermes_interactive_ctx pattern. HERMES_SUBAGENT env
var is kept as a fallback.

Unblocks PR #1550 / accepted issue #1542 (suppress pending_approval retries —
44% of terminal failures).

Co-Authored-By: Hermes Evolution <evolution@hermes.ai>
@Lexus2016

Copy link
Copy Markdown
Owner Author

Code-review (evolution-integration, 2026-08-01): PASSED review but BLOCKED by autonomous merge gate for two policy reasons (both require human merge):

  1. HIGH_RISK_PATH: touches tools/approval.py (core approval-gating machinery).
  2. DIFF_TOO_LARGE: 236 changed lines > 200-line self-merge cap.

Verification performed: (a) Not dead code — _is_unattended_context() has 2 real call sites (check_all_command_guards, check_execute_code_guard); set_hermes_subagent_context() is called from delegate_tool._run_single_child with proper reset in finally. (b) No category error — requires a positive signal (HERMES_CRON_SESSION or the new _hermes_subagent_ctx contextvar), fixing exactly the #1554 misfire; the contextvar mirrors the existing _hermes_interactive_ctx pattern. (c) Independently confirmed HERMES_SUBAGENT does NOT exist on origin/main (the competing #1555 relied on it → dead code, closed). (d) CI fully green (8 slices + e2e). Ready for human squash-merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Bug or fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FIX] PR #1550 CI failures — _is_unattended_context too aggressive in test env

1 participant