fix(gateway): warn on stale control UI bundle - #242
Conversation
Gateway boot and agentos doctor warn when the bundled React control UI predates the frontend sources in a checkout (gateway.control_ui.dist_stale). Wheel installs ship no frontend sources and stay silent.
Review follow-up on the stale-bundle warning. The detection was right; how it was wired made it unable to change its mind, and it leaked into unrelated tests. The verdict was memoized for the process lifetime. Gateway boot calls it once so caching bought nothing there, but doctor.status is a long-lived RPC: an operator ran the rebuild the FixStep told them to run, re-ran `agentos doctor`, and got the same warning — until they restarted the gateway. The evidence contradicted itself too, because bundleMtime was re-stat'd while stale and sourceMtime came from the cache: doctor use-agent-os#1: stale=True sourceMtime=...300 bundleMtime=...200 doctor use-agent-os#2: stale=True sourceMtime=...300 bundleMtime=...400 Nothing is memoized now, and inspect_control_ui_bundle() returns one report computed in a single pass, so the three fields can no longer disagree. The collector also broke two existing doctor tests. It read the real repo and was not stubbed by _patch_ready_support_surfaces, so a plain `git checkout` — which rewrites frontend source mtimes without touching the gitignored dist/ — leaked a degrades finding into tests asserting exact impact counts. CI only stayed green because ci.yml builds the Control UI immediately before pytest. The helper is stubbed now, and the finding is readiness_impact="optional" rather than "degrades": mtimes are a hint, and a heuristic that git checkout can trip must not turn the whole report yellow. Also: - Moved to health/control_ui.py. The top-level module existed to dodge the import contract, since health has no approved outgoing edge to gateway. Under health/ the evaluator imports it intra-package and the gateway reaches it over the already-approved ("gateway", "health") edge — no dodge, and the mirrored DIST_REL constant is gone because both gateway callers already own _DIST_DIR and now pass the bundle path in. - The doctor collector takes ctx and honours config.control_ui.enabled, which boot already did. No more warning about a bundle nobody serves. - The walk starts at frontend/ instead of a named allowlist, which makes the node_modules/dist prune load-bearing (it pruned nothing before, since the walk roots were frontend/src and frontend/public) and picks up inputs the list had missed: eslint.config.js, components.json, and scripts/check-bundle-budget.mjs, which npm run build invokes. - checkout_root() looks for the checkout markers instead of counting parent levels, so relocating the module cannot silently retarget it above site-packages. - The boot ordering (missing outranks stale) lives in control_ui_boot_warning so it is testable without standing up a gateway; both consumers now have coverage, and a guard test keeps boot.py going through the helper. - CHANGELOG blank line, `key != "stale"`, and the redundant lambda. Gate: build_control_ui, npm check (1738), ruff, mypy (592), pytest (7349), uv build --wheel.
|
Thanks for taking this over and carrying the original commit. Appreciate you keeping the authorship on it. Read through #242. The detection idea survived, but the wiring is yours. #238 had the right instinct and the wrong mechanics. The memoized verdict was a real bug:
Synced local Thanks again. |
Supersedes #238, which conflicts with
mainafter #239/#240 landed and couldnot be updated in place (pushes to that fork are rejected despite
maintainer_can_modify). The original commit is carried here unchanged, so@Callmedas69 keeps authorship — please merge with rebase, not squash.
What
#238's detection is right: nothing signalled that
git pullhad left thebundled Control UI older than the frontend sources. This keeps that and fixes
how it was wired.
The verdict could not change its mind
frontend_input_mtime/control_ui_is_stalewere@functools.cached.Gateway boot calls them once so caching bought nothing there, but
doctor.statusis a long-lived RPC. An operator ran the rebuild theFixSteptold them to run, re-ran
agentos doctor, and got the same warning — untilthey restarted the gateway. The evidence contradicted itself too, because
bundleMtimewas re-stat'd fresh whilestaleandsourceMtimecame fromthe cache:
No test could catch it: every test used a fresh
tmp_path, so no cache key wasever hit twice.
Nothing is memoized now, and
inspect_control_ui_bundle()returns one reportcomputed in a single pass, so those three fields can no longer disagree.
It broke two existing doctor tests
The collector read the real repo and wasn't stubbed by
_patch_ready_support_surfaces, so a plaingit checkout— which rewritesfrontend source mtimes without touching the gitignored
dist/— leaked adegradesfinding into tests asserting exact impact counts. CI only stayedgreen because
ci.ymlrunsbuild_control_ui.py buildimmediately beforepytest.Two fixes, both worth having: the helper is stubbed in that shared helper, and
the finding is
readiness_impact="optional"instead of"degrades". Sourcemtimes are a hint, and a heuristic
git checkoutcan trip must not turn thewhole report yellow. The finding still shows up with its rebuild step.
Also
health/control_ui.py. The top-levelagentos/control_ui_check.pyexisted to dodge the import contract —
healthhas no approved outgoing edgeto
gateway, so the evaluator couldn't reach_DIST_DIR. Underhealth/theevaluator imports it intra-package and the gateway reaches it over the
already-approved
("gateway", "health")edge. No dodge, and the mirroredDIST_RELconstant is gone: both gateway callers already own_DIST_DIRandnow pass the bundle path in, which also deletes the contract test that only
existed to police the duplication.
ctxand honoursconfig.control_ui.enabled,which boot already did. No more warning about a bundle nobody serves.
frontend/instead of a named allowlist. That makesthe
node_modules/distprune load-bearing — it pruned nothing before, sincethe walk roots were
frontend/srcandfrontend/publicwhile the realnode_modulessits atfrontend/node_modules— and picks up inputs the listhad missed:
eslint.config.js,components.json, andscripts/check-bundle-budget.mjs, whichnpm run buildinvokes viabundle:budget. An allowlist's failure mode is a false negative nobody sees.checkout_root()looks for the checkout markers instead of countingparent levels, so relocating the module can't silently retarget it at whatever
sits above
site-packages.moved into
control_ui_boot_warningso it's testable without standing up agateway, plus a guard test that keeps
boot.pygoing through the helper.key != "stale", and the redundantlambda.Validation
python scripts/build_control_ui.py buildnpm --prefix frontend run check— 1738 passeduv run ruff check src testsuv run mypy src/agentos --show-error-codes— 592 filesuv run pytest tests -q— 7349 passed, 27 skippeduv build --wheelFixes #200. Closes #238.