chore(observability): expose 7 silent broad-except fallbacks (Wave Agent 7) - #150
Conversation
…ent 7)
Wave Agent 7 (Remaining/Skipped Wave 2026-05-09) flagged 7 silent
broad-except sites that returned an empty default with no log. Tighten
observability without changing behavior.
Sites
- services/module_runtime.py:_private_runtime_env — warning, runs through
redact_text because env-import exceptions can carry path/token fragments
- api/routes/design.py:module_version_lookup — debug
- api/routes/update_center.py:read proof_events insert fallback — warning
- api/routes/system.py:metrics psutil fallback — debug
- core/orchestration/repair_agent.py:extract_json fallback — debug
(uses pre-existing log symbol)
- core/agents/auto_recovery.py:printer_state_poll_soft — debug
(uses pre-existing log symbol)
- core/agents/auto_recovery.py:firmware_restart — debug
(uses pre-existing log symbol)
All sites preserve silent-fallback semantics (the broad-except still
catches and the path still returns its default); only observability is
added. Each site marked `# noqa: BLE001` so future linting passes don't
re-flag.
Tests added (8, all green)
- 04_testing/pytest/unit/test_broad_except_cleanup.py
* 7 parameterized tests pin each site's log-marker substring
* 1 test confirms module_runtime imports redact_text for safe
exception logging (Wave Agent 7 caveat: env exceptions may
carry secrets)
Verification
- py_compile: OK on all 6 touched source files
- Focused tests: 8/8 pass
- Pre-push hook: passed
Scope
- Observability only. Zero behavior change. No new dependencies.
Swarm provenance
- Wave Agent 7 of the 10-agent Remaining/Skipped Blocker Wave
enumerated the 10 candidate sites; on inspection 3 were already
using LOG.exception or specific exceptions; the remaining 7 are
cleaned up here.
References
- https://owasp.org/Top10/A09_2021-Security_Logging_and_Monitoring_Failures/
- https://docs.python.org/3/library/logging.html
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request replaces several silent pass statements in broad exception handlers with logging calls to improve observability across various modules, including design, system, and update center routes. It also introduces a new test suite to verify the presence of these logging markers. The review feedback suggests removing redundant nested try...except blocks around the new logging calls, moving imports to the top level to follow PEP 8, and using a more robust method for locating source files in the tests to avoid brittle path dependencies.
| try: | ||
| import logging | ||
| logging.getLogger(__name__).debug( | ||
| "design.module_version_lookup: metadata.version(%r) failed: %s: %s", | ||
| module_name, type(exc).__name__, exc, | ||
| ) | ||
| except Exception: # noqa: BLE001 | ||
| pass |
There was a problem hiding this comment.
The nested try...except Exception: pass block around the logging call is redundant and adds unnecessary complexity. In a pull request specifically aimed at cleaning up silent broad-except fallbacks, adding a new one for the logger is contradictory. If the concern is that logging might fail (e.g., during interpreter shutdown), it is better to import logging at the top level to ensure it is available in sys.modules and remove the inner try block.
import logging
logging.getLogger(__name__).debug(
"design.module_version_lookup: metadata.version(%r) failed: %s: %s",
module_name, type(exc).__name__, exc,
)| try: | ||
| import logging | ||
| logging.getLogger(__name__).debug( | ||
| "system.metrics: psutil unavailable; cpu/ram default to 0: %s: %s", | ||
| type(exc).__name__, exc, | ||
| ) | ||
| except Exception: # noqa: BLE001 | ||
| pass |
There was a problem hiding this comment.
The nested try...except Exception: pass block around the logging call is redundant. Additionally, import logging should ideally be moved to the top level of the module to follow PEP 8 and avoid repeated imports during exception handling.
import logging
logging.getLogger(__name__).debug(
"system.metrics: psutil unavailable; cpu/ram default to 0: %s: %s",
type(exc).__name__, exc,
)| try: | ||
| import logging | ||
| logging.getLogger(__name__).warning( | ||
| "update_center.read: proof_events insert failed: %s: %s", | ||
| type(exc).__name__, exc, | ||
| ) | ||
| except Exception: # noqa: BLE001 | ||
| pass |
There was a problem hiding this comment.
The nested try...except Exception: pass block around the logging call is redundant. Moving import logging to the top level of the module is recommended to adhere to standard practices and improve efficiency.
import logging
logging.getLogger(__name__).warning(
"update_center.read: proof_events insert failed: %s: %s",
type(exc).__name__, exc,
)| try: | ||
| import logging | ||
|
|
||
| from hermes3d.gateways.redaction import redact_text | ||
|
|
||
| logging.getLogger(__name__).warning( | ||
| "_private_runtime_env: private_env import/load failed: %s", | ||
| redact_text(f"{type(exc).__name__}: {exc}")[:200], | ||
| ) | ||
| except Exception: # noqa: BLE001 -- best-effort during teardown | ||
| pass |
There was a problem hiding this comment.
The nested try...except block and lazy imports inside the exception handler should be avoided. Furthermore, there is an inconsistency in the use of redaction: this block imports redact_text from hermes3d.gateways.redaction, while a local _redact_text function already exists in this module (line 3240). If the external redact_text is preferred for its more robust masking, it should be imported at the top level to maintain consistency and avoid circular dependency risks during lazy loading.
import logging
from hermes3d.gateways.redaction import redact_text
logging.getLogger(__name__).warning(
"_private_runtime_env: private_env import/load failed: %s",
redact_text(f"{type(exc).__name__}: {exc}")[:200],
)| def _read_source(dotted: str) -> str: | ||
| """Read a Python source file under hermes3d by dotted module name.""" | ||
| parts = dotted.split(".") | ||
| from pathlib import Path | ||
|
|
||
| src_root = Path(__file__).resolve().parents[3] / "03_implementation" / "src" | ||
| file_path = src_root / Path(*parts).with_suffix(".py") | ||
| return file_path.read_text(encoding="utf-8") |
There was a problem hiding this comment.
The hardcoded path depth (parents[3]) in _read_source is brittle and may cause tests to fail if run from a different environment or if the repository structure changes. Using importlib.util.find_spec is a more robust way to locate the source file for a given module regardless of the current working directory or test execution context.
| def _read_source(dotted: str) -> str: | |
| """Read a Python source file under hermes3d by dotted module name.""" | |
| parts = dotted.split(".") | |
| from pathlib import Path | |
| src_root = Path(__file__).resolve().parents[3] / "03_implementation" / "src" | |
| file_path = src_root / Path(*parts).with_suffix(".py") | |
| return file_path.read_text(encoding="utf-8") | |
| def _read_source(dotted: str) -> str: | |
| """Read a Python source file under hermes3d by dotted module name.""" | |
| import importlib.util | |
| from pathlib import Path | |
| spec = importlib.util.find_spec(dotted) | |
| if spec is None or spec.origin is None: | |
| raise ImportError(f"Could not find source for {dotted}") | |
| return Path(spec.origin).read_text(encoding="utf-8") |
…ontinuation) (#151) User-mandated FIRST step in the E2E Master Continuation. Reconciles every existing audit / registry / matrix / handoff / merged PR into a single source of truth. Inputs reconciled - E2E_BLOCKER_REGISTRY_2026-05-09.md (PR #142) - REMAINING_SKIPPED_BLOCKERS_WAVE_2026-05-09.md (PR #146) - 60_APP_UPDATE_READINESS_AUDIT_2026-05-09.md (PR #135) - 60-apps-batch2/* (Bonus 12, Bonus 13, agent7-11, bonus14) - Images-GUI/ reference pack (PR #128/#134) - All 15 merged PRs #136-#150 today - Wave 1 (20-agent Blocker Elimination Swarm) receipts - Wave 2 (10-agent Remaining/Skipped Wave) receipts Sections 1. PR ledger: 15 PRs today, all squash, no fake passes 2. Reconciled blocker matrix: closed (13), partial (2), upstream-blocked (1), open (7), deferred-by-lock (1) 3. E2E Definition of Done: 14 items checklist with status 4. Hard gates: v0.13 retry CLOSED, GUI Playwright OPEN (dashboard-advanced), RC v2 resume OPEN, OpenCode/Hands OPEN 5. Open by category: P0 (none), P1 (4), P2 (4) 6. Hard external blockers (cannot fix in code): BLK-009 server, BLK-011 upstream, license / repo-identity / Hunyuan reviews needed from user 7. Provenance: 30+ research receipts banked, MCP evidence chain unbroken, 1 honest 1-loop escalation, 0 fake passes, 0 secret values exposed Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…auto-backup proof_event) (#153) Master Continuation Wave Agent A6 (adversarial reviewer) found 2 residual updater surfaces after PRs #136-#150 were applied. Both are small, focused correctness fixes. BLK-022 — subprocess.TimeoutExpired escapes the for-tag auto-repair guard - Pre-fix: PR #137's try/except in the per-tag loop catches ONLY HTTPException. _run_git's underlying subprocess.run(timeout=120) raises subprocess.TimeoutExpired BEFORE the HTTPException wrapper kicks in if the git operation hangs past timeout. That exception escaped the loop and bypassed _auto_repair_to_backup, leaving the repo on the previous (still-unverified) tag — same fail-class PR #137 was supposed to close. - Post-fix: catch (HTTPException, subprocess.TimeoutExpired). Synthetic step entry distinguishes the two cases; auto-repair fires identically. BLK-023 — auto-backup writes no proof_event - Pre-fix: staged_update line 113 calls _create_backup() which writes to agent_config (line 297-298) but NOT proof_events. The manual /backup endpoint at line 80-84 emits "hermes_agent_backup_created" via _append_proof_event; the auto path was silent. - Post-fix: emit "hermes_agent_backup_auto_created" proof_event right after _create_backup, mirroring the manual path. Symmetric observability. Tests added (2 + 3 pre-existing = 5/5 pass) - test_blk022_subprocess_timeout_expired_pivots_to_auto_repair: monkeypatch _run_git to raise subprocess.TimeoutExpired on checkout; assert auto_repair fires once, repair payload populated, synthetic check name says "timeout" - test_blk023_auto_backup_emits_proof_event: capture _append_proof_event calls; assert exactly 1 "hermes_agent_backup_auto_created" event with the correct backup_id and source_agent Verification - py_compile: OK - Focused tests: 5/5 pass - Pre-push hook: passed Scope - 2 small fixes in one file (~25 LoC source + ~110 LoC tests). - No behavior change on the happy path. - v0.13.0 stays formally deferred. - RC v2 commits 3-5 still paused. Swarm provenance - Master Continuation Wave Agent A6 (adversarial reviewer) found these residuals during a re-audit of PRs #136-#150. Receipts in E2E_COMPLETION_MASTER_REGISTRY_2026-05-09.md. References - https://docs.python.org/3/library/subprocess.html#subprocess.TimeoutExpired - https://owasp.org/www-project-top-10-ci-cd-security-risks/CICD-SEC-01-Insufficient-Flow-Control-Mechanisms Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
W7-3 truth-check found 5 parametrized test_firmware_source_inventory_is_reference_only_not_executable failures returning status="blocked" instead of "ready". Root cause: PR #120 (firmware source inventory probes) changed BUILTIN_RUNTIME_PROBES firmware row entries: - kind: "source_inventory" -> "firmware_source_inventory" - path: "" -> hardcoded absolute paths The dispatcher _safe_runtime_probe has no branch for "firmware_source_inventory" kind, so the probe falls through to the default executable-path block and returns "blocked". Even with the kind fixed, the hardcoded path skips the mod.local_path fallback in _source_inventory_probe, which CI (and tmp_path-based unit tests) need. W7-3 hypothesised a PR #150 (broad-except cleanup) interaction; on read, PR #150 only edited _private_runtime_env and is innocent of this regression. PR #150's broad-except cleanup is intact — NOT reverted. Fix: surgically revert kind/path on the 6 firmware rows (firmware_klipper, marlin, prusa_firmware, reprapfirmware, repetier_firmware, smoothieware) to their pre-#120 values. Inline comments mark these as load-bearing for _runner_status mapping. firmware-specific provenance is preserved by tool_key="firmware_source_inventory" and proof_gate_version="firmware-source-inventory-v1". The new probe_firmware_source_inventory() function and FIRMWARE_SOURCE_PATHS registry from PR #120 are unchanged. Tests - 5 previously-failing test_firmware_source_inventory_* tests now PASS - 1 new regression-pin: test_firmware_probe_returns_ready_when_files_exist_and_verifier_index_empty - Adjacent: test_source_runtime_contracts.py (42), test_module_runtime.py (72), test_firmware_farm_probes.py (49) — 166 PASS / 0 regressions Sources - PEP 8 Programming Recommendations (broad-except guidance): https://peps.python.org/pep-0008/#programming-recommendations - PR #150 description (ba4194d): scope is observability-only; explicitly not behavioral. Doc: 03_implementation/docs/handoffs/W8-8_FIRMWARE_PROBE_REGRESSION_2026-05-09.md Hermes evidence chain: PASS Task ID: H3D-CLAUDE-W8-8-FIRMWARE-PROBE Lock owner: claude-w8-8-firmware-probe hermes_run_gate: tests.unit Hermes locks released after merge Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Wave Agent 7 (Remaining/Skipped Wave 2026-05-09) flagged 7 silent broad-except sites that returned an empty default with no log. Tighten observability without changing behavior.
Sites cleaned up
module_runtime.py:_private_runtime_envredact_text(env-import exceptions can carry token fragments)design.py:module_version_lookupupdate_center.py:read proof_events insertsystem.py:metrics psutil fallbackrepair_agent.py:extract_json fallbacklogsymbolauto_recovery.py:printer_state_poll_softlogsymbolauto_recovery.py:firmware_restartlogsymbolAll sites preserve silent-fallback semantics; only observability is added. Each site marked
# noqa: BLE001so future linting doesn't re-flag.Tests added (8, all green)
module_runtimeimportsredact_textfor safe exception loggingTest plan
Scope
Swarm provenance
Wave Agent 7 of the 10-agent Remaining/Skipped Blocker Wave enumerated 10 candidate sites; 3 were already using
LOG.exceptionor specific exceptions; the remaining 7 are cleaned up here.🤖 Generated with Claude Code