Skip to content

fix(tools): Python 3.14 compatibility for DaemonThreadPoolExecutor - #74452

Closed
djbclark wants to merge 1 commit into
NousResearch:mainfrom
djbclark:fix/python-314-daemon-pool
Closed

fix(tools): Python 3.14 compatibility for DaemonThreadPoolExecutor#74452
djbclark wants to merge 1 commit into
NousResearch:mainfrom
djbclark:fix/python-314-daemon-pool

Conversation

@djbclark

Copy link
Copy Markdown

Problem

Python 3.14 removed _initializer / _initargs from ThreadPoolExecutor.__init__, replacing them with _create_worker_context(). The _worker function signature also changed from 4 args to 3. DaemonThreadPoolExecutor._adjust_thread_count() still referenced the old API, causing:

AttributeError: 'DaemonThreadPoolExecutor' object has no attribute '_initializer'

This breaks read_file, search_files, skill_view, and other tools that use the thread pool for dispatch.

Fixes: #69359, #58596

Fix

Version-guarded branch that uses _create_worker_context() on Python 3.14+ (3-arg _worker) and the existing _initializer/_initargs path on Python 3.13- (4-arg _worker).

Why not the approach in #57459?

PR #57459 uses getattr(self, '_initializer', None) which avoids the AttributeError but still passes 4 args to _worker. On Python 3.14, _worker takes only 3 args — so it would fail with TypeError: _worker() takes 3 positional arguments but 4 were given.

Testing

  • ✅ Python 3.14.6: DaemonThreadPoolExecutor.submit(lambda: 42).result() returns 42
  • ✅ Python 3.13.14: same test passes
  • ✅ Live Hermes gateway (Python 3.14): read_file, search_files, skill_view all work after applying this patch

Python 3.14 removed _initializer / _initargs from ThreadPoolExecutor,
replacing them with _create_worker_context() and changing _worker() from
4 args to 3. DaemonThreadPoolExecutor._adjust_thread_count() still
referenced the old API, causing AttributeError on Python 3.14.

Add a version guard that uses the 3.14+ API when available and falls
back to the 3.13- API otherwise.

Fixes NousResearch#69359, obsoletes NousResearch#57459 (which used getattr fallback that still
passes the wrong number of args to 3.14's _worker).

Tested: passes on both Python 3.13 and 3.14.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tools Tool registry, model_tools, toolsets duplicate This issue or pull request already exists needs-decision Awaiting maintainer decision before any implementation labels Jul 30, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #65182: both update tools/daemon_pool.py for Python 3.14's WorkerContext/three-argument _worker contract. #65182 is the earlier open same-mechanism PR; please consolidate there.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for targeting the actual daemon-pool compatibility break. Current main still passes the obsolete four-item tuple at tools/daemon_pool.py:55-60, and this PR's WorkerContext tuple at tools/daemon_pool.py:55-60 addresses that contract change.

Problems

  • The new Python 3.14-only branch has no repository regression coverage. tests/tools/test_daemon_pool.py:20-75 covers daemon behavior, reuse, and exit behavior, but not the WorkerContext tuple. With requires-python = ">=3.11,<3.14" in pyproject.toml:15, ordinary supported-interpreter runs do not exercise this branch.

Suggested changes

  • Add a mocked test that captures the threading.Thread invocation and verifies the three-element (executor_ref, WorkerContext, work_queue) tuple and daemon=True for the 3.14 path, while preserving a real submit test for the legacy path.

This is an automated hermes-sweeper review.

Comment thread tools/daemon_pool.py
name=thread_name,
target=_worker,
args=(
if sys.version_info >= (3, 14):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a regression test for this branch. The current daemon-pool tests do not force a Python 3.14 WorkerContext path, and the project's supported interpreter range remains below 3.14, so normal CI coverage will only exercise the legacy tuple.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
@sheawinkler

Copy link
Copy Markdown

Independent live verification on Hermes Agent v0.19.0 (2026.7.20) with CPython 3.14.6:

  1. Baseline reproduction against current main/installed tools/daemon_pool.py:
    • DaemonThreadPoolExecutor(max_workers=1).submit(lambda: 42) fails at _adjust_thread_count with AttributeError: ... has no attribute _initializer.
  2. Applied this PR's patch verbatim.
  3. Deterministic executor test passed:
    • two concurrent results: [9, 16]
    • initializer invoked twice with both values preserved
    • both workers were daemon threads
    • neither worker was registered in concurrent.futures.thread._threads_queues
  4. Fresh-process Hermes integration passed:
    • the model emitted exactly two read_file calls in one assistant turn
    • both tool results returned successfully
    • final output matched both fixtures
    • process exited 0

This patch addresses both Python 3.14 changes correctly: _create_worker_context() and the three-argument _worker signature. A getattr(_initializer, None)-only workaround would remain incorrect.

@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Eighteen PRs address or reference the shared Python 3.14 DaemonThreadPoolExecutor failure: sixteen visible diffs construct the required three-element WorkerContext tuple, while #59157 and #63777 retain the incompatible four-argument _worker call. #61224 overlaps on the executor repair but also contains distinct gateway-liveness and dotenv-reload changes.

Related pull requests

Duplicates

#57459, #58598, #58699, #59897, #60061, #63780, #65182, #69108, #69209, #69311, #72955, #74452, #76212, #76756, and #76817 substantially implement the same WorkerContext-aware spawn repair; #59157 and #63777 duplicate the incomplete four-argument fallback. #61224 overlaps only in its daemon-pool portion because its gateway and env-loader changes are distinct.

Suggested consolidation

Author action: rebase #58699 onto main, or split out the part that can merge, preserving its recorded best-fix implementation and mocked cross-version coverage; keep recorded best fix #65182 open with the salvage path of adopting the focused mocked tuple test and reconciling it with #58699. Despite the keep_open reviews or verdicts on #57459, #60061, #69108, #69209, #69311, #74452, and #76817, their visible diffs provide no stronger cause coverage than #58699/#65182, so close them as explicit duplicates, keep the already-closed PRs closed, and request author action on #61224 to split its distinct gateway-liveness and dotenv-retry work from the duplicate daemon-pool repair.

Complex graph

flowchart TD
    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
    I58596(["issue #58596 (open)"])
    I59896(["issue #59896 (open)"])
    I63769(["issue #63769 (open)"])
    I69359(["issue #69359 (open)"])
    I76621(["issue #76621 (open)"])
    subgraph Dup57459 ["PRs duplicating each other"]
        P57459["PR #57459 (open)"]
        P58598["PR #58598 (closed)"]
        P58699["PR #58699 (open)"]
        P59157["PR #59157 (closed)"]
        P59897["PR #59897 (closed)"]
        P60061["PR #60061 (open)"]
        P61224["PR #61224 (open)"]
        P63777["PR #63777 (closed)"]
        P63780["PR #63780 (closed)"]
        P65182["PR #65182 (open)"]
        P69108["PR #69108 (open)"]
        P69209["PR #69209 (open)"]
        P69311["PR #69311 (open)"]
        P72955["PR #72955 (closed)"]
        P74452["PR #74452 (open)"]
        P76212["PR #76212 (closed)"]
        P76756["PR #76756 (closed)"]
        P76817["PR #76817 (open)"]
    end
    P74452 -.->|partial| I58596
    P74452 -.->|partial| I59896
    P74452 -->|fixes| I63769
    P74452 -.->|partial| I69359
    P74452 -->|fixes| I76621
    class I58596 open
    class I59896 open
    class I63769 open
    class I69359 open
    class I76621 open
    class P57459 open
    class P58598 closed
    class P58699 open
    class P59157 closed
    class P59897 closed
    class P60061 open
    class P61224 open
    class P63777 closed
    class P63780 closed
    class P65182 open
    class P69108 open
    class P69209 open
    class P69311 open
    class P72955 closed
    class P74452 open
    class P76212 closed
    class P76756 closed
    class P76817 open
    class P58699 best
    class P58699 best
    class P58699 best
    class P58699 best
    class P58699 best
    class P65182 best
    class P74452 target
    click I58596 "https://github.com/NousResearch/hermes-agent/issues/58596"
    click I59896 "https://github.com/NousResearch/hermes-agent/issues/59896"
    click I63769 "https://github.com/NousResearch/hermes-agent/issues/63769"
    click I69359 "https://github.com/NousResearch/hermes-agent/issues/69359"
    click I76621 "https://github.com/NousResearch/hermes-agent/issues/76621"
    click P57459 "https://github.com/NousResearch/hermes-agent/pull/57459"
    click P58598 "https://github.com/NousResearch/hermes-agent/pull/58598"
    click P58699 "https://github.com/NousResearch/hermes-agent/pull/58699"
    click P59157 "https://github.com/NousResearch/hermes-agent/pull/59157"
    click P59897 "https://github.com/NousResearch/hermes-agent/pull/59897"
    click P60061 "https://github.com/NousResearch/hermes-agent/pull/60061"
    click P61224 "https://github.com/NousResearch/hermes-agent/pull/61224"
    click P63777 "https://github.com/NousResearch/hermes-agent/pull/63777"
    click P63780 "https://github.com/NousResearch/hermes-agent/pull/63780"
    click P65182 "https://github.com/NousResearch/hermes-agent/pull/65182"
    click P69108 "https://github.com/NousResearch/hermes-agent/pull/69108"
    click P69209 "https://github.com/NousResearch/hermes-agent/pull/69209"
    click P69311 "https://github.com/NousResearch/hermes-agent/pull/69311"
    click P72955 "https://github.com/NousResearch/hermes-agent/pull/72955"
    click P74452 "https://github.com/NousResearch/hermes-agent/pull/74452"
    click P76212 "https://github.com/NousResearch/hermes-agent/pull/76212"
    click P76756 "https://github.com/NousResearch/hermes-agent/pull/76756"
    click P76817 "https://github.com/NousResearch/hermes-agent/pull/76817"
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 18 pull requests and 5 issues in this complex. Each diff was read against this issue; Assessment working set: 70 kB of PR diffs, 52 kB of issue/PR text, 40 kB of discussion (59 comments), 115 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@alt-glitch alt-glitch removed needs-decision Awaiting maintainer decision before any implementation sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Aug 4, 2026
@djbclark

djbclark commented Aug 8, 2026

Copy link
Copy Markdown
Author

Migrated to the authoritative fork: djbclark#3. Closing this upstream copy per operator direction; no further work should be coordinated here.

@djbclark

djbclark commented Aug 8, 2026

Copy link
Copy Markdown
Author

Migrated to the authoritative fork: djbclark#3. Closing this upstream copy per operator direction.

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

Labels

comp/tools Tool registry, model_tools, toolsets duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: DaemonThreadPoolExecutor crashes on Python 3.14 — AttributeError: "_initializer"

5 participants