Skip to content

fix(cli): suppress SyntaxWarning in _voice_processing by moving return out of finally - #25570

Closed
alaamohanad169-ship-it wants to merge 3 commits into
NousResearch:mainfrom
alaamohanad169-ship-it:fix/voice-processing-syntax-warning
Closed

alaamohanad169-ship-it wants to merge 3 commits into
NousResearch:mainfrom
alaamohanad169-ship-it:fix/voice-processing-syntax-warning

Conversation

@alaamohanad169-ship-it

Copy link
Copy Markdown
Contributor

What does this PR do?

The return statement at the end of the no-speech-count check was nested inside the finally: block, which silently suppresses exceptions from the except clause.

Moved the no-speech tracking logic (lines 9686-9709) outside of finally: so cleanup still runs via the finally: block but return executes after it, restoring normal exception semantics. No functional change.

Type of Change

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

Changes Made

  • cli.py: Moved no-speech tracking and restart logic out of finally: block

Testing

  • python -m py_compile cli.py — syntax valid
  • No functional change; existing behavior preserved

The installer creates ~/.hermes/.env via cp or touch, both of which
inherit the process umask. On Ubuntu (umask 0022) the result is 0644;
on some server environments with umask 0002 the result is 0664. No
explicit chmod was applied after file creation, leaving API keys and
tokens visible to other users on the system.

Add chmod 0600 immediately after .env creation in copy_config_templates(),
with a || true fallback for NixOS/containers where the activation script
owns permissions. Also tightens permissions when the file already exists,
so users with 0664 on disk are hardened on next upgrade.

Fixes #25477
When shutdown_forensics.py detects ppid == 1, it assumes the process
runs under systemd. On macOS, PID 1 is launchd, not systemd. This
causes the gateway to incorrectly use systemd-specific shutdown logic
on macOS.

Restrict the ppid == 1 heuristic to Linux only:
  sys.platform.startswith("linux") and ppid == 1

The INVOCATION_ID env var check remains platform-independent (it's only
set by systemd on Linux, so no false positive on macOS).

Fixes #25508

Test plan:
- test_under_systemd_false_on_macos_when_ppid_is_one
- test_under_systemd_true_on_linux_when_ppid_is_one
- test_under_systemd_invocation_id_overrides_platform
- All 33 tests in test_shutdown_forensics.py pass
…n out of finally

The return statement at the end of the no-speech-count check was nested
inside the finally: block, which silently suppresses exceptions from
the except clause.

Moved the no-speech tracking logic (lines 9686-9709) outside of finally:
so cleanup still runs via the finally: block but return executes after it,
restoring normal exception semantics. No functional change.
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have labels May 14, 2026
@alt-glitch

Copy link
Copy Markdown

Duplicate of #21100 — same return-in-finally fix for _voice_stop_and_transcribe (Python 3.14+ SyntaxWarning). This is the 6th PR for issue #21088.

Also note: this PR bundles unrelated changes — gateway/shutdown_forensics.py (systemd platform check, same as #25566/#25525), scripts/install.sh (chmod 0600 on .env), and their tests. Only the cli.py change relates to the stated fix.

@liuhao1024

Copy link
Copy Markdown
Contributor

The de-indent from the `finally` block is incomplete. The outer `if` block was moved from 12-space to 8-space indent, but the inner `_restart_recording` function and its body were left at their original indentation (16 spaces), creating an 8-space indent from the `if` — inconsistent with the rest of the file which uses 4-space indentation.

Current (after this PR):

        if self._voice_continuous and not submitted and not self._voice_recording:
                def _restart_recording():
                    try:
                        self._voice_start_recording()

Expected:

        if self._voice_continuous and not submitted and not self._voice_recording:
            def _restart_recording():
                try:
                    self._voice_start_recording()

The `threading.Thread(target=_restart_recording, ...).start()` call and any other content inside this `if` block will also need the same 4-space de-indent.

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

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard comp/gateway Gateway runner, session dispatch, delivery duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants