diff --git a/config/settings.py b/config/settings.py index 541d751b8..ee65e1379 100644 --- a/config/settings.py +++ b/config/settings.py @@ -490,6 +490,38 @@ class TimeoutSettings(BaseModel): "Env: TIMEOUTS__CATCHUP_DISABLED_WARN_HOURS." ), ) + beacon_settle_timeout_s: float = Field( + default=90.0, + ge=0.0, + le=600.0, + description=( + "Settle window (seconds) the release verify waits on a process " + "that is mid-boot -- exec'd, but not yet at its boot-beacon write " + "(scripts/update/service.py::verify_running_release_settled). A " + "bridge or worker restarted while `/update` runs presents exactly " + "like a broken beacon (missing, or orphaned by the previous " + "image), so the verify reports `unknown` and masks both a genuine " + "`stale` and a clean `matches`. Only a LIVE process younger than " + "this window is waited on; every other `unknown` is terminal and " + "returns immediately. GRAIN OF SALT: provisional/tunable. The " + "bridge writes its beacon after the Telegram connect (~30s after " + "exec on this fleet); this is that plus headroom for a slow " + "catchup, and it doubles as the cap on how long a never-arriving " + "beacon can stall the run. Env: TIMEOUTS__BEACON_SETTLE_TIMEOUT_S." + ), + ) + beacon_settle_interval_s: float = Field( + default=3.0, + gt=0.0, + le=60.0, + description=( + "Re-classification interval (seconds) inside the boot-beacon " + "settle window above. GRAIN OF SALT: provisional/tunable -- fine " + "enough to exit promptly once the beacon lands, coarse enough not " + "to spin `git log` per second. Env: " + "TIMEOUTS__BEACON_SETTLE_INTERVAL_S." + ), + ) class HybridEvalSettings(BaseModel): diff --git a/docs/features/bridge-self-healing.md b/docs/features/bridge-self-healing.md index 355103ee8..7d00ce68e 100644 --- a/docs/features/bridge-self-healing.md +++ b/docs/features/bridge-self-healing.md @@ -549,6 +549,8 @@ Four coordinated pieces verify that the running processes actually execute the p | `stale` | beacon belongs to the current image AND that relevant-range log is non-empty | | `unknown` | beacon missing/malformed, no PID, `process_start_ts` unavailable, an orphaned beacon (`beacon_ts <= process_start_ts`), or `boot_sha` unresolvable by git | +**Boot-window settle poll** (`scripts/update/service.py::verify_running_release_settled`): the classification table above is read at one instant, and a process that is *mid-boot* at that instant — exec'd, but not yet at its beacon write — presents exactly like a broken beacon (missing, or orphaned by the previous image). Before classifying, the verify re-polls any process whose `unknown` is mid-boot (running, start ts readable, exec'd less than `settings.timeouts.beacon_settle_timeout_s` ago, beacon absent or orphaned) every `settings.timeouts.beacon_settle_interval_s` until it resolves or the window elapses. Both are provisional and env-overridable (`TIMEOUTS__BEACON_SETTLE_TIMEOUT_S`, `TIMEOUTS__BEACON_SETTLE_INTERVAL_S`). This is the generalization of the worker's `--since` poll: it covers a restart by *any* actor — this run's own kickstart, the watchdog's recovery chain, a plain launchd relaunch — not just the ones able to set the planned-restart skip signal, which is why a bridge restarting outside the update's control no longer degrades the verdict to `unknown`. Terminal unknowns (no PID, unreadable start ts, a long-running process with no beacon, an unresolvable `boot_sha`) return immediately and never burn the window. + Staleness is positive-only and scoped to each process's own relevant path set (bridge: `bridge/ agent/ mcp_servers/ models/ tools/ config/ pyproject.toml`; worker: `worker/ agent/ mcp_servers/ models/ tools/ bridge/ reflections/ pyproject.toml`), the same sets the restart gates diff, so classifier and restart gate agree by construction. A raw `boot_sha == HEAD` comparison is never used: docs-only or plan-migration commits advance HEAD past a healthy, correctly-un-restarted process, and a literal-equality check would false-fail on the majority of this repo's commit stream. `unknown` never fails a run and never triggers a restart. Only a positive, confirmed staleness escalates. **Bridge kickstart in `remote-update.sh`**: After the pull and the worker kickstart, the shell computes `NEED_BRIDGE_RESTART` from a `BEFORE_SHA..AFTER_SHA` diff of the bridge-relevant paths, gated on the bridge plist being installed on this machine (`[ -f "$BRIDGE_DST" ]`; a skills-only machine has no bridge plist and skips the block entirely). When true, it runs `launchctl kickstart -k {prefix}.bridge` as the **last** thing the script does. This is safe because the bridge holds no agent sessions (the worker is the sole session executor) and its Telethon `catch_up=True` scan backfills anything missed during the brief restart. It is the last act because the kickstart SIGKILLs the whole bridge launchd job, including `handle_update_command` and the `remote-update.sh` child it spawned, since they share the job's process group. Nothing in the shell runs after a successful kickstart. Both worker and bridge kickstart failures surface as a distinct `RESTART FAILED` line and a non-zero terminal exit (`RESTART_FAILED || VERIFY_FAILED`). diff --git a/scripts/update/run.py b/scripts/update/run.py index dbeaf7de6..2023a099b 100644 --- a/scripts/update/run.py +++ b/scripts/update/run.py @@ -800,7 +800,9 @@ def run_release_verify( """ try: head_short = git.get_short_sha(project_dir) - release_check = service.verify_running_release(project_dir, head_short, machine_check) + release_check = service.verify_running_release_settled( + project_dir, head_short, machine_check + ) # Self-heal a stale worker in place before alerting (issues #2400/#2220). worker_info = release_check.get("worker") diff --git a/scripts/update/service.py b/scripts/update/service.py index 49756cb1a..9a1dd9ae6 100644 --- a/scripts/update/service.py +++ b/scripts/update/service.py @@ -1218,14 +1218,18 @@ def _classify_process( "process_start_ts": None, "classification": "unknown", } + # Read the process start ts FIRST so it is present even when the beacon is + # missing — the settle poll needs the process's age to tell a mid-boot + # process (worth waiting for) from a long-running one with no beacon. + if pid is not None: + result["process_start_ts"] = get_process_start_ts(pid) beacon = read_boot_beacon(project_dir / "data" / f"{process_name}_boot_sha") if beacon is None: return result result["boot_sha"], result["beacon_ts"] = beacon if pid is None: return result - start_ts = get_process_start_ts(pid) - result["process_start_ts"] = start_ts + start_ts = result["process_start_ts"] if start_ts is None: return result if result["beacon_ts"] <= start_ts: @@ -1274,3 +1278,71 @@ def verify_running_release(project_dir: Path, head_sha: str, machine_check: dict project_dir, head_sha, "worker", get_worker_pid(), WORKER_RELEVANT_PATHS ) return results + + +def _is_mid_boot_unknown(info: dict, now: float, timeout_s: float) -> bool: + """True when this ``unknown`` is a live process that has not booted yet. + + A mid-boot unknown is: a running process, whose start ts is readable, that + started less than ``timeout_s`` ago, and whose beacon is either absent or + orphaned (written by the previous image). Every other ``unknown`` — no PID, + unreadable start ts, an old process with no beacon, an unresolvable boot + SHA — is terminal, and polling it would only burn the window. + """ + if info.get("classification") != "unknown" or not info.get("running"): + return False + start_ts = info.get("process_start_ts") + if start_ts is None or (now - start_ts) >= timeout_s: + return False + beacon_ts = info.get("beacon_ts") + return beacon_ts is None or beacon_ts <= start_ts + + +def verify_running_release_settled( + project_dir: Path, + head_sha: str, + machine_check: dict, + settle_skip: tuple[str, ...] = (), + timeout_s: float | None = None, + interval_s: float | None = None, +) -> dict: + """:func:`verify_running_release`, re-polled past a process's boot window. + + Same return shape. While any classified process is a *mid-boot* ``unknown`` + (see :func:`_is_mid_boot_unknown`), re-classify every + ``settings.timeouts.beacon_settle_interval_s`` until it resolves or + ``settings.timeouts.beacon_settle_timeout_s`` elapses — so a bridge or worker that was + restarting when /update reached its verify step lands on a real + ``matches``/``stale`` verdict instead of an unactionable ``unknown``. + + ``settle_skip`` names processes never waited on (a deliberately + about-to-restart bridge under ``--skip-bridge``: its verdict is discarded + anyway, so waiting for it buys nothing). A terminal unknown returns + immediately — this never sleeps on a beacon that cannot arrive. + """ + if timeout_s is None or interval_s is None: + # Read at call time, never at import (TimeoutSettings, #1968): both + # knobs are env-overridable via TIMEOUTS__BEACON_SETTLE_* and + # provisional. + from config.settings import settings # noqa: PLC0415 + + if timeout_s is None: + timeout_s = settings.timeouts.beacon_settle_timeout_s + if interval_s is None: + interval_s = settings.timeouts.beacon_settle_interval_s + deadline = time.monotonic() + timeout_s + + def _needs_settle(results: dict) -> bool: + now = time.time() + return any( + _is_mid_boot_unknown(info, now, timeout_s) + for name, info in results.items() + if name not in settle_skip + ) + + results = verify_running_release(project_dir, head_sha, machine_check) + while _needs_settle(results) and time.monotonic() < deadline: + logger.info("verify_running_release: process still mid-boot — polling for a fresh beacon") + time.sleep(interval_s) + results = verify_running_release(project_dir, head_sha, machine_check) + return results diff --git a/scripts/update/verify_release.py b/scripts/update/verify_release.py index 904b1c6ea..14aabff6e 100644 --- a/scripts/update/verify_release.py +++ b/scripts/update/verify_release.py @@ -22,6 +22,13 @@ the Race 1 mitigation. A beacon that never freshens past ``--since`` within the window means the worker failed to come up on new code → stale/fail. +A process that is mid-boot when this runs (its beacon is missing or belongs to +the previous image) is re-polled by +``service.verify_running_release_settled`` until it resolves or the settle +window elapses — a bridge restarting for ANY reason (this run's kickstart, the +watchdog's recovery chain, a launchd relaunch) must not degrade the verdict to +``unknown``. + ``--skip-bridge`` is passed when a bridge restart is queued this cycle (the deliberately-about-to-restart bridge must not be escalated as stale). Independently of the flag, a fresh ``data/update-restart-in-progress`` @@ -117,7 +124,15 @@ def main(argv: list[str] | None = None) -> int: if args.since > 0 and machine_check.get("projects"): forced_worker_stale = not _poll_worker_beacon(project_dir, args.since) - results = service.verify_running_release(project_dir, head_short, machine_check) + # Settle past any mid-boot process (a bridge/worker restarted by this run, + # the watchdog, or launchd) so a restart in flight cannot degrade the + # verdict to `unknown`. A skipped bridge is never waited on. + results = service.verify_running_release_settled( + project_dir, + head_short, + machine_check, + settle_skip=("bridge",) if skip_bridge else (), + ) if skip_bridge: results.pop("bridge", None) if forced_worker_stale and "worker" in results: diff --git a/tests/unit/test_update_release_verify.py b/tests/unit/test_update_release_verify.py index cd7c2ea89..e0862c343 100644 --- a/tests/unit/test_update_release_verify.py +++ b/tests/unit/test_update_release_verify.py @@ -205,6 +205,87 @@ def test_process_not_running_classifies_unknown(repo, live_processes, monkeypatc assert results["bridge"]["classification"] == "unknown" +# --------------------------------------------------------------------------- +# Boot-beacon settle poll: a process restarting while /update verifies must +# resolve to a real verdict instead of `unknown`. +# --------------------------------------------------------------------------- + + +@pytest.fixture +def booting_bridge(monkeypatch, live_processes): + """Bridge exec'd 10s ago (mid-boot); worker long-running. Sleep is captured.""" + bridge_start = time.time() - 10 + monkeypatch.setattr( + service, + "get_process_start_ts", + lambda pid: bridge_start if pid == 4242 else PROC_START_TS, + ) + sleeps: list[float] = [] + monkeypatch.setattr(service.time, "sleep", lambda s: sleeps.append(s)) + return bridge_start, sleeps + + +def test_mid_boot_bridge_settles_to_matches(repo, booting_bridge, monkeypatch): + """No beacon yet from a just-exec'd bridge: poll until it writes one.""" + bridge_start, sleeps = booting_bridge + head = get_short_sha(repo) + + def _sleep(_seconds): + sleeps.append(_seconds) + _write_beacon(repo, "bridge", head, bridge_start + 5) + + monkeypatch.setattr(service.time, "sleep", _sleep) + results = service.verify_running_release_settled(repo, head, FULL_MACHINE_CHECK) + assert results["bridge"]["classification"] == "matches" + assert len(sleeps) == 1 + + +def test_orphaned_beacon_settles_to_stale(repo, booting_bridge, monkeypatch): + """The masked verdict was STALE: settling must surface it, not swallow it.""" + bridge_start, sleeps = booting_bridge + old_sha = get_short_sha(repo) + _write_beacon(repo, "bridge", old_sha, bridge_start - 100) # previous image + _commit(repo, "bridge/new_handler.py", "bridge-relevant commit") + head = get_short_sha(repo) + + def _sleep(_seconds): + sleeps.append(_seconds) + _write_beacon(repo, "bridge", old_sha, bridge_start + 5) + + monkeypatch.setattr(service.time, "sleep", _sleep) + results = service.verify_running_release_settled(repo, head, FULL_MACHINE_CHECK) + assert results["bridge"]["classification"] == "stale" + assert len(sleeps) == 1 + + +def test_settle_gives_up_at_the_timeout(repo, booting_bridge): + """A beacon that never arrives still returns — unknown, bounded.""" + _, sleeps = booting_bridge + results = service.verify_running_release_settled( + repo, get_short_sha(repo), FULL_MACHINE_CHECK, timeout_s=0.05, interval_s=0 + ) + assert results["bridge"]["classification"] == "unknown" + + +def test_long_running_process_without_beacon_never_polls(repo, live_processes, monkeypatch): + """Terminal unknown (process far older than the settle window): no waiting.""" + sleeps: list[float] = [] + monkeypatch.setattr(service.time, "sleep", lambda s: sleeps.append(s)) + results = service.verify_running_release_settled(repo, get_short_sha(repo), FULL_MACHINE_CHECK) + assert results["bridge"]["classification"] == "unknown" + assert sleeps == [] + + +def test_settle_skip_does_not_wait_for_a_skipped_bridge(repo, booting_bridge): + """--skip-bridge discards the bridge verdict, so it must not be waited on.""" + _, sleeps = booting_bridge + _write_beacon(repo, "worker", get_short_sha(repo), PROC_START_TS + 100) + service.verify_running_release_settled( + repo, get_short_sha(repo), FULL_MACHINE_CHECK, settle_skip=("bridge",) + ) + assert sleeps == [] + + # --------------------------------------------------------------------------- # get_process_start_ts (generalized, any-PID) # ---------------------------------------------------------------------------