Skip to content

fix(config): warn on legacy restart drain setting - #70435

Open
hoddukzoa12 wants to merge 1 commit into
NousResearch:mainfrom
hoddukzoa12:fix/legacy-restart-drain-warning
Open

fix(config): warn on legacy restart drain setting#70435
hoddukzoa12 wants to merge 1 commit into
NousResearch:mainfrom
hoddukzoa12:fix/legacy-restart-drain-warning

Conversation

@hoddukzoa12

Copy link
Copy Markdown

What does this PR do?

hermes config check now warns when the persisted config explicitly contains numeric agent.restart_drain_timeout: 180.

That value matches the former default changed to 0 by #54066 after long drain windows were found to interfere with supervised gateway restarts. Existing config files can still retain the old explicit value, so merging with the current defaults does not help those installations.

The warning is advisory and read-only. It reports the current default and gives the exact remediation command:

hermes config set agent.restart_drain_timeout 0

It does not rewrite user config because an explicit value may be intentional.

This is intentionally separate from launchd recovery work such as #68238 and #43181: those PRs recover a failed restart, while this change only detects a persisted legacy default during hermes config check.

Related Issue

Follow-up to #31981 and PR #54066. No new issue was filed for this small, self-contained follow-up.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • hermes_cli/config.py
    • detect an explicitly persisted numeric legacy value of 180
    • print a warning and remediation command from hermes config check
    • preserve existing output/exit semantics and avoid config mutation
  • tests/hermes_cli/test_config.py
    • cover integer and float numeric 180
    • verify no warning for an absent key, current default 0, another positive value, quoted string, boolean, or null
    • verify config.yaml remains unchanged

How to Test

scripts/run_tests.sh \
  tests/hermes_cli/test_config.py \
  tests/hermes_cli/test_set_config_value.py \
  tests/hermes_cli/test_config_validation.py -q

Result: 309 passed, 0 failed.

A broader restart-related regression selection also passed:

366 passed in 35.18s

Additional checks:

ruff check hermes_cli/config.py tests/hermes_cli/test_config.py
python -m compileall -q hermes_cli/config.py tests/hermes_cli/test_config.py
git diff --check

All passed.

Manual smoke with isolated HERMES_HOME values confirmed:

# persisted numeric 180
⚠️  agent.restart_drain_timeout is explicitly set to 180, which matches the former default; the current default is 0.
    Long drain windows can delay or interfere with supervised gateway restarts.
    Run: hermes config set agent.restart_drain_timeout 0

# persisted 0
(no legacy warning)

The config hashes were unchanged after both checks.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.3.1

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — N/A; no public option or workflow changed
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A; no config key or default changed
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — config parsing/output is platform-independent
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

Not applicable; CLI output and verification are included above.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Jul 24, 2026
@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants