mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-05-26 06:01:49 +00:00
refactor(tui): simplify TUI build logic, remove stale staleness checks
The old mtime-tracking staleness machinery (_tui_build_needed, _hermes_ink_bundle_stale, _find_bundled_tui) tried to avoid rebuilding by comparing source timestamps to dist/entry.js. This was fragile and added ~100 lines of code. Replace with three clear paths: 1. HERMES_TUI_DIR set (prebuilt/nix): just node dist/entry.js, no build 2. --dev mode: tsx src/entry.tsx, no build, hot reload 3. Normal: always npm run build (esbuild is ~1s, correctness > caching) Also error when HERMES_TUI_DIR is set with --dev (footgun: prebuilt bundle has no source code to hot-reload).
This commit is contained in:
parent
3197b4de6d
commit
c6ca11618a
2 changed files with 33 additions and 138 deletions
|
|
@ -967,66 +967,6 @@ def _tui_need_npm_install(root: Path) -> bool:
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
|
||||||
def _find_bundled_tui(tui_dir: Path) -> Optional[Path]:
|
|
||||||
"""Directory whose dist/entry.js we should run: HERMES_TUI_DIR first, else repo ui-tui."""
|
|
||||||
env = os.environ.get("HERMES_TUI_DIR")
|
|
||||||
if env:
|
|
||||||
p = Path(env)
|
|
||||||
if (p / "dist" / "entry.js").exists() and not _tui_need_npm_install(p):
|
|
||||||
return p
|
|
||||||
if (tui_dir / "dist" / "entry.js").exists() and not _tui_need_npm_install(tui_dir):
|
|
||||||
return tui_dir
|
|
||||||
return None
|
|
||||||
|
|
||||||
|
|
||||||
def _tui_build_needed(tui_dir: Path) -> bool:
|
|
||||||
# esbuild bundles @hermes/ink + source directly into dist/entry.js;
|
|
||||||
# the old ink-bundle.js check only fires when entry.js hasn't been
|
|
||||||
# produced yet (a dev checkout that hasn't been built at all). Once
|
|
||||||
# entry.js exists, the mtime walk below covers all source trees.
|
|
||||||
entry = tui_dir / "dist" / "entry.js"
|
|
||||||
if not entry.exists():
|
|
||||||
# Nothing built yet — signal that a build is needed.
|
|
||||||
return True
|
|
||||||
dist_m = entry.stat().st_mtime
|
|
||||||
skip = frozenset({"node_modules", "dist"})
|
|
||||||
for dirpath, dirnames, filenames in os.walk(tui_dir, topdown=True):
|
|
||||||
dirnames[:] = [d for d in dirnames if d not in skip]
|
|
||||||
for fn in filenames:
|
|
||||||
if fn.endswith((".ts", ".tsx")):
|
|
||||||
if os.path.getmtime(os.path.join(dirpath, fn)) > dist_m:
|
|
||||||
return True
|
|
||||||
for meta in (
|
|
||||||
"package.json",
|
|
||||||
"package-lock.json",
|
|
||||||
"tsconfig.json",
|
|
||||||
"tsconfig.build.json",
|
|
||||||
):
|
|
||||||
mp = tui_dir / meta
|
|
||||||
if mp.exists() and mp.stat().st_mtime > dist_m:
|
|
||||||
return True
|
|
||||||
return False
|
|
||||||
|
|
||||||
|
|
||||||
def _hermes_ink_bundle_stale(tui_dir: Path) -> bool:
|
|
||||||
ink_root = tui_dir / "packages" / "hermes-ink"
|
|
||||||
bundle = ink_root / "dist" / "ink-bundle.js"
|
|
||||||
if not bundle.exists():
|
|
||||||
return True
|
|
||||||
bm = bundle.stat().st_mtime
|
|
||||||
skip = frozenset({"node_modules", "dist"})
|
|
||||||
for dirpath, dirnames, filenames in os.walk(ink_root, topdown=True):
|
|
||||||
dirnames[:] = [d for d in dirnames if d not in skip]
|
|
||||||
for fn in filenames:
|
|
||||||
if fn.endswith((".ts", ".tsx")):
|
|
||||||
if os.path.getmtime(os.path.join(dirpath, fn)) > bm:
|
|
||||||
return True
|
|
||||||
mp = ink_root / "package.json"
|
|
||||||
if mp.exists() and mp.stat().st_mtime > bm:
|
|
||||||
return True
|
|
||||||
return False
|
|
||||||
|
|
||||||
|
|
||||||
def _ensure_tui_node() -> None:
|
def _ensure_tui_node() -> None:
|
||||||
"""Make sure `node` + `npm` are on PATH for the TUI.
|
"""Make sure `node` + `npm` are on PATH for the TUI.
|
||||||
|
|
||||||
|
|
@ -1085,7 +1025,7 @@ def _ensure_tui_node() -> None:
|
||||||
|
|
||||||
|
|
||||||
def _make_tui_argv(tui_dir: Path, tui_dev: bool) -> tuple[list[str], Path]:
|
def _make_tui_argv(tui_dir: Path, tui_dev: bool) -> tuple[list[str], Path]:
|
||||||
"""TUI: --dev → tsx src; else node dist (HERMES_TUI_DIR or ui-tui, build when stale)."""
|
"""TUI: --dev → tsx src; else node dist (HERMES_TUI_DIR prebuilt or esbuild)."""
|
||||||
_ensure_tui_node()
|
_ensure_tui_node()
|
||||||
|
|
||||||
def _node_bin(bin: str) -> str:
|
def _node_bin(bin: str) -> str:
|
||||||
|
|
@ -1099,23 +1039,31 @@ def _make_tui_argv(tui_dir: Path, tui_dev: bool) -> tuple[list[str], Path]:
|
||||||
sys.exit(1)
|
sys.exit(1)
|
||||||
return path
|
return path
|
||||||
|
|
||||||
# pre-built dist + node_modules (nix / full HERMES_TUI_DIR) skips npm.
|
# Footgun: --dev against a prebuilt bundle that has no source/node_modules.
|
||||||
|
ext_dir = os.environ.get("HERMES_TUI_DIR")
|
||||||
|
if tui_dev and ext_dir:
|
||||||
|
print(
|
||||||
|
f"Error: --dev is incompatible with HERMES_TUI_DIR={ext_dir}\n"
|
||||||
|
f"The prebuilt TUI has no source code to hot-reload.\n"
|
||||||
|
f"Unset HERMES_TUI_DIR (e.g. `unset HERMES_TUI_DIR`) to use --dev from a checkout.",
|
||||||
|
file=sys.stderr,
|
||||||
|
)
|
||||||
|
sys.exit(1)
|
||||||
|
|
||||||
|
# 1. Prebuilt bundle (nix / packaged release): just run it.
|
||||||
if not tui_dev:
|
if not tui_dev:
|
||||||
ext_dir = os.environ.get("HERMES_TUI_DIR")
|
|
||||||
if ext_dir:
|
if ext_dir:
|
||||||
p = Path(ext_dir)
|
p = Path(ext_dir)
|
||||||
if (p / "dist" / "entry.js").exists() and not _tui_need_npm_install(p):
|
if (p / "dist" / "entry.js").is_file():
|
||||||
node = _node_bin("node")
|
node = _node_bin("node")
|
||||||
return [node, str(p / "dist" / "entry.js")], p
|
return [node, str(p / "dist" / "entry.js")], p
|
||||||
|
|
||||||
npm = _node_bin("npm")
|
# 2. Normal flow: npm install if needed, always esbuild, then node dist/entry.js.
|
||||||
|
# --dev flow: npm install if needed, then tsx src/entry.tsx (no build).
|
||||||
if _tui_need_npm_install(tui_dir):
|
if _tui_need_npm_install(tui_dir):
|
||||||
|
npm = _node_bin("npm")
|
||||||
if not os.environ.get("HERMES_QUIET"):
|
if not os.environ.get("HERMES_QUIET"):
|
||||||
print("Installing TUI dependencies…")
|
print("Installing TUI dependencies…")
|
||||||
# Capture stdout as well as stderr — some npm errors (notably EACCES on a
|
|
||||||
# root-owned node_modules in containers) are emitted on stdout, and a
|
|
||||||
# bare "npm install failed." with no preview defeats debugging. We keep
|
|
||||||
# the failure-only print path so a successful install stays silent.
|
|
||||||
result = subprocess.run(
|
result = subprocess.run(
|
||||||
[npm, "install", "--silent", "--no-fund", "--no-audit", "--progress=false"],
|
[npm, "install", "--silent", "--no-fund", "--no-audit", "--progress=false"],
|
||||||
cwd=str(tui_dir),
|
cwd=str(tui_dir),
|
||||||
|
|
@ -1133,47 +1081,30 @@ def _make_tui_argv(tui_dir: Path, tui_dev: bool) -> tuple[list[str], Path]:
|
||||||
sys.exit(1)
|
sys.exit(1)
|
||||||
|
|
||||||
if tui_dev:
|
if tui_dev:
|
||||||
if _hermes_ink_bundle_stale(tui_dir):
|
|
||||||
result = subprocess.run(
|
|
||||||
[npm, "run", "build", "--prefix", "packages/hermes-ink"],
|
|
||||||
cwd=str(tui_dir),
|
|
||||||
capture_output=True,
|
|
||||||
text=True,
|
|
||||||
)
|
|
||||||
if result.returncode != 0:
|
|
||||||
combined = f"{result.stdout or ''}{result.stderr or ''}".strip()
|
|
||||||
preview = "\n".join(combined.splitlines()[-30:])
|
|
||||||
print("@hermes/ink build failed.")
|
|
||||||
if preview:
|
|
||||||
print(preview)
|
|
||||||
sys.exit(1)
|
|
||||||
tsx = tui_dir / "node_modules" / ".bin" / "tsx"
|
tsx = tui_dir / "node_modules" / ".bin" / "tsx"
|
||||||
if tsx.exists():
|
if tsx.exists():
|
||||||
return [str(tsx), "src/entry.tsx"], tui_dir
|
return [str(tsx), "src/entry.tsx"], tui_dir
|
||||||
|
npm = _node_bin("npm")
|
||||||
return [npm, "start"], tui_dir
|
return [npm, "start"], tui_dir
|
||||||
|
|
||||||
if _tui_build_needed(tui_dir):
|
# Always rebuild — esbuild is fast and this avoids staleness-edge-case bugs.
|
||||||
result = subprocess.run(
|
npm = _node_bin("npm")
|
||||||
[npm, "run", "build"],
|
result = subprocess.run(
|
||||||
cwd=str(tui_dir),
|
[npm, "run", "build"],
|
||||||
capture_output=True,
|
cwd=str(tui_dir),
|
||||||
text=True,
|
capture_output=True,
|
||||||
)
|
text=True,
|
||||||
if result.returncode != 0:
|
)
|
||||||
combined = f"{result.stdout or ''}{result.stderr or ''}".strip()
|
if result.returncode != 0:
|
||||||
preview = "\n".join(combined.splitlines()[-30:])
|
combined = f"{result.stdout or ''}{result.stderr or ''}".strip()
|
||||||
print("TUI build failed.")
|
preview = "\n".join(combined.splitlines()[-30:])
|
||||||
if preview:
|
print("TUI build failed.")
|
||||||
print(preview)
|
if preview:
|
||||||
sys.exit(1)
|
print(preview)
|
||||||
|
|
||||||
root = _find_bundled_tui(tui_dir)
|
|
||||||
if not root:
|
|
||||||
print("TUI build did not produce dist/entry.js")
|
|
||||||
sys.exit(1)
|
sys.exit(1)
|
||||||
|
|
||||||
node = _node_bin("node")
|
node = _node_bin("node")
|
||||||
return [node, str(root / "dist" / "entry.js")], root
|
return [node, str(tui_dir / "dist" / "entry.js")], tui_dir
|
||||||
|
|
||||||
|
|
||||||
def _normalize_tui_toolsets(toolsets: object) -> list[str]:
|
def _normalize_tui_toolsets(toolsets: object) -> list[str]:
|
||||||
|
|
@ -5503,7 +5434,6 @@ def _gateway_prompt(prompt_text: str, default: str = "", timeout: float = 300.0)
|
||||||
def _web_ui_build_needed(web_dir: Path) -> bool:
|
def _web_ui_build_needed(web_dir: Path) -> bool:
|
||||||
"""Return True if the web UI dist is missing or stale.
|
"""Return True if the web UI dist is missing or stale.
|
||||||
|
|
||||||
Mirrors the staleness logic used by ``_tui_build_needed()`` for the TUI.
|
|
||||||
The Vite build outputs to ``hermes_cli/web_dist/`` (per vite.config.ts
|
The Vite build outputs to ``hermes_cli/web_dist/`` (per vite.config.ts
|
||||||
outDir: "../hermes_cli/web_dist"), NOT to ``web/dist/``. Uses the Vite
|
outDir: "../hermes_cli/web_dist"), NOT to ``web/dist/``. Uses the Vite
|
||||||
manifest as the sentinel because it is written last and therefore has the
|
manifest as the sentinel because it is written last and therefore has the
|
||||||
|
|
|
||||||
|
|
@ -25,12 +25,6 @@ def _touch_tui_entry(root: Path) -> None:
|
||||||
entry.write_text("console.log('tui')")
|
entry.write_text("console.log('tui')")
|
||||||
|
|
||||||
|
|
||||||
def _touch_ink_bundle(root: Path) -> None:
|
|
||||||
bundle = root / "packages" / "hermes-ink" / "dist" / "ink-bundle.js"
|
|
||||||
bundle.parent.mkdir(parents=True, exist_ok=True)
|
|
||||||
bundle.write_text("export {}")
|
|
||||||
|
|
||||||
|
|
||||||
def test_need_install_when_ink_missing(tmp_path: Path, main_mod) -> None:
|
def test_need_install_when_ink_missing(tmp_path: Path, main_mod) -> None:
|
||||||
(tmp_path / "package-lock.json").write_text("{}")
|
(tmp_path / "package-lock.json").write_text("{}")
|
||||||
assert main_mod._tui_need_npm_install(tmp_path) is True
|
assert main_mod._tui_need_npm_install(tmp_path) is True
|
||||||
|
|
@ -126,32 +120,3 @@ def test_no_install_prebuilt_bundle_mode(tmp_path: Path, main_mod) -> None:
|
||||||
"""dist/entry.js present and no package-lock.json → prebuilt bundle, skip npm install."""
|
"""dist/entry.js present and no package-lock.json → prebuilt bundle, skip npm install."""
|
||||||
_touch_tui_entry(tmp_path)
|
_touch_tui_entry(tmp_path)
|
||||||
assert main_mod._tui_need_npm_install(tmp_path) is False
|
assert main_mod._tui_need_npm_install(tmp_path) is False
|
||||||
|
|
||||||
|
|
||||||
def test_build_needed_when_source_newer_than_entry(tmp_path: Path, main_mod) -> None:
|
|
||||||
_touch_tui_entry(tmp_path)
|
|
||||||
_touch_ink(tmp_path)
|
|
||||||
src = tmp_path / "src" / "entry.tsx"
|
|
||||||
src.parent.mkdir(parents=True, exist_ok=True)
|
|
||||||
src.write_text("console.log('newer')")
|
|
||||||
os.utime(src, (200, 200))
|
|
||||||
os.utime(tmp_path / "dist" / "entry.js", (100, 100))
|
|
||||||
|
|
||||||
assert main_mod._tui_need_npm_install(tmp_path) is False
|
|
||||||
assert main_mod._tui_build_needed(tmp_path) is True
|
|
||||||
|
|
||||||
|
|
||||||
def test_build_not_needed_when_entry_exists_and_sources_unchanged(tmp_path: Path, main_mod) -> None:
|
|
||||||
_touch_tui_entry(tmp_path)
|
|
||||||
_touch_ink(tmp_path)
|
|
||||||
|
|
||||||
assert main_mod._tui_need_npm_install(tmp_path) is False
|
|
||||||
assert main_mod._tui_build_needed(tmp_path) is False
|
|
||||||
|
|
||||||
|
|
||||||
def test_build_not_needed_when_entry_and_ink_bundle_present(tmp_path: Path, main_mod) -> None:
|
|
||||||
_touch_tui_entry(tmp_path)
|
|
||||||
_touch_ink(tmp_path)
|
|
||||||
_touch_ink_bundle(tmp_path)
|
|
||||||
|
|
||||||
assert main_mod._tui_build_needed(tmp_path) is False
|
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue