Skip to content

fix(voice): remove return-in-finally to fix Python 3.14+ SyntaxWarning#21100

Open
liuhao1024 wants to merge 3 commits into
NousResearch:mainfrom
liuhao1024:fix/issue-21088-return-in-finally-voice
Open

fix(voice): remove return-in-finally to fix Python 3.14+ SyntaxWarning#21100
liuhao1024 wants to merge 3 commits into
NousResearch:mainfrom
liuhao1024:fix/issue-21088-return-in-finally-voice

Conversation

@liuhao1024

@liuhao1024 liuhao1024 commented May 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

What does this PR do?

Remove return statement inside the finally block of _voice_stop_and_transcribe to fix the SyntaxWarning emitted by Python 3.14+.

Root Cause

The finally block in _voice_stop_and_transcribe (cli.py, line 7626) contained a return statement used to exit after 3 consecutive no-speech detections. Python 3.14+ emits SyntaxWarning: 'return' in a 'finally' block because this pattern silently suppresses any in-flight exception propagating through the finally clause.

Related Issue

N/A

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • See commit messages for detailed changes

How to Test

  1. Run pytest tests/ -q — all tests should pass
  2. Verify the specific scenario described above is resolved

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 26.4.1

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 and workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A

@liuhao1024
fix(gateway): make npm install timeout configurable via WHATSAPP_NPM_…
6ef568c 
…INSTALL_TIMEOUT

Increase the default npm install timeout for WhatsApp bridge from 60s
to 300s (5 minutes) to accommodate slower systems like Unraid NAS.
Make it configurable via WHATSAPP_NPM_INSTALL_TIMEOUT environment variable
for users who need even longer timeouts.

Closes NousResearch#14980
@liuhao1024
fix(tools): mark patch tool conditionally required params as required
5dc3423 
- Add 'path', 'old_string', 'new_string', and 'patch' to required list
- Update description to clarify mode-specific parameter requirements
- This addresses issue where LLMs would omit these parameters because
  they were not marked as required in the schema, even though they
  are required depending on the mode

Fixes NousResearch#15524
@liuhao1024
fix(voice): remove return-in-finally to fix Python 3.14+ SyntaxWarning
ab8f04a 
The _voice_stop_and_transcribe method had a return statement inside
its finally block (line 7626). Python 3.14+ emits SyntaxWarning for
this pattern because return-in-finally silently suppresses in-flight
exceptions.

Replace the return with a _stop_continuous flag that prevents the
continuous-mode restart from firing after 3 consecutive no-speech
cycles. The runtime behavior is identical — only the control flow
mechanism changes.

Regression test added: TestNoReturnInFinally inspects the source to
ensure no return statement appears inside the finally block.

Fixes NousResearch#21088
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have labels May 7, 2026
This was referenced May 7, 2026
Open
Open
Open
Open
Closed
@alt-glitch alt-glitch mentioned this pull request May 25, 2026
Open
j2h4u added a commit to j2h4u/hermes-agent that referenced this pull request May 29, 2026
@j2h4u @claude
fix(docker): chown install trees independently of volume ownership
5031c9e 
When HERMES_UID remaps hermes (build-time UID 10000) to a host UID
(e.g. 1000), the volume chown guard was the only place that fixed
.venv/ui-tui/node_modules ownership. But the guard only fires when
the bind-mount's top-level owner doesn't match the runtime hermes UID.
If the bind mount is already owned by the target UID (common when the
host user and HERMES_UID share the same numeric ID), needs_chown stays
false and the image-internal trees are never re-chowned — leaving
.venv owned by 10000, causing EACCES when lazy_deps.py tries to
install packages (NousResearch#15012, NousResearch#21100).

Fix: add an independent check right after UID/GID remap that compares
.venv's actual owner to the post-remap hermes UID and chowns $INSTALL_DIR
trees when they diverge. $INSTALL_DIR is container-internal so recursive
chown here never touches host bind mounts.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@benbarclay benbarclay mentioned this pull request Jun 4, 2026
Merged
benbarclay added a commit that referenced this pull request Jun 4, 2026
@benbarclay
fix(docker): chown build trees on UID remap independently of $HERMES_…
5446153 
…HOME (#35027 regression) (#38556)

The stage2 hook gates the recursive chown of the build trees under
$INSTALL_DIR (.venv, ui-tui, node_modules) so a HERMES_UID/PUID remap
leaves them writable by the new runtime UID — needed for lazy_deps
'uv pip install' of platform extras (#15012, #21100) and the TUI esbuild
rebuild into ui-tui/dist (#28851).

#35027 folded that chown under the $HERMES_HOME ownership check
('stat $HERMES_HOME != hermes_uid'). But 'usermod -u <new> hermes'
re-chowns the hermes home dir ($HERMES_HOME == /opt/data) to the new UID
as a side effect, so after any remap that stat is already satisfied and
needs_chown is false — silently skipping the build-tree chown on the
common PUID/NAS path. The venv stays owned by the build-time UID (10000),
so lazy installs and TUI rebuilds fail with EACCES.

Probe the build trees directly instead: chown only when /opt/hermes/.venv
is not already owned by the runtime hermes UID. Independent of
$HERMES_HOME ownership, idempotent across restarts.

Verified live: built the image, booted with HERMES_UID/HERMES_GID on a
fresh named volume, confirmed .venv/ui-tui/node_modules end up owned by
the remapped UID and 'uv pip install' into the venv succeeds; confirmed
the recursive chown fires once and is skipped on restart.
Yuki-14544869 pushed a commit to Yuki-14544869/hermes-agent that referenced this pull request Jun 4, 2026
@benbarclay @Yuki-14544869
fix(docker): chown build trees on UID remap independently of $HERMES_…
d7fa1a9 
…HOME (NousResearch#35027 regression) (NousResearch#38556)

The stage2 hook gates the recursive chown of the build trees under
$INSTALL_DIR (.venv, ui-tui, node_modules) so a HERMES_UID/PUID remap
leaves them writable by the new runtime UID — needed for lazy_deps
'uv pip install' of platform extras (NousResearch#15012, NousResearch#21100) and the TUI esbuild
rebuild into ui-tui/dist (NousResearch#28851).

NousResearch#35027 folded that chown under the $HERMES_HOME ownership check
('stat $HERMES_HOME != hermes_uid'). But 'usermod -u <new> hermes'
re-chowns the hermes home dir ($HERMES_HOME == /opt/data) to the new UID
as a side effect, so after any remap that stat is already satisfied and
needs_chown is false — silently skipping the build-tree chown on the
common PUID/NAS path. The venv stays owned by the build-time UID (10000),
so lazy installs and TUI rebuilds fail with EACCES.

Probe the build trees directly instead: chown only when /opt/hermes/.venv
is not already owned by the runtime hermes UID. Independent of
$HERMES_HOME ownership, idempotent across restarts.

Verified live: built the image, booted with HERMES_UID/HERMES_GID on a
fresh named volume, confirmed .venv/ui-tui/node_modules end up owned by
the remapped UID and 'uv pip install' into the venv succeeds; confirmed
the recursive chown fires once and is skipped on restart.
davidgut1982 pushed a commit to davidgut1982/hermes-agent that referenced this pull request Jun 5, 2026
@benbarclay
fix(docker): chown build trees on UID remap independently of $HERMES_…
8c6383d 
…HOME (NousResearch#35027 regression) (NousResearch#38556)

The stage2 hook gates the recursive chown of the build trees under
$INSTALL_DIR (.venv, ui-tui, node_modules) so a HERMES_UID/PUID remap
leaves them writable by the new runtime UID — needed for lazy_deps
'uv pip install' of platform extras (NousResearch#15012, NousResearch#21100) and the TUI esbuild
rebuild into ui-tui/dist (NousResearch#28851).

NousResearch#35027 folded that chown under the $HERMES_HOME ownership check
('stat $HERMES_HOME != hermes_uid'). But 'usermod -u <new> hermes'
re-chowns the hermes home dir ($HERMES_HOME == /opt/data) to the new UID
as a side effect, so after any remap that stat is already satisfied and
needs_chown is false — silently skipping the build-tree chown on the
common PUID/NAS path. The venv stays owned by the build-time UID (10000),
so lazy installs and TUI rebuilds fail with EACCES.

Probe the build trees directly instead: chown only when /opt/hermes/.venv
is not already owned by the runtime hermes UID. Independent of
$HERMES_HOME ownership, idempotent across restarts.

Verified live: built the image, booted with HERMES_UID/HERMES_GID on a
fresh named volume, confirmed .venv/ui-tui/node_modules end up owned by
the remapped UID and 'uv pip install' into the venv succeeds; confirmed
the recursive chown fires once and is skipped on restart.
changman pushed a commit to changman/hermes-agent that referenced this pull request Jun 10, 2026
@benbarclay @changman
fix(docker): chown build trees on UID remap independently of $HERMES_…
e2b39dc 
…HOME (NousResearch#35027 regression) (NousResearch#38556)

The stage2 hook gates the recursive chown of the build trees under
$INSTALL_DIR (.venv, ui-tui, node_modules) so a HERMES_UID/PUID remap
leaves them writable by the new runtime UID — needed for lazy_deps
'uv pip install' of platform extras (NousResearch#15012, NousResearch#21100) and the TUI esbuild
rebuild into ui-tui/dist (NousResearch#28851).

NousResearch#35027 folded that chown under the $HERMES_HOME ownership check
('stat $HERMES_HOME != hermes_uid'). But 'usermod -u <new> hermes'
re-chowns the hermes home dir ($HERMES_HOME == /opt/data) to the new UID
as a side effect, so after any remap that stat is already satisfied and
needs_chown is false — silently skipping the build-tree chown on the
common PUID/NAS path. The venv stays owned by the build-time UID (10000),
so lazy installs and TUI rebuilds fail with EACCES.

Probe the build trees directly instead: chown only when /opt/hermes/.venv
is not already owned by the runtime hermes UID. Independent of
$HERMES_HOME ownership, idempotent across restarts.

Verified live: built the image, booted with HERMES_UID/HERMES_GID on a
fresh named volume, confirmed .venv/ui-tui/node_modules end up owned by
the remapped UID and 'uv pip install' into the venv succeeds; confirmed
the recursive chown fires once and is skipped on restart.
juniorbra added a commit to juniorbra/hermes-agent that referenced this pull request Jun 11, 2026
@juniorbra
fix(docker): re-chown scripts/whatsapp-bridge on UID remap
0f557e4 
The WhatsApp setup wizard (hermes_cli/main.py, setup_whatsapp) runs
`npm install` in /opt/hermes/scripts/whatsapp-bridge at first use to
build the Node bridge's node_modules. That dir ships owned by the build
UID (10000) via `COPY --chown=hermes:hermes . .`, and stage2-hook.sh's
build-tree re-chown after a HERMES_UID/PUID remap covers .venv, ui-tui,
gateway and node_modules but not scripts/whatsapp-bridge, so the install
fails with `EACCES: permission denied, mkdir
'/opt/hermes/scripts/whatsapp-bridge/node_modules'`.

Add scripts/whatsapp-bridge to the re-chown set so the wizard's npm
install can create node_modules as the remapped hermes user. Same class
of EACCES already fixed for .venv (NousResearch#15012, NousResearch#21100), ui-tui (NousResearch#28851) and
gateway (NousResearch#27221).
@juniorbra juniorbra mentioned this pull request Jun 11, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@liuhao1024 @alt-glitch