fix(voice): remove return-in-finally to fix Python 3.14+ SyntaxWarning - #21100
fix(voice): remove return-in-finally to fix Python 3.14+ SyntaxWarning#21100liuhao1024 wants to merge 1 commit into
Conversation
…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.
…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.
…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.
…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.
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).
…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.
…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.
…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.
…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.
teknium1
left a comment
There was a problem hiding this comment.
Thanks for identifying the Python 3.14 warning. The voice premise is still present on current main: cli.py:11305-11328 has a return inside _voice_stop_and_transcribe's finally block, so the focused control-flow fix is worth salvaging.
Problems
tools/file_tools.py:922makes mutually exclusive replace-mode and patch-mode arguments all JSON-schema-required. Current main deliberately keeps onlymoderequired and documents per-mode requirements (tools/file_tools.py:2051-2093;3adcc6441916c40f0c5135e65194ff9642c99f29).tests/tools/test_voice_cli_integration.py:1332does not stop at a sibling class method, so its line scanner is not reliably scoped to_voice_stop_and_transcribe.- The WhatsApp timeout change has already landed (
1a4e8f70415e073db257c4a31908c21e9921dd5b) and now lives inplugins/platforms/whatsapp/adapter.py:545.
Suggested changes
- Salvage only the voice hunk and replace the source scanner with an AST-scoped assertion plus a three-silent-captures behavior test.
Automated hermes-sweeper review.
| }, | ||
| "required": ["mode"] | ||
| "required": ["mode", "path", "old_string", "new_string", "patch"] |
There was a problem hiding this comment.
mode selects mutually exclusive payloads, so requiring both all replace fields and patch makes valid calls schema-invalid. Keep required as ["mode"] and retain description-level per-mode requirements, as current main does in tools/file_tools.py:2051-2093.
| if not stripped or stripped.startswith("#"): | ||
| continue | ||
| # Detect method end (next def/class at same or lower indent) | ||
| if (stripped.startswith("def ") or stripped.startswith("class ")) and not line.startswith(" "): |
There was a problem hiding this comment.
This does not stop at the next HermesCLI method because sibling methods start with four spaces. Use ast to locate _voice_stop_and_transcribe and inspect only its Try.finalbody nodes.
Replace the return statement inside the finally block of _voice_stop_and_transcribe with a _stop_continuous flag. Python 3.14+ emits SyntaxWarning for return-in-finally because it silently suppresses in-flight exceptions. Fixes NousResearch#21088
ab8f04a to
4d3e294
Compare
…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.
…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.
…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.
|
You were the earliest to fix the return-in-finally restart race — thank you! #73520 merged @brunopirz's #52004 (rebased onto the current (Landed via #73520, merge |
…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.
What does this PR do?
Removes the
returnstatement inside thefinallyblock of_voice_stop_and_transcribeto fix theSyntaxWarningemitted by Python 3.14+.Root Cause
The
finallyblock in_voice_stop_and_transcribe(cli.py) contains areturnstatement used to exit after 3 consecutive no-speech detections. Python 3.14+ emitsSyntaxWarning: 'return' in a 'finally' blockbecause this pattern silently suppresses any in-flight exception propagating through thefinallyclause.Fix
Replace the
returnwith a_stop_continuousflag. The flag is set when the 3-strike no-speech limit is reached, and the restart-recording guard checks the flag to skip the auto-restart. This preserves the original behavior (stop continuous mode after 3 consecutive no-speech cycles) without suppressing exceptions.This is a rebased version of the original PR — only the return-in-finally fix is included, all unrelated changes have been removed.
Related Issue
Fixes #21088
Type of Change
Changes Made
cli.py: Replacedreturninsidefinallyblock of_voice_stop_and_transcribewith a_stop_continuousflagcli.py: Addednot _stop_continuousguard to the restart-recording conditionHow to Test
python3 -m pytest tests/tools/test_voice_cli_integration.py -q— should pass (84 tests)SyntaxWarningwithpython3 -W error::SyntaxWarning -c "import ast; ast.parse(open('cli.py').read())"— should pass with no outputChecklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/A