Files
hermes-agent/hermes_cli
Halldrix 3f9150e5c4 fix(secrets_cli): defer bitwarden backend import to first attribute access
Address blocking review on #86782 (trevorgordon981, 2026-08-15): the
previous lazy closures in main.py only deferred the module-level
"import secrets_cli" statement, but were themselves invoked at parse
time — so the chain main -> secrets_cli -> bitwarden -> cryptography
still ran eagerly on every command, including `hermes update --check`.
The closure indirection was dead laziness.

Move the laziness to where the crypto payload actually lives:

1. secrets_cli.py: drop the module-top "from agent.secret_sources
   import bitwarden as bw" import. Each cmd_* handler now resolves the
   backend via a local _load_bw() helper, which imports
   agent.secret_sources.bitwarden on first use. register_cli() no
   longer touches crypto at all — it only wires argparse structure.

2. _BWS_VERSION is duplicated in secrets_cli as a plain string so the
   "install" subparser help text renders without importing the
   backend. agent.secret_sources.bitwarden._BWS_VERSION stays the
   source of truth; bump both together when pinning a new bws release.

3. Module-level PEP 562 __getattr__ resolves "secrets_cli.bw" lazily.
   Existing upstream tests (test_secrets_bitwarden_non_tty.py) that
   monkeypatch "hermes_cli.secrets_cli.bw.find_bws" keep working —
   monkeypatch resolves the string one level deep, triggering
   __getattr__, which imports the real bitwarden module and lets the
   patch land on the same cached module object the handlers import.

4. main.py: revert the closure indirection back to a direct parse-time
   _secrets_cli.register_cli() call — safe now that register_cli is
   crypto-free by construction. The argparse wiring is again visible
   at the call site (matching checkpoints.py / curator.py convention),
   which addresses the original parse-time-vs-post-parse contract
   concern from the previous review round.

Adds a decisive main()-level regression test requested by review:
test_main_update_check_crypto_absent_in_sys_modules spawns main() in a
subprocess with argv=['hermes', 'update', '--check'], patches
hermes_cli.main._cmd_update_check to short-circuit before any network,
and asserts cryptography.hazmat.bindings._rust stays out of
sys.modules both at dispatch time and after main() returns. This is
the exact invariant the Windows self-lock depends on; the previous
import-only tests could not observe the failure because parser
construction runs inside main().

Verification:
- scripts/run_tests.sh tests/test_lazy_secrets_import.py
  tests/test_lazy_secrets_dispatch.py
  tests/hermes_cli/test_secrets_bitwarden_non_tty.py
  -> 13/13 passed (includes the new decisive test + the 2 upstream
     tests that broke under the earlier _LazyBitwarden proxy).
- Sabotage run: same suite against the pre-fix main.py + secrets_cli.py
  fails the new decisive test with "cryptography._rust loaded by main()
  before update dispatch" — confirming the test guards the bug.
- Manual trace: at _cmd_update_check dispatch time, sys.modules
  contains hermes_cli.secrets_cli (parse-time structure only) but NOT
  agent.secret_sources.bitwarden and NOT cryptography._rust.

Refs: #86781
Refs: #83569
2026-08-15 01:55:22 -07:00
..
2026-08-13 11:52:47 -07:00
…
2026-08-10 12:43:46 -07:00