hermes-agent/tests/tools/test_clarify_gateway.py
Teknium 597615ade4
fix(ci): make tests, workflows, and attribution reliable under load (#66373)
* feat(attribution): conflict-free contributor mappings via contributors/emails/ directory

The AUTHOR_MAP dict in scripts/release.py was a merge-conflict magnet:
every concurrent salvage PR appended entries to the same lines of the
same file, so parallel PRs re-conflicted on every merge to main.

New system: one file per email under contributors/emails/ — filename is
the commit-author email, first non-comment line is the GitHub login.
File additions never conflict, so any number of PRs can add mappings
concurrently.

- scripts/release.py: AUTHOR_MAP is now LEGACY_AUTHOR_MAP (frozen)
  merged with the directory at import time (directory wins). All
  existing consumers (resolve_author, contributor_audit.py) unchanged.
- scripts/add_contributor.py: idempotent CLI to add a mapping; refuses
  conflicting reassignments (incl. against the legacy map), validates
  email/login shapes.
- contributor-check.yml: attribution gate now accepts a mapping file OR
  a legacy entry; failure message prints the exact add_contributor
  command. Also auto-resolves bare <login>@users.noreply.github.com
  emails is intentionally NOT added (kept id+login form only, matching
  previous behavior).
- contributor_audit.py: guidance now points at add_contributor.py.
- tests/scripts/test_contributor_map.py: 12 tests covering loader,
  merge precedence, CLI idempotency/conflict/validation, subprocess E2E.

* feat(ci): one-shot per-file flake retry in the parallel test runner

A failing test FILE is re-run once in a fresh subprocess. Pass-on-retry
counts as green but is loudly reported in a '⚠ FLAKY' summary section
(with both attempts' output preserved) so the flake gets fixed instead
of eating a full-run rerun. Deterministic failures fail both attempts —
regressions cannot be laundered green.

- --file-retries N / HERMES_TEST_FILE_RETRIES (default 1, 0 disables)
- E2E verified: simulated first-run-fail flake goes green with banner;
  deterministic failure still exits 1; retries=0 restores old behavior.

This converts the dominant CI failure mode (one timing-sensitive test
flaking a 4600-test shard, requiring a manual 10-minute rerun and an
agent triage loop) into a self-healing retry that costs one file's
runtime.

* test(approval): loosen wall-clock perf bounds 0.15s -> 2.0s

These guard against catastrophic regex backtracking (seconds-to-minutes
class), but 0.15s is within scheduler-stall noise on loaded shared CI
runners — test_max_accepted_separator_free_input_is_fast failed a CI
shard this week on runner load alone. 2.0s still catches the regression
class with zero flake surface.

* fix(ci): job timeouts everywhere + retries on all network installs

Reliability pass over every workflow:
- timeout-minutes on all 21 jobs that lacked one (a hung job previously
  burned the 6-hour default runner budget)
- ./.github/actions/retry wrapped around every network-fetching install
  that lacked it: pip installs (deploy-site, skills-index), npm ci
  (deploy-site website, upload_to_pypi web + ui-tui), uv sync (docker
  test deps). Deterministic build steps (npm run build) deliberately
  NOT retried — split into separate steps so a real build failure fails
  fast instead of retrying 3x.

* docs(agents): document the file-retry flake policy

* fix(ci): curl retries on deploy hook + skills-index probe

* fix(ci): kill the remaining transient-failure classes in workflows + Dockerfile

From the workflow reliability audit:
- tests.yml: duration-cache restore had NO restore-keys while saves use
  run_id-suffixed keys — the cache never matched once, so LPT slicing
  always ran blind and unbalanced slices pushed heavy files toward the
  per-file timeout. One-line restore-keys fixes slice balancing.
- Label gates (lint ci-reviewed, supply-chain mcp-catalog-reviewed):
  'gh pr view || true' turned an API blip into 'label absent' → false
  BLOCKING failure. Now 3x retry, and API failure is reported as an API
  failure instead of a missing label.
- detect-changes action: compare API retried before failing open (was
  silently running all lanes on any blip).
- uv-lockfile-check: 'uv lock --check' resolves against PyPI — retried
  so registry blips don't read as 'lockfile stale'.
- docker.yml merge job: imagetools create retried (Docker Hub eventual
  consistency on just-pushed digests).
- Dockerfile: apt-get Acquire::Retries=3; s6-overlay ADDs converted to
  curl --retry 3 (ADD cannot retry; checksums still enforced); npm
  --fetch-retries=5; playwright chromium fetch retried 3x.
- Advisory artifact uploads (per-slice durations, ci-timings report)
  get continue-on-error so an artifact-service blip can't fail a green
  test slice.

* fix(tests): kill the two root-cause flakes — leaking pre-warm timer + env-dependent provider list

- test_tui_gateway_server.py: session.create / non-eager session.resume
  arm a 50ms threading.Timer (_schedule_agent_build) that outlives its
  test and fires into the NEXT test's _make_agent mock, racily
  corrupting captured state (the recurring session_resume shard
  failures). Replaced the per-test whack-a-mole stub with a module-wide
  autouse fixture; the 3 worker-lifecycle tests that genuinely need the
  deferred build opt back in via @pytest.mark.real_agent_prewarm (new
  marker in pyproject).
- test_api_key_providers.py: PROVIDER_ENV_VARS is now derived from the
  live PROVIDER_REGISTRY instead of a hand-list that had drifted
  (missing HF_TOKEN / DEEPINFRA_API_KEY) — resolve_provider('auto')
  tests failed on any machine with HF_TOKEN exported. E2E-verified with
  HF_TOKEN/DEEPINFRA_API_KEY set: 42/42 pass.

* test: de-flake 30 timing-sensitive test files for loaded CI runners

Root-cause fixes from the flake audit (session-DB mining + repo sweep):

Event-based sync instead of sleep-sync:
- title_generator: mock sets threading.Event, wait(10) replaces
  sleep(0.3) hoping the daemon thread got scheduled
- docker zombie_reaping / profile_gateway: poll-for-state helpers
  replace fixed 1-3s sleeps (s6 transitions + SIGCHLD reaping are async)
- process_registry tree test: select()-bounded readline replaces an
  unbounded blocking read (parent wedge now fails THIS test with a clear
  message instead of an opaque rc=124 file kill); SIGTERM grace 1s->2s
  (the 1s partition window mid-interpreter-startup is how a child PID
  escaped the live-system guard in CI)

Timeout raises (loaded 8-way-sliced runners see ~5s scheduling floors;
all of these complete in ms-to-1s when healthy so the raises cost
nothing on green runs):
- subprocess/thread waits <= 2s raised to 10-15s across mcp_tool,
  mcp_circuit_breaker, mcp_reconnect_retry_reset, mcp_parked_self_probe,
  mcp_cancelled_error_propagation, registry, clarify_gateway, interrupt,
  voice_cli_integration, docker_environment, session_store_lock_io,
  planned_stop_watcher, cli_interrupt_subagent, thread_scoped_output
  (joins now also assert not is_alive() so stragglers fail loudly)
- wall-clock discrimination ceilings loosened where the guarded hang is
  10x larger: local_background_child_hang 4s->10s, interrupt_cleanup
  setup 5s->20s + pgid-exit 30s->60s, mcp_stability grandchild spinup
  5s->15s, protocol/gil-starvation fast-handler 0.5s->2s,
  iso_certify_seam 1.5s->5s, wait_for_mcp_discovery 0.1s->1s
- narrow assertion windows widened: honcho first-turn wait 0.4..0.65 ->
  0.25..2.0 (property is bounded-not-hung, not an exact wall-clock);
  compression fork-lock TTL 1s->3s (12 refresh chances per lease);
  compression-lock expiry margins symmetric (ttl 0.05->0.5, sleep 1.0)
- telegram hung-DNS bound 1.0->1.4 (fake hang is 1.5s — must stay under)

* fix(tests): repair indentation from de-flake batch edit

* fix(tests): harden env isolation and replace remaining sleep-sync races

The full 42k-test run and complete npm check surfaced three more classes:

- Environment isolation: local ~/.honcho defaultHost and SSH_* variables
  leaked into Python/TUI tests. Pin the default Honcho host in the
  hermetic fixture, isolate the one fallback test from ~/.honcho, and
  blank SSH_* around terminalSetup tests. This flipped 20 false failures
  back to deterministic behavior on developer machines.
- Background-thread sleep-sync: Honcho async writer tests patched
  time.sleep globally, then busy-polled with that same mocked sleep. Under
  full-suite load the poller could starve the writer. Each test now waits
  on an Event emitted by the exact flush/retry transition; 30/30 passed
  under 15-way contention.
- Desktop streaming: the test slept 80ms and assumed a 500ms timer could
  not fire before its assertion. A loaded runner descheduled the test for
  >500ms and both chunks arrived. Producer controls now gate second-chunk
  and completion transitions explicitly.

Also make file-retry observability complete: a self-healed flaky file now
prints BOTH attempts' full output in the FLAKY summary. Two behavioral
runner tests prove pass-on-retry is green+loud+traceback-preserving, while
a deterministic failure remains red.

* refactor(ci): use gh bot pat, better retries

refactor(ci): use retry action for PR label fetch
the retry action now captures stdout as a step output, so it can serve
double duty: retry + output capture for commands like 'gh pr view' whose
result must be consumed by later steps.

Retry action gains:
- 'stdout' output (heredoc-delimited to preserve newlines)
- tee to temp file so stdout still streams to the job log
- step id 'retry' for output reference

Both lint.yml and supply-chain-audit.yml now use the retry action
directly with 'command: gh pr view ...' and read
steps.<id>.outputs.stdout.

ci: use AUTOFIX_BOT_PAT for all gh CLI / GitHub API auth

Replace secrets.GITHUB_TOKEN and github.token with
secrets.AUTOFIX_BOT_PAT across all workflows and composite actions
that use the gh CLI or GitHub API. The PAT has consistent permissions
across fork PRs (where GITHUB_TOKEN is read-only), avoids API rate
limit sharing with the default token, and is already used by
js-autofix.yml for the same reasons.

19 sites swapped across 9 files:
- lint.yml (3): label fetch, comment post/edit, comment update
- supply-chain-audit.yml (5): scan, critical comment, unbounded dep
  comment, label fetch, mcp-catalog comment
- lockfile-diff.yml (1): PR comment post/update
- skills-index-freshness.yml (1): issue creation on degraded probe
- skills-index.yml (2): index build, trigger deploy workflow
- upload_to_pypi.yml (2): release view poll, release upload
- ci.yml (1): timings report
- deploy-site.yml (2): skills index crawl
- detect-changes/action.yml (1): compare API call

---------

Co-authored-by: ethernet <arilotter@gmail.com>
2026-07-17 20:55:24 +00:00

252 lines
9.4 KiB
Python

"""Tests for the gateway-side clarify primitive (tools/clarify_gateway.py).
The clarify tool needs to ask the user a question and block the agent
thread until they respond. These tests cover the module-level state
machine: register, wait, resolve via button, resolve via text-fallback,
"Other"-button text-capture flip, timeout, session boundary cleanup.
"""
from __future__ import annotations
import threading
import time
from concurrent.futures import ThreadPoolExecutor
def _clear_clarify_state():
"""Reset module-level state between tests."""
from tools import clarify_gateway as cm
with cm._lock:
cm._entries.clear()
cm._session_index.clear()
cm._notify_cbs.clear()
class TestClarifyPrimitive:
"""Core register/wait/resolve mechanics."""
def setup_method(self):
_clear_clarify_state()
def test_button_choice_resolves_wait(self):
"""resolve_gateway_clarify unblocks wait_for_response with the chosen string."""
from tools import clarify_gateway as cm
cm.register("id1", "sk1", "Pick one", ["A", "B", "C"])
def resolver():
time.sleep(0.05)
cm.resolve_gateway_clarify("id1", "B")
threading.Thread(target=resolver).start()
result = cm.wait_for_response("id1", timeout=10.0)
assert result == "B"
def test_open_ended_auto_awaits_text(self):
"""Clarify with no choices is in text-capture mode immediately."""
from tools import clarify_gateway as cm
entry = cm.register("id2", "sk2", "Free form?", None)
assert entry.awaiting_text is True
# get_pending_for_session returns the entry so the gateway
# text-intercept can find it.
pending = cm.get_pending_for_session("sk2")
assert pending is not None
assert pending.clarify_id == "id2"
def test_button_choice_does_not_auto_await(self):
"""Multi-choice clarify should NOT be in text-capture mode initially."""
from tools import clarify_gateway as cm
entry = cm.register("id3", "sk3", "Pick", ["X", "Y"])
assert entry.awaiting_text is False
assert cm.get_pending_for_session("sk3") is None
def test_include_choice_prompts_returns_multi_choice_entry(self):
"""Gateway typed replies must see active choice prompts too."""
from tools import clarify_gateway as cm
cm.register("id3b", "sk3b", "Pick", ["X", "Y"])
pending = cm.get_pending_for_session("sk3b", include_choice_prompts=True)
assert pending is not None
assert pending.clarify_id == "id3b"
def test_resolve_text_response_maps_numeric_choice(self):
"""Typed numbers should resolve to the canonical choice string."""
from tools import clarify_gateway as cm
cm.register("id3c", "sk3c", "Pick", ["X", "Y"])
assert cm.resolve_text_response_for_session("sk3c", "2") is True
assert cm.wait_for_response("id3c", timeout=0.1) == "Y"
def test_resolve_text_response_accepts_custom_other_text(self):
"""Arbitrary typed text should resolve as a custom Other answer."""
from tools import clarify_gateway as cm
cm.register("id3d", "sk3d", "Pick", ["X", "Y"])
custom = "None of those are valid options"
assert cm.resolve_text_response_for_session("sk3d", custom) is True
assert cm.wait_for_response("id3d", timeout=0.1) == custom
def test_other_button_flips_to_text_mode(self):
"""mark_awaiting_text makes get_pending_for_session find the entry."""
from tools import clarify_gateway as cm
cm.register("id4", "sk4", "Pick", ["X", "Y"])
assert cm.get_pending_for_session("sk4") is None
flipped = cm.mark_awaiting_text("id4")
assert flipped is True
pending = cm.get_pending_for_session("sk4")
assert pending is not None
assert pending.clarify_id == "id4"
def test_mark_awaiting_text_unknown_id(self):
"""mark_awaiting_text on a non-existent id returns False."""
from tools import clarify_gateway as cm
assert cm.mark_awaiting_text("nope") is False
def test_timeout_returns_none(self):
"""wait_for_response returns None when no resolve fires within the timeout."""
from tools import clarify_gateway as cm
cm.register("id5", "sk5", "Q?", ["A"])
result = cm.wait_for_response("id5", timeout=0.2)
assert result is None
def test_resolve_unknown_id_returns_false(self):
"""resolve_gateway_clarify is idempotent on unknown ids."""
from tools import clarify_gateway as cm
assert cm.resolve_gateway_clarify("nope", "anything") is False
def test_resolve_after_wait_completes_is_noop(self):
"""A late resolve on a finished entry doesn't blow up."""
from tools import clarify_gateway as cm
cm.register("id6", "sk6", "Q?", ["A"])
# Time out, entry gets cleaned up
cm.wait_for_response("id6", timeout=0.1)
# Late button click — should not raise
result = cm.resolve_gateway_clarify("id6", "A")
assert result is False
def test_clear_session_cancels_pending_entries(self):
"""clear_session unblocks blocked threads with empty response."""
from tools import clarify_gateway as cm
cm.register("id7", "sk7", "Q?", ["A"])
def waiter():
return cm.wait_for_response("id7", timeout=10.0)
with ThreadPoolExecutor(1) as pool:
fut = pool.submit(waiter)
time.sleep(0.05)
cancelled = cm.clear_session("sk7")
assert cancelled == 1
result = fut.result(timeout=10.0)
# clear_session sets response="" then the wait returns it
assert result == ""
def test_has_pending(self):
from tools import clarify_gateway as cm
cm.register("id8", "sk8", "Q?", ["A"])
assert cm.has_pending("sk8") is True
assert cm.has_pending("nonexistent") is False
def test_notify_register_unregister_clears_pending(self):
"""unregister_notify cancels any pending clarify so threads unwind."""
from tools import clarify_gateway as cm
cm.register("id9", "sk9", "Q?", ["A"])
def waiter():
return cm.wait_for_response("id9", timeout=10.0)
with ThreadPoolExecutor(1) as pool:
fut = pool.submit(waiter)
time.sleep(0.05)
cm.register_notify("sk9", lambda entry: None)
cm.unregister_notify("sk9")
# unregister_notify calls clear_session; thread unwinds
result = fut.result(timeout=10.0)
assert result == ""
def test_session_index_isolation(self):
"""Entries from different sessions don't leak across get_pending lookups."""
from tools import clarify_gateway as cm
cm.register("idA", "alpha", "Q?", None) # auto-await text
cm.register("idB", "beta", "Q?", None) # auto-await text
a = cm.get_pending_for_session("alpha")
b = cm.get_pending_for_session("beta")
assert a is not None and a.clarify_id == "idA"
assert b is not None and b.clarify_id == "idB"
def test_clarify_timeout_config_default(self):
"""get_clarify_timeout returns a positive int (default 3600)."""
from tools import clarify_gateway as cm
timeout = cm.get_clarify_timeout()
# Default 3600s OR whatever is in the user's loaded config.
# Floor check: must be a positive int, not crashed.
assert isinstance(timeout, int)
assert timeout > 0
class TestGatewayTextIntercept:
"""The gateway's _handle_message intercepts text replies to pending clarifies."""
def setup_method(self):
_clear_clarify_state()
def test_get_pending_for_session_returns_oldest_text_awaiting(self):
"""When two clarifies are pending, get_pending_for_session returns the
first that is awaiting_text (the older one if both)."""
from tools import clarify_gateway as cm
# Older multi-choice (not awaiting text)
cm.register("first", "sk", "Q1?", ["A"])
# Newer open-ended (awaiting text)
cm.register("second", "sk", "Q2?", None)
pending = cm.get_pending_for_session("sk")
# The newer one is awaiting text; the older isn't.
assert pending is not None
assert pending.clarify_id == "second"
# Now flip the first to text mode too. Both are awaiting text,
# FIFO returns the older one.
cm.mark_awaiting_text("first")
pending2 = cm.get_pending_for_session("sk")
assert pending2 is not None
assert pending2.clarify_id == "first"
def test_text_fallback_enables_awaiting_text_for_multi_choice(self):
"""When base send_clarify renders choices as text, mark_awaiting_text
is called so the gateway text-intercept can capture the reply."""
from tools import clarify_gateway as cm
entry = cm.register("id-tf", "sk-tf", "Pick one", ["A", "B", "C"])
# Initially, multi-choice does NOT await text (button path)
assert entry.awaiting_text is False
# After the base send_clarify text fallback calls mark_awaiting_text:
flipped = cm.mark_awaiting_text("id-tf")
assert flipped is True
# Now get_pending_for_session should find it
pending = cm.get_pending_for_session("sk-tf")
assert pending is not None
assert pending.clarify_id == "id-tf"
# Clean up
cm.clear_session("sk-tf")