test(interrupt-propagation): seed provider attr on bare agent to match __init__ by briandevans · Pull Request #12908 · NousResearch/hermes-agent

briandevans · 2026-04-20T08:14:48Z

TL;DR

Same class of bug as my landed #12574 — a test stub that uses AIAgent.__new__(AIAgent) skips __init__ and therefore misses self.provider. The full tests/ suite under pytest-xdist intermittently hits a code path that reads self.provider directly (without getattr), causing AttributeError: 'AIAgent' object has no attribute 'provider'.

Test-only change. Zero production-code touched.

Symptom

From the Tests run on #12889 (24655153236):

FAILED tests/run_agent/test_interrupt_propagation.py::TestInterruptPropagationToChild::test_interrupt_during_child_api_call_detected
- AttributeError: 'AIAgent' object has no attribute 'provider'

Appears on every recent PR's Tests job under -n auto; passes in isolation and under most narrower invocations.

Root cause

AIAgent.__init__ sets self.provider at run_agent.py:736. The bare-agent helper in this test file bypasses __init__:

def _make_bare_agent(self):
    from run_agent import AIAgent
    agent = AIAgent.__new__(AIAgent)
    agent._interrupt_requested = False
    # … other interrupt-related attrs …
    # provider is NOT set
    return agent

test_interrupt_during_child_api_call_detected sets child.client = MagicMock() expecting the Mock fast-path in _create_request_openai_client to short-circuit:

# run_agent.py:4921
if isinstance(primary_client, Mock):
    return primary_client  # short-circuit

Under pytest-xdist worker-import ordering, the short-circuit doesn't always engage, and the code reaches _create_openai_client at run_agent.py:4660/4671:

if self.provider == "copilot-acp" or ...:   # direct access — no getattr
if self.provider == "google-gemini-cli" or ...:

→ AttributeError.

Fix

Seed agent.provider = "" in _make_bare_agent, matching what __init__ would produce:

# AIAgent.__init__ sets ``provider`` (run_agent.py:736).  The
# client-creation path at ``_create_openai_client`` (run_agent.py:4660, 4671)
# accesses ``self.provider`` directly — not via ``getattr`` — so a bare agent
# built with ``__new__`` that reaches that path raises AttributeError.
# Seed the attribute defensively so these interrupt tests don't depend on
# worker-import ordering.
agent.provider = ""

Narrow scope — explicitly not changed

No production-code change. The self.provider accesses in _create_openai_client are correct for the real call path (real agents always have provider set by __init__); the test harness just needs to mirror that invariant.
No other bare-agent attributes added. Only the one the reported failure references, same as the test(concurrent-interrupt): add missing _apply_pending_steer_to_tool_results no-op to _Stub #12574 precedent. If a future refactor adds another missing attr it can be a separate follow-up.
No change to _make_bare_agent's intent — it's still a bare stub wiring only what interrupt tests need.

Validation

source venv/bin/activate
python -m pytest tests/run_agent/test_interrupt_propagation.py -q
# 7 passed

Broader tests/run_agent/ under -n auto → 781 passed, 7 skipped, 0 new failures. The 2 test_concurrent_interrupt failures that remain are the separately-tracked baseline handled by #12574 (landed as 1cf10b45).

Same surgical pattern as #12574 (landed — _Stub in test_concurrent_interrupt.py missing _apply_pending_steer_to_tool_results). Each time a prod refactor adds a new attribute or method read unconditionally along a path the test stub reaches, the bare-stub test harness drifts and needs a one-line seed. Each fix is test-only and reduces the baseline-flake set that blocks every contributor's CI.

_{Co-authored via LLM assistance; I've reviewed every line and am responsible for correctness.}

…match __init__ ``tests/run_agent/test_interrupt_propagation.py::_make_bare_agent`` constructs an ``AIAgent`` via ``__new__``, bypassing ``__init__``, and manually wires up the interrupt-related attributes used by the tests. ``AIAgent.__init__`` (``run_agent.py:736``) sets ``self.provider`` but ``_make_bare_agent`` did not. Under the full ``tests/`` suite with pytest-xdist (``-n auto``), the ``test_interrupt_during_child_api_call_detected`` path can reach ``_create_openai_client`` (``run_agent.py:4660``, ``4671``) which accesses ``self.provider`` directly (not via ``getattr``), raising:: AttributeError: 'AIAgent' object has no attribute 'provider' The mocked-client fast-path in ``_create_request_openai_client`` normally short-circuits before reaching that code, but under xdist worker-import ordering the short-circuit doesn't always engage, so the test fails intermittently in CI while passing in isolation and under broader-but-not-full module invocations. Fix --- Seed ``agent.provider = ""`` in the ``_make_bare_agent`` helper so the bare agent matches what ``__init__`` would produce for the same attribute. Same surgical pattern as the landed ``1cf10b45`` (NousResearch#12574) ``_Stub`` fix in ``tests/run_agent/test_concurrent_interrupt.py`` — test-only, zero production-code change, minimal scope. Validation ---------- ``source venv/bin/activate && python -m pytest tests/run_agent/test_interrupt_propagation.py -q`` → **7 passed**. Broader ``tests/run_agent/`` under ``-n auto`` → **781 passed, 7 skipped, 0 new failures** (the 2 existing ``test_concurrent_interrupt`` failures are the separately-tracked baseline handled by NousResearch#12574). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Test-only change to eliminate an intermittent pytest-xdist failure where a bare AIAgent stub (constructed via __new__) can reach code that reads self.provider directly and crash with AttributeError.

Changes:

Seed agent.provider = "" in the bare-agent factory used by interrupt propagation tests.
Add an explanatory comment documenting why the attribute is required when bypassing __init__.

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

… (follow-up to NousResearch#12908) Previous commit seeded only ``provider``. The next CI run (Tests [24655838688](https://github.com/NousResearch/hermes-agent/actions/runs/24655838688)) shows the same test now trips on ``self.model``: FAILED test_interrupt_during_child_api_call_detected - AttributeError: 'AIAgent' object has no attribute 'model' Hit pattern NousResearch#8 from my own pre-push self-review checklist: "fixed one call site, missed sibling with same pattern." The ``_create_openai_client`` path reads ``self.provider``, ``self.model``, and ``self.base_url`` directly — if the mocked-client short-circuit doesn't engage (as apparently happens under xdist here), each missing attr surfaces as its own ``AttributeError``. Seed all three core identity attrs that ``AIAgent.__init__`` sets (run_agent.py:708, 734, 736) to make the bare-agent stub robust against any code path that touches them. Zero production-code change. 7 focused tests still pass. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

briandevans · 2026-04-20T08:28:09Z

Self-correction: my initial commit only seeded provider. The follow-up CI run (24655838688) showed the same test now tripping on self.model:

FAILED test_interrupt_during_child_api_call_detected
- AttributeError: 'AIAgent' object has no attribute 'model'

Hit pattern #8 from my pre-push self-review checklist ("fixed one call site, missed sibling with same pattern"). _create_openai_client reads self.provider, self.model, AND self.base_url directly — any of the three can surface as its own AttributeError when the mocked-client short-circuit doesn't engage (apparently the case under xdist here).

Pushed 4725a861 which seeds all three core AIAgent.__init__ identity attrs defensively. 7 focused tests still pass. Zero production change.

Baseline-failure classification for the Tests job is unchanged (same 4 pre-existing xdist flakes; test_interrupt_propagation now fixed by this PR, the others covered by separate PRs or still baseline).

briandevans · 2026-04-20T09:05:44Z

CI on 4725a861 (24656377417):

✅ test_interrupt_propagation::test_interrupt_during_child_api_call_detected now passes in CI — the trio-seed (provider + model + base_url) closed the remaining AttributeError path.

Remaining failures on this run (3, down from 4 on the initial push) are all standing baselines that pre-exist this PR and reproduce on clean origin/main:

test_gemini_provider.py::test_provider_models_exist — catalog drift (gemini-2.5-pro dropped from curated list)
2× test_modal_sandbox_fixes.py::TestToolResolution — xdist worker-import ordering (the exact same flake pattern as the other baseline failures I've audited across the repo)

None touch test_interrupt_propagation.py or anything this PR changes.

briandevans · 2026-04-21T08:21:16Z

Supply-chain FAILURE on this PR is a false positive from a scan bug, not a real finding. Opened #13411 with a root-cause analysis and a one-character fix.

Quick version: .github/workflows/supply-chain-audit.yml computes the PR diff with git diff "$BASE".."$HEAD" (two-dot) which includes every file main has drifted through since the PR was forked. On this PR, that adds hermes_cli/setup.py (modified upstream, not by this PR) to the scan's file list, which trips the install-hook check. The 3-dot merge-base diff shows only this PR's actual 2 files (neither of which matches any attack pattern).

Was masked pre-19db7fa3 because that commit changed the fail condition from critical == 'true' to found == 'true', unmasking the pre-existing 2-dot bug.

briandevans · 2026-04-24T13:08:18Z

Closing as superseded — all three attrs this PR seeded (provider, model, _base_url) are now set on the bare-agent fixture upstream via 62cbeb63 (#13363 "test: stop testing mutable data"). Current origin/main tests/run_agent/test_interrupt_propagation.py:

agent.provider = "openrouter"
agent.model = "test/model"
agent._base_url = "http://localhost:1234"

Exactly the trio this PR proposed. Clean landing via the broader test-robustness sweep — closing to keep the queue tidy.

Copilot AI review requested due to automatic review settings April 20, 2026 08:14

Copilot started reviewing on behalf of briandevans April 20, 2026 08:15 View session

Copilot AI reviewed Apr 20, 2026

View reviewed changes

briandevans mentioned this pull request Apr 21, 2026

ci(supply-chain): use merge-base diff (A...B) so upstream drift doesn't false-positive #13411

Closed

19 tasks

alt-glitch added type/test Test coverage or test infrastructure P3 Low — cosmetic, nice to have comp/agent Core agent loop, run_agent.py, prompt builder labels Apr 22, 2026

briandevans closed this Apr 24, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(interrupt-propagation): seed provider attr on bare agent to match init#12908

test(interrupt-propagation): seed provider attr on bare agent to match init#12908
briandevans wants to merge 2 commits into
NousResearch:mainfrom
briandevans:fix/test-stub-interrupt-propagation-provider

briandevans commented Apr 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

briandevans commented Apr 20, 2026

Uh oh!

briandevans commented Apr 20, 2026

Uh oh!

briandevans commented Apr 21, 2026

Uh oh!

briandevans commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

briandevans commented Apr 20, 2026

TL;DR

Symptom

Root cause

Fix

Narrow scope — explicitly not changed

Validation

Related

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

briandevans commented Apr 20, 2026

Uh oh!

briandevans commented Apr 20, 2026

Uh oh!

briandevans commented Apr 21, 2026

Uh oh!

briandevans commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants