fix(relay): avoid concurrent turn scope corruption

[bbednarski9]

What does this PR do?

Fixes the NeMo Relay non-LIFO scope failure when two Hermes turns overlap for the same session. Relay 0.6 gives a session one physical LIFO scope stack; opening a scope for both turns lets turn B sit above turn A, so finishing A first attempts to pop a non-top handle.

Hermes now admits only one instrumented turn per shared session. An overlapping turn remains a normal Hermes turn but is marked relay_enabled=False, so it cannot create turn, task, model, tool, logical-call, or shared-metrics Relay scopes. The context-local skipped state is preserved until that turn ends—even if the first turn ends while the skipped turn is current—and stale copied contexts remain fail-closed instead of falling back to the session handle.

Relay version compatibility

This is the conservative path for Hermes's current nemo-relay>=0.6.0,<0.7 dependency. It is forward-compatible with Relay 0.7.x as written, but still deliberately instruments only one overlapping turn. A follow-up can capability-detect Relay 0.7.x's public use_scope_stack() API and give every turn an isolated stack while retaining this admission guard as the Relay 0.6 fallback.

Related Issue

Fixes #73746

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✨ New feature (non-breaking change that adds functionality)
• 🔒 Security fix
• 📝 Documentation update
• ✅ Tests (adding or improving test coverage)
• ♻️ Refactor (no behavior change)
• 🎯 New skill (bundled or hub)

Changes Made

• agent/relay_runtime.py
  • reserves one active Relay turn per profile/session;
  • skips every managed Relay path for an overlapping turn;
  • replaces out-of-order ContextVar token resets with an explicit predecessor chain, so ending turn A cannot erase current turn B;
  • keeps copied contexts for ended/skipped turns fail-closed.
• hermes_cli/observability/relay_shared_metrics.py
  • gates lifecycle hooks before lazy task/model scope creation.
• run_agent.py
  • avoids the explicit shared-metrics task scope for a skipped turn while preserving legacy runtime shims.
• tests/hermes_cli/test_relay_shared_metrics_runtime.py
  • makes the Relay fake enforce the native LIFO contract;
  • covers A/B overlap, shared-metrics pre_api_request, ending A before continuing B, copied contexts, and subagent parent restoration.

How to Test

pytest -q \
  tests/agent/test_relay_llm.py \
  tests/agent/test_relay_tools.py \
  tests/agent/test_auxiliary_relay.py \
  tests/hermes_cli/test_relay_shared_metrics.py \
  tests/hermes_cli/test_relay_shared_metrics_runtime.py \
  tests/plugins/test_nemo_relay_plugin.py

Current macOS arm64 validation against the pinned Relay 0.6 binding:

• focused Relay suites: 57 passed;
• Ruff: All checks passed!;
• real native overlap E2E: first turn instrumented, second turn intentionally skipped, second-turn tool callback completed after the first ended, and non_lifo_errors=0;
• expanded suite run reached 1895 passed before an unrelated order-dependent test_file_safety failure that passes in isolation on both rebased branches. The full-suite checkbox remains unchecked pending CI's isolated slices.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix/feature (no unrelated commits)
• I've run pytest tests/ -q and all tests pass
• I've added tests for my changes (required for bug fixes, strongly encouraged for features)
• I've tested on my platform: macOS arm64

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A

For New Skills

Not applicable.

Screenshots / Logs

Real Relay 0.6 overlap result:

{'first_instrumented': True, 'second_instrumented': False, 'skipped_turn_callbacks': 1, 'non_lifo_errors': 0}

[alt-glitch]

This was generated by AI during triage.

Related to #73746 and merged #73493, but this is not a duplicate: #73493 repairs scope cleanup, while this PR prevents overlapping turns from opening non-LIFO Relay scopes. Current CI has one failing Python test: an existing mock lacks the new relay_enabled attribute.

[teknium1]

Thanks for isolating the overlap from the earlier logical-call cleanup work. The current-main premise is verified: agent/relay_runtime.py:603-624 pushes a turn scope before recording an active turn, while agent/relay_runtime.py:640-657 later pops that handle.

Problems

• The added direct-runtime regression cannot exercise the native LIFO failure it describes. _Relay._scope_pop() accepts any handle without checking whether it is the current top (tests/hermes_cli/test_relay_shared_metrics_runtime.py:71-73), so it would not fail for the reported out-of-order pop.

Suggested changes

• Make the fake enforce LIFO (or add a real_binding_runtime regression) and verify that ending the first turn while the second overlaps does not produce a non-top-of-stack pop. The new assertions for relay_enabled are useful, but this would validate the Relay contract directly.

Automated hermes-sweeper review.

[bbednarski9]

Addressed Teknium's LIFO-regression feedback in af30b39.

The direct-runtime fake now maintains a context-local scope stack and rejects any pop whose handle is not at the top, matching the native Relay contract and error (scope handle is not at the top of the stack). I also added test_direct_runtime_fake_enforces_lifo_scope_contract, which deliberately pushes two scopes and proves an attempt to pop the first one is rejected. This makes the test harness itself fail if it stops enforcing LIFO.

The overlapping-turn regression now runs against that strict fake, finishes the first turn while the second still overlaps, and asserts that no scope.pop.rejected event occurred and that the first turn closed exactly once. The second turn remains deliberately uninstrumented, so it cannot place a scope above the first turn. The related shared-metrics regression also verifies that a skipped turn cannot lazily reopen task/model scopes through pre_api_request.

Validation on the branch's Relay 0.6 environment:

3 passed, 15 deselected

[GottZ]

This was generated by AI during triage.

Summary

One PR addresses #73746. #74864 prevents overlapping turns in one Relay session from interleaving scopes by instrumenting only the first active turn; this directly removes the tested non-LIFO overlap path, but does not cover other cleanup-ordering causes or the separately reported stale-session gateway deadlock.

Related pull requests

• fix(relay): avoid concurrent turn scope corruption #74864 best fix — (+175/-16) — partial fix; keep open with a salvage path: the concurrency admission guard suppresses turn, task, and managed-execution scopes for later overlapping turns, while the updated fake now enforces LIFO and the regression verifies that ending the first turn causes no rejected pop. This remains consistent with the MAINTAINER-BOT keep_open verdict, although its original LIFO-test objection is addressed by the current diff; the valuable salvage is the single-instrumented-turn fallback and its strict overlap regression.

Suggested consolidation

Keep #74864 open with a salvage path: retain the single-instrumented-turn guard and LIFO-enforcing regression as the focused fix for the demonstrated overlap mechanism, and track other cleanup causes and stale-session gateway recovery separately because they are outside this diff. There are no listed duplicate PRs to close.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I73746(["issue #73746 (open)"])
    P74864["PR #74864 (open)"]
    P74864 -->|best fix| I73746
    class I73746 open
    class P74864 open
    class P74864 best
    class P74864 target
    click I73746 "https://github.com/NousResearch/hermes-agent/issues/73746"
    click P74864 "https://github.com/NousResearch/hermes-agent/pull/74864"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 11 kB of PR diffs, 7 kB of issue/PR text, 3 kB of discussion (3 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[jquesnelle]

@bbednarski9 one quick item:

Success-path finish_task_run is not gated on task_started — run_agent.py:7681.
The new gate skips start_task_run for a skipped turn run_agent.py:7629-7634, but the success path still calls finish_task_run(**task_context, result=result) unconditionally. Before this PR that was harmless because start always ran, so the (session_id, task_id) lookup always hit. Now, for a never-started task, _task_session(..., allow_task_id_fallback=True) (relay_shared_metrics.py:481-506) falls through to the cross-session task_id fallback: if exactly one other session has a live task with the same task_id string (explicit task_ids passed by callers, cron, etc.), the skipped turn's finish will close that other session's task scope — popping a scope on a stack it doesn't own, i.e. a new instance of the very bug class being fixed. The except path is already gated correctly (if task_started and not task_finished, line 7695); mirror it:

task_finished = True
if task_started:
    finish_task_run(**task_context, result=result)

[bbednarski9]

@jquesnelle thanks for that. Addressed in 80c7ccf