Skip to content

fix(gateway): harden kanban and provider cleanup races - #29479

Merged
teknium1 merged 1 commit into
NousResearch:mainfrom
helix4u:fix/kanban-client-close-hardening
May 20, 2026
Merged

fix(gateway): harden kanban and provider cleanup races#29479
teknium1 merged 1 commit into
NousResearch:mainfrom
helix4u:fix/kanban-client-close-hardening

Conversation

@helix4u

@helix4u helix4u commented May 20, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Hardens two gateway-adjacent failure paths that can combine under long-running Kanban workloads:

  • OpenAI/httpx client cleanup now finds sockets under httpcore 1.x wrapper objects before force-closing stale transports.
  • Request-scoped OpenAI clients are closed through a single take-and-clear path so stale/interrupt cleanup and worker-thread finalizers do not double-close the same client.
  • Kanban DB opens now validate an existing non-empty DB header before WAL setup, with a specific diagnostic for TLS-looking page-0 clobbers such as 17 03 03 00 13 at byte offset 5.

This does not claim the fd-reuse/TLS writer root cause is proven. It removes a concrete broken cleanup path, reduces request-client lifecycle races, and makes corrupted Kanban DBs fail with actionable evidence instead of a generic PRAGMA journal_mode=WAL traceback.

Related Issue

Related to #26479 and #10324. Replaces the useful socket traversal portion of closed PR #6216.

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/agent_runtime_helpers.py: add shared socket-pool traversal that descends through conn._connection for httpcore 1.x and reuse it from dead-connection cleanup and force-close cleanup.
  • agent/chat_completion_helpers.py: add thread-safe request-client take-and-close helpers for non-streaming and streaming calls so stale/interrupt/final cleanup paths close each request client at most once.
  • hermes_cli/kanban_db.py: validate existing non-empty DB headers before sqlite3.connect()/WAL setup and include TLS-record signature details when page 0 looks clobbered.
  • tests/run_agent/test_create_openai_client_reuse.py: cover httpcore 1.x wrapper socket traversal.
  • tests/run_agent/test_openai_client_lifecycle.py: cover single-owner request-client cleanup on stale non-streaming calls.
  • tests/hermes_cli/test_kanban_db.py: cover TLS-looking Kanban DB header classification.

How to Test

  1. scripts/run_tests.sh tests/run_agent/test_openai_client_lifecycle.py tests/run_agent/test_create_openai_client_reuse.py tests/hermes_cli/test_kanban_db.py tests/hermes_cli/test_kanban_core_functionality.py -n 4
  2. scripts/run_tests.sh tests/run_agent/test_openai_client_lifecycle.py tests/run_agent/test_create_openai_client_reuse.py tests/hermes_cli/test_kanban_db.py -n 4
  3. python -m compileall -q agent/agent_runtime_helpers.py agent/chat_completion_helpers.py hermes_cli/kanban_db.py
  4. scripts/run_tests.sh -n 4

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: Ubuntu / WSL Linux, Python 3.11.14

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

  • N/A — this PR does not add a skill.

Screenshots / Logs

Targeted tests passed:


332 passed in 17.13s
167 passed in 9.55s
compileall completed successfully

Full suite result after creating the draft PR:


scripts/run_tests.sh -n 4
9 failed, 24640 passed, 54 skipped, 250 warnings in 588.11s (0:09:48)

Full-suite failures observed:

  • tests/gateway/test_api_server.py::TestAdapterInit::test_default_config
  • tests/gateway/test_approve_deny_commands.py::TestBlockingApprovalE2E::test_blocking_approval_approve_once
  • tests/gateway/test_approve_deny_commands.py::TestBlockingApprovalE2E::test_blocking_approval_deny
  • tests/gateway/test_config.py::TestLoadGatewayConfig::test_bridges_quoted_false_platform_enabled_from_config_yaml
  • tests/gateway/test_discord_bot_filter.py::TestDiscordBotFilter::test_default_is_none
  • tests/agent/test_context_compressor.py::TestCompress::test_truncation_fallback_no_client
  • tests/gateway/test_runner_startup_failures.py::test_start_gateway_replace_force_uses_terminate_pid
  • tests/plugins/web/test_web_search_provider_plugins.py::TestBundledPluginsRegister::test_all_seven_plugins_present_in_registry
  • tests/tools/test_tirith_security.py::TestDiskFailureMarker::test_cosign_missing_marker_clears_when_cosign_appears

These failures are outside the files changed in this PR and appear unrelated to the Kanban/provider-cleanup hardening. The approval E2E failures included tirith not found — downloading latest release...; the gateway config failures showed local config/default-port differences such as port: 8643 where tests expected 8642.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard labels May 20, 2026
@helix4u
helix4u marked this pull request as ready for review May 20, 2026 21:15
@teknium1
teknium1 merged commit 1a7bb98 into NousResearch:main May 20, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants