mirror of
https://github.com/ArchiveBox/ArchiveBox.git
synced 2026-09-14 11:06:13 +05:00
Trust ready OpenCode processes (#1862)
The rc419 DigestBox test showed that health-based replacement still
killed OpenCode during legitimate initial UI load: health became
temporarily unavailable for more than five seconds while the child was
busy, causing repeated process replacement and 502s.
This removes the health-latency heuristic entirely. ArchiveBox now
distinguishes ready from merely running, trusts a ready live owned PID,
serializes cold starts until readiness, and only replaces children that
exited or never became ready. Stale tabs still recover after an
ArchiveBox/container restart because process state resets and the
detached health wake starts a new child.
This is also a net cleanup: 38 lines removed from the prior lifecycle
implementation.
Verification:
- `uv run pytest archivebox/tests/test_opencode_agent.py -q` (19 passed)
- focused real startup/readiness/proxy/shutdown tests (5 passed)
- `uv run prek run --all-files`
- independent gpt-5.6-medium lifecycle review clean
<!-- This is an auto-generated description by cubic. -->
---
## Summary by cubic
Removes the health-latency heuristic that replaced OpenCode processes
during legitimate initial UI loads, causing 502s. Ready owned processes
are now trusted without probing, and startup health checks are bounded
so they can't run past the startup deadline.
- Only replaces children that exited or never became ready.
- Cold starts are serialized until readiness; stale tabs still recover
after a restart.
<sup>Written for commit 57e7820a0b.
Summary will update on new commits.</sup>
<a
href="https://cubic.dev/pr/ArchiveBox/ArchiveBox/pull/1862?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
This commit is contained in:
commit
79fc5c34d2
@ -38,7 +38,6 @@ _PROCESS_READY: subprocess.Popen | None = None
|
||||
_PROCESS_LOCK = threading.Lock()
|
||||
_SESSION_LOCK = threading.Lock()
|
||||
_LOGGER = logging.getLogger(__name__)
|
||||
_PROCESS_HEALTH_GRACE = 5
|
||||
_PROXY_PREFIX = "/admin/agent/opencode"
|
||||
_PROXY_PREFIX_REGEX = _PROXY_PREFIX.replace("/", r"\/")
|
||||
_PROXY_PREFIX_NO_SLASH_REGEX = _PROXY_PREFIX.lstrip("/").replace("/", r"\/")
|
||||
@ -413,23 +412,14 @@ def _ensure_opencode(settings: dict) -> tuple[bool, str]:
|
||||
workdir = settings["workdir"].resolve()
|
||||
|
||||
with _PROCESS_LOCK:
|
||||
if _owned_process_running():
|
||||
deadline = time.monotonic() + min(_PROCESS_HEALTH_GRACE, settings["timeout"])
|
||||
while _owned_process_running():
|
||||
remaining = deadline - time.monotonic()
|
||||
if remaining <= 0:
|
||||
break
|
||||
if _health(settings, timeout=min(2, remaining)):
|
||||
if time.monotonic() <= deadline:
|
||||
_PROCESS_READY = _PROCESS
|
||||
return True, ""
|
||||
break
|
||||
remaining = deadline - time.monotonic()
|
||||
if remaining > 0:
|
||||
time.sleep(min(0.25, remaining))
|
||||
_stop_owned_process(_PROCESS)
|
||||
elif _health(settings):
|
||||
if _owned_process_ready():
|
||||
return True, ""
|
||||
if _health(settings):
|
||||
if _owned_process_running():
|
||||
_PROCESS_READY = _PROCESS
|
||||
return True, ""
|
||||
if _owned_process_running():
|
||||
_stop_owned_process(_PROCESS)
|
||||
|
||||
try:
|
||||
binary, git_binary, binary_env = _resolve_binary(
|
||||
@ -511,17 +501,24 @@ def _ensure_opencode(settings: dict) -> tuple[bool, str]:
|
||||
return False, f"OpenCode binary not found: {settings['binary']}"
|
||||
|
||||
deadline = time.monotonic() + settings["timeout"]
|
||||
while time.monotonic() < deadline:
|
||||
if _health(settings):
|
||||
_PROCESS_READY = started_process
|
||||
return True, ""
|
||||
while True:
|
||||
remaining = deadline - time.monotonic()
|
||||
if remaining <= 0:
|
||||
break
|
||||
if _health(settings, timeout=min(2, remaining)):
|
||||
if time.monotonic() <= deadline:
|
||||
_PROCESS_READY = started_process
|
||||
return True, ""
|
||||
break
|
||||
if started_process and started_process.poll() is not None:
|
||||
if _PROCESS is started_process:
|
||||
_PROCESS = None
|
||||
if _PROCESS_READY is started_process:
|
||||
_PROCESS_READY = None
|
||||
return False, "OpenCode exited before the web server became ready."
|
||||
time.sleep(0.25)
|
||||
remaining = deadline - time.monotonic()
|
||||
if remaining > 0:
|
||||
time.sleep(min(0.25, remaining))
|
||||
|
||||
_stop_owned_process(started_process)
|
||||
return False, "Timed out waiting for OpenCode to start."
|
||||
|
||||
@ -3,7 +3,6 @@ import os
|
||||
import signal
|
||||
import socket
|
||||
import subprocess
|
||||
import time
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
@ -128,13 +127,14 @@ def test_opencode_disabled_route_does_not_start_server(client, initialized_archi
|
||||
assert views._PROCESS is None or views._PROCESS.poll() is not None
|
||||
|
||||
|
||||
def test_stop_owned_process_falls_back_when_process_has_no_dedicated_group():
|
||||
def test_stop_owned_process_falls_back_for_stopped_process_without_dedicated_group():
|
||||
from archivebox.opencode import views
|
||||
|
||||
process = subprocess.Popen(["sleep", "60"])
|
||||
try:
|
||||
process.send_signal(signal.SIGSTOP)
|
||||
views._stop_owned_process(process)
|
||||
assert process.poll() is not None
|
||||
assert process.returncode == -signal.SIGTERM
|
||||
finally:
|
||||
if process.poll() is None:
|
||||
process.kill()
|
||||
@ -287,47 +287,19 @@ def test_concurrent_opencode_startup_waits_until_server_is_ready(live_opencode):
|
||||
assert views._health(live_opencode.settings)
|
||||
|
||||
|
||||
def test_opencode_restarts_an_unhealthy_owned_process(live_opencode):
|
||||
def test_opencode_does_not_probe_or_replace_a_ready_owned_process(live_opencode):
|
||||
from archivebox.opencode import views
|
||||
|
||||
old_process = views._PROCESS
|
||||
process = views._PROCESS
|
||||
settings = {**live_opencode.settings, "port": _free_port()}
|
||||
settings["origin"] = f"http://{settings['host']}:{settings['port']}"
|
||||
|
||||
ok, error = views._ensure_opencode(settings)
|
||||
|
||||
assert ok, error
|
||||
assert old_process is not None
|
||||
assert old_process.poll() is not None
|
||||
assert views._PROCESS is not old_process
|
||||
assert views._health(settings)
|
||||
|
||||
|
||||
def test_opencode_preserves_a_transiently_unhealthy_owned_process(live_opencode):
|
||||
from archivebox.opencode import views
|
||||
|
||||
process = views._PROCESS
|
||||
assert process is not None
|
||||
views._signal_owned_process(process, signal.SIGSTOP)
|
||||
|
||||
def resume_process():
|
||||
time.sleep(1.5)
|
||||
views._signal_owned_process(process, signal.SIGCONT)
|
||||
|
||||
try:
|
||||
with ThreadPoolExecutor(max_workers=1) as executor:
|
||||
resumed = executor.submit(resume_process)
|
||||
started_at = time.monotonic()
|
||||
ok, error = views._ensure_opencode(live_opencode.settings)
|
||||
elapsed = time.monotonic() - started_at
|
||||
resumed.result()
|
||||
finally:
|
||||
views._signal_owned_process(process, signal.SIGCONT)
|
||||
|
||||
assert ok, error
|
||||
assert views._PROCESS is process
|
||||
assert process.poll() is None
|
||||
assert elapsed < views._PROCESS_HEALTH_GRACE
|
||||
|
||||
|
||||
def test_opencode_proxy_does_not_wait_for_recovery_lock(admin_client, live_opencode):
|
||||
|
||||
Loading…
Reference in New Issue
Block a user