Skip to content

spawn-guard: post-update/boot 3-spawn guard over the committed generation (t_7b0df4cf) - #127176

Closed
smurilosrocha wants to merge 1 commit into
NousResearch:mainfrom
smurilosrocha:t_0c4f7255-spawn-guard-v2
Closed

smurilosrocha wants to merge 1 commit into
NousResearch:mainfrom
smurilosrocha:t_0c4f7255-spawn-guard-v2

Conversation

@smurilosrocha

Copy link
Copy Markdown

What

The durable net over upstream's own t_7b0df4cf/#122222 fix. Upstream main already carries the pin fix itself (committed-generation site restore f57d235748, boot through the committed selection 9ec6b94eca, worker PM boot 308072df89) — this PR adds only the guard that catches the next wipe. Purely additive (+407/−0).

  • hermes_cli/post_update.py: new step_spawn_guard in HOME_STEPS (hence also BOOT_HOME_STEPS, which excludes only sync_skills). Each leg builds its env with the same builders production uses and spawns the real argv in a subprocess — never an in-process import of cron/hermes_cli. Legs: L1 kanban worker --help (PASS = rc 0), L2 DM channel --help on the bare stripped env (PASS = rc 0), L3 cron external worker with a missing payload file (PASS = exit 1 with NO stderr traceback; any ModuleNotFoundError/No module named/traceback fails naming the leg). The L3 leg sets WORKER_MARKER exactly like the production spawn (cron/scheduler.py), so the child's PM boot is exercised too — a leg without it would skip half the real path.
  • Generation-gated, NOT revision-gated (explicit design decision): boot keys its record by installed revision (boot_bootstrap.py), but the dependency generation (facts.json) can move without a revision change (PM sync), and extending the boot identity would re-run every boot step — including the expensive SQLite guard — on a generation-only change. The step owns a per-profile spawn-guard/ record with the generation fingerprint and skips when unchanged. Recorded skips, never silent: wheel-install, no-committed-site, generation-unchanged.
  • hermes_cli/update_cmd_maint.py: _run_post_update_maintenance runs the step best-effort with a one-line report (_report_spawn_guard_after_update), loud on red.
  • tests/hermes_cli/test_post_update_spawn_guard.py: registry membership (home + boot), the three skips, pass-records-generation, failure-names-the-leg, marker fidelity, root-only mutation shape, tail reporting.

Verification (all in-turn, command + output)

  • Suites green on this head: pytest tests/hermes_cli/test_post_update_spawn_guard.py tests/hermes_cli/test_post_update.py tests/hermes_cli/test_boot_bootstrap.py tests/cron/test_restart_safe_worker.py -> 74 passed. Broader run earlier: 98 passed, 3 skipped; the only failures (test_boot_convergence.py::test_each_start_checks_pm_once_before_dispatch, 3 params) also fail on clean main, pre-existing and unrelated.
  • Wired step against a real installed tree (bare store interpreter, clean ambient PYTHONPATH, harness-only sys.path): {"ok": true, "legs": "3/3"}.
  • Mutation GUARDA_3_SPAWNS_MUTATION=root-only (rebuilds the pre-fix root-only pin): {"ok": false, "error": "L3-cron-externo: IMPORT-FAIL stderr: ... utils.py line 17 -> hermes_yaml.py line 10 -> ModuleNotFoundError: No module named 'ruamel'"} — L1/L2 pass, only L3 fails, with the exact production signature of the incident.
  • Note: the acceptance tree above predates the worker-boot series, so its child cannot self-heal. On current trees the marked L3 child additionally runs worker_bootstrap (cron/worker_bootstrap.py:39-45), so a pin-only regression heals in-child while a full wipe still fails L3 — the guard stays red iff the composed production path (pin + boot) fails, which is exactly "the next boot's worker would die".

Scope

Guard + wiring + tests only. No touch to the fix itself or to hermes_cli/kanban_pr_acceptance.py.

Supersedes #127167 (same content, pre-rebase head).

Autor: tech-lead

…tion (t_7b0df4cf)

Upstream main already carries the pin fix itself (committed_venv site restore
#f57d235748 plus the worker PM boot #308072df89) — this change adds only the
durable net that catches the next wipe:

- hermes_cli/post_update.py: new step_spawn_guard in HOME_STEPS (hence also
  BOOT_HOME_STEPS). Each leg builds its env with the same builders production
  uses and spawns the real argv in a subprocess, never an in-process import.
  L1 kanban worker --help (rc 0), L2 DM channel --help on the bare stripped
  env (rc 0), L3 cron external worker with a missing payload (PASS = exit 1
  with NO stderr traceback; any ModuleNotFoundError/No module named/traceback
  fails naming the leg).
- Generation-gated, not revision-gated: the step owns a per-profile
  spawn-guard/ record with the facts.json fingerprint and skips when
  unchanged (boot keys by revision; a PM sync can move the generation
  without a revision change, and must not rerun every boot step). Recorded
  skips: wheel-install, no-committed-site, generation-unchanged.
- hermes_cli/update_cmd_maint.py: _run_post_update_maintenance runs the step
  best-effort with a one-line report, loud on red.
- tests/hermes_cli/test_post_update_spawn_guard.py: registry, skips,
  verdicts, root-only mutation shape, tail reporting.

Autor: tech-lead
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Sep 29, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

P1 — the guard's own entry gate returns ok on the exact state it exists to catch.

step_spawn_guard decides whether anything needs verifying by probing the production pin and inspecting the shape of its PYTHONPATH (hermes_cli/post_update.py:469-473):

probe = pin_hermes_tree_on_pythonpath({}, root)
if "PYTHONPATH" not in probe:
    return {"ok": True, "skipped": "wheel-install"}
if probe["PYTHONPATH"] == str(root):
    return {"ok": True, "skipped": "no-committed-site"}

But _committed_dependency_site_packages (cron/scheduler_worker_env.py) answers "pin the tree only" whenever the committed generation's site-packages is not a directory. The pin then collapses to root-only — precisely the string compared on line 473. So the step returns no-committed-site and spawns zero legs exactly when the guard is needed most.

That state isn't hypothetical: cron/worker_bootstrap.py records it — "the PM collector may remove it between the gateway's exit and the worker's next import (#122290 review)" — generation gone under a live install, facts.json intact. The store interpreter owns no third-party deps, so the next external cron worker dies with No module named 'ruamel': the exact wipe this PR guards.

I drove the real head step_spawn_guard (only the three legs stubbed, as the PR's own tests do) against a sandboxed install with pm.environments.installs_root redirected:

state ok result legs
generation site present (control) true 3/3 3
site deleted, facts.json byte-identical true no-committed-site 0
site deleted, no record yet true no-committed-site 0
facts.json names a generation outside the install true no-committed-site 0

Row 1 is the positive control; rows 2-4 report ok having verified nothing. In the last row _recorded_venv raises and the pin swallows it, so an error state reads as a clean skip too.

The root-only fallback conflates never committed (dev checkout) with committed and now gone. _committed_dependency_site_packages already distinguishes them internally (environment is None vs selected.is_dir() false), but the gate never asks.

tests/hermes_cli/test_post_update_spawn_guard.py:41-47 monkeypatches the pin to root-only and asserts the skip, pinning the conflated branch as intended.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have 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