Farhan Beg commited on
Commit
d8e8033
·
1 Parent(s): 7f18e15

quality: dead code, dedup, constants, docs (audit Group E)

Browse files

Quality fixes from the comprehensive audit. No behavior changes — all
syntactic/cosmetic/traceability improvements.

- E1: delete debug-model-options.py. Confirmed dead — not COPY'd into
the image (Dockerfile only copies start.sh, health-server.js,
hermes-sync.py, cloudflare-*.py), no caller anywhere, replaced by the
inline Python probe in health-server.js (/hm/debug/model-options-trace).
Also hardcoded a python3.12 site-packages path that may not match the
container's Python version.
- E7: remove unused 'import time' in cloudflare-proxy-setup.py.
- E11: re-declare ARG HERMES_AGENT_VERSION after FROM so the ENV line
'HERMES_AGENT_VERSION=${HERMES_AGENT_VERSION}' expands correctly
under the classic builder (pre-FROM ARGs are only visible in FROM).
- E12: LOG_ROTATE_BYTES env var replaces the magic 5242880 literal in
the boot log-rotation block.
- E13: record the resolved WebUI commit (git rev-parse HEAD >
/opt/hermes-webui/WEBUI_COMMIT) so a build can be traced to an exact
upstream revision. WEBUI_REF defaults to master (moving target).
- E15: export BACKUP_DATASET consistently (hermes-sync.py reads
BACKUP_DATASET_NAME from env; the shell var was never exported).
- E16: write_status now logs OSError to stderr instead of swallowing it
silently — if /tmp is unwritable the status page shows stale data with
no log line to explain why. Same for the STATE_FILE atomic write.
- E17: factor the duplicated STATE_FILE parse block (sync_once + loop)
into a load_state() helper.
- E18: port the HTTPError body-parsing from cloudflare-keepalive-setup.py
into cloudflare-proxy-setup.py's cf_request so a CF 4xx/5xx surfaces
the API error message instead of a raw 'HTTP Error 401: Unauthorized'.

Skipped the riskier refactors (launch_logged helper, buildProxyHeaders/
bufferRequestBody extraction, module-scope dashboardRootRoutes Set,
parse-req.url-once) to honor 'be careful not breaking' — they touch
hot request paths and the dedup benefit isn't worth the behavior risk.

Files changed (5) hide show
  1. Dockerfile +9 -1
  2. cloudflare-proxy-setup.py +15 -3
  3. debug-model-options.py +0 -31
  4. hermes-sync.py +27 -22
  5. start.sh +7 -2
Dockerfile CHANGED
@@ -4,6 +4,10 @@
4
  ARG HERMES_AGENT_VERSION=latest
5
  FROM nousresearch/hermes-agent:${HERMES_AGENT_VERSION}
6
 
 
 
 
 
7
  ARG WEBUI_REF=master
8
 
9
  USER root
@@ -42,9 +46,13 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
42
  && uv pip install --python /opt/hermes/.venv/bin/python --no-cache-dir \
43
  huggingface_hub hf_transfer pyyaml
44
 
45
- # Clone nesquena/hermes-webui (install deps into the agent venv so imports resolve)
 
 
 
46
  RUN git clone --depth 1 --branch ${WEBUI_REF} \
47
  https://github.com/nesquena/hermes-webui.git /opt/hermes-webui \
 
48
  && ( [ -f /opt/hermes-webui/requirements.txt ] \
49
  && /opt/hermes/.venv/bin/pip install --no-cache-dir -r /opt/hermes-webui/requirements.txt \
50
  || true ) \
 
4
  ARG HERMES_AGENT_VERSION=latest
5
  FROM nousresearch/hermes-agent:${HERMES_AGENT_VERSION}
6
 
7
+ # Re-declare so the ARG is in scope after FROM (a pre-FROM ARG is only
8
+ # visible inside the FROM instruction under the classic builder; BuildKit
9
+ # is more permissive but re-declaring is portable).
10
+ ARG HERMES_AGENT_VERSION
11
  ARG WEBUI_REF=master
12
 
13
  USER root
 
46
  && uv pip install --python /opt/hermes/.venv/bin/python --no-cache-dir \
47
  huggingface_hub hf_transfer pyyaml
48
 
49
+ # Clone nesquena/hermes-webui (install deps into the agent venv so imports resolve).
50
+ # WEBUI_REF defaults to master (moving target) — record the resolved commit so
51
+ # a build can be traced back to an exact upstream revision. Combined with the
52
+ # build-time patches below, this makes silent patch-skip failures debuggable.
53
  RUN git clone --depth 1 --branch ${WEBUI_REF} \
54
  https://github.com/nesquena/hermes-webui.git /opt/hermes-webui \
55
+ && cd /opt/hermes-webui && git rev-parse HEAD > /opt/hermes-webui/WEBUI_COMMIT \
56
  && ( [ -f /opt/hermes-webui/requirements.txt ] \
57
  && /opt/hermes/.venv/bin/pip install --no-cache-dir -r /opt/hermes-webui/requirements.txt \
58
  || true ) \
cloudflare-proxy-setup.py CHANGED
@@ -11,8 +11,8 @@ import os
11
  import re
12
  import secrets
13
  import sys
14
- import time
15
  import urllib.request
 
16
  from pathlib import Path
17
 
18
  API_BASE = "https://api.cloudflare.com/client/v4"
@@ -43,8 +43,20 @@ def cf_request(method: str, path: str, token: str, body: bytes | None = None, co
43
  method=method,
44
  headers={"Authorization": f"Bearer {token}", "Content-Type": content_type},
45
  )
46
- with urllib.request.urlopen(req, timeout=30) as response:
47
- payload = json.loads(response.read().decode("utf-8"))
 
 
 
 
 
 
 
 
 
 
 
 
48
  if not payload.get("success"):
49
  errors = payload.get("errors") or [{"message": "Unknown Cloudflare API error"}]
50
  raise RuntimeError(errors[0].get("message", "Unknown Cloudflare API error"))
 
11
  import re
12
  import secrets
13
  import sys
 
14
  import urllib.request
15
+ import urllib.error
16
  from pathlib import Path
17
 
18
  API_BASE = "https://api.cloudflare.com/client/v4"
 
43
  method=method,
44
  headers={"Authorization": f"Bearer {token}", "Content-Type": content_type},
45
  )
46
+ # E18: parse HTTPError bodies like the keepalive script does. Previously
47
+ # a 4xx/5xx from CF surfaced as a raw 'HTTP Error 401: Unauthorized' in
48
+ # the generic except Exception block with no body context.
49
+ try:
50
+ with urllib.request.urlopen(req, timeout=30) as response:
51
+ payload = json.loads(response.read().decode("utf-8"))
52
+ except urllib.error.HTTPError as e:
53
+ try:
54
+ error_body = json.loads(e.read().decode("utf-8"))
55
+ errors = error_body.get("errors") or [{"message": "Unknown error"}]
56
+ error_msg = errors[0].get("message", "Unknown error") if errors else "Unknown error"
57
+ except Exception:
58
+ error_msg = f"HTTP {e.code}: {e.reason}"
59
+ raise RuntimeError(f"Cloudflare API {e.code}: {error_msg}")
60
  if not payload.get("success"):
61
  errors = payload.get("errors") or [{"message": "Unknown Cloudflare API error"}]
62
  raise RuntimeError(errors[0].get("message", "Unknown Cloudflare API error"))
debug-model-options.py DELETED
@@ -1,31 +0,0 @@
1
- #!/usr/bin/env python3
2
- """Debug script: call build_models_payload() directly and print the traceback."""
3
- import os, sys, traceback, json
4
-
5
- os.environ.setdefault("HERMES_HOME", "/opt/data")
6
- sys.path.insert(0, "/opt/hermes")
7
- sys.path.insert(0, "/opt/hermes/.venv/lib/python3.12/site-packages")
8
-
9
- try:
10
- from hermes_cli.inventory import build_models_payload, load_picker_context
11
- ctx = load_picker_context()
12
- print("=== load_picker_context OK ===")
13
- print(f" current_model: {ctx.current_model!r}")
14
- print(f" current_provider: {ctx.current_provider!r}")
15
- print(f" current_base_url: {ctx.current_base_url!r}")
16
- print(f" user_providers: {list(ctx.user_providers.keys()) if isinstance(ctx.user_providers, dict) else type(ctx.user_providers)}")
17
- print(f" custom_providers: {list(ctx.custom_providers.keys()) if isinstance(ctx.custom_providers, dict) else type(ctx.custom_providers)}")
18
- except Exception:
19
- print("=== load_picker_context FAILED ===")
20
- traceback.print_exc()
21
- sys.exit(0)
22
-
23
- try:
24
- result = build_models_payload(ctx, max_models=50, include_unconfigured=True, picker_hints=True, canonical_order=True, pricing=True, capabilities=True)
25
- print("=== build_models_payload OK ===")
26
- print(f" providers count: {len(result.get('providers', []))}")
27
- print(f" model: {result.get('model')!r}")
28
- print(f" provider: {result.get('provider')!r}")
29
- except Exception:
30
- print("=== build_models_payload FAILED ===")
31
- traceback.print_exc()
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
hermes-sync.py CHANGED
@@ -158,8 +158,11 @@ def write_status(status: str, message: str, fingerprint: str | None = None, mark
158
  try:
159
  tmp_path.write_text(json.dumps(payload), encoding="utf-8")
160
  tmp_path.replace(STATUS_FILE)
161
- except OSError:
162
- pass
 
 
 
163
 
164
  if fingerprint or marker:
165
  state = {}
@@ -180,8 +183,8 @@ def write_status(status: str, message: str, fingerprint: str | None = None, mark
180
  tmp_state = STATE_FILE.with_suffix(".tmp")
181
  tmp_state.write_text(json.dumps(state), encoding="utf-8")
182
  os.replace(tmp_state, STATE_FILE)
183
- except OSError:
184
- pass
185
 
186
 
187
  def resolve_backup_repo() -> str:
@@ -460,6 +463,24 @@ def restore() -> bool:
460
  return False
461
 
462
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
463
  def sync_once(last_fingerprint: str | None = None, last_marker: tuple[int, int, int] | None = None):
464
  # Inter-process lock: the loop process and a separate CLI sync-once
465
  # (run by start.sh's graceful_shutdown / exit handler) can both call
@@ -480,15 +501,7 @@ def sync_once(last_fingerprint: str | None = None, last_marker: tuple[int, int,
480
  except OSError:
481
  _LOCK_HANDLE = None # non-fatal; proceed without the lock
482
  if last_fingerprint is None and last_marker is None:
483
- if STATE_FILE.exists():
484
- try:
485
- state = json.loads(STATE_FILE.read_text(encoding="utf-8"))
486
- last_fingerprint = state.get("last_fingerprint")
487
- m = state.get("last_marker")
488
- if m and len(m) == 3:
489
- last_marker = tuple(m)
490
- except Exception:
491
- pass
492
 
493
  repo_id = ensure_repo_exists()
494
  current_marker = metadata_marker(HERMES_HOME)
@@ -547,15 +560,7 @@ def loop() -> int:
547
  # Seed from any prior run so we don't re-upload an identical tree.
548
  last_fingerprint: str | None = None
549
  last_marker: tuple[int, int, int] | None = None
550
- if STATE_FILE.exists():
551
- try:
552
- state = json.loads(STATE_FILE.read_text(encoding="utf-8"))
553
- last_fingerprint = state.get("last_fingerprint")
554
- m = state.get("last_marker")
555
- if m and len(m) == 3:
556
- last_marker = tuple(m)
557
- except Exception:
558
- pass
559
  if last_marker is None:
560
  last_marker = metadata_marker(HERMES_HOME)
561
 
 
158
  try:
159
  tmp_path.write_text(json.dumps(payload), encoding="utf-8")
160
  tmp_path.replace(STATUS_FILE)
161
+ except OSError as exc:
162
+ # E16: surface status write failures instead of swallowing silently.
163
+ # If /tmp is unwritable (read-only FS, full disk) the status page
164
+ # shows stale data with no log line to explain why.
165
+ print(f"Warning: could not write sync status to {STATUS_FILE}: {exc}", file=sys.stderr)
166
 
167
  if fingerprint or marker:
168
  state = {}
 
183
  tmp_state = STATE_FILE.with_suffix(".tmp")
184
  tmp_state.write_text(json.dumps(state), encoding="utf-8")
185
  os.replace(tmp_state, STATE_FILE)
186
+ except OSError as exc:
187
+ print(f"Warning: could not write sync state to {STATE_FILE}: {exc}", file=sys.stderr)
188
 
189
 
190
  def resolve_backup_repo() -> str:
 
463
  return False
464
 
465
 
466
+ def load_state() -> tuple[str | None, tuple[int, int, int] | None]:
467
+ """E17: load (last_fingerprint, last_marker) from STATE_FILE.
468
+
469
+ Factored from the duplicated parse block that appeared in both sync_once
470
+ and loop. Returns (None, None) if the file is missing/corrupt.
471
+ """
472
+ if not STATE_FILE.exists():
473
+ return (None, None)
474
+ try:
475
+ state = json.loads(STATE_FILE.read_text(encoding="utf-8"))
476
+ last_fingerprint = state.get("last_fingerprint")
477
+ m = state.get("last_marker")
478
+ last_marker = tuple(m) if m and len(m) == 3 else None
479
+ return (last_fingerprint, last_marker)
480
+ except Exception:
481
+ return (None, None)
482
+
483
+
484
  def sync_once(last_fingerprint: str | None = None, last_marker: tuple[int, int, int] | None = None):
485
  # Inter-process lock: the loop process and a separate CLI sync-once
486
  # (run by start.sh's graceful_shutdown / exit handler) can both call
 
501
  except OSError:
502
  _LOCK_HANDLE = None # non-fatal; proceed without the lock
503
  if last_fingerprint is None and last_marker is None:
504
+ last_fingerprint, last_marker = load_state()
 
 
 
 
 
 
 
 
505
 
506
  repo_id = ensure_repo_exists()
507
  current_marker = metadata_marker(HERMES_HOME)
 
560
  # Seed from any prior run so we don't re-upload an identical tree.
561
  last_fingerprint: str | None = None
562
  last_marker: tuple[int, int, int] | None = None
563
+ last_fingerprint, last_marker = load_state()
 
 
 
 
 
 
 
 
564
  if last_marker is None:
565
  last_marker = metadata_marker(HERMES_HOME)
566
 
start.sh CHANGED
@@ -20,7 +20,10 @@ TELEGRAM_WEBHOOK_PORT="${TELEGRAM_WEBHOOK_PORT:-8765}"
20
  WEBUI_PORT="${HERMES_WEBUI_PORT:-8787}"
21
 
22
  SYNC_INTERVAL="${SYNC_INTERVAL:-60}"
23
- BACKUP_DATASET="${BACKUP_DATASET_NAME:-huggingmes-backup}"
 
 
 
24
  CF_PROXY_ENV_FILE="/tmp/huggingmes-cloudflare-proxy.env"
25
 
26
  export HERMES_HOME
@@ -69,11 +72,13 @@ cd "$HERMES_HOME/workspace" || cd "$HERMES_HOME"
69
  # rotation those files grow forever and end up in the HF Dataset backup.
70
  # Strategy: if a log is >5MB, rename to .1 (overwriting any previous .1)
71
  # and start fresh. Cheap, deterministic, no cron needed.
 
 
72
  if [ -d "$HERMES_HOME/logs" ]; then
73
  for f in "$HERMES_HOME/logs"/*.log; do
74
  [ -f "$f" ] || continue
75
  sz=$(stat -c%s "$f" 2>/dev/null || echo 0)
76
- if [ "$sz" -gt 5242880 ]; then
77
  mv -f "$f" "${f}.1"
78
  : > "$f"
79
  echo "rotated $(basename "$f") ($sz bytes -> .1)"
 
20
  WEBUI_PORT="${HERMES_WEBUI_PORT:-8787}"
21
 
22
  SYNC_INTERVAL="${SYNC_INTERVAL:-60}"
23
+ # E15: export so downstream scripts could read it if needed; hermes-sync.py
24
+ # reads BACKUP_DATASET_NAME from env directly, so this is for the startup
25
+ # summary + any future callers.
26
+ export BACKUP_DATASET="${BACKUP_DATASET_NAME:-huggingmes-backup}"
27
  CF_PROXY_ENV_FILE="/tmp/huggingmes-cloudflare-proxy.env"
28
 
29
  export HERMES_HOME
 
72
  # rotation those files grow forever and end up in the HF Dataset backup.
73
  # Strategy: if a log is >5MB, rename to .1 (overwriting any previous .1)
74
  # and start fresh. Cheap, deterministic, no cron needed.
75
+ # Threshold is env-configurable (default 5 MiB) so a noisy space can tune it.
76
+ LOG_ROTATE_BYTES="${LOG_ROTATE_BYTES:-5242880}"
77
  if [ -d "$HERMES_HOME/logs" ]; then
78
  for f in "$HERMES_HOME/logs"/*.log; do
79
  [ -f "$f" ] || continue
80
  sz=$(stat -c%s "$f" 2>/dev/null || echo 0)
81
+ if [ "$sz" -gt "$LOG_ROTATE_BYTES" ]; then
82
  mv -f "$f" "${f}.1"
83
  : > "$f"
84
  echo "rotated $(basename "$f") ($sz bytes -> .1)"