Skip to content

fix: batch of small robustness/correctness fixes from @kyssta-exe - #37146

Merged
teknium1 merged 1 commit into
mainfrom
hermes/hermes-8fdcada3
Jun 2, 2026
Merged

fix: batch of small robustness/correctness fixes from @kyssta-exe#37146
teknium1 merged 1 commit into
mainfrom
hermes/hermes-8fdcada3

Conversation

@teknium1

@teknium1 teknium1 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Summary

Salvages a batch of small, surgical robustness/correctness fixes from @kyssta-exe, reapplied onto current main (the original branches were stale) with a few refinements. Each was verified to be a real, still-present issue.

Fixes (8 distinct)

Area Fix Orig PR
cron load_jobs() validates top-level JSON shape — bare list auto-repairs, scalars/null raise a clear error instead of an uncaught AttributeError that crashed the whole cron subsystem #37065 (closes #36867)
web close the per-action log fd after Popen (was leaking one fd per spawned action) #36843
web DELETE /api/env → 400 for invalid key names instead of a misleading 500 (mirrors PUT) #36840
gateway read /proc/<pid>/cmdline in a with-block so the fd releases immediately #36804
web-tools include xai in check_web_api_key() so a configured X.AI web backend reports available #36802
compression set the feasibility-checked flag only after the check completes; default to "not checked" if the attr is missing #36803
completion replace ls with directory globbing in generated bash/zsh/fish profile listers (handles spaces, skips non-dirs) #36806
terminal drop a duplicate import threading #36808
claw migrate recommendation points at the real hermes gateway stop, not the non-existent hermes stop #36795, #36796 (closes #36771)
tests guard a leaked HERMES_CRON_SESSION from breaking gateway approval tests — added to the hermetic conftest unset list (root cause) and popped in the affected test's setup_method #36796

Refinements beyond the originals

  • completion: zsh uses the idiomatic one-liner *(N/:t); all three shells now filter to directories only (the old ls also listed stray files).
  • claw hermes stop: verified live that hermes stop errors (invalid choice: 'stop') and hermes gateway stop is valid.
  • conftest: added the root-cause unset for HERMES_CRON_SESSION so every test is protected, not just the one the contributor patched.

Not included (verified, deliberately dropped)

Validation

  • E2E: load_jobs() exercised against all top-level shapes (dict/list/str/int/null/garbage) — bare list auto-repairs, scalars/null raise a clean RuntimeError, no uncaught AttributeError.
  • FD-leak fixes proven empirically (5 spawns → 5 leaked fds before, 0 after).
  • The HERMES_CRON_SESSION flake reproduced and confirmed fixed both ways (with/without the conftest unset).
  • Targeted suites green: cron, tools/approval, web_tools, terminal_tool, completion, claw, web_server, gateway, compression, feasibility — 1098 tests, 0 failures.

Co-authored-by: kyssta-exe kyssta-exe@users.noreply.github.com

Infographic

robustness-batch

Salvages 8 distinct fixes from a batch of PRs by @kyssta-exe, reapplied
onto current main (original branches were stale) with a few refinements.

- cron(jobs.py): load_jobs() validates top-level JSON shape — a bare
  list auto-repairs into the {"jobs": [...]} dict; scalars/null raise a
  clear RuntimeError instead of an uncaught AttributeError that took
  down the whole cron subsystem (#37065, closes #36867).
- web(web_server.py): close the per-action log file handle after Popen
  so the parent stops leaking one fd per spawned action (#36843).
- web(web_server.py): DELETE /api/env returns 400 for invalid key names
  instead of a misleading 500, mirroring PUT /api/env (#36840).
- gateway(gateway.py): read /proc/<pid>/cmdline inside a with-block so
  the fd is released immediately instead of relying on GC (#36804).
- web-tools(web_tools.py): include "xai" in check_web_api_key() so a
  configured X.AI web backend reports as available (#36802).
- compression(conversation_compression.py): mark the feasibility check
  done only after it completes, and default the gate to "not checked"
  if the attribute is missing (#36803).
- completion(completion.py): replace `ls` with directory globbing in the
  generated bash/zsh/fish profile listers — handles names with spaces
  and skips non-directory entries (#36806).
- terminal-tool(terminal_tool.py): drop a duplicate `import threading`
  (#36808).
- claw(claw.py): the migrate recommendation now points at the real
  `hermes gateway stop` command instead of the non-existent
  `hermes stop` (#36795, #36796, closes #36771).
- tests: guard against a leaked HERMES_CRON_SESSION breaking gateway
  approval tests — add it to the hermetic conftest unset list (root
  cause, protects every test) and pop it in the affected test's
  setup_method (#36796).

Co-authored-by: kyssta-exe <kyssta-exe@users.noreply.github.com>
@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

🔎 Lint report: hermes/hermes-8fdcada3 vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 9603 on HEAD, 9603 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 4977 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@teknium1
teknium1 merged commit d4b533d into main Jun 2, 2026
23 checks passed
@teknium1
teknium1 deleted the hermes/hermes-8fdcada3 branch June 2, 2026 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants